[Freeipa-devel] [PATCH] 0084 cert-revoke: fix permission check bypass
Fraser Tweedale
ftweedal at redhat.com
Fri Aug 19 10:55:10 UTC 2016
This patch fixes CVE-2016-5404. Versions for master, ipa-4-3 and
ipa-4-2 branches are attached.
Thanks,
Fraser
-------------- next part --------------
From 61590c223aa51668b3f661fc91bc35f2dfae8ae6 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at 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
---
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.5.5
-------------- next part --------------
From d68f99203c5bab33e8bc4af6becea57e0736bbc5 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at 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
-------------- next part --------------
From aa114a658b1f30e45f905bb6e19b04e9504da8a7 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at 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 7a07039a8488cc11d9bf05ef23642b8059d5921e..42dc4f571b9274f45bd6c20910362cf676764f3a 100644
--- a/ipalib/plugins/cert.py
+++ b/ipalib/plugins/cert.py
@@ -236,6 +236,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.')
@@ -601,29 +620,23 @@ class cert_show(VirtualCommand):
def execute(self, serial_number, **options):
ca_enabled_check()
- hostname = None
- try:
- self.check_access()
- except errors.ACIError, 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)
@@ -669,17 +682,17 @@ class cert_revoke(VirtualCommand):
def execute(self, serial_number, **kw):
ca_enabled_check()
- hostname = None
try:
self.check_access()
except errors.ACIError, 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
More information about the Freeipa-devel
mailing list