URL: https://github.com/freeipa/freeipa/pull/490 Author: HonzaCholasta Title: #490: certdb: use certutil and match_hostname for cert verification Action: synchronized
To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/490/head:pr490 git checkout pr490
From 4b315119f6b99dafec77326757adbb0d8db39001 Mon Sep 17 00:00:00 2001 From: Jan Cholasta <jchol...@redhat.com> Date: Mon, 2 Jan 2017 13:53:18 +0100 Subject: [PATCH 1/2] certdb: use certutil and match_hostname for cert verification Use certutil and ssl.match_hostname calls instead of python-nss for certificate verification. --- freeipa.spec.in | 16 +++++------ ipalib/x509.py | 71 ++++++++++++++++++++++++++++++++++++----------- ipapython/certdb.py | 80 ++++++++++++++++++++--------------------------------- ipasetup.py.in | 2 +- 4 files changed, 94 insertions(+), 75 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index 829c3f0..a588683 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -160,8 +160,8 @@ BuildRequires: python3-wheel # %if 0%{?with_lint} BuildRequires: samba-python -# 1.4: the version where Certificate.serial changed to .serial_number -BuildRequires: python-cryptography >= 1.4 +# 1.6: x509.Name.rdns (https://github.com/pyca/cryptography/issues/3199) +BuildRequires: python-cryptography >= 1.6 BuildRequires: python-gssapi >= 1.2.0 BuildRequires: pylint >= 1.6 # workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1096506 @@ -196,8 +196,8 @@ BuildRequires: python2-jinja2 %if 0%{?with_python3} # FIXME: this depedency is missing - server will not work #BuildRequires: python3-samba -# 1.4: the version where Certificate.serial changed to .serial_number -BuildRequires: python3-cryptography >= 1.4 +# 1.6: x509.Name.rdns (https://github.com/pyca/cryptography/issues/3199) +BuildRequires: python3-cryptography >= 1.6 BuildRequires: python3-gssapi >= 1.2.0 BuildRequires: python3-pylint >= 1.6 # workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1096506 @@ -636,7 +636,7 @@ Requires: gnupg Requires: keyutils Requires: pyOpenSSL Requires: python-nss >= 0.16 -Requires: python-cryptography >= 1.4 +Requires: python-cryptography >= 1.6 Requires: python-netaddr Requires: python-libipa_hbac Requires: python-qrcode-core >= 5.0.0 @@ -685,7 +685,7 @@ Requires: gnupg Requires: keyutils Requires: python3-pyOpenSSL Requires: python3-nss >= 0.16 -Requires: python3-cryptography >= 1.4 +Requires: python3-cryptography >= 1.6 Requires: python3-netaddr Requires: python3-libipa_hbac Requires: python3-qrcode-core >= 5.0.0 @@ -760,7 +760,7 @@ Requires: python-pytest-multihost >= 0.5 Requires: python-pytest-sourceorder Requires: ldns-utils Requires: python-sssdconfig -Requires: python2-cryptography >= 1.4 +Requires: python2-cryptography >= 1.6 Provides: %{alt_name}-tests = %{version} Conflicts: %{alt_name}-tests @@ -794,7 +794,7 @@ Requires: python3-pytest-multihost >= 0.5 Requires: python3-pytest-sourceorder Requires: ldns-utils Requires: python3-sssdconfig -Requires: python3-cryptography >= 1.4 +Requires: python3-cryptography >= 1.6 %description -n python3-ipatests IPA is an integrated solution to provide centrally managed Identity (users, diff --git a/ipalib/x509.py b/ipalib/x509.py index f65cf81..dbcbb59 100644 --- a/ipalib/x509.py +++ b/ipalib/x509.py @@ -35,6 +35,7 @@ import binascii import datetime import ipaddress +import ssl import base64 import re @@ -49,6 +50,7 @@ from ipalib import util from ipalib import errors from ipapython.dn import DN +from ipapython.dnsutil import DNSName if six.PY3: unicode = str @@ -406,6 +408,27 @@ def process_othernames(gns): yield gn +def _pyasn1_get_san_general_names(cert): + tbs = decoder.decode( + cert.tbs_certificate_bytes, + asn1Spec=rfc2459.TBSCertificate() + )[0] + OID_SAN = univ.ObjectIdentifier('2.5.29.17') + # One would expect KeyError or empty iterable when the key ('extensions' + # in this particular case) is not pressent in the certificate but pyasn1 + # returns None here + extensions = tbs['extensions'] or [] + gns = [] + for ext in extensions: + if ext['extnID'] == OID_SAN: + der = decoder.decode( + ext['extnValue'], asn1Spec=univ.OctetString())[0] + gns = decoder.decode(der, asn1Spec=rfc2459.SubjectAltName())[0] + break + + return gns + + def get_san_general_names(cert): """ Return SAN general names from a python-cryptography @@ -430,22 +453,7 @@ def get_san_general_names(cert): and should go away. """ - tbs = decoder.decode( - cert.tbs_certificate_bytes, - asn1Spec=rfc2459.TBSCertificate() - )[0] - OID_SAN = univ.ObjectIdentifier('2.5.29.17') - # One would expect KeyError or empty iterable when the key ('extensions' - # in this particular case) is not pressent in the certificate but pyasn1 - # returns None here - extensions = tbs['extensions'] or [] - gns = [] - for ext in extensions: - if ext['extnID'] == OID_SAN: - der = decoder.decode( - ext['extnValue'], asn1Spec=univ.OctetString())[0] - gns = decoder.decode(der, asn1Spec=rfc2459.SubjectAltName())[0] - break + gns = _pyasn1_get_san_general_names(cert) GENERAL_NAME_CONSTRUCTORS = { 'rfc822Name': lambda x: cryptography.x509.RFC822Name(unicode(x)), @@ -504,6 +512,17 @@ def _pyasn1_to_cryptography_oid(oid): return cryptography.x509.ObjectIdentifier(str(oid)) +def get_san_a_label_dns_names(cert): + gns = _pyasn1_get_san_general_names(cert) + result = [] + + for gn in gns: + if gn.getName() == 'dNSName': + result.append(unicode(gn.getComponent())) + + return result + + def chunk(size, s): """Yield chunks of the specified size from the given string. @@ -543,3 +562,23 @@ def format_datetime(t): if t.tzinfo is None: t = t.replace(tzinfo=UTC()) return unicode(t.strftime("%a %b %d %H:%M:%S %Y %Z")) + + +def match_hostname(cert, hostname): + match_cert = {} + + match_cert['subject'] = match_subject = [] + for rdn in cert.subject.rdns: + match_rdn = [] + for ava in rdn: + if ava.oid == cryptography.x509.oid.NameOID.COMMON_NAME: + match_rdn.append(('commonName', ava.value)) + match_subject.append(match_rdn) + + values = get_san_a_label_dns_names(cert) + if values: + match_cert['subjectAltName'] = match_san = [] + for value in values: + match_san.append(('DNS', value)) + + ssl.match_hostname(match_cert, DNSName(hostname).ToASCII()) diff --git a/ipapython/certdb.py b/ipapython/certdb.py index f1410e5..82b3869 100644 --- a/ipapython/certdb.py +++ b/ipapython/certdb.py @@ -25,9 +25,9 @@ import tempfile import shutil import base64 + from cryptography.hazmat.primitives import serialization -from nss import nss -from nss.error import NSPRError +import cryptography.x509 from ipapython.dn import DN from ipapython.ipa_log_manager import root_logger @@ -543,59 +543,39 @@ def verify_server_cert_validity(self, nickname, hostname): Raises a ValueError if the certificate is invalid. """ - certdb = cert = None - if nss.nss_is_initialized(): - nss.nss_shutdown() - nss.nss_init(self.secdir) + cert = self.get_cert(nickname) + cert = x509.load_certificate(cert, x509.DER) + try: - certdb = nss.get_default_certdb() - cert = nss.find_cert_from_nickname(nickname) - intended_usage = nss.certificateUsageSSLServer - try: - approved_usage = cert.verify_now(certdb, True, intended_usage) - except NSPRError as e: - if e.errno != -8102: - raise ValueError(e.strerror) - approved_usage = 0 - if not approved_usage & intended_usage: - raise ValueError('invalid for a SSL server') - if not cert.verify_hostname(hostname): - raise ValueError('invalid for server %s' % hostname) - finally: - del certdb, cert - nss.nss_shutdown() + self.run_certutil(['-V', '-n', nickname, '-u', 'V']) + except ipautil.CalledProcessError: + raise ValueError('invalid for a SSL server') - return None + try: + x509.match_hostname(cert, hostname) + except ValueError: + raise ValueError('invalid for server %s' % hostname) def verify_ca_cert_validity(self, nickname): - certdb = cert = None - if nss.nss_is_initialized(): - nss.nss_shutdown() - nss.nss_init(self.secdir) + cert = self.get_cert(nickname) + cert = x509.load_certificate(cert, x509.DER) + + if not cert.subject: + raise ValueError("has empty subject") + try: - certdb = nss.get_default_certdb() - cert = nss.find_cert_from_nickname(nickname) - if not cert.subject: - raise ValueError("has empty subject") - try: - bc = cert.get_extension(nss.SEC_OID_X509_BASIC_CONSTRAINTS) - except KeyError: - raise ValueError("missing basic constraints") - bc = nss.BasicConstraints(bc.value) - if not bc.is_ca: - raise ValueError("not a CA certificate") - intended_usage = nss.certificateUsageSSLCA - try: - approved_usage = cert.verify_now(certdb, True, intended_usage) - except NSPRError as e: - if e.errno != -8102: # SEC_ERROR_INADEQUATE_KEY_USAGE - raise ValueError(e.strerror) - approved_usage = 0 - if approved_usage & intended_usage != intended_usage: - raise ValueError('invalid for a CA') - finally: - del certdb, cert - nss.nss_shutdown() + bc = cert.extensions.get_extension_for_class( + cryptography.x509.BasicConstraints) + except cryptography.x509.ExtensionNotFound: + raise ValueError("missing basic constraints") + + if not bc.ca: + raise ValueError("not a CA certificate") + + try: + self.run_certutil(['-V', '-n', nickname, '-u', 'L']) + except ipautil.CalledProcessError: + raise ValueError('invalid for a CA') def publish_ca_cert(self, canickname, location): args = ["-L", "-n", canickname, "-a"] diff --git a/ipasetup.py.in b/ipasetup.py.in index 7f9b2c9..979e683 100644 --- a/ipasetup.py.in +++ b/ipasetup.py.in @@ -63,7 +63,7 @@ if SETUPTOOLS_VERSION < (8, 0, 0): PACKAGE_VERSION = { - 'cryptography': 'cryptography >= 1.4', + 'cryptography': 'cryptography >= 1.6', 'custodia': 'custodia >= 0.3.1', 'dnspython': 'dnspython >= 1.15', 'gssapi': 'gssapi >= 1.2.0', From 25c3bd2b587cebe638c7214dc1a9802b0164ad10 Mon Sep 17 00:00:00 2001 From: Jan Cholasta <jchol...@redhat.com> Date: Thu, 30 Mar 2017 10:26:29 +0000 Subject: [PATCH 2/2] setup, pylint, spec file: drop python-nss dependency Remove the unused python-nss dependency. --- freeipa.spec.in | 3 --- ipalib/setup.py | 1 - ipapython/setup.py | 1 - ipasetup.py.in | 1 - ipatests/setup.py | 2 +- pylintrc | 3 +-- 6 files changed, 2 insertions(+), 9 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index a588683..a6100ec 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -134,7 +134,6 @@ BuildRequires: python-lesscpy # makeapi/makeaci is using Python 2 only for now # BuildRequires: python-ldap -BuildRequires: python-nss BuildRequires: python-netaddr BuildRequires: python-pyasn1 BuildRequires: python-pyasn1-modules @@ -635,7 +634,6 @@ Requires: python-gssapi >= 1.2.0 Requires: gnupg Requires: keyutils Requires: pyOpenSSL -Requires: python-nss >= 0.16 Requires: python-cryptography >= 1.6 Requires: python-netaddr Requires: python-libipa_hbac @@ -684,7 +682,6 @@ Requires: python3-gssapi >= 1.2.0 Requires: gnupg Requires: keyutils Requires: python3-pyOpenSSL -Requires: python3-nss >= 0.16 Requires: python3-cryptography >= 1.6 Requires: python3-netaddr Requires: python3-libipa_hbac diff --git a/ipalib/setup.py b/ipalib/setup.py index 4239f0c..cdbd61c 100644 --- a/ipalib/setup.py +++ b/ipalib/setup.py @@ -41,7 +41,6 @@ "netaddr", "pyasn1", "pyasn1-modules", - "python-nss", "six", ], extras_require={ diff --git a/ipapython/setup.py b/ipapython/setup.py index 2fc039f..f4bc3f8 100755 --- a/ipapython/setup.py +++ b/ipapython/setup.py @@ -46,7 +46,6 @@ "pyldap", "netaddr", "netifaces", - "python-nss", "requests", "six", ], diff --git a/ipasetup.py.in b/ipasetup.py.in index 979e683..0940f5f 100644 --- a/ipasetup.py.in +++ b/ipasetup.py.in @@ -75,7 +75,6 @@ PACKAGE_VERSION = { 'kdcproxy': 'kdcproxy >= 0.3', 'netifaces': 'netifaces >= 0.10.4', 'pyldap': 'pyldap >= 2.4.15', - 'python-nss': 'python-nss >= 0.16', 'python-yubico': 'python-yubico >= 1.2.3', 'qrcode': 'qrcode >= 5.0', } diff --git a/ipatests/setup.py b/ipatests/setup.py index 2f67ec4..4c02c79 100644 --- a/ipatests/setup.py +++ b/ipatests/setup.py @@ -75,7 +75,7 @@ ], extras_require={ "integration": ["dbus-python", "pyyaml", "ipaserver"], - "ipaserver": ["ipaserver", "python-nss"], + "ipaserver": ["ipaserver"], "webui": ["selenium", "pyyaml", "ipaserver"], "xmlrpc": ["ipaserver"], } diff --git a/pylintrc b/pylintrc index bff2305..f92adee 100644 --- a/pylintrc +++ b/pylintrc @@ -16,8 +16,7 @@ extension-pkg-whitelist= _ldap, cryptography, gssapi, - netifaces, - nss + netifaces [MESSAGES CONTROL]
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code