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

Jan Cholasta jcholast at redhat.com
Thu Oct 6 08:02:55 UTC 2016


On 23.9.2016 05:29, Fraser Tweedale wrote:
> Bump for review.
>
> Rebased patches attached (there was a trivial conflict in imports).
>
> Thanks,
> Fraser
>
> On Tue, Sep 06, 2016 at 02:05:06AM +1000, Fraser Tweedale wrote:
>> On Fri, Aug 26, 2016 at 10:28:58AM +0200, Jan Cholasta wrote:
>>> On 19.8.2016 13:11, Fraser Tweedale wrote:
>>>> Bump for review.
>>>>
>>>> On Wed, Aug 17, 2016 at 12:09:39AM +1000, Fraser Tweedale wrote:
>>>>> 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).
>>>
>>> I don't like it, but OK. Another alternative would be to use pyasn1.
>>>
>> I don't like it either, but neither did I like the idea of
>> reimplementing the wheel with pyasn1.  Now is not the time for
>> busywork :)
>>
>>>>>
>>>>> 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.
>>>
>>> I don't have a strong feeling about this one way or the other, but the same
>>> scheme should be used for cert-show in the future. Does it make sense to do
>>> it this way for cert-show?
>>>
>>> I'm not sure about always returning the chain in cert-show. Now that we have
>>> a --chain flag rather than two out options, maybe we should go back to
>>> returning the chain only if --chain is specified. What do you think?
>>>
>> I think we should go for consistency and always include both over
>> the wire.

My point exactly - ca-show output should be equivalent to cert-show on 
the CA certificate, as far as the certificate and chain are concerned.

> If we want to hide cert or chain or both at the `ipa' CLI
>> depending on options, I also don't feel strongly either way.  For
>> now they're both displayed.

I think I would prefer if the certificate was always returned by the 
server, but the chain only if --chain (or --all) is specified.

Additionally, ca-add should also get the new options and do all of this.

>>
>>>
>>> Patch 0099:
>>>
>>> 1) Please fix this:
>>>
>>> $ git show -U0 | pep8 --diff
>>> ./ipalib/x509.py:59:80: E501 line too long (93 > 79 characters)
>>>
>> Done.
>>
>>>
>>> Patch 0097:
>>>
>>> 1) `certificate` and `certificate_chain` are actually attributes of the ca
>>> object, so they should be defined in ca.takes_params rather than
>>> ca_show.has_output_params.
>>>
>> Done.  Out of interest, now that they are part of ca_takes_params is
>> there a way to hide them by default in CLI output, and only show
>> them when `--all' is given?
>>
>>>
>>> 2) Please fix these:
>>>
>>> $ git show -U0 | pep8 --diff
>>> ./ipaclient/plugins/ca.py:21:9: E124 closing bracket does not match visual
>>> indentation
>>> ./ipaclient/plugins/ca.py:23:13: E128 continuation line under-indented for
>>> visual indent
>>> ./ipaclient/plugins/ca.py:24:13: E128 continuation line under-indented for
>>> visual indent
>>> ./ipaclient/plugins/ca.py:25:13: E128 continuation line under-indented for
>>> visual indent
>>> ./ipaclient/plugins/ca.py:26:9: E124 closing bracket does not match visual
>>> indentation
>>> ./ipaclient/plugins/ca.py:38:13: E731 do not assign a lambda expression, use
>>> a def
>>>
>> Done.  Updated patches attached.
>>
>> Thanks,
>> Fraser
>
>> From 046b3dd078c4ccc3732a0106786bae4c01d30a89 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                  | 23 +++++++++++++++++-
>>  ipapython/certdb.py             | 14 ++++-------
>>  ipaserver/install/cainstance.py | 52 +++++++++++++++--------------------------
>>  3 files changed, 45 insertions(+), 44 deletions(-)
>>
>> diff --git a/ipalib/x509.py b/ipalib/x509.py
>> index e986a97a58aafd3aeab08765a397edbf67c7841a..0461553a73e3862c85f1ffcfe4432cabf4fdf7a1 100644
>> --- a/ipalib/x509.py
>> +++ b/ipalib/x509.py
>> @@ -51,11 +51,14 @@ 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'
>> @@ -148,6 +151,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

Moving this to the try block is a completely unnecessary change.

>>
>>                      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 c4b8e9ae326fb7ebda9e927cd4d0b5bad9743db4..f57c724b0273a275f8146f0d6055e2ee2e51192c 100644
>> --- a/ipaserver/install/cainstance.py
>> +++ b/ipaserver/install/cainstance.py
>> @@ -844,44 +844,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 fba36bd2b86c2aee1d77e05aa563ced4633ab182 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 params to the `ca'
>> object for carrying 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     | 50 +++++++++++++++++++++++++++++++++++++++++++++
>>  ipaserver/plugins/ca.py     | 31 ++++++++++++++++++++++++----
>>  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..f7e55dec196495f820ebf745eb49e8ddce6b3ee7
>> --- /dev/null
>> +++ b/ipaclient/plugins/ca.py
>> @@ -0,0 +1,50 @@
>> +#
>> +# 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:
>> +            def to_pem(x):
>> +                return x509.make_pem(base64.b64encode(x))
>> +            if chain:
>> +                ders = result['result']['certificate_chain']
>> +                data = '\n'.join(map(to_pem, ders))

Generator expressions are generally preferred over map():

     data = '\n'.join(to_pem(der) for der in 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..0684ddaed0ebfcab8910c1ea356550b504af15e2 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__ = _("""
>> @@ -79,6 +79,18 @@ class ca(LDAPObject):
>>              doc=_('Issuer Distinguished Name'),
>>              flags=['no_create', 'no_update'],
>>          ),
>> +        Bytes(
>> +            'certificate',
>> +            label=_("Certificate"),
>> +            doc=_("X.509 certificate"),
>> +            flags={'no_create', 'no_update', 'no_search', 'no_display'},

Note that the no_display flag currently does nothing.


>> +        ),
>> +        Bytes(
>> +            'certificate_chain*',
>> +            label=_("Certificate chain"),
>> +            doc=_("PKCS #7 certificate chain"),

Looks like you forgot to update the doc string.

>> +            flags={'no_create', 'no_update', 'no_search', 'no_display'},
>> +        ),
>>      )
>>
>>      permission_filter_objectclasses = ['ipaca']
>> @@ -140,9 +152,20 @@ class ca_find(LDAPSearch):
>>  class ca_show(LDAPRetrieve):
>>      __doc__ = _("Display the properties of a CA.")
>>
>> -    def execute(self, *args, **kwargs):
>> +    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)

I would think list comprehension is the obvious choice over generator 
expression when generating a list:

     ders = [x509.normalize_certificate(pem) for pem in pems]
     result['result']['certificate_chain'] = 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
>>

-- 
Jan Cholasta




More information about the Freeipa-devel mailing list