Dne 29.9.2014 v 15:05 Rob Crittenden napsal(a):
Jan Cholasta wrote:
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...
What's the point of a database missing certificates?
It won't be missing any certificates if /etc/pki/nssdb was not missing
any certificates before the update.
As I said, the update will not break anything. It will not fix anything
either, but I don't think this kind of fixing should be done during
client RPM upgrade. It is not consistent with anything else we do during
client RPM upgrade, it does not scale well and it just does not feel
right to me in overall.
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.
The result could be a deleted cert, that was my concern. It does seem to
be a rather slim case.
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
-----------------------------------------------------------
Sorry, that was a rhetorical question. Presumably NSS hunts around for
libnssckbi.so but it doesn't seem to look very hard, except that it does
seem to find it if it exists in the same directory as the DB, so I
symlink it in. I think mod_nss is probably the only thing that does
this, but it can make things rather interesting (for good and bad)
dealing with global certs.
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.
Gah, ok then, I'd definitely document that as it's a rather major minefield.
OK.
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?
No, only the original CA cert is there which explains why the helpers
didn't update things.
The LDAP update happens in renew_ca_cert. Are there any relevant errors
in /var/log/messages? Is there caRenewalMaster in ipaConfigString of the
master entry of the master where you run ipa-cacert-manage renew?
rob
--
Jan Cholasta
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
