[Freeipa-devel] [PATCH] 0091 Allow full customisability of CA subject name
Jan Cholasta
jcholast at redhat.com
Fri Sep 23 06:51:02 UTC 2016
On 25.8.2016 12:08, Jan Cholasta wrote:
> On 22.8.2016 07:00, Fraser Tweedale wrote:
>> On Fri, Aug 19, 2016 at 08:09:33PM +1000, Fraser Tweedale wrote:
>>> On Mon, Aug 15, 2016 at 10:54:25PM +1000, Fraser Tweedale wrote:
>>>> On Mon, Aug 15, 2016 at 02:08:54PM +0200, Jan Cholasta wrote:
>>>>> On 19.7.2016 12:05, Jan Cholasta wrote:
>>>>>> On 19.7.2016 11:54, Fraser Tweedale wrote:
>>>>>>> On Tue, Jul 19, 2016 at 09:36:17AM +0200, Jan Cholasta wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 15.7.2016 07:05, Fraser Tweedale wrote:
>>>>>>>>> On Fri, Jul 15, 2016 at 03:04:48PM +1000, Fraser Tweedale wrote:
>>>>>>>>>> The attached patch is a work in progress for
>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/2614 (BZ 828866).
>>>>>>>>>>
>>>>>>>>>> I am sharing now to make the approach clear and solicit feedback.
>>>>>>>>>>
>>>>>>>>>> It has been tested for server install, replica install (with and
>>>>>>>>>> without CA) and CA-replica install (all hosts running
>>>>>>>>>> master+patch).
>>>>>>>>>>
>>>>>>>>>> Migration from earlier versions and server/replica/CA install
>>>>>>>>>> on a
>>>>>>>>>> CA-less deployment are not yet tested; these will be tested over
>>>>>>>>>> coming days and patch will be tweaked as necessary.
>>>>>>>>>>
>>>>>>>>>> Commit message has a fair bit to say so I won't repeat here
>>>>>>>>>> but let
>>>>>>>>>> me know your questions and comments.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Fraser
>>>>>>>>>>
>>>>>>>>> It does help to attach the patch, of course ^_^
>>>>>>>>
>>>>>>>> IMO explicit is better than implicit, so instead of introducing
>>>>>>>> additional
>>>>>>>> magic around --subject, I would rather add a new separate option
>>>>>>>> for
>>>>>>>> specifying the CA subject name (I think --ca-subject, for
>>>>>>>> consistency
>>>>>>>> with
>>>>>>>> --ca-signing-algorithm).
>>>>>>>>
>>>>>>> The current situation - the --subject argument which specifies the
>>>>>>> not the subject but the subject base, is confusing enough (to say
>>>>>>> nothing of the limitations that give rise to the RFE).
>>>>>>>
>>>>>>> Retaining --subject for specifying the subject base and adding
>>>>>>> --ca-subject for specifying the *actual* subject DN gets us over the
>>>>>>> line in terms of the RFE, but does not make the installer less
>>>>>>> confusing. This is why I made --subject accept the full subject DN,
>>>>>>> with provisions to retain existing behaviour.
>>>>>>>
>>>>>>> IMO if we want to have separate arguments for subject DN and subject
>>>>>>> base (I am not against it), let's bite the bullet and name arguments
>>>>>>> accordingly. --subject should be used to specify full Subject DN,
>>>>>>> --subject-base (or similar) for specifying subject base.
>>>>>>
>>>>>> IMHO --ca-subject is better than --subject, because it is more
>>>>>> explicit
>>>>>> whose subject name that is (the CA's). I agree that --subject
>>>>>> should be
>>>>>> deprecated and replaced with --subject-base.
>>>>>>
>>>>>>>
>>>>>>> (I intentionally defer discussion of specific behaviour if one, none
>>>>>>> or both are specified; let's resolve the question or renaming /
>>>>>>> changing meaning of arguments first).
>>>>>>>
>>>>>>>
>>>>>>>> By specifying the option you would override the default
>>>>>>>> "CN=Certificate
>>>>>>>> Authority,$SUBJECT_BASE" subject name. If --external-ca was not
>>>>>>>> used,
>>>>>>>> additional validation would be done to make sure the subject
>>>>>>>> name meets
>>>>>>>> Dogtag's expectations. Actually, it might make sense to always
>>>>>>>> do the
>>>>>>>> additional validation, to be able to print a warning that if a
>>>>>>>> Dogtag-incompatible subject name is used, it won't be possible to
>>>>>>>> change the
>>>>>>>> CA cert chaining from externally signed to self-signed later.
>>>>>>>>
>>>>>>>> Honza
>>>>>
>>>>> Bump, any update on this?
>>>>>
>>>> I have an updated patch that fixes some issues Sebastian encountered
>>>> in testing, but I've not yet tackled the main change requested by
>>>> Honza (in brief: adding --ca-subject for specifying that, adding
>>>> --subject-base for specifying that, and deprecating --subject;
>>>> Sebastian, see discussion above and feel free to give your
>>>> thoughts). I expect I'll get back onto this work within the next
>>>> few days.
>>>>
>>> Update: I've got an updated version of patch almost ready for
>>> review, but I'm still ironing out some wrinkles in replica
>>> installation.
>>>
>>> Expect to be able to send it Monday or Tuesday for review.
>>>
>> Updated patch attached.
>>
>> I expect it will take a while to review, test and get the ACK, but
>> in any case: IMO it should not be merged until after ipa-4-4 branch
>> gets created.
>
> 1) Please fix these:
>
> $ git show -U0 | pep8 --diff
> ./ipaserver/install/cainstance.py:508:13: E128 continuation line
> under-indented for visual indent
> ./ipaserver/install/dsinstance.py:259:5: E303 too many blank lines (2)
> ./ipaserver/install/installutils.py:999:1: E302 expected 2 blank lines,
> found 1
> ./ipaserver/install/server/common.py:161:80: E501 line too long (101 >
> 79 characters)
> ./ipaserver/install/server/replicainstall.py:93:80: E501 line too long
> (82 > 79 characters)
> ./ipaserver/install/server/replicainstall.py:604:80: E501 line too long
> (81 > 79 characters)
>
>
> 2) Please put 3rd party library (such as six) imports between standard
> library imports and ipa imports.
>
>
> 3) ipa-ca-install should also have the --subject-base and --ca-subject
> options.
>
>
> 4) Please use the original method of getting the configured subject base
> from ipacertificatesubjectbase of the config object rather than
> DSInstance.find_subject_base(), which is horrendous and should in fact
> be obliterated (not necessarily in this patch).
>
>
> 5) You can use "unicode(x509.get_subject(cert))" to get subject name of
> a cert instead of "unicode(x509.load_certificate(cert).subject)".
>
>
> 6) For every removed "options.subject = ..." there should be a
> "options.subject_base ..." added.
>
>
> 7) The subject base read from replica config should be respected, i.e.
> this bit in ipa-ca-install should stay:
>
> - if config.subject_base is None:
> - attrs = conn.get_ipa_config()
> - config.subject_base = attrs.get('ipacertificatesubjectbase')[0]
>
> Also I would move the rest of the "look up CA subject name" to between
> options.ca_cert_file assignment and ca.install_check() call:
>
> else:
> options.ca_cert_file = None
>
> + # look up CA subject name (needed for DS certmap.conf)
> + ipa_ca_nickname = get_ca_nickname(config.realm_name)
> + db = certs.CertDB(config.realm_name, nssdir=paths.IPA_NSSDB_DIR)
> + cert = db.get_cert_from_db(ipa_ca_nickname)
> + options.ca_subject = unicode(x509.load_certificate(cert).subject)
> +
> ca.install_check(True, config, options)
> if options.promote:
> ca_data = (os.path.join(config.dir, 'cacert.p12'),
>
>
> 8) I think we should remove --subject from ipa-server-install man page
> altogether.
>
>
> 9) I don't like that the default values are set in multiple places
> (CAInstance.configure_instance(), CAInstance.configure_replica(),
> KRAInstance.configure_instance(), KRAInstance.configure_replica(),
> ipa-server-install). The defaults should be handled in one place - ca.py
> - and the values (read from configuration or specified by user or
> default) should be passed through arguments to CAInstance/KRAInstance.
>
>
> 10) Nitpick, but the ca_subject_dn argument of CertDB() would be better
> called just ca_subject and be located after subject_base, for
> consistency with the rest of the patch.
>
> Maybe also rename the subject argument of the various CAInstance and
> KRAInstance methods to ca_subject?
>
>
> 11) I see that there was some code which ignored the configured subject
> base. I think the fixes for that should be moved into a separate patch
> and maybe even a separate ticket.
>
>
> 12) The proper way to rename a Knob and keep the old name is to put the
> old name in cli_aliases of the renamed Knob rather than add a new Knob,
> like this:
>
> subject_base = Knob(
> str, None,
> description="The certificate subject base (default
> O=<realm-name>)",
> cli_aliases=['subject'],
> )
>
> This way you wouldn't be able to issue a warning when --subject is used,
> but that's OK, as we don't do it for any other renamed options too.
>
>
> 13) AFAIK CN is in fact not valid in a subject base, so it should not be
> added to VALID_SUBJECT_ATTRS.
>
>
> 14) NACK on the normalization stuff. It's not really normalization if
> the original value is not equal to the normalized value. Instead of this
> you should validate if the provided subject name is suitable for Dogtag
> and if it isn't, fail and inform the user what the acceptable format is.
>
>
> 15) Subject base setting is not respected for most of our certs. This is
> with --subject-base='O=Test':
>
> $ sudo getcert list | grep subject
> subject: CN=CA Audit,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject: CN=OCSP Subsystem,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject: CN=CA Subsystem,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject: CN=Certificate Authority,O=Test
> subject: CN=IPA RA,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject:
> CN=vm-058-193.abc.idm.lab.eng.brq.redhat.com,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
>
> subject: CN=vm-058-193.abc.idm.lab.eng.brq.redhat.com,O=Test
> subject: CN=vm-058-193.abc.idm.lab.eng.brq.redhat.com,O=Test
>
> This is most likely because of 5) and 8) combined.
>
>
> 16) Spaces do not work. This is with --subject-base='O=My Organization':
>
> $ ipa config-show | grep 'Subject base'
> Certificate Subject base: O=My
>
> $ sudo getcert list | grep 'subject'
> subject: CN=CA Audit,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject: CN=OCSP Subsystem,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject: CN=CA Subsystem,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject: CN=Certificate Authority,O=My
> subject: CN=IPA RA,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject:
> CN=vm-058-193.abc.idm.lab.eng.brq.redhat.com,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
>
> subject: CN=vm-058-193.abc.idm.lab.eng.brq.redhat.com,O=My
> subject: CN=vm-058-193.abc.idm.lab.eng.brq.redhat.com,O=My
>
> I blame the normalization. See also 13).
>
>
> 17) CN in subject base does not work. This is with
> --subject-base='CN=Test':
>
> $ sudo getcert list | grep subject
> subject: CN=CA Audit,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject: CN=OCSP Subsystem,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject: CN=CA Subsystem,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject: CN=Certificate Authority,CN=Test
> subject: CN=IPA RA,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
> subject:
> CN=vm-058-193.abc.idm.lab.eng.brq.redhat.com,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
>
> subject: CN=Test,CN=Test
> subject: CN=Test,CN=Test
>
> See 12).
>
>
> 18) In CA-less topology, ipa-replica-install fails:
>
> 2016-08-25T09:54:09Z DEBUG Starting external process
> 2016-08-25T09:54:09Z DEBUG args=/usr/bin/certutil -d /etc/ipa/nssdb -L
> -n ABC.IDM.LAB.ENG.BRQ.REDHAT.COM IPA CA -a
> 2016-08-25T09:54:09Z DEBUG Process finished, return code=255
> 2016-08-25T09:54:09Z DEBUG stdout=
> 2016-08-25T09:54:09Z DEBUG stderr=certutil: Could not find cert:
> ABC.IDM.LAB.ENG.BRQ.REDHAT.COM IPA CA
> : PR_FILE_NOT_FOUND_ERROR: File not found
>
> 2016-08-25T09:54:09Z DEBUG Destroyed connection
> context.ldap2_140045224192976
> 2016-08-25T09:54:09Z DEBUG Starting external process
> 2016-08-25T09:54:09Z DEBUG args=/usr/sbin/ipa-client-install
> --unattended --uninstall
> 2016-08-25T09:54:18Z DEBUG Process finished, return code=0
> 2016-08-25T09:54:18Z DEBUG File
> "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in
> execute
> return_value = self.run()
> ----8<------
> File
> "/usr/lib/python2.7/site-packages/ipaserver/install/server/replicainstall.py",
> line 1722, in main
> promote_check(self)
> File
> "/usr/lib/python2.7/site-packages/ipaserver/install/server/replicainstall.py",
> line 364, in decorated
> func(installer)
> File
> "/usr/lib/python2.7/site-packages/ipaserver/install/server/replicainstall.py",
> line 386, in decorated
> func(installer)
> File
> "/usr/lib/python2.7/site-packages/ipaserver/install/server/replicainstall.py",
> line 1266, in promote_check
> options.ca_subject = unicode(x509.load_certificate(cert).subject)
> File "/usr/lib/python2.7/site-packages/ipalib/x509.py", line 125, in
> load_certificate
> return nss.Certificate(buffer(data)) # pylint: disable=buffer-builtin
>
> 2016-08-25T09:54:18Z DEBUG The ipa-replica-install command failed,
> exception: NSPRError: (SEC_ERROR_LIBRARY_FAILURE) security library failure.
> 2016-08-25T09:54:18Z ERROR (SEC_ERROR_LIBRARY_FAILURE) security library
> failure.
> 2016-08-25T09:54:18Z ERROR The ipa-replica-install command failed. See
> /var/log/ipareplica-install.log for more information
>
>
> 19) In CA-less topology, ipa-ca-install fails:
>
> 2016-08-25T09:58:21Z DEBUG raw: ca_show(u'ipa', version=u'2.212')
> 2016-08-25T09:58:21Z DEBUG ca_show(u'ipa', rights=False, all=False,
> raw=False, version=u'2.212')
> 2016-08-25T09:58:21Z DEBUG raw: ca_is_enabled(version=u'2.212')
> 2016-08-25T09:58:21Z DEBUG ca_is_enabled(version=u'2.212')
> 2016-08-25T09:58:21Z DEBUG File
> "/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py",
> line 752, in run_script
> return_value = main_function()
>
> File "/sbin/ipa-ca-install", line 309, in main
> promote(safe_options, options, filename)
>
> File "/sbin/ipa-ca-install", line 279, in promote
> install_master(safe_options, options)
>
> File "/sbin/ipa-ca-install", line 232, in install_master
> subject = api.Command.ca_show(IPA_CA_CN)['result']['ipacasubjectdn']
>
> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 449,
> in __call__
> return self.__do_call(*args, **options)
>
> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 477,
> in __do_call
> ret = self.run(*args, **options)
>
> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 799,
> in run
> return self.execute(*args, **options)
>
> File "/usr/lib/python2.7/site-packages/ipaserver/plugins/ca.py", line
> 144, in execute
> ca_enabled_check()
>
> File "/usr/lib/python2.7/site-packages/ipaserver/plugins/cert.py",
> line 222, in ca_enabled_check
> raise errors.NotFound(reason=_('CA is not configured'))
>
> 2016-08-25T09:58:21Z DEBUG The ipa-ca-install command failed, exception:
> NotFound: CA is not configured
>
> This is related to 3).
Bump.
--
Jan Cholasta
More information about the Freeipa-devel
mailing list