[Freeipa-devel] [PATCH] 0091 Allow full customisability of CA subject name
Fraser Tweedale
ftweedal at redhat.com
Fri Sep 23 07:15:00 UTC 2016
On Fri, Sep 23, 2016 at 08:51:02AM +0200, Jan Cholasta wrote:
> 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.
>
I expect (hope...) to have cycles to push this forward after my PTO
next week.
Thanks for your comprehensive initial review; there is plenty of
work still to do :)
Cheers,
Fraser
More information about the Freeipa-devel
mailing list