From acc6b2e1e00db080bfb0cd892621800812b58bc3 Mon Sep 17 00:00:00 2001 From: jeanluc Date: Tue, 29 Aug 2023 12:36:12 +0200 Subject: [PATCH] Add unhappy path tests, improve validation --- salt/utils/x509.py | 61 ++++++-- tests/pytests/unit/utils/test_x509.py | 200 ++++++++++++++++++++++++++ 2 files changed, 247 insertions(+), 14 deletions(-) diff --git a/salt/utils/x509.py b/salt/utils/x509.py index 9d4aa664e50..37ee5155f73 100644 --- a/salt/utils/x509.py +++ b/salt/utils/x509.py @@ -1683,22 +1683,40 @@ def _deserialize_openssl_confstring(conf, multiple=False): def _parse_general_names(val): - def idna_encode(val, allow_leading_dot=False): - if HAS_IDNA: - # A leading dot is allowed in some values. - # idna complains about it not being a valid domain name - has_dot = False - if allow_leading_dot: - has_dot = val.startswith(".") - val = val.lstrip(".") - ret = ".".join( - idna.encode(elem).decode() if elem != "*" else elem - for elem in val.split(".") + def idna_encode(val, allow_leading_dot=False, allow_wildcard=False): + # A leading dot is allowed in some values (nameConstraints). + # idna complains about it not being a valid domain name + try: + has_dot = val.startswith(".") + except AttributeError: + raise SaltInvocationError( + f"Expected string value, got {type(val).__name__}: `{val}`" ) + if has_dot: + if not allow_leading_dot: + raise CommandExecutionError( + "Leading dots are not allowed in this context" + ) + val = val.lstrip(".") + has_wildcard = val.startswith("*.") + if has_wildcard: + if not allow_wildcard: + raise CommandExecutionError("Wildcards are not allowed in this context") if has_dot: - return f".{ret}" - return ret + raise CommandExecutionError( + "Wildcards and leading dots cannot be present together" + ) + val = val[2:] + if val.startswith("."): + raise CommandExecutionError("Empty label") + if HAS_IDNA: + try: + ret = idna.encode(val).decode() + except idna.IDNAError as err: + raise CommandExecutionError(str(err)) from err else: + if not val: + raise CommandExecutionError("Empty domain") try: val.encode(encoding="ascii") except UnicodeEncodeError as err: @@ -1706,6 +1724,20 @@ def _parse_general_names(val): "Cannot encode non-ASCII strings to internationalized domain " "name format, missing library: idna" ) from err + for elem in val.split("."): + if not elem: + raise CommandExecutionError("Empty Label") + invalid = re.search(r"[^A-Za-z\d\-\.]", elem) + if invalid is not None: + raise CommandExecutionError( + f"Codepoint U+00{hex(ord(invalid.group()))[2:]} at position {invalid.end()} of '{val}' not allowed" + ) + ret = val + if has_dot: + return f".{ret}" + if has_wildcard: + return f"*.{ret}" + return ret valid_types = { "email": cx509.general_name.RFC822Name, @@ -1741,6 +1773,7 @@ def _parse_general_names(val): domain = idna_encode(domain) v = "@".join((user, domain)) else: + # nameConstraints v = idna_encode(splits[0], allow_leading_dot=True) elif typ == "uri": url = urlparse(v) @@ -1750,7 +1783,7 @@ def _parse_general_names(val): (url.scheme, domain, url.path, url.params, url.query, url.fragment) ) elif typ == "dns": - v = idna_encode(v, allow_leading_dot=True) + v = idna_encode(v, allow_leading_dot=True, allow_wildcard=True) elif typ == "othername": raise SaltInvocationError("otherName is currently not implemented") if typ in valid_types: diff --git a/tests/pytests/unit/utils/test_x509.py b/tests/pytests/unit/utils/test_x509.py index 66a870fc1c4..f13ac97fb33 100644 --- a/tests/pytests/unit/utils/test_x509.py +++ b/tests/pytests/unit/utils/test_x509.py @@ -1053,6 +1053,7 @@ class TestCreateExtension: "inpt,cls,parsed", [ (("email", "me@example.com"), cx509.RFC822Name, "me@example.com"), + (("email", ".example.com"), cx509.RFC822Name, ".example.com"), ( ("email", "me@überexample.com"), cx509.RFC822Name, @@ -1068,9 +1069,16 @@ class TestCreateExtension: cx509.UniformResourceIdentifier, "https://www.xn--berexample-8db.com", ), + (("URI", "some/path/only"), cx509.UniformResourceIdentifier, "some/path/only"), (("DNS", "example.com"), cx509.DNSName, "example.com"), (("DNS", "überexample.com"), cx509.DNSName, "xn--berexample-8db.com"), (("DNS", "*.überexample.com"), cx509.DNSName, "*.xn--berexample-8db.com"), + (("DNS", ".überexample.com"), cx509.DNSName, ".xn--berexample-8db.com"), + ( + ("DNS", "γνῶθι.σεαυτόν.gr"), + cx509.DNSName, + "xn--oxakdo9327a.xn--mxahzvhf4c.gr", + ), (("RID", "1.2.3.4"), cx509.RegisteredID, cx509.ObjectIdentifier("1.2.3.4")), ( ("IP", "13.37.13.37"), @@ -1187,9 +1195,108 @@ class TestCreateExtension: ), ], ), + ( + ("DNS", "some.invalid_doma.in"), + salt.exceptions.CommandExecutionError, + "at position 8.*not allowed$", + ), + ( + ("DNS", "some..invalid-doma.in"), + salt.exceptions.CommandExecutionError, + "Empty Label", + ), + ( + ("DNS", "invalid*.wild.card"), + salt.exceptions.CommandExecutionError, + "at position 8.*not allowed", + ), + ( + ("DNS", "invalid.*.wild.card"), + salt.exceptions.CommandExecutionError, + "at position 1.*not allowed", + ), + ( + ("DNS", "*..whats.this"), + salt.exceptions.CommandExecutionError, + "Empty label", + ), + ( + ("DNS", 42), + salt.exceptions.SaltInvocationError, + "Expected string value, got int", + ), + ( + ("DNS", ""), + salt.exceptions.CommandExecutionError, + "Empty domain", + ), + ( + ("DNS", "ἀνεῤῥίφθω.κύβος͵.gr"), + salt.exceptions.CommandExecutionError, + "not allowed at position 6 in 'κύβος͵'$", + ), + ( + ("DNS", "می\u200cخواهم\u200c.iran"), + salt.exceptions.CommandExecutionError, + "^Unknown codepoint adjacent to joiner.* at position 9", + ), + ( + ("DNS", ".*.wildcard-dot.test"), + salt.exceptions.CommandExecutionError, + "Wildcards and leading dots cannot be present together", + ), + ( + ("email", "invalid@*.mail.address"), + salt.exceptions.CommandExecutionError, + "Wildcards are not allowed in this context", + ), + ( + ("email", "invalid@.mail.address"), + salt.exceptions.CommandExecutionError, + "Leading dots are not allowed in this context", + ), + ( + ("email", "Invalid Email "), + salt.exceptions.CommandExecutionError, + "not allowed$", + ), + ( + ("IP", "this is not an IP address"), + salt.exceptions.CommandExecutionError, + "does not seem to be an IP address or network range.", + ), + ( + ("URI", "https://*.χάος.σκάλα.gr"), + salt.exceptions.CommandExecutionError, + "Wildcards are not allowed in this context", + ), + ( + ("URI", "https://.invalid.host"), + salt.exceptions.CommandExecutionError, + "Leading dots are not allowed in this context", + ), + ( + ("dirName", "Et tu, Brute?"), + salt.exceptions.CommandExecutionError, + "Failed parsing rfc4514 dirName string", + ), + ( + ("otherName", "otherName:1.2.3.4;UTF8:some other identifier"), + salt.exceptions.SaltInvocationError, + "otherName is currently not implemented", + ), + ( + ("invalidType", "L'état c'est moi!"), + salt.exceptions.CommandExecutionError, + "GeneralName type invalidtype is invalid", + ), ], ) def test_parse_general_names(inpt, cls, parsed): + if issubclass(cls, Exception): + with pytest.raises(cls, match=parsed): + x509._parse_general_names([inpt]) + return expected = cls(parsed) res = x509._parse_general_names([inpt]) if inpt[0] == "dirName": @@ -1198,6 +1305,99 @@ def test_parse_general_names(inpt, cls, parsed): assert res[0] == expected +@pytest.mark.parametrize( + "inpt,cls,parsed", + [ + (("email", "me@example.com"), cx509.RFC822Name, "me@example.com"), + ( + ("URI", "https://www.example.com"), + cx509.UniformResourceIdentifier, + "https://www.example.com", + ), + (("DNS", "example.com"), cx509.DNSName, "example.com"), + (("DNS", "*.example.com"), cx509.DNSName, "*.example.com"), + (("DNS", ".example.com"), cx509.DNSName, ".example.com"), + ( + ("DNS", "invalid*.wild.card"), + salt.exceptions.CommandExecutionError, + "at position 8.*not allowed", + ), + ( + ("DNS", "invalid.*.wild.card"), + salt.exceptions.CommandExecutionError, + "at position 1.*not allowed", + ), + ( + ("DNS", ".*.wildcard-dot.test"), + salt.exceptions.CommandExecutionError, + "Wildcards and leading dots cannot be present together", + ), + ( + ("DNS", "gott.würfelt.nicht"), + salt.exceptions.CommandExecutionError, + "Cannot encode non-ASCII strings", + ), + ( + ("DNS", "some.invalid_doma.in"), + salt.exceptions.CommandExecutionError, + "at position 8.*not allowed$", + ), + ( + ("DNS", "some..invalid-doma.in"), + salt.exceptions.CommandExecutionError, + "Empty Label", + ), + ( + ("DNS", 42), + salt.exceptions.SaltInvocationError, + "Expected string value, got int", + ), + ( + ("DNS", ""), + salt.exceptions.CommandExecutionError, + "Empty domain", + ), + ( + ("DNS", "*..whats.this"), + salt.exceptions.CommandExecutionError, + "Empty label", + ), + ( + ("email", "invalid@*.mail.address"), + salt.exceptions.CommandExecutionError, + "Wildcards are not allowed in this context", + ), + ( + ("email", "invalid@.mail.address"), + salt.exceptions.CommandExecutionError, + "Leading dots are not allowed in this context", + ), + ( + ("email", "Invalid Email "), + salt.exceptions.CommandExecutionError, + "not allowed$", + ), + ( + ("URI", "https://.invalid.host"), + salt.exceptions.CommandExecutionError, + "Leading dots are not allowed in this context", + ), + ], +) +def test_parse_general_names_without_idna(inpt, cls, parsed): + with patch("salt.utils.x509.HAS_IDNA", False): + if issubclass(cls, Exception): + with pytest.raises(cls, match=parsed): + x509._parse_general_names([inpt]) + return + expected = cls(parsed) + res = x509._parse_general_names([inpt]) + if inpt[0] == "dirName": + assert res[0].value == expected + else: + assert res[0] == expected + + @pytest.mark.parametrize( "inpt", [