URL: https://github.com/freeipa/freeipa/pull/1111
Author: stlaz
Title: #1111: ipalib.x509 autumn cleanup
Action: opened
PR body:
"""
This is a cleanup of the x509 module which removes an ugly hack that prevents a
circular dependency and the `strip_header()` function which is properly
replaced by the `load_unknown_x509_certificate()` function where needed.
"""
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/1111/head:pr1111
git checkout pr1111
From 087bda72c1f5a54aa4571ed930089ac4dff1e2a3 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Mon, 25 Sep 2017 09:54:53 +0200
Subject: [PATCH 1/2] x509: remove the strip_header() function
We don't need the strip_header() function, to load an unknown
x509 certificate, load_unknown_x509_certificate() should be used.
---
ipaclient/plugins/cert.py | 3 ++-
ipaclient/plugins/certmap.py | 2 +-
ipalib/x509.py | 15 ---------------
ipatests/test_integration/test_caless.py | 13 ++++++-------
ipatests/test_xmlrpc/testcert.py | 19 +++++++++++++++++--
5 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/ipaclient/plugins/cert.py b/ipaclient/plugins/cert.py
index d5daaf3a15..19b8bb9317 100644
--- a/ipaclient/plugins/cert.py
+++ b/ipaclient/plugins/cert.py
@@ -209,6 +209,7 @@ def forward(self, *args, **options):
raise errors.MutuallyExclusiveError(
reason=_("cannot specify both raw certificate and file"))
if 'certificate' not in options and 'file' in options:
- options['certificate'] = x509.strip_header(options.pop('file'))
+ options['certificate'] = x509.load_unknown_x509_certificate(
+ options.pop('file'))
return super(cert_find, self).forward(*args, **options)
diff --git a/ipaclient/plugins/certmap.py b/ipaclient/plugins/certmap.py
index 50a594f396..981ba292f6 100644
--- a/ipaclient/plugins/certmap.py
+++ b/ipaclient/plugins/certmap.py
@@ -40,7 +40,7 @@ def forward(self, *args, **options):
raise errors.MutuallyExclusiveError(
reason=_("cannot specify both raw certificate and file"))
if args:
- args = [x509.strip_header(args[0])]
+ args = [x509.load_unknown_x509_certificate(args[0])]
elif 'certificate' in options:
args = [options.pop('certificate')]
else:
diff --git a/ipalib/x509.py b/ipalib/x509.py
index 0b3a423ce3..9e2b365ea2 100644
--- a/ipalib/x509.py
+++ b/ipalib/x509.py
@@ -88,21 +88,6 @@ def subject_base():
return _subject_base
-def strip_header(pem):
- """
- Remove the header and footer from a certificate.
- """
- regexp = (
- u"^-----BEGIN CERTIFICATE-----(.*?)-----END CERTIFICATE-----"
- )
- if isinstance(pem, bytes):
- regexp = regexp.encode('ascii')
- s = re.search(regexp, pem, re.MULTILINE | re.DOTALL)
- if s is not None:
- return s.group(1)
- else:
- return pem
-
@crypto_utils.register_interface(crypto_x509.Certificate)
class IPACertificate(object):
diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index 994396dd7b..4037a5e514 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -359,10 +359,9 @@ def verify_installation(self):
expected_cacrt = f.read()
logger.debug('Expected /etc/ipa/ca.crt contents:\n%s',
expected_cacrt)
- expected_binary_cacrt = base64.b64decode(x509.strip_header(
- expected_cacrt))
+ expected_cacrt = x509.load_unknown_x509_certificate(expected_cacrt)
logger.debug('Expected binary CA cert:\n%r',
- expected_binary_cacrt)
+ expected_cacrt)
for host in [self.master] + self.replicas:
# Check the LDAP entry
ldap = host.ldap_connect()
@@ -372,7 +371,7 @@ def verify_installation(self):
cert_from_ldap = entry.single_value['cACertificate']
logger.debug('CA cert from LDAP on %s:\n%r',
host, cert_from_ldap)
- assert cert_from_ldap == expected_binary_cacrt
+ assert cert_from_ldap == expected_cacrt
# Verify certmonger was not started
result = host.run_command(['getcert', 'list'], raiseonerr=False)
@@ -383,10 +382,10 @@ def verify_installation(self):
remote_cacrt = host.get_file_contents(paths.IPA_CA_CRT)
logger.debug('%s:/etc/ipa/ca.crt contents:\n%s',
host, remote_cacrt)
- binary_cacrt = base64.b64decode(x509.strip_header(remote_cacrt))
+ cacrt = x509.load_unknown_x509_certificate(remote_cacrt)
logger.debug('%s: Decoded /etc/ipa/ca.crt:\n%r',
- host, binary_cacrt)
- assert expected_binary_cacrt == binary_cacrt
+ host, cacrt)
+ assert expected_cacrt == cacrt
class TestServerInstall(CALessBase):
diff --git a/ipatests/test_xmlrpc/testcert.py b/ipatests/test_xmlrpc/testcert.py
index 151919180b..6ea5a50eef 100644
--- a/ipatests/test_xmlrpc/testcert.py
+++ b/ipatests/test_xmlrpc/testcert.py
@@ -30,6 +30,7 @@
import shutil
import six
import base64
+import re
from ipalib import api, x509
from ipaserver.plugins import rabase
@@ -40,6 +41,20 @@
unicode = str
+def strip_cert_header(pem):
+ """
+ Remove the header and footer from a certificate.
+ """
+ regexp = (
+ r"^-----BEGIN CERTIFICATE-----(.*?)-----END CERTIFICATE-----"
+ )
+ s = re.search(regexp, pem, re.MULTILINE | re.DOTALL)
+ if s is not None:
+ return s.group(1)
+ else:
+ return pem
+
+
def get_testcert(subject, principal):
"""Get the certificate, creating it if it doesn't exist"""
reqdir = tempfile.mkdtemp(prefix="tmp-")
@@ -48,7 +63,7 @@ def get_testcert(subject, principal):
principal)
finally:
shutil.rmtree(reqdir)
- return x509.strip_header(_testcert)
+ return strip_cert_header(_testcert.decode('utf-8'))
def run_certutil(reqdir, args, stdin=None):
@@ -99,4 +114,4 @@ def makecert(reqdir, subject, principal):
res = api.Command['cert_request'](csr, principal=principal, add=True)
cert = x509.load_der_x509_certificate(
base64.b64decode(res['result']['certificate']))
- return cert.public_bytes(x509.Encoding.PEM).decode('utf-8')
+ return cert.public_bytes(x509.Encoding.PEM)
From 0e05a1c6321e8cf3ccbc28cb2855f8fe9ddb355e Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Mon, 25 Sep 2017 09:58:07 +0200
Subject: [PATCH 2/2] x509: remove subject_base() function
The x509.subject_base() function is only used in tests. During
the recent certificate refactoring, we had to get rid of the
ipalib.x509 import from the module scope so that there were no
circular dependecies and add it exactly to this funcion which
is not used in the production code.
Note that we cannot move the x509 module from ipalib to ipapython
yet because it still needs to import ipalib.errors.
---
ipalib/x509.py | 13 -------------
ipatests/test_xmlrpc/test_cert_plugin.py | 4 ++--
ipatests/test_xmlrpc/test_host_plugin.py | 6 +++---
ipatests/test_xmlrpc/test_service_plugin.py | 18 +++++++++---------
ipatests/test_xmlrpc/testcert.py | 14 ++++++++++++++
5 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/ipalib/x509.py b/ipalib/x509.py
index 9e2b365ea2..fa6a6f12a5 100644
--- a/ipalib/x509.py
+++ b/ipalib/x509.py
@@ -51,7 +51,6 @@
import six
from ipalib import errors
-from ipapython.dn import DN
from ipapython.dnsutil import DNSName
if six.PY3:
@@ -76,18 +75,6 @@
SAN_UPN = '1.3.6.1.4.1.311.20.2.3'
SAN_KRB5PRINCIPALNAME = '1.3.6.1.5.2.2'
-_subject_base = None
-
-def subject_base():
- from ipalib import api
- global _subject_base
-
- if _subject_base is None:
- config = api.Command['config_show']()['result']
- _subject_base = DN(config['ipacertificatesubjectbase'][0])
-
- return _subject_base
-
@crypto_utils.register_interface(crypto_x509.Certificate)
class IPACertificate(object):
diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py
index 0de5b75346..4b962aafd8 100644
--- a/ipatests/test_xmlrpc/test_cert_plugin.py
+++ b/ipatests/test_xmlrpc/test_cert_plugin.py
@@ -30,11 +30,11 @@
import tempfile
from ipalib import api
from ipalib import errors
-from ipalib import x509
from ipaplatform.paths import paths
from ipapython import ipautil
from ipapython.dn import DN
from ipapython.ipautil import run
+from ipatests.test_xmlrpc.testcert import subject_base
from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test
from nose.tools import raises, assert_raises
@@ -109,7 +109,7 @@ def setup(self):
# Create our temporary NSS database
self.run_certutil(["-N", "-f", self.pwname])
- self.subject = DN(('CN', self.host_fqdn), x509.subject_base())
+ self.subject = DN(('CN', self.host_fqdn), subject_base())
def teardown(self):
shutil.rmtree(self.reqdir, ignore_errors=True)
diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py
index eab5bf7b51..b67fcaaf04 100644
--- a/ipatests/test_xmlrpc/test_host_plugin.py
+++ b/ipatests/test_xmlrpc/test_host_plugin.py
@@ -41,7 +41,7 @@
from ipatests.test_xmlrpc.test_user_plugin import get_group_dn
from ipatests.test_xmlrpc import objectclasses
from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
-from ipatests.test_xmlrpc.testcert import get_testcert
+from ipatests.test_xmlrpc.testcert import get_testcert, subject_base
from ipatests.util import assert_deepequal
from ipaplatform.paths import paths
@@ -97,7 +97,7 @@
hostgroup1_dn = DN(('cn',hostgroup1),('cn','hostgroups'),('cn','accounts'),
api.env.basedn)
-host_cert = get_testcert(DN(('CN', api.env.host), x509.subject_base()),
+host_cert = get_testcert(DN(('CN', api.env.host), subject_base()),
'host/%s@%s' % (api.env.host, api.env.realm))
@@ -237,7 +237,7 @@ def test_update_simple(self, host):
serial_number_hex=fuzzy_hex,
sha1_fingerprint=fuzzy_hash,
sha256_fingerprint=fuzzy_hash,
- subject=DN(('CN', api.env.host), x509.subject_base()),
+ subject=DN(('CN', api.env.host), subject_base()),
valid_not_before=fuzzy_date,
valid_not_after=fuzzy_date,
))
diff --git a/ipatests/test_xmlrpc/test_service_plugin.py b/ipatests/test_xmlrpc/test_service_plugin.py
index 514ca5b26f..b50b1bb0a2 100644
--- a/ipatests/test_xmlrpc/test_service_plugin.py
+++ b/ipatests/test_xmlrpc/test_service_plugin.py
@@ -21,12 +21,12 @@
Test the `ipaserver/plugins/service.py` module.
"""
-from ipalib import api, errors, x509
+from ipalib import api, errors
from ipatests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid, fuzzy_hash
from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_digits, fuzzy_date, fuzzy_issuer
from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_hex, XMLRPC_test
from ipatests.test_xmlrpc import objectclasses
-from ipatests.test_xmlrpc.testcert import get_testcert
+from ipatests.test_xmlrpc.testcert import get_testcert, subject_base
from ipatests.test_xmlrpc.test_user_plugin import get_user_result, get_group_dn
from ipatests.test_xmlrpc.tracker.service_plugin import ServiceTracker
@@ -50,7 +50,7 @@
role1 = u'Test Role'
role1_dn = DN(('cn', role1), api.env.container_rolegroup, api.env.basedn)
-servercert= get_testcert(DN(('CN', api.env.host), x509.subject_base()),
+servercert= get_testcert(DN(('CN', api.env.host), subject_base()),
'unittest/%s@%s' % (api.env.host, api.env.realm))
randomissuercert = (
"MIICbzCCAdigAwIBAgICA/4wDQYJKoZIhvcNAQEFBQAwKTEnMCUGA1UEAxMeSVBBIFRlc3Q"
@@ -485,7 +485,7 @@ class test_service(Declarative):
managedby_host=[fqdn1],
valid_not_before=fuzzy_date,
valid_not_after=fuzzy_date,
- subject=DN(('CN',api.env.host),x509.subject_base()),
+ subject=DN(('CN',api.env.host),subject_base()),
serial_number=fuzzy_digits,
serial_number_hex=fuzzy_hex,
sha1_fingerprint=fuzzy_hash,
@@ -522,7 +522,7 @@ class test_service(Declarative):
ipakrbauthzdata=[u'MS-PAC'],
valid_not_before=fuzzy_date,
valid_not_after=fuzzy_date,
- subject=DN(('CN',api.env.host),x509.subject_base()),
+ subject=DN(('CN',api.env.host),subject_base()),
serial_number=fuzzy_digits,
serial_number_hex=fuzzy_hex,
sha1_fingerprint=fuzzy_hash,
@@ -551,7 +551,7 @@ class test_service(Declarative):
# test case.
valid_not_before=fuzzy_date,
valid_not_after=fuzzy_date,
- subject=DN(('CN',api.env.host),x509.subject_base()),
+ subject=DN(('CN',api.env.host),subject_base()),
serial_number=fuzzy_digits,
serial_number_hex=fuzzy_hex,
sha1_fingerprint=fuzzy_hash,
@@ -576,7 +576,7 @@ class test_service(Declarative):
ipakrbauthzdata=[u'MS-PAC'],
valid_not_before=fuzzy_date,
valid_not_after=fuzzy_date,
- subject=DN(('CN',api.env.host),x509.subject_base()),
+ subject=DN(('CN',api.env.host),subject_base()),
serial_number=fuzzy_digits,
serial_number_hex=fuzzy_hex,
sha1_fingerprint=fuzzy_hash,
@@ -604,7 +604,7 @@ class test_service(Declarative):
ipakrbauthzdata=[u'MS-PAC'],
valid_not_before=fuzzy_date,
valid_not_after=fuzzy_date,
- subject=DN(('CN',api.env.host),x509.subject_base()),
+ subject=DN(('CN',api.env.host),subject_base()),
serial_number=fuzzy_digits,
serial_number_hex=fuzzy_hex,
sha1_fingerprint=fuzzy_hash,
@@ -630,7 +630,7 @@ class test_service(Declarative):
ipakrbauthzdata=[u'MS-PAC'],
valid_not_before=fuzzy_date,
valid_not_after=fuzzy_date,
- subject=DN(('CN',api.env.host),x509.subject_base()),
+ subject=DN(('CN',api.env.host),subject_base()),
serial_number=fuzzy_digits,
serial_number_hex=fuzzy_hex,
sha1_fingerprint=fuzzy_hash,
diff --git a/ipatests/test_xmlrpc/testcert.py b/ipatests/test_xmlrpc/testcert.py
index 6ea5a50eef..3874d75f29 100644
--- a/ipatests/test_xmlrpc/testcert.py
+++ b/ipatests/test_xmlrpc/testcert.py
@@ -35,12 +35,26 @@
from ipalib import api, x509
from ipaserver.plugins import rabase
from ipapython import ipautil
+from ipapython.dn import DN
from ipaplatform.paths import paths
if six.PY3:
unicode = str
+_subject_base = None
+
+
+def subject_base():
+ global _subject_base
+
+ if _subject_base is None:
+ config = api.Command['config_show']()['result']
+ _subject_base = DN(config['ipacertificatesubjectbase'][0])
+
+ return _subject_base
+
+
def strip_cert_header(pem):
"""
Remove the header and footer from a certificate.
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
