Re: [PATCH 1/2] tls: Validate peer certificate's DNSNames against mask

Re: [PATCH 1/2] tls: Validate peer certificate's DNSNames against mask

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

Hi Andrew,

On 8/23/19 7:47 PM, Andrew Zaborowski wrote:
> Also return success in the domain_mask check if any of the DNSNames in
> the peer certificate's subjectAltName extension matches any of the mask
> strings supplied.
> ---
>   ell/tls.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 62 insertions(+), 8 deletions(-)
> 

Both applied, thanks.

Regards,
-Denis

[PATCH 1/2] tls: Validate peer certificate's DNSNames against mask

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 3312 bytes --]

Also return success in the domain_mask check if any of the DNSNames in
the peer certificate's subjectAltName extension matches any of the mask
strings supplied.
---
 ell/tls.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 62 insertions(+), 8 deletions(-)

diff --git a/ell/tls.c b/ell/tls.c
index cbcdfa4..d0e2a66 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -673,26 +673,80 @@ ok_next:
 	}
 }
 
+static const struct asn1_oid subject_alt_name_oid =
+	{ 3, { 0x55, 0x1d, 0x11 } };
 static const struct asn1_oid dn_common_name_oid =
 	{ 3, { 0x55, 0x04, 0x03 } };
 
+#define SAN_DNS_NAME_ID ASN1_CONTEXT_IMPLICIT(2)
+
 static bool tls_cert_domains_match_mask(struct l_cert *cert, char **mask)
 {
-	const uint8_t *dn, *end;
-	size_t dn_size;
+	const uint8_t *san, *dn, *end;
+	size_t san_len, dn_len;
+	uint8_t san_tag;
 	const char *cn = NULL;
 	size_t cn_len;
+	char **i;
+	bool dns_name_present = false;
+
+	/*
+	 * Locate SubjectAltName (RFC5280 Section 4.2.1.6) and descend into
+	 * the sole SEQUENCE element, check if any DNSName matches.
+	 */
+	san = cert_get_extension(cert, &subject_alt_name_oid, NULL, &san_len);
+	if (san) {
+		san = asn1_der_find_elem(san, san_len, 0, &san_tag, &san_len);
+		if (unlikely(!san || san_tag != ASN1_ID_SEQUENCE))
+			return false;
+
+		end = san + san_len;
+		while (san < end) {
+			const uint8_t *value;
+			uint8_t tag;
+			size_t len;
+
+			value = asn1_der_find_elem(san, end - san,
+							SAN_DNS_NAME_ID,
+							&tag, &len);
+			if (!value)
+				break;
+
+			/* Type is implicitly IA5STRING */
+
+			for (i = mask; *i; i++)
+				if (tls_domain_match_mask((const char *) value,
+							len, *i, strlen(*i)))
+					return true;
+
+			san = value + len;
+			dns_name_present = true;
+		}
+	}
 
 	/*
 	 * Retrieve the Common Name from the Subject DN and check if it
-	 * matches.  TODO: possibly also look at SubjectAltName.
+	 * matches.
+	 *
+	 * We look at the Common Name only if no DNSNames were present in
+	 * the certificate, following Wi-Fi Alliance's Hotspot 2.0
+	 * Specification v3.1 section 7.3.3.2 step 2:
+	 * "Verify in the AAA server certificate that the domain name from
+	 * the FQDN [...] is a suffix match of the domain name in@least
+	 * one of the DNSName SubjectAltName extensions. If a SubjectAltName
+	 * of type DNSName is not present, then the domain name from the
+	 * FQDN shall be a suffix match to the CommonName portion of the
+	 * SubjectName. If neither of these conditions holds, then
+	 * verification fails."
 	 */
+	if (unlikely(dns_name_present))
+		return false;
 
-	dn = l_cert_get_dn(cert, &dn_size);
+	dn = l_cert_get_dn(cert, &dn_len);
 	if (unlikely(!dn))
 		return false;
 
-	end = dn + dn_size;
+	end = dn + dn_len;
 	while (dn < end) {
 		const uint8_t *set, *seq, *oid, *name;
 		uint8_t tag;
@@ -725,11 +779,11 @@ static bool tls_cert_domains_match_mask(struct l_cert *cert, char **mask)
 		}
 	}
 
-	if (!cn)
+	if (unlikely(!cn))
 		return false;
 
-	for (; *mask; mask++)
-		if (tls_domain_match_mask(cn, cn_len, *mask, strlen(*mask)))
+	for (i = mask; *i; i++)
+		if (tls_domain_match_mask(cn, cn_len, *i, strlen(*i)))
 			return true;
 
 	return false;
-- 
2.20.1

[PATCH 2/2] unit: Add TLS tests for cert's DNSName matching

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 13587 bytes --]

Note we switch around the server and client certificates in the test
cases for the Common Name because now the code only looks at the CN if
no DNSNames are present which is only the case in the client-cert.
---
 unit/test-tls.c | 170 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 119 insertions(+), 51 deletions(-)

diff --git a/unit/test-tls.c b/unit/test-tls.c
index 36f9934..f4c5cb1 100644
--- a/unit/test-tls.c
+++ b/unit/test-tls.c
@@ -621,36 +621,66 @@ static void test_tls_version_mismatch_test(const void *data)
 }
 
 static const struct tls_conn_test tls_conn_test_domain_match1 = {
-	.server_cert_path = CERTDIR "cert-server.pem",
-	.server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.server_cert_path = CERTDIR "cert-client.pem",
+	.server_key_path = CERTDIR "cert-client-key-pkcs8.pem",
 	.server_ca_cert_path = CERTDIR "cert-ca.pem",
-	.server_expect_identity = "/O=Bar Example Organization"
-		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
-	.client_cert_path = CERTDIR "cert-client.pem",
-	.client_key_path = CERTDIR "cert-client-key-pkcs8.pem",
-	.client_ca_cert_path = CERTDIR "cert-ca.pem",
-	.client_expect_identity = "/O=Foo Example Organization"
+	.server_expect_identity = "/O=Foo Example Organization"
 		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
-	.client_domain_mask = (char *[]) { "Foo Example Organization", NULL },
+	.client_cert_path = CERTDIR "cert-server.pem",
+	.client_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.client_ca_cert_path = CERTDIR "cert-ca.pem",
+	.client_expect_identity = "/O=Bar Example Organization"
+		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
+	.client_domain_mask = (char *[]) { "Bar Example Organization", NULL },
 };
 
 static const struct tls_conn_test tls_conn_test_domain_match2 = {
-	.server_cert_path = CERTDIR "cert-server.pem",
-	.server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.server_cert_path = CERTDIR "cert-client.pem",
+	.server_key_path = CERTDIR "cert-client-key-pkcs8.pem",
 	.server_ca_cert_path = CERTDIR "cert-ca.pem",
-	.server_expect_identity = "/O=Bar Example Organization"
-		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
-	.client_cert_path = CERTDIR "cert-client.pem",
-	.client_key_path = CERTDIR "cert-client-key-pkcs8.pem",
+	.server_expect_identity = "/O=Foo Example Organization"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_cert_path = CERTDIR "cert-server.pem",
+	.client_key_path = CERTDIR "cert-server-key-pkcs8.pem",
 	.client_ca_cert_path = CERTDIR "cert-ca.pem",
-	.client_expect_identity = "/O=Foo Example Organization"
+	.client_expect_identity = "/O=Bar Example Organization"
+		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
+	.client_domain_mask = (char *[]) {
+		"Bar Example Organization", "Foo Example Organization", NULL
+	},
+};
+
+static const struct tls_conn_test tls_conn_test_domain_match3 = {
+	.server_cert_path = CERTDIR "cert-client.pem",
+	.server_key_path = CERTDIR "cert-client-key-pkcs8.pem",
+	.server_ca_cert_path = CERTDIR "cert-ca.pem",
+	.server_expect_identity = "/O=Foo Example Organization"
 		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_cert_path = CERTDIR "cert-server.pem",
+	.client_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.client_ca_cert_path = CERTDIR "cert-ca.pem",
+	.client_expect_identity = "/O=Bar Example Organization"
+		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
 	.client_domain_mask = (char *[]) {
 		"Foo Example Organization", "Bar Example Organization", NULL
 	},
 };
 
-static const struct tls_conn_test tls_conn_test_domain_match3 = {
+static const struct tls_conn_test tls_conn_test_domain_match4 = {
+	.server_cert_path = CERTDIR "cert-client.pem",
+	.server_key_path = CERTDIR "cert-client-key-pkcs8.pem",
+	.server_ca_cert_path = CERTDIR "cert-ca.pem",
+	.server_expect_identity = "/O=Foo Example Organization"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_cert_path = CERTDIR "cert-server.pem",
+	.client_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.client_ca_cert_path = CERTDIR "cert-ca.pem",
+	.client_expect_identity = "/O=Bar Example Organization"
+		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
+	.client_domain_mask = (char *[]) { "*", NULL },
+};
+
+static const struct tls_conn_test tls_conn_test_domain_match5 = {
 	.server_cert_path = CERTDIR "cert-server.pem",
 	.server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
 	.server_ca_cert_path = CERTDIR "cert-ca.pem",
@@ -661,12 +691,10 @@ static const struct tls_conn_test tls_conn_test_domain_match3 = {
 	.client_ca_cert_path = CERTDIR "cert-ca.pem",
 	.client_expect_identity = "/O=Foo Example Organization"
 		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
-	.client_domain_mask = (char *[]) {
-		"Bar Example Organization", "Foo Example Organization", NULL
-	},
+	.client_domain_mask = (char *[]) { "foo.int.com", NULL },
 };
 
-static const struct tls_conn_test tls_conn_test_domain_match4 = {
+static const struct tls_conn_test tls_conn_test_domain_match6 = {
 	.server_cert_path = CERTDIR "cert-server.pem",
 	.server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
 	.server_ca_cert_path = CERTDIR "cert-ca.pem",
@@ -677,10 +705,10 @@ static const struct tls_conn_test tls_conn_test_domain_match4 = {
 	.client_ca_cert_path = CERTDIR "cert-ca.pem",
 	.client_expect_identity = "/O=Foo Example Organization"
 		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
-	.client_domain_mask = (char *[]) { "*", NULL },
+	.client_domain_mask = (char *[]) { "*.*", NULL },
 };
 
-static const struct tls_conn_test tls_conn_test_domain_mismatch1 = {
+static const struct tls_conn_test tls_conn_test_domain_match7 = {
 	.server_cert_path = CERTDIR "cert-server.pem",
 	.server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
 	.server_ca_cert_path = CERTDIR "cert-ca.pem",
@@ -691,64 +719,96 @@ static const struct tls_conn_test tls_conn_test_domain_mismatch1 = {
 	.client_ca_cert_path = CERTDIR "cert-ca.pem",
 	.client_expect_identity = "/O=Foo Example Organization"
 		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_domain_mask = (char *[]) { "*.*.*", NULL },
+};
+
+static const struct tls_conn_test tls_conn_test_domain_mismatch1 = {
+	.server_cert_path = CERTDIR "cert-client.pem",
+	.server_key_path = CERTDIR "cert-client-key-pkcs8.pem",
+	.server_ca_cert_path = CERTDIR "cert-ca.pem",
+	.server_expect_identity = "/O=Foo Example Organization"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_cert_path = CERTDIR "cert-server.pem",
+	.client_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.client_ca_cert_path = CERTDIR "cert-ca.pem",
+	.client_expect_identity = "/O=Bar Example Organization"
+		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
 	.client_domain_mask = (char *[]) { "", NULL },
 	.expect_alert = true,
 	.alert_desc = TLS_ALERT_BAD_CERT,
 };
 
 static const struct tls_conn_test tls_conn_test_domain_mismatch2 = {
-	.server_cert_path = CERTDIR "cert-server.pem",
-	.server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.server_cert_path = CERTDIR "cert-client.pem",
+	.server_key_path = CERTDIR "cert-client-key-pkcs8.pem",
 	.server_ca_cert_path = CERTDIR "cert-ca.pem",
-	.server_expect_identity = "/O=Bar Example Organization"
-		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
-	.client_cert_path = CERTDIR "cert-client.pem",
-	.client_key_path = CERTDIR "cert-client-key-pkcs8.pem",
-	.client_ca_cert_path = CERTDIR "cert-ca.pem",
-	.client_expect_identity = "/O=Foo Example Organization"
+	.server_expect_identity = "/O=Foo Example Organization"
 		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
-	.client_domain_mask = (char *[]) { "Bar Example Organization", NULL },
+	.client_cert_path = CERTDIR "cert-server.pem",
+	.client_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.client_ca_cert_path = CERTDIR "cert-ca.pem",
+	.client_expect_identity = "/O=Bar Example Organization"
+		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
+	.client_domain_mask = (char *[]) { "Foo Example Organization", NULL },
 	.expect_alert = true,
 	.alert_desc = TLS_ALERT_BAD_CERT,
 };
 
 static const struct tls_conn_test tls_conn_test_domain_mismatch3 = {
-	.server_cert_path = CERTDIR "cert-server.pem",
-	.server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.server_cert_path = CERTDIR "cert-client.pem",
+	.server_key_path = CERTDIR "cert-client-key-pkcs8.pem",
 	.server_ca_cert_path = CERTDIR "cert-ca.pem",
-	.server_expect_identity = "/O=Bar Example Organization"
-		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
-	.client_cert_path = CERTDIR "cert-client.pem",
-	.client_key_path = CERTDIR "cert-client-key-pkcs8.pem",
-	.client_ca_cert_path = CERTDIR "cert-ca.pem",
-	.client_expect_identity = "/O=Foo Example Organization"
+	.server_expect_identity = "/O=Foo Example Organization"
 		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_cert_path = CERTDIR "cert-server.pem",
+	.client_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.client_ca_cert_path = CERTDIR "cert-ca.pem",
+	.client_expect_identity = "/O=Bar Example Organization"
+		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
 	.client_domain_mask = (char *[]) {
-		"Foo Example Organization.com", NULL
+		"Bar Example Organization.com", NULL
 	},
 	.expect_alert = true,
 	.alert_desc = TLS_ALERT_BAD_CERT,
 };
 
 static const struct tls_conn_test tls_conn_test_domain_mismatch4 = {
-	.server_cert_path = CERTDIR "cert-server.pem",
-	.server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.server_cert_path = CERTDIR "cert-client.pem",
+	.server_key_path = CERTDIR "cert-client-key-pkcs8.pem",
 	.server_ca_cert_path = CERTDIR "cert-ca.pem",
-	.server_expect_identity = "/O=Bar Example Organization"
-		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
-	.client_cert_path = CERTDIR "cert-client.pem",
-	.client_key_path = CERTDIR "cert-client-key-pkcs8.pem",
-	.client_ca_cert_path = CERTDIR "cert-ca.pem",
-	.client_expect_identity = "/O=Foo Example Organization"
+	.server_expect_identity = "/O=Foo Example Organization"
 		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_cert_path = CERTDIR "cert-server.pem",
+	.client_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.client_ca_cert_path = CERTDIR "cert-ca.pem",
+	.client_expect_identity = "/O=Bar Example Organization"
+		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
 	.client_domain_mask = (char *[]) {
-		"Foo Example Organization.*", NULL
+		"Bar Example Organization.*", NULL
 	},
 	.expect_alert = true,
 	.alert_desc = TLS_ALERT_BAD_CERT,
 };
 
 static const struct tls_conn_test tls_conn_test_domain_mismatch5 = {
+	.server_cert_path = CERTDIR "cert-client.pem",
+	.server_key_path = CERTDIR "cert-client-key-pkcs8.pem",
+	.server_ca_cert_path = CERTDIR "cert-ca.pem",
+	.server_expect_identity = "/O=Foo Example Organization"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_cert_path = CERTDIR "cert-server.pem",
+	.client_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+	.client_ca_cert_path = CERTDIR "cert-ca.pem",
+	.client_expect_identity = "/O=Bar Example Organization"
+		"/CN=Bar Example Organization/emailAddress=bar(a)mail.example",
+	.client_domain_mask = (char *[]) {
+		"*.Bar Example Organization", NULL
+	},
+	.expect_alert = true,
+	.alert_desc = TLS_ALERT_BAD_CERT,
+};
+
+static const struct tls_conn_test tls_conn_test_domain_mismatch6 = {
 	.server_cert_path = CERTDIR "cert-server.pem",
 	.server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
 	.server_ca_cert_path = CERTDIR "cert-ca.pem",
@@ -760,7 +820,7 @@ static const struct tls_conn_test tls_conn_test_domain_mismatch5 = {
 	.client_expect_identity = "/O=Foo Example Organization"
 		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
 	.client_domain_mask = (char *[]) {
-		"*.Foo Example Organization", NULL
+		"foo.*", NULL
 	},
 	.expect_alert = true,
 	.alert_desc = TLS_ALERT_BAD_CERT,
@@ -878,6 +938,12 @@ int main(int argc, char *argv[])
 			&tls_conn_test_domain_match3);
 	l_test_add("TLS connection domain match 4", test_tls_test,
 			&tls_conn_test_domain_match4);
+	l_test_add("TLS connection domain match 5", test_tls_test,
+			&tls_conn_test_domain_match5);
+	l_test_add("TLS connection domain match 6", test_tls_test,
+			&tls_conn_test_domain_match6);
+	l_test_add("TLS connection domain match 7", test_tls_test,
+			&tls_conn_test_domain_match7);
 	l_test_add("TLS connection domain mismatch 1", test_tls_test,
 			&tls_conn_test_domain_mismatch1);
 	l_test_add("TLS connection domain mismatch 2", test_tls_test,
@@ -888,6 +954,8 @@ int main(int argc, char *argv[])
 			&tls_conn_test_domain_mismatch4);
 	l_test_add("TLS connection domain mismatch 5", test_tls_test,
 			&tls_conn_test_domain_mismatch5);
+	l_test_add("TLS connection domain mismatch 6", test_tls_test,
+			&tls_conn_test_domain_mismatch6);
 
 	for (i = 0; tls_cipher_suite_pref[i]; i++) {
 		struct tls_cipher_suite *suite = tls_cipher_suite_pref[i];
-- 
2.20.1