[PATCH 1/4] tls: Implement the Signature Algorithms extension

[PATCH 1/4] tls: Implement the Signature Algorithms extension

[Andrew Zaborowski]

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

---
 ell/tls-extensions.c | 279 +++++++++++++++++++++++++++++++++++++++++++
 ell/tls-private.h    |   4 +
 ell/tls.c            |  10 +-
 3 files changed, 284 insertions(+), 9 deletions(-)

diff --git a/ell/tls-extensions.c b/ell/tls-extensions.c
index d179e67..7b20e17 100644
--- a/ell/tls-extensions.c
+++ b/ell/tls-extensions.c
@@ -568,6 +568,278 @@ static ssize_t tls_ec_point_formats_server_write(struct l_tls *tls,
 	return 2;
 }
 
+/*
+ * This is used to append the list of signature algorithm and hash type
+ * combinations we support to the Signature Algorithms client hello
+ * extension (on the client) and the Certificate Request message (on the
+ * server).  In both cases we need to list the algorithms we support for
+ * two use cases: certificate chain verification and signing/verifying
+ * Server Key Exchange params (server->client) or Certificate Verify
+ * data (client->server).
+ *
+ * For the server side RFC 5462, Section 7.4.1.4.1 says:
+ * "If the client [...] is willing to use them for verifying
+ * messages sent by the server, i.e., server certificates and
+ * server key exchange [...] it MUST send the
+ * signature_algorithms extension, listing the algorithms it
+ * is willing to accept."
+ *
+ * As for the certificate chains we mostly rely on the kernel to do
+ * this so when we receive the list we do not currently verify the
+ * that the whole chain uses only algorithms from the list on either
+ * side (TODO). But we know that the chain verification in the kernel
+ * can use a superset of the hash algorithms l_checksum supports.
+ * For the Server Key Exchange and Certificate Verify signatures we
+ * use l_checksum but we need to map the TLS-specific hash IDs to
+ * enum l_checksum_type using the tls_handshake_hash_data list in
+ * signature->sign() and signature->verify(), so we use
+ * tls_handshake_hash_data as the definitive list of allowed hash
+ * algorithms.
+ *
+ * Our supported signature algorithms can work with any hash type so we
+ * basically have to send all possible combinations of the signature
+ * algorithm IDs from the supported cipher suites (except anonymous)
+ * with the hash algorithms we can use for signature verification,
+ * i.e. those in the tls_handshake_hash_data table.
+ */
+static ssize_t tls_write_signature_algorithms(struct l_tls *tls,
+						uint8_t *buf, size_t len)
+{
+	uint8_t *ptr = buf;
+	unsigned int i, j;
+	struct tls_cipher_suite **suite;
+	uint8_t sig_alg_ids[16];
+	uint8_t hash_ids[16];
+	unsigned int sig_alg_cnt = 0;
+	unsigned int hash_cnt = 0;
+
+	for (suite = tls->cipher_suite_pref_list; *suite; suite++) {
+		uint8_t id;
+
+		if (!(*suite)->signature)
+			continue;
+
+		id = (*suite)->signature->id;
+
+		if (memchr(sig_alg_ids, id, sig_alg_cnt))
+			continue;
+
+		if (!tls_cipher_suite_is_compatible(tls, *suite, NULL))
+			continue;
+
+		if (sig_alg_cnt >= sizeof(sig_alg_ids))
+			return -ENOMEM;
+
+		sig_alg_ids[sig_alg_cnt++] = id;
+	}
+
+	for (i = 0; i < __HANDSHAKE_HASH_COUNT; i++) {
+		const struct tls_hash_algorithm *hash =
+			&tls_handshake_hash_data[i];
+		bool supported;
+
+		/*
+		 * The hash types in the Signature Algorithms extension are
+		 * all supported hashes but the ones in the Certificate
+		 * Request (server->client) must be in the set for which we
+		 * maintain handshake message hashes because that is going
+		 * to be used in Certificate Verify.
+		 */
+		if (tls->server)
+			supported = !!tls->handshake_hash[i];
+		else
+			supported = l_checksum_is_supported(hash->l_id, false);
+
+		if (supported)
+			hash_ids[hash_cnt++] = hash->tls_id;
+	}
+
+	if (len < 2 + sig_alg_cnt * hash_cnt * 2)
+		return -ENOMEM;
+
+	l_put_be16(sig_alg_cnt * hash_cnt * 2, ptr);
+	ptr += 2;
+
+	for (i = 0; i < sig_alg_cnt; i++)
+		for (j = 0; j < hash_cnt; j++) {
+			*ptr++ = hash_ids[j];
+			*ptr++ = sig_alg_ids[i];
+		}
+
+	return ptr - buf;
+}
+
+static ssize_t tls_parse_signature_algorithms(struct l_tls *tls,
+						const uint8_t *buf, size_t len)
+{
+	const uint8_t *ptr = buf;
+	enum handshake_hash_type first_supported, hash;
+	const struct tls_hash_algorithm *preferred;
+	struct tls_cipher_suite **suite;
+	uint8_t sig_alg_ids[16];
+	unsigned int sig_alg_cnt = 0;
+
+	/*
+	 * This only makes sense as a variable-length field, assume
+	 * there's a typo in RFC5246 7.4.4 here.
+	 */
+	if (len < 4)
+		return -EINVAL;
+
+	if (l_get_be16(ptr) > len - 2)
+		return -EINVAL;
+
+	len = l_get_be16(ptr);
+	ptr += 2;
+
+	if (len & 1)
+		return -EINVAL;
+
+	for (suite = tls->cipher_suite_pref_list; *suite; suite++) {
+		uint8_t id;
+
+		if (!(*suite)->signature)
+			continue;
+
+		id = (*suite)->signature->id;
+
+		if (memchr(sig_alg_ids, id, sig_alg_cnt))
+			continue;
+
+		if (!tls_cipher_suite_is_compatible(tls, *suite, NULL))
+			continue;
+
+		if (sig_alg_cnt >= sizeof(sig_alg_ids))
+			return -ENOMEM;
+
+		sig_alg_ids[sig_alg_cnt++] = id;
+	}
+
+	/*
+	 * In 1.2 we force our preference for SHA256/SHA384 (depending on
+	 * cipher suite's PRF hmac) if it is supported by the peer because
+	 * that must be supported anyway for the PRF and the Finished hash
+	 * meaning that we only need to keep one hash instead of two.
+	 * If not available fall back to the first common hash algorithm.
+	 */
+	first_supported = -1;
+
+	if (tls->prf_hmac)
+		preferred = tls->prf_hmac;
+	else
+		preferred = &tls_handshake_hash_data[HANDSHAKE_HASH_SHA256];
+
+	while (len) {
+		uint8_t hash_id = *ptr++;
+		uint8_t sig_alg_id = *ptr++;
+		bool supported;
+
+		len -= 2;
+
+		/* Ignore hash types for signatures other than ours */
+		if (tls->pending.cipher_suite &&
+				(!tls->pending.cipher_suite->signature ||
+				 tls->pending.cipher_suite->signature->id !=
+				 sig_alg_id))
+			continue;
+		else if (!tls->pending.cipher_suite &&
+				!memchr(sig_alg_ids, sig_alg_id, sig_alg_cnt))
+			continue;
+
+		if (hash_id == preferred->tls_id) {
+			for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
+				if (&tls_handshake_hash_data[hash] == preferred)
+					break;
+			break;
+		}
+
+		if ((int) first_supported != -1)
+			continue;
+
+		for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
+			if (hash_id == tls_handshake_hash_data[hash].tls_id)
+				break;
+
+		if (hash == __HANDSHAKE_HASH_COUNT)
+			continue;
+
+		if (tls->server)
+			supported = l_checksum_is_supported(
+					tls_handshake_hash_data[hash].l_id,
+					false);
+		else
+			supported = !!tls->handshake_hash[hash];
+
+		if (supported)
+			first_supported = hash;
+	}
+
+	if (len)
+		tls->signature_hash = hash;
+	else if ((int) first_supported != -1)
+		tls->signature_hash = first_supported;
+	else
+		return -ENOTSUP;
+
+	return ptr + len - buf;
+}
+
+/* RFC 5462, Section 7.4.1.4.1 */
+static ssize_t tls_signature_algorithms_client_write(struct l_tls *tls,
+						uint8_t *buf, size_t len)
+{
+	/*
+	 * "Note: this extension is not meaningful for TLS versions
+	 * prior to 1.2.  Clients MUST NOT offer it if they are offering
+	 * prior versions."
+	 */
+	if (tls->max_version < L_TLS_V12)
+		return -ENOMSG;
+
+	return tls_write_signature_algorithms(tls, buf, len);
+}
+
+static bool tls_signature_algorithms_client_handle(struct l_tls *tls,
+						const uint8_t *buf, size_t len)
+{
+	ssize_t ret;
+
+	/*
+	 * "However, even if clients do offer it, the rules specified in
+	 * [TLSEXT] require servers to ignore extensions they do not
+	 * understand."
+	 */
+	if (tls->max_version < L_TLS_V12)
+		return true;
+
+	ret = tls_parse_signature_algorithms(tls, buf, len);
+
+	if (ret == -ENOTSUP)
+		TLS_DEBUG("No common signature algorithms");
+
+	/*
+	 * TODO: also check our certificate chain against the parsed
+	 * signature algorithms.
+	 */
+
+	return ret == (ssize_t) len;
+}
+
+static bool tls_signature_algorithms_client_absent(struct l_tls *tls)
+{
+	/*
+	 * "If the client does not send the signature_algorithms extension,
+	 * the server MUST do the following:
+	 *    - [...] behave as if client had sent the value {sha1,rsa}.
+	 *    - [...] behave as if client had sent the value {sha1,dsa}.
+	 *    - [...] behave as if client had sent the value {sha1,ecdsa}.
+	 */
+	if (tls->max_version >= L_TLS_V12)
+		tls->signature_hash = HANDSHAKE_HASH_SHA1;
+
+	return true;
+}
+
 const struct tls_hello_extension tls_extensions[] = {
 	{
 		"Supported Groups", "elliptic_curves", 10,
@@ -584,6 +856,13 @@ const struct tls_hello_extension tls_extensions[] = {
 		tls_ec_point_formats_server_write,
 		NULL, NULL,
 	},
+	{
+		"Signature Algorithms", "signature_algoritms", 13,
+		tls_signature_algorithms_client_write,
+		tls_signature_algorithms_client_handle,
+		tls_signature_algorithms_client_absent,
+		NULL, NULL, NULL,
+	},
 	{}
 };
 
diff --git a/ell/tls-private.h b/ell/tls-private.h
index 03ad07c..2d1424e 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -308,6 +308,10 @@ bool tls_handle_message(struct l_tls *tls, const uint8_t *message,
 
 void tls_tx_handshake(struct l_tls *tls, int type, uint8_t *buf, size_t length);
 
+bool tls_cipher_suite_is_compatible(struct l_tls *tls,
+					const struct tls_cipher_suite *suite,
+					const char **error);
+
 /* Optionally limit allowed cipher suites to a custom set */
 bool tls_set_cipher_suites(struct l_tls *tls, const char **suite_list);
 
diff --git a/ell/tls.c b/ell/tls.c
index 02e49f9..fd898f6 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -379,7 +379,7 @@ static void tls_reset_cipher_spec(struct l_tls *tls, bool txrx)
 	tls_change_cipher_spec(tls, txrx, NULL);
 }
 
-static bool tls_cipher_suite_is_compatible(struct l_tls *tls,
+bool tls_cipher_suite_is_compatible(struct l_tls *tls,
 					const struct tls_cipher_suite *suite,
 					const char **error)
 {
@@ -1486,13 +1486,6 @@ static void tls_handle_client_hello(struct l_tls *tls,
 	 * trying to connect somewhere else.  We might want to throw an error.
 	 */
 
-	/*
-	 * TODO: Obligatory in 1.2: check for signature_algorithms extension,
-	 * store the list of algorithms for later checking in
-	 * tls_send_certificate on both server and client sides.  If not
-	 * present assume only SHA1+RSA (7.4.1.4.1).
-	 */
-
 	/* Save client_version for Premaster Secret verification */
 	tls->client_version = l_get_be16(buf);
 
@@ -2407,7 +2400,6 @@ LIB_EXPORT struct l_tls *l_tls_new(bool server,
 	tls->disconnected = disconnect_handler;
 	tls->user_data = user_data;
 	tls->cipher_suite_pref_list = tls_cipher_suite_pref;
-	tls->signature_hash = HANDSHAKE_HASH_SHA256;
 	tls->min_version = TLS_MIN_VERSION;
 	tls->max_version = TLS_MAX_VERSION;
 
-- 
2.19.1

[PATCH 2/4] tls: Handle CertificateRequest using common code

[Andrew Zaborowski]

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

Use the new, more complete code added for the Signature Algorithms
extensions to parse and generate the signature algorithms data in the
Certificate Request messages.  Get rid of a few TODOs this way.

The signature algorithms data in both messages serves mostly the same
purpose.
---
 ell/tls-extensions.c |   8 +--
 ell/tls-private.h    |   5 ++
 ell/tls-suites.c     |   2 +-
 ell/tls.c            | 138 +++++++++++++------------------------------
 4 files changed, 52 insertions(+), 101 deletions(-)

diff --git a/ell/tls-extensions.c b/ell/tls-extensions.c
index 7b20e17..2e2d9da 100644
--- a/ell/tls-extensions.c
+++ b/ell/tls-extensions.c
@@ -602,8 +602,8 @@ static ssize_t tls_ec_point_formats_server_write(struct l_tls *tls,
  * with the hash algorithms we can use for signature verification,
  * i.e. those in the tls_handshake_hash_data table.
  */
-static ssize_t tls_write_signature_algorithms(struct l_tls *tls,
-						uint8_t *buf, size_t len)
+ssize_t tls_write_signature_algorithms(struct l_tls *tls,
+					uint8_t *buf, size_t len)
 {
 	uint8_t *ptr = buf;
 	unsigned int i, j;
@@ -669,8 +669,8 @@ static ssize_t tls_write_signature_algorithms(struct l_tls *tls,
 	return ptr - buf;
 }
 
-static ssize_t tls_parse_signature_algorithms(struct l_tls *tls,
-						const uint8_t *buf, size_t len)
+ssize_t tls_parse_signature_algorithms(struct l_tls *tls,
+					const uint8_t *buf, size_t len)
 {
 	const uint8_t *ptr = buf;
 	enum handshake_hash_type first_supported, hash;
diff --git a/ell/tls-private.h b/ell/tls-private.h
index 2d1424e..bf9e2ec 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -325,6 +325,11 @@ const struct tls_named_group *tls_find_ff_group(const uint8_t *prime,
 						const uint8_t *generator,
 						size_t generator_len);
 
+ssize_t tls_write_signature_algorithms(struct l_tls *tls,
+					uint8_t *buf, size_t len);
+ssize_t tls_parse_signature_algorithms(struct l_tls *tls,
+					const uint8_t *buf, size_t len);
+
 int tls_parse_certificate_list(const void *data, size_t len,
 				struct l_certchain **out_certchain);
 
diff --git a/ell/tls-suites.c b/ell/tls-suites.c
index c11c28d..18ac366 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -213,7 +213,7 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t in_len,
 }
 
 static struct tls_signature_algorithm tls_rsa_signature = {
-	.id = 1, /* RSA_sign */
+	.id = 1, /* SignatureAlgorithm.rsa */
 	.validate_cert_key_type = tls_rsa_validate_cert_key,
 	.sign = tls_rsa_sign,
 	.verify = tls_rsa_verify,
diff --git a/ell/tls.c b/ell/tls.c
index fd898f6..27f291a 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -951,7 +951,7 @@ static bool tls_send_certificate(struct l_tls *tls)
 	/*
 	 * TODO: check that the certificate is compatible with hash and
 	 * signature algorithms lists supplied to us in the Client Hello
-	 * extensions (if we're a server) or in the Certificate Request
+	 * extensions (if we're a 1.2+ server) or in the Certificate Request
 	 * (if we act as a 1.2+ client).
 	 *
 	 *  - for the hash and signature_algorithms list, check all
@@ -988,26 +988,20 @@ static bool tls_send_certificate(struct l_tls *tls)
 	return true;
 }
 
+/*
+ * Note: ClientCertificateType.rsa_sign value coincides with the
+ * SignatureAlgorithm.rsa value but other values in those enum are
+ * different so we don't mix them, can't extract them from
+ * tls->pending.cipher_suite->signature.
+ */
 static uint8_t tls_cert_type_pref[] = {
 	1, /* RSA_sign */
 };
 
-struct tls_signature_hash_algorithms {
-	uint8_t hash_id;
-	uint8_t signature_id;
-};
-
-static struct tls_signature_hash_algorithms tls_signature_hash_pref[] = {
-	{ 6, 1 }, /* SHA512 + RSA */
-	{ 5, 1 }, /* SHA384 + RSA */
-	{ 4, 1 }, /* SHA256 + RSA */
-	{ 2, 1 }, /* SHA1 + RSA */
-	{ 1, 1 }, /* MD5 + RSA */
-};
-
 static bool tls_send_certificate_request(struct l_tls *tls)
 {
-	uint8_t *buf, *ptr, *dn_ptr, *signature_hash_ptr;
+	uint8_t *buf, *ptr, *dn_ptr;
+	size_t len;
 	const struct l_queue_entry *entry;
 	unsigned int i;
 	size_t dn_total = 0;
@@ -1021,9 +1015,8 @@ static bool tls_send_certificate_request(struct l_tls *tls)
 			dn_total += 10 + dn_size;
 	}
 
-	buf = l_malloc(128 + L_ARRAY_SIZE(tls_cert_type_pref) +
-			2 * L_ARRAY_SIZE(tls_signature_hash_pref) +
-			dn_total);
+	len = 256 + L_ARRAY_SIZE(tls_cert_type_pref) + dn_total;
+	buf = l_malloc(len);
 	ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
 
 	/* Fill in the Certificate Request body */
@@ -1032,27 +1025,19 @@ static bool tls_send_certificate_request(struct l_tls *tls)
 	for (i = 0; i < L_ARRAY_SIZE(tls_cert_type_pref); i++)
 		*ptr++ = tls_cert_type_pref[i];
 
-	/*
-	 * This only makes sense as a variable-length field, assume there's
-	 * a typo in RFC5246 7.4.4 here.
-	 *
-	 * TODO: we support the full list of hash algorithms when used
-	 * in the client certificate chain but we can only verify the
-	 * Certificate Verify signature when the hash algorithm matches
-	 * one of HANDSHAKE_HASH_*.  The values we include here will
-	 * affect both of these steps so revisit which set we're passing
-	 * here.
-	 */
 	if (tls->negotiated_version >= L_TLS_V12) {
-		signature_hash_ptr = ptr;
-		ptr += 2;
+		ssize_t ret = tls_write_signature_algorithms(tls, ptr,
+							buf + len - ptr);
 
-		for (i = 0; i < L_ARRAY_SIZE(tls_signature_hash_pref); i++) {
-			*ptr++ = tls_signature_hash_pref[i].hash_id;
-			*ptr++ = tls_signature_hash_pref[i].signature_id;
+		if (ret < 0) {
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+					"tls_write_signature_algorithms: %s",
+					strerror(-ret));
+			l_free(buf);
+			return false;
 		}
 
-		l_put_be16(ptr - (signature_hash_ptr + 2), signature_hash_ptr);
+		ptr += ret;
 	}
 
 	dn_ptr = ptr;
@@ -1857,18 +1842,24 @@ done:
 static void tls_handle_certificate_request(struct l_tls *tls,
 						const uint8_t *buf, size_t len)
 {
-	int cert_type_len, signature_hash_len, dn_len, i;
-	enum handshake_hash_type first_supported, hash;
-	const uint8_t *signature_hash_data;
-	uint8_t hash_id;
+	unsigned int cert_type_len, dn_len, i;
 
 	tls->cert_requested = 1;
 
 	cert_type_len = *buf++;
-	if (len < (size_t) 1 + cert_type_len + 2)
+	if (len < 1 + cert_type_len + 2)
 		goto decode_error;
 
-	/* Skip certificate_types */
+	for (i = 0; i < sizeof(tls_cert_type_pref); i++)
+		if (memchr(buf, tls_cert_type_pref[i], cert_type_len))
+			break;
+
+	if (i == sizeof(tls_cert_type_pref)) {
+		TLS_DISCONNECT(TLS_ALERT_UNSUPPORTED_CERT, 0,
+				"Requested certificate types not supported");
+		return;
+	}
+
 	buf += cert_type_len;
 	len -= 1 + cert_type_len;
 
@@ -1879,66 +1870,21 @@ static void tls_handle_certificate_request(struct l_tls *tls,
 	 */
 
 	if (tls->negotiated_version >= L_TLS_V12) {
-		/*
-		 * This only makes sense as a variable-length field, assume
-		 * there's a typo in RFC5246 7.4.4 here.
-		 */
-		signature_hash_len = l_get_be16(buf);
-		signature_hash_data = buf + 2;
-
-		if (len < (size_t) 2 + signature_hash_len + 2 ||
-				(signature_hash_len & 1))
-			goto decode_error;
-
-		len -= 2 + signature_hash_len;
-		buf += 2 + signature_hash_len;
-
-		/*
-		 * In 1.2 SHA256 is the default because that is most likely
-		 * to be supported in all the scenarios and optimal because
-		 * SHA256 is required independently for the Finished hash
-		 * meaning that we'll just need one hash type instead of
-		 * two.  If not available fall back to the first common
-		 * hash algorithm.
-		 */
-		first_supported = -1;
-
-		for (i = 0; i < signature_hash_len; i += 2) {
-			hash_id = signature_hash_data[i + 0];
-
-			/* Ignore hash types for signatures other than ours */
-			if (signature_hash_data[i + 1] !=
-					tls->pending.cipher_suite->
-						signature->id)
-				continue;
-
-			if (hash_id == tls_handshake_hash_data[
-						HANDSHAKE_HASH_SHA256].tls_id)
-				break;
-
-			if ((int) first_supported != -1)
-				continue;
-
-			for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
-				if (hash_id == tls_handshake_hash_data[hash].
-						tls_id &&
-						tls->handshake_hash[hash]) {
-					first_supported = hash;
-					break;
-				}
-		}
+		enum handshake_hash_type hash;
+		ssize_t ret = tls_parse_signature_algorithms(tls, buf, len);
 
-		if (i < signature_hash_len)
-			tls->signature_hash = HANDSHAKE_HASH_SHA256;
-		else if ((int) first_supported != -1)
-			tls->signature_hash = first_supported;
-		else {
+		if (ret == -ENOTSUP) {
 			TLS_DISCONNECT(TLS_ALERT_UNSUPPORTED_CERT, 0,
 					"No supported signature hash type");
-
 			return;
 		}
 
+		if (ret < 0)
+			goto decode_error;
+
+		len -= ret;
+		buf += ret;
+
 		/*
 		 * We can now safely stop maintaining handshake message
 		 * hashes other than the PRF hash and the one selected for
@@ -1951,7 +1897,7 @@ static void tls_handle_certificate_request(struct l_tls *tls,
 	}
 
 	dn_len = l_get_be16(buf);
-	if ((size_t) 2 + dn_len != len)
+	if (2 + dn_len != len)
 		goto decode_error;
 
 	return;
-- 
2.19.1

[PATCH 3/4] tls: Drop duplicate check

[Andrew Zaborowski]

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

The Hello message handlers already call tls_cipher_suite_is_compatible
which makes sure a cipher suite incompatible with the loaded certificate
is not selected so drop the later check for certificate compatibility.
---
 ell/tls.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/ell/tls.c b/ell/tls.c
index 27f291a..8aa58d5 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -921,7 +921,6 @@ static bool tls_send_certificate(struct l_tls *tls)
 {
 	uint8_t *buf, *ptr;
 	size_t total;
-	struct l_cert *leaf;
 
 	if (tls->server && !tls->cert) {
 		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, TLS_ALERT_BAD_CERT,
@@ -936,18 +935,6 @@ static bool tls_send_certificate(struct l_tls *tls)
 		return false;
 	}
 
-	/* TODO: might want check this earlier and exclude the cipher suite */
-	leaf = l_certchain_get_leaf(tls->cert);
-	if (leaf && !tls->pending.cipher_suite->signature->
-			validate_cert_key_type(leaf)) {
-		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, TLS_ALERT_CERT_UNKNOWN,
-				"Local certificate has key type incompatible "
-				"with pending cipher suite %s",
-				tls->pending.cipher_suite->name);
-
-		return false;
-	}
-
 	/*
 	 * TODO: check that the certificate is compatible with hash and
 	 * signature algorithms lists supplied to us in the Client Hello
-- 
2.19.1

[PATCH 4/4] key: Don't generate description string from pointer

[Andrew Zaborowski]

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

Within a keyring each key must have a unique key type + description
string combination.  If a key is linked to a keyring with the same
description and key type as another key in the keyring, the other key is
unlinked (this is documented).  Since we allow our references to the
kernel keys to be freed without revoking them, through
l_key_free_norevoke, and we depend on them staying usable in the
keyrings we can't generate the key descriptions from just the pointer
because after a key is freed in the userspace another key may get the
same pointer and the same description string.  In fact if we're adding
keys to a keyring in a loop and freeing our references to them
immediately each key will get the same description and we end up
removing the previous key in each iteration.  Fix this by using a
global (to the process) sequential ids for the keys and keyrings.
---
 ell/key.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ell/key.c b/ell/key.c
index 304344f..ce4fbad 100644
--- a/ell/key.c
+++ b/ell/key.c
@@ -286,6 +286,7 @@ LIB_EXPORT struct l_key *l_key_new(enum l_key_type type, const void *payload,
 {
 	struct l_key *key;
 	char *description;
+	static unsigned long key_idx;
 
 	if (unlikely(!payload))
 		return NULL;
@@ -298,7 +299,7 @@ LIB_EXPORT struct l_key *l_key_new(enum l_key_type type, const void *payload,
 
 	key = l_new(struct l_key, 1);
 	key->type = type;
-	description = l_strdup_printf("ell-key-%p", key);
+	description = l_strdup_printf("ell-key-%lu", key_idx++);
 	key->serial = kernel_add_key(key_type_names[type], description, payload,
 					payload_length, internal_keyring);
 	l_free(description);
@@ -657,12 +658,13 @@ LIB_EXPORT struct l_keyring *l_keyring_new(void)
 {
 	struct l_keyring *keyring;
 	char *description;
+	static unsigned long keyring_idx;
 
 	if (!internal_keyring && !setup_internal_keyring())
 		return NULL;
 
 	keyring = l_new(struct l_keyring, 1);
-	description = l_strdup_printf("ell-keyring-%p", keyring);
+	description = l_strdup_printf("ell-keyring-%lu", keyring_idx++);
 	keyring->serial = kernel_add_key("keyring", description, NULL, 0,
 						internal_keyring);
 	l_free(description);
-- 
2.19.1

Re: [PATCH 1/4] tls: Implement the Signature Algorithms extension

[Denis Kenzior]

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

Hi Andrew,

On 02/08/2019 09:31 AM, Andrew Zaborowski wrote:
> ---
>   ell/tls-extensions.c | 279 +++++++++++++++++++++++++++++++++++++++++++
>   ell/tls-private.h    |   4 +
>   ell/tls.c            |  10 +-
>   3 files changed, 284 insertions(+), 9 deletions(-)
> 


All applied, thanks.

Regards,
-Denis