[Freeipa-devel] [PATCH] 0081 Add --ca option to cert-revoke and cert-remove-hold

Fraser Tweedale ftweedal at redhat.com
Wed Jun 29 04:11:03 UTC 2016


Dear team,

The attached patch implements the --ca option for the rest of the
cert-blah commands (https://fedorahosted.org/freeipa/ticket/5999).

Thanks,
Fraser
-------------- next part --------------
From 668b826d94237d33e34605a5517b40c17de36780 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Wed, 29 Jun 2016 13:57:53 +1000
Subject: [PATCH] Add --ca option to cert-revoke and cert-remove-hold

Implement the --ca option for cert-revoke and cert-remove-hold.
Defaults to the IPA CA.  Raise NotFound if the cert with the given
serial was not issued by the nominated CA.

Also default the --ca option of cert-show to the IPA CA.

Add commentary to cert-status to explain why it does not use the
--ca option.

Fixes: https://fedorahosted.org/freeipa/ticket/5999
---
 API.txt                   | 10 ++++++----
 ipaserver/plugins/cert.py | 47 ++++++++++++++++++++++++++++++++---------------
 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/API.txt b/API.txt
index 76e58aeec4301577952f919b17a58b777771c06a..77a67cbc7a0533cf13404abd3a7d27972e2d65cc 100644
--- a/API.txt
+++ b/API.txt
@@ -760,8 +760,9 @@ output: ListOfEntries('result')
 output: Output('summary', type=[<type 'unicode'>, <type 'NoneType'>])
 output: Output('truncated', type=[<type 'bool'>])
 command: cert_remove_hold/1
-args: 1,1,1
+args: 1,2,1
 arg: Int('serial_number')
+option: Str('cacn?', autofill=True, cli_name='ca', default=u'ipa')
 option: Str('version?')
 output: Output('result')
 command: cert_request/1
@@ -769,7 +770,7 @@ args: 1,8,3
 arg: Str('csr', cli_name='csr_file')
 option: Flag('add', autofill=True, default=False)
 option: Flag('all', autofill=True, cli_name='all', default=False)
-option: Str('cacn?', cli_name='ca')
+option: Str('cacn?', autofill=True, cli_name='ca', default=u'ipa')
 option: Str('principal')
 option: Str('profile_id?')
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
@@ -779,8 +780,9 @@ output: Entry('result')
 output: Output('summary', type=[<type 'unicode'>, <type 'NoneType'>])
 output: PrimaryKey('value')
 command: cert_revoke/1
-args: 1,2,1
+args: 1,3,1
 arg: Int('serial_number')
+option: Str('cacn?', autofill=True, cli_name='ca', default=u'ipa')
 option: Int('revocation_reason', autofill=True, default=0)
 option: Str('version?')
 output: Output('result')
@@ -788,7 +790,7 @@ command: cert_show/1
 args: 1,6,3
 arg: Int('serial_number')
 option: Flag('all', autofill=True, cli_name='all', default=False)
-option: Str('cacn?', cli_name='ca')
+option: Str('cacn?', autofill=True, cli_name='ca', default=u'ipa')
 option: Flag('no_members', autofill=True, default=False)
 option: Str('out?')
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 564d582c77ef63e780604fd7fc55e6cc7889a351..5137a1e1cff7554b3aca8a6f830199f220fcaa2b 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -321,6 +321,8 @@ class BaseCertMethod(Method):
     def get_options(self):
         yield Str('cacn?',
             cli_name='ca',
+            default=IPA_CA_CN,
+            autofill=True,
             query=True,
             label=_('Issuing CA'),
             doc=_('Name of issuing CA'),
@@ -408,7 +410,7 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):
         # enforcement so that user gets better error message if
         # referencing nonexistant CA) and look up authority ID.
         #
-        ca = kw.get('cacn', IPA_CA_CN)
+        ca = kw['cacn']
         ca_id = api.Command.ca_show(ca)['result']['ipacaid'][0]
 
         """
@@ -624,6 +626,8 @@ class cert_status(Retrieve, BaseCertMethod, VirtualCommand):
     def get_options(self):
         for option in super(cert_status, self).get_options():
             if option.name == 'cacn':
+                # Dogtag requests are uniquely identified by their
+                # number; there is no need to distinguish by CA.
                 continue
             yield option
 
@@ -734,10 +738,8 @@ class cert_show(Retrieve, CertMethod, VirtualCommand):
                 raise acierr
             hostname = get_host_from_principal(bind_principal)
 
-        issuer_dn = None
-        if 'cacn' in options:
-            ca_obj = api.Command.ca_show(options['cacn'])['result']
-            issuer_dn = ca_obj['ipacasubjectdn'][0]
+        ca_obj = api.Command.ca_show(options['cacn'])['result']
+        issuer_dn = ca_obj['ipacasubjectdn'][0]
 
         # Dogtag lightweight CAs have shared serial number domain, so
         # we don't tell Dogtag the issuer (but we check the cert after).
@@ -745,7 +747,7 @@ class cert_show(Retrieve, CertMethod, VirtualCommand):
         result = self.Backend.ra.get_certificate(str(serial_number))
         cert = x509.load_certificate(result['certificate'])
 
-        if issuer_dn is not None and DN(unicode(cert.issuer)) != DN(issuer_dn):
+        if DN(unicode(cert.issuer)) != DN(issuer_dn):
             # DN of cert differs from what we requested
             raise errors.NotFound(
                 reason=_("Certificate with serial number %(serial)s "
@@ -796,12 +798,16 @@ class cert_revoke(PKQuery, CertMethod, VirtualCommand):
         )
 
         for option in super(cert_revoke, self).get_options():
-            if option.name == 'cacn':
-                continue
             yield option
 
     def execute(self, serial_number, **kw):
         ca_enabled_check()
+
+        # Make sure that the cert specified by issuer+serial exists.
+        # Will raise NotFound if it does not.
+        cert_show_options = dict(cacn=kw['cacn'])
+        api.Command.cert_show(unicode(serial_number), **cert_show_options)
+
         hostname = None
         try:
             self.check_access()
@@ -810,13 +816,18 @@ class cert_revoke(PKQuery, CertMethod, VirtualCommand):
             try:
                 # Let cert_show() handle verifying that the subject of the
                 # cert we're dealing with matches the hostname in the principal
-                result = api.Command['cert_show'](unicode(serial_number))['result']
+                result = api.Command['cert_show'](
+                    unicode(serial_number), **cert_show_options
+                )['result']
             except errors.NotImplementedError:
                 pass
         revocation_reason = kw['revocation_reason']
         if revocation_reason == 7:
             raise errors.CertificateOperationError(error=_('7 is not a valid revocation reason'))
         return dict(
+            # Dogtag lightweight CAs have shared serial number domain, so
+            # we don't tell Dogtag the issuer (but we already checked that
+            # the given serial was issued by the named ca).
             result=self.Backend.ra.revoke_certificate(
                 str(serial_number), revocation_reason=revocation_reason)
         )
@@ -837,16 +848,18 @@ class cert_remove_hold(PKQuery, CertMethod, VirtualCommand):
     )
     operation = "certificate remove hold"
 
-    def get_options(self):
-        for option in super(cert_remove_hold, self).get_options():
-            if option.name == 'cacn':
-                continue
-            yield option
-
     def execute(self, serial_number, **kw):
         ca_enabled_check()
+
+        # Make sure that the cert specified by issuer+serial exists.
+        # Will raise NotFound if it does not.
+        api.Command.cert_show(serial_number, cacn=kw['cacn'])
+
         self.check_access()
         return dict(
+            # Dogtag lightweight CAs have shared serial number domain, so
+            # we don't tell Dogtag the issuer (but we already checked that
+            # the given serial was issued by the named ca).
             result=self.Backend.ra.take_certificate_off_hold(
                 str(serial_number))
         )
@@ -944,6 +957,10 @@ class cert_find(Search, CertMethod):
             if option.name == 'no_members':
                 option = option.clone(default=True,
                                       flags=set(option.flags) | {'no_option'})
+            elif option.name == 'cacn':
+                # make CA optional, so that user may directly
+                # specify Issuer DN instead
+                option = option.clone(default=None, autofill=None)
             yield option
 
         for owner in self.obj._owners():
-- 
2.5.5



More information about the Freeipa-devel mailing list