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

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:<OID>;UTF8:string value``.

plugins/module_utils/crypto/_asn1.py

@@ -0,0 +1,153 @@
+# -*- coding: utf-8 -*-
+
+# (c) 2020, Jordan Borean <jborean93@gmail.com>
+# 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:<tag_number><tag_class>,' or 'EXPLICIT:<tag_number><tag_class>' 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 <type> specified.
+"""
+ASN1_STRING_REGEX = re.compile(r'^((?P<tag_type>IMPLICIT|EXPLICIT):(?P<tag_number>\d+)(?P<tag_class>U|A|P|C)?,)?'
+                               r'(?P<value_type>[\w\d]+):(?P<value>.*)')
+
+
+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

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:
+            if 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)))
-            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:<OID>;<ASN.1 OpenSSL Encoded String>" or '
+                                         '"otherName:<OID>;<hex string>"'.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:

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'''

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

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"
         ]

tests/unit/plugins/module_utils/crypto/test_asn1.py

@@ -0,0 +1,91 @@
+# -*- coding: utf-8 -*-
+
+# (c) 2020, Jordan Borean <jborean93@gmail.com>
+# 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

tests/unit/plugins/module_utils/crypto/test_cryptography_support.py

@@ -0,0 +1,33 @@
+# -*- coding: utf-8 -*-
+
+# (c) 2020, Jordan Borean <jborean93@gmail.com>
+# 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'