From d0d29ccc324bb9f95bffbe3162ee5c3c61c6086a Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Thu, 11 Jul 2019 15:17:04 +1000 Subject: [PATCH] move MSCSTemplate classes to ipalib As we expand the integration tests for external CA functionality, it is helpful (and avoids duplication) to use the MSCSTemplate* classes. These currently live in ipaserver.install.cainstance, but ipatests is no longer permitted to import from ipaserver (see commit 81714976e5e13131654c78eb734746a20237c933). So move these classes to ipalib. Part of: https://pagure.io/freeipa/issue/7548 Reviewed-By: Florence Blanc-Renaud --- install/tools/ipa-ca-install.in | 6 +- ipalib/x509.py | 171 +++++++++++++++++ ipaserver/install/ca.py | 11 +- ipaserver/install/cainstance.py | 180 +----------------- ipaserver/install/ipa_cacert_manage.py | 14 +- ipatests/test_integration/test_external_ca.py | 11 +- ipatests/test_ipalib/test_x509.py | 115 +++++++++++ .../test_install/test_cainstance.py | 123 ------------ 8 files changed, 307 insertions(+), 324 deletions(-) delete mode 100644 ipatests/test_ipaserver/test_install/test_cainstance.py diff --git a/install/tools/ipa-ca-install.in b/install/tools/ipa-ca-install.in index 0700c0c38b..ce6d5fcb52 100644 --- a/install/tools/ipa-ca-install.in +++ b/install/tools/ipa-ca-install.in @@ -37,7 +37,7 @@ from ipaserver.install import cainstance, service from ipaserver.install import custodiainstance from ipaserver.masters import find_providing_server from ipapython import version -from ipalib import api +from ipalib import api, x509 from ipalib.constants import DOMAIN_LEVEL_1 from ipapython.config import IPAOptionParser from ipapython.ipa_log_manager import standard_logging_setup @@ -68,13 +68,13 @@ def parse_options(): default=False, help="unattended installation never prompts the user") parser.add_option("--external-ca", dest="external_ca", action="store_true", default=False, help="Generate a CSR to be signed by an external CA") - ext_cas = tuple(x.value for x in cainstance.ExternalCAType) + ext_cas = tuple(x.value for x in x509.ExternalCAType) parser.add_option("--external-ca-type", dest="external_ca_type", type="choice", choices=ext_cas, metavar="{{{0}}}".format(",".join(ext_cas)), help="Type of the external CA. Default: generic") parser.add_option("--external-ca-profile", dest="external_ca_profile", - type='constructor', constructor=cainstance.ExternalCAProfile, + type='constructor', constructor=x509.ExternalCAProfile, default=None, metavar="PROFILE-SPEC", help="Specify the certificate profile/template to use " "at the external CA") diff --git a/ipalib/x509.py b/ipalib/x509.py index ab3c5f553d..1f612a3797 100644 --- a/ipalib/x509.py +++ b/ipalib/x509.py @@ -34,6 +34,7 @@ import os import binascii import datetime +import enum import ipaddress import ssl import base64 @@ -47,6 +48,7 @@ Encoding, PublicFormat, PrivateFormat, load_pem_private_key ) import pyasn1 +import pyasn1.error from pyasn1.type import univ, char, namedtype, tag from pyasn1.codec.der import decoder, encoder from pyasn1_modules import rfc2315, rfc2459 @@ -745,3 +747,172 @@ 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")) + + +class ExternalCAType(enum.Enum): + GENERIC = 'generic' + MS_CS = 'ms-cs' + + +class ExternalCAProfile: + """ + An external CA profile configuration. Currently the only + subclasses are for Microsoft CAs, for providing data in the + "Certificate Template" extension. + + Constructing this class will actually return an instance of a + subclass. + + Subclasses MUST set ``valid_for``. + + """ + def __init__(self, s=None): + self.unparsed_input = s + + # Which external CA types is the data valid for? + # A set of VALUES of the ExternalCAType enum. + valid_for = set() + + def __new__(cls, s=None): + """Construct the ExternalCAProfile value. + + Return an instance of a subclass determined by + the format of the argument. + + """ + # we are directly constructing a subclass; instantiate + # it and be done + if cls is not ExternalCAProfile: + return super(ExternalCAProfile, cls).__new__(cls) + + # construction via the base class; therefore the string + # argument is required, and is used to determine which + # subclass to construct + if s is None: + raise ValueError('string argument is required') + + parts = s.split(':') + + try: + # Is the first part on OID? + _oid = univ.ObjectIdentifier(parts[0]) + + # It is; construct a V2 template + # pylint: disable=too-many-function-args + return MSCSTemplateV2.__new__(MSCSTemplateV2, s) + + except pyasn1.error.PyAsn1Error: + # It is not an OID; treat as a template name + # pylint: disable=too-many-function-args + return MSCSTemplateV1.__new__(MSCSTemplateV1, s) + + def __getstate__(self): + return self.unparsed_input + + def __setstate__(self, state): + # explicitly call __init__ method to initialise object + self.__init__(state) + + +class MSCSTemplate(ExternalCAProfile): + """ + An Microsoft AD-CS Template specifier. + + Subclasses MUST set ext_oid. + + Subclass constructors MUST set asn1obj. + + """ + valid_for = set([ExternalCAType.MS_CS.value]) + + ext_oid = None # extension OID, as a Python str + asn1obj = None # unencoded extension data + + def get_ext_data(self): + """Return DER-encoded extension data.""" + return encoder.encode(self.asn1obj) + + +class MSCSTemplateV1(MSCSTemplate): + """ + A v1 template specifier, per + https://msdn.microsoft.com/en-us/library/cc250011.aspx. + + :: + + CertificateTemplateName ::= SEQUENCE { + Name UTF8String + } + + But note that a bare BMPString is used in practice. + + """ + ext_oid = "1.3.6.1.4.1.311.20.2" + + def __init__(self, s): + super(MSCSTemplateV1, self).__init__(s) + parts = s.split(':') + if len(parts) > 1: + raise ValueError( + "Cannot specify certificate template version when using name.") + self.asn1obj = char.BMPString(str(parts[0])) + + +class MSCSTemplateV2(MSCSTemplate): + """ + A v2 template specifier, per + https://msdn.microsoft.com/en-us/library/windows/desktop/aa378274(v=vs.85).aspx + + :: + + CertificateTemplate ::= SEQUENCE { + templateID EncodedObjectID, + templateMajorVersion TemplateVersion, + templateMinorVersion TemplateVersion OPTIONAL + } + + TemplateVersion ::= INTEGER (0..4294967295) + + """ + ext_oid = "1.3.6.1.4.1.311.21.7" + + @staticmethod + def check_version_in_range(desc, n): + if n < 0 or n >= 2**32: + raise ValueError( + "Template {} version must be in range 0..4294967295" + .format(desc)) + + def __init__(self, s): + super(MSCSTemplateV2, self).__init__(s) + + parts = s.split(':') + + obj = CertificateTemplateV2() + if len(parts) < 2 or len(parts) > 3: + raise ValueError( + "Incorrect template specification; required format is: " + ":[:]") + try: + obj['templateID'] = univ.ObjectIdentifier(parts[0]) + + major = int(parts[1]) + self.check_version_in_range("major", major) + obj['templateMajorVersion'] = major + + if len(parts) > 2: + minor = int(parts[2]) + self.check_version_in_range("minor", minor) + obj['templateMinorVersion'] = int(parts[2]) + + except pyasn1.error.PyAsn1Error: + raise ValueError("Could not parse certificate template specifier.") + self.asn1obj = obj + + +class CertificateTemplateV2(univ.Sequence): + componentType = namedtype.NamedTypes( + namedtype.NamedType('templateID', univ.ObjectIdentifier()), + namedtype.NamedType('templateMajorVersion', univ.Integer()), + namedtype.OptionalNamedType('templateMinorVersion', univ.Integer()) + ) diff --git a/ipaserver/install/ca.py b/ipaserver/install/ca.py index 6b040b311a..8fb5e3ec91 100644 --- a/ipaserver/install/ca.py +++ b/ipaserver/install/ca.py @@ -28,7 +28,7 @@ from ipaplatform.paths import paths from ipaserver.install import installutils, certs from ipaserver.install.replication import replica_conn_check -from ipalib import api, errors +from ipalib import api, errors, x509 from ipapython.dn import DN from . import conncheck, dogtag, cainstance @@ -216,8 +216,7 @@ def install_check(standalone, replica_config, options): paths.ROOT_IPA_CSR) if not options.external_ca_type: - options.external_ca_type = \ - cainstance.ExternalCAType.GENERIC.value + options.external_ca_type = x509.ExternalCAType.GENERIC.value if options.external_ca_profile is not None: # check that profile is valid for the external ca type @@ -478,13 +477,11 @@ class CAInstallInterface(dogtag.DogtagInstallInterface, external_ca = master_install_only(external_ca) external_ca_type = knob( - cainstance.ExternalCAType, None, - description="Type of the external CA", - ) + x509.ExternalCAType, None, description="Type of the external CA") external_ca_type = master_install_only(external_ca_type) external_ca_profile = knob( - type=cainstance.ExternalCAProfile, + type=x509.ExternalCAProfile, default=None, description=( "Specify the certificate profile/template to use at the " diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py index 6e1fc724db..2295581870 100644 --- a/ipaserver/install/cainstance.py +++ b/ipaserver/install/cainstance.py @@ -26,7 +26,6 @@ import logging import dbus -import enum import ldap import os import pwd @@ -39,10 +38,6 @@ import tempfile from configparser import RawConfigParser -from pyasn1.codec.der import encoder -from pyasn1.type import char, univ, namedtype -import pyasn1.error - from ipalib import api from ipalib import x509 from ipalib import errors @@ -80,11 +75,6 @@ ] -class ExternalCAType(enum.Enum): - GENERIC = 'generic' - MS_CS = 'ms-cs' - - def check_ports(): """Check that dogtag ports (8080, 8443) are available. @@ -367,7 +357,7 @@ def configure_instance(self, host_name, dm_password, admin_password, if ca_type is not None: self.ca_type = ca_type else: - self.ca_type = ExternalCAType.GENERIC.value + self.ca_type = x509.ExternalCAType.GENERIC.value self.external_ca_profile = external_ca_profile self.no_db_setup = promote @@ -537,12 +527,12 @@ def __spawn_instance(self): pki_ca_signing_csr_path=self.csr_file, ) - if self.ca_type == ExternalCAType.MS_CS.value: + if self.ca_type == x509.ExternalCAType.MS_CS.value: # Include MS template name extension in the CSR template = self.external_ca_profile if template is None: # default template name - template = MSCSTemplateV1(u"SubCA") + template = x509.MSCSTemplateV1(u"SubCA") ext_data = binascii.hexlify(template.get_ext_data()) cfg.update( @@ -2081,170 +2071,6 @@ def update_ipa_conf(): parser.write(f) -class ExternalCAProfile: - """ - An external CA profile configuration. Currently the only - subclasses are for Microsoft CAs, for providing data in the - "Certificate Template" extension. - - Constructing this class will actually return an instance of a - subclass. - - Subclasses MUST set ``valid_for``. - - """ - def __init__(self, s=None): - self.unparsed_input = s - - # Which external CA types is the data valid for? - # A set of VALUES of the ExternalCAType enum. - valid_for = set() - - def __new__(cls, s=None): - """Construct the ExternalCAProfile value. - - Return an instance of a subclass determined by - the format of the argument. - - """ - # we are directly constructing a subclass; instantiate - # it and be done - if cls is not ExternalCAProfile: - return super(ExternalCAProfile, cls).__new__(cls) - - # construction via the base class; therefore the string - # argument is required, and is used to determine which - # subclass to construct - if s is None: - raise ValueError('string argument is required') - - parts = s.split(':') - - try: - # Is the first part on OID? - _oid = univ.ObjectIdentifier(parts[0]) - - # It is; construct a V2 template - # pylint: disable=too-many-function-args - return MSCSTemplateV2.__new__(MSCSTemplateV2, s) - - except pyasn1.error.PyAsn1Error: - # It is not an OID; treat as a template name - # pylint: disable=too-many-function-args - return MSCSTemplateV1.__new__(MSCSTemplateV1, s) - - def __getstate__(self): - return self.unparsed_input - - def __setstate__(self, state): - # explicitly call __init__ method to initialise object - self.__init__(state) - - -class MSCSTemplate(ExternalCAProfile): - """ - An Microsoft AD-CS Template specifier. - - Subclasses MUST set ext_oid. - - Subclass constructors MUST set asn1obj. - - """ - valid_for = set([ExternalCAType.MS_CS.value]) - - ext_oid = None # extension OID, as a Python str - asn1obj = None # unencoded extension data - - def get_ext_data(self): - """Return DER-encoded extension data.""" - return encoder.encode(self.asn1obj) - - -class MSCSTemplateV1(MSCSTemplate): - """ - A v1 template specifier, per - https://msdn.microsoft.com/en-us/library/cc250011.aspx. - - :: - - CertificateTemplateName ::= SEQUENCE { - Name UTF8String - } - - But note that a bare BMPString is used in practice. - - """ - ext_oid = "1.3.6.1.4.1.311.20.2" - - def __init__(self, s): - super(MSCSTemplateV1, self).__init__(s) - parts = s.split(':') - if len(parts) > 1: - raise ValueError( - "Cannot specify certificate template version when using name.") - self.asn1obj = char.BMPString(str(parts[0])) - - -class MSCSTemplateV2(MSCSTemplate): - """ - A v2 template specifier, per - https://msdn.microsoft.com/en-us/library/windows/desktop/aa378274(v=vs.85).aspx - - :: - - CertificateTemplate ::= SEQUENCE { - templateID EncodedObjectID, - templateMajorVersion TemplateVersion, - templateMinorVersion TemplateVersion OPTIONAL - } - - TemplateVersion ::= INTEGER (0..4294967295) - - """ - ext_oid = "1.3.6.1.4.1.311.21.7" - - @staticmethod - def check_version_in_range(desc, n): - if n < 0 or n >= 2**32: - raise ValueError( - "Template {} version must be in range 0..4294967295" - .format(desc)) - - def __init__(self, s): - super(MSCSTemplateV2, self).__init__(s) - - parts = s.split(':') - - obj = CertificateTemplateV2() - if len(parts) < 2 or len(parts) > 3: - raise ValueError( - "Incorrect template specification; required format is: " - ":[:]") - try: - obj['templateID'] = univ.ObjectIdentifier(parts[0]) - - major = int(parts[1]) - self.check_version_in_range("major", major) - obj['templateMajorVersion'] = major - - if len(parts) > 2: - minor = int(parts[2]) - self.check_version_in_range("minor", minor) - obj['templateMinorVersion'] = int(parts[2]) - - except pyasn1.error.PyAsn1Error: - raise ValueError("Could not parse certificate template specifier.") - self.asn1obj = obj - - -class CertificateTemplateV2(univ.Sequence): - componentType = namedtype.NamedTypes( - namedtype.NamedType('templateID', univ.ObjectIdentifier()), - namedtype.NamedType('templateMajorVersion', univ.Integer()), - namedtype.OptionalNamedType('templateMinorVersion', univ.Integer()) - ) - - if __name__ == "__main__": standard_logging_setup("install.log") ds = dsinstance.DsInstance() diff --git a/ipaserver/install/ipa_cacert_manage.py b/ipaserver/install/ipa_cacert_manage.py index 3f113c35bf..37dcc2befa 100644 --- a/ipaserver/install/ipa_cacert_manage.py +++ b/ipaserver/install/ipa_cacert_manage.py @@ -65,7 +65,7 @@ def add_options(cls, parser): "--external-ca", dest='self_signed', action='store_false', help="Sign the renewed certificate by external CA") - ext_cas = tuple(x.value for x in cainstance.ExternalCAType) + ext_cas = tuple(x.value for x in x509.ExternalCAType) renew_group.add_option( "--external-ca-type", dest="external_ca_type", type="choice", choices=ext_cas, @@ -73,7 +73,7 @@ def add_options(cls, parser): help="Type of the external CA. Default: generic") renew_group.add_option( "--external-ca-profile", dest="external_ca_profile", - type='constructor', constructor=cainstance.ExternalCAProfile, + type='constructor', constructor=x509.ExternalCAProfile, default=None, metavar="PROFILE-SPEC", help="Specify the certificate profile/template to use " "at the external CA") @@ -224,11 +224,11 @@ def renew_external_step_1(self, ca): options = self.options if not options.external_ca_type: - options.external_ca_type = cainstance.ExternalCAType.GENERIC.value + options.external_ca_type = x509.ExternalCAType.GENERIC.value - if options.external_ca_type == cainstance.ExternalCAType.MS_CS.value \ + if options.external_ca_type == x509.ExternalCAType.MS_CS.value \ and options.external_ca_profile is None: - options.external_ca_profile = cainstance.MSCSTemplateV1(u"SubCA") + options.external_ca_profile = x509.MSCSTemplateV1(u"SubCA") if options.external_ca_profile is not None: # check that profile is valid for the external ca type @@ -352,11 +352,11 @@ def resubmit_request(self, ca=RENEWAL_CA_NAME, profile=None): timeout = api.env.startup_timeout + 60 cm_profile = None - if isinstance(profile, cainstance.MSCSTemplateV1): + if isinstance(profile, x509.MSCSTemplateV1): cm_profile = profile.unparsed_input cm_template = None - if isinstance(profile, cainstance.MSCSTemplateV2): + if isinstance(profile, x509.MSCSTemplateV2): cm_template = profile.unparsed_input logger.debug("resubmitting certmonger request '%s'", self.request_id) diff --git a/ipatests/test_integration/test_external_ca.py b/ipatests/test_integration/test_external_ca.py index a42355217d..5aa2b7bba0 100644 --- a/ipatests/test_integration/test_external_ca.py +++ b/ipatests/test_integration/test_external_ca.py @@ -108,14 +108,14 @@ def check_ipaca_issuerDN(host, expected_dn): assert "Issuer DN: {}".format(expected_dn) in result.stdout_text -def check_mscs_extension(ipa_csr, oid, value): +def check_mscs_extension(ipa_csr, template): csr = x509.load_pem_x509_csr(ipa_csr, default_backend()) extensions = [ ext for ext in csr.extensions - if ext.oid.dotted_string == oid + if ext.oid.dotted_string == template.ext_oid ] assert extensions - assert extensions[0].value.value == value + assert extensions[0].value.value == template.get_ext_data() class TestExternalCA(IntegrationTest): @@ -134,10 +134,7 @@ def test_external_ca(self): # check CSR for extension ipa_csr = self.master.get_file_contents(paths.ROOT_IPA_CSR) - # Values for MSCSTemplateV1('SubCA') - oid = "1.3.6.1.4.1.311.20.2" - value = b'\x1e\n\x00S\x00u\x00b\x00C\x00A' - check_mscs_extension(ipa_csr, oid, value) + check_mscs_extension(ipa_csr, ipa_x509.MSCSTemplateV1(u'SubCA')) # Sign CA, transport it to the host and get ipa a root ca paths. root_ca_fname, ipa_ca_fname = tasks.sign_ca_and_transport( diff --git a/ipatests/test_ipalib/test_x509.py b/ipatests/test_ipalib/test_x509.py index ff7e6de2f7..284b998316 100644 --- a/ipatests/test_ipalib/test_x509.py +++ b/ipatests/test_ipalib/test_x509.py @@ -22,7 +22,11 @@ """ import base64 +from binascii import hexlify +from configparser import RawConfigParser import datetime +from io import StringIO +import pickle import pytest @@ -268,3 +272,114 @@ def test_ipa_demo_letsencrypt(self): b'0 \x06\x03U\x1d%\x01\x01\xff\x04\x160\x14\x06\x08+\x06\x01' b'\x05\x05\x07\x03\x01\x06\x08+\x06\x01\x05\x05\x07\x03\x02' ) + + +class test_ExternalCAProfile: + def test_MSCSTemplateV1_good(self): + o = x509.MSCSTemplateV1("MySubCA") + assert hexlify(o.get_ext_data()) == b'1e0e004d007900530075006200430041' + + def test_MSCSTemplateV1_bad(self): + with pytest.raises(ValueError): + x509.MSCSTemplateV1("MySubCA:1") + + def test_MSCSTemplateV1_pickle_roundtrip(self): + o = x509.MSCSTemplateV1("MySubCA") + s = pickle.dumps(o) + assert o.get_ext_data() == pickle.loads(s).get_ext_data() + + def test_MSCSTemplateV2_too_few_parts(self): + with pytest.raises(ValueError): + x509.MSCSTemplateV2("1.2.3.4") + + def test_MSCSTemplateV2_too_many_parts(self): + with pytest.raises(ValueError): + x509.MSCSTemplateV2("1.2.3.4:100:200:300") + + def test_MSCSTemplateV2_bad_oid(self): + with pytest.raises(ValueError): + x509.MSCSTemplateV2("not_an_oid:1") + + def test_MSCSTemplateV2_non_numeric_major_version(self): + with pytest.raises(ValueError): + x509.MSCSTemplateV2("1.2.3.4:major:200") + + def test_MSCSTemplateV2_non_numeric_minor_version(self): + with pytest.raises(ValueError): + x509.MSCSTemplateV2("1.2.3.4:100:minor") + + def test_MSCSTemplateV2_major_version_lt_zero(self): + with pytest.raises(ValueError): + x509.MSCSTemplateV2("1.2.3.4:-1:200") + + def test_MSCSTemplateV2_minor_version_lt_zero(self): + with pytest.raises(ValueError): + x509.MSCSTemplateV2("1.2.3.4:100:-1") + + def test_MSCSTemplateV2_major_version_gt_max(self): + with pytest.raises(ValueError): + x509.MSCSTemplateV2("1.2.3.4:4294967296:200") + + def test_MSCSTemplateV2_minor_version_gt_max(self): + with pytest.raises(ValueError): + x509.MSCSTemplateV2("1.2.3.4:100:4294967296") + + def test_MSCSTemplateV2_good_major(self): + o = x509.MSCSTemplateV2("1.2.3.4:4294967295") + assert hexlify(o.get_ext_data()) == b'300c06032a0304020500ffffffff' + + def test_MSCSTemplateV2_good_major_minor(self): + o = x509.MSCSTemplateV2("1.2.3.4:4294967295:0") + assert hexlify(o.get_ext_data()) \ + == b'300f06032a0304020500ffffffff020100' + + def test_MSCSTemplateV2_pickle_roundtrip(self): + o = x509.MSCSTemplateV2("1.2.3.4:4294967295:0") + s = pickle.dumps(o) + assert o.get_ext_data() == pickle.loads(s).get_ext_data() + + def test_ExternalCAProfile_dispatch(self): + """ + Test that constructing ExternalCAProfile actually returns an + instance of the appropriate subclass. + """ + assert isinstance( + x509.ExternalCAProfile("MySubCA"), + x509.MSCSTemplateV1) + assert isinstance( + x509.ExternalCAProfile("1.2.3.4:100"), + x509.MSCSTemplateV2) + + def test_write_pkispawn_config_file_MSCSTemplateV1(self): + template = x509.MSCSTemplateV1(u"SubCA") + expected = ( + '[CA]\n' + 'pki_req_ext_oid = 1.3.6.1.4.1.311.20.2\n' + 'pki_req_ext_data = 1e0a00530075006200430041\n\n' + ) + self._test_write_pkispawn_config_file(template, expected) + + def test_write_pkispawn_config_file_MSCSTemplateV2(self): + template = x509.MSCSTemplateV2(u"1.2.3.4:4294967295") + expected = ( + '[CA]\n' + 'pki_req_ext_oid = 1.3.6.1.4.1.311.21.7\n' + 'pki_req_ext_data = 300c06032a0304020500ffffffff\n\n' + ) + self._test_write_pkispawn_config_file(template, expected) + + def _test_write_pkispawn_config_file(self, template, expected): + """ + Test that the values we read from an ExternalCAProfile + object can be used to produce a reasonable-looking pkispawn + configuration. + """ + config = RawConfigParser() + config.optionxform = str + config.add_section("CA") + config.set("CA", "pki_req_ext_oid", template.ext_oid) + config.set("CA", "pki_req_ext_data", + hexlify(template.get_ext_data()).decode('ascii')) + out = StringIO() + config.write(out) + assert out.getvalue() == expected diff --git a/ipatests/test_ipaserver/test_install/test_cainstance.py b/ipatests/test_ipaserver/test_install/test_cainstance.py deleted file mode 100644 index 02d9758e4a..0000000000 --- a/ipatests/test_ipaserver/test_install/test_cainstance.py +++ /dev/null @@ -1,123 +0,0 @@ -# -# Copyright (C) 2017 FreeIPA Contributors see COPYING for license -# - -from binascii import hexlify -from io import StringIO -import pickle -from configparser import RawConfigParser -import pytest -from ipaserver.install import cainstance - -pytestmark = pytest.mark.tier0 - - -class test_ExternalCAProfile: - def test_MSCSTemplateV1_good(self): - o = cainstance.MSCSTemplateV1("MySubCA") - assert hexlify(o.get_ext_data()) == b'1e0e004d007900530075006200430041' - - def test_MSCSTemplateV1_bad(self): - with pytest.raises(ValueError): - cainstance.MSCSTemplateV1("MySubCA:1") - - def test_MSCSTemplateV1_pickle_roundtrip(self): - o = cainstance.MSCSTemplateV1("MySubCA") - s = pickle.dumps(o) - assert o.get_ext_data() == pickle.loads(s).get_ext_data() - - def test_MSCSTemplateV2_too_few_parts(self): - with pytest.raises(ValueError): - cainstance.MSCSTemplateV2("1.2.3.4") - - def test_MSCSTemplateV2_too_many_parts(self): - with pytest.raises(ValueError): - cainstance.MSCSTemplateV2("1.2.3.4:100:200:300") - - def test_MSCSTemplateV2_bad_oid(self): - with pytest.raises(ValueError): - cainstance.MSCSTemplateV2("not_an_oid:1") - - def test_MSCSTemplateV2_non_numeric_major_version(self): - with pytest.raises(ValueError): - cainstance.MSCSTemplateV2("1.2.3.4:major:200") - - def test_MSCSTemplateV2_non_numeric_minor_version(self): - with pytest.raises(ValueError): - cainstance.MSCSTemplateV2("1.2.3.4:100:minor") - - def test_MSCSTemplateV2_major_version_lt_zero(self): - with pytest.raises(ValueError): - cainstance.MSCSTemplateV2("1.2.3.4:-1:200") - - def test_MSCSTemplateV2_minor_version_lt_zero(self): - with pytest.raises(ValueError): - cainstance.MSCSTemplateV2("1.2.3.4:100:-1") - - def test_MSCSTemplateV2_major_version_gt_max(self): - with pytest.raises(ValueError): - cainstance.MSCSTemplateV2("1.2.3.4:4294967296:200") - - def test_MSCSTemplateV2_minor_version_gt_max(self): - with pytest.raises(ValueError): - cainstance.MSCSTemplateV2("1.2.3.4:100:4294967296") - - def test_MSCSTemplateV2_good_major(self): - o = cainstance.MSCSTemplateV2("1.2.3.4:4294967295") - assert hexlify(o.get_ext_data()) == b'300c06032a0304020500ffffffff' - - def test_MSCSTemplateV2_good_major_minor(self): - o = cainstance.MSCSTemplateV2("1.2.3.4:4294967295:0") - assert hexlify(o.get_ext_data()) \ - == b'300f06032a0304020500ffffffff020100' - - def test_MSCSTemplateV2_pickle_roundtrip(self): - o = cainstance.MSCSTemplateV2("1.2.3.4:4294967295:0") - s = pickle.dumps(o) - assert o.get_ext_data() == pickle.loads(s).get_ext_data() - - def test_ExternalCAProfile_dispatch(self): - """ - Test that constructing ExternalCAProfile actually returns an - instance of the appropriate subclass. - """ - assert isinstance( - cainstance.ExternalCAProfile("MySubCA"), - cainstance.MSCSTemplateV1) - assert isinstance( - cainstance.ExternalCAProfile("1.2.3.4:100"), - cainstance.MSCSTemplateV2) - - def test_write_pkispawn_config_file_MSCSTemplateV1(self): - template = cainstance.MSCSTemplateV1(u"SubCA") - expected = ( - '[CA]\n' - 'pki_req_ext_oid = 1.3.6.1.4.1.311.20.2\n' - 'pki_req_ext_data = 1e0a00530075006200430041\n\n' - ) - self._test_write_pkispawn_config_file(template, expected) - - def test_write_pkispawn_config_file_MSCSTemplateV2(self): - template = cainstance.MSCSTemplateV2(u"1.2.3.4:4294967295") - expected = ( - '[CA]\n' - 'pki_req_ext_oid = 1.3.6.1.4.1.311.21.7\n' - 'pki_req_ext_data = 300c06032a0304020500ffffffff\n\n' - ) - self._test_write_pkispawn_config_file(template, expected) - - def _test_write_pkispawn_config_file(self, template, expected): - """ - Test that the values we read from an ExternalCAProfile - object can be used to produce a reasonable-looking pkispawn - configuration. - """ - config = RawConfigParser() - config.optionxform = str - config.add_section("CA") - config.set("CA", "pki_req_ext_oid", template.ext_oid) - config.set("CA", "pki_req_ext_data", - hexlify(template.get_ext_data()).decode('ascii')) - out = StringIO() - config.write(out) - assert out.getvalue() == expected From e632b220798833bcd65c6b266610c800ed0914d7 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Fri, 12 Jul 2019 13:13:02 +1000 Subject: [PATCH] install: fix --external-ca-profile option Commit dd47cfc75a69618f486abefb70f2649ebf8264e7 removed the ability to set pki_req_ext_oid and pki_req_ext_data in the pkispawn config. This results in the --external-ca-profile option never setting the requested values in the CSR (the default V1 template type specifying "SubCA" is always used). Remove relevant fields from both ipaca_default.ini and ipaca_customize.ini. This allows the IPA framework to set the values (i.e. when --external-ca-type=ms-cs and --external-ca-profile=... demand it). It also allows users to override the pki_req_ext_* settings. Part of: https://pagure.io/freeipa/issue/7548 Related: https://pagure.io/freeipa/issue/5608 Reviewed-By: Florence Blanc-Renaud --- install/share/ipaca_customize.ini | 5 ----- install/share/ipaca_default.ini | 1 - 2 files changed, 6 deletions(-) diff --git a/install/share/ipaca_customize.ini b/install/share/ipaca_customize.ini index 130ec2c102..6d58579af8 100644 --- a/install/share/ipaca_customize.ini +++ b/install/share/ipaca_customize.ini @@ -93,11 +93,6 @@ pki_ca_signing_key_type=%(ipa_ca_key_type)s pki_ca_signing_signing_algorithm=%(ipa_ca_signing_algorithm)s pki_ca_signing_token=%(pki_token_name)s -# MS subca request ext data -pki_req_ext_oid=1.3.6.1.4.1.311.20.2 -pki_req_ext_critical=False -pki_req_ext_data=1E0A00530075006200430041 - ## ocspSigningCert cert-pki-ca pki_ocsp_signing_key_algorithm=%(ipa_key_algorithm)s pki_ocsp_signing_key_size=%(ipa_key_size)s diff --git a/install/share/ipaca_default.ini b/install/share/ipaca_default.ini index fedc1b9a74..2b9900286e 100644 --- a/install/share/ipaca_default.ini +++ b/install/share/ipaca_default.ini @@ -115,7 +115,6 @@ pki_ca_starting_crl_number=0 pki_external=False pki_external_step_two=False -pki_req_ext_add=False pki_external_pkcs12_path=%(pki_pkcs12_path)s pki_external_pkcs12_password=%(pki_pkcs12_password)s From 71af731b3069fa1b2c0b51a3b917b5bc4da54350 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Fri, 12 Jul 2019 13:24:51 +1000 Subject: [PATCH] Fix use of incorrect variable Part of: https://pagure.io/freeipa/issue/7548 Related: https://pagure.io/freeipa/issue/5608 Reviewed-By: Florence Blanc-Renaud --- ipaserver/install/dogtaginstance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipaserver/install/dogtaginstance.py b/ipaserver/install/dogtaginstance.py index cc75d89746..5dca721d6c 100644 --- a/ipaserver/install/dogtaginstance.py +++ b/ipaserver/install/dogtaginstance.py @@ -853,7 +853,7 @@ def _verify_immutable(self, config, immutable_settings, filename): if errs: raise ValueError( '{} overrides immutable options:\n{}'.format( - filename, '\n'.join(errors) + filename, '\n'.join(errs) ) ) From 83ed05725110de19a7098678274ecaaaf6a2c9c9 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Wed, 20 Feb 2019 18:34:33 +1100 Subject: [PATCH] Add more tests for --external-ca-profile handling Add tests for remaining untested scenarios of --external-ca-profile handling in ipa-server-install. ipa-ca-install and ipa-cacert-manage remain untested at present. Fixes: https://pagure.io/freeipa/issue/7548 Reviewed-By: Florence Blanc-Renaud --- ipatests/test_integration/test_external_ca.py | 97 ++++++++++++++++++- 1 file changed, 95 insertions(+), 2 deletions(-) diff --git a/ipatests/test_integration/test_external_ca.py b/ipatests/test_integration/test_external_ca.py index 5aa2b7bba0..dc9a09b43b 100644 --- a/ipatests/test_integration/test_external_ca.py +++ b/ipatests/test_integration/test_external_ca.py @@ -74,10 +74,10 @@ def match_in_journal(host, string, since='today', services=('certmonger',)): return match -def install_server_external_ca_step1(host, extra_args=()): +def install_server_external_ca_step1(host, extra_args=(), raiseonerr=True): """Step 1 to install the ipa server with external ca""" return tasks.install_master( - host, external_ca=True, extra_args=extra_args + host, external_ca=True, extra_args=extra_args, raiseonerr=raiseonerr, ) @@ -478,3 +478,96 @@ def test_master_install_ca2(self): 'certutil', '-L', '-d', paths.PKI_TOMCAT_ALIAS_DIR, '-n', cert_nick]) assert "CN=RootCA2" in result.stdout_text + + +def _step1_profile(master, s): + return install_server_external_ca_step1( + master, + extra_args=['--external-ca-type=ms-cs', f'--external-ca-profile={s}'], + raiseonerr=False, + ) + + +def _test_invalid_profile(master, profile): + result = _step1_profile(master, profile) + assert result.returncode != 0 + assert '--external-ca-profile' in result.stderr_text + + +def _test_valid_profile(master, profile_cls, profile): + result = _step1_profile(master, profile) + assert result.returncode == 0 + ipa_csr = master.get_file_contents(paths.ROOT_IPA_CSR) + check_mscs_extension(ipa_csr, profile_cls(profile)) + + +class TestExternalCAProfileV1(IntegrationTest): + """ + Test that --external-ca-profile=Foo gets propagated to the CSR. + + The default template extension when --external-ca-type=ms-cs, + a V1 extension with value "SubCA", already gets tested by the + ``TestExternalCA`` class. + + We only need to do Step 1 of installation, then check the CSR. + + """ + def test_invalid_v1_template(self): + _test_invalid_profile(self.master, 'NotAnOid:1') + + def test_valid_v1_template(self): + _test_valid_profile( + self.master, ipa_x509.MSCSTemplateV1, 'TemplateOfAwesome') + + +class TestExternalCAProfileV2MajorOnly(IntegrationTest): + """ + Test that V2 template specifiers without minor version get + propagated to CSR. This class also tests all error modes in + specifying a V2 template, those being: + + - no major version specified + - too many parts specified (i.e. major, minor, and then some more) + - major version is not an int + - major version is negative + - minor version is not an int + - minor version is negative + + We only need to do Step 1 of installation, then check the CSR. + + """ + def test_v2_template_too_few_parts(self): + _test_invalid_profile(self.master, '1.2.3.4') + + def test_v2_template_too_many_parts(self): + _test_invalid_profile(self.master, '1.2.3.4:100:200:300') + + def test_v2_template_major_version_not_int(self): + _test_invalid_profile(self.master, '1.2.3.4:wat:200') + + def test_v2_template_major_version_negative(self): + _test_invalid_profile(self.master, '1.2.3.4:-1:200') + + def test_v2_template_minor_version_not_int(self): + _test_invalid_profile(self.master, '1.2.3.4:100:wat') + + def test_v2_template_minor_version_negative(self): + _test_invalid_profile(self.master, '1.2.3.4:100:-2') + + def test_v2_template_valid_major_only(self): + _test_valid_profile( + self.master, ipa_x509.MSCSTemplateV2, '1.2.3.4:100') + + +class TestExternalCAProfileV2MajorMinor(IntegrationTest): + """ + Test that V2 template specifiers _with_ minor version get + propagated to CSR. All error modes of V2 template specifiers + were tested in ``TestExternalCAProfileV2Major``. + + We only need to do Step 1 of installation, then check the CSR. + + """ + def test_v2_template_valid_major_minor(self): + _test_valid_profile( + self.master, ipa_x509.MSCSTemplateV2, '1.2.3.4:100:200') From a627df87c31e4d8399bd9fab43c0c4772ddd8955 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Thu, 11 Jul 2019 20:22:33 +1000 Subject: [PATCH] Collapse --external-ca-profile tests into single class To avoid having to spawn new CI hosts for each kind of --external-ca-profile argument we are testing, collapse the three separate test classes into one. Uninstall the half-installed IPA after each section of tests. This change is in response to review comment https://github.com/freeipa/freeipa/pull/2852#pullrequestreview-220442170. Part of: https://pagure.io/freeipa/issue/7548 Reviewed-By: Florence Blanc-Renaud --- ipatests/test_integration/test_external_ca.py | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/ipatests/test_integration/test_external_ca.py b/ipatests/test_integration/test_external_ca.py index dc9a09b43b..714aebd4a8 100644 --- a/ipatests/test_integration/test_external_ca.py +++ b/ipatests/test_integration/test_external_ca.py @@ -501,8 +501,18 @@ def _test_valid_profile(master, profile_cls, profile): check_mscs_extension(ipa_csr, profile_cls(profile)) -class TestExternalCAProfileV1(IntegrationTest): +class TestExternalCAProfileScenarios(IntegrationTest): """ + Test the various --external-ca-profile scenarios. + This test is broken into sections, with each section first + testing invalid arguments, then a valid argument, and finally + uninstalling the half-installed IPA. + + """ + + ''' + Tranche 1: version 1 templates. + Test that --external-ca-profile=Foo gets propagated to the CSR. The default template extension when --external-ca-type=ms-cs, @@ -511,7 +521,7 @@ class TestExternalCAProfileV1(IntegrationTest): We only need to do Step 1 of installation, then check the CSR. - """ + ''' def test_invalid_v1_template(self): _test_invalid_profile(self.master, 'NotAnOid:1') @@ -519,9 +529,12 @@ def test_valid_v1_template(self): _test_valid_profile( self.master, ipa_x509.MSCSTemplateV1, 'TemplateOfAwesome') + def test_uninstall_1(self): + tasks.uninstall_master(self.master) + + ''' + Tranche 2: V2 templates without minor version. -class TestExternalCAProfileV2MajorOnly(IntegrationTest): - """ Test that V2 template specifiers without minor version get propagated to CSR. This class also tests all error modes in specifying a V2 template, those being: @@ -535,7 +548,7 @@ class TestExternalCAProfileV2MajorOnly(IntegrationTest): We only need to do Step 1 of installation, then check the CSR. - """ + ''' def test_v2_template_too_few_parts(self): _test_invalid_profile(self.master, '1.2.3.4') @@ -558,16 +571,21 @@ def test_v2_template_valid_major_only(self): _test_valid_profile( self.master, ipa_x509.MSCSTemplateV2, '1.2.3.4:100') + def test_uninstall_2(self): + tasks.uninstall_master(self.master) + + ''' + Tranche 3: V2 templates with minor version. -class TestExternalCAProfileV2MajorMinor(IntegrationTest): - """ Test that V2 template specifiers _with_ minor version get propagated to CSR. All error modes of V2 template specifiers were tested in ``TestExternalCAProfileV2Major``. We only need to do Step 1 of installation, then check the CSR. - """ + ''' def test_v2_template_valid_major_minor(self): _test_valid_profile( self.master, ipa_x509.MSCSTemplateV2, '1.2.3.4:100:200') + + # this is the end; no need to uninstall. From 740964c3c47fd2cd216c233d8d9df1840eaa01ee Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Thu, 11 Jul 2019 20:27:02 +1000 Subject: [PATCH] ci: add --external-ca-profile tests to nightly Part of: https://pagure.io/freeipa/issue/7548 Reviewed-By: Florence Blanc-Renaud --- ipatests/prci_definitions/nightly_f28.yaml | 12 ++++++++++++ ipatests/prci_definitions/nightly_f29.yaml | 12 ++++++++++++ ipatests/prci_definitions/nightly_master.yaml | 12 ++++++++++++ ipatests/prci_definitions/nightly_master_pki.yaml | 12 ++++++++++++ ipatests/prci_definitions/nightly_rawhide.yaml | 12 ++++++++++++ 5 files changed, 60 insertions(+) diff --git a/ipatests/prci_definitions/nightly_f28.yaml b/ipatests/prci_definitions/nightly_f28.yaml index fe86730444..d1605e6b5c 100644 --- a/ipatests/prci_definitions/nightly_f28.yaml +++ b/ipatests/prci_definitions/nightly_f28.yaml @@ -75,6 +75,18 @@ jobs: timeout: 3600 topology: *master_1repl + fedora-28/external_ca_templates: + requires: [fedora-28/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{fedora-28/build_url}' + test_suite: test_integration/test_external_ca.py::TestExternalCAProfileScenarios + template: *ci-master-f28 + timeout: 3600 + topology: *master_1repl + fedora-28/test_topologies: requires: [fedora-28/build] priority: 50 diff --git a/ipatests/prci_definitions/nightly_f29.yaml b/ipatests/prci_definitions/nightly_f29.yaml index 57c1b624fe..ed88eb15c8 100644 --- a/ipatests/prci_definitions/nightly_f29.yaml +++ b/ipatests/prci_definitions/nightly_f29.yaml @@ -75,6 +75,18 @@ jobs: timeout: 3600 topology: *master_1repl + fedora-29/external_ca_templates: + requires: [fedora-29/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{fedora-29/build_url}' + test_suite: test_integration/test_external_ca.py::TestExternalCAProfileScenarios + template: *ci-master-f29 + timeout: 3600 + topology: *master_1repl + fedora-29/test_topologies: requires: [fedora-29/build] priority: 50 diff --git a/ipatests/prci_definitions/nightly_master.yaml b/ipatests/prci_definitions/nightly_master.yaml index dc63f37426..0a66a13490 100644 --- a/ipatests/prci_definitions/nightly_master.yaml +++ b/ipatests/prci_definitions/nightly_master.yaml @@ -75,6 +75,18 @@ jobs: timeout: 3600 topology: *master_1repl + fedora-30/external_ca_templates: + requires: [fedora-30/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{fedora-30/build_url}' + test_suite: test_integration/test_external_ca.py::TestExternalCAProfileScenarios + template: *ci-master-f30 + timeout: 3600 + topology: *master_1repl + fedora-30/test_topologies: requires: [fedora-30/build] priority: 50 diff --git a/ipatests/prci_definitions/nightly_master_pki.yaml b/ipatests/prci_definitions/nightly_master_pki.yaml index 1bb0af0244..ed2e38d3ed 100644 --- a/ipatests/prci_definitions/nightly_master_pki.yaml +++ b/ipatests/prci_definitions/nightly_master_pki.yaml @@ -75,6 +75,18 @@ jobs: timeout: 3600 topology: *master_1repl + fedora-29/external_ca_templates: + requires: [fedora-29/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{fedora-29/build_url}' + test_suite: test_integration/test_external_ca.py::TestExternalCAProfileScenarios + template: *pki-master-f29 + timeout: 3600 + topology: *master_1repl + fedora-29/test_vault: requires: [fedora-29/build] priority: 50 diff --git a/ipatests/prci_definitions/nightly_rawhide.yaml b/ipatests/prci_definitions/nightly_rawhide.yaml index 301878467c..14433fcc0a 100644 --- a/ipatests/prci_definitions/nightly_rawhide.yaml +++ b/ipatests/prci_definitions/nightly_rawhide.yaml @@ -75,6 +75,18 @@ jobs: timeout: 3600 topology: *master_1repl + fedora-rawhide/external_ca_templates: + requires: [fedora-rawhide/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{fedora-rawhide/build_url}' + test_suite: test_integration/test_external_ca.py::TestExternalCAProfileScenarios + template: *ci-master-frawhide + timeout: 3600 + topology: *master_1repl + fedora-rawhide/test_topologies: requires: [fedora-rawhide/build] priority: 50 From 011c5283cec28ea4361eff5d2ee98da9cd3db41a Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Thu, 11 Jul 2019 20:27:02 +1000 Subject: [PATCH] ci: add --external-ca-profile tests to gating Part of: https://pagure.io/freeipa/issue/7548 Reviewed-By: Florence Blanc-Renaud --- ipatests/prci_definitions/gating.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ipatests/prci_definitions/gating.yaml b/ipatests/prci_definitions/gating.yaml index 4d0107d956..81fa4bba10 100644 --- a/ipatests/prci_definitions/gating.yaml +++ b/ipatests/prci_definitions/gating.yaml @@ -87,6 +87,18 @@ jobs: timeout: 3600 topology: *master_1repl + fedora-30/external_ca_templates: + requires: [fedora-30/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{fedora-30/build_url}' + test_suite: test_integration/test_external_ca.py::TestExternalCAProfileScenarios + template: *ci-master-f30 + timeout: 3600 + topology: *master_1repl + fedora-30/test_topologies: requires: [fedora-30/build] priority: 50