[Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
Jan Cholasta
jcholast at redhat.com
Fri Sep 26 17:54:11 UTC 2014
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:
>
>
--
Jan Cholasta
More information about the Freeipa-devel
mailing list