[PATCH 01/17] tls: Only accept the Certificate Request in client mode

[PATCH 01/17] tls: Only accept the Certificate Request in client mode

[Andrew Zaborowski]

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

---
 ell/tls.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ell/tls.c b/ell/tls.c
index eab9cc2..f4cd94c 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -2312,6 +2312,12 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 		break;
 
 	case TLS_CERTIFICATE_REQUEST:
+		if (tls->server) {
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in server mode");
+			break;
+		}
+
 		/*
 		 * Server sends this optionally so in the WAIT_HELLO_DONE
 		 * state we accept either this or a Server Hello Done (below).
-- 
2.19.1

[PATCH 02/17] tls: Add Hello extension sending support

[Andrew Zaborowski]

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

Declare a struct for ServerHello/ClientHello extensions with write and
handle callbacks, modify our ServerHello and ClientHello message
builders to append the extensions to the messages.
---
 ell/tls-private.h |  18 +++++++++
 ell/tls.c         | 101 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 112 insertions(+), 7 deletions(-)

diff --git a/ell/tls-private.h b/ell/tls-private.h
index 0908b9c..5ea8208 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -99,6 +99,24 @@ struct tls_compression_method {
 	const char *name;
 };
 
+struct tls_hello_extension {
+	const char *name;
+	const char *short_name;
+	uint16_t id;
+	ssize_t (*client_write)(struct l_tls *tls, uint8_t *buf, size_t len);
+	/* Handle a Client Hello extension (on server), can't be NULL */
+	bool (*client_handle)(struct l_tls *tls,
+				const uint8_t *buf, size_t len);
+	/* Handle a Client Hello extension's absence (on server) */
+	bool (*client_handle_absent)(struct l_tls *tls);
+	ssize_t (*server_write)(struct l_tls *tls, uint8_t *buf, size_t len);
+	/* Handle a Server Hello extension (on client) */
+	bool (*server_handle)(struct l_tls *tls,
+				const uint8_t *buf, size_t len);
+	/* Handle a Server Hello extension's absence (on client) */
+	bool (*server_handle_absent)(struct l_tls *tls);
+};
+
 enum tls_handshake_state {
 	TLS_HANDSHAKE_WAIT_START,
 	TLS_HANDSHAKE_WAIT_HELLO,
diff --git a/ell/tls.c b/ell/tls.c
index f4cd94c..ffa09e1 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -709,6 +709,8 @@ static const struct tls_hash_algorithm *tls_set_prf_hmac(struct l_tls *tls)
 	return NULL;
 }
 
+static const struct tls_hello_extension tls_extensions[] = {};
+
 enum tls_handshake_type {
 	TLS_HELLO_REQUEST	= 0,
 	TLS_CLIENT_HELLO	= 1,
@@ -812,11 +814,12 @@ static void tls_tx_handshake(struct l_tls *tls, int type, uint8_t *buf,
 
 static bool tls_send_client_hello(struct l_tls *tls)
 {
-	uint8_t buf[128 + L_ARRAY_SIZE(tls_compression_pref) +
+	uint8_t buf[1024 + L_ARRAY_SIZE(tls_compression_pref) +
 			2 * L_ARRAY_SIZE(tls_cipher_suite_pref)];
 	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
 	uint8_t *len_ptr;
-	int i;
+	unsigned int i;
+	uint8_t *extensions_len_ptr;
 
 	/* Fill in the Client Hello body */
 
@@ -832,7 +835,7 @@ static bool tls_send_client_hello(struct l_tls *tls)
 	len_ptr = ptr;
 	ptr += 2;
 
-	for (i = 0; i < (int) L_ARRAY_SIZE(tls_cipher_suite_pref); i++) {
+	for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++) {
 		if (!tls_cipher_suite_is_compatible(tls,
 						&tls_cipher_suite_pref[i],
 						NULL))
@@ -850,17 +853,58 @@ static bool tls_send_client_hello(struct l_tls *tls)
 	l_put_be16((ptr - len_ptr - 2), len_ptr);
 	*ptr++ = L_ARRAY_SIZE(tls_compression_pref);
 
-	for (i = 0; i < (int) L_ARRAY_SIZE(tls_compression_pref); i++)
+	for (i = 0; i < L_ARRAY_SIZE(tls_compression_pref); i++)
 		*ptr++ = tls_compression_pref[i].id;
 
+	extensions_len_ptr = ptr;
+	ptr += 2;
+
+	for (i = 0; tls_extensions[i].name; i++) {
+		const struct tls_hello_extension *extension =
+			&tls_extensions[i];
+		ssize_t ext_len;
+
+		/*
+		 * Note: could handle NULL client_write with non-NULL
+		 * server_handle or server_handle_absent as "server-oriented"
+		 * extension (7.4.1.4) and write empty extension_data and
+		 * simliarly require empty extension_data in
+		 * tls_handle_client_hello if client_handle NULL.
+		 */
+		if (!extension->client_write)
+			continue;
+
+		ext_len = extension->client_write(tls, ptr + 4,
+						buf + sizeof(buf) - (ptr + 4));
+		if (ext_len == -ENOMSG)
+			continue;
+
+		if (ext_len < 0) {
+			TLS_DEBUG("%s extension's client_write: %s",
+					extension->name, strerror(-ext_len));
+			return false;
+		}
+
+		l_put_be16(extension->id, ptr + 0);
+		l_put_be16(ext_len, ptr + 2);
+		ptr += 4 + ext_len;
+	}
+
+	if (ptr > extensions_len_ptr + 2)
+		l_put_be16(ptr - (extensions_len_ptr + 2), extensions_len_ptr);
+	else /* Skip the length if no extensions */
+		ptr = extensions_len_ptr;
+
 	tls_tx_handshake(tls, TLS_CLIENT_HELLO, buf, ptr - buf);
 	return true;
 }
 
-static void tls_send_server_hello(struct l_tls *tls)
+static bool tls_send_server_hello(struct l_tls *tls, struct l_queue *extensions)
 {
-	uint8_t buf[128];
+	uint8_t buf[1024];
 	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
+	uint8_t *extensions_len_ptr;
+	const struct l_queue_entry *entry;
 
 	/* Fill in the Server Hello body */
 
@@ -878,7 +922,49 @@ static void tls_send_server_hello(struct l_tls *tls)
 
 	*ptr++ = tls->pending.compression_method->id;
 
+	extensions_len_ptr = ptr;
+	ptr += 2;
+
+	for (entry = l_queue_get_entries(extensions); entry;
+			entry = entry->next) {
+		uint16_t ext_id = L_PTR_TO_UINT(entry->data);
+		const struct tls_hello_extension *extension = NULL;
+		unsigned int i;
+		ssize_t ext_len;
+
+		for (i = 0; tls_extensions[i].name; i++)
+			if (tls_extensions[i].id == ext_id) {
+				extension = &tls_extensions[i];
+				break;
+			}
+
+		if (!extension || !extension->server_write)
+			continue;
+
+		ext_len = extension->server_write(tls, ptr + 4,
+						buf + sizeof(buf) - (ptr + 4));
+		if (ext_len == -ENOMSG)
+			continue;
+
+		if (ext_len < 0) {
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+					"%s extension's server_write: %s",
+					extension->name, strerror(-ext_len));
+			return false;
+		}
+
+		l_put_be16(ext_id, ptr + 0);
+		l_put_be16(ext_len, ptr + 2);
+		ptr += 4 + ext_len;
+	}
+
+	if (ptr > extensions_len_ptr + 2)
+		l_put_be16(ptr - (extensions_len_ptr + 2), extensions_len_ptr);
+	else /* Skip the length if no extensions */
+		ptr = extensions_len_ptr;
+
 	tls_tx_handshake(tls, TLS_SERVER_HELLO, buf, ptr - buf);
+	return true;
 }
 
 static bool tls_cert_list_add_size(struct l_cert *cert, void *user_data)
@@ -1644,7 +1730,8 @@ static void tls_handle_client_hello(struct l_tls *tls,
 
 	TLS_DEBUG("Negotiated %s", tls->pending.compression_method->name);
 
-	tls_send_server_hello(tls);
+	if (!tls_send_server_hello(tls, NULL))
+		return;
 
 	if (tls->pending.cipher_suite->key_xchg->certificate_check && tls->cert)
 		if (!tls_send_certificate(tls))
-- 
2.19.1

[PATCH 03/17] tls: Parse and process received extensions

[Andrew Zaborowski]

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

On the server call the .client_handle callbacks for each supported
extension in the received ClientHello and pass the list of extensions
we've handled to the ServerHello builder.  On the client, process the
extension responses in the ServerHello by calling .server_handle
callbacks.
---
 ell/tls.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 154 insertions(+), 25 deletions(-)

diff --git a/ell/tls.c b/ell/tls.c
index ffa09e1..f8ba3d0 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -1577,6 +1577,135 @@ static bool tls_verify_finished(struct l_tls *tls, const uint8_t *received,
 	return true;
 }
 
+static bool tls_ptr_match(const void *a, const void *b)
+{
+	return a == b;
+}
+
+static bool tls_handle_hello_extensions(struct l_tls *tls,
+					const uint8_t *buf, size_t len,
+					struct l_queue *seen)
+{
+	unsigned int i;
+	const struct tls_hello_extension *extension;
+	bool client_hello = tls->server;
+	uint16_t extensions_size;
+
+	if (!len)
+		return true;
+
+	if (len < 2 || len > 2 + 65535)
+		goto decode_error;
+
+	extensions_size = l_get_be16(buf);
+	len -= 2;
+	buf += 2;
+
+	if (len != extensions_size)
+		goto decode_error;
+
+	while (len) {
+		uint16_t ext_id;
+		size_t ext_len;
+		bool (*handler)(struct l_tls *tls,
+				const uint8_t *buf, size_t len);
+
+		if (len < 4)
+			goto decode_error;
+
+		ext_id = l_get_be16(buf + 0);
+		ext_len = l_get_be16(buf + 2);
+		buf += 4;
+		len -= 4;
+
+		if (ext_len > len)
+			goto decode_error;
+
+		/*
+		 * RFC 5246, Section 7.4.1.4: "There MUST NOT be more than
+		 * one extension of the same type."
+		 */
+		if (l_queue_find(seen, tls_ptr_match, L_UINT_TO_PTR(ext_id))) {
+			TLS_DEBUG("Duplicate extension %u", ext_id);
+			goto decode_error;
+		}
+
+		l_queue_push_tail(seen, L_UINT_TO_PTR(ext_id));
+
+		extension = NULL;
+
+		for (i = 0; tls_extensions[i].name; i++)
+			if (tls_extensions[i].id == ext_id) {
+				extension = &tls_extensions[i];
+				break;
+			}
+
+		if (!extension)
+			goto next;
+
+		handler = client_hello ?
+			extension->client_handle : extension->server_handle;
+
+		/*
+		 * RFC 5246, Section 7.4.1.4: "If a client receives an
+		 * extension type in ServerHello that it did not request in
+		 * the associated ClientHello, it MUST abort the handshake
+		 * with an unsupported_extension fatal alert."
+		 */
+		if (!client_hello && !handler) {
+			TLS_DISCONNECT(TLS_ALERT_UNSUPPORTED_EXTENSION, 0,
+					"%s extension not expected in "
+					"a ServerHello", extension->name);
+			return false;
+		}
+
+		if (!handler(tls, buf, ext_len)) {
+			TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+					"Hello %s extension parse error",
+					extension->name);
+			return false;
+		}
+
+next:
+		buf += ext_len;
+		len -= ext_len;
+	}
+
+	/*
+	 * Trigger any actions needed when an extension is missing and its
+	 * handler has not been called yet.
+	 */
+	for (i = 0; tls_extensions[i].name; i++) {
+		bool (*handler)(struct l_tls *tls);
+
+		extension = &tls_extensions[i];
+		handler = client_hello ?
+			extension->client_handle_absent :
+			extension->server_handle_absent;
+
+		if (!handler)
+			continue;
+
+		if (l_queue_find(seen, tls_ptr_match,
+					L_UINT_TO_PTR(extension->id)))
+			continue;
+
+		if (!handler(tls)) {
+			TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+					"Hello %s extension missing",
+					extension->name);
+			return false;
+		}
+	}
+
+	return true;
+
+decode_error:
+	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+			"Hello extensions decode error");
+	return false;
+}
+
 static void tls_handle_client_hello(struct l_tls *tls,
 					const uint8_t *buf, size_t len)
 {
@@ -1585,6 +1714,7 @@ static void tls_handle_client_hello(struct l_tls *tls,
 	const uint8_t *cipher_suites;
 	const uint8_t *compression_methods;
 	int i;
+	struct l_queue *extensions_offered = NULL;
 
 	/* Do we have enough for ProtocolVersion + Random + SessionID size? */
 	if (len < 2 + 32 + 1)
@@ -1626,22 +1756,12 @@ static void tls_handle_client_hello(struct l_tls *tls,
 
 	len -= compression_methods_size;
 
-	if (len) {
-		uint16_t extensions_size;
+	extensions_offered = l_queue_new();
 
-		if (len < 2 || len > 2 + 65535)
-			goto decode_error;
-
-		extensions_size = l_get_be16(compression_methods +
-						compression_methods_size);
-		len -= 2;
-
-		if (len != extensions_size)
-			goto decode_error;
-
-		/* TODO: validate each extension in the vector, 7.4.1.4 */
-		/* TODO: check for duplicates? */
-	}
+	if (!tls_handle_hello_extensions(tls, compression_methods +
+					compression_methods_size,
+					len, extensions_offered))
+		goto cleanup;
 
 	/*
 	 * Note: if the client is supplying a SessionID we know it is false
@@ -1664,7 +1784,7 @@ static void tls_handle_client_hello(struct l_tls *tls,
 		TLS_DISCONNECT(TLS_ALERT_PROTOCOL_VERSION, 0,
 				"Client version too low: %02x",
 				tls->client_version);
-		return;
+		goto cleanup;
 	}
 
 	tls->negotiated_version = TLS_VERSION < tls->client_version ?
@@ -1697,13 +1817,13 @@ static void tls_handle_client_hello(struct l_tls *tls,
 		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
 				"No common cipher suites matching negotiated "
 				"TLS version and our certificate's key type");
-		return;
+		goto cleanup;
 	}
 
 	if (!tls_set_prf_hmac(tls)) {
 		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
 				"Error selecting the PRF HMAC");
-		return;
+		goto cleanup;
 	}
 
 	TLS_DEBUG("Negotiated %s", tls->pending.cipher_suite->name);
@@ -1714,7 +1834,7 @@ static void tls_handle_client_hello(struct l_tls *tls,
 	if (!memchr(compression_methods, 0, compression_methods_size)) {
 		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
 				"No common compression methods");
-		return;
+		goto cleanup;
 	}
 
 	while (compression_methods_size) {
@@ -1730,8 +1850,10 @@ static void tls_handle_client_hello(struct l_tls *tls,
 
 	TLS_DEBUG("Negotiated %s", tls->pending.compression_method->name);
 
-	if (!tls_send_server_hello(tls, NULL))
-		return;
+	if (!tls_send_server_hello(tls, extensions_offered))
+		goto cleanup;
+
+	l_queue_destroy(extensions_offered, NULL);
 
 	if (tls->pending.cipher_suite->key_xchg->certificate_check && tls->cert)
 		if (!tls_send_certificate(tls))
@@ -1756,6 +1878,9 @@ static void tls_handle_client_hello(struct l_tls *tls,
 decode_error:
 	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
 			"ClientHello decode error");
+
+cleanup:
+	l_queue_destroy(extensions_offered, NULL);
 }
 
 static void tls_handle_server_hello(struct l_tls *tls,
@@ -1764,6 +1889,8 @@ static void tls_handle_server_hello(struct l_tls *tls,
 	uint8_t session_id_size, cipher_suite_id[2], compression_method_id;
 	const char *error;
 	int i;
+	struct l_queue *extensions_seen;
+	bool result;
 
 	/* Do we have enough for ProtocolVersion + Random + SessionID len ? */
 	if (len < 2 + 32 + 1)
@@ -1782,11 +1909,13 @@ static void tls_handle_server_hello(struct l_tls *tls,
 	compression_method_id = buf[35 + session_id_size + 2];
 	len -= session_id_size + 2 + 1;
 
-	if (len != 0) { /* We know we haven't solicited any extensions */
-		TLS_DISCONNECT(TLS_ALERT_UNSUPPORTED_EXTENSION, 0,
-				"ServerHello contains extensions");
+	extensions_seen = l_queue_new();
+	result = tls_handle_hello_extensions(tls, buf + 38 + session_id_size,
+						len, extensions_seen);
+	l_queue_destroy(extensions_seen, NULL);
+
+	if (!result)
 		return;
-	}
 
 	tls->negotiated_version = l_get_be16(buf);
 
-- 
2.19.1

[PATCH 04/17] tls: Implement the Supported Elliptic Curves extension

[Andrew Zaborowski]

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

Defined in RFC 8422 5.1.1
---
 Makefile.am          |   1 +
 ell/tls-extensions.c | 108 +++++++++++++++++++++++++++++++++++++++++++
 ell/tls-private.h    |  10 ++++
 ell/tls.c            |   3 +-
 4 files changed, 120 insertions(+), 2 deletions(-)
 create mode 100644 ell/tls-extensions.c

diff --git a/Makefile.am b/Makefile.am
index 8401972..297c395 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -102,6 +102,7 @@ ell_libell_la_SOURCES = $(linux_headers) \
 			ell/tls-private.h \
 			ell/tls.c \
 			ell/tls-record.c \
+			ell/tls-extensions.c \
 			ell/uuid.c \
 			ell/key.c \
 			ell/pkcs5-private.h \
diff --git a/ell/tls-extensions.c b/ell/tls-extensions.c
new file mode 100644
index 0000000..749b8e6
--- /dev/null
+++ b/ell/tls-extensions.c
@@ -0,0 +1,108 @@
+/*
+ *  Embedded Linux library
+ *
+ *  Copyright (C) 2018  Intel Corporation. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <errno.h>
+
+#include "util.h"
+#include "tls.h"
+#include "cipher.h"
+#include "checksum.h"
+#include "cert.h"
+#include "tls-private.h"
+
+static const struct tls_named_curve tls_curve_pref[] = {
+	{ "secp256r1", 23, 19, 64 },
+	{ "secp384r1", 24, 20, 96 },
+};
+
+/* RFC 8422, Section 5.1 */
+static ssize_t tls_elliptic_curves_client_write(struct l_tls *tls,
+						uint8_t *buf, size_t len)
+{
+	uint8_t *ptr = buf;
+	unsigned int i;
+
+	if (len < 2 + L_ARRAY_SIZE(tls_curve_pref) * 2)
+		return -ENOMEM;
+
+	l_put_be16(L_ARRAY_SIZE(tls_curve_pref) * 2, ptr);
+	ptr += 2;
+
+	for (i = 0; i < L_ARRAY_SIZE(tls_curve_pref); i++) {
+		l_put_be16(tls_curve_pref[i].id, ptr);
+		ptr += 2;
+	}
+
+	return ptr - buf;
+}
+
+static bool tls_elliptic_curves_client_handle(struct l_tls *tls,
+						const uint8_t *buf, size_t len)
+{
+	if (len < 2)
+		return false;
+
+	if (l_get_be16(buf) != len - 2 || (len & 1))
+		return false;
+
+	buf += 2;
+	len -= 2;
+
+	while (len) {
+		unsigned int i;
+		uint16_t id;
+
+		id = l_get_be16(buf);
+		buf += 2;
+		len -= 2;
+
+		for (i = 0; i < L_ARRAY_SIZE(tls_curve_pref); i++)
+			if (tls_curve_pref[i].id == id) {
+				tls->negotiated_curve = &tls_curve_pref[i];
+				TLS_DEBUG("Negotiated %s",
+						tls_curve_pref[i].name);
+				return true;
+			}
+	}
+
+	TLS_DEBUG("non-fatal: No common supported elliptic curves");
+	return true;
+}
+
+static bool tls_elliptic_curves_client_absent(struct l_tls *tls)
+{
+	tls->negotiated_curve = &tls_curve_pref[0];
+	return true;
+}
+
+const struct tls_hello_extension tls_extensions[] = {
+	{
+		"Supported Elliptic Curves", "elliptic_curves", 10,
+		tls_elliptic_curves_client_write,
+		tls_elliptic_curves_client_handle,
+		tls_elliptic_curves_client_absent,
+		NULL, NULL, NULL,
+	},
+	{}
+};
diff --git a/ell/tls-private.h b/ell/tls-private.h
index 5ea8208..d1cfcac 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -117,6 +117,15 @@ struct tls_hello_extension {
 	bool (*server_handle_absent)(struct l_tls *tls);
 };
 
+extern const struct tls_hello_extension tls_extensions[];
+
+struct tls_named_curve {
+	const char *name;
+	uint16_t id;
+	unsigned int l_group;
+	size_t point_bytes;
+};
+
 enum tls_handshake_state {
 	TLS_HANDSHAKE_WAIT_START,
 	TLS_HANDSHAKE_WAIT_HELLO,
@@ -198,6 +207,7 @@ struct l_tls {
 	size_t peer_pubkey_size;
 	enum handshake_hash_type signature_hash;
 	const struct tls_hash_algorithm *prf_hmac;
+	const struct tls_named_curve *negotiated_curve;
 
 	/* SecurityParameters current and pending */
 
diff --git a/ell/tls.c b/ell/tls.c
index f8ba3d0..ed7f0c7 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -191,6 +191,7 @@ static void tls_reset_handshake(struct l_tls *tls)
 	tls->peer_cert = NULL;
 	tls->peer_pubkey = NULL;
 	tls->peer_pubkey_size = 0;
+	tls->negotiated_curve = NULL;
 
 	for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
 		tls_drop_handshake_hash(tls, hash);
@@ -709,8 +710,6 @@ static const struct tls_hash_algorithm *tls_set_prf_hmac(struct l_tls *tls)
 	return NULL;
 }
 
-static const struct tls_hello_extension tls_extensions[] = {};
-
 enum tls_handshake_type {
 	TLS_HELLO_REQUEST	= 0,
 	TLS_CLIENT_HELLO	= 1,
-- 
2.19.1

[PATCH 05/17] tls: Implement the Supported Point Formats extension

[Andrew Zaborowski]

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

Defined in RFC 8422 5.1.2
---
 ell/tls-extensions.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/ell/tls-extensions.c b/ell/tls-extensions.c
index 749b8e6..3d75037 100644
--- a/ell/tls-extensions.c
+++ b/ell/tls-extensions.c
@@ -96,6 +96,40 @@ static bool tls_elliptic_curves_client_absent(struct l_tls *tls)
 	return true;
 }
 
+static bool tls_ec_point_formats_client_handle(struct l_tls *tls,
+						const uint8_t *buf, size_t len)
+{
+	if (len < 2)
+		return false;
+
+	if (buf[0] != len - 1)
+		return false;
+
+	if (!memchr(buf + 1, 0, len - 1)) {
+		TLS_DEBUG("Uncompressed point format missing");
+		return false;
+	}
+
+	return true;
+}
+
+/*
+ * For compatibility with clients respond to a valid Client Hello Supported
+ * Point Formats extension with the hardcoded confirmation that we do
+ * support the single valid point format.  As a client we never send this
+ * extension so we never have to handle a server response to it either.
+ */
+static ssize_t tls_ec_point_formats_server_write(struct l_tls *tls,
+						uint8_t *buf, size_t len)
+{
+	if (len < 2)
+		return -ENOMEM;
+
+	buf[0] = 0x01;	/* ec_point_format_list length */
+	buf[1] = 0x00;	/* uncompressed */
+	return 2;
+}
+
 const struct tls_hello_extension tls_extensions[] = {
 	{
 		"Supported Elliptic Curves", "elliptic_curves", 10,
@@ -104,5 +138,13 @@ const struct tls_hello_extension tls_extensions[] = {
 		tls_elliptic_curves_client_absent,
 		NULL, NULL, NULL,
 	},
+	{
+		"Supported Point Formats", "ec_point_formats", 11,
+		NULL,
+		tls_ec_point_formats_client_handle,
+		NULL,
+		tls_ec_point_formats_server_write,
+		NULL, NULL,
+	},
 	{}
 };
-- 
2.19.1

[PATCH 06/17] tls: Allow user to set custom list of cipher suites

[Andrew Zaborowski]

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

TLS "security profiles" define which minimum cipher suites and other
parameter values are allowed in a specific usage scenario for TLS, for
example WPA3 in theory requires a specific security profile for its
TLS-based EAP method.  This will also allow the unit tests to test
individual cipher suites.
---
 ell/tls-private.h |   5 +
 ell/tls.c         | 260 +++++++++++++++++++++++++++++++---------------
 2 files changed, 181 insertions(+), 84 deletions(-)

diff --git a/ell/tls-private.h b/ell/tls-private.h
index d1cfcac..938c358 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -180,6 +180,8 @@ struct l_tls {
 	struct l_key *priv_key;
 	size_t priv_key_size;
 
+	struct tls_cipher_suite **cipher_suite_pref_list;
+
 	/* Record layer */
 
 	uint8_t *record_buf;
@@ -267,6 +269,9 @@ void tls_tx_record(struct l_tls *tls, enum tls_content_type type,
 bool tls_handle_message(struct l_tls *tls, const uint8_t *message,
 			int len, enum tls_content_type type, uint16_t version);
 
+/* Optionally limit allowed cipher suites to a custom set */
+bool tls_set_cipher_suites(struct l_tls *tls, const char **suite_list);
+
 int tls_parse_certificate_list(const void *data, size_t len,
 				struct l_certchain **out_certchain);
 
diff --git a/ell/tls.c b/ell/tls.c
index ed7f0c7..1154bcd 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -41,6 +41,7 @@
 #include "tls-private.h"
 #include "key.h"
 #include "asn1-private.h"
+#include "strv.h"
 
 bool tls10_prf(const void *secret, size_t secret_len,
 		const char *label,
@@ -453,78 +454,80 @@ static struct tls_mac_algorithm tls_md5 = {
 	.mac_length = 32,
 };
 
-static struct tls_cipher_suite tls_cipher_suite_pref[] = {
-	{
-		.id = { 0x00, 0x35 },
-		.name = "TLS_RSA_WITH_AES_256_CBC_SHA",
-		.verify_data_length = 12,
-		.encryption = &tls_aes256,
-		.mac = &tls_sha,
-		.key_xchg = &tls_rsa,
-	},
-	{
-		.id = { 0x00, 0x2f },
-		.name = "TLS_RSA_WITH_AES_128_CBC_SHA",
-		.verify_data_length = 12,
-		.encryption = &tls_aes128,
-		.mac = &tls_sha,
-		.key_xchg = &tls_rsa,
-	},
-	{
-		.id = { 0x00, 0x3d },
-		.name = "TLS_RSA_WITH_AES_256_CBC_SHA256",
-		.verify_data_length = 12,
-		.encryption = &tls_aes256,
-		.mac = &tls_sha256,
-		.key_xchg = &tls_rsa,
-	},
-	{
-		.id = { 0x00, 0x3c },
-		.name = "TLS_RSA_WITH_AES_128_CBC_SHA256",
-		.verify_data_length = 12,
-		.encryption = &tls_aes128,
-		.mac = &tls_sha256,
-		.key_xchg = &tls_rsa,
-	},
-	{
-		.id = { 0x00, 0x9d },
-		.name = "TLS_RSA_WITH_AES_256_GCM_SHA384",
-		.verify_data_length = 12,
-		.encryption = &tls_aes256_gcm,
-		.prf_hmac = L_CHECKSUM_SHA384,
-		.key_xchg = &tls_rsa,
-	},
-	{
-		.id = { 0x00, 0x9c },
-		.name = "TLS_RSA_WITH_AES_128_GCM_SHA256",
-		.verify_data_length = 12,
-		.encryption = &tls_aes128_gcm,
-		.key_xchg = &tls_rsa,
-	},
-	{
-		.id = { 0x00, 0x0a },
-		.name = "TLS_RSA_WITH_3DES_EDE_CBC_SHA",
-		.verify_data_length = 12,
-		.encryption = &tls_3des_ede,
-		.mac = &tls_sha,
-		.key_xchg = &tls_rsa,
-	},
-	{
-		.id = { 0x00, 0x05 },
-		.name = "TLS_RSA_WITH_RC4_128_SHA",
-		.verify_data_length = 12,
-		.encryption = &tls_rc4,
-		.mac = &tls_sha,
-		.key_xchg = &tls_rsa,
-	},
-	{
-		.id = { 0x00, 0x04 },
-		.name = "TLS_RSA_WITH_RC4_128_MD5",
-		.verify_data_length = 12,
-		.encryption = &tls_rc4,
-		.mac = &tls_md5,
-		.key_xchg = &tls_rsa,
-	},
+static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
+	.id = { 0x00, 0x04 },
+	.name = "TLS_RSA_WITH_RC4_128_MD5",
+	.verify_data_length = 12,
+	.encryption = &tls_rc4,
+	.mac = &tls_md5,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_rc4_128_sha = {
+	.id = { 0x00, 0x05 },
+	.name = "TLS_RSA_WITH_RC4_128_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_rc4,
+	.mac = &tls_sha,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_3des_ede_cbc_sha = {
+	.id = { 0x00, 0x0a },
+	.name = "TLS_RSA_WITH_3DES_EDE_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_3des_ede,
+	.mac = &tls_sha,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_128_cbc_sha = {
+	.id = { 0x00, 0x2f },
+	.name = "TLS_RSA_WITH_AES_128_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128,
+	.mac = &tls_sha,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_256_cbc_sha = {
+	.id = { 0x00, 0x35 },
+	.name = "TLS_RSA_WITH_AES_256_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256,
+	.mac = &tls_sha,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_128_cbc_sha256 = {
+	.id = { 0x00, 0x3c },
+	.name = "TLS_RSA_WITH_AES_128_CBC_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128,
+	.mac = &tls_sha256,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_256_cbc_sha256 = {
+	.id = { 0x00, 0x3d },
+	.name = "TLS_RSA_WITH_AES_256_CBC_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256,
+	.mac = &tls_sha256,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_128_gcm_sha256 = {
+	.id = { 0x00, 0x9c },
+	.name = "TLS_RSA_WITH_AES_128_GCM_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128_gcm,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_256_gcm_sha384 = {
+	.id = { 0x00, 0x9d },
+	.name = "TLS_RSA_WITH_AES_256_GCM_SHA384",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256_gcm,
+	.prf_hmac = L_CHECKSUM_SHA384,
+	.key_xchg = &tls_rsa,
+};
+
+static struct tls_cipher_suite *tls_cipher_suite_pref[] = {
+	&tls_rsa_with_aes_256_cbc_sha,
+	&tls_rsa_with_aes_128_cbc_sha,
+	&tls_rsa_with_aes_256_cbc_sha256,
+	&tls_rsa_with_aes_128_cbc_sha256,
+	&tls_rsa_with_aes_256_gcm_sha384,
+	&tls_rsa_with_aes_128_gcm_sha256,
+	&tls_rsa_with_3des_ede_cbc_sha,
+	&tls_rsa_with_rc4_128_sha,
+	&tls_rsa_with_rc4_128_md5,
 };
 
 static bool tls_cipher_suite_is_compatible(struct l_tls *tls,
@@ -629,9 +632,9 @@ static struct tls_cipher_suite *tls_find_cipher_suite(const uint8_t *id)
 	int i;
 
 	for (i = 0; i < (int) L_ARRAY_SIZE(tls_cipher_suite_pref); i++)
-		if (tls_cipher_suite_pref[i].id[0] == id[0] &&
-				tls_cipher_suite_pref[i].id[1] == id[1])
-			return &tls_cipher_suite_pref[i];
+		if (tls_cipher_suite_pref[i]->id[0] == id[0] &&
+				tls_cipher_suite_pref[i]->id[1] == id[1])
+			return tls_cipher_suite_pref[i];
 
 	return NULL;
 }
@@ -819,6 +822,7 @@ static bool tls_send_client_hello(struct l_tls *tls)
 	uint8_t *len_ptr;
 	unsigned int i;
 	uint8_t *extensions_len_ptr;
+	struct tls_cipher_suite **suite;
 
 	/* Fill in the Client Hello body */
 
@@ -834,14 +838,16 @@ static bool tls_send_client_hello(struct l_tls *tls)
 	len_ptr = ptr;
 	ptr += 2;
 
-	for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++) {
-		if (!tls_cipher_suite_is_compatible(tls,
-						&tls_cipher_suite_pref[i],
-						NULL))
+	for (suite = tls->cipher_suite_pref_list; *suite; suite++) {
+		const char *error;
+
+		if (!tls_cipher_suite_is_compatible(tls, *suite, &error)) {
+			TLS_DEBUG("non-fatal: %s", error);
 			continue;
+		}
 
-		*ptr++ = tls_cipher_suite_pref[i].id[0];
-		*ptr++ = tls_cipher_suite_pref[i].id[1];
+		*ptr++ = (*suite)->id[0];
+		*ptr++ = (*suite)->id[1];
 	}
 
 	if (ptr == len_ptr + 2) {
@@ -1714,6 +1720,7 @@ static void tls_handle_client_hello(struct l_tls *tls,
 	const uint8_t *compression_methods;
 	int i;
 	struct l_queue *extensions_offered = NULL;
+	enum l_tls_alert_desc alert_desc = TLS_ALERT_HANDSHAKE_FAIL;
 
 	/* Do we have enough for ProtocolVersion + Random + SessionID size? */
 	if (len < 2 + 32 + 1)
@@ -1798,12 +1805,39 @@ static void tls_handle_client_hello(struct l_tls *tls,
 	TLS_DEBUG("Negotiated TLS " TLS_VER_FMT,
 			TLS_VER_ARGS(tls->negotiated_version));
 
+	if (!tls->cipher_suite_pref_list) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"No usable cipher suites");
+		return;
+	}
+
 	/* Select a cipher suite according to client's preference list */
 	while (cipher_suites_size) {
 		struct tls_cipher_suite *suite =
 			tls_find_cipher_suite(cipher_suites);
+		struct tls_cipher_suite **iter;
+		const char *error;
 
-		if (suite && tls_cipher_suite_is_compatible(tls, suite, NULL)) {
+		for (iter = tls->cipher_suite_pref_list; *iter; iter++)
+			if (*iter == suite)
+				break;
+
+		if (!suite)
+			TLS_DEBUG("non-fatal: Cipher suite %04x unknown",
+					l_get_be16(cipher_suites));
+		else if (!tls_cipher_suite_is_compatible(tls, suite, &error))
+			TLS_DEBUG("non-fatal: %s", error);
+		else if (!*iter) {
+			/*
+			 * We have at least one matching compatible suite but
+			 * it is not allowed in this security profile.  If the
+			 * handshake ends up failing then we blame the security
+			 * profile.
+			 */
+			alert_desc = TLS_ALERT_INSUFFICIENT_SECURITY;
+			TLS_DEBUG("non-fatal: Cipher suite %s disallowed "
+					"by config", suite->name);
+		} else {
 			tls->pending.cipher_suite = suite;
 			break;
 		}
@@ -1813,7 +1847,7 @@ static void tls_handle_client_hello(struct l_tls *tls,
 	}
 
 	if (!cipher_suites_size) {
-		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+		TLS_DISCONNECT(alert_desc, 0,
 				"No common cipher suites matching negotiated "
 				"TLS version and our certificate's key type");
 		goto cleanup;
@@ -1887,6 +1921,7 @@ static void tls_handle_server_hello(struct l_tls *tls,
 {
 	uint8_t session_id_size, cipher_suite_id[2], compression_method_id;
 	const char *error;
+	struct tls_cipher_suite **iter;
 	int i;
 	struct l_queue *extensions_seen;
 	bool result;
@@ -1946,6 +1981,16 @@ static void tls_handle_server_hello(struct l_tls *tls,
 		return;
 	}
 
+	for (iter = tls->cipher_suite_pref_list; *iter; iter++)
+		if (*iter == tls->pending.cipher_suite)
+			break;
+	if (!*iter) {
+		TLS_DISCONNECT(TLS_ALERT_INSUFFICIENT_SECURITY, 0,
+				"Selected cipher suite %s disallowed by config",
+				tls->pending.cipher_suite->name);
+		return;
+	}
+
 	if (!tls_cipher_suite_is_compatible(tls, tls->pending.cipher_suite,
 						&error)) {
 		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
@@ -2679,7 +2724,7 @@ LIB_EXPORT struct l_tls *l_tls_new(bool server,
 	tls->ready_handle = ready_handler;
 	tls->disconnected = disconnect_handler;
 	tls->user_data = user_data;
-
+	tls->cipher_suite_pref_list = tls_cipher_suite_pref;
 	tls->signature_hash = HANDSHAKE_HASH_SHA256;
 
 	/* If we're the server wait for the Client Hello already */
@@ -2719,6 +2764,9 @@ LIB_EXPORT void l_tls_free(struct l_tls *tls)
 	if (tls->debug_destroy)
 		tls->debug_destroy(tls->debug_data);
 
+	if (tls->cipher_suite_pref_list != tls_cipher_suite_pref)
+		l_free(tls->cipher_suite_pref_list);
+
 	l_free(tls);
 }
 
@@ -2867,6 +2915,9 @@ bool tls_handle_message(struct l_tls *tls, const uint8_t *message,
 
 LIB_EXPORT bool l_tls_start(struct l_tls *tls)
 {
+	if (!tls->cipher_suite_pref_list)
+		return false;
+
 	/* This is a nop in server mode */
 	if (tls->server)
 		return true;
@@ -2972,6 +3023,47 @@ LIB_EXPORT bool l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
 	return true;
 }
 
+bool tls_set_cipher_suites(struct l_tls *tls, const char **suite_list)
+{
+	struct tls_cipher_suite **suite;
+
+	if (tls->cipher_suite_pref_list != tls_cipher_suite_pref)
+		l_free(tls->cipher_suite_pref_list);
+
+	if (!suite_list) {
+		/* Use our default cipher suite preference list */
+		tls->cipher_suite_pref_list = tls_cipher_suite_pref;
+		return true;
+	}
+
+	tls->cipher_suite_pref_list = l_new(struct tls_cipher_suite *,
+				l_strv_length((char **) suite_list) + 1);
+	suite = tls->cipher_suite_pref_list;
+
+	for (; *suite_list; suite_list++) {
+		unsigned int i;
+
+		for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++)
+			if (!strcmp(tls_cipher_suite_pref[i]->name,
+						*suite_list))
+				break;
+
+		if (i < L_ARRAY_SIZE(tls_cipher_suite_pref))
+			*suite++ = tls_cipher_suite_pref[i];
+		else
+			TLS_DEBUG("Cipher suite %s is not supported",
+					*suite_list);
+	}
+
+	if (suite > tls->cipher_suite_pref_list)
+		return true;
+
+	TLS_DEBUG("None of the supplied suite names is supported");
+	l_free(suite);
+	tls->cipher_suite_pref_list = NULL;
+	return false;
+}
+
 LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
 {
 	switch (desc) {
-- 
2.19.1

[PATCH 07/17] tls: Move cipher suite definitions to tls-suites.c

[Andrew Zaborowski]

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

Move the cipher suite-specific code to new file ell/tls-suites.c.
Unfortunately this requires a few functions and arrays to be exported
from tls.c.
---
 Makefile.am       |   1 +
 ell/tls-private.h |  25 +++
 ell/tls-suites.c  | 450 ++++++++++++++++++++++++++++++++++++++++++++
 ell/tls.c         | 465 ++--------------------------------------------
 4 files changed, 490 insertions(+), 451 deletions(-)
 create mode 100644 ell/tls-suites.c

diff --git a/Makefile.am b/Makefile.am
index 297c395..3eecbe0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -103,6 +103,7 @@ ell_libell_la_SOURCES = $(linux_headers) \
 			ell/tls.c \
 			ell/tls-record.c \
 			ell/tls-extensions.c \
+			ell/tls-suites.c \
 			ell/uuid.c \
 			ell/key.c \
 			ell/pkcs5-private.h \
diff --git a/ell/tls-private.h b/ell/tls-private.h
index 938c358..442dc02 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -56,6 +56,8 @@ struct tls_hash_algorithm {
 	const char *name;
 };
 
+extern const struct tls_hash_algorithm tls_handshake_hash_data[];
+
 typedef bool (*tls_get_hash_t)(struct l_tls *tls, uint8_t tls_id,
 				uint8_t *out, size_t *len,
 				enum l_checksum_type *type);
@@ -94,6 +96,8 @@ struct tls_cipher_suite {
 	enum l_checksum_type prf_hmac;
 };
 
+extern struct tls_cipher_suite *tls_cipher_suite_pref[];
+
 struct tls_compression_method {
 	int id;
 	const char *name;
@@ -145,6 +149,19 @@ enum tls_content_type {
 	TLS_CT_APPLICATION_DATA		= 23,
 };
 
+enum tls_handshake_type {
+	TLS_HELLO_REQUEST	= 0,
+	TLS_CLIENT_HELLO	= 1,
+	TLS_SERVER_HELLO	= 2,
+	TLS_CERTIFICATE		= 11,
+	TLS_SERVER_KEY_EXCHANGE	= 12,
+	TLS_CERTIFICATE_REQUEST	= 13,
+	TLS_SERVER_HELLO_DONE	= 14,
+	TLS_CERTIFICATE_VERIFY	= 15,
+	TLS_CLIENT_KEY_EXCHANGE	= 16,
+	TLS_FINISHED		= 20,
+};
+
 /*
  * Support the minimum required set of handshake hash types for the
  * Certificate Verify digital signature and the Finished PRF seed so we
@@ -269,9 +286,17 @@ void tls_tx_record(struct l_tls *tls, enum tls_content_type type,
 bool tls_handle_message(struct l_tls *tls, const uint8_t *message,
 			int len, enum tls_content_type type, uint16_t version);
 
+#define TLS_HANDSHAKE_HEADER_SIZE	4
+
+void tls_tx_handshake(struct l_tls *tls, int type, uint8_t *buf, size_t length);
+
 /* Optionally limit allowed cipher suites to a custom set */
 bool tls_set_cipher_suites(struct l_tls *tls, const char **suite_list);
 
+void tls_generate_master_secret(struct l_tls *tls,
+				const uint8_t *pre_master_secret,
+				int pre_master_secret_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
new file mode 100644
index 0000000..a3aed4c
--- /dev/null
+++ b/ell/tls-suites.c
@@ -0,0 +1,450 @@
+/*
+ *  Embedded Linux library
+ *
+ *  Copyright (C) 2018  Intel Corporation. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+
+#include "util.h"
+#include "tls.h"
+#include "cipher.h"
+#include "checksum.h"
+#include "cert.h"
+#include "tls-private.h"
+#include "key.h"
+#include "random.h"
+
+static bool tls_rsa_validate_cert_key(struct l_cert *cert)
+{
+	return l_cert_get_pubkey_type(cert) == L_CERT_KEY_RSA;
+}
+
+static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
+{
+	uint8_t buf[1024 + 32];
+	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
+	uint8_t pre_master_secret[48];
+	ssize_t bytes_encrypted;
+
+	if (!tls->peer_pubkey) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Peer public key not received");
+
+		return false;
+	}
+
+	/* Must match the version in tls_send_client_hello */
+	pre_master_secret[0] = (uint8_t) (TLS_VERSION >> 8);
+	pre_master_secret[1] = (uint8_t) (TLS_VERSION >> 0);
+
+	l_getrandom(pre_master_secret + 2, 46);
+
+	if (tls->peer_pubkey_size + 32 > (int) sizeof(buf)) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Peer public key too big: %zi",
+				tls->peer_pubkey_size);
+
+		return false;
+	}
+
+	l_put_be16(tls->peer_pubkey_size, ptr);
+	bytes_encrypted = l_key_encrypt(tls->peer_pubkey,
+					L_KEY_RSA_PKCS1_V1_5, L_CHECKSUM_NONE,
+					pre_master_secret, ptr + 2, 48,
+					tls->peer_pubkey_size);
+	ptr += tls->peer_pubkey_size + 2;
+
+	if (bytes_encrypted != (ssize_t) tls->peer_pubkey_size) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Encrypting PreMasterSecret failed: %s",
+				strerror(-bytes_encrypted));
+
+		return false;
+	}
+
+	tls_tx_handshake(tls, TLS_CLIENT_KEY_EXCHANGE, buf, ptr - buf);
+
+	tls_generate_master_secret(tls, pre_master_secret, 48);
+	memset(pre_master_secret, 0, 48);
+
+	return true;
+}
+
+static void tls_handle_rsa_client_key_xchg(struct l_tls *tls,
+						const uint8_t *buf, size_t len)
+{
+	uint8_t pre_master_secret[48], random_secret[46];
+	ssize_t bytes_decrypted;
+
+	if (!tls->priv_key || !tls->priv_key_size) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, TLS_ALERT_BAD_CERT,
+				"No private key");
+
+		return;
+	}
+
+	if (len != tls->priv_key_size + 2) {
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"ClientKeyExchange len %zi not %zi", len,
+				tls->priv_key_size + 2);
+
+		return;
+	}
+
+	len = l_get_be16(buf);
+
+	if (len != tls->priv_key_size) {
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"EncryptedPreMasterSecret len %zi not %zi",
+				len, tls->priv_key_size);
+
+		return;
+	}
+
+	bytes_decrypted = l_key_decrypt(tls->priv_key, L_KEY_RSA_PKCS1_V1_5,
+					L_CHECKSUM_NONE, buf + 2,
+					pre_master_secret, tls->priv_key_size,
+					48);
+
+	/*
+	 * Assume correct premaster secret client version which according
+	 * to the TLS1.2 spec is unlikely in client implementations SSLv3
+	 * and prior.  Spec suggests either not supporting them or adding
+	 * a configurable override for <= SSLv3 clients.  For now we have
+	 * no need to support them.
+	 *
+	 * On any decode error randomise the Pre Master Secret as per the
+	 * countermeasures in 7.4.7.1 and don't generate any alerts.
+	 */
+	l_getrandom(random_secret, 46);
+
+	pre_master_secret[0] = tls->client_version >> 8;
+	pre_master_secret[1] = tls->client_version >> 0;
+
+	if (bytes_decrypted != 48) {
+		memcpy(pre_master_secret + 2, random_secret, 46);
+
+		TLS_DEBUG("Error decrypting PreMasterSecret: %s",
+				strerror(-bytes_decrypted));
+	}
+
+	tls_generate_master_secret(tls, pre_master_secret, 48);
+	memset(pre_master_secret, 0, 48);
+	memset(random_secret, 0, 46);
+}
+
+static ssize_t tls_rsa_sign(struct l_tls *tls, uint8_t *out, size_t len,
+				tls_get_hash_t get_hash)
+{
+	ssize_t result = -EMSGSIZE;
+	enum l_checksum_type sign_checksum_type;
+	uint8_t sign_input[HANDSHAKE_HASH_MAX_SIZE + 36];
+	size_t sign_input_len;
+	uint8_t *ptr = out;
+
+	if (!tls->priv_key || !tls->priv_key_size) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, TLS_ALERT_BAD_CERT,
+				"No private key loaded");
+
+		return -ENOKEY;
+	}
+
+	if (tls->negotiated_version >= L_TLS_V12) {
+		const struct tls_hash_algorithm *hash_type =
+			&tls_handshake_hash_data[tls->signature_hash];
+
+		/* Build the DigitallySigned struct */
+		if (len < 2)	/* Is there space for the algorithm IDs */
+			goto error;
+
+		get_hash(tls, hash_type->tls_id, sign_input, NULL, NULL);
+		sign_checksum_type = hash_type->l_id;
+		sign_input_len = hash_type->length;
+
+		*ptr++ = hash_type->tls_id;
+		*ptr++ = 1;	/* RSA_sign */
+		len -= 2;
+	} else {
+		get_hash(tls, 1, sign_input + 0, NULL, NULL);	/* MD5 */
+		get_hash(tls, 2, sign_input + 16, NULL, NULL);	/* SHA1 */
+		sign_checksum_type = L_CHECKSUM_NONE;
+		sign_input_len = 36;
+	}
+
+	if (len < tls->priv_key_size + 2)
+		goto error;
+
+	l_put_be16(tls->priv_key_size, ptr);
+	result = l_key_sign(tls->priv_key, L_KEY_RSA_PKCS1_V1_5,
+				sign_checksum_type, sign_input, ptr + 2,
+				sign_input_len, tls->priv_key_size);
+	ptr += tls->priv_key_size + 2;
+
+	if (result == (ssize_t) tls->priv_key_size)
+		return ptr - out; /* Success */
+
+error:
+	TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+			"Signing the hash failed: %s",
+			strerror(-result));
+	return result;
+}
+
+static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
+				tls_get_hash_t get_hash)
+{
+	enum l_checksum_type hash_type;
+	uint8_t expected[HANDSHAKE_HASH_MAX_SIZE + 36];
+	size_t expected_len;
+	unsigned int offset;
+	bool success;
+
+	/* 2 bytes for SignatureAndHashAlgorithm if version >= 1.2 */
+	offset = 2;
+	if (tls->negotiated_version < L_TLS_V12)
+		offset = 0;
+
+	if (len < offset + 2 ||
+			(size_t) l_get_be16(in + offset) + offset + 2 != len) {
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0, "Signature msg too "
+				"short (%zi) or signature length doesn't match",
+				len);
+
+		return false;
+	}
+
+	/* Only the default hash type supported */
+	if (len != offset + 2 + tls->peer_pubkey_size) {
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"Signature length %zi not equal %zi", len,
+				offset + 2 + tls->peer_pubkey_size);
+
+		return false;
+	}
+
+	if (tls->negotiated_version >= L_TLS_V12) {
+		/* Only RSA supported */
+		if (in[1] != 1 /* RSA_sign */) {
+			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
+					"Unknown signature algorithm %i",
+					in[1]);
+
+			return false;
+		}
+
+		if (!get_hash(tls, in[0], expected, &expected_len,
+				&hash_type)) {
+			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
+					"Unknown hash type %i", in[0]);
+
+			return false;
+		}
+
+		/*
+		 * Note: Next we let the l_key_verify's underlying kernel
+		 * operation prepend the OID to the hash to build the
+		 * DigestInfo struct.  However according to 4.7 we need to
+		 * support at least two forms of the signed content in the
+		 * verification:
+		 *  - DigestInfo with NULL AlgorithmIdentifier.parameters,
+		 *  - DigestInfo with empty AlgorithmIdentifier.parameters,
+		 *
+		 * while the kernel only understands the former encoding.
+		 * Note PKCS#1 versions 2.0 and later section A.2.4 do
+		 * mandate NULL AlgorithmIdentifier.parameters.
+		 *
+		 * Additionally PKCS#1 v1.5 said BER is used in place of DER
+		 * for DigestInfo encoding which adds more ambiguity in the
+		 * encoding.
+		 */
+	} else {
+		get_hash(tls, 1, expected + 0, NULL, NULL);	/* MD5 */
+		get_hash(tls, 2, expected + 16, NULL, NULL);	/* SHA1 */
+		expected_len = 36;
+		hash_type = L_CHECKSUM_NONE;
+
+		/*
+		 * Note: Within the RSA padding for signatures PKCS#1 1.5
+		 * allows the block format to be either 0 or 1, while PKCS#1
+		 * v2.0+ mandates block type 1 making the signatures
+		 * unambiguous.  TLS 1.0 doesn't additionally specify which
+		 * block type is to be used (TLS 1.2 does) meaning that both
+		 * PKCS#1 v1.5 types are allowed.  The l_key_verify's
+		 * underlying kernel implementation only accepts block type
+		 * 1.  If this ever becomes an issue we'd need to go back to
+		 * using L_KEY_RSA_RAW and our own PKCS#1 v1.5 verify logic.
+		 */
+	}
+
+	success = l_key_verify(tls->peer_pubkey, L_KEY_RSA_PKCS1_V1_5,
+				hash_type, expected, in + offset + 2,
+				expected_len, tls->peer_pubkey_size);
+
+	if (!success)
+		TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
+				"Peer signature verification failed");
+	else
+		TLS_DEBUG("Peer signature verified");
+
+	return success;
+}
+
+static struct tls_key_exchange_algorithm tls_rsa = {
+	.id = 1, /* RSA_sign */
+	.certificate_check = true,
+	.validate_cert_key_type = tls_rsa_validate_cert_key,
+	.send_client_key_exchange = tls_send_rsa_client_key_xchg,
+	.handle_client_key_exchange = tls_handle_rsa_client_key_xchg,
+	.sign = tls_rsa_sign,
+	.verify = tls_rsa_verify,
+};
+
+static struct tls_bulk_encryption_algorithm tls_rc4 = {
+	.cipher_type = TLS_CIPHER_STREAM,
+	.l_id = L_CIPHER_ARC4,
+	.key_length = 16,
+}, tls_aes128 = {
+	.cipher_type = TLS_CIPHER_BLOCK,
+	.l_id = L_CIPHER_AES_CBC,
+	.key_length = 16,
+	.iv_length = 16,
+	.block_length = 16,
+}, tls_aes256 = {
+	.cipher_type = TLS_CIPHER_BLOCK,
+	.l_id = L_CIPHER_AES_CBC,
+	.key_length = 32,
+	.iv_length = 16,
+	.block_length = 16,
+}, tls_3des_ede = {
+	.cipher_type = TLS_CIPHER_BLOCK,
+	.l_id = L_CIPHER_DES3_EDE_CBC,
+	.key_length = 24,
+	.iv_length = 8,
+	.block_length = 8,
+}, tls_aes128_gcm = {
+	.cipher_type = TLS_CIPHER_AEAD,
+	.l_aead_id = L_AEAD_CIPHER_AES_GCM,
+	.key_length = 16,
+	.iv_length = 12,
+	.fixed_iv_length = 4,
+	.auth_tag_length = 16,
+}, tls_aes256_gcm = {
+	.cipher_type = TLS_CIPHER_AEAD,
+	.l_aead_id = L_AEAD_CIPHER_AES_GCM,
+	.key_length = 32,
+	.iv_length = 12,
+	.fixed_iv_length = 4,
+	.auth_tag_length = 16,
+};
+
+static struct tls_mac_algorithm tls_md5 = {
+	.id = 1,
+	.hmac_type = L_CHECKSUM_MD5,
+	.mac_length = 16,
+}, tls_sha = {
+	.id = 2,
+	.hmac_type = L_CHECKSUM_SHA1,
+	.mac_length = 20,
+}, tls_sha256 = {
+	.id = 4,
+	.hmac_type = L_CHECKSUM_SHA256,
+	.mac_length = 32,
+};
+
+static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
+	.id = { 0x00, 0x04 },
+	.name = "TLS_RSA_WITH_RC4_128_MD5",
+	.verify_data_length = 12,
+	.encryption = &tls_rc4,
+	.mac = &tls_md5,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_rc4_128_sha = {
+	.id = { 0x00, 0x05 },
+	.name = "TLS_RSA_WITH_RC4_128_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_rc4,
+	.mac = &tls_sha,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_3des_ede_cbc_sha = {
+	.id = { 0x00, 0x0a },
+	.name = "TLS_RSA_WITH_3DES_EDE_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_3des_ede,
+	.mac = &tls_sha,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_128_cbc_sha = {
+	.id = { 0x00, 0x2f },
+	.name = "TLS_RSA_WITH_AES_128_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128,
+	.mac = &tls_sha,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_256_cbc_sha = {
+	.id = { 0x00, 0x35 },
+	.name = "TLS_RSA_WITH_AES_256_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256,
+	.mac = &tls_sha,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_128_cbc_sha256 = {
+	.id = { 0x00, 0x3c },
+	.name = "TLS_RSA_WITH_AES_128_CBC_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128,
+	.mac = &tls_sha256,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_256_cbc_sha256 = {
+	.id = { 0x00, 0x3d },
+	.name = "TLS_RSA_WITH_AES_256_CBC_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256,
+	.mac = &tls_sha256,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_128_gcm_sha256 = {
+	.id = { 0x00, 0x9c },
+	.name = "TLS_RSA_WITH_AES_128_GCM_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128_gcm,
+	.key_xchg = &tls_rsa,
+}, tls_rsa_with_aes_256_gcm_sha384 = {
+	.id = { 0x00, 0x9d },
+	.name = "TLS_RSA_WITH_AES_256_GCM_SHA384",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256_gcm,
+	.prf_hmac = L_CHECKSUM_SHA384,
+	.key_xchg = &tls_rsa,
+};
+
+struct tls_cipher_suite *tls_cipher_suite_pref[] = {
+	&tls_rsa_with_aes_256_cbc_sha,
+	&tls_rsa_with_aes_128_cbc_sha,
+	&tls_rsa_with_aes_256_cbc_sha256,
+	&tls_rsa_with_aes_128_cbc_sha256,
+	&tls_rsa_with_aes_256_gcm_sha384,
+	&tls_rsa_with_aes_128_gcm_sha256,
+	&tls_rsa_with_3des_ede_cbc_sha,
+	&tls_rsa_with_rc4_128_sha,
+	&tls_rsa_with_rc4_128_md5,
+	NULL,
+};
diff --git a/ell/tls.c b/ell/tls.c
index 1154bcd..1910b92 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -378,158 +378,6 @@ static void tls_reset_cipher_spec(struct l_tls *tls, bool txrx)
 	tls_change_cipher_spec(tls, txrx, NULL);
 }
 
-static bool tls_send_rsa_client_key_xchg(struct l_tls *tls);
-static void tls_handle_rsa_client_key_xchg(struct l_tls *tls,
-						const uint8_t *buf, size_t len);
-
-static ssize_t tls_rsa_sign(struct l_tls *tls, uint8_t *out, size_t len,
-				tls_get_hash_t get_hash);
-static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
-				tls_get_hash_t get_hash);
-
-static bool tls_rsa_validate_cert_key(struct l_cert *cert)
-{
-	return l_cert_get_pubkey_type(cert) == L_CERT_KEY_RSA;
-}
-
-static struct tls_key_exchange_algorithm tls_rsa = {
-	.id = 1, /* RSA_sign */
-	.certificate_check = true,
-	.validate_cert_key_type = tls_rsa_validate_cert_key,
-	.send_client_key_exchange = tls_send_rsa_client_key_xchg,
-	.handle_client_key_exchange = tls_handle_rsa_client_key_xchg,
-	.sign = tls_rsa_sign,
-	.verify = tls_rsa_verify,
-};
-
-static struct tls_bulk_encryption_algorithm tls_rc4 = {
-	.cipher_type = TLS_CIPHER_STREAM,
-	.l_id = L_CIPHER_ARC4,
-	.key_length = 16,
-}, tls_aes128 = {
-	.cipher_type = TLS_CIPHER_BLOCK,
-	.l_id = L_CIPHER_AES_CBC,
-	.key_length = 16,
-	.iv_length = 16,
-	.block_length = 16,
-}, tls_aes256 = {
-	.cipher_type = TLS_CIPHER_BLOCK,
-	.l_id = L_CIPHER_AES_CBC,
-	.key_length = 32,
-	.iv_length = 16,
-	.block_length = 16,
-}, tls_3des_ede = {
-	.cipher_type = TLS_CIPHER_BLOCK,
-	.l_id = L_CIPHER_DES3_EDE_CBC,
-	.key_length = 24,
-	.iv_length = 8,
-	.block_length = 8,
-}, tls_aes128_gcm = {
-	.cipher_type = TLS_CIPHER_AEAD,
-	.l_aead_id = L_AEAD_CIPHER_AES_GCM,
-	.key_length = 16,
-	.iv_length = 12,
-	.fixed_iv_length = 4,
-	.auth_tag_length = 16,
-}, tls_aes256_gcm = {
-	.cipher_type = TLS_CIPHER_AEAD,
-	.l_aead_id = L_AEAD_CIPHER_AES_GCM,
-	.key_length = 32,
-	.iv_length = 12,
-	.fixed_iv_length = 4,
-	.auth_tag_length = 16,
-};
-
-static struct tls_mac_algorithm tls_md5 = {
-	.id = 1,
-	.hmac_type = L_CHECKSUM_MD5,
-	.mac_length = 16,
-}, tls_sha = {
-	.id = 2,
-	.hmac_type = L_CHECKSUM_SHA1,
-	.mac_length = 20,
-}, tls_sha256 = {
-	.id = 4,
-	.hmac_type = L_CHECKSUM_SHA256,
-	.mac_length = 32,
-};
-
-static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
-	.id = { 0x00, 0x04 },
-	.name = "TLS_RSA_WITH_RC4_128_MD5",
-	.verify_data_length = 12,
-	.encryption = &tls_rc4,
-	.mac = &tls_md5,
-	.key_xchg = &tls_rsa,
-}, tls_rsa_with_rc4_128_sha = {
-	.id = { 0x00, 0x05 },
-	.name = "TLS_RSA_WITH_RC4_128_SHA",
-	.verify_data_length = 12,
-	.encryption = &tls_rc4,
-	.mac = &tls_sha,
-	.key_xchg = &tls_rsa,
-}, tls_rsa_with_3des_ede_cbc_sha = {
-	.id = { 0x00, 0x0a },
-	.name = "TLS_RSA_WITH_3DES_EDE_CBC_SHA",
-	.verify_data_length = 12,
-	.encryption = &tls_3des_ede,
-	.mac = &tls_sha,
-	.key_xchg = &tls_rsa,
-}, tls_rsa_with_aes_128_cbc_sha = {
-	.id = { 0x00, 0x2f },
-	.name = "TLS_RSA_WITH_AES_128_CBC_SHA",
-	.verify_data_length = 12,
-	.encryption = &tls_aes128,
-	.mac = &tls_sha,
-	.key_xchg = &tls_rsa,
-}, tls_rsa_with_aes_256_cbc_sha = {
-	.id = { 0x00, 0x35 },
-	.name = "TLS_RSA_WITH_AES_256_CBC_SHA",
-	.verify_data_length = 12,
-	.encryption = &tls_aes256,
-	.mac = &tls_sha,
-	.key_xchg = &tls_rsa,
-}, tls_rsa_with_aes_128_cbc_sha256 = {
-	.id = { 0x00, 0x3c },
-	.name = "TLS_RSA_WITH_AES_128_CBC_SHA256",
-	.verify_data_length = 12,
-	.encryption = &tls_aes128,
-	.mac = &tls_sha256,
-	.key_xchg = &tls_rsa,
-}, tls_rsa_with_aes_256_cbc_sha256 = {
-	.id = { 0x00, 0x3d },
-	.name = "TLS_RSA_WITH_AES_256_CBC_SHA256",
-	.verify_data_length = 12,
-	.encryption = &tls_aes256,
-	.mac = &tls_sha256,
-	.key_xchg = &tls_rsa,
-}, tls_rsa_with_aes_128_gcm_sha256 = {
-	.id = { 0x00, 0x9c },
-	.name = "TLS_RSA_WITH_AES_128_GCM_SHA256",
-	.verify_data_length = 12,
-	.encryption = &tls_aes128_gcm,
-	.key_xchg = &tls_rsa,
-}, tls_rsa_with_aes_256_gcm_sha384 = {
-	.id = { 0x00, 0x9d },
-	.name = "TLS_RSA_WITH_AES_256_GCM_SHA384",
-	.verify_data_length = 12,
-	.encryption = &tls_aes256_gcm,
-	.prf_hmac = L_CHECKSUM_SHA384,
-	.key_xchg = &tls_rsa,
-};
-
-static struct tls_cipher_suite *tls_cipher_suite_pref[] = {
-	&tls_rsa_with_aes_256_cbc_sha,
-	&tls_rsa_with_aes_128_cbc_sha,
-	&tls_rsa_with_aes_256_cbc_sha256,
-	&tls_rsa_with_aes_128_cbc_sha256,
-	&tls_rsa_with_aes_256_gcm_sha384,
-	&tls_rsa_with_aes_128_gcm_sha256,
-	&tls_rsa_with_3des_ede_cbc_sha,
-	&tls_rsa_with_rc4_128_sha,
-	&tls_rsa_with_rc4_128_md5,
-};
-
 static bool tls_cipher_suite_is_compatible(struct l_tls *tls,
 					const struct tls_cipher_suite *suite,
 					const char **error)
@@ -629,12 +477,11 @@ static bool tls_cipher_suite_is_compatible(struct l_tls *tls,
 
 static struct tls_cipher_suite *tls_find_cipher_suite(const uint8_t *id)
 {
-	int i;
+	struct tls_cipher_suite **suite;
 
-	for (i = 0; i < (int) L_ARRAY_SIZE(tls_cipher_suite_pref); i++)
-		if (tls_cipher_suite_pref[i]->id[0] == id[0] &&
-				tls_cipher_suite_pref[i]->id[1] == id[1])
-			return tls_cipher_suite_pref[i];
+	for (suite = tls_cipher_suite_pref; *suite; suite++)
+		if ((*suite)->id[0] == id[0] && (*suite)->id[1] == id[1])
+			return *suite;
 
 	return NULL;
 }
@@ -658,7 +505,7 @@ static struct tls_compression_method *tls_find_compression_method(
 	return NULL;
 }
 
-static const struct tls_hash_algorithm tls_handshake_hash_data[] = {
+const struct tls_hash_algorithm tls_handshake_hash_data[] = {
 	[HANDSHAKE_HASH_SHA384]	= { 5, L_CHECKSUM_SHA384, 48, "SHA384" },
 	[HANDSHAKE_HASH_SHA256]	= { 4, L_CHECKSUM_SHA256, 32, "SHA256" },
 	[HANDSHAKE_HASH_MD5]	= { 1, L_CHECKSUM_MD5, 16, "MD5" },
@@ -713,19 +560,6 @@ static const struct tls_hash_algorithm *tls_set_prf_hmac(struct l_tls *tls)
 	return NULL;
 }
 
-enum tls_handshake_type {
-	TLS_HELLO_REQUEST	= 0,
-	TLS_CLIENT_HELLO	= 1,
-	TLS_SERVER_HELLO	= 2,
-	TLS_CERTIFICATE		= 11,
-	TLS_SERVER_KEY_EXCHANGE	= 12,
-	TLS_CERTIFICATE_REQUEST	= 13,
-	TLS_SERVER_HELLO_DONE	= 14,
-	TLS_CERTIFICATE_VERIFY	= 15,
-	TLS_CLIENT_KEY_EXCHANGE	= 16,
-	TLS_FINISHED		= 20,
-};
-
 #define SWITCH_ENUM_TO_STR(val) \
 	case (val):		\
 		return L_STRINGIFY(val);
@@ -789,10 +623,7 @@ void tls_disconnect(struct l_tls *tls, enum l_tls_alert_desc desc,
 				tls->user_data);
 }
 
-#define TLS_HANDSHAKE_HEADER_SIZE	4
-
-static void tls_tx_handshake(struct l_tls *tls, int type, uint8_t *buf,
-				size_t length)
+void tls_tx_handshake(struct l_tls *tls, int type, uint8_t *buf, size_t length)
 {
 	int i;
 
@@ -816,8 +647,7 @@ static void tls_tx_handshake(struct l_tls *tls, int type, uint8_t *buf,
 
 static bool tls_send_client_hello(struct l_tls *tls)
 {
-	uint8_t buf[1024 + L_ARRAY_SIZE(tls_compression_pref) +
-			2 * L_ARRAY_SIZE(tls_cipher_suite_pref)];
+	uint8_t buf[1024 + L_ARRAY_SIZE(tls_compression_pref)];
 	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
 	uint8_t *len_ptr;
 	unsigned int i;
@@ -1177,9 +1007,9 @@ static void tls_send_server_hello_done(struct l_tls *tls)
 				TLS_HANDSHAKE_HEADER_SIZE);
 }
 
-static void tls_generate_master_secret(struct l_tls *tls,
-					const uint8_t *pre_master_secret,
-					int pre_master_secret_len)
+void tls_generate_master_secret(struct l_tls *tls,
+				const uint8_t *pre_master_secret,
+				int pre_master_secret_len)
 {
 	uint8_t seed[64];
 	int key_block_size;
@@ -1224,211 +1054,6 @@ static void tls_generate_master_secret(struct l_tls *tls,
 	memset(seed, 0, 64);
 }
 
-static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
-{
-	uint8_t buf[1024 + 32];
-	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
-	uint8_t pre_master_secret[48];
-	ssize_t bytes_encrypted;
-
-	if (!tls->peer_pubkey) {
-		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
-				"Peer public key not received");
-
-		return false;
-	}
-
-	pre_master_secret[0] = (uint8_t) (TLS_VERSION >> 8);
-	pre_master_secret[1] = (uint8_t) (TLS_VERSION >> 0);
-	l_getrandom(pre_master_secret + 2, 46);
-
-	if (tls->peer_pubkey_size + 32 > (int) sizeof(buf)) {
-		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
-				"Peer public key too big: %zi",
-				tls->peer_pubkey_size);
-
-		return false;
-	}
-
-	l_put_be16(tls->peer_pubkey_size, ptr);
-	bytes_encrypted = l_key_encrypt(tls->peer_pubkey,
-					L_KEY_RSA_PKCS1_V1_5, L_CHECKSUM_NONE,
-					pre_master_secret, ptr + 2, 48,
-					tls->peer_pubkey_size);
-	ptr += tls->peer_pubkey_size + 2;
-
-	if (bytes_encrypted != (ssize_t) tls->peer_pubkey_size) {
-		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
-				"Encrypting PreMasterSecret failed: %s",
-				strerror(-bytes_encrypted));
-
-		return false;
-	}
-
-	tls_tx_handshake(tls, TLS_CLIENT_KEY_EXCHANGE, buf, ptr - buf);
-
-	tls_generate_master_secret(tls, pre_master_secret, 48);
-	memset(pre_master_secret, 0, 48);
-
-	return true;
-}
-
-static ssize_t tls_rsa_sign(struct l_tls *tls, uint8_t *out, size_t len,
-				tls_get_hash_t get_hash)
-{
-	ssize_t result = -EMSGSIZE;
-	enum l_checksum_type sign_checksum_type;
-	uint8_t sign_input[HANDSHAKE_HASH_MAX_SIZE + 36];
-	size_t sign_input_len;
-	uint8_t *ptr = out;
-
-	if (!tls->priv_key || !tls->priv_key_size) {
-		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, TLS_ALERT_BAD_CERT,
-				"No private key loaded");
-
-		return -ENOKEY;
-	}
-
-	if (tls->negotiated_version >= L_TLS_V12) {
-		const struct tls_hash_algorithm *hash_type =
-			&tls_handshake_hash_data[tls->signature_hash];
-
-		/* Build the DigitallySigned struct */
-		if (len < 2)	/* Is there space for the algorithm IDs */
-			goto error;
-
-		get_hash(tls, hash_type->tls_id, sign_input, NULL, NULL);
-		sign_checksum_type = hash_type->l_id;
-		sign_input_len = hash_type->length;
-
-		*ptr++ = hash_type->tls_id;
-		*ptr++ = 1;	/* RSA_sign */
-		len -= 2;
-	} else {
-		get_hash(tls, 1, sign_input + 0, NULL, NULL);	/* MD5 */
-		get_hash(tls, 2, sign_input + 16, NULL, NULL);	/* SHA1 */
-		sign_checksum_type = L_CHECKSUM_NONE;
-		sign_input_len = 36;
-	}
-
-	if (len < tls->priv_key_size + 2)
-		goto error;
-
-	l_put_be16(tls->priv_key_size, ptr);
-	result = l_key_sign(tls->priv_key, L_KEY_RSA_PKCS1_V1_5,
-				sign_checksum_type, sign_input, ptr + 2,
-				sign_input_len, tls->priv_key_size);
-	ptr += tls->priv_key_size + 2;
-
-	if (result == (ssize_t) tls->priv_key_size)
-		return ptr - out; /* Success */
-
-error:
-	TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
-			"Signing the hash failed: %s",
-			strerror(-result));
-	return result;
-}
-
-static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
-				tls_get_hash_t get_hash)
-{
-	enum l_checksum_type hash_type;
-	uint8_t expected[HANDSHAKE_HASH_MAX_SIZE + 36];
-	size_t expected_len;
-	unsigned int offset;
-	bool success;
-
-	/* 2 bytes for SignatureAndHashAlgorithm if version >= 1.2 */
-	offset = 2;
-	if (tls->negotiated_version < L_TLS_V12)
-		offset = 0;
-
-	if (len < offset + 2 ||
-			(size_t) l_get_be16(in + offset) + offset + 2 != len) {
-		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0, "Signature msg too "
-				"short (%zi) or signature length doesn't match",
-				len);
-
-		return false;
-	}
-
-	/* Only the default hash type supported */
-	if (len != offset + 2 + tls->peer_pubkey_size) {
-		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
-				"Signature length %zi not equal %zi", len,
-				offset + 2 + tls->peer_pubkey_size);
-
-		return false;
-	}
-
-	if (tls->negotiated_version >= L_TLS_V12) {
-		/* Only RSA supported */
-		if (in[1] != 1 /* RSA_sign */) {
-			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
-					"Unknown signature algorithm %i",
-					in[1]);
-
-			return false;
-		}
-
-		if (!get_hash(tls, in[0], expected, &expected_len,
-				&hash_type)) {
-			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
-					"Unknown hash type %i", in[0]);
-
-			return false;
-		}
-
-		/*
-		 * Note: Next we let the l_key_verify's underlying kernel
-		 * operation prepend the OID to the hash to build the
-		 * DigestInfo struct.  However according to 4.7 we need to
-		 * support at least two forms of the signed content in the
-		 * verification:
-		 *  - DigestInfo with NULL AlgorithmIdentifier.parameters,
-		 *  - DigestInfo with empty AlgorithmIdentifier.parameters,
-		 *
-		 * while the kernel only understands the former encoding.
-		 * Note PKCS#1 versions 2.0 and later section A.2.4 do
-		 * mandate NULL AlgorithmIdentifier.parameters.
-		 *
-		 * Additionally PKCS#1 v1.5 said BER is used in place of DER
-		 * for DigestInfo encoding which adds more ambiguity in the
-		 * encoding.
-		 */
-	} else {
-		get_hash(tls, 1, expected + 0, NULL, NULL);	/* MD5 */
-		get_hash(tls, 2, expected + 16, NULL, NULL);	/* SHA1 */
-		expected_len = 36;
-		hash_type = L_CHECKSUM_NONE;
-
-		/*
-		 * Note: Within the RSA padding for signatures PKCS#1 1.5
-		 * allows the block format to be either 0 or 1, while PKCS#1
-		 * v2.0+ mandates block type 1 making the signatures
-		 * unambiguous.  TLS 1.0 doesn't additionally specify which
-		 * block type is to be used (TLS 1.2 does) meaning that both
-		 * PKCS#1 v1.5 types are allowed.  The l_key_verify's
-		 * underlying kernel implementation only accepts block type
-		 * 1.  If this ever becomes an issue we'd need to go back to
-		 * using L_KEY_RSA_RAW and our own PKCS#1 v1.5 verify logic.
-		 */
-	}
-
-	success = l_key_verify(tls->peer_pubkey, L_KEY_RSA_PKCS1_V1_5,
-				hash_type, expected, in + offset + 2,
-				expected_len, tls->peer_pubkey_size);
-
-	if (!success)
-		TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
-				"Peer signature verification failed");
-	else
-		TLS_DEBUG("Peer signature verified");
-
-	return success;
-}
-
 static void tls_get_handshake_hash(struct l_tls *tls,
 					enum handshake_hash_type type,
 					uint8_t *out)
@@ -2200,7 +1825,8 @@ static void tls_handle_certificate_request(struct l_tls *tls,
 			hash_id = signature_hash_data[i + 0];
 
 			/* Ignore hash types for signatures other than ours */
-			if (signature_hash_data[i + 1] != tls_rsa.id)
+			if (signature_hash_data[i + 1] !=
+					tls->pending.cipher_suite->key_xchg->id)
 				continue;
 
 			if (hash_id == tls_handshake_hash_data[
@@ -2287,69 +1913,6 @@ static void tls_handle_server_hello_done(struct l_tls *tls,
 	TLS_SET_STATE(TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC);
 }
 
-static void tls_handle_rsa_client_key_xchg(struct l_tls *tls,
-						const uint8_t *buf, size_t len)
-{
-	uint8_t pre_master_secret[48], random_secret[46];
-	ssize_t bytes_decrypted;
-
-	if (!tls->priv_key || !tls->priv_key_size) {
-		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, TLS_ALERT_BAD_CERT,
-				"No private key");
-
-		return;
-	}
-
-	if (len != tls->priv_key_size + 2) {
-		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
-				"ClientKeyExchange len %zi not %zi", len,
-				tls->priv_key_size + 2);
-
-		return;
-	}
-
-	len = l_get_be16(buf);
-
-	if (len != tls->priv_key_size) {
-		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
-				"EncryptedPreMasterSecret len %zi not %zi",
-				len, tls->priv_key_size);
-
-		return;
-	}
-
-	bytes_decrypted = l_key_decrypt(tls->priv_key, L_KEY_RSA_PKCS1_V1_5,
-					L_CHECKSUM_NONE, buf + 2,
-					pre_master_secret, tls->priv_key_size,
-					48);
-
-	/*
-	 * Assume correct premaster secret client version which according
-	 * to the TLS1.2 spec is unlikely in client implementations SSLv3
-	 * and prior.  Spec suggests either not supporting them or adding
-	 * a configurable override for <= SSLv3 clients.  For now we have
-	 * no need to support them.
-	 *
-	 * On any decode error randomise the Pre Master Secret as per the
-	 * countermeasures in 7.4.7.1 and don't generate any alerts.
-	 */
-	l_getrandom(random_secret, 46);
-
-	pre_master_secret[0] = tls->client_version >> 8;
-	pre_master_secret[1] = tls->client_version >> 0;
-
-	if (bytes_decrypted != 48) {
-		memcpy(pre_master_secret + 2, random_secret, 46);
-
-		TLS_DEBUG("Error decrypting PreMasterSecret: %s",
-				strerror(-bytes_decrypted));
-	}
-
-	tls_generate_master_secret(tls, pre_master_secret, 48);
-	memset(pre_master_secret, 0, 48);
-	memset(random_secret, 0, 46);
-}
-
 static bool tls_get_prev_digest_by_id(struct l_tls *tls, uint8_t hash_id,
 					uint8_t *out, size_t *out_len,
 					enum l_checksum_type *type)
@@ -3043,12 +2606,12 @@ bool tls_set_cipher_suites(struct l_tls *tls, const char **suite_list)
 	for (; *suite_list; suite_list++) {
 		unsigned int i;
 
-		for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++)
+		for (i = 0; tls_cipher_suite_pref[i]; i++)
 			if (!strcmp(tls_cipher_suite_pref[i]->name,
 						*suite_list))
 				break;
 
-		if (i < L_ARRAY_SIZE(tls_cipher_suite_pref))
+		if (tls_cipher_suite_pref[i])
 			*suite++ = tls_cipher_suite_pref[i];
 		else
 			TLS_DEBUG("Cipher suite %s is not supported",
-- 
2.19.1

[PATCH 08/17] tls: Add ServerKeyExchange callbacks to key exchange struct

[Andrew Zaborowski]

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

TLS Key Exchange algorithms using ephemeral keys (DHE_* and ECDHE_*)
need to use the ServerKeyExchange message during the handshake unlike
our current RSA key exchanhe, provide callbacks for sending and receiving
those messages.
---
 ell/tls-private.h |  4 ++++
 ell/tls.c         | 61 +++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/ell/tls-private.h b/ell/tls-private.h
index 442dc02..375af2b 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -69,6 +69,10 @@ struct tls_key_exchange_algorithm {
 
 	bool (*validate_cert_key_type)(struct l_cert *cert);
 
+	bool (*send_server_key_exchange)(struct l_tls *tls);
+	void (*handle_server_key_exchange)(struct l_tls *tls,
+						const uint8_t *buf, size_t len);
+
 	bool (*send_client_key_exchange)(struct l_tls *tls);
 	void (*handle_client_key_exchange)(struct l_tls *tls,
 						const uint8_t *buf, size_t len);
diff --git a/ell/tls.c b/ell/tls.c
index 1910b92..0aa04a1 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -1517,6 +1517,11 @@ static void tls_handle_client_hello(struct l_tls *tls,
 		if (!tls_send_certificate(tls))
 			return;
 
+	if (tls->pending.cipher_suite->key_xchg->send_server_key_exchange)
+		if (!tls->pending.cipher_suite->key_xchg->
+				send_server_key_exchange(tls))
+			return;
+
 	/* TODO: don't bother if configured to not authenticate client */
 	if (tls->pending.cipher_suite->key_xchg->certificate_check &&
 			tls->ca_certs)
@@ -1757,7 +1762,8 @@ static void tls_handle_certificate(struct l_tls *tls,
 
 	tls->peer_pubkey_size /= 8;
 
-	if (tls->server)
+	if (tls->server || tls->pending.cipher_suite->key_xchg->
+			handle_server_key_exchange)
 		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_KEY_EXCHANGE);
 	else
 		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO_DONE);
@@ -2134,6 +2140,27 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 
 		break;
 
+	case TLS_SERVER_KEY_EXCHANGE:
+		if (tls->server) {
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in server mode");
+			break;
+		}
+
+		if (tls->state != TLS_HANDSHAKE_WAIT_KEY_EXCHANGE) {
+			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
+					"Message invalid in state %s",
+					tls_handshake_state_to_str(tls->state));
+			break;
+		}
+
+		tls->pending.cipher_suite->key_xchg->handle_server_key_exchange(
+								tls, buf, len);
+
+		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO_DONE);
+
+		break;
+
 	case TLS_CERTIFICATE_REQUEST:
 		if (tls->server) {
 			TLS_DISCONNECT(TLS_ALERT_UNEXPECTED_MESSAGE, 0,
@@ -2240,19 +2267,33 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 		}
 
 		/*
-		 * On the client, the server's certificate is only now
-		 * verified, based on the following logic:
+		 * On the client, the server's certificate is now verified
+		 * regardless of the key exchange method, based on the
+		 * following logic:
+		 *
 		 *  - tls->ca_certs is non-NULL so tls_handle_certificate
 		 *    (always called on the client) must have veritifed the
 		 *    server's certificate chain to be valid and additionally
 		 *    trusted by our CA.
-		 *  - the correct receival of this Finished message confirms
-		 *    that the peer owns the end-entity certificate because
-		 *    it was able to decrypt the master secret which we had
-		 *    encrypted with the public key from that certificate, and
-		 *    the posession of the master secret in turn is verified
-		 *    by both the successful decryption and the MAC of this
-		 *    message (either should be enough).
+		 *
+		 *  - the peer owns the end-entity certificate because:
+		 *    either:
+		 *
+		 *    * (RSA key exchange algorithm case) the correct
+		 *      receival of this Finished message confirms the
+		 *      posession of the master secret, it is verified by
+		 *      both the successful decryption and the MAC of this
+		 *      message (either should be enough) because we entered
+		 *      the TLS_HANDSHAKE_WAIT_FINISHED state only after
+		 *      encryption and MAC were enabled in ChangeCipherSpec.
+		 *      To obtain the master secret the server must have been
+		 *      able to decrypt the pre_master_secret which we had
+		 *      encrypted with the public key from that certificate.
+		 *
+		 *    * (ECDHE and DHE key exchange algorithms) server was
+		 *      able to sign the client random together with the
+		 *      ServerKeyExchange parameters using its certified key
+		 *      pair.
 		 */
 		if (!tls->server && tls->cipher_suite[0]->key_xchg->
 				certificate_check && tls->ca_certs)
-- 
2.19.1

[PATCH 09/17] tls: ECHDE_RSA key exchange implementation client side

[Andrew Zaborowski]

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

Add a new key exchange definition with the callbacks needed to handle the
client side of the ECDHE_RSA key exchange, defined in RFC 8422.  Key
Exchange method-specific state is kept in a struct allocated by the
method and pointed to by a void pointer in the l_tls struct so that the
internals of the method are kept in tls-suites.c
---
 ell/tls-extensions.c |  11 ++
 ell/tls-private.h    |   5 +
 ell/tls-suites.c     | 246 +++++++++++++++++++++++++++++++++++++++++++
 ell/tls.c            |   4 +
 4 files changed, 266 insertions(+)

diff --git a/ell/tls-extensions.c b/ell/tls-extensions.c
index 3d75037..be2fa10 100644
--- a/ell/tls-extensions.c
+++ b/ell/tls-extensions.c
@@ -148,3 +148,14 @@ const struct tls_hello_extension tls_extensions[] = {
 	},
 	{}
 };
+
+const struct tls_named_curve *tls_find_curve(uint16_t id)
+{
+	unsigned int i;
+
+	for (i = 0; i < L_ARRAY_SIZE(tls_curve_pref); i++)
+		if (tls_curve_pref[i].id == id)
+			return &tls_curve_pref[i];
+
+	return NULL;
+}
diff --git a/ell/tls-private.h b/ell/tls-private.h
index 375af2b..0ed3eee 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -81,6 +81,8 @@ struct tls_key_exchange_algorithm {
 			tls_get_hash_t get_hash);
 	bool (*verify)(struct l_tls *tls, const uint8_t *in, size_t len,
 			tls_get_hash_t get_hash);
+
+	void (*free_params)(struct l_tls *tls);
 };
 
 struct tls_mac_algorithm {
@@ -246,6 +248,7 @@ struct l_tls {
 		 * 6.3 v1.2 + two IVs of 32 bytes.
 		 */
 		uint8_t key_block[192];
+		void *key_xchg_params;
 	} pending;
 
 	enum tls_cipher_type cipher_type[2];
@@ -301,6 +304,8 @@ void tls_generate_master_secret(struct l_tls *tls,
 				const uint8_t *pre_master_secret,
 				int pre_master_secret_len);
 
+const struct tls_named_curve *tls_find_curve(uint16_t id);
+
 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 a3aed4c..da31982 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -34,6 +34,8 @@
 #include "tls-private.h"
 #include "key.h"
 #include "random.h"
+#include "ecc.h"
+#include "ecdh.h"
 
 static bool tls_rsa_validate_cert_key(struct l_cert *cert)
 {
@@ -320,6 +322,250 @@ static struct tls_key_exchange_algorithm tls_rsa = {
 	.verify = tls_rsa_verify,
 };
 
+struct tls_ecdhe_params {
+	const struct l_ecc_curve *curve;
+	struct l_ecc_scalar *private;
+	struct l_ecc_point *public;
+};
+
+static void tls_free_ecdhe_params(struct l_tls *tls)
+{
+	struct tls_ecdhe_params *params = tls->pending.key_xchg_params;
+
+	if (!params)
+		return;
+
+	tls->pending.key_xchg_params = NULL;
+
+	l_ecc_scalar_free(params->private);
+	l_ecc_point_free(params->public);
+	l_free(params);
+}
+
+static size_t tls_write_ecpoint(uint8_t *buf,
+				const struct tls_named_curve *curve,
+				const struct l_ecc_point *point)
+{
+	/* RFC 8422, Section 5.4.1 */
+	buf[0] = 1 + curve->point_bytes;	/* length */
+	buf[1] = 4;				/* form: uncompressed */
+	return 2 + l_ecc_point_get_data(point, buf + 2, curve->point_bytes);
+}
+
+static size_t tls_write_server_ecdh_params(struct l_tls *tls, uint8_t *buf)
+{
+	struct tls_ecdhe_params *params = tls->pending.key_xchg_params;
+
+	/* RFC 8422, Section 5.4 */
+	buf[0] = 3;				/* curve_type: named_curve */
+	l_put_be16(tls->negotiated_curve->id, buf + 1);
+	return 3 + tls_write_ecpoint(buf + 3, tls->negotiated_curve,
+					params->public);
+}
+
+static bool tls_get_server_ecdh_params_hash(struct l_tls *tls, uint8_t tls_id,
+						uint8_t *out, size_t *len,
+						enum l_checksum_type *type)
+{
+	unsigned int hash;
+	struct l_checksum *checksum;
+	uint8_t params[1024];
+	size_t params_len;
+	ssize_t hash_len, ret;
+
+	params_len = tls_write_server_ecdh_params(tls, params);
+
+	for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
+		if (tls_handshake_hash_data[hash].tls_id == tls_id)
+			break;
+
+	if (hash == __HANDSHAKE_HASH_COUNT)
+		return false;
+
+	hash_len = tls_handshake_hash_data[hash].length;
+
+	checksum = l_checksum_new(tls_handshake_hash_data[hash].l_id);
+	if (!checksum)
+		return false;
+
+	/*
+	 * The ServerKeyExchange signature hash input format for RSA_sign is
+	 * not really specified in either RFC 8422 or RFC 5246 explicitly
+	 * but we use this format by analogy to DHE_RSA which uses RSA_sign
+	 * as well.  Also matches ecdsa, ed25519 and ed448 formats.
+	 */
+	l_checksum_update(checksum, tls->pending.client_random, 32);
+	l_checksum_update(checksum, tls->pending.server_random, 32);
+	l_checksum_update(checksum, params, params_len);
+	ret = l_checksum_get_digest(checksum, out, hash_len);
+	l_checksum_free(checksum);
+
+	if (ret != (ssize_t) hash_len)
+		return false;
+
+	if (len)
+		*len = hash_len;
+
+	if (type)
+		*type = tls_handshake_hash_data[hash].l_id;
+
+	return true;
+}
+
+static void tls_handle_ecdhe_server_key_xchg(struct l_tls *tls,
+						const uint8_t *buf, size_t len)
+{
+	struct tls_ecdhe_params *params;
+	uint16_t namedcurve;
+
+	/* RFC 8422, Section 5.4 */
+
+	if (len < 5)
+		goto decode_error;
+
+	if (*buf != 3) {
+		TLS_DISCONNECT(TLS_ALERT_ILLEGAL_PARAM, 0,
+				"Unsupported (deprecated?) ECCurveType %u",
+				*buf);
+		return;
+	}
+
+	namedcurve = l_get_be16(buf + 1);
+	buf += 3;
+	len -= 3;
+
+	tls->negotiated_curve = tls_find_curve(namedcurve);
+
+	if (!tls->negotiated_curve) {
+		TLS_DISCONNECT(TLS_ALERT_ILLEGAL_PARAM, 0,
+				"Unsupported NamedCurve %u", namedcurve);
+		return;
+	}
+
+	TLS_DEBUG("Negotiated %s", tls->negotiated_curve->name);
+
+	if (*buf++ != 1 + tls->negotiated_curve->point_bytes)
+		goto decode_error;
+
+	if (*buf != 4) {	/* uncompressed */
+		TLS_DISCONNECT(TLS_ALERT_ILLEGAL_PARAM, 0,
+				"Unsupported (deprecated?) PointConversionForm "
+				"%u", *buf);
+		return;
+	}
+
+	buf++;
+	len -= 2;
+
+	if (len < tls->negotiated_curve->point_bytes)
+		goto decode_error;
+
+	/*
+	 * RFC 8422, Section 5.11: "A receiving party MUST check that the
+	 * x and y parameters from the peer's public value satisfy the
+	 * curve equation, y^2 = x^3 + ax + b mod p."
+	 * This happens in l_ecc_point_from_data when the L_ECC_POINT_TYPE_FULL
+	 * format is used.
+	 */
+	params = l_new(struct tls_ecdhe_params, 1);
+	params->curve = l_ecc_curve_get(tls->negotiated_curve->l_group);
+	params->public = l_ecc_point_from_data(params->curve,
+						L_ECC_POINT_TYPE_FULL,
+						buf, len);
+	tls->pending.key_xchg_params = params;
+	buf += tls->negotiated_curve->point_bytes;
+	len -= tls->negotiated_curve->point_bytes;
+
+	if (!params->public) {
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"ServerKeyExchange.params.public decode error");
+		return;
+	}
+
+	if (!tls->pending.cipher_suite->key_xchg->verify(tls, buf, len,
+					tls_get_server_ecdh_params_hash))
+		return;
+
+	TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO_DONE);
+
+	return;
+
+decode_error:
+	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+			"ServerKeyExchange decode error");
+}
+
+static bool tls_send_ecdhe_client_key_xchg(struct l_tls *tls)
+{
+	uint8_t buf[1024];
+	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
+	uint8_t pre_master_secret[128];
+	ssize_t pre_master_secret_len;
+	struct tls_ecdhe_params *params = tls->pending.key_xchg_params;
+	struct l_ecc_point *our_public;
+	struct l_ecc_scalar *secret;
+
+	/* RFC 8422, Section 5.7 */
+
+	if (!l_ecdh_generate_key_pair(params->curve,
+					&params->private, &our_public)) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Generating ECDH key pair failed");
+		return false;
+	}
+
+	ptr += tls_write_ecpoint(ptr, tls->negotiated_curve, our_public);
+	l_ecc_point_free(our_public);
+
+	/*
+	 * Neither 5.4 or 5.7 "Actions" paragraphs say when the ECDH shared
+	 * secret is calculated but we can either do this in
+	 * tls_handle_ecdhe_server_key_xchg or here.  In both cases we only
+	 * need to store the public key in the client's key_xchg_params and
+	 * can free all of the params after sending the ClientKeyExchange.
+	 * By doing this calculation here we're aligned with RSA and also
+	 * with the server mode where the shared secret can only be
+	 * calculated after the ClientKeyExchange is received.
+	 */
+	if (!l_ecdh_generate_shared_secret(params->private, params->public,
+						&secret)) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Generating ECDH shared-secret failed");
+		return false;
+	}
+
+	tls_free_ecdhe_params(tls);
+	pre_master_secret_len = l_ecc_scalar_get_data(secret,
+						pre_master_secret,
+						sizeof(pre_master_secret));
+	l_ecc_scalar_free(secret);
+
+	if (pre_master_secret_len < 0) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"l_ecc_scalar_get_data(secret) failed");
+		return false;
+	}
+
+	tls_tx_handshake(tls, TLS_CLIENT_KEY_EXCHANGE, buf, ptr - buf);
+
+	tls_generate_master_secret(tls, pre_master_secret,
+					pre_master_secret_len);
+	memset(pre_master_secret, 0, pre_master_secret_len);
+
+	return true;
+}
+
+static struct tls_key_exchange_algorithm tls_ecdhe_rsa = {
+	.id = 1, /* RSA_sign */
+	.certificate_check = true,
+	.validate_cert_key_type = tls_rsa_validate_cert_key,
+	.handle_server_key_exchange = tls_handle_ecdhe_server_key_xchg,
+	.send_client_key_exchange = tls_send_ecdhe_client_key_xchg,
+	.free_params = tls_free_ecdhe_params,
+	.sign = tls_rsa_sign,
+	.verify = tls_rsa_verify,
+};
+
 static struct tls_bulk_encryption_algorithm tls_rc4 = {
 	.cipher_type = TLS_CIPHER_STREAM,
 	.l_id = L_CIPHER_ARC4,
diff --git a/ell/tls.c b/ell/tls.c
index 0aa04a1..df827f3 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -186,6 +186,10 @@ static void tls_reset_handshake(struct l_tls *tls)
 
 	memset(tls->pending.key_block, 0, sizeof(tls->pending.key_block));
 
+	if (tls->pending.cipher_suite &&
+			tls->pending.cipher_suite->key_xchg->free_params)
+		tls->pending.cipher_suite->key_xchg->free_params(tls);
+
 	l_cert_free(tls->peer_cert);
 	l_key_free(tls->peer_pubkey);
 
-- 
2.19.1

[PATCH 10/17] tls: ECHDE_RSA key exchange implementation server side

[Andrew Zaborowski]

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

Add the handler for the ECDHE ClientKeyExchange message rx and a
builder for the ServerKeyExchange message tx, both specified in
RFC 8422.
---
 ell/tls-private.h |   1 +
 ell/tls-suites.c  | 122 ++++++++++++++++++++++++++++++++++++++++++++++
 ell/tls.c         |  18 +++++++
 3 files changed, 141 insertions(+)

diff --git a/ell/tls-private.h b/ell/tls-private.h
index 0ed3eee..12d350a 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -66,6 +66,7 @@ struct tls_key_exchange_algorithm {
 	uint8_t id;
 
 	bool certificate_check;
+	bool need_ecc;
 
 	bool (*validate_cert_key_type)(struct l_cert *cert);
 
diff --git a/ell/tls-suites.c b/ell/tls-suites.c
index da31982..cda192b 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -412,6 +412,47 @@ static bool tls_get_server_ecdh_params_hash(struct l_tls *tls, uint8_t tls_id,
 	return true;
 }
 
+static bool tls_send_ecdhe_server_key_xchg(struct l_tls *tls)
+{
+	uint8_t buf[1024];
+	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
+	struct tls_ecdhe_params *params;
+	ssize_t sign_len;
+
+	/*
+	 * RFC 8422, Section 5.4
+	 *
+	 * If we're getting here we can assume that tls->pending.key_xchg_params
+	 * is NULL, tls->priv_key is our signing key and tls->negotiated_curve
+	 * is non-NULL.
+	 */
+
+	params = l_new(struct tls_ecdhe_params, 1);
+	params->curve = l_ecc_curve_get(tls->negotiated_curve->l_group);
+	tls->pending.key_xchg_params = params;
+
+	if (!l_ecdh_generate_key_pair(params->curve,
+					&params->private, &params->public)) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Generating ECDH key pair failed");
+		return false;
+	}
+
+	ptr += tls_write_server_ecdh_params(tls, ptr);
+
+	sign_len = tls->pending.cipher_suite->key_xchg->sign(tls, ptr,
+					buf + sizeof(buf) - ptr,
+					tls_get_server_ecdh_params_hash);
+	if (sign_len < 0)
+		return false;
+
+	ptr += sign_len;
+
+	tls_tx_handshake(tls, TLS_SERVER_KEY_EXCHANGE, buf, ptr - buf);
+
+	return true;
+}
+
 static void tls_handle_ecdhe_server_key_xchg(struct l_tls *tls,
 						const uint8_t *buf, size_t len)
 {
@@ -555,12 +596,93 @@ static bool tls_send_ecdhe_client_key_xchg(struct l_tls *tls)
 	return true;
 }
 
+static void tls_handle_ecdhe_client_key_xchg(struct l_tls *tls,
+						const uint8_t *buf, size_t len)
+{
+	struct tls_ecdhe_params *params = tls->pending.key_xchg_params;
+	uint8_t pre_master_secret[128];
+	ssize_t pre_master_secret_len;
+	struct l_ecc_point *other_public;
+	struct l_ecc_scalar *secret;
+
+	/* RFC 8422, Section 5.7 */
+
+	if (len < 2)
+		goto decode_error;
+
+	if (*buf++ != 1 + tls->negotiated_curve->point_bytes)
+		goto decode_error;
+
+	if (*buf != 4) {	/* uncompressed */
+		TLS_DISCONNECT(TLS_ALERT_ILLEGAL_PARAM, 0,
+				"Unsupported (deprecated?) PointConversionForm "
+				"%u", *buf);
+		return;
+	}
+
+	buf++;
+	len -= 2;
+
+	if (len != tls->negotiated_curve->point_bytes)
+		goto decode_error;
+
+	/*
+	 * RFC 8422, Section 5.11: "A receiving party MUST check that the
+	 * x and y parameters from the peer's public value satisfy the
+	 * curve equation, y^2 = x^3 + ax + b mod p."
+	 * This happens in l_ecc_point_from_data when the L_ECC_POINT_TYPE_FULL
+	 * format is used.
+	 */
+	other_public = l_ecc_point_from_data(params->curve,
+						L_ECC_POINT_TYPE_FULL,
+						buf, len);
+	if (!other_public) {
+		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+				"ClientKeyExchange.exchange_keys.ecdh_Yc "
+				"decode error");
+		return;
+	}
+
+	if (!l_ecdh_generate_shared_secret(params->private, other_public,
+						&secret)) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Generating ECDH shared-secret failed");
+		return;
+	}
+
+	tls_free_ecdhe_params(tls);
+	l_ecc_point_free(other_public);
+	pre_master_secret_len = l_ecc_scalar_get_data(secret,
+						pre_master_secret,
+						sizeof(pre_master_secret));
+	l_ecc_scalar_free(secret);
+
+	if (pre_master_secret_len < 0) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"l_ecc_scalar_get_data(secret) failed");
+		return;
+	}
+
+	tls_generate_master_secret(tls, pre_master_secret,
+					pre_master_secret_len);
+	memset(pre_master_secret, 0, pre_master_secret_len);
+
+	return;
+
+decode_error:
+	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+			"ClientKeyExchange decode error");
+}
+
 static struct tls_key_exchange_algorithm tls_ecdhe_rsa = {
 	.id = 1, /* RSA_sign */
 	.certificate_check = true,
+	.need_ecc = true,
 	.validate_cert_key_type = tls_rsa_validate_cert_key,
+	.send_server_key_exchange = tls_send_ecdhe_server_key_xchg,
 	.handle_server_key_exchange = tls_handle_ecdhe_server_key_xchg,
 	.send_client_key_exchange = tls_send_ecdhe_client_key_xchg,
+	.handle_client_key_exchange = tls_handle_ecdhe_client_key_xchg,
 	.free_params = tls_free_ecdhe_params,
 	.sign = tls_rsa_sign,
 	.verify = tls_rsa_verify,
diff --git a/ell/tls.c b/ell/tls.c
index df827f3..fc8ab41 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -476,6 +476,24 @@ static bool tls_cipher_suite_is_compatible(struct l_tls *tls,
 		return false;
 	}
 
+	/*
+	 * On the server we know what elliptic curve we'll be using as soon
+	 * as we've processed the ClientHello so for EC-based key exchange
+	 * methods require that a curve has been selected.
+	 */
+	if (suite->key_xchg->need_ecc && tls->server &&
+			!tls->negotiated_curve) {
+		if (error) {
+			*error = error_buf;
+			snprintf(error_buf, sizeof(error_buf),
+					"No common supported elliptic curves "
+					"with the client, can't use %s",
+					suite->name);
+		}
+
+		return false;
+	}
+
 	return true;
 }
 
-- 
2.19.1

[PATCH 11/17] tls: Add RFC4492 suites using the ECDHE_RSA key exchange

[Andrew Zaborowski]

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

Add the 4 cipher suites defined in RFC 4492 that we can now support
with the new ECDHE_RSA key exchange (see section 6 in RFC 4492 and 8422
for the full list of ECDH-based cipher suites.)
---
 ell/tls-suites.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/ell/tls-suites.c b/ell/tls-suites.c
index cda192b..297b3d4 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -802,16 +802,48 @@ static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
 	.encryption = &tls_aes256_gcm,
 	.prf_hmac = L_CHECKSUM_SHA384,
 	.key_xchg = &tls_rsa,
+}, tls_ecdhe_rsa_with_rc4_128_sha = {
+	.id = { 0xc0, 0x11 },
+	.name = "TLS_ECDHE_RSA_WITH_RC4_128_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_rc4,
+	.mac = &tls_sha,
+	.key_xchg = &tls_ecdhe_rsa,
+}, tls_ecdhe_rsa_with_3des_ede_cbc_sha = {
+	.id = { 0xc0, 0x12 },
+	.name = "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_3des_ede,
+	.mac = &tls_sha,
+	.key_xchg = &tls_ecdhe_rsa,
+}, tls_ecdhe_rsa_with_aes_128_cbc_sha = {
+	.id = { 0xc0, 0x13 },
+	.name = "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128,
+	.mac = &tls_sha,
+	.key_xchg = &tls_ecdhe_rsa,
+}, tls_ecdhe_rsa_with_aes_256_cbc_sha = {
+	.id = { 0xc0, 0x14 },
+	.name = "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256,
+	.mac = &tls_sha,
+	.key_xchg = &tls_ecdhe_rsa,
 };
 
 struct tls_cipher_suite *tls_cipher_suite_pref[] = {
+	&tls_ecdhe_rsa_with_aes_256_cbc_sha,
+	&tls_ecdhe_rsa_with_aes_128_cbc_sha,
 	&tls_rsa_with_aes_256_cbc_sha,
 	&tls_rsa_with_aes_128_cbc_sha,
 	&tls_rsa_with_aes_256_cbc_sha256,
 	&tls_rsa_with_aes_128_cbc_sha256,
 	&tls_rsa_with_aes_256_gcm_sha384,
 	&tls_rsa_with_aes_128_gcm_sha256,
+	&tls_ecdhe_rsa_with_3des_ede_cbc_sha,
 	&tls_rsa_with_3des_ede_cbc_sha,
+	&tls_ecdhe_rsa_with_rc4_128_sha,
 	&tls_rsa_with_rc4_128_sha,
 	&tls_rsa_with_rc4_128_md5,
 	NULL,
-- 
2.19.1

[PATCH 12/17] tls: Add RFC5289 suites using the ECDHE_RSA key exchange

[Andrew Zaborowski]

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

Add the 4 cipher suites defined in RFC 5289 that we can now support
with the new ECDHE_RSA key exchange, 2 of these are the same ones
defined in RFC 8422.
---
 ell/tls-suites.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/ell/tls-suites.c b/ell/tls-suites.c
index 297b3d4..af8cbcd 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -738,6 +738,10 @@ static struct tls_mac_algorithm tls_md5 = {
 	.id = 4,
 	.hmac_type = L_CHECKSUM_SHA256,
 	.mac_length = 32,
+}, tls_sha384 = {
+	.id = 5,
+	.hmac_type = L_CHECKSUM_SHA384,
+	.mac_length = 48,
 };
 
 static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
@@ -830,6 +834,34 @@ static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
 	.encryption = &tls_aes256,
 	.mac = &tls_sha,
 	.key_xchg = &tls_ecdhe_rsa,
+}, tls_ecdhe_rsa_with_aes_128_cbc_sha256 = {
+	.id = { 0xc0, 0x27 },
+	.name = "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128,
+	.mac = &tls_sha256,
+	.key_xchg = &tls_ecdhe_rsa,
+}, tls_ecdhe_rsa_with_aes_256_cbc_sha384 = {
+	.id = { 0xc0, 0x28 },
+	.name = "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256,
+	.mac = &tls_sha384,
+	.prf_hmac = L_CHECKSUM_SHA384,
+	.key_xchg = &tls_ecdhe_rsa,
+}, tls_ecdhe_rsa_with_aes_128_gcm_sha256 = {
+	.id = { 0xc0, 0x2f },
+	.name = "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128_gcm,
+	.key_xchg = &tls_ecdhe_rsa,
+}, tls_ecdhe_rsa_with_aes_256_gcm_sha384 = {
+	.id = { 0xc0, 0x30 },
+	.name = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256_gcm,
+	.prf_hmac = L_CHECKSUM_SHA384,
+	.key_xchg = &tls_ecdhe_rsa,
 };
 
 struct tls_cipher_suite *tls_cipher_suite_pref[] = {
@@ -837,8 +869,12 @@ struct tls_cipher_suite *tls_cipher_suite_pref[] = {
 	&tls_ecdhe_rsa_with_aes_128_cbc_sha,
 	&tls_rsa_with_aes_256_cbc_sha,
 	&tls_rsa_with_aes_128_cbc_sha,
+	&tls_ecdhe_rsa_with_aes_256_cbc_sha384,
+	&tls_ecdhe_rsa_with_aes_128_cbc_sha256,
 	&tls_rsa_with_aes_256_cbc_sha256,
 	&tls_rsa_with_aes_128_cbc_sha256,
+	&tls_ecdhe_rsa_with_aes_256_gcm_sha384,
+	&tls_ecdhe_rsa_with_aes_128_gcm_sha256,
 	&tls_rsa_with_aes_256_gcm_sha384,
 	&tls_rsa_with_aes_128_gcm_sha256,
 	&tls_ecdhe_rsa_with_3des_ede_cbc_sha,
-- 
2.19.1

[PATCH 13/17] key: Add l_key_validate_dh_payload

[Andrew Zaborowski]

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

Add a function to validate local private value payloads against a
the group's prime before creating kernel keys out of them, and to
validate public values received from the peer.
---
 ell/ell.sym |  1 +
 ell/key.c   | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 ell/key.h   |  3 +++
 3 files changed, 52 insertions(+)

diff --git a/ell/ell.sym b/ell/ell.sym
index ea64ac2..c3e6156 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -303,6 +303,7 @@ global:
 	l_key_get_info;
 	l_key_compute_dh_public;
 	l_key_compute_dh_secret;
+	l_key_validate_dh_payload;
 	l_key_encrypt;
 	l_key_decrypt;
 	l_key_sign;
diff --git a/ell/key.c b/ell/key.c
index 415658b..297e456 100644
--- a/ell/key.c
+++ b/ell/key.c
@@ -456,6 +456,54 @@ LIB_EXPORT bool l_key_compute_dh_secret(struct l_key *other_public,
 	return compute_common(other_public, private, prime, payload, len);
 }
 
+static int be_bignum_compare(const uint8_t *a, size_t a_len,
+				const uint8_t *b, size_t b_len)
+{
+	unsigned int i;
+
+	if (a_len >= b_len) {
+		for (i = 0; i < a_len - b_len; i++)
+			if (a[i])
+				return 1;
+
+		return memcmp(a + i, b, b_len);
+	} else {
+		for (i = 0; i < b_len - a_len; i++)
+			if (b[i])
+				return -1;
+
+		return memcmp(a, b + i, a_len);
+	}
+}
+
+/*
+ * Validate that @payload is within range for a private and public key for
+ * a DH computation in the finite field group defined by modulus @prime_buf,
+ * both numbers stored as big-endian integers.  We require a key in the
+ * [2, prime - 2] (inclusive) interval.  PKCS #3 does not exclude 1 as a
+ * private key but other specs do.
+ */
+LIB_EXPORT bool l_key_validate_dh_payload(const void *payload, size_t len,
+				const void *prime_buf, size_t prime_len)
+{
+	static const uint8_t one[] = { 1 };
+	uint8_t prime_1[prime_len];
+	unsigned int pos = prime_len;
+
+	memcpy(prime_1, prime_buf, prime_len);
+
+	/* Produce prime - 1 for the payload < prime - 1 check */
+	while (pos && !prime_1[--pos]--);
+
+	if (be_bignum_compare(payload, len, one, 1) <= 0)
+		return false;
+
+	if (be_bignum_compare(payload, len, prime_1, prime_len) >= 0)
+		return false;
+
+	return true;
+}
+
 /* Common code for encrypt/decrypt/sign */
 static ssize_t eds_common(struct l_key *key,
 				enum l_key_cipher_type cipher,
diff --git a/ell/key.h b/ell/key.h
index 1529135..e8688c6 100644
--- a/ell/key.h
+++ b/ell/key.h
@@ -80,6 +80,9 @@ bool l_key_compute_dh_secret(struct l_key *other_public, struct l_key *private,
 				struct l_key *prime,
 				void *payload, size_t *len);
 
+bool l_key_validate_dh_payload(const void *payload, size_t len,
+				const void *prime_buf, size_t prime_len);
+
 ssize_t l_key_encrypt(struct l_key *key, enum l_key_cipher_type cipher,
 			enum l_checksum_type checksum, const void *in,
 			void *out, size_t len_in, size_t len_out);
-- 
2.19.1

[PATCH 14/17] tls: DHE_RSA key exchange implementation client side

[Andrew Zaborowski]

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

Add a new key exchange definition with the callbacks needed to handle the
client side of the DHE_RSA key exchange.
---
 ell/tls-suites.c | 354 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 354 insertions(+)

diff --git a/ell/tls-suites.c b/ell/tls-suites.c
index af8cbcd..ef1f2cc 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -688,6 +688,360 @@ static struct tls_key_exchange_algorithm tls_ecdhe_rsa = {
 	.verify = tls_rsa_verify,
 };
 
+struct tls_dhe_params {
+	/*
+	 * We need both the raw contents here for tls_write_server_dh_params
+	 * and the l_key pointers to cache the l_keys created for the
+	 * ServerKeyExchange and not have to allocate them again during
+	 * ClientKeyExchange.  public_buf is used for both our and the
+	 * peer's DH public value while public is only used for the peer's
+	 * public value.
+	 */
+	struct l_key *prime;
+	uint8_t *prime_buf;
+	size_t prime_len;
+	struct l_key *generator;
+	uint8_t *generator_buf;
+	size_t generator_len;
+	struct l_key *private;
+	const uint8_t *private_buf;
+	size_t private_len;
+	struct l_key *public;
+	const uint8_t *public_buf;
+	size_t public_len;
+};
+
+#define TLS_DHE_MAX_SIZE 1024
+
+static void tls_free_dhe_params(struct l_tls *tls)
+{
+	struct tls_dhe_params *params = tls->pending.key_xchg_params;
+
+	if (!params)
+		return;
+
+	tls->pending.key_xchg_params = NULL;
+
+	l_free(params->prime_buf);
+	l_free(params->generator_buf);
+	l_key_free(params->prime);
+	l_key_free(params->generator);
+	l_key_free(params->private);
+	l_key_free(params->public);
+	l_free(params);
+}
+
+static bool tls_create_dhe_keys(struct l_tls *tls)
+{
+	struct tls_dhe_params *params = tls->pending.key_xchg_params;
+
+	if (!params)
+		return false;
+
+	if (params->prime_buf && !params->prime) {
+		params->prime = l_key_new(L_KEY_RAW, params->prime_buf,
+						params->prime_len);
+		if (!params->prime) {
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+					"l_key_new(prime) failed");
+			return false;
+		}
+	}
+
+	if (params->generator_buf && !params->generator) {
+		params->generator = l_key_new(L_KEY_RAW, params->generator_buf,
+						params->generator_len);
+		if (!params->generator) {
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+					"l_key_new(generator) failed");
+			return false;
+		}
+	}
+
+	if (params->private_buf && !params->private) {
+		params->private = l_key_new(L_KEY_RAW, params->private_buf,
+						params->private_len);
+		if (!params->private) {
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+					"l_key_new(private) failed");
+			return false;
+		}
+	}
+
+	if (params->public_buf && !params->public) {
+		params->public = l_key_new(L_KEY_RAW, params->public_buf,
+						params->public_len);
+		if (!params->public) {
+			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+					"l_key_new(public) failed");
+			return false;
+		}
+	}
+
+	return true;
+}
+
+static bool tls_generate_dhe_private(struct l_tls *tls, uint8_t *buf, size_t len)
+{
+	struct tls_dhe_params *params = tls->pending.key_xchg_params;
+
+	do {
+		unsigned int written = 0;
+
+		/* Leading zeros in the private value are fine */
+
+		do {
+			size_t chunk_len =
+				written + 256 <= len ? 256 : len - written;
+
+			if (!l_getrandom(buf + written, chunk_len)) {
+				TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+						"l_getrandom(buf, %zu) failed",
+						chunk_len);
+				return false;
+			}
+
+			written += chunk_len;
+		} while (written < len);
+	} while (!l_key_validate_dh_payload(buf, len,
+						params->prime_buf,
+						params->prime_len));
+
+	return true;
+}
+
+static size_t tls_write_server_dh_params(struct l_tls *tls, uint8_t *buf)
+{
+	struct tls_dhe_params *params = tls->pending.key_xchg_params;
+	uint8_t *ptr = buf;
+
+	/* RFC 5246, Section 7.4.3 */
+
+	l_put_be16(params->prime_len, ptr);
+	memcpy(ptr + 2, params->prime_buf, params->prime_len);
+	ptr += 2 + params->prime_len;
+
+	l_put_be16(params->generator_len, ptr);
+	memcpy(ptr + 2, params->generator_buf, params->generator_len);
+	ptr += 2 + params->generator_len;
+
+	l_put_be16(params->public_len, ptr);
+	memcpy(ptr + 2, params->public_buf, params->public_len);
+	ptr += 2 + params->public_len;
+
+	return ptr - buf;
+}
+
+static bool tls_get_server_dh_params_hash(struct l_tls *tls, uint8_t tls_id,
+						uint8_t *out, size_t *len,
+						enum l_checksum_type *type)
+{
+	unsigned int hash;
+	struct l_checksum *checksum;
+	uint8_t params[32 + TLS_DHE_MAX_SIZE * 3];
+	size_t params_len;
+	ssize_t hash_len, ret;
+
+	params_len = tls_write_server_dh_params(tls, params);
+
+	for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
+		if (tls_handshake_hash_data[hash].tls_id == tls_id)
+			break;
+
+	if (hash == __HANDSHAKE_HASH_COUNT)
+		return false;
+
+	hash_len = tls_handshake_hash_data[hash].length;
+
+	checksum = l_checksum_new(tls_handshake_hash_data[hash].l_id);
+	if (!checksum)
+		return false;
+
+	l_checksum_update(checksum, tls->pending.client_random, 32);
+	l_checksum_update(checksum, tls->pending.server_random, 32);
+	l_checksum_update(checksum, params, params_len);
+	ret = l_checksum_get_digest(checksum, out, hash_len);
+	l_checksum_free(checksum);
+
+	if (ret != (ssize_t) hash_len)
+		return false;
+
+	if (len)
+		*len = hash_len;
+
+	if (type)
+		*type = tls_handshake_hash_data[hash].l_id;
+
+	return true;
+}
+
+static void tls_handle_dhe_server_key_xchg(struct l_tls *tls,
+						const uint8_t *buf, size_t len)
+{
+	struct tls_dhe_params *params;
+
+	params = l_new(struct tls_dhe_params, 1);
+	tls->pending.key_xchg_params = params;
+
+	if (len < 2)
+		goto decode_error;
+
+	params->prime_len = l_get_be16(buf);
+	if (len < 2 + params->prime_len + 2)
+		goto decode_error;
+
+	params->prime_buf = l_memdup(buf + 2, params->prime_len);
+	buf += 2 + params->prime_len;
+	len -= 2 + params->prime_len;
+
+	params->generator_len = l_get_be16(buf);
+	if (len < 2 + params->generator_len + 2)
+		goto decode_error;
+
+	params->generator_buf = l_memdup(buf + 2, params->generator_len);;
+	buf += 2 + params->generator_len;
+	len -= 2 + params->generator_len;
+
+	params->public_len = l_get_be16(buf);
+	if (len < 2 + params->public_len)
+		goto decode_error;
+
+	params->public_buf = buf + 2;
+	buf += 2 + params->public_len;
+	len -= 2 + params->public_len;
+
+	/*
+	 * Validate the values received.  Without RFC 7919 we basically have
+	 * to blindly accept the provided prime value.  We have no way to
+	 * confirm that it's actually prime or that it's a "safe prime" or
+	 * that it forms a group without small sub-groups.  There's also no
+	 * way to whitelist all valid values.  But we do a basic sanity
+	 * check and require it to be 1024-bit or longer (see weakdh.org),
+	 * might need to move to 2048 bits actually.
+	 */
+
+	if (params->prime_len > TLS_DHE_MAX_SIZE ||
+			params->generator_len > params->prime_len) {
+		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+				"Server DH prime modulus or generator "
+				"too long");
+		return;
+	}
+
+	if (params->prime_len < 128 || params->prime_buf[0] == 0x00 ||
+			!(params->prime_buf[params->prime_len - 1] & 1)) {
+		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+				"Server DH prime modulus invalid");
+		return;
+	}
+
+	/*
+	 * RFC 7919 Section 3.0:
+	 * "the client MUST verify that dh_Ys is in the range
+	 * 1 < dh_Ys < dh_p - 1.  If dh_Ys is not in this range, the client
+	 * MUST terminate the connection with a fatal handshake_failure(40)
+	 * alert."
+	 */
+	if (params->public_len > params->prime_len ||
+			!l_key_validate_dh_payload(params->public_buf,
+							params->public_len,
+							params->prime_buf,
+							params->prime_len)) {
+		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+				"Server DH public value invalid");
+		return;
+	}
+
+	if (!tls_create_dhe_keys(tls))
+		return;
+
+	if (!tls->pending.cipher_suite->key_xchg->verify(tls, buf, len,
+						tls_get_server_dh_params_hash))
+		return;
+
+	TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO_DONE);
+	return;
+
+decode_error:
+	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+			"ServerKeyExchange decode error");
+}
+
+static bool tls_send_dhe_client_key_xchg(struct l_tls *tls)
+{
+	uint8_t buf[128 + TLS_DHE_MAX_SIZE];
+	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
+	struct tls_dhe_params *params = tls->pending.key_xchg_params;
+	uint8_t pre_master_secret[params->prime_len];
+	size_t pre_master_secret_len;
+	unsigned int offset = 0;
+	uint8_t private_buf[params->prime_len];
+	uint8_t public_buf[params->prime_len];
+
+	if (!tls_generate_dhe_private(tls, private_buf, params->prime_len))
+		return false;
+
+	params->private_buf = private_buf;
+	params->private_len = params->prime_len;
+
+	if (!tls_create_dhe_keys(tls))
+		return false;
+
+	memset(private_buf, 0, params->private_len);
+	params->public_buf = public_buf;
+	params->public_len = params->prime_len;
+
+	if (!l_key_compute_dh_public(params->generator, params->private,
+					params->prime, public_buf,
+					&params->public_len)) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"l_key_compute_dh_public failed");
+		return false;
+	}
+
+	while (params->public_buf[0] == 0x00 && params->public_len) {
+		params->public_buf++;
+		params->public_len--;
+	}
+
+	l_put_be16(params->public_len, ptr);
+	memcpy(ptr + 2, params->public_buf, params->public_len);
+	ptr += 2 + params->public_len;
+	pre_master_secret_len = params->prime_len;
+
+	if (!l_key_compute_dh_secret(params->public, params->private,
+					params->prime, pre_master_secret,
+					&pre_master_secret_len)) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Generating DH shared-secret failed");
+		return false;
+	}
+
+	while (pre_master_secret[offset] == 0x00 &&
+			offset < pre_master_secret_len)
+		offset++;
+
+	tls_tx_handshake(tls, TLS_CLIENT_KEY_EXCHANGE, buf, ptr - buf);
+
+	tls_free_dhe_params(tls);
+	tls_generate_master_secret(tls, pre_master_secret + offset,
+					pre_master_secret_len - offset);
+	memset(pre_master_secret, 0, pre_master_secret_len);
+	return true;
+}
+
+static struct tls_key_exchange_algorithm tls_dhe_rsa = {
+	.id = 1, /* RSA_sign */
+	.certificate_check = true,
+	.need_ecc = true,
+	.validate_cert_key_type = tls_rsa_validate_cert_key,
+	.handle_server_key_exchange = tls_handle_dhe_server_key_xchg,
+	.send_client_key_exchange = tls_send_dhe_client_key_xchg,
+	.free_params = tls_free_dhe_params,
+	.sign = tls_rsa_sign,
+	.verify = tls_rsa_verify,
+};
+
 static struct tls_bulk_encryption_algorithm tls_rc4 = {
 	.cipher_type = TLS_CIPHER_STREAM,
 	.l_id = L_CIPHER_ARC4,
-- 
2.19.1

[PATCH 15/17] tls: DHE_RSA key exchange implementation server side

[Andrew Zaborowski]

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

Add the handler for the ClientKeyExchange message and a builder callback
for the ServerKeyExchange message for DHE_RSA.
---
 ell/tls-suites.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 164 insertions(+)

diff --git a/ell/tls-suites.c b/ell/tls-suites.c
index ef1f2cc..8e975a0 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -688,6 +688,34 @@ static struct tls_key_exchange_algorithm tls_ecdhe_rsa = {
 	.verify = tls_rsa_verify,
 };
 
+/* RFC 3526, Section 3 */
+static const uint8_t tls_dh14_prime[] = {
+	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc9, 0x0f, 0xda, 0xa2,
+	0x21, 0x68, 0xc2, 0x34, 0xc4, 0xc6, 0x62, 0x8b, 0x80, 0xdc, 0x1c, 0xd1,
+	0x29, 0x02, 0x4e, 0x08, 0x8a, 0x67, 0xcc, 0x74, 0x02, 0x0b, 0xbe, 0xa6,
+	0x3b, 0x13, 0x9b, 0x22, 0x51, 0x4a, 0x08, 0x79, 0x8e, 0x34, 0x04, 0xdd,
+	0xef, 0x95, 0x19, 0xb3, 0xcd, 0x3a, 0x43, 0x1b, 0x30, 0x2b, 0x0a, 0x6d,
+	0xf2, 0x5f, 0x14, 0x37, 0x4f, 0xe1, 0x35, 0x6d, 0x6d, 0x51, 0xc2, 0x45,
+	0xe4, 0x85, 0xb5, 0x76, 0x62, 0x5e, 0x7e, 0xc6, 0xf4, 0x4c, 0x42, 0xe9,
+	0xa6, 0x37, 0xed, 0x6b, 0x0b, 0xff, 0x5c, 0xb6, 0xf4, 0x06, 0xb7, 0xed,
+	0xee, 0x38, 0x6b, 0xfb, 0x5a, 0x89, 0x9f, 0xa5, 0xae, 0x9f, 0x24, 0x11,
+	0x7c, 0x4b, 0x1f, 0xe6, 0x49, 0x28, 0x66, 0x51, 0xec, 0xe4, 0x5b, 0x3d,
+	0xc2, 0x00, 0x7c, 0xb8, 0xa1, 0x63, 0xbf, 0x05, 0x98, 0xda, 0x48, 0x36,
+	0x1c, 0x55, 0xd3, 0x9a, 0x69, 0x16, 0x3f, 0xa8, 0xfd, 0x24, 0xcf, 0x5f,
+	0x83, 0x65, 0x5d, 0x23, 0xdc, 0xa3, 0xad, 0x96, 0x1c, 0x62, 0xf3, 0x56,
+	0x20, 0x85, 0x52, 0xbb, 0x9e, 0xd5, 0x29, 0x07, 0x70, 0x96, 0x96, 0x6d,
+	0x67, 0x0c, 0x35, 0x4e, 0x4a, 0xbc, 0x98, 0x04, 0xf1, 0x74, 0x6c, 0x08,
+	0xca, 0x18, 0x21, 0x7c, 0x32, 0x90, 0x5e, 0x46, 0x2e, 0x36, 0xce, 0x3b,
+	0xe3, 0x9e, 0x77, 0x2c, 0x18, 0x0e, 0x86, 0x03, 0x9b, 0x27, 0x83, 0xa2,
+	0xec, 0x07, 0xa2, 0x8f, 0xb5, 0xc5, 0x5d, 0xf0, 0x6f, 0x4c, 0x52, 0xc9,
+	0xde, 0x2b, 0xcb, 0xf6, 0x95, 0x58, 0x17, 0x18, 0x39, 0x95, 0x49, 0x7c,
+	0xea, 0x95, 0x6a, 0xe5, 0x15, 0xd2, 0x26, 0x18, 0x98, 0xfa, 0x05, 0x10,
+	0x15, 0x72, 0x8e, 0x5a, 0x8a, 0xac, 0xaa, 0x68, 0xff, 0xff, 0xff, 0xff,
+	0xff, 0xff, 0xff, 0xff,
+};
+
+static const unsigned int tls_dh14_generator = 2;
+
 struct tls_dhe_params {
 	/*
 	 * We need both the raw contents here for tls_write_server_dh_params
@@ -875,6 +903,77 @@ static bool tls_get_server_dh_params_hash(struct l_tls *tls, uint8_t tls_id,
 	return true;
 }
 
+static bool tls_send_dhe_server_key_xchg(struct l_tls *tls)
+{
+	uint8_t buf[1024 + TLS_DHE_MAX_SIZE * 3];
+	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
+	struct tls_dhe_params *params;
+	uint8_t private_buf[TLS_DHE_MAX_SIZE];
+	uint8_t public_buf[TLS_DHE_MAX_SIZE];
+	uint8_t generator = tls_dh14_generator;
+	ssize_t sign_len;
+	bool result;
+
+	params = l_new(struct tls_dhe_params, 1);
+	tls->pending.key_xchg_params = params;
+
+	/*
+	 * For now hardcode the Oakley Group 14 parameters like some other
+	 * TLS implementations do without RFC 7919, which is actually a
+	 * downside because the more common the group parameters are the
+	 * less secure they are assumed to be, but also is a test that the
+	 * group is sufficiently good.
+	 *
+	 * Eventually we should make this configurable so that a unique
+	 * likely-prime number generated by either 'openssl dhparam' or
+	 * 'ssh-keygen -G' can be set, or parse /etc/ssh/moduli to select
+	 * a random pre-generated FFDH group each time.
+	 */
+	params->prime_buf = (uint8_t *) tls_dh14_prime;
+	params->prime_len = sizeof(tls_dh14_prime);
+	params->generator_buf = &generator;
+	params->generator_len = 1;
+	params->private_buf = private_buf;
+	params->private_len = params->prime_len;
+
+	result = tls_generate_dhe_private(tls, private_buf, params->prime_len) &&
+		tls_create_dhe_keys(tls);
+	params->prime_buf = NULL;
+	params->generator_buf = NULL;
+
+	if (!result)
+		return false;
+
+	memset(private_buf, 0, params->private_len);
+	params->public_buf = public_buf;
+	params->public_len = params->prime_len;
+
+	if (!l_key_compute_dh_public(params->generator, params->private,
+					params->prime, public_buf,
+					&params->public_len)) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"l_key_compute_dh_public failed");
+		return false;
+	}
+
+	while (params->public_buf[0] == 0x00 && params->public_len) {
+		params->public_buf++;
+		params->public_len--;
+	}
+
+	ptr += tls_write_server_dh_params(tls, ptr);
+	sign_len = tls->pending.cipher_suite->key_xchg->sign(tls, ptr,
+					buf + sizeof(buf) - ptr,
+					tls_get_server_dh_params_hash);
+	ptr += sign_len;
+
+	if (sign_len < 0)
+		return false;
+
+	tls_tx_handshake(tls, TLS_SERVER_KEY_EXCHANGE, buf, ptr - buf);
+	return true;
+}
+
 static void tls_handle_dhe_server_key_xchg(struct l_tls *tls,
 						const uint8_t *buf, size_t len)
 {
@@ -1030,13 +1129,78 @@ static bool tls_send_dhe_client_key_xchg(struct l_tls *tls)
 	return true;
 }
 
+static void tls_handle_dhe_client_key_xchg(struct l_tls *tls,
+						const uint8_t *buf, size_t len)
+{
+	struct tls_dhe_params *params = tls->pending.key_xchg_params;
+	uint8_t pre_master_secret[params->prime_len];
+	size_t pre_master_secret_len;
+	unsigned int offset = 0;
+
+	if (len < 2)
+		goto decode_error;
+
+	params->public_len = l_get_be16(buf);
+	buf += 2;
+	len -= 2;
+	params->public_buf = buf;
+
+	if (params->public_len != len)
+		goto decode_error;
+
+	/*
+	 * RFC 7919 Section 4:
+	 * "the server MUST verify that 1 < dh_Yc < dh_p - 1.  If dh_Yc is
+	 * out of range, the server MUST terminate the connection with
+	 * a fatal handshake_failure(40) alert."
+	 */
+	if (params->public_len > params->prime_len ||
+			!l_key_validate_dh_payload(params->public_buf,
+							params->public_len,
+							params->prime_buf,
+							params->prime_len)) {
+		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+				"Client DH public value invalid");
+		return;
+	}
+
+	if (!tls_create_dhe_keys(tls))
+		return;
+
+	pre_master_secret_len = params->prime_len;
+
+	if (!l_key_compute_dh_secret(params->public, params->private,
+					params->prime, pre_master_secret,
+					&pre_master_secret_len)) {
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Generating DH shared-secret failed");
+		return;
+	}
+
+	while (pre_master_secret[offset] == 0x00 &&
+			offset < pre_master_secret_len)
+		offset++;
+
+	tls_free_dhe_params(tls);
+	tls_generate_master_secret(tls, pre_master_secret + offset,
+					pre_master_secret_len - offset);
+	memset(pre_master_secret, 0, pre_master_secret_len);
+	return;
+
+decode_error:
+	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
+			"ClientKeyExchange decode error");
+}
+
 static struct tls_key_exchange_algorithm tls_dhe_rsa = {
 	.id = 1, /* RSA_sign */
 	.certificate_check = true,
 	.need_ecc = true,
 	.validate_cert_key_type = tls_rsa_validate_cert_key,
+	.send_server_key_exchange = tls_send_dhe_server_key_xchg,
 	.handle_server_key_exchange = tls_handle_dhe_server_key_xchg,
 	.send_client_key_exchange = tls_send_dhe_client_key_xchg,
+	.handle_client_key_exchange = tls_handle_dhe_client_key_xchg,
 	.free_params = tls_free_dhe_params,
 	.sign = tls_rsa_sign,
 	.verify = tls_rsa_verify,
-- 
2.19.1

[PATCH 16/17] tls: Add DHE_RSA-based cipher suites

[Andrew Zaborowski]

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

Add the 5 DHE_RSA suites defined in RFC 5246 and the 2 defined in
RFC 5288.
---
 ell/tls-suites.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/ell/tls-suites.c b/ell/tls-suites.c
index 8e975a0..7b2ed27 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -1283,6 +1283,13 @@ static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
 	.encryption = &tls_3des_ede,
 	.mac = &tls_sha,
 	.key_xchg = &tls_rsa,
+}, tls_dhe_rsa_with_3des_ede_cbc_sha = {
+	.id = { 0x00, 0x16 },
+	.name = "TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_3des_ede,
+	.mac = &tls_sha,
+	.key_xchg = &tls_dhe_rsa,
 }, tls_rsa_with_aes_128_cbc_sha = {
 	.id = { 0x00, 0x2f },
 	.name = "TLS_RSA_WITH_AES_128_CBC_SHA",
@@ -1290,6 +1297,13 @@ static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
 	.encryption = &tls_aes128,
 	.mac = &tls_sha,
 	.key_xchg = &tls_rsa,
+}, tls_dhe_rsa_with_aes_128_cbc_sha = {
+	.id = { 0x00, 0x33 },
+	.name = "TLS_DHE_RSA_WITH_AES_128_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128,
+	.mac = &tls_sha,
+	.key_xchg = &tls_dhe_rsa,
 }, tls_rsa_with_aes_256_cbc_sha = {
 	.id = { 0x00, 0x35 },
 	.name = "TLS_RSA_WITH_AES_256_CBC_SHA",
@@ -1297,6 +1311,13 @@ static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
 	.encryption = &tls_aes256,
 	.mac = &tls_sha,
 	.key_xchg = &tls_rsa,
+}, tls_dhe_rsa_with_aes_256_cbc_sha = {
+	.id = { 0x00, 0x39 },
+	.name = "TLS_DHE_RSA_WITH_AES_256_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256,
+	.mac = &tls_sha,
+	.key_xchg = &tls_dhe_rsa,
 }, tls_rsa_with_aes_128_cbc_sha256 = {
 	.id = { 0x00, 0x3c },
 	.name = "TLS_RSA_WITH_AES_128_CBC_SHA256",
@@ -1311,6 +1332,20 @@ static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
 	.encryption = &tls_aes256,
 	.mac = &tls_sha256,
 	.key_xchg = &tls_rsa,
+}, tls_dhe_rsa_with_aes_128_cbc_sha256 = {
+	.id = { 0x00, 0x67 },
+	.name = "TLS_DHE_RSA_WITH_AES_128_CBC_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128,
+	.mac = &tls_sha256,
+	.key_xchg = &tls_dhe_rsa,
+}, tls_dhe_rsa_with_aes_256_cbc_sha256 = {
+	.id = { 0x00, 0x6b },
+	.name = "TLS_DHE_RSA_WITH_AES_256_CBC_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256,
+	.mac = &tls_sha256,
+	.key_xchg = &tls_dhe_rsa,
 }, tls_rsa_with_aes_128_gcm_sha256 = {
 	.id = { 0x00, 0x9c },
 	.name = "TLS_RSA_WITH_AES_128_GCM_SHA256",
@@ -1324,6 +1359,19 @@ static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
 	.encryption = &tls_aes256_gcm,
 	.prf_hmac = L_CHECKSUM_SHA384,
 	.key_xchg = &tls_rsa,
+}, tls_dhe_rsa_with_aes_128_gcm_sha256 = {
+	.id = { 0x00, 0x9e },
+	.name = "TLS_DHE_RSA_WITH_AES_128_GCM_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128_gcm,
+	.key_xchg = &tls_dhe_rsa,
+}, tls_dhe_rsa_with_aes_256_gcm_sha384 = {
+	.id = { 0x00, 0x9f },
+	.name = "TLS_DHE_RSA_WITH_AES_256_GCM_SHA384",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256_gcm,
+	.prf_hmac = L_CHECKSUM_SHA384,
+	.key_xchg = &tls_dhe_rsa,
 }, tls_ecdhe_rsa_with_rc4_128_sha = {
 	.id = { 0xc0, 0x11 },
 	.name = "TLS_ECDHE_RSA_WITH_RC4_128_SHA",
@@ -1385,17 +1433,24 @@ static struct tls_cipher_suite tls_rsa_with_rc4_128_md5 = {
 struct tls_cipher_suite *tls_cipher_suite_pref[] = {
 	&tls_ecdhe_rsa_with_aes_256_cbc_sha,
 	&tls_ecdhe_rsa_with_aes_128_cbc_sha,
+	&tls_dhe_rsa_with_aes_256_cbc_sha,
+	&tls_dhe_rsa_with_aes_128_cbc_sha,
 	&tls_rsa_with_aes_256_cbc_sha,
 	&tls_rsa_with_aes_128_cbc_sha,
 	&tls_ecdhe_rsa_with_aes_256_cbc_sha384,
 	&tls_ecdhe_rsa_with_aes_128_cbc_sha256,
+	&tls_dhe_rsa_with_aes_256_cbc_sha256,
+	&tls_dhe_rsa_with_aes_128_cbc_sha256,
 	&tls_rsa_with_aes_256_cbc_sha256,
 	&tls_rsa_with_aes_128_cbc_sha256,
 	&tls_ecdhe_rsa_with_aes_256_gcm_sha384,
 	&tls_ecdhe_rsa_with_aes_128_gcm_sha256,
+	&tls_dhe_rsa_with_aes_256_gcm_sha384,
+	&tls_dhe_rsa_with_aes_128_gcm_sha256,
 	&tls_rsa_with_aes_256_gcm_sha384,
 	&tls_rsa_with_aes_128_gcm_sha256,
 	&tls_ecdhe_rsa_with_3des_ede_cbc_sha,
+	&tls_dhe_rsa_with_3des_ede_cbc_sha,
 	&tls_rsa_with_3des_ede_cbc_sha,
 	&tls_ecdhe_rsa_with_rc4_128_sha,
 	&tls_rsa_with_rc4_128_sha,
-- 
2.19.1

[PATCH 17/17] tls: Switch state before handling Key Echange messages

[Andrew Zaborowski]

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

Where we handle a Client Key Exchange or a Server Key Exchange message
and then switch to a new state, do the state change first so that if
the tls_handle_*_key_exchange functions detect and error and move to an
error state, tls->state is not overwritten afterwards.  This seems
simpler than adding a success/failure return value to these methods so
that the callers can just stop execution on error.
---
 ell/tls.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/ell/tls.c b/ell/tls.c
index fc8ab41..7db1d9b 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -2176,11 +2176,11 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 			break;
 		}
 
+		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO_DONE);
+
 		tls->pending.cipher_suite->key_xchg->handle_server_key_exchange(
 								tls, buf, len);
 
-		TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO_DONE);
-
 		break;
 
 	case TLS_CERTIFICATE_REQUEST:
@@ -2247,9 +2247,6 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 			break;
 		}
 
-		tls->pending.cipher_suite->key_xchg->handle_client_key_exchange(
-								tls, buf, len);
-
 		/*
 		 * If we accepted a client Certificate message with a
 		 * certificate that has signing capability (TODO: check
@@ -2262,6 +2259,9 @@ static void tls_handle_handshake(struct l_tls *tls, int type,
 		else
 			TLS_SET_STATE(TLS_HANDSHAKE_WAIT_CHANGE_CIPHER_SPEC);
 
+		tls->pending.cipher_suite->key_xchg->handle_client_key_exchange(
+								tls, buf, len);
+
 		break;
 
 	case TLS_FINISHED:
-- 
2.19.1

Re: [PATCH 01/17] tls: Only accept the Certificate Request in client mode

[Denis Kenzior]

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

Hi Andrew,

On 01/01/2019 01:49 PM, Andrew Zaborowski wrote:
> ---
>   ell/tls.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH 13/17] key: Add l_key_validate_dh_payload

[Denis Kenzior]

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

Hi Andrew,

On 01/01/2019 01:49 PM, Andrew Zaborowski wrote:
> Add a function to validate local private value payloads against a
> the group's prime before creating kernel keys out of them, and to

Do we have the same issue in iwd EAP-WSC?

> validate public values received from the peer.

Note that the kernel already performs almost the same check in 
dh_is_pubkey_valid

Might make more sense to create an API to generate a valid private key 
with the needed constraints.  E.g. by twiddling the MSBs to 0 until the 
private key is less than the prime.

> ---
>   ell/ell.sym |  1 +
>   ell/key.c   | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>   ell/key.h   |  3 +++
>   3 files changed, 52 insertions(+)
> 

Regards,
-Denis

Re: [PATCH 14/17] tls: DHE_RSA key exchange implementation client side

[Denis Kenzior]

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

Hi Andrew,

On 01/01/2019 01:49 PM, Andrew Zaborowski wrote:
> Add a new key exchange definition with the callbacks needed to handle the
> client side of the DHE_RSA key exchange.
> ---
>   ell/tls-suites.c | 354 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 354 insertions(+)
> 

<snip>

> +static bool tls_create_dhe_keys(struct l_tls *tls)
> +{
> +	struct tls_dhe_params *params = tls->pending.key_xchg_params;
> +
> +	if (!params)
> +		return false;
> +
> +	if (params->prime_buf && !params->prime) {
> +		params->prime = l_key_new(L_KEY_RAW, params->prime_buf,
> +						params->prime_len);
> +		if (!params->prime) {
> +			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> +					"l_key_new(prime) failed");
> +			return false;
> +		}
> +	}
> +
> +	if (params->generator_buf && !params->generator) {
> +		params->generator = l_key_new(L_KEY_RAW, params->generator_buf,
> +						params->generator_len);
> +		if (!params->generator) {
> +			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> +					"l_key_new(generator) failed");
> +			return false;
> +		}
> +	}
> +
> +	if (params->private_buf && !params->private) {
> +		params->private = l_key_new(L_KEY_RAW, params->private_buf,
> +						params->private_len);
> +		if (!params->private) {
> +			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> +					"l_key_new(private) failed");
> +			return false;
> +		}
> +	}
> +
> +	if (params->public_buf && !params->public) {
> +		params->public = l_key_new(L_KEY_RAW, params->public_buf,
> +						params->public_len);
> +		if (!params->public) {
> +			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> +					"l_key_new(public) failed");
> +			return false;
> +		}
> +	}
> +
> +	return true;
> +}

It seems like the intent of this function is to be called more than 
once, piecemeal creating the l_key objects.  Why?  That type of pattern 
isn't terribly readable / maintainable.

> +
> +static bool tls_generate_dhe_private(struct l_tls *tls, uint8_t *buf, size_t len)
> +{
> +	struct tls_dhe_params *params = tls->pending.key_xchg_params;
> +
> +	do {
> +		unsigned int written = 0;
> +
> +		/* Leading zeros in the private value are fine */
> +
> +		do {
> +			size_t chunk_len =
> +				written + 256 <= len ? 256 : len - written;
> +
> +			if (!l_getrandom(buf + written, chunk_len)) {
> +				TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> +						"l_getrandom(buf, %zu) failed",
> +						chunk_len);
> +				return false;
> +			}
> +
> +			written += chunk_len;
> +		} while (written < len);

So where is the magic 256 byte thing is coming from?  Is this due to the 
EINTR possibility?  If so either use TFR() inside l_getrandom itself, or 
have l_getrandom perform this magic.  This blocking logic doesn't belong 
here.

> +	} while (!l_key_validate_dh_payload(buf, len,
> +						params->prime_buf,
> +						params->prime_len));

Generating a whole new random number is really a bad way of doing 
things.  The entropy pool is a very limited resource, so don't abuse it. 
  A much easier way would be to simply set the most-significant-byte to 
0.  Or maybe even just the most-significant-bit.  The prime numbers are 
usually always starting with 0xff, so fudging a single byte out of 192+ 
should be safe.

> +
> +	return true;
> +}
> +

<snip>

> +static bool tls_get_server_dh_params_hash(struct l_tls *tls, uint8_t tls_id,
> +						uint8_t *out, size_t *len,
> +						enum l_checksum_type *type)
> +{
> +	unsigned int hash;
> +	struct l_checksum *checksum;
> +	uint8_t params[32 + TLS_DHE_MAX_SIZE * 3];

Have you considered making this a static buf inside 
tls_write_server_dh_params?

> +	size_t params_len;
> +	ssize_t hash_len, ret;
> +
> +	params_len = tls_write_server_dh_params(tls, params);
> +
> +	for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
> +		if (tls_handshake_hash_data[hash].tls_id == tls_id)
> +			break;
> +
> +	if (hash == __HANDSHAKE_HASH_COUNT)
> +		return false;

Why not perform the search prior to tls_write_server_dh_params?

> +
> +	hash_len = tls_handshake_hash_data[hash].length;
> +
> +	checksum = l_checksum_new(tls_handshake_hash_data[hash].l_id);
> +	if (!checksum)
> +		return false;
> +
> +	l_checksum_update(checksum, tls->pending.client_random, 32);
> +	l_checksum_update(checksum, tls->pending.server_random, 32);
> +	l_checksum_update(checksum, params, params_len);
> +	ret = l_checksum_get_digest(checksum, out, hash_len);
> +	l_checksum_free(checksum);
> +
> +	if (ret != (ssize_t) hash_len)
> +		return false;
> +
> +	if (len)
> +		*len = hash_len;
> +
> +	if (type)
> +		*type = tls_handshake_hash_data[hash].l_id;
> +
> +	return true;
> +}
> +
> +static void tls_handle_dhe_server_key_xchg(struct l_tls *tls,
> +						const uint8_t *buf, size_t len)
> +{
> +	struct tls_dhe_params *params;
> +
> +	params = l_new(struct tls_dhe_params, 1);
> +	tls->pending.key_xchg_params = params;

Why do this assignment here?

> +
> +	if (len < 2)
> +		goto decode_error;
> +
> +	params->prime_len = l_get_be16(buf);
> +	if (len < 2 + params->prime_len + 2)
> +		goto decode_error;
> +
> +	params->prime_buf = l_memdup(buf + 2, params->prime_len);
> +	buf += 2 + params->prime_len;
> +	len -= 2 + params->prime_len;
> +
> +	params->generator_len = l_get_be16(buf);
> +	if (len < 2 + params->generator_len + 2)
> +		goto decode_error;
> +
> +	params->generator_buf = l_memdup(buf + 2, params->generator_len);;
> +	buf += 2 + params->generator_len;
> +	len -= 2 + params->generator_len;
> +
> +	params->public_len = l_get_be16(buf);
> +	if (len < 2 + params->public_len)
> +		goto decode_error;
> +
> +	params->public_buf = buf + 2;
> +	buf += 2 + params->public_len;
> +	len -= 2 + params->public_len;
> +
> +	/*
> +	 * Validate the values received.  Without RFC 7919 we basically have
> +	 * to blindly accept the provided prime value.  We have no way to
> +	 * confirm that it's actually prime or that it's a "safe prime" or
> +	 * that it forms a group without small sub-groups.  There's also no
> +	 * way to whitelist all valid values.  But we do a basic sanity
> +	 * check and require it to be 1024-bit or longer (see weakdh.org),
> +	 * might need to move to 2048 bits actually.
> +	 */
> +
> +	if (params->prime_len > TLS_DHE_MAX_SIZE ||
> +			params->generator_len > params->prime_len) {
> +		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
> +				"Server DH prime modulus or generator "
> +				"too long");
> +		return;
> +	}
> +
> +	if (params->prime_len < 128 || params->prime_buf[0] == 0x00 ||
> +			!(params->prime_buf[params->prime_len - 1] & 1)) {
> +		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
> +				"Server DH prime modulus invalid");
> +		return;
> +	}
> +
> +	/*
> +	 * RFC 7919 Section 3.0:
> +	 * "the client MUST verify that dh_Ys is in the range
> +	 * 1 < dh_Ys < dh_p - 1.  If dh_Ys is not in this range, the client
> +	 * MUST terminate the connection with a fatal handshake_failure(40)
> +	 * alert."
> +	 */
> +	if (params->public_len > params->prime_len ||
> +			!l_key_validate_dh_payload(params->public_buf,
> +							params->public_len,
> +							params->prime_buf,
> +							params->prime_len)) {

So can we rely on the kernel to do this part for us?

> +		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
> +				"Server DH public value invalid");
> +		return;
> +	}
> +
> +	if (!tls_create_dhe_keys(tls))
> +		return;
> +
> +	if (!tls->pending.cipher_suite->key_xchg->verify(tls, buf, len,
> +						tls_get_server_dh_params_hash))
> +		return;
> +
> +	TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO_DONE);
> +	return;
> +
> +decode_error:
> +	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
> +			"ServerKeyExchange decode error");
> +}
> +

Regards,
-Denis

Re: [PATCH 13/17] key: Add l_key_validate_dh_payload

[Andrew Zaborowski]

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

Hi Denis,

On Wed, 2 Jan 2019 at 21:47, Denis Kenzior <denkenz@gmail.com> wrote:
> On 01/01/2019 01:49 PM, Andrew Zaborowski wrote:
> > Add a function to validate local private value payloads against a
> > the group's prime before creating kernel keys out of them, and to
>
> Do we have the same issue in iwd EAP-WSC?

I was going to send a patch for that later, we probably should
validate the values but obviously the probability of the check failing
in the first iteration is vert low.  In TLS I guess it's higher
because we may have to deal with a custom prime, but I didn't add this
because of actually hitting a problem.

>
> > validate public values received from the peer.
>
> Note that the kernel already performs almost the same check in
> dh_is_pubkey_valid

Oh true, this was apprently added in July.  So as far as I understand
that function is called in dh_compute so our compute_dh_secret should
fail.  But, in unit/test-key we have this "dh_degenerate" test case
and it's passing, I have to look at when this check is supposed to
trigger in the kernel, maybe we can still use it.

>
> Might make more sense to create an API to generate a valid private key
> with the needed constraints.

Yep, may be a good idea, and this could hide the 256 byte limit for
l_getrandom too (I was also thinking maybe l_getrandom itself should
try to hide that detail)

> E.g. by twiddling the MSBs to 0 until the
> private key is less than the prime.

I think this is not normally done so as not to affect the probability
distribution of the private values.  But we probably should at least
clear the top few bits of the random number that we know have to be 0
for the number to be lower than the prime.  A TLS client can
potentially a receive a prime from the server that has 01 in the first
byte and then on average it'd need 256 iterations before it draws a
value lower than the prime the naive way.

Best regards

Re: [PATCH 02/17] tls: Add Hello extension sending support

[Denis Kenzior]

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

Hi Andrew,

On 01/01/2019 01:49 PM, Andrew Zaborowski wrote:
> Declare a struct for ServerHello/ClientHello extensions with write and
> handle callbacks, modify our ServerHello and ClientHello message
> builders to append the extensions to the messages.
> ---
>   ell/tls-private.h |  18 +++++++++
>   ell/tls.c         | 101 ++++++++++++++++++++++++++++++++++++++++++----
>   2 files changed, 112 insertions(+), 7 deletions(-)
> 

<snip>

> @@ -812,11 +814,12 @@ static void tls_tx_handshake(struct l_tls *tls, int type, uint8_t *buf,
>   
>   static bool tls_send_client_hello(struct l_tls *tls)
>   {
> -	uint8_t buf[128 + L_ARRAY_SIZE(tls_compression_pref) +
> +	uint8_t buf[1024 + L_ARRAY_SIZE(tls_compression_pref) +
>   			2 * L_ARRAY_SIZE(tls_cipher_suite_pref)];

So you're sort of depending on the buffer to be large enough, maybe not 
a great strategy long-term.

>   	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
>   	uint8_t *len_ptr;
> -	int i;
> +	unsigned int i;
> +	uint8_t *extensions_len_ptr;
>   
>   	/* Fill in the Client Hello body */
>   
> @@ -832,7 +835,7 @@ static bool tls_send_client_hello(struct l_tls *tls)
>   	len_ptr = ptr;
>   	ptr += 2;
>   
> -	for (i = 0; i < (int) L_ARRAY_SIZE(tls_cipher_suite_pref); i++) {
> +	for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++) {
>   		if (!tls_cipher_suite_is_compatible(tls,
>   						&tls_cipher_suite_pref[i],
>   						NULL))
> @@ -850,17 +853,58 @@ static bool tls_send_client_hello(struct l_tls *tls)
>   	l_put_be16((ptr - len_ptr - 2), len_ptr);
>   	*ptr++ = L_ARRAY_SIZE(tls_compression_pref);
>   
> -	for (i = 0; i < (int) L_ARRAY_SIZE(tls_compression_pref); i++)
> +	for (i = 0; i < L_ARRAY_SIZE(tls_compression_pref); i++)
>   		*ptr++ = tls_compression_pref[i].id;
>   
> +	extensions_len_ptr = ptr;
> +	ptr += 2;
> +
> +	for (i = 0; tls_extensions[i].name; i++) {
> +		const struct tls_hello_extension *extension =
> +			&tls_extensions[i];
> +		ssize_t ext_len;
> +
> +		/*
> +		 * Note: could handle NULL client_write with non-NULL
> +		 * server_handle or server_handle_absent as "server-oriented"
> +		 * extension (7.4.1.4) and write empty extension_data and
> +		 * simliarly require empty extension_data in
> +		 * tls_handle_client_hello if client_handle NULL.
> +		 */
> +		if (!extension->client_write)
> +			continue;
> +
> +		ext_len = extension->client_write(tls, ptr + 4,
> +						buf + sizeof(buf) - (ptr + 4));
> +		if (ext_len == -ENOMSG)
> +			continue;
> +
> +		if (ext_len < 0) {
> +			TLS_DEBUG("%s extension's client_write: %s",
> +					extension->name, strerror(-ext_len));
> +			return false;
> +		}
> +
> +		l_put_be16(extension->id, ptr + 0);
> +		l_put_be16(ext_len, ptr + 2);
> +		ptr += 4 + ext_len;
> +	}
> +
> +	if (ptr > extensions_len_ptr + 2)
> +		l_put_be16(ptr - (extensions_len_ptr + 2), extensions_len_ptr);
> +	else /* Skip the length if no extensions */
> +		ptr = extensions_len_ptr;
> +
>   	tls_tx_handshake(tls, TLS_CLIENT_HELLO, buf, ptr - buf);
>   	return true;
>   }
>   
> -static void tls_send_server_hello(struct l_tls *tls)
> +static bool tls_send_server_hello(struct l_tls *tls, struct l_queue *extensions)
>   {
> -	uint8_t buf[128];
> +	uint8_t buf[1024];
>   	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
> +	uint8_t *extensions_len_ptr;
> +	const struct l_queue_entry *entry;
>   
>   	/* Fill in the Server Hello body */
>   
> @@ -878,7 +922,49 @@ static void tls_send_server_hello(struct l_tls *tls)
>   
>   	*ptr++ = tls->pending.compression_method->id;
>   
> +	extensions_len_ptr = ptr;
> +	ptr += 2;
> +
> +	for (entry = l_queue_get_entries(extensions); entry;
> +			entry = entry->next) {
> +		uint16_t ext_id = L_PTR_TO_UINT(entry->data);
> +		const struct tls_hello_extension *extension = NULL;
> +		unsigned int i;
> +		ssize_t ext_len;
> +
> +		for (i = 0; tls_extensions[i].name; i++)
> +			if (tls_extensions[i].id == ext_id) {
> +				extension = &tls_extensions[i];
> +				break;
> +			}
> +
> +		if (!extension || !extension->server_write)
> +			continue;
> +
> +		ext_len = extension->server_write(tls, ptr + 4,
> +						buf + sizeof(buf) - (ptr + 4));
> +		if (ext_len == -ENOMSG)
> +			continue;
> +
> +		if (ext_len < 0) {
> +			TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> +					"%s extension's server_write: %s",
> +					extension->name, strerror(-ext_len));
> +			return false;
> +		}
> +
> +		l_put_be16(ext_id, ptr + 0);
> +		l_put_be16(ext_len, ptr + 2);
> +		ptr += 4 + ext_len;
> +	}
> +
> +	if (ptr > extensions_len_ptr + 2)
> +		l_put_be16(ptr - (extensions_len_ptr + 2), extensions_len_ptr);
> +	else /* Skip the length if no extensions */
> +		ptr = extensions_len_ptr;
> +
>   	tls_tx_handshake(tls, TLS_SERVER_HELLO, buf, ptr - buf);
> +	return true;
>   }
>   

So both the server and the client versions are almost the same code. 
Signatures of client_write and server_write are also identical.  Can 
these two functions be combined?

Also, don't you want the supported client extensions to also be 
dynamically configurable per tls object?

Regards,
-Denis

Re: [PATCH 06/17] tls: Allow user to set custom list of cipher suites

[Denis Kenzior]

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

Hi Andrew,

On 01/01/2019 01:49 PM, Andrew Zaborowski wrote:
> TLS "security profiles" define which minimum cipher suites and other
> parameter values are allowed in a specific usage scenario for TLS, for
> example WPA3 in theory requires a specific security profile for its
> TLS-based EAP method.  This will also allow the unit tests to test
> individual cipher suites.
> ---
>   ell/tls-private.h |   5 +
>   ell/tls.c         | 260 +++++++++++++++++++++++++++++++---------------
>   2 files changed, 181 insertions(+), 84 deletions(-)

Looks good to me.

Regards,
-Denis

Re: [PATCH 14/17] tls: DHE_RSA key exchange implementation client side

[Andrew Zaborowski]

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

Hi Denis,

On Wed, 2 Jan 2019 at 23:33, Denis Kenzior <denkenz@gmail.com> wrote:
> On 01/01/2019 01:49 PM, Andrew Zaborowski wrote:
> > +static bool tls_create_dhe_keys(struct l_tls *tls)
> > +{
> > +     struct tls_dhe_params *params = tls->pending.key_xchg_params;
> > +
> > +     if (!params)
> > +             return false;
> > +
> > +     if (params->prime_buf && !params->prime) {
> > +             params->prime = l_key_new(L_KEY_RAW, params->prime_buf,
> > +                                             params->prime_len);
> > +             if (!params->prime) {
> > +                     TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> > +                                     "l_key_new(prime) failed");
> > +                     return false;
> > +             }
> > +     }
> > +
> > +     if (params->generator_buf && !params->generator) {
> > +             params->generator = l_key_new(L_KEY_RAW, params->generator_buf,
> > +                                             params->generator_len);
> > +             if (!params->generator) {
> > +                     TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> > +                                     "l_key_new(generator) failed");
> > +                     return false;
> > +             }
> > +     }
> > +
> > +     if (params->private_buf && !params->private) {
> > +             params->private = l_key_new(L_KEY_RAW, params->private_buf,
> > +                                             params->private_len);
> > +             if (!params->private) {
> > +                     TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> > +                                     "l_key_new(private) failed");
> > +                     return false;
> > +             }
> > +     }
> > +
> > +     if (params->public_buf && !params->public) {
> > +             params->public = l_key_new(L_KEY_RAW, params->public_buf,
> > +                                             params->public_len);
> > +             if (!params->public) {
> > +                     TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> > +                                     "l_key_new(public) failed");
> > +                     return false;
> > +             }
> > +     }
> > +
> > +     return true;
> > +}
>
> It seems like the intent of this function is to be called more than
> once, piecemeal creating the l_key objects.  Why?  That type of pattern
> isn't terribly readable / maintainable.

I guess the intent is to simplify error handling in the callers.  I
can perhaps split this in 4 separate functions, to avoid inlining all
the l_key_new and TLS_DISCONNECT calls.

>
> > +
> > +static bool tls_generate_dhe_private(struct l_tls *tls, uint8_t *buf, size_t len)
> > +{
> > +     struct tls_dhe_params *params = tls->pending.key_xchg_params;
> > +
> > +     do {
> > +             unsigned int written = 0;
> > +
> > +             /* Leading zeros in the private value are fine */
> > +
> > +             do {
> > +                     size_t chunk_len =
> > +                             written + 256 <= len ? 256 : len - written;
> > +
> > +                     if (!l_getrandom(buf + written, chunk_len)) {
> > +                             TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> > +                                             "l_getrandom(buf, %zu) failed",
> > +                                             chunk_len);
> > +                             return false;
> > +                     }
> > +
> > +                     written += chunk_len;
> > +             } while (written < len);
>
> So where is the magic 256 byte thing is coming from?

It's to workaround this check in random.c:

62:        if (len > 256)
63:                return false;

I don't know exactly why it's needed but I'll play with it and see if
we can remove/work around this limit.

> Is this due to the
> EINTR possibility?  If so either use TFR() inside l_getrandom itself, or
> have l_getrandom perform this magic.  This blocking logic doesn't belong
> here.
>
> > +     } while (!l_key_validate_dh_payload(buf, len,
> > +                                             params->prime_buf,
> > +                                             params->prime_len));
>
> Generating a whole new random number is really a bad way of doing
> things.  The entropy pool is a very limited resource, so don't abuse it.
>   A much easier way would be to simply set the most-significant-byte to
> 0.

It's a pattern we use in the ecdh code (where the group order is only
~1/2 of the number space -- can improved by switching from the "Black
box" method to the "Deterministic" method iirc) and in iwd so we'd
probably need to change it everywhere.  But it seems that every way to
"fix" the number instead of generating a new one affects the
distribution making the algorithm weaker.  If that's ok then yes, we
can simply force the top bits of the private value do "0" (down to the
first "1" bit of the prime).

> Or maybe even just the most-significant-bit. The prime numbers are
> usually always starting with 0xff, so fudging a single byte out of 192+
> should be safe.

Not in TLS, most of the https servers I test against use custom
generated prime numbers, I guess the openssl or gnutls package
generates a random (likely-to-be) prime at install.

>
> > +
> > +     return true;
> > +}
> > +
>
> <snip>
>
> > +static bool tls_get_server_dh_params_hash(struct l_tls *tls, uint8_t tls_id,
> > +                                             uint8_t *out, size_t *len,
> > +                                             enum l_checksum_type *type)
> > +{
> > +     unsigned int hash;
> > +     struct l_checksum *checksum;
> > +     uint8_t params[32 + TLS_DHE_MAX_SIZE * 3];
>
> Have you considered making this a static buf inside
> tls_write_server_dh_params?

I guess I can do that.

>
> > +     size_t params_len;
> > +     ssize_t hash_len, ret;
> > +
> > +     params_len = tls_write_server_dh_params(tls, params);
> > +
> > +     for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
> > +             if (tls_handshake_hash_data[hash].tls_id == tls_id)
> > +                     break;
> > +
> > +     if (hash == __HANDSHAKE_HASH_COUNT)
> > +             return false;
>
> Why not perform the search prior to tls_write_server_dh_params?

Ok.

>
> > +
> > +     hash_len = tls_handshake_hash_data[hash].length;
> > +
> > +     checksum = l_checksum_new(tls_handshake_hash_data[hash].l_id);
> > +     if (!checksum)
> > +             return false;
> > +
> > +     l_checksum_update(checksum, tls->pending.client_random, 32);
> > +     l_checksum_update(checksum, tls->pending.server_random, 32);
> > +     l_checksum_update(checksum, params, params_len);
> > +     ret = l_checksum_get_digest(checksum, out, hash_len);
> > +     l_checksum_free(checksum);
> > +
> > +     if (ret != (ssize_t) hash_len)
> > +             return false;
> > +
> > +     if (len)
> > +             *len = hash_len;
> > +
> > +     if (type)
> > +             *type = tls_handshake_hash_data[hash].l_id;
> > +
> > +     return true;
> > +}
> > +
> > +static void tls_handle_dhe_server_key_xchg(struct l_tls *tls,
> > +                                             const uint8_t *buf, size_t len)
> > +{
> > +     struct tls_dhe_params *params;
> > +
> > +     params = l_new(struct tls_dhe_params, 1);
> > +     tls->pending.key_xchg_params = params;
>
> Why do this assignment here?

The reason is so that in case of error tls_reset_handshake will free
this and we don't have to worry about it, I can add a comment.

>
> > +
> > +     if (len < 2)
> > +             goto decode_error;
> > +
> > +     params->prime_len = l_get_be16(buf);
> > +     if (len < 2 + params->prime_len + 2)
> > +             goto decode_error;
> > +
> > +     params->prime_buf = l_memdup(buf + 2, params->prime_len);
> > +     buf += 2 + params->prime_len;
> > +     len -= 2 + params->prime_len;
> > +
> > +     params->generator_len = l_get_be16(buf);
> > +     if (len < 2 + params->generator_len + 2)
> > +             goto decode_error;
> > +
> > +     params->generator_buf = l_memdup(buf + 2, params->generator_len);;
> > +     buf += 2 + params->generator_len;
> > +     len -= 2 + params->generator_len;
> > +
> > +     params->public_len = l_get_be16(buf);
> > +     if (len < 2 + params->public_len)
> > +             goto decode_error;
> > +
> > +     params->public_buf = buf + 2;
> > +     buf += 2 + params->public_len;
> > +     len -= 2 + params->public_len;
> > +
> > +     /*
> > +      * Validate the values received.  Without RFC 7919 we basically have
> > +      * to blindly accept the provided prime value.  We have no way to
> > +      * confirm that it's actually prime or that it's a "safe prime" or
> > +      * that it forms a group without small sub-groups.  There's also no
> > +      * way to whitelist all valid values.  But we do a basic sanity
> > +      * check and require it to be 1024-bit or longer (see weakdh.org),
> > +      * might need to move to 2048 bits actually.
> > +      */
> > +
> > +     if (params->prime_len > TLS_DHE_MAX_SIZE ||
> > +                     params->generator_len > params->prime_len) {
> > +             TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
> > +                             "Server DH prime modulus or generator "
> > +                             "too long");
> > +             return;
> > +     }
> > +
> > +     if (params->prime_len < 128 || params->prime_buf[0] == 0x00 ||
> > +                     !(params->prime_buf[params->prime_len - 1] & 1)) {
> > +             TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
> > +                             "Server DH prime modulus invalid");
> > +             return;
> > +     }
> > +
> > +     /*
> > +      * RFC 7919 Section 3.0:
> > +      * "the client MUST verify that dh_Ys is in the range
> > +      * 1 < dh_Ys < dh_p - 1.  If dh_Ys is not in this range, the client
> > +      * MUST terminate the connection with a fatal handshake_failure(40)
> > +      * alert."
> > +      */
> > +     if (params->public_len > params->prime_len ||
> > +                     !l_key_validate_dh_payload(params->public_buf,
> > +                                                     params->public_len,
> > +                                                     params->prime_buf,
> > +                                                     params->prime_len)) {
>
> So can we rely on the kernel to do this part for us?

Apparently we can't without modifying security/keys/dh.c and our keys
code.  The keyctl operation is too smart and assumes
generate_public_key and generate_shared_secret are the same operation,
so when crypto/dh.c receives the parameters it always thinks it's
dealing with a generator and the private value, instead of private and
public values.  So it never runs the public value validation.
dh_is_pubkey_valid also has one minor unimplemented feature.

Best regards

Re: [PATCH 14/17] tls: DHE_RSA key exchange implementation client side

[Denis Kenzior]

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

Hi Andrew,

>> It seems like the intent of this function is to be called more than
>> once, piecemeal creating the l_key objects.  Why?  That type of pattern
>> isn't terribly readable / maintainable.
> 
> I guess the intent is to simplify error handling in the callers.  I
> can perhaps split this in 4 separate functions, to avoid inlining all
> the l_key_new and TLS_DISCONNECT calls.
> 

By my reading it seems like this is only needs to be called once for the 
client side.  I can see how this is more useful for the server side.

>> So where is the magic 256 byte thing is coming from?
> 
> It's to workaround this check in random.c:
> 
> 62:        if (len > 256)
> 63:                return false;
> 
> I don't know exactly why it's needed but I'll play with it and see if
> we can remove/work around this limit.

So that check is there due to EINTR.  getrandom guarantees to succeed 
for len up to 256.  After that EINTR is a possibility.  We never needed 
random buffers bigger than 256, so it was a lazy fix.

>> Generating a whole new random number is really a bad way of doing
>> things.  The entropy pool is a very limited resource, so don't abuse it.
>>    A much easier way would be to simply set the most-significant-byte to
>> 0.
> 
> It's a pattern we use in the ecdh code (where the group order is only
> ~1/2 of the number space -- can improved by switching from the "Black
> box" method to the "Deterministic" method iirc) and in iwd so we'd

Yeah we might need to fix that, but there might not be an easy way as 
opposed to here.

> probably need to change it everywhere.  But it seems that every way to
> "fix" the number instead of generating a new one affects the
> distribution making the algorithm weaker.  If that's ok then yes, we
> can simply force the top bits of the private value do "0" (down to the
> first "1" bit of the prime).

This would be fine with all DH groups from RFC 3526.  If TLS uses a 
different set of primes, then setting the upper MSB bits to 0 might be 
okay.  If you're worried about this, then perhaps simply re-generating 
the upper N MSB bytes is another solution.  Might want to check what 
other implementations do.

>>> +static void tls_handle_dhe_server_key_xchg(struct l_tls *tls,
>>> +                                             const uint8_t *buf, size_t len)
>>> +{
>>> +     struct tls_dhe_params *params;
>>> +
>>> +     params = l_new(struct tls_dhe_params, 1);
>>> +     tls->pending.key_xchg_params = params;
>>
>> Why do this assignment here?
> 
> The reason is so that in case of error tls_reset_handshake will free
> this and we don't have to worry about it, I can add a comment.

Gah, that's really not obvious.  A comment is needed, but it might be 
better to have decode_error label take care of this instead.

>>> +     /*
>>> +      * RFC 7919 Section 3.0:
>>> +      * "the client MUST verify that dh_Ys is in the range
>>> +      * 1 < dh_Ys < dh_p - 1.  If dh_Ys is not in this range, the client
>>> +      * MUST terminate the connection with a fatal handshake_failure(40)
>>> +      * alert."
>>> +      */
>>> +     if (params->public_len > params->prime_len ||
>>> +                     !l_key_validate_dh_payload(params->public_buf,
>>> +                                                     params->public_len,
>>> +                                                     params->prime_buf,
>>> +                                                     params->prime_len)) {
>>
>> So can we rely on the kernel to do this part for us?
> 
> Apparently we can't without modifying security/keys/dh.c and our keys
> code.  The keyctl operation is too smart and assumes
> generate_public_key and generate_shared_secret are the same operation,
> so when crypto/dh.c receives the parameters it always thinks it's
> dealing with a generator and the private value, instead of private and
> public values.  So it never runs the public value validation.
> dh_is_pubkey_valid also has one minor unimplemented feature.

That's a bug in the kernel then.  We should fix that.

Regards,
-Denis

Re: [PATCH 14/17] tls: DHE_RSA key exchange implementation client side

[Andrew Zaborowski]

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

On Thu, 3 Jan 2019 at 04:33, Denis Kenzior <denkenz@gmail.com> wrote:
> >> Generating a whole new random number is really a bad way of doing
> >> things.  The entropy pool is a very limited resource, so don't abuse it.
> >>    A much easier way would be to simply set the most-significant-byte to
> >> 0.
> >
> > It's a pattern we use in the ecdh code (where the group order is only
> > ~1/2 of the number space -- can improved by switching from the "Black
> > box" method to the "Deterministic" method iirc) and in iwd so we'd
>
> Yeah we might need to fix that, but there might not be an easy way as
> opposed to here.
>
> > probably need to change it everywhere.  But it seems that every way to
> > "fix" the number instead of generating a new one affects the
> > distribution making the algorithm weaker.  If that's ok then yes, we
> > can simply force the top bits of the private value do "0" (down to the
> > first "1" bit of the prime).
>
> This would be fine with all DH groups from RFC 3526.  If TLS uses a
> different set of primes, then setting the upper MSB bits to 0 might be
> okay.

PKCS #3 also mentions generating a priv key between 2^(n-1) and
(2^n)-1, meaning forcing an MSB to 1, which accounts for both the min
and max checks so I think I'm going to do that if we don't care about
uniform distribution.

> If you're worried about this, then perhaps simply re-generating
> the upper N MSB bytes is another solution.

Unfortunately this still would skew the distribution a little.

> Might want to check what
> other implementations do.
>
> >>> +static void tls_handle_dhe_server_key_xchg(struct l_tls *tls,
> >>> +                                             const uint8_t *buf, size_t len)
> >>> +{
> >>> +     struct tls_dhe_params *params;
> >>> +
> >>> +     params = l_new(struct tls_dhe_params, 1);
> >>> +     tls->pending.key_xchg_params = params;
> >>
> >> Why do this assignment here?
> >
> > The reason is so that in case of error tls_reset_handshake will free
> > this and we don't have to worry about it, I can add a comment.
>
> Gah, that's really not obvious.  A comment is needed, but it might be
> better to have decode_error label take care of this instead.
>
> >>> +     /*
> >>> +      * RFC 7919 Section 3.0:
> >>> +      * "the client MUST verify that dh_Ys is in the range
> >>> +      * 1 < dh_Ys < dh_p - 1.  If dh_Ys is not in this range, the client
> >>> +      * MUST terminate the connection with a fatal handshake_failure(40)
> >>> +      * alert."
> >>> +      */
> >>> +     if (params->public_len > params->prime_len ||
> >>> +                     !l_key_validate_dh_payload(params->public_buf,
> >>> +                                                     params->public_len,
> >>> +                                                     params->prime_buf,
> >>> +                                                     params->prime_len)) {
> >>
> >> So can we rely on the kernel to do this part for us?
> >
> > Apparently we can't without modifying security/keys/dh.c and our keys
> > code.  The keyctl operation is too smart and assumes
> > generate_public_key and generate_shared_secret are the same operation,
> > so when crypto/dh.c receives the parameters it always thinks it's
> > dealing with a generator and the private value, instead of private and
> > public values.  So it never runs the public value validation.
> > dh_is_pubkey_valid also has one minor unimplemented feature.
>
> That's a bug in the kernel then.  We should fix that.

As far as I understand we'd also need to fix the API then because
keyctl has no way to know which operation we're requesting.  I.e.
split KEYCTL_DH_COMPUTE into two operations or add a field to struct
keyctl_dh_params.  We could also force the check for both private and
public values but others might argue the requirements for private
values may be less strict (in RFC7919 they're the same though, iirc),
or the min and max may have been checked already.

Best regards

Re: [PATCH 02/17] tls: Add Hello extension sending support

[Andrew Zaborowski]

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

On Thu, 3 Jan 2019 at 01:40, Denis Kenzior <denkenz@gmail.com> wrote:
> On 01/01/2019 01:49 PM, Andrew Zaborowski wrote:
> >   static bool tls_send_client_hello(struct l_tls *tls)
> >   {
> > -     uint8_t buf[128 + L_ARRAY_SIZE(tls_compression_pref) +
> > +     uint8_t buf[1024 + L_ARRAY_SIZE(tls_compression_pref) +
> >                       2 * L_ARRAY_SIZE(tls_cipher_suite_pref)];
>
> So you're sort of depending on the buffer to be large enough, maybe not
> a great strategy long-term.

True, but there's always a fixed maximum size we need, only it's
tricky to calculate that size dynamically because we'd need to
complicate the API.
>
> >       uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
> >       uint8_t *len_ptr;
> > -     int i;
> > +     unsigned int i;
> > +     uint8_t *extensions_len_ptr;
> >
> >       /* Fill in the Client Hello body */
> >
> > @@ -832,7 +835,7 @@ static bool tls_send_client_hello(struct l_tls *tls)
> >       len_ptr = ptr;
> >       ptr += 2;
> >
> > -     for (i = 0; i < (int) L_ARRAY_SIZE(tls_cipher_suite_pref); i++) {
> > +     for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++) {
> >               if (!tls_cipher_suite_is_compatible(tls,
> >                                               &tls_cipher_suite_pref[i],
> >                                               NULL))
> > @@ -850,17 +853,58 @@ static bool tls_send_client_hello(struct l_tls *tls)
> >       l_put_be16((ptr - len_ptr - 2), len_ptr);
> >       *ptr++ = L_ARRAY_SIZE(tls_compression_pref);
> >
> > -     for (i = 0; i < (int) L_ARRAY_SIZE(tls_compression_pref); i++)
> > +     for (i = 0; i < L_ARRAY_SIZE(tls_compression_pref); i++)
> >               *ptr++ = tls_compression_pref[i].id;
> >
> > +     extensions_len_ptr = ptr;
> > +     ptr += 2;
> > +
> > +     for (i = 0; tls_extensions[i].name; i++) {
> > +             const struct tls_hello_extension *extension =
> > +                     &tls_extensions[i];
> > +             ssize_t ext_len;
> > +
> > +             /*
> > +              * Note: could handle NULL client_write with non-NULL
> > +              * server_handle or server_handle_absent as "server-oriented"
> > +              * extension (7.4.1.4) and write empty extension_data and
> > +              * simliarly require empty extension_data in
> > +              * tls_handle_client_hello if client_handle NULL.
> > +              */
> > +             if (!extension->client_write)
> > +                     continue;
> > +
> > +             ext_len = extension->client_write(tls, ptr + 4,
> > +                                             buf + sizeof(buf) - (ptr + 4));
> > +             if (ext_len == -ENOMSG)
> > +                     continue;
> > +
> > +             if (ext_len < 0) {
> > +                     TLS_DEBUG("%s extension's client_write: %s",
> > +                                     extension->name, strerror(-ext_len));
> > +                     return false;
> > +             }
> > +
> > +             l_put_be16(extension->id, ptr + 0);
> > +             l_put_be16(ext_len, ptr + 2);
> > +             ptr += 4 + ext_len;
> > +     }
> > +
> > +     if (ptr > extensions_len_ptr + 2)
> > +             l_put_be16(ptr - (extensions_len_ptr + 2), extensions_len_ptr);
> > +     else /* Skip the length if no extensions */
> > +             ptr = extensions_len_ptr;
> > +
> >       tls_tx_handshake(tls, TLS_CLIENT_HELLO, buf, ptr - buf);
> >       return true;
> >   }
> >
> > -static void tls_send_server_hello(struct l_tls *tls)
> > +static bool tls_send_server_hello(struct l_tls *tls, struct l_queue *extensions)
> >   {
> > -     uint8_t buf[128];
> > +     uint8_t buf[1024];
> >       uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
> > +     uint8_t *extensions_len_ptr;
> > +     const struct l_queue_entry *entry;
> >
> >       /* Fill in the Server Hello body */
> >
> > @@ -878,7 +922,49 @@ static void tls_send_server_hello(struct l_tls *tls)
> >
> >       *ptr++ = tls->pending.compression_method->id;
> >
> > +     extensions_len_ptr = ptr;
> > +     ptr += 2;
> > +
> > +     for (entry = l_queue_get_entries(extensions); entry;
> > +                     entry = entry->next) {
> > +             uint16_t ext_id = L_PTR_TO_UINT(entry->data);
> > +             const struct tls_hello_extension *extension = NULL;
> > +             unsigned int i;
> > +             ssize_t ext_len;
> > +
> > +             for (i = 0; tls_extensions[i].name; i++)
> > +                     if (tls_extensions[i].id == ext_id) {
> > +                             extension = &tls_extensions[i];
> > +                             break;
> > +                     }
> > +
> > +             if (!extension || !extension->server_write)
> > +                     continue;
> > +
> > +             ext_len = extension->server_write(tls, ptr + 4,
> > +                                             buf + sizeof(buf) - (ptr + 4));
> > +             if (ext_len == -ENOMSG)
> > +                     continue;
> > +
> > +             if (ext_len < 0) {
> > +                     TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> > +                                     "%s extension's server_write: %s",
> > +                                     extension->name, strerror(-ext_len));
> > +                     return false;
> > +             }
> > +
> > +             l_put_be16(ext_id, ptr + 0);
> > +             l_put_be16(ext_len, ptr + 2);
> > +             ptr += 4 + ext_len;
> > +     }
> > +
> > +     if (ptr > extensions_len_ptr + 2)
> > +             l_put_be16(ptr - (extensions_len_ptr + 2), extensions_len_ptr);
> > +     else /* Skip the length if no extensions */
> > +             ptr = extensions_len_ptr;
> > +
> >       tls_tx_handshake(tls, TLS_SERVER_HELLO, buf, ptr - buf);
> > +     return true;
> >   }
> >
>
> So both the server and the client versions are almost the same code.
> Signatures of client_write and server_write are also identical.  Can
> these two functions be combined?

Ok.

>
> Also, don't you want the supported client extensions to also be
> dynamically configurable per tls object?

We can add this at some point but I'm not sure if there's any use for
disabling extensions (probably less than for disabling cipher suites
which we decided to have no public api for)

Best regards