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

Reply via email to