116 lines
4.7 KiB
Diff
116 lines
4.7 KiB
Diff
From d68f99203c5bab33e8bc4af6becea57e0736bbc5 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
|
|
---
|
|
ipalib/plugins/cert.py | 49 +++++++++++++++++++++++++++++++------------------
|
|
1 file changed, 31 insertions(+), 18 deletions(-)
|
|
|
|
diff --git a/ipalib/plugins/cert.py b/ipalib/plugins/cert.py
|
|
index b4ea2feae5de9ffc020709092f79791d99472ffc..f257088e2d45a0c991cce68222577dbe212415d9 100644
|
|
--- a/ipalib/plugins/cert.py
|
|
+++ b/ipalib/plugins/cert.py
|
|
@@ -243,6 +243,25 @@ def caacl_check(principal_type, principal_string, ca, profile_id):
|
|
)
|
|
)
|
|
|
|
+
|
|
+def bind_principal_can_manage_cert(cert):
|
|
+ """Check that the bind principal can manage the given cert.
|
|
+
|
|
+ ``cert``
|
|
+ An NSS certificate object.
|
|
+
|
|
+ """
|
|
+ bind_principal = getattr(context, 'principal')
|
|
+ if not bind_principal.startswith('host/'):
|
|
+ return False
|
|
+
|
|
+ hostname = get_host_from_principal(bind_principal)
|
|
+
|
|
+ # 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
|
|
+
|
|
+
|
|
@register()
|
|
class cert_request(VirtualCommand):
|
|
__doc__ = _('Submit a certificate signing request.')
|
|
@@ -608,29 +627,23 @@ class cert_show(VirtualCommand):
|
|
|
|
def execute(self, serial_number, **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 = getattr(context, 'principal')
|
|
- if not bind_principal.startswith('host/'):
|
|
- raise acierr
|
|
- hostname = get_host_from_principal(bind_principal)
|
|
|
|
result=self.Backend.ra.get_certificate(serial_number)
|
|
cert = x509.load_certificate(result['certificate'])
|
|
+
|
|
+ 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
|
|
+
|
|
result['subject'] = unicode(cert.subject)
|
|
result['issuer'] = unicode(cert.issuer)
|
|
result['valid_not_before'] = unicode(cert.valid_not_before_str)
|
|
result['valid_not_after'] = unicode(cert.valid_not_after_str)
|
|
result['md5_fingerprint'] = unicode(nss.data_to_hex(nss.md5_digest(cert.der_data), 64)[0])
|
|
result['sha1_fingerprint'] = unicode(nss.data_to_hex(nss.sha1_digest(cert.der_data), 64)[0])
|
|
- 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)
|
|
|
|
@@ -676,17 +689,17 @@ class cert_revoke(VirtualCommand):
|
|
|
|
def execute(self, serial_number, **kw):
|
|
ca_enabled_check()
|
|
- 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))['result']
|
|
+ cert = x509.load_certificate(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.5.5
|
|
|