[Freeipa-devel] [PATCH] 0097 Add options to write lightweight CA cert or chain to file

Fraser Tweedale ftweedal at redhat.com
Tue Aug 16 14:09:39 UTC 2016


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
-------------- next part --------------
From 77d7d0185bcf3cb86f56bd128507a8e2cfedc775 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at 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

-------------- next part --------------
From 4779e2cd4c30b896dbe914c095c0dbc068e38b63 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at 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()
+
+
+ at 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



More information about the Freeipa-devel mailing list