[Freeipa-devel] [PATCHES] 267-294 Support multiple CA certificates in LDAP
Rob Crittenden
rcritten at redhat.com
Mon Jun 16 20:36:34 UTC 2014
Rob Crittenden wrote:
> Jan Cholasta wrote:
>> Hi,
>>
>> the attached patches implement
>> <https://fedorahosted.org/freeipa/ticket/3259> and
>> <https://fedorahosted.org/freeipa/ticket/3520>.
>>
>> This work depends on my patches 241-253 and 262-266
>> (<http://www.redhat.com/archives/freeipa-devel/2014-June/msg00276.html>).
>>
>> Note that automatic distribution of CA certificates to IPA systems is
>> not implemented yet (it's planned for IPA 4.2, see
>> <https://fedorahosted.org/freeipa/ticket/4322>), so /etc/ipa/ca.crt,
>> /etc/pki/nssdb, /etc/dirsrv/slapd-REALM and /etc/httpd/alias are updated
>> *only* during client/server install.
>
> 267
>
> What is the purpose of this change? Won't this cause no certificates to
> be exported in the default case?
>
> diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py
> index 61a954f..3cd7a53 100644
> --- a/ipaserver/install/certs.py
> +++ b/ipaserver/install/certs.py
> @@ -463,7 +463,7 @@ class CertDB(object):
> do that step."""
> # export the CA cert for use with other apps
> ipautil.backup_file(self.cacert_fname)
> - root_nicknames = self.find_root_cert(nickname)
> + root_nicknames = self.find_root_cert(nickname)[:-1]
> fd = open(self.cacert_fname, "w")
> for root in root_nicknames:
> (cert, stderr, returncode) = self.run_certutil(["-L", "-n",
> root, "-a"])
>
> Or are you separating out issuing CA from the rest of the chain?
>
> 268 ACK
>
> 269 ACK
>
> 270
>
> If a user has their own CA installed, such as the case where they used
> ipa-server-certinstall, this will break it.
>
> What can be in the other set? Docs are needed.
>
> You potentially add a bunch of certs with no trust, what is the purpose?
>
> 271
>
> ipaSecretKey isn't mentioned on
> http://www.freeipa.org/page/V4/CA_certificate_renewal . What is it and
> does it need to be added now?
>
> 272 ACK
>
> 273 ACK
>
> 274 ACK
>
> 275 ACK
>
> 276
>
> There isn't any error handling around the ASN.1 decoder. Is that wise?
>
> There should be unit tests
>
> 277
>
> There should be unit tests
>
> 278
>
> When the certificate must be passed in as DER can you use dercert as the
> argument name so it's clear what is needed?
>
> In update_ca_cert() and get_ca_certs() should the values for trust be
> case insensitive?
>
> This breaks the convention where attribute names are always lower-case.
>
> I can't really grok add_ca_cert(). You are adding certs but removing
> values? Is this un-setting the list of trusted CA's maybe?
>
> There isn't a single comment in this file beyond the header.
>
> 279
>
> Looks ok
>
> 280
>
> Why add the chain with no trust?
>
> 281 / 282
>
> What is the difference between add_cert and import_cert other than
> passing in trust and not having the db password? Do we even need
> add_single_pem_cert anymore?
>
> 283 / 284
>
> In __import_ca_certs() I assume the pass is there for NotFound for the
> case of creating a replica with an older master that doesn't have these?
> How is SSL trust setup then?
>
> This code looks awfully similar.
>
> 285
>
> ACK
>
> 286
>
> It looks ok, just one wild thought. If writing the certificate fails and
> you go to clean up by removing ca_file, what if that removal fails, for
> the same reason the write failed, SELinux for example?
>
> 287
>
> If this weren't addressed in a later patch I've have dinged you on the:
>
> + return [cert]
>
> There are some places where you pluralize the variables (certs) and
> others where you do not (ca_cert, existing_ca_cert).
>
> 288
>
> Is a rawcert a dercert? I'd use that name instead for consistency.
>
> normalize_certificate() can raise exceptions. Those should be handled in
> write_certificate_list()
>
> 289
>
> ACK
>
> 290
>
> This can be dangerous because if another database is opened in the
> process things may fail. Additional warnings and red flags should be
> provided, or the context should be compared to known places this will
> blow up (e.g server).
>
> 291
>
> You use a temporary database to make cleanup easier I suppose? Did you
> test this in enforcing?
>
> 292
>
> Need unit tests
>
> 293
>
> Why the fixme for the x509 import?
>
> Isn't this changing already published API for
> [insert|remove]_ca_cert_into_systemwide_ca_store?
>
> 294
>
> Can you change the old occurances of
>
> cafile = config.dir + "/ca.crt"
>
> to use CACERT instead?
>
> Bad case in exception, "Ca cert file is not available". Is there any
> additional information we can provide, like what to do about it and
> where we looked?
>
> You actually remove one occurrence of CACERT and replace it with a
> hardcoded path, is that on purpose?
>
> ---
>
> I still need to do functional testing and will probably take another
> pass the changes through but this patchset generally looks ok.
Several issues found during testing:
1. Enrollment from RHEL-5 fails because the entire cert chain is not
retrieved, only the issuing CA cert.
Joining realm failed: libcurl failed to execute the HTTP POST
transaction. SSL certificate problem, verify that the CA cert is OK.
Details:
error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate
verify failed
Installation failed. Rolling back changes.
2. Not quite sure the cause, but this is on a replica:
# ldapsearch -x -Z
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
ldap_start_tls: Operations error (1)
additional info: SSL connection already established.
# extended LDIF
#
# LDAPv3
# base <dc=greyoak,dc=com> (default) with scope subtree
# filter: (objectclass=*)
Same command on initial master doesn't spit out the p11-kit errors.
Get similar errors out of certutil:
# certutil -L -d /etc/httpd/alias
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
p11-kit: invalid basic constraints certificate extension
Certificate Nickname Trust
Attributes
SSL,S/MIME,JAR/XPI
GREYOAK.COM IPA CA CT,C,C
Server-Cert u,u,u
CN=External Authority ,,
ipaCert u,u,u
Same version of p11-kit on both machines.
p11-kit-trust-0.20.2-1.fc20.x86_64
p11-kit-0.20.2-1.fc20.x86_64
3. On uninstall the CA's are not removed from /etc/pki/nssdb and
/etc/httpd/alias in one of my tests (the first one, so I no longer have
the logs).
4. This one isn't your issue AFAICT, not sure if you've seen this one:
# ipa-ca-install ...
[snip]
2014-06-16T18:07:44Z DEBUG stdout=Loading deployment configuration from
/tmp/tmprI2isq.
2014-06-16T18:07:44Z DEBUG stderr=Traceback (most recent call last):
File "/usr/sbin/pkispawn", line 530, in <module>
main(sys.argv)
File "/usr/sbin/pkispawn", line 439, in main
info = parser.sd_get_info()
File
"/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.py",
line 442, in sd_get_info
info = sd.getSecurityDomainInfo()
File "/usr/lib/python2.7/site-packages/pki/system.py", line 40, in
getSecurityDomainInfo
info.name = j['DomainInfo']['@id']
KeyError: 'DomainInfo'
2014-06-16T18:07:44Z CRITICAL failed to configure ca instance Command
''/usr/sbin/pkispawn' '-s' 'CA' '-f' '/tmp/tmprI2isq'' returned non-zero
exit status 1
2014-06-16T18:07:44Z DEBUG File
"/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py",
line 639, in run_script
return_value = main_function()
5. This one may be for Tomas, but:
2014-06-16T19:01:28Z INFO Deleting schedule 2358-2359 0 from agreement
cn=meTogrindle.greyoak.com,cn=replica,cn=dc\=greyoak\,dc\=com,cn=mapping
tree,cn=config
2014-06-16T19:01:29Z ERROR unable to convert the attribute
u'nsds5replicalastupdateend' value '0' to type <type 'datetime.datetime'>
2014-06-16T19:01:29Z INFO Replication Update in progress: TRUE: status:
0 Replica acquired successfully: Incremental update started: start: 0:
end: 0
2014-06-16T19:01:30Z ERROR unable to convert the attribute
u'nsds5replicalastupdateend' value '0' to type <type 'datetime.datetime'>
2014-06-16T19:01:30Z INFO Replication Update in progress: TRUE: status:
0 Replica acquired successfully: Incremental update started: start: 0:
end: 0
2014-06-16T19:01:31Z ERROR unable to convert the attribute
u'nsds5replicalastupdateend' value '0' to type <type 'datetime.datetime'>
2014-06-16T19:01:31Z INFO Replication Update in progress: TRUE: status:
0 Replica acquired successfully: Incremental update started: start: 0:
end: 0
2014-06-16T19:01:32Z ERROR unable to convert the attribute
u'nsds5replicalastupdateend' value '0' to type <type 'datetime.datetime'>
2014-06-16T19:01:32Z INFO Replication Update in progress: TRUE: status:
0 Replica acquired successfully: Incremental update started: start: 0:
end: 0
6. With an external CA install of F-20 vanilla, upgraded to this patch,
the external CA is not added to /etc/pki/nssdb. On a pure install of
this patch, it is added. Not sure if we care since the cert is in the
global cert store.
7. Something for the future, but I wonder if test test_0002_find_CA in
ipatests/test_xmlrpc/test_cert_plugin.py should be able to handle
external CAs.
rob
More information about the Freeipa-devel
mailing list