[PATCH 1/4] tls: Escape characters in the peer_identity string

[PATCH 1/4] tls: Escape characters in the peer_identity string

[Andrew Zaborowski]

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

Escape the /, = and \ characters in the strings we take from the peer
certificate's Subject DN to build the peer_identity string passed to the
ell client's ready callback to enable it to be parsed unamiguously.
---
 ell/tls.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/ell/tls.c b/ell/tls.c
index 7405098..37f5b3f 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -2021,6 +2021,25 @@ static const struct dn_element_info dn_elements[] = {
 	{}
 };
 
+static void tls_str_escape_append(struct l_string *out, char *str, size_t len)
+{
+	while (len--) {
+		switch (*str) {
+		case '\\':
+		case '/':
+		case '=':
+			l_string_append_c(out, '\\');
+			l_string_append_c(out, *str);
+			break;
+		default:
+			l_string_append_c(out, *str);
+			break;
+		}
+
+		str++;
+	}
+}
+
 static char *tls_get_peer_identity_str(struct l_cert *cert)
 {
 	const uint8_t *dn, *end;
@@ -2072,7 +2091,7 @@ static char *tls_get_peer_identity_str(struct l_cert *cert)
 		l_string_append_c(id_str, '/');
 		l_string_append(id_str, info->str);
 		l_string_append_c(id_str, '=');
-		l_string_append_fixed(id_str, (char *) name, name_len);
+		tls_str_escape_append(id_str, (char *) name, name_len);
 	}
 
 	return l_string_unwrap(id_str);
-- 
2.20.1

[PATCH 2/4] tls: Disconnect if ls_get_peer_identity_str fails

[Andrew Zaborowski]

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

Make sure the ell client's ready callback can safely assume that
whenever a CA certificate was set, the peer_identity string is
non-NULL if that callback is reached.
---
 ell/tls.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/ell/tls.c b/ell/tls.c
index 37f5b3f..b1fec9f 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -2107,8 +2107,11 @@ static void tls_finished(struct l_tls *tls)
 
 	if (tls->peer_authenticated) {
 		peer_identity = tls_get_peer_identity_str(tls->peer_cert);
-		if (!peer_identity)
-			TLS_DEBUG("tls_get_peer_identity_str failed");
+		if (!peer_identity) {
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+					"tls_get_peer_identity_str failed");
+			return;
+		}
 	}
 
 	/* Free up the resources used in the handshake */
-- 
2.20.1

[PATCH 3/4] tls: Implement l_tls_set_domain_mask

[Andrew Zaborowski]

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

Allow user to set a mask that the a peer certificate's subject CN
has to match.
---
 ell/ell.sym       |   1 +
 ell/tls-private.h |   1 +
 ell/tls.c         | 132 ++++++++++++++++++++++++++++++++++++++++++++++
 ell/tls.h         |  16 ++++++
 4 files changed, 150 insertions(+)

diff --git a/ell/ell.sym b/ell/ell.sym
index 9bf0934..fe0ff1f 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -441,6 +441,7 @@ global:
 	l_tls_set_cacert;
 	l_tls_set_auth_data;
 	l_tls_set_version_range;
+	l_tls_set_domain_mask;
 	l_tls_alert_to_str;
 	l_tls_set_debug;
 	/* uintset */
diff --git a/ell/tls-private.h b/ell/tls-private.h
index c1de257..bcd45fe 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -212,6 +212,7 @@ struct l_tls {
 	struct l_certchain *cert;
 	struct l_key *priv_key;
 	size_t priv_key_size;
+	char *subject_mask;
 
 	struct tls_cipher_suite **cipher_suite_pref_list;
 
diff --git a/ell/tls.c b/ell/tls.c
index b1fec9f..e4c9a1e 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -629,6 +629,120 @@ static const struct tls_hash_algorithm *tls_set_prf_hmac(struct l_tls *tls)
 	return NULL;
 }
 
+static bool tls_domain_match_mask(const char *name, size_t name_len,
+					const char *mask, size_t mask_len)
+{
+	bool at_start = true;
+
+	while (1) {
+		const char *name_seg_end = memchr(name, '.', name_len);
+		const char *mask_seg_end = memchr(mask, '.', mask_len);
+		size_t name_seg_len = name_seg_end ?
+			(size_t) (name_seg_end - name) : name_len;
+		size_t mask_seg_len = mask_seg_end ?
+			(size_t) (mask_seg_end - mask) : mask_len;
+
+		if (mask_seg_len == 1 && mask[0] == '*') {
+			/*
+			 * A * at the beginning of the mask matches any
+			 * number of labels.
+			 */
+			if (at_start && name_seg_end &&
+					tls_domain_match_mask(name_seg_end + 1,
+						name_len - name_seg_len - 1,
+						mask, mask_len))
+				return true;
+
+			goto ok_next;
+		}
+
+		if (name_seg_len != mask_seg_len ||
+				memcmp(name, mask, name_seg_len))
+			return false;
+
+ok_next:
+		/* If either string ends here both must end here */
+		if (!name_seg_end || !mask_seg_end)
+			return !name_seg_end && !mask_seg_end;
+
+		at_start = false;
+		name = name_seg_end + 1;
+		name_len -= name_seg_len - 1;
+		mask = mask_seg_end + 1;
+		mask_len -= mask_seg_len - 1;
+	}
+}
+
+static const struct asn1_oid dn_common_name_oid =
+	{ 3, { 0x55, 0x04, 0x03 } };
+
+static bool tls_cert_subject_match_mask(struct l_cert *cert, const char *mask)
+{
+	const uint8_t *dn, *end;
+	size_t dn_size;
+	const char *cn = NULL;
+	size_t cn_len;
+
+	/*
+	 * Retrieve the Common Name from the Subject DN and check if it
+	 * matches.  TODO: possibly also look at SubjectAltName.
+	 */
+
+	dn = l_cert_get_dn(cert, &dn_size);
+	if (!dn)
+		return false;
+
+	end = dn + dn_size;
+	while (dn < end) {
+		const uint8_t *set, *seq, *oid, *name;
+		uint8_t tag;
+		size_t len, oid_len, name_len;
+
+		set = asn1_der_find_elem(dn, end - dn, 0, &tag, &len);
+		if (!set || tag != ASN1_ID_SET)
+			return false;
+
+		dn = set + len;
+
+		seq = asn1_der_find_elem(set, len, 0, &tag, &len);
+		if (!seq || tag != ASN1_ID_SEQUENCE)
+			return false;
+
+		oid = asn1_der_find_elem(seq, len, 0, &tag, &oid_len);
+		if (!oid || tag != ASN1_ID_OID)
+			return false;
+
+		name = asn1_der_find_elem(seq, len, 1, &tag, &name_len);
+		if (!name || (tag != ASN1_ID_PRINTABLESTRING &&
+					tag != ASN1_ID_UTF8STRING &&
+					tag != ASN1_ID_IA5STRING))
+			continue;
+
+		if (asn1_oid_eq(&dn_common_name_oid, oid_len, oid)) {
+			cn = (const char *) name;
+			cn_len = name_len;
+			break;
+		}
+	}
+
+	if (!cn)
+		return false;
+
+	while (1) {
+		const char *mask_end = strchr(mask, ';') ?: mask + strlen(mask);
+
+		if (tls_domain_match_mask(cn, cn_len, mask, mask_end - mask))
+			return true;
+
+		if (!mask_end[0])
+			break;
+
+		mask = mask_end + 1;
+	}
+
+	return false;
+}
+
 #define SWITCH_ENUM_TO_STR(val) \
 	case (val):		\
 		return L_STRINGIFY(val);
@@ -1793,6 +1907,15 @@ static void tls_handle_certificate(struct l_tls *tls,
 		goto done;
 	}
 
+	if (tls->subject_mask && !tls_cert_subject_match_mask(leaf,
+							tls->subject_mask)) {
+		TLS_DISCONNECT(TLS_ALERT_BAD_CERT, 0,
+				"Peer certificate's subject domain "
+				"doesn't match %s", tls->subject_mask);
+
+		goto done;
+	}
+
 	/* Save the end-entity certificate and free the chain */
 	der = l_cert_get_der_data(leaf, &der_len);
 	tls->peer_cert = l_cert_new_from_der(der, der_len);
@@ -2420,6 +2543,7 @@ LIB_EXPORT void l_tls_free(struct l_tls *tls)
 
 	l_tls_set_cacert(tls, NULL);
 	l_tls_set_auth_data(tls, NULL, NULL, NULL);
+	l_tls_set_domain_mask(tls, NULL);
 
 	tls_reset_handshake(tls);
 	tls_cleanup_handshake(tls);
@@ -2766,6 +2890,14 @@ LIB_EXPORT void l_tls_set_version_range(struct l_tls *tls,
 		max_version : TLS_MAX_VERSION;
 }
 
+LIB_EXPORT void l_tls_set_domain_mask(struct l_tls *tls,
+					const char *mask)
+{
+	l_free(tls->subject_mask);
+
+	tls->subject_mask = l_strdup(mask);
+}
+
 LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
 {
 	switch (desc) {
diff --git a/ell/tls.h b/ell/tls.h
index b0db34f..d860459 100644
--- a/ell/tls.h
+++ b/ell/tls.h
@@ -117,6 +117,22 @@ void l_tls_set_version_range(struct l_tls *tls,
 				enum l_tls_version min_version,
 				enum l_tls_version max_version);
 
+/*
+ * Set a mask for domain names contained in the peer certificate
+ * (eg. the subject Common Name) to be matched against.  If none of the
+ * domains match the mask, authentication will fail.
+ *
+ * Multiple masks can be joined using semicolons to form alternatives.
+ * The masks are each split into segments at the dot characters and each
+ * segment must match the corresponding label of the domain name --
+ * a domain name is a sequence of labels joined by dots.  An asterisk
+ * segment in the mask matches any label.  An asterisk segment at the
+ * beginning of the mask matches any numer of labels from the beginning
+ * of the domain string.
+ */
+void l_tls_set_domain_mask(struct l_tls *tls,
+				const char *mask);
+
 const char *l_tls_alert_to_str(enum l_tls_alert_desc desc);
 
 enum l_checksum_type;
-- 
2.20.1

[PATCH 4/4] unit: Add l_tls_set_domain_mask tests

[Andrew Zaborowski]

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

---
 unit/test-tls.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/unit/test-tls.c b/unit/test-tls.c
index d701f42..1014365 100644
--- a/unit/test-tls.c
+++ b/unit/test-tls.c
@@ -309,6 +309,7 @@ struct tls_conn_test {
 	const char *client_ca_cert_path;
 	const char *client_expect_identity;
 	const char **client_cipher_suites;
+	const char *client_domain_mask;
 	bool expect_alert;
 	bool expect_client_start_fail;
 	enum l_tls_alert_desc alert_desc;
@@ -566,6 +567,9 @@ static void test_tls_with_ver(const struct tls_conn_test *test,
 	assert(l_tls_set_cacert(s[0].tls, test->server_ca_cert_path));
 	assert(l_tls_set_cacert(s[1].tls, test->client_ca_cert_path));
 
+	if (test->client_domain_mask)
+		l_tls_set_domain_mask(s[1].tls, test->client_domain_mask);
+
 	assert(l_tls_start(s[0].tls));
 	assert(!!l_tls_start(s[1].tls) == !test->expect_client_start_fail);
 
@@ -616,6 +620,144 @@ static void test_tls_version_mismatch_test(const void *data)
 				L_TLS_V10, L_TLS_V11);
 }
 
+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_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"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_domain_mask = "Foo Example Organization",
+};
+
+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_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"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_domain_mask = "Foo Example Organization;"
+		"Bar Example Organization",
+};
+
+static const struct tls_conn_test tls_conn_test_domain_match3 = {
+	.server_cert_path = CERTDIR "cert-server.pem",
+	.server_key_path = CERTDIR "cert-server-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"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_domain_mask = "Bar Example Organization;"
+		"Foo Example Organization",
+};
+
+static const struct tls_conn_test tls_conn_test_domain_match4 = {
+	.server_cert_path = CERTDIR "cert-server.pem",
+	.server_key_path = CERTDIR "cert-server-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"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_domain_mask = "*",
+};
+
+static const struct tls_conn_test tls_conn_test_domain_mismatch1 = {
+	.server_cert_path = CERTDIR "cert-server.pem",
+	.server_key_path = CERTDIR "cert-server-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"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_domain_mask = "",
+	.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_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"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_domain_mask = "Bar Example Organization",
+	.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_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"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_domain_mask = "Foo Example Organization.com",
+	.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_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"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_domain_mask = "Foo Example Organization.*",
+	.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-server.pem",
+	.server_key_path = CERTDIR "cert-server-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"
+		"/CN=Foo Example Organization/emailAddress=foo(a)mail.example",
+	.client_domain_mask = "*.Foo Example Organization",
+	.expect_alert = true,
+	.alert_desc = TLS_ALERT_BAD_CERT,
+};
+
 static void test_tls_suite_test(const void *data)
 {
 	const char *suite_name = data;
@@ -720,6 +862,25 @@ int main(int argc, char *argv[])
 	l_test_add("TLS connection version mismatch",
 			test_tls_version_mismatch_test, NULL);
 
+	l_test_add("TLS connection domain match 1", test_tls_test,
+			&tls_conn_test_domain_match1);
+	l_test_add("TLS connection domain match 2", test_tls_test,
+			&tls_conn_test_domain_match2);
+	l_test_add("TLS connection domain match 3", test_tls_test,
+			&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 mismatch 1", test_tls_test,
+			&tls_conn_test_domain_mismatch1);
+	l_test_add("TLS connection domain mismatch 2", test_tls_test,
+			&tls_conn_test_domain_mismatch2);
+	l_test_add("TLS connection domain mismatch 3", test_tls_test,
+			&tls_conn_test_domain_mismatch3);
+	l_test_add("TLS connection domain mismatch 4", test_tls_test,
+			&tls_conn_test_domain_mismatch4);
+	l_test_add("TLS connection domain mismatch 5", test_tls_test,
+			&tls_conn_test_domain_mismatch5);
+
 	for (i = 0; tls_cipher_suite_pref[i]; i++) {
 		struct tls_cipher_suite *suite = tls_cipher_suite_pref[i];
 		struct tls_bulk_encryption_algorithm *alg = suite->encryption;
-- 
2.20.1

Re: [PATCH 1/4] tls: Escape characters in the peer_identity string

[Denis Kenzior]

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

Hi Andrew,

On 8/16/19 7:28 PM, Andrew Zaborowski wrote:
> Escape the /, = and \ characters in the strings we take from the peer
> certificate's Subject DN to build the peer_identity string passed to the
> ell client's ready callback to enable it to be parsed unamiguously.
> ---
>   ell/tls.c | 21 ++++++++++++++++++++-
>   1 file changed, 20 insertions(+), 1 deletion(-)
> 

Patch 1 & 2 applied, thanks.

Regards,
-Denis

Re: [PATCH 3/4] tls: Implement l_tls_set_domain_mask

[Denis Kenzior]

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

Hi Andrew,

On 8/16/19 7:28 PM, Andrew Zaborowski wrote:
> Allow user to set a mask that the a peer certificate's subject CN
> has to match.
> ---
>   ell/ell.sym       |   1 +
>   ell/tls-private.h |   1 +
>   ell/tls.c         | 132 ++++++++++++++++++++++++++++++++++++++++++++++
>   ell/tls.h         |  16 ++++++
>   4 files changed, 150 insertions(+)
> 

<snip>

> diff --git a/ell/tls.c b/ell/tls.c
> index b1fec9f..e4c9a1e 100644
> --- a/ell/tls.c
> +++ b/ell/tls.c
> @@ -629,6 +629,120 @@ static const struct tls_hash_algorithm *tls_set_prf_hmac(struct l_tls *tls)
>   	return NULL;
>   }
>   
> +static bool tls_domain_match_mask(const char *name, size_t name_len,
> +					const char *mask, size_t mask_len)
> +{
> +	bool at_start = true;
> +
> +	while (1) {
> +		const char *name_seg_end = memchr(name, '.', name_len);
> +		const char *mask_seg_end = memchr(mask, '.', mask_len);
> +		size_t name_seg_len = name_seg_end ?
> +			(size_t) (name_seg_end - name) : name_len;
> +		size_t mask_seg_len = mask_seg_end ?
> +			(size_t) (mask_seg_end - mask) : mask_len;
> +
> +		if (mask_seg_len == 1 && mask[0] == '*') {
> +			/*
> +			 * A * at the beginning of the mask matches any
> +			 * number of labels.
> +			 */
> +			if (at_start && name_seg_end &&
> +					tls_domain_match_mask(name_seg_end + 1,
> +						name_len - name_seg_len - 1,
> +						mask, mask_len))
> +				return true;
> +
> +			goto ok_next;
> +		}
> +
> +		if (name_seg_len != mask_seg_len ||
> +				memcmp(name, mask, name_seg_len))
> +			return false;
> +
> +ok_next:
> +		/* If either string ends here both must end here */
> +		if (!name_seg_end || !mask_seg_end)
> +			return !name_seg_end && !mask_seg_end;
> +
> +		at_start = false;
> +		name = name_seg_end + 1;
> +		name_len -= name_seg_len - 1;
> +		mask = mask_seg_end + 1;
> +		mask_len -= mask_seg_len - 1;
> +	}
> +}
> +
> +static const struct asn1_oid dn_common_name_oid =
> +	{ 3, { 0x55, 0x04, 0x03 } };
> +
> +static bool tls_cert_subject_match_mask(struct l_cert *cert, const char *mask)
> +{
> +	const uint8_t *dn, *end;
> +	size_t dn_size;
> +	const char *cn = NULL;
> +	size_t cn_len;
> +
> +	/*
> +	 * Retrieve the Common Name from the Subject DN and check if it
> +	 * matches.  TODO: possibly also look at SubjectAltName.

We definitely need to look in the SubjectAltName dNSName extensions for 
Hotspot.  So can we add that as well?

> +	 */
> +
> +	dn = l_cert_get_dn(cert, &dn_size);
> +	if (!dn)
> +		return false;
> +
> +	end = dn + dn_size;
> +	while (dn < end) {
> +		const uint8_t *set, *seq, *oid, *name;
> +		uint8_t tag;
> +		size_t len, oid_len, name_len;
> +
> +		set = asn1_der_find_elem(dn, end - dn, 0, &tag, &len);
> +		if (!set || tag != ASN1_ID_SET)
> +			return false;
> +
> +		dn = set + len;

Do you really mean to do this in the loop?

> +
> +		seq = asn1_der_find_elem(set, len, 0, &tag, &len);
> +		if (!seq || tag != ASN1_ID_SEQUENCE)
> +			return false;
> +
> +		oid = asn1_der_find_elem(seq, len, 0, &tag, &oid_len);
> +		if (!oid || tag != ASN1_ID_OID)
> +			return false;
> +
> +		name = asn1_der_find_elem(seq, len, 1, &tag, &name_len);
> +		if (!name || (tag != ASN1_ID_PRINTABLESTRING &&
> +					tag != ASN1_ID_UTF8STRING &&
> +					tag != ASN1_ID_IA5STRING))
> +			continue;
> +
> +		if (asn1_oid_eq(&dn_common_name_oid, oid_len, oid)) {
> +			cn = (const char *) name;
> +			cn_len = name_len;
> +			break;
> +		}
> +	}
> +
> +	if (!cn)
> +		return false;
> +
> +	while (1) {
> +		const char *mask_end = strchr(mask, ';') ?: mask + strlen(mask);
> +
> +		if (tls_domain_match_mask(cn, cn_len, mask, mask_end - mask))
> +			return true;
> +
> +		if (!mask_end[0])
> +			break;
> +
> +		mask = mask_end + 1;
> +	}
> +
> +	return false;
> +}
> +
>   #define SWITCH_ENUM_TO_STR(val) \
>   	case (val):		\
>   		return L_STRINGIFY(val);
> @@ -1793,6 +1907,15 @@ static void tls_handle_certificate(struct l_tls *tls,
>   		goto done;
>   	}
>   
> +	if (tls->subject_mask && !tls_cert_subject_match_mask(leaf,
> +							tls->subject_mask)) {
> +		TLS_DISCONNECT(TLS_ALERT_BAD_CERT, 0,
> +				"Peer certificate's subject domain "
> +				"doesn't match %s", tls->subject_mask);
> +
> +		goto done;
> +	}
> +
>   	/* Save the end-entity certificate and free the chain */
>   	der = l_cert_get_der_data(leaf, &der_len);
>   	tls->peer_cert = l_cert_new_from_der(der, der_len);
> @@ -2420,6 +2543,7 @@ LIB_EXPORT void l_tls_free(struct l_tls *tls)
>   
>   	l_tls_set_cacert(tls, NULL);
>   	l_tls_set_auth_data(tls, NULL, NULL, NULL);
> +	l_tls_set_domain_mask(tls, NULL);
>   
>   	tls_reset_handshake(tls);
>   	tls_cleanup_handshake(tls);
> @@ -2766,6 +2890,14 @@ LIB_EXPORT void l_tls_set_version_range(struct l_tls *tls,
>   		max_version : TLS_MAX_VERSION;
>   }
>   
> +LIB_EXPORT void l_tls_set_domain_mask(struct l_tls *tls,
> +					const char *mask)

Actually I wonder if it would make your life easier if the mask was a 
char **?  E.g. a strv array?  Note that l_settings already has a method 
for getting lists of strings.  So this is what we would use in iwd most 
likely anyway.  e.g. l_settings_get_string_list()

> +{
> +	l_free(tls->subject_mask);
> +
> +	tls->subject_mask = l_strdup(mask);
> +}
> +
>   LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
>   {
>   	switch (desc) {
> diff --git a/ell/tls.h b/ell/tls.h
> index b0db34f..d860459 100644
> --- a/ell/tls.h
> +++ b/ell/tls.h
> @@ -117,6 +117,22 @@ void l_tls_set_version_range(struct l_tls *tls,
>   				enum l_tls_version min_version,
>   				enum l_tls_version max_version);
>   
> +/*
> + * Set a mask for domain names contained in the peer certificate
> + * (eg. the subject Common Name) to be matched against.  If none of the
> + * domains match the mask, authentication will fail.
> + *
> + * Multiple masks can be joined using semicolons to form alternatives.
> + * The masks are each split into segments at the dot characters and each
> + * segment must match the corresponding label of the domain name --
> + * a domain name is a sequence of labels joined by dots.  An asterisk
> + * segment in the mask matches any label.  An asterisk segment at the
> + * beginning of the mask matches any numer of labels from the beginning
> + * of the domain string.
> + */

Can we move this documentation to tls.c instead and put the normal doc 
formatting around it?

> +void l_tls_set_domain_mask(struct l_tls *tls,
> +				const char *mask);
> +
>   const char *l_tls_alert_to_str(enum l_tls_alert_desc desc);
>   
>   enum l_checksum_type;
> 

Regards,
-Denis

Re: [PATCH 3/4] tls: Implement l_tls_set_domain_mask

[Andrew Zaborowski]

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

Hi Denis,

On Tue, 20 Aug 2019 at 21:35, Denis Kenzior <denkenz@gmail.com> wrote:
> On 8/16/19 7:28 PM, Andrew Zaborowski wrote:
> > +     /*
> > +      * Retrieve the Common Name from the Subject DN and check if it
> > +      * matches.  TODO: possibly also look at SubjectAltName.
>
> We definitely need to look in the SubjectAltName dNSName extensions for
> Hotspot.  So can we add that as well?

Yep, but that should probably be a separate patch.

>
> > +      */
> > +
> > +     dn = l_cert_get_dn(cert, &dn_size);
> > +     if (!dn)
> > +             return false;
> > +
> > +     end = dn + dn_size;
> > +     while (dn < end) {
> > +             const uint8_t *set, *seq, *oid, *name;
> > +             uint8_t tag;
> > +             size_t len, oid_len, name_len;
> > +
> > +             set = asn1_der_find_elem(dn, end - dn, 0, &tag, &len);
> > +             if (!set || tag != ASN1_ID_SET)
> > +                     return false;
> > +
> > +             dn = set + len;
>
> Do you really mean to do this in the loop?

Yep, the DN is sort of a dictionary where we look for the CN entry.
The 'dn' variable doesn't point at the actual start of the DN after
the first iteration.

>
> > +
> > +             seq = asn1_der_find_elem(set, len, 0, &tag, &len);
> > +             if (!seq || tag != ASN1_ID_SEQUENCE)
> > +                     return false;
> > +
> > +             oid = asn1_der_find_elem(seq, len, 0, &tag, &oid_len);
> > +             if (!oid || tag != ASN1_ID_OID)
> > +                     return false;
> > +
> > +             name = asn1_der_find_elem(seq, len, 1, &tag, &name_len);
> > +             if (!name || (tag != ASN1_ID_PRINTABLESTRING &&
> > +                                     tag != ASN1_ID_UTF8STRING &&
> > +                                     tag != ASN1_ID_IA5STRING))
> > +                     continue;
> > +
> > +             if (asn1_oid_eq(&dn_common_name_oid, oid_len, oid)) {
> > +                     cn = (const char *) name;
> > +                     cn_len = name_len;
> > +                     break;
> > +             }
> > +     }
> > +
> > +     if (!cn)
> > +             return false;
> > +
> > +     while (1) {
> > +             const char *mask_end = strchr(mask, ';') ?: mask + strlen(mask);
> > +
> > +             if (tls_domain_match_mask(cn, cn_len, mask, mask_end - mask))
> > +                     return true;
> > +
> > +             if (!mask_end[0])
> > +                     break;
> > +
> > +             mask = mask_end + 1;
> > +     }
> > +
> > +     return false;
> > +}
> > +
> >   #define SWITCH_ENUM_TO_STR(val) \
> >       case (val):             \
> >               return L_STRINGIFY(val);
> > @@ -1793,6 +1907,15 @@ static void tls_handle_certificate(struct l_tls *tls,
> >               goto done;
> >       }
> >
> > +     if (tls->subject_mask && !tls_cert_subject_match_mask(leaf,
> > +                                                     tls->subject_mask)) {
> > +             TLS_DISCONNECT(TLS_ALERT_BAD_CERT, 0,
> > +                             "Peer certificate's subject domain "
> > +                             "doesn't match %s", tls->subject_mask);
> > +
> > +             goto done;
> > +     }
> > +
> >       /* Save the end-entity certificate and free the chain */
> >       der = l_cert_get_der_data(leaf, &der_len);
> >       tls->peer_cert = l_cert_new_from_der(der, der_len);
> > @@ -2420,6 +2543,7 @@ LIB_EXPORT void l_tls_free(struct l_tls *tls)
> >
> >       l_tls_set_cacert(tls, NULL);
> >       l_tls_set_auth_data(tls, NULL, NULL, NULL);
> > +     l_tls_set_domain_mask(tls, NULL);
> >
> >       tls_reset_handshake(tls);
> >       tls_cleanup_handshake(tls);
> > @@ -2766,6 +2890,14 @@ LIB_EXPORT void l_tls_set_version_range(struct l_tls *tls,
> >               max_version : TLS_MAX_VERSION;
> >   }
> >
> > +LIB_EXPORT void l_tls_set_domain_mask(struct l_tls *tls,
> > +                                     const char *mask)
>
> Actually I wonder if it would make your life easier if the mask was a
> char **?  E.g. a strv array?  Note that l_settings already has a method
> for getting lists of strings.  So this is what we would use in iwd most
> likely anyway.  e.g. l_settings_get_string_list()

Yes, let me change it to an strv.

>
> > +{
> > +     l_free(tls->subject_mask);
> > +
> > +     tls->subject_mask = l_strdup(mask);
> > +}
> > +
> >   LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
> >   {
> >       switch (desc) {
> > diff --git a/ell/tls.h b/ell/tls.h
> > index b0db34f..d860459 100644
> > --- a/ell/tls.h
> > +++ b/ell/tls.h
> > @@ -117,6 +117,22 @@ void l_tls_set_version_range(struct l_tls *tls,
> >                               enum l_tls_version min_version,
> >                               enum l_tls_version max_version);
> >
> > +/*
> > + * Set a mask for domain names contained in the peer certificate
> > + * (eg. the subject Common Name) to be matched against.  If none of the
> > + * domains match the mask, authentication will fail.
> > + *
> > + * Multiple masks can be joined using semicolons to form alternatives.
> > + * The masks are each split into segments at the dot characters and each
> > + * segment must match the corresponding label of the domain name --
> > + * a domain name is a sequence of labels joined by dots.  An asterisk
> > + * segment in the mask matches any label.  An asterisk segment at the
> > + * beginning of the mask matches any numer of labels from the beginning
> > + * of the domain string.
> > + */
>
> Can we move this documentation to tls.c instead and put the normal doc
> formatting around it?

Ok.

Best regards

Re: [PATCH 3/4] tls: Implement l_tls_set_domain_mask

[Denis Kenzior]

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

Hi Andrew,

On 8/21/19 12:52 AM, Andrew Zaborowski wrote:
> Hi Denis,
> 
> On Tue, 20 Aug 2019 at 21:35, Denis Kenzior <denkenz@gmail.com> wrote:
>> On 8/16/19 7:28 PM, Andrew Zaborowski wrote:
>>> +     /*
>>> +      * Retrieve the Common Name from the Subject DN and check if it
>>> +      * matches.  TODO: possibly also look at SubjectAltName.
>>
>> We definitely need to look in the SubjectAltName dNSName extensions for
>> Hotspot.  So can we add that as well?
> 
> Yep, but that should probably be a separate patch.
> 

Sure, that'd be fine.

Regards,
-Denis