The Identity, Policy and Audit system
CentOS Sources
2016-11-03 403b09ab980c02ef36095973349a13e0181c794a
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
From 18d4af260f777920dc2b474a588c3dcda20d8801 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal@redhat.com>
Date: Thu, 30 Jun 2016 10:21:01 +1000
Subject: [PATCH] cert-revoke: fix permission check bypass (CVE-2016-5404)
 
The 'cert_revoke' command checks the 'revoke certificate'
permission, however, if an ACIError is raised, it then invokes the
'cert_show' command.  The rational was to re-use a "host manages
certificate" check that is part of the 'cert_show' command, however,
it is sufficient that 'cert_show' executes successfully for
'cert_revoke' to recover from the ACIError continue.  Therefore,
anyone with 'retrieve certificate' permission can revoke *any*
certificate and cause various kinds of DoS.
 
Fix the problem by extracting the "host manages certificate" check
to its own method and explicitly calling it from 'cert_revoke'.
 
Fixes: https://fedorahosted.org/freeipa/ticket/6232
Reviewed-By: Jan Cholasta <jcholast@redhat.com>
---
 ipaserver/plugins/cert.py | 60 +++++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 28 deletions(-)
 
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index b8df074a186ca91daa8e8f5e725724ea7bc5a663..6dd9f6ffcdcd9d051d50d912996fea2104d71dff 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -242,6 +242,24 @@ def validate_certificate(value):
     return x509.validate_certificate(value, x509.DER)
 
 
+def bind_principal_can_manage_cert(cert):
+    """Check that the bind principal can manage the given cert.
+
+    ``cert``
+        An NSS certificate object.
+
+    """
+    bind_principal = kerberos.Principal(getattr(context, 'principal'))
+    if not bind_principal.is_host:
+        return False
+
+    hostname = bind_principal.hostname
+
+    # If we have a hostname we want to verify that the subject
+    # of the certificate matches it.
+    return hostname == cert.subject.common_name  #pylint: disable=E1101
+
+
 class BaseCertObject(Object):
     takes_params = (
         Bytes(
@@ -744,18 +762,6 @@ class cert_show(Retrieve, CertMethod, VirtualCommand):
     def execute(self, serial_number, all=False, raw=False, no_members=False,
                 **options):
         ca_enabled_check()
-        hostname = None
-        try:
-            self.check_access()
-        except errors.ACIError as acierr:
-            self.debug("Not granted by ACI to retrieve certificate, looking at principal")
-            bind_principal = kerberos.Principal(getattr(context, 'principal'))
-            if not bind_principal.is_host:
-                raise acierr
-            hostname = bind_principal.hostname
-
-        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).
@@ -763,7 +769,15 @@ class cert_show(Retrieve, CertMethod, VirtualCommand):
         result = self.Backend.ra.get_certificate(str(serial_number))
         cert = x509.load_certificate(result['certificate'])
 
-        if DN(unicode(cert.issuer)) != DN(issuer_dn):
+        try:
+            self.check_access()
+        except errors.ACIError as acierr:
+            self.debug("Not granted by ACI to retrieve certificate, looking at principal")
+            if not bind_principal_can_manage_cert(cert):
+                raise acierr  # pylint: disable=E0702
+
+        ca_obj = api.Command.ca_show(options['cacn'])['result']
+        if DN(unicode(cert.issuer)) != DN(ca_obj['ipacasubjectdn'][0]):
             # DN of cert differs from what we requested
             raise errors.NotFound(
                 reason=_("Certificate with serial number %(serial)s "
@@ -789,12 +803,6 @@ class cert_show(Retrieve, CertMethod, VirtualCommand):
             result['revoked'] = ('revocation_reason' in result)
             self.obj._fill_owners(result)
 
-        if hostname:
-            # If we have a hostname we want to verify that the subject
-            # of the certificate matches it, otherwise raise an error
-            if hostname != cert.subject.common_name:    #pylint: disable=E1101
-                raise acierr
-
         return dict(result=result, value=pkey_to_value(serial_number, options))
 
 
@@ -819,22 +827,18 @@ class cert_revoke(PKQuery, CertMethod, VirtualCommand):
 
         # 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)
+        resp = api.Command.cert_show(unicode(serial_number), cacn=kw['cacn'])
 
-        hostname = None
         try:
             self.check_access()
         except errors.ACIError as acierr:
             self.debug("Not granted by ACI to revoke certificate, looking at principal")
             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), **cert_show_options
-                )['result']
+                cert = x509.load_certificate(resp['result']['certificate'])
+                if not bind_principal_can_manage_cert(cert):
+                    raise acierr
             except errors.NotImplementedError:
-                pass
+                raise acierr
         revocation_reason = kw['revocation_reason']
         if revocation_reason == 7:
             raise errors.CertificateOperationError(error=_('7 is not a valid revocation reason'))
-- 
2.9.3