On Tue, Aug 16, 2016 at 08:10:08AM +0200, Jan Cholasta wrote: > On 16.8.2016 07:24, Fraser Tweedale wrote: > > On Mon, Aug 15, 2016 at 08:19:33AM +0200, Jan Cholasta wrote: > > > On 9.8.2016 16:47, Fraser Tweedale wrote: > > > > On Mon, Aug 08, 2016 at 10:49:27AM +0200, Jan Cholasta wrote: > > > > > On 8.8.2016 09:06, Fraser Tweedale wrote: > > > > > > On Mon, Aug 08, 2016 at 08:54:05AM +0200, Jan Cholasta wrote: > > > > > > > Hi, > > > > > > > > > > > > > > On 8.8.2016 06:34, Fraser Tweedale wrote: > > > > > > > > Please review the attached patch with adds --certificate-out and > > > > > > > > --certificate-chain-out options to `ca-show' command. > > > > > > > > > > > > > > > > Note that --certificate-chain-out currently writes a bogus file > > > > > > > > due > > > > > > > > to a bug in Dogtag that will be fixed in this week's build. > > > > > > > > > > > > > > > > https://fedorahosted.org/freeipa/ticket/6178 > > > > > > > > > > > > > > 1) The client-side *-out options should be defined on the client > > > > > > > side, not > > > > > > > on the server side. > > > > > > > > > > > > > Will option defined on client side be propagated to, and observable > > > > > > in the ipaserver plugin? The ipaserver plugin needs to observe that > > > > > > *-out has been requested and executes additional command(s) on that > > > > > > basis. > > > > > > > > > > Is there a reason not to *always* return the certs? > > > > > > > > > We hit Dogtag to retrieve them. > > > > > > I don't think that's an issue in a -show command. > > > > > cert_show is invoked by other commands (cert_find*, cert_show, > > cert_request, cert_status, ca_del) but these all hit Dogtag anyway > > so I suppose that's fine. I'll return the cert *and* the chain in > > separate attributes, unconditionally. > > > > > > > > > > > > > > > > > > > > > > > > > > 2) I don't think there should be additional information included > > > > > > > in summary > > > > > > > (and it definitely should not be multi-line). I would rather > > > > > > > inform the user > > > > > > > via an error message when unable to write the files. > > > > > > > > > > > > > I was just following the pattern of other commands that write certs, > > > > > > profile config, etc. Apart from consistency with other commands I > > > > > > agree that there is no need to have it. So I will remove it. > > > > > > > > > > > > > If you think there is an actual value in informing the user about > > > > > > > successfully writing the files, please use ipalib.messages for > > > > > > > the job. > > > > > > > > > > > > > > > > > > > > > 3) IMO a better format for the certificate chain than PKCS#7 > > > > > > > would be > > > > > > > concatenated PEM, as that's the most commonly used format in IPA > > > > > > > (in > > > > > > > installers, there are no cert chains in API commands ATM). > > > > > > > > > > > > > Sure, but the main use case isn't IPA. Other apps require PKCS #7 > > > > > > or concatenated PEMs, but sometimes they must be concatenated > > > > > > forward, and othertimes backwards. There is no one size fits all. > > > > > > > > > > True, which is exactly why I think we should at least be > > > > > self-consistent and > > > > > use concatenated PEM (and multi-value DER over the wire). > > > > > > > > > Dogtag returns a PKCS7 (either DER or PEM, according to HTTP Accept > > > > header). > > > > > > > > If we want list-of-PEMs between server and client we have to convert > > > > on the server. Do we have a good way of doing this without exec'ing > > > > `openssl pkcs7' on the server? Is it acceptable to exec 'openssl' > > > > to do the conversion on the server? python-nss does not have PKCS7 > > > > functions and I am not keen on adding a pyasn1 PKCS7 parser just for > > > > the sake of pushing bits as list-of-PEMs. > > > > > > I'm afraid we can't avoid conversion to/from PKCS#7 one way or the other. > > > For example, if we added a call to retrieve external CA chain using certs > > > from cn=certificates,cn=ipa,cn=etc, we would have to convert the result to > > > PKCS#7 if it was our cert chain format of choice. > > > > > > What we can avoid though is executing "openssl pkcs7" to do the > > > conversion - > > > we can use an approach similar to our DNSSEC code and use python-cffi to > > > call libcrypto's PKCS#7 conversion routines instead. > > > > > I had a look at the OpenSSL API for parsing PKCS #7; now I prefer to > > exec `openssl' to do the job :) > > > > I will transmit DER-encoded PKCS #7 object on the wire; we cannot > > used multi-valued DER attribute because order is important. Client > > will convert to PEMs. > > Well, my point was not to send PKCS#7 over the wire, so that clients > (including 3rd party clients) do not have to convert from PKCS#7 themselves. > > In fact we can use multi-valued DER - whatever you send over the wire from > the server will be received in the exact same order by the client. Even if > it wasn't, you can easily restore the order by matching issuer and subject > names of the certificates. > > > > > Should have new patch on list this afternoon. > > > > Thanks, > > Fraser > > > > > > > > > > FWIW, man pages and code suggest that PKCS #7 is accepted in > > > > installer, etc. > > > > > > True, but that's a relatively new feature (since 4.1) and the installer > > > internally executes "openssl pkcs7" to convert PKCS #7 to list of certs > > > :-) > > > > > > > > > > > > > We can add an option to control the format later, but for now, > > > > > > Dogtag returns a PKCS #7 (PEM or DER) so let's go with that. Worst > > > > > > case is an admin has to invoke `openssl pkcs7' and concat the certs > > > > > > themselves. > > > > > > > > > > AFAIK none of NSS, OpenSSL or p11-kit can use PKCS#7 cert chains > > > > > directly, > > > > > so I'm afraid the worst case would happen virtually always. > > > > > > > > > If you're OK with invoking OpenSSL on the client to convert PKCS #7 > > > > to list-of-PEMs (similar to what is done in > > > > ipapython.certdb.NSSDatabase) then we can have the client perform > > > > the conversion. > > > > > > See above. > > > > > > > > > > > > > > > > > > > > > > > > > > > 4) Over the wire, the certs should be DER-formatted, as that's > > > > > > > the most > > > > > > > common wire format in other API commands. > > > > > > > > > > > > > OK. > > > > > > > > > > > > > > > > > > > > 5) What is the benefit in having the CA cert and the rest of the > > > > > > > chain > > > > > > > separate? For end-entity certs it makes sense to separate the > > > > > > > cert from the > > > > > > > CA chain, but for CA certs, you usually want the full chain, no? > > > > > > > > > > > > > If you want to anchor trust directly at a subca (e.g. restrict VPN > > > > > > login to certs issued by VPN sub-CA) then you often just want the > > > > > > cert. The chain option does subsume it, at cost of more work for > > > > > > administrators with this use case. I think it makes sense to keep > > > > > > both options. > > > > > > > > > > Does it? From what you described above, you either want just the > > > > > sub-CA > > > > > cert, or the full chain including the sub-CA cert, in which case it > > > > > might > > > > > make more sense to have a single --out option and a --chain flag. > > > > > > > > > How about --certificate-out which defaults to single cert, but does > > > > chain (as list-of-PEMs) when --chain flag given. > > > > > > > > Per https://fedorahosted.org/freeipa/ticket/5166 let's not add more > > > > `--out' options. > > > > > > +1 > > > Updated patch 0097-2 attached, and new patch 0099 which must be applied first.
I have implemented the suggested changes, except for cffi (I execute `openssl pkcs7' instead). There are two new output attributes on the wire, 'certificate' (single-value DER X.509), and 'certificate_chain' (ordered multi-value DER X.509). They are always returned. The first cert in the chain is always the same as 'certificate'; obviously this is redunant but I have left it this way because I think usage is clearer. Thanks, Fraser
From 77d7d0185bcf3cb86f56bd128507a8e2cfedc775 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale <ftwee...@redhat.com> Date: Tue, 16 Aug 2016 13:16:58 +1000 Subject: [PATCH] Add function for extracting PEM certs from PKCS #7 Add a single function for extracting X.509 certs in PEM format from a PKCS #7 object. Refactor sites that execute ``openssl pkcs7`` to use the new function. Part of: https://fedorahosted.org/freeipa/ticket/6178 --- ipalib/x509.py | 21 ++++++++++++++++- ipapython/certdb.py | 14 ++++------- ipaserver/install/cainstance.py | 52 +++++++++++++++-------------------------- 3 files changed, 43 insertions(+), 44 deletions(-) diff --git a/ipalib/x509.py b/ipalib/x509.py index 82194922d151a1b0f2df03df3578ad45b43b71c9..782519ce45e05e09d4dc02d41a537daab84db9e4 100644 --- a/ipalib/x509.py +++ b/ipalib/x509.py @@ -50,11 +50,12 @@ from ipalib import util from ipalib import errors from ipaplatform.paths import paths from ipapython.dn import DN +from ipapython import ipautil PEM = 0 DER = 1 -PEM_REGEX = re.compile(r'(?<=-----BEGIN CERTIFICATE-----).*?(?=-----END CERTIFICATE-----)', re.DOTALL) +PEM_REGEX = re.compile(r'-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----', re.DOTALL) EKU_SERVER_AUTH = '1.3.6.1.5.5.7.3.1' EKU_CLIENT_AUTH = '1.3.6.1.5.5.7.3.2' @@ -144,6 +145,24 @@ def load_certificate_list(data, dbdir=None): certs = [load_certificate(cert, PEM, dbdir) for cert in certs] return certs + +def pkcs7_to_pems(data, datatype=PEM): + """ + Extract certificates from a PKCS #7 object. + + Return a ``list`` of X.509 PEM strings. + + May throw ``ipautil.CalledProcessError`` on invalid data. + + """ + cmd = [ + paths.OPENSSL, "pkcs7", "-print_certs", + "-inform", "PEM" if datatype == PEM else "DER", + ] + result = ipautil.run(cmd, stdin=data, capture_output=True) + return PEM_REGEX.findall(result.output) + + def load_certificate_list_from_file(filename, dbdir=None): """ Load a certificate list from a PEM file. diff --git a/ipapython/certdb.py b/ipapython/certdb.py index e19f712d82f160ebc5de9c5b8d6627cb941c2cef..fd18023794a2daace60efd97aff54180b8409bbd 100644 --- a/ipapython/certdb.py +++ b/ipapython/certdb.py @@ -270,13 +270,11 @@ class NSSDatabase(object): continue if label in ('PKCS7', 'PKCS #7 SIGNED DATA', 'CERTIFICATE'): - args = [ - paths.OPENSSL, 'pkcs7', - '-print_certs', - ] try: - result = ipautil.run( - args, stdin=body, capture_output=True) + certs = x509.pkcs7_to_pems(body) + extracted_certs += '\n'.join(certs) + '\n' + loaded = True + continue except ipautil.CalledProcessError as e: if label == 'CERTIFICATE': root_logger.warning( @@ -287,10 +285,6 @@ class NSSDatabase(object): "Skipping PKCS#7 in %s at line %s: %s", filename, line, e) continue - else: - extracted_certs += result.output + '\n' - loaded = True - continue if label in ('PRIVATE KEY', 'ENCRYPTED PRIVATE KEY', 'RSA PRIVATE KEY', 'DSA PRIVATE KEY', diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py index 070498fe8a394802ea55f848a268e2b6563ec472..12c973c8acdcb0e657d92695dc28043b29966f2c 100644 --- a/ipaserver/install/cainstance.py +++ b/ipaserver/install/cainstance.py @@ -839,44 +839,30 @@ class CAInstance(DogtagInstance): # makes openssl throw up. data = base64.b64decode(chain) - result = ipautil.run( - [paths.OPENSSL, - "pkcs7", - "-inform", - "DER", - "-print_certs", - ], stdin=data, capture_output=True) - certlist = result.output + certlist = x509.pkcs7_to_pems(data, x509.DER) # Ok, now we have all the certificates in certs, walk through it # and pull out each certificate and add it to our database - st = 1 - en = 0 - subid = 0 ca_dn = DN(('CN','Certificate Authority'), self.subject_base) - while st > 0: - st = certlist.find('-----BEGIN', en) - en = certlist.find('-----END', en+1) - if st > 0: - try: - (chain_fd, chain_name) = tempfile.mkstemp() - os.write(chain_fd, certlist[st:en+25]) - os.close(chain_fd) - (_rdn, subject_dn) = certs.get_cert_nickname(certlist[st:en+25]) - if subject_dn == ca_dn: - nick = get_ca_nickname(self.realm) - trust_flags = 'CT,C,C' - else: - nick = str(subject_dn) - trust_flags = ',,' - self.__run_certutil( - ['-A', '-t', trust_flags, '-n', nick, '-a', - '-i', chain_name] - ) - finally: - os.remove(chain_name) - subid += 1 + for cert in certlist: + try: + (chain_fd, chain_name) = tempfile.mkstemp() + os.write(chain_fd, cert) + os.close(chain_fd) + (_rdn, subject_dn) = certs.get_cert_nickname(cert) + if subject_dn == ca_dn: + nick = get_ca_nickname(self.realm) + trust_flags = 'CT,C,C' + else: + nick = str(subject_dn) + trust_flags = ',,' + self.__run_certutil( + ['-A', '-t', trust_flags, '-n', nick, '-a', + '-i', chain_name] + ) + finally: + os.remove(chain_name) def __request_ra_certificate(self): # Create a noise file for generating our private key -- 2.5.5
From 4779e2cd4c30b896dbe914c095c0dbc068e38b63 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale <ftwee...@redhat.com> Date: Mon, 8 Aug 2016 14:27:20 +1000 Subject: [PATCH] Add options to write lightweight CA cert or chain to file Administrators need a way to retrieve the certificate or certificate chain of an IPA-managed lightweight CA. Add output attributes to `ca_show' containing the CA certificate and chain (as multiple DER values), and add the `--certificate-out' option and `--chain' flag as client-side options for writing one or the other to a file. Fixes: https://fedorahosted.org/freeipa/ticket/6178 --- ipaclient/plugins/ca.py | 47 +++++++++++++++++++++++++++++++++++++++++++++ ipaserver/plugins/ca.py | 34 ++++++++++++++++++++++++++++---- ipaserver/plugins/dogtag.py | 12 ++++++++++++ 3 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 ipaclient/plugins/ca.py diff --git a/ipaclient/plugins/ca.py b/ipaclient/plugins/ca.py new file mode 100644 index 0000000000000000000000000000000000000000..63770ff42af10f3f7d9419b692107433ddb35198 --- /dev/null +++ b/ipaclient/plugins/ca.py @@ -0,0 +1,47 @@ +# +# Copyright (C) 2016 FreeIPA Contributors see COPYING for license +# + +import base64 +from ipaclient.frontend import MethodOverride +from ipalib import util, x509, Flag, Str +from ipalib.plugable import Registry +from ipalib.text import _ + +register = Registry() + + +@register(override=True, no_fail=True) +class ca_show(MethodOverride): + + takes_options = ( + Str('certificate_out?', + doc=_('Write certificate to file'), + include='cli', + ), + Flag('chain', + default=False, + doc=_('Write certificate chain instead of single certificate'), + include='cli', + ), + ) + + def forward(self, *keys, **options): + filename = None + if 'certificate_out' in options: + filename = options.pop('certificate_out') + util.check_writable_file(filename) + chain = options.pop('chain', False) + + result = super(ca_show, self).forward(*keys, **options) + if filename: + to_pem = lambda x: x509.make_pem(base64.b64encode(x)) + if chain: + ders = result['result']['certificate_chain'] + data = '\n'.join(map(to_pem, ders)) + else: + data = to_pem(result['result']['certificate']) + with open(filename, 'wb') as f: + f.write(data) + + return result diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py index 966ae2b1bdb4bb0207dfa58f0e9c951bc930f766..2e1787707bf002455478c969cd1914692c743b9c 100644 --- a/ipaserver/plugins/ca.py +++ b/ipaserver/plugins/ca.py @@ -2,14 +2,14 @@ # Copyright (C) 2016 FreeIPA Contributors see COPYING for license # -from ipalib import api, errors, DNParam, Str +from ipalib import api, errors, Bytes, DNParam, Str from ipalib.constants import IPA_CA_CN from ipalib.plugable import Registry from ipaserver.plugins.baseldap import ( LDAPObject, LDAPSearch, LDAPCreate, LDAPDelete, LDAPUpdate, LDAPRetrieve) from ipaserver.plugins.cert import ca_enabled_check -from ipalib import _, ngettext +from ipalib import _, ngettext, x509 __doc__ = _(""" @@ -140,9 +140,35 @@ class ca_find(LDAPSearch): class ca_show(LDAPRetrieve): __doc__ = _("Display the properties of a CA.") - def execute(self, *args, **kwargs): + has_output_params = LDAPRetrieve.has_output_params + ( + Bytes( + 'certificate', + label=_("Certificate"), + doc=_("X.509 certificate"), + flags={'no_display'}, + ), + Bytes( + 'certificate_chain*', + label=_("Certificate chain"), + doc=_("PKCS #7 certificate chain"), + flags={'no_display'}, + ), + ) + + def execute(self, *keys, **options): ca_enabled_check() - return super(ca_show, self).execute(*args, **kwargs) + result = super(ca_show, self).execute(*keys, **options) + + ca_id = result['result']['ipacaid'][0] + with self.api.Backend.ra_lightweight_ca as ca_api: + result['result']['certificate'] = ca_api.read_ca_cert(ca_id) + + pkcs7_der = ca_api.read_ca_chain(ca_id) + pems = x509.pkcs7_to_pems(pkcs7_der, x509.DER) + ders = (x509.normalize_certificate(pem) for pem in pems) + result['result']['certificate_chain'] = list(ders) + + return result @register() diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py index aef1e888eb1b6c273c1fd12cbf4912407f8f8132..1fd3106e0ae723eb30dbe32c61e637790f6085d2 100644 --- a/ipaserver/plugins/dogtag.py +++ b/ipaserver/plugins/dogtag.py @@ -2205,6 +2205,18 @@ class ra_lightweight_ca(RestClient): except: raise errors.RemoteRetrieveError(reason=_("Response from CA was not valid JSON")) + def read_ca_cert(self, ca_id): + status, resp_headers, resp_body = self._ssldo( + 'GET', '{}/cert'.format(ca_id), + headers={'Accept': 'application/pkix-cert'}) + return resp_body + + def read_ca_chain(self, ca_id): + status, resp_headers, resp_body = self._ssldo( + 'GET', '{}/chain'.format(ca_id), + headers={'Accept': 'application/pkcs7-mime'}) + return resp_body + def disable_ca(self, ca_id): self._ssldo( 'POST', ca_id + '/disable', -- 2.5.5
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code