[Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes

Mon Sep 29 12:21:42 UTC 2014

Dne 26.9.2014 v 19:54 Jan Cholasta napsal(a):
> Dne 26.9.2014 v 17:13 Rob Crittenden napsal(a):
>> Jan Cholasta wrote:
>>> Dne 23.9.2014 v 20:39 Rob Crittenden napsal(a):
>>>> Jan Cholasta wrote:
>>>>> Hi,
>>>>>
>>>>> the attached patches fix various bugs and shortcomings in the CA
>>>>> management and renewal code. Related tickets:
>>>>> <https://fedorahosted.org/freeipa/ticket/4416>,
>>>>> <https://fedorahosted.org/freeipa/ticket/4460>.
>>>>>
>>>>> (Patch 319 was originally posted at
>>>>> <http://www.redhat.com/archives/freeipa-devel/2014-September/msg00132.html>.)
>>>>>
>>>>>
>>>>
>>>> Only two of the patches includes what ticket(s) they address. Most have
>>>> the tersest of commit messages. If got more and more difficult to see
>>>> why the changes were needed at all, as you'll see.
>>>
>>> Sorry, fixed (hopefully).
>>>
>>> Note that most of these patches fix stuff I didn't have time to fix
>>> before I posted the original CA management patches, hence the missing
>>> tickets.
>>
>> Well, the policy is that every commit should have a ticket. So I guess
>> re-open the old tickets or open new ones. This will help someone in the
>> future know the "why" of a patch. I've certainly been guilty
>
> OK, I will reopen the related tickets.
>
>>
>> Here is a new set of reviews as trying to intermingle was making my eyes
>> cross:
>>
>> 319:
>>
>> I guess I still don't understand why you can't pull the certs out of
>> LDAP when creating this database. When this code runs, we know that the
>> client is configured, so we have access to authentication. Why can't
>> create_ipa_nssdb pull the certs directly? It seems more robust to me,
>> and the code is already written in ipa-client-install to do this.
>
> Well, I don't understand why do you want them to be updated so much, as
> nothing will break if they are not. Also try to imagine what would
> happen if, say, 10k clients were updated at the same moment...
>
>>
>> When adding the CA certificates to the temporary database it will report
>> that a failure occurred, but not the exception:
>>
>> +        except CalledProcessError, e:
>> +            root_logger.info("Failed to add CA to temporary NSS
>> database.")
>> +            return CLIENT_INSTALL_ERROR
>>
>> Granted, NSS errors are often obtuse, but should it at least DEBUG log
>> it?
>
> It is already logged in ipautil.run. The exception only says that the
> command failed, there's no point in logging it.
>
>>
>> 324, 325, 326: ACK
>>
>> 327:
>>
>> So the idea is to just mirror the certs and us the new db to know what
>> was added?
>
> Exactly.
>
>> What if someone has the same nickname, different cert in the
>> two databases? Is that too much of a corner case?
>
> IMO it is too much of a corner case, but I plan on adding a better
> diff/patch algorithm in the future if it turns out to be a problem.
>
>>
>> 328, 329, 330, 331: ACK
>>
>> As an aside, do you know why the global certs are seen by mod_nss?
>> libnssckbi.so is symlinked into the directory (certutil -L -d
>> /etc/httpd/alias -h all will show all the certs).
>
> I'm not sure why it is this way, but the module is linked to the NSS
> database:
>
> $ sudo modutil -list -dbdir /etc/httpd/alias
>
> Listing of PKCS #11 Modules
> -----------------------------------------------------------
>    1. NSS Internal PKCS #11 Module
>       slots: 2 slots attached
>      status: loaded
>
>       slot: NSS Internal Cryptographic Services
>      token: NSS Generic Crypto Services
>
>       slot: NSS User Private Key and Certificate Services
>      token: NSS Certificate DB
>
>    2. Root Certs
>      library name: /etc/httpd/alias/libnssckbi.so
>       slots: 2 slots attached
>      status: loaded
>
>       slot: /etc/pki/ca-trust/source
>      token: System Trust
>
>       slot: /usr/share/pki/ca-trust-source
>      token: Default Trust
> -----------------------------------------------------------
>
>>
>> 332-335
>>
>> I think the naming and/or comments can be improved. For example, there
>> are now 3 *retrieve_cert commands, all of which do slightly different
>> things. All have external handlers (via the oddly named profile), but
>> some are called internally as well.
>
> I have spent quite some time trying to come up with good names for them,
> but I was not able to do so. Suggestions are welcome.
>
>>
>> This is rather complex code: a command passes a call onto certmonger
>> which then exuecutes the IPA CA helper... More documentation would
>> definitely help a newcomer figure out how renewal, CA retrieval, etc.
>> works.
>
> OK, I'll try to add some.
>
>>
>> Not to be too pedantic but there is a lot of this in
>> dogtag-ipa-ca-renew-agent-submit:
>>
>> if variable: OR if not variable:
>>
>> Where variable defaults to None. Shouldn't the test be:
>>
>> if variable is [not] None:
>
> This doesn't catch empty strings, which may occur in some of these checks.
>
>>
>> 340: ACK
>>
>> Through some combination of ipa-certupdate and ipa-cacert-manage I seem
>> to have hosed things up:
>>
>> ipa: DEBUG: certmonger request is in state
>> dbus.String(u'CA_UNREACHABLE', variant_level=1)
>> ipa.ipaserver.install.ipa_cacert_manage.CACertManage: DEBUG:   File
>> "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in
>> execute
>>      return_value = self.run()
>>    File
>> "/usr/lib/python2.7/site-packages/ipaserver/install/ipa_cacert_manage.py",
>>
>> line 118, in run
>>      rc = self.renew()
>>    File
>> "/usr/lib/python2.7/site-packages/ipaserver/install/ipa_cacert_manage.py",
>>
>> line 181, in renew
>>      return self.renew_self_signed(ca)
>>    File
>> "/usr/lib/python2.7/site-packages/ipaserver/install/ipa_cacert_manage.py",
>>
>> line 193, in renew_self_signed
>>      self.resubmit_request(ca, 'caCACert')
>>    File
>> "/usr/lib/python2.7/site-packages/ipaserver/install/ipa_cacert_manage.py",
>>
>> line 315, in resubmit_request
>>      "please check the request manually" % self.request_id)
>>
>> ipa.ipaserver.install.ipa_cacert_manage.CACertManage: DEBUG: The
>> ipa-cacert-manage command failed, exception: ScriptError: Error
>> resubmitting certmonger request '20140909142830', please check the
>> request manually
>> ipa.ipaserver.install.ipa_cacert_manage.CACertManage: ERROR: Error
>> resubmitting certmonger request '20140909142830', please check the
>> request manually
>>
>> Incidentally, IMHO it should include the command to execute to check the
>> request manually.
>
> Will add.
>
>>
>> The CA is actually unreachable. Restarting it fixed things. I'll chalk
>> this up to cosmic rays.
>
> OK.
>
>>
>> Re-running ipa-cacert-manage renew was successful.
>
> Good.
>
>>
>> I confirmed that the CA signing cert was updated in the dogtag database.
>>
>> I then ran ipa-certupdate to distribute this new CA cert around and none
>> of the databases got the updated CA cert, nor did /etc/ipa/ca.crt.
>
> Can you see the new CA cert in cn=certificates,cn=ipa,cn=etc,$SUFFIX?
>
>>
>> Still doing some functional testing.
>>
>> Unrelated to this:
>>
>>     9:freeipa-python-4.0.0GITf6875d4-0.#################################
>> [100%]
>> Update failed: Operations error:
>>
>> Seems to be related to this:
>>
>> 2014-09-25T19:28:22Z DEBUG ---------------------------------------------
>> 2014-09-25T19:28:22Z DEBUG Final value after applying updates
>> 2014-09-25T19:28:22Z DEBUG dn: cn=referential integrity
>> postoperation,cn=plugins,cn=config
>> 2014-09-25T19:28:22Z DEBUG cn:
>> 2014-09-25T19:28:22Z DEBUG      referential integrity postoperation
>> 2014-09-25T19:28:22Z DEBUG objectClass:
>> 2014-09-25T19:28:22Z DEBUG      top
>> 2014-09-25T19:28:22Z DEBUG      nsSlapdPlugin
>> 2014-09-25T19:28:22Z DEBUG      extensibleObject
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginprecedence:
>> 2014-09-25T19:28:22Z DEBUG      40
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginPath:
>> 2014-09-25T19:28:22Z DEBUG      libreferint-plugin
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginVersion:
>> 2014-09-25T19:28:22Z DEBUG      1.3.3.2
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg5:
>> 2014-09-25T19:28:22Z DEBUG      owner
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg4:
>> 2014-09-25T19:28:22Z DEBUG      uniquemember
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg7:
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg6:
>> 2014-09-25T19:28:22Z DEBUG      seeAlso
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg1:
>> 2014-09-25T19:28:22Z DEBUG
>> /var/log/dirsrv/slapd-GREYOAK-COM/referint
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg0:
>> 2014-09-25T19:28:22Z DEBUG      0
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg3:
>> 2014-09-25T19:28:22Z DEBUG      member
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg2:
>> 2014-09-25T19:28:22Z DEBUG      0
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg9:
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg8:
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginInitfunc:
>> 2014-09-25T19:28:22Z DEBUG      referint_postop_init
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginexcludeentryscope:
>> 2014-09-25T19:28:22Z DEBUG      cn=provisioning,dc=greyoak,dc=com
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg11:
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg10:
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg13:
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg12:
>> 2014-09-25T19:28:22Z DEBUG nsslapd-plugin-depends-on-type:
>> 2014-09-25T19:28:22Z DEBUG      database
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginVendor:
>> 2014-09-25T19:28:22Z DEBUG      389 Project
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg17:
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg16:
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg18:
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginType:
>> 2014-09-25T19:28:22Z DEBUG      betxnpostoperation
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginDescription:
>> 2014-09-25T19:28:22Z DEBUG      referential integrity plugin
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginentryscope:
>> 2014-09-25T19:28:22Z DEBUG      dc=greyoak,dc=com
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginEnabled:
>> 2014-09-25T19:28:22Z DEBUG      on
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginId:
>> 2014-09-25T19:28:22Z DEBUG      referint
>> 2014-09-25T19:28:22Z DEBUG nsslapd-plugincontainerscope:
>> 2014-09-25T19:28:22Z DEBUG      dc=greyoak,dc=com
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg15:
>> 2014-09-25T19:28:22Z DEBUG referint-membership-attr:
>> 2014-09-25T19:28:22Z DEBUG      manager
>> 2014-09-25T19:28:22Z DEBUG      secretary
>> 2014-09-25T19:28:22Z DEBUG      memberuser
>> 2014-09-25T19:28:22Z DEBUG      memberhost
>> 2014-09-25T19:28:22Z DEBUG      sourcehost
>> 2014-09-25T19:28:22Z DEBUG      memberservice
>> 2014-09-25T19:28:22Z DEBUG      managedby
>> 2014-09-25T19:28:22Z DEBUG      memberallowcmd
>> 2014-09-25T19:28:22Z DEBUG      memberdenycmd
>> 2014-09-25T19:28:22Z DEBUG      ipasudorunas
>> 2014-09-25T19:28:22Z DEBUG      ipasudorunasgroup
>> 2014-09-25T19:28:22Z DEBUG      ipatokenradiusconfiglink
>> 2014-09-25T19:28:22Z DEBUG nsslapd-pluginarg14:
>> 2014-09-25T19:28:22Z DEBUG [(1, u'nsslapd-pluginarg16', None), (1,
>> u'nsslapd-pluginarg7', None), (1, u'nsslapd-pluginarg9', None), (1,
>> u'nsslapd-pluginarg8', None), (1, u'nsslapd-pluginarg11', None), (1,
>> u'nsslapd-pluginarg10', None), (1, u'nsslapd-pluginarg13', None), (1,
>> u'nsslapd-pluginarg12', None), (1, u'nsslapd-pluginarg17', None), (1,
>> u'nsslapd-pluginarg18', None), (1, u'nsslapd-pluginarg15', None), (2,
>> u'referint-membership-attr', ['sourcehost', 'memberallowcmd',
>> 'memberdenycmd', 'memberuser', 'managedby', 'manager', 'memberservice',
>> 'ipasudorunas', 'ipasudorunasgroup', 'secretary', 'memberhost',
>> 'ipatokenradiusconfiglink']), (1, u'nsslapd-pluginarg14', None)]
>> 2014-09-25T19:28:22Z DEBUG Live 1, updated 1
>> 2014-09-25T19:28:22Z DEBUG Unhandled LDAPError: OPERATIONS_ERROR:
>> {'desc': 'Operations error'}
>> 2014-09-25T19:28:22Z ERROR Update failed: Operations error:
>>

Updated rebased patches attached, now with ticket URLs.

I have decided to drop 332-335 for now, since I won't be able to finish 
them before the release. We can continue with them later.

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-319.3-Introduce-NSS-database-etc-ipa-nssdb.patch
Type: text/x-patch
Size: 16042 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140929/199ae784/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-324.2-Move-NSSDatabase-from-ipaserver.certs-to-ipapython.c.patch
Type: text/x-patch
Size: 20092 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140929/199ae784/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-325.2-Add-NSSDatabase.has_nickname-for-checking-nickname-p.patch
Type: text/x-patch
Size: 1075 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140929/199ae784/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-326.2-Use-NSSDatabase-instead-of-direct-certutil-calls-in-.patch
Type: text/x-patch
Size: 9212 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140929/199ae784/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-327.2-Use-etc-ipa-nssdb-to-get-nicknames-of-IPA-certs-inst.patch
Type: text/x-patch
Size: 8895 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140929/199ae784/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-328.2-Check-if-IPA-client-is-configured-in-ipa-certupdate.patch
Type: text/x-patch
Size: 1081 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140929/199ae784/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-329.2-Get-server-hostname-from-jsonrpc_uri-in-ipa-certupda.patch
Type: text/x-patch
Size: 1209 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140929/199ae784/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-330.2-Remove-ipa-ca.crt-from-systemwide-CA-store-on-client.patch
Type: text/x-patch
Size: 4461 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140929/199ae784/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-331.2-Fix-certmonger.wait_for_request.patch
Type: text/x-patch
Size: 960 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140929/199ae784/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-340.1-Fix-certmonger-search-for-the-CA-cert-in-ipa-certupd.patch
Type: text/x-patch
Size: 1897 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140929/199ae784/attachment-0009.bin>