From 18d4af260f777920dc2b474a588c3dcda20d8801 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale 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 --- 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