[Freeipa-devel] [freeipa PR#567][opened] Configure KDC to use certs after they are deployed

[simo5]

URL: https://github.com/freeipa/freeipa/pull/567
Author: simo5
Title: #567: Configure KDC to use certs after they are deployed
Action: opened

PR body:
"""
Certmonger needs to access the KDC when it tries to obtain certs,
so make sure the KDC can run, then reconfigure it to use pkinit anchors
once certs are deployed.

"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/567/head:pr567
git checkout pr567

[simo5]

URL: https://github.com/freeipa/freeipa/pull/567
Title: #567: Configure KDC to use certs after they are deployed

simo5 commented:
"""
Still testing but this should be the way to go to fix the bug reported in #564
"""

See the full comment at https://github.com/freeipa/freeipa/pull/567#issuecomment-285493679

[martbab]

URL: https://github.com/freeipa/freeipa/pull/567
Title: #567: Configure KDC to use certs after they are deployed

martbab commented:
"""
I think we can avoid the copy-pasta by actually moving PKINIT requesting code into `__common_post_setup` like this:

```diff
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -142,10 +142,15 @@ class KrbInstance(service.Service):
self.step("starting the KDC", self.__start_instance)
self.step("configuring KDC to start on boot", self.__enable)

+ if self.setup_pkinit:
+ self.step("installing X509 Certificate for PKINIT",
+ self.setup_pkinit)
+
def create_instance(self, realm_name, host_name, domain_name, admin_password, master_password, setup_pkinit=False, pkcs12_info=None, subject_base=None):
self.master_password = master_password
self.pkcs12_info = pkcs12_info
self.subject_base = subject_base
+ self.setup_pkinit = setup_pkinit

self.__common_setup(realm_name, host_name, domain_name, admin_password)

@@ -160,10 +165,6 @@ class KrbInstance(service.Service):

self.__common_post_setup()

- if setup_pkinit:
- self.step("installing X509 Certificate for PKINIT",
- self.setup_pkinit)
-
self.start_creation(runtime=30)

self.kpasswd = KpasswdInstance()
@@ -178,14 +179,12 @@ class KrbInstance(service.Service):
self.pkcs12_info = pkcs12_info
self.subject_base = subject_base
self.master_fqdn = master_fqdn
+ self.setup_pkinit = setup_pkinit

self.__common_setup(realm_name, host_name, domain_name, admin_password)

self.step("configuring KDC", self.__configure_instance)
self.step("adding the password extension to the directory", self.__add_pwd_extop_module)
- if setup_pkinit:
- self.step("installing X509 Certificate for PKINIT",
- self.setup_pkinit)

self.__common_post_setup()

```
Yes we have now duplicated member assignment but still better that duplicate logic. Also I have some inline comments.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/567#issuecomment-285599143

[martbab]

URL: https://github.com/freeipa/freeipa/pull/567
Title: #567: Configure KDC to use certs after they are deployed

martbab commented:
"""
I think we can avoid the copy-pasta by actually moving PKINIT requesting code into `__common_post_setup` like this:

```diff
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -142,10 +142,15 @@ class KrbInstance(service.Service):
self.step("starting the KDC", self.__start_instance)
self.step("configuring KDC to start on boot", self.__enable)

+ if self.setup_pkinit:
+ self.step("installing X509 Certificate for PKINIT",
+ self.setup_pkinit)
+
def create_instance(self, realm_name, host_name, domain_name, admin_password, master_password, setup_pkinit=False, pkcs12_info=None, subject_base=None):
self.master_password = master_password
self.pkcs12_info = pkcs12_info
self.subject_base = subject_base
+ self.setup_pkinit = setup_pkinit

self.__common_setup(realm_name, host_name, domain_name, admin_password)

@@ -160,10 +165,6 @@ class KrbInstance(service.Service):

self.__common_post_setup()

- if setup_pkinit:
- self.step("installing X509 Certificate for PKINIT",
- self.setup_pkinit)
-
self.start_creation(runtime=30)

self.kpasswd = KpasswdInstance()
@@ -178,14 +179,12 @@ class KrbInstance(service.Service):
self.pkcs12_info = pkcs12_info
self.subject_base = subject_base
self.master_fqdn = master_fqdn
+ self.setup_pkinit = setup_pkinit

self.__common_setup(realm_name, host_name, domain_name, admin_password)

self.step("configuring KDC", self.__configure_instance)
self.step("adding the password extension to the directory", self.__add_pwd_extop_module)
- if setup_pkinit:
- self.step("installing X509 Certificate for PKINIT",
- self.setup_pkinit)

self.__common_post_setup()

```
Yes we have now duplicated member assignment but still better that duplicate logic. Also I have some inline comments.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/567#issuecomment-285599143

[simo5]

URL: https://github.com/freeipa/freeipa/pull/567
Author: simo5
Title: #567: Configure KDC to use certs after they are deployed
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/567/head:pr567
git checkout pr567

[simo5]

URL: https://github.com/freeipa/freeipa/pull/567
Title: #567: Configure KDC to use certs after they are deployed

simo5 commented:
"""
Should have addressed all concerns in this push
"""

See the full comment at https://github.com/freeipa/freeipa/pull/567#issuecomment-285660566

[martbab]

URL: https://github.com/freeipa/freeipa/pull/567
Title: #567: Configure KDC to use certs after they are deployed

martbab commented:
"""
@simo5 actually I found multiple issues during review and concluded that setting up PKINIT on DL1 replica never worked correctly actually. Will open respective blocker tickets ASAP.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/567#issuecomment-286355471

[simo5]

URL: https://github.com/freeipa/freeipa/pull/567
Title: #567: Configure KDC to use certs after they are deployed

simo5 commented:
"""
Can you figure out exactly why certmonger is doing this ?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/567#issuecomment-286366985

[martbab]

URL: https://github.com/freeipa/freeipa/pull/567
Title: #567: Configure KDC to use certs after they are deployed

martbab commented:
"""
@simo5 yes the whole PKINIT setup logic on replica is flawed and will probably need to be moved into a later point in master/replica install. Can I re-use your PR and prepare a new one that will fix it properly? I will keep you the author of this commit if you wish.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/567#issuecomment-286389719

[simo5]

URL: https://github.com/freeipa/freeipa/pull/567
Title: #567: Configure KDC to use certs after they are deployed

simo5 commented:
"""
Sure no prob
"""

See the full comment at https://github.com/freeipa/freeipa/pull/567#issuecomment-286391140

[martbab]

URL: https://github.com/freeipa/freeipa/pull/567
Title: #567: Configure KDC to use certs after they are deployed

martbab commented:
"""
@simo5 thank you
"""

See the full comment at https://github.com/freeipa/freeipa/pull/567#issuecomment-286392161

[martbab]

URL: https://github.com/freeipa/freeipa/pull/567
Author: simo5
Title: #567: Configure KDC to use certs after they are deployed
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/567/head:pr567
git checkout pr567

[martbab]

URL: https://github.com/freeipa/freeipa/pull/567
Title: #567: Configure KDC to use certs after they are deployed

martbab commented:
"""
Superseded by https://github.com/freeipa/freeipa/pull/584
"""

See the full comment at https://github.com/freeipa/freeipa/pull/567#issuecomment-286765122

[martbab]

URL: https://github.com/freeipa/freeipa/pull/567
Title: #567: Configure KDC to use certs after they are deployed

Label: +rejected