From 70683e540d09b4931bf3dec20ed8fb5c02b25059 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Wed, 24 Jun 2020 06:38:42 +1000 Subject: [PATCH] Support otherName in subAltName in CSR for UTF8 strings (#53) * Support otherName in subAltName in CSR for UTF8 strings * Remove uneeded docs and added changelog fragment * Missed a merge conflict * Fix up sanity issues and added test expectation * Rename function --- .../fragments/openssl_csr-otherName.yml | 2 + plugins/module_utils/crypto/_asn1.py | 153 ++++++++++++++++++ .../crypto/cryptography_support.py | 19 ++- plugins/modules/openssl_csr.py | 11 +- .../targets/openssl_csr/tasks/impl.yml | 3 + .../targets/openssl_csr/tests/validate.yml | 1 + .../plugins/module_utils/crypto/__init__.py | 0 .../plugins/module_utils/crypto/test_asn1.py | 91 +++++++++++ .../crypto/test_cryptography_support.py | 33 ++++ 9 files changed, 309 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/openssl_csr-otherName.yml create mode 100644 plugins/module_utils/crypto/_asn1.py create mode 100644 tests/unit/plugins/module_utils/crypto/__init__.py create mode 100644 tests/unit/plugins/module_utils/crypto/test_asn1.py create mode 100644 tests/unit/plugins/module_utils/crypto/test_cryptography_support.py diff --git a/changelogs/fragments/openssl_csr-otherName.yml b/changelogs/fragments/openssl_csr-otherName.yml new file mode 100644 index 00000000..d97f9de3 --- /dev/null +++ b/changelogs/fragments/openssl_csr-otherName.yml @@ -0,0 +1,2 @@ +minor_changes: +- openssl_csr - Add support for specifying the SAN ``otherName`` value in the OpenSSL ASN.1 UTF8 string format, ``otherName:;UTF8:string value``. diff --git a/plugins/module_utils/crypto/_asn1.py b/plugins/module_utils/crypto/_asn1.py new file mode 100644 index 00000000..17feb2f7 --- /dev/null +++ b/plugins/module_utils/crypto/_asn1.py @@ -0,0 +1,153 @@ +# -*- coding: utf-8 -*- + +# (c) 2020, Jordan Borean +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +import re + +from ansible.module_utils._text import to_bytes + + +""" +An ASN.1 serialized as a string in the OpenSSL format: + [modifier,]type[:value] + +modifier: + The modifier can be 'IMPLICIT:,' or 'EXPLICIT:' where IMPLICIT + changes the tag of the universal value to encode and EXPLICIT prefixes its tag to the existing universal value. + The tag_number must be set while the tag_class can be 'U', 'A', 'P', or 'C" for 'Universal', 'Application', + 'Private', or 'Context Specific' with C being the default. + +type: + The underlying ASN.1 type of the value specified. Currently only the following have been implemented: + UTF8: The value must be a UTF-8 encoded string. + +value: + The value to encode, the format of this value depends on the specified. +""" +ASN1_STRING_REGEX = re.compile(r'^((?PIMPLICIT|EXPLICIT):(?P\d+)(?PU|A|P|C)?,)?' + r'(?P[\w\d]+):(?P.*)') + + +class TagClass: + universal = 0 + application = 1 + context_specific = 2 + private = 3 + + +# Universal tag numbers that can be encoded. +class TagNumber: + utf8_string = 12 + + +def _pack_octet_integer(value): + """ Packs an integer value into 1 or multiple octets. """ + # NOTE: This is *NOT* the same as packing an ASN.1 INTEGER like value. + octets = bytearray() + + # Continue to shift the number by 7 bits and pack into an octet until the + # value is fully packed. + while value: + octet_value = value & 0b01111111 + + # First round (last octet) must have the MSB set. + if len(octets): + octet_value |= 0b10000000 + + octets.append(octet_value) + value >>= 7 + + # Reverse to ensure the higher order octets are first. + octets.reverse() + return bytes(octets) + + +def serialize_asn1_string_as_der(value): + """ Deserializes an ASN.1 string to a DER encoded byte string. """ + asn1_match = ASN1_STRING_REGEX.match(value) + if not asn1_match: + raise ValueError("The ASN.1 serialized string must be in the format [modifier,]type[:value]") + + tag_type = asn1_match.group('tag_type') + tag_number = asn1_match.group('tag_number') + tag_class = asn1_match.group('tag_class') or 'C' + value_type = asn1_match.group('value_type') + asn1_value = asn1_match.group('value') + + if value_type != 'UTF8': + raise ValueError('The ASN.1 serialized string is not a known type "{0}", only UTF8 types are ' + 'supported'.format(value_type)) + + b_value = to_bytes(asn1_value, encoding='utf-8', errors='surrogate_or_strict') + + # We should only do a universal type tag if not IMPLICITLY tagged or the tag class is not universal. + if not tag_type or (tag_type == 'EXPLICIT' and tag_class != 'U'): + b_value = pack_asn1(TagClass.universal, False, TagNumber.utf8_string, b_value) + + if tag_type: + tag_class = { + 'U': TagClass.universal, + 'A': TagClass.application, + 'P': TagClass.private, + 'C': TagClass.context_specific, + }[tag_class] + + # When adding support for more types this should be looked into further. For now it works with UTF8Strings. + constructed = tag_type == 'EXPLICIT' and tag_class != TagClass.universal + b_value = pack_asn1(tag_class, constructed, int(tag_number), b_value) + + return b_value + + +def pack_asn1(tag_class, constructed, tag_number, b_data): + """Pack the value into an ASN.1 data structure. + + The structure for an ASN.1 element is + + | Identifier Octet(s) | Length Octet(s) | Data Octet(s) | + """ + b_asn1_data = bytearray() + + if tag_class < 0 or tag_class > 3: + raise ValueError("tag_class must be between 0 and 3 not %s" % tag_class) + + # Bit 8 and 7 denotes the class. + identifier_octets = tag_class << 6 + # Bit 6 denotes whether the value is primitive or constructed. + identifier_octets |= ((1 if constructed else 0) << 5) + + # Bits 5-1 contain the tag number, if it cannot be encoded in these 5 bits + # then they are set and another octet(s) is used to denote the tag number. + if tag_number < 31: + identifier_octets |= tag_number + b_asn1_data.append(identifier_octets) + else: + identifier_octets |= 31 + b_asn1_data.append(identifier_octets) + b_asn1_data.extend(_pack_octet_integer(tag_number)) + + length = len(b_data) + + # If the length can be encoded in 7 bits only 1 octet is required. + if length < 128: + b_asn1_data.append(length) + + else: + # Otherwise the length must be encoded across multiple octets + length_octets = bytearray() + while length: + length_octets.append(length & 0b11111111) + length >>= 8 + + length_octets.reverse() # Reverse to make the higher octets first. + + # The first length octet must have the MSB set alongside the number of + # octets the length was encoded in. + b_asn1_data.append(len(length_octets) | 0b10000000) + b_asn1_data.extend(length_octets) + + return bytes(b_asn1_data) + b_data diff --git a/plugins/module_utils/crypto/cryptography_support.py b/plugins/module_utils/crypto/cryptography_support.py index 3cf4174e..93fb67ce 100644 --- a/plugins/module_utils/crypto/cryptography_support.py +++ b/plugins/module_utils/crypto/cryptography_support.py @@ -24,6 +24,7 @@ import binascii import re from ansible.module_utils._text import to_text +from ._asn1 import serialize_asn1_string_as_der try: import cryptography @@ -205,10 +206,22 @@ def cryptography_get_name(name): raise OpenSSLObjectError('Cannot parse Subject Alternative Name "{0}"'.format(name)) return x509.RegisteredID(x509.oid.ObjectIdentifier(m.group(1))) if name.startswith('otherName:'): + # otherName can either be a raw ASN.1 hex string or in the format that OpenSSL works with. m = re.match(r'^([0-9]+(?:\.[0-9]+)*);([0-9a-fA-F]{1,2}(?::[0-9a-fA-F]{1,2})*)$', to_text(name[10:])) - if not m: - raise OpenSSLObjectError('Cannot parse Subject Alternative Name "{0}"'.format(name)) - return x509.OtherName(x509.oid.ObjectIdentifier(m.group(1)), _parse_hex(m.group(2))) + if m: + return x509.OtherName(x509.oid.ObjectIdentifier(m.group(1)), _parse_hex(m.group(2))) + + # See https://www.openssl.org/docs/man1.0.2/man5/x509v3_config.html - Subject Alternative Name for more + # defailts on the format expected. + name = to_text(name[10:], errors='surrogate_or_strict') + if ';' not in name: + raise OpenSSLObjectError('Cannot parse Subject Alternative Name otherName "{0}", must be in the ' + 'format "otherName:;" or ' + '"otherName:;"'.format(name)) + + oid, value = name.split(';', 1) + b_value = serialize_asn1_string_as_der(value) + return x509.OtherName(x509.ObjectIdentifier(oid), b_value) if name.startswith('dirName:'): return x509.DirectoryName(x509.Name(_parse_dn(to_text(name[8:])))) except Exception as e: diff --git a/plugins/modules/openssl_csr.py b/plugins/modules/openssl_csr.py index f5c6c7ed..c32fe572 100644 --- a/plugins/modules/openssl_csr.py +++ b/plugins/modules/openssl_csr.py @@ -119,7 +119,7 @@ options: - SAN extension to attach to the certificate signing request. - This can either be a 'comma separated string' or a YAML list. - Values must be prefixed by their options. (i.e., C(email), C(URI), C(DNS), C(RID), C(IP), C(dirName), - C(otherName) and the ones specific to your CA) + C(otherName) and the ones specific to your CA). - Note that if no SAN is specified, but a common name, the common name will be added as a SAN except if C(useCommonNameForSAN) is set to I(false). @@ -350,6 +350,15 @@ EXAMPLES = r''' privatekey_path: /etc/ssl/private/ansible.com.pem common_name: www.ansible.com ocsp_must_staple: yes + +- name: Generate an OpenSSL Certificate Signing Request for WinRM Certificate authentication + community.crypto.openssl_csr: + path: /etc/ssl/csr/winrm.auth.csr + privatekey_path: /etc/ssl/private/winrm.auth.pem + common_name: username + extended_key_usage: + - clientAuth + subject_alt_name: otherName:1.3.6.1.4.1.311.20.2.3;UTF8:username@localhost ''' RETURN = r''' diff --git a/tests/integration/targets/openssl_csr/tasks/impl.yml b/tests/integration/targets/openssl_csr/tasks/impl.yml index 040e1909..aa6f9c0c 100644 --- a/tests/integration/targets/openssl_csr/tasks/impl.yml +++ b/tests/integration/targets/openssl_csr/tasks/impl.yml @@ -593,6 +593,7 @@ - "URI:https://example.org/test/index.html" - "RID:1.2.3.4" - "otherName:1.2.3.4;0c:07:63:65:72:74:72:65:71" + - "otherName:1.3.6.1.4.1.311.20.2.3;UTF8:bob@localhost" - "dirName:O = Example Net, CN = example.net" - "dirName:/O=Example Com/CN=example.com" register: everything_1 @@ -673,6 +674,7 @@ - "URI:https://example.org/test/index.html" - "RID:1.2.3.4" - "otherName:1.2.3.4;0c:07:63:65:72:74:72:65:71" + - "otherName:1.3.6.1.4.1.311.20.2.3;UTF8:bob@localhost" - "dirName:O=Example Net,CN=example.net" - "dirName:/O = Example Com/CN = example.com" check_mode: yes @@ -754,6 +756,7 @@ - "URI:https://example.org/test/index.html" - "RID:1.2.3.4" - "otherName:1.2.3.4;0c:07:63:65:72:74:72:65:71" + - "otherName:1.3.6.1.4.1.311.20.2.3;UTF8:bob@localhost" - "dirName:O =Example Net, CN= example.net" - "dirName:/O =Example Com/CN= example.com" register: everything_3 diff --git a/tests/integration/targets/openssl_csr/tests/validate.yml b/tests/integration/targets/openssl_csr/tests/validate.yml index 76822fbe..dbbd07ae 100644 --- a/tests/integration/targets/openssl_csr/tests/validate.yml +++ b/tests/integration/targets/openssl_csr/tests/validate.yml @@ -268,6 +268,7 @@ "URI:https://example.org/test/index.html", "RID:1.2.3.4", "otherName:1.2.3.4;0c:07:63:65:72:74:72:65:71", + "otherName:1.3.6.1.4.1.311.20.2.3;0c:0d:62:6f:62:40:6c:6f:63:61:6c:68:6f:73:74", "dirName:/O=Example Net/CN=example.net", "dirName:/O=Example Com/CN=example.com" ] diff --git a/tests/unit/plugins/module_utils/crypto/__init__.py b/tests/unit/plugins/module_utils/crypto/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/plugins/module_utils/crypto/test_asn1.py b/tests/unit/plugins/module_utils/crypto/test_asn1.py new file mode 100644 index 00000000..762ebfcc --- /dev/null +++ b/tests/unit/plugins/module_utils/crypto/test_asn1.py @@ -0,0 +1,91 @@ +# -*- coding: utf-8 -*- + +# (c) 2020, Jordan Borean +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +import base64 +import re +import subprocess + +import pytest + +from ansible_collections.community.crypto.plugins.module_utils.crypto._asn1 import ( + serialize_asn1_string_as_der, + pack_asn1, +) + + +TEST_CASES = [ + ('UTF8:Hello World', b'\x0c\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + + ('EXPLICIT:10,UTF8:Hello World', b'\xaa\x0d\x0c\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + ('EXPLICIT:12U,UTF8:Hello World', b'\x0c\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + ('EXPLICIT:10A,UTF8:Hello World', b'\x6a\x0d\x0c\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + ('EXPLICIT:10P,UTF8:Hello World', b'\xea\x0d\x0c\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + ('EXPLICIT:10C,UTF8:Hello World', b'\xaa\x0d\x0c\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + ('EXPLICIT:1024P,UTF8:Hello World', b'\xff\x88\x00\x0d\x0c\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + + ('IMPLICIT:10,UTF8:Hello World', b'\x8a\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + ('IMPLICIT:12U,UTF8:Hello World', b'\x0c\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + ('IMPLICIT:10A,UTF8:Hello World', b'\x4a\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + ('IMPLICIT:10P,UTF8:Hello World', b'\xca\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + ('IMPLICIT:10C,UTF8:Hello World', b'\x8a\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + ('IMPLICIT:1024P,UTF8:Hello World', b'\xdf\x88\x00\x0b\x48\x65\x6c\x6c\x6f\x20\x57\x6f\x72\x6c\x64'), + + # Tests large data lengths, special logic for the length octet encoding. + ('UTF8:' + ('A' * 600), b'\x0c\x82\x02\x58' + (b'\x41' * 600)), + + # This isn't valid with openssl asn1parse but has been validated against an ASN.1 parser. OpenSSL seems to read the + # data u"café" encoded as UTF-8 bytes b"caf\xc3\xa9", decodes that internally with latin-1 (or similar variant) as + # u"café" then encodes that to UTF-8 b"caf\xc3\x83\xc2\xa9" for the UTF8String. Ultimately openssl is wrong here + # so we keep our assertion happening. + (u'UTF8:café', b'\x0c\x05\x63\x61\x66\xc3\xa9'), +] + + +@pytest.mark.parametrize('value, expected', TEST_CASES) +def test_serialize_asn1_string_as_der(value, expected): + actual = serialize_asn1_string_as_der(value) + print("%s | %s" % (value, base64.b16encode(actual).decode())) + assert actual == expected + + +@pytest.mark.parametrize('value', [ + 'invalid', + 'EXPLICIT,UTF:value', +]) +def test_serialize_asn1_string_as_der_invalid_format(value): + expected = "The ASN.1 serialized string must be in the format [modifier,]type[:value]" + with pytest.raises(ValueError, match=re.escape(expected)): + serialize_asn1_string_as_der(value) + + +def test_serialize_asn1_string_as_der_invalid_type(): + expected = "The ASN.1 serialized string is not a known type \"OID\", only UTF8 types are supported" + with pytest.raises(ValueError, match=re.escape(expected)): + serialize_asn1_string_as_der("OID:1.2.3.4") + + +def test_pack_asn_invalid_class(): + with pytest.raises(ValueError, match="tag_class must be between 0 and 3 not 4"): + pack_asn1(4, True, 0, b"") + + +@pytest.mark.skip() # This is to just to build the test case assertions and shouldn't run normally. +@pytest.mark.parametrize('value, expected', TEST_CASES) +def test_test_cases(value, expected, tmp_path): + test_file = tmp_path / 'test.der' + subprocess.run(['openssl', 'asn1parse', '-genstr', value, '-noout', '-out', test_file]) + + with open(test_file, mode='rb') as fd: + b_data = fd.read() + + hex_str = base64.b16encode(b_data).decode().lower() + print("%s | \\x%s" % (value, "\\x".join([hex_str[i:i + 2] for i in range(0, len(hex_str), 2)]))) + + # This is a know edge case where openssl asn1parse does not work properly. + if value != u'UTF8:café': + assert b_data == expected diff --git a/tests/unit/plugins/module_utils/crypto/test_cryptography_support.py b/tests/unit/plugins/module_utils/crypto/test_cryptography_support.py new file mode 100644 index 00000000..6a5545b0 --- /dev/null +++ b/tests/unit/plugins/module_utils/crypto/test_cryptography_support.py @@ -0,0 +1,33 @@ +# -*- coding: utf-8 -*- + +# (c) 2020, Jordan Borean +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +import pytest + +from ansible_collections.community.crypto.plugins.module_utils.crypto.basic import ( + OpenSSLObjectError, +) + +from ansible_collections.community.crypto.plugins.module_utils.crypto.cryptography_support import ( + cryptography_get_name, +) + + +def test_cryptography_get_name_invalid_prefix(): + with pytest.raises(OpenSSLObjectError, match="Cannot parse Subject Alternative Name"): + cryptography_get_name('fake:value') + + +def test_cryptography_get_name_other_name_no_oid(): + with pytest.raises(OpenSSLObjectError, match="Cannot parse Subject Alternative Name otherName"): + cryptography_get_name('otherName:value') + + +def test_cryptography_get_name_other_name_utfstring(): + actual = cryptography_get_name('otherName:1.3.6.1.4.1.311.20.2.3;UTF8:Hello World') + assert actual.type_id.dotted_string == '1.3.6.1.4.1.311.20.2.3' + assert actual.value == b'\x0c\x0bHello World'