[PATCH 1/9] cert/key: Add support for EC based certificates

[PATCH 1/9] cert/key: Add support for EC based certificates

[Denis Kenzior]

Mostly for use with Elliptic Curve (EC) Digital Signature
Algorithm (DSA) based certificates.  Other combinations of EC +
signature algorithms are also possible.

This requires your kernel to be built with CRYPTO_ECDSA support.
---

NOTE: At the time this patch was created, kernel had to be patched with
the following fix in order for ECDSA support to function properly from
userspace:
https://lore.kernel.org/linux-crypto/20220715182810.30505-1-denkenz@gmail.com/

 ell/cert.c | 18 ++++++++++++++++--
 ell/cert.h |  1 +
 ell/key.c  |  1 +
 ell/key.h  |  1 +
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/ell/cert.c b/ell/cert.c
index 141ea1cec038..a158142445ec 100644
--- a/ell/cert.c
+++ b/ell/cert.c
@@ -77,7 +77,15 @@ static const struct pkcs1_encryption_oid {
 } pkcs1_encryption_oids[] = {
 	{ /* rsaEncryption */
 		L_CERT_KEY_RSA,
-		{ 9, { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01 } },
+		{ .asn1_len = 9, .asn1 = {
+			0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01 }
+		},
+	},
+	{ /* ecPublicKey */
+		L_CERT_KEY_ECC,
+		{ .asn1_len = 7, .asn1 = {
+			0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01 }
+		},
 	},
 };
 
@@ -261,8 +269,14 @@ LIB_EXPORT struct l_key *l_cert_get_pubkey(struct l_cert *cert)
 		return NULL;
 
 	/* Use kernel's ASN.1 certificate parser to find the key data for us */
-	if (cert->pubkey_type == L_CERT_KEY_RSA)
+	switch (cert->pubkey_type) {
+	case L_CERT_KEY_RSA:
 		return l_key_new(L_KEY_RSA, cert->asn1, cert->asn1_len);
+	case L_CERT_KEY_ECC:
+		return l_key_new(L_KEY_ECC, cert->asn1, cert->asn1_len);
+	case L_CERT_KEY_UNKNOWN:
+		break;
+	}
 
 	return NULL;
 }
diff --git a/ell/cert.h b/ell/cert.h
index 605e427c3d05..f637588e6d66 100644
--- a/ell/cert.h
+++ b/ell/cert.h
@@ -36,6 +36,7 @@ struct l_certchain;
 
 enum l_cert_key_type {
 	L_CERT_KEY_RSA,
+	L_CERT_KEY_ECC,
 	L_CERT_KEY_UNKNOWN,
 };
 
diff --git a/ell/key.c b/ell/key.c
index b28bf4dbf085..73f38581f736 100644
--- a/ell/key.c
+++ b/ell/key.c
@@ -108,6 +108,7 @@ struct l_keyring {
 static const char * const key_type_names[] = {
 	[L_KEY_RAW] = "user",
 	[L_KEY_RSA] = "asymmetric",
+	[L_KEY_ECC] = "asymmetric",
 };
 
 static long kernel_add_key(const char *type, const char *description,
diff --git a/ell/key.h b/ell/key.h
index d25d09385b6f..f26f7ecb26c3 100644
--- a/ell/key.h
+++ b/ell/key.h
@@ -45,6 +45,7 @@ enum l_key_feature {
 enum l_key_type {
 	L_KEY_RAW = 0,
 	L_KEY_RSA,
+	L_KEY_ECC,
 };
 
 enum l_keyring_restriction {
-- 
2.35.1

[PATCH 2/9] unit: Add basic EC-DSA verification test

[Denis Kenzior]

---
 .gitignore      |  2 ++
 Makefile.am     | 39 ++++++++++++++++++++++++++++++++++++++-
 unit/test-tls.c | 22 +++++++++++++++++++++-
 3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 346243a8f9c7..76f10aecfdd3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -67,6 +67,8 @@ unit/cert-*.csr
 unit/cert-*.srl
 unit/cert-*.crt
 unit/cert-*.p12
+unit/ec-cert-*.pem
+unit/ec-cert-*.csr
 unit/key-*.dat
 unit/key-*.h
 unit/*.log
diff --git a/Makefile.am b/Makefile.am
index 2bf728bbde7a..e5d7143af236 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -219,6 +219,7 @@ cert_tests = unit/test-pem \
 cert_files = unit/cert-chain.pem \
 			unit/cert-entity-int.pem \
 			unit/cert-server.pem \
+			unit/ec-cert-server.pem \
 			unit/cert-server-key-pkcs8.pem \
 			unit/cert-client.pem \
 			unit/cert-client.crt \
@@ -246,6 +247,7 @@ cert_files = unit/cert-chain.pem \
 cert_checks = unit/cert-intca \
 			unit/cert-entity-int \
 			unit/cert-server \
+			unit/ec-cert-server \
 			unit/cert-client \
 			unit/cert-no-keyid
 
@@ -417,15 +419,30 @@ false_redirect_openssl = 2>/dev/null
 unit/cert-ca-key.pem:
 	$(AM_V_GEN)openssl genrsa -out $@ 2048 $($(AM_V_P)_redirect_openssl)
 
+unit/ec-cert-ca-key.pem:
+	$(AM_V_GEN)openssl ecparam -out $@ -name secp384r1 \
+				-genkey $($(AM_V_P)_redirect_openssl)
+
+
 unit/cert-ca.pem: unit/cert-ca-key.pem unit/gencerts.cnf
 	$(AM_V_GEN)openssl req -x509 -new -nodes -extensions ca_ext \
 			-config $(srcdir)/unit/gencerts.cnf \
 			-subj '/O=International Union of Example Organizations/CN=Certificate issuer guy/emailAddress=ca@mail.example' \
 			-key $< -sha256 -days 10000 -out $@
 
+unit/ec-cert-ca.pem: unit/ec-cert-ca-key.pem unit/gencerts.cnf
+	$(AM_V_GEN)openssl req -x509 -new -nodes -extensions ca_ext \
+			-config $(srcdir)/unit/gencerts.cnf \
+			-subj '/O=International Union of Example Organizations/CN=Certificate issuer guy/emailAddress=ca@mail.example' \
+			-key $< -sha256 -days 10000 -out $@
+
 unit/cert-server-key.pem:
 	$(AM_V_GEN)openssl genrsa -out $@ $($(AM_V_P)_redirect_openssl)
 
+unit/ec-cert-server-key.pem:
+	$(AM_V_GEN)openssl ecparam -out $@ -name secp384r1 \
+				-genkey $($(AM_V_P)_redirect_openssl)
+
 unit/cert-server-key-pkcs8.pem: unit/cert-server-key.pem
 	$(AM_V_GEN)openssl pkcs8 -topk8 -nocrypt -in $< -out $@
 
@@ -435,6 +452,12 @@ unit/cert-server.csr: unit/cert-server-key.pem unit/gencerts.cnf
 			-subj '/O=Foo Example Organization/CN=Foo Example Organization/emailAddress=foo@mail.example' \
 			-key $< -out $@
 
+unit/ec-cert-server.csr: unit/ec-cert-server-key.pem unit/gencerts.cnf
+	$(AM_V_GEN)openssl req -new -extensions cert_ext \
+			-config $(srcdir)/unit/gencerts.cnf \
+			-subj '/O=Foo Example Organization/CN=Foo Example Organization/emailAddress=foo@mail.example' \
+			-key $< -out $@
+
 unit/cert-server.pem: unit/cert-server.csr unit/cert-ca.pem unit/gencerts.cnf
 	$(AM_V_GEN)openssl x509 -req -extensions server_ext \
 			-extfile $(srcdir)/unit/gencerts.cnf \
@@ -443,9 +466,22 @@ unit/cert-server.pem: unit/cert-server.csr unit/cert-ca.pem unit/gencerts.cnf
 			-CAserial $(builddir)/unit/cert-ca.srl \
 			-CAcreateserial -sha256 -days 10000 -out $@ $($(AM_V_P)_redirect_openssl)
 
+unit/ec-cert-server.pem: unit/ec-cert-server.csr unit/ec-cert-ca.pem \
+				unit/gencerts.cnf
+	$(AM_V_GEN)openssl x509 -req -extensions server_ext \
+			-extfile $(srcdir)/unit/gencerts.cnf \
+			-in $< -CA $(builddir)/unit/ec-cert-ca.pem \
+			-CAkey $(builddir)/unit/ec-cert-ca-key.pem \
+			-CAserial $(builddir)/unit/cert-ca.srl \
+			-CAcreateserial -sha256 -days 10000 \
+			-out $@ $($(AM_V_P)_redirect_openssl)
+
 unit/cert-server: unit/cert-server.pem unit/cert-ca.pem
 	$(AM_V_GEN)openssl verify -CAfile $(builddir)/unit/cert-ca.pem $<
 
+unit/ec-cert-server: unit/ce-cert-server.pem unit/ce-cert-ca.pem
+	$(AM_V_GEN)openssl verify -CAfile $(builddir)/unit/ce-cert-ca.pem $<
+
 unit/cert-client-key-pkcs1.pem:
 	$(AM_V_GEN)openssl genrsa -out $@ $($(AM_V_P)_redirect_openssl)
 
@@ -623,7 +659,8 @@ check-local: $(cert_checks)
 endif
 
 clean-local:
-	-rm -f unit/cert-*.pem unit/cert-*.csr unit/cert-*.srl unit/key-*.dat
+	-rm -f unit/ec-cert*.pem unit/ec-cert-*.csr \
+		unit/cert-*.pem unit/cert-*.csr unit/cert-*.srl unit/key-*.dat
 
 maintainer-clean-local:
 	-rm -rf build-aux
diff --git a/unit/test-tls.c b/unit/test-tls.c
index 7937962cf8a0..aee5b2e36b78 100644
--- a/unit/test-tls.c
+++ b/unit/test-tls.c
@@ -325,6 +325,24 @@ static void test_certificates(const void *data)
 	l_queue_destroy(twocas, (l_queue_destroy_func_t) l_cert_free);
 }
 
+static void test_ec_certificates(const void *data)
+{
+	struct l_queue *cacert;
+	struct l_certchain *chain;
+
+	cacert = l_pem_load_certificate_list(CERTDIR "ec-cert-ca.pem");
+	assert(cacert && !l_queue_isempty(cacert));
+
+	chain = l_pem_load_certificate_chain(CERTDIR "ec-cert-server.pem");
+	assert(chain);
+
+	assert(l_certchain_verify(chain, cacert, NULL));
+	assert(l_certchain_verify(chain, NULL, NULL));
+
+	l_certchain_free(chain);
+	l_queue_destroy(cacert, (l_queue_destroy_func_t) l_cert_free);
+}
+
 struct tls_conn_test {
 	const char *server_cert_path;
 	const char *server_key_path;
@@ -948,8 +966,10 @@ int main(int argc, char *argv[])
 	l_test_add("TLS 1.2 PRF with SHA512", test_tls12_prf,
 			&tls12_prf_sha512_0);
 
-	if (l_key_is_supported(L_KEY_FEATURE_RESTRICT))
+	if (l_key_is_supported(L_KEY_FEATURE_RESTRICT)) {
 		l_test_add("Certificate chains", test_certificates, NULL);
+		l_test_add("ECDSA Certificates", test_ec_certificates, NULL);
+	}
 
 	if (!l_getrandom_is_supported()) {
 		printf("getrandom missing, skipping TLS connection tests...\n");
-- 
2.35.1

[PATCH 3/9] key: ECDSA data is given in x962 format

[Denis Kenzior]

When using the verify operation with ECDSA based public keys, the new
format type must be used.
---
 ell/key.c | 12 +++++-------
 ell/key.h |  1 +
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/ell/key.c b/ell/key.c
index 73f38581f736..24374a5d836d 100644
--- a/ell/key.c
+++ b/ell/key.c
@@ -383,18 +383,16 @@ LIB_EXPORT ssize_t l_key_get_payload_size(struct l_key *key)
 
 static const char *lookup_cipher(enum l_key_cipher_type cipher)
 {
-	const char* ret = NULL;
-
 	switch (cipher) {
 	case L_KEY_RSA_PKCS1_V1_5:
-		ret = "pkcs1";
-		break;
+		return "pkcs1";
 	case L_KEY_RSA_RAW:
-		ret = "raw";
-		break;
+		return "raw";
+	case L_KEY_ECDSA_X962:
+		return "x962";
 	}
 
-	return ret;
+	return NULL;
 }
 
 static const char *lookup_checksum(enum l_checksum_type checksum)
diff --git a/ell/key.h b/ell/key.h
index f26f7ecb26c3..68971052ffde 100644
--- a/ell/key.h
+++ b/ell/key.h
@@ -56,6 +56,7 @@ enum l_keyring_restriction {
 enum l_key_cipher_type {
 	L_KEY_RSA_PKCS1_V1_5,
 	L_KEY_RSA_RAW,
+	L_KEY_ECDSA_X962,
 };
 
 struct l_key *l_key_new(enum l_key_type type, const void *payload,
-- 
2.35.1

[PATCH 4/9] tls: Support peer certificates that use ECDSA

[Denis Kenzior]

---
 ell/tls.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/ell/tls.c b/ell/tls.c
index b2f7411f3b36..75b9d45c6523 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -1899,6 +1899,8 @@ static void tls_handle_certificate(struct l_tls *tls,
 	bool dummy;
 	const char *error_str;
 	char *subject_str;
+	enum l_key_cipher_type format_type;
+	enum l_checksum_type checksum_type;
 
 	if (len < 3)
 		goto decode_error;
@@ -2028,9 +2030,23 @@ static void tls_handle_certificate(struct l_tls *tls,
 		return;
 	}
 
-	if (!l_key_get_info(tls->peer_pubkey, L_KEY_RSA_PKCS1_V1_5,
-					L_CHECKSUM_NONE, &tls->peer_pubkey_size,
-					&dummy)) {
+	switch (l_cert_get_pubkey_type(tls->peer_cert)) {
+	case L_CERT_KEY_RSA:
+		format_type = L_KEY_RSA_PKCS1_V1_5;
+		checksum_type = L_CHECKSUM_NONE;
+		break;
+	case L_CERT_KEY_ECC:
+		format_type = L_KEY_ECDSA_X962;
+		checksum_type = L_CHECKSUM_SHA1;
+		break;
+	case L_CERT_KEY_UNKNOWN:
+		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+				"Unknown public key type");
+		return;
+	}
+
+	if (!l_key_get_info(tls->peer_pubkey, format_type, checksum_type,
+				&tls->peer_pubkey_size, &dummy)) {
 		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
 				"Can't l_key_get_info for peer public key");
 
-- 
2.35.1

[PATCH 5/9] tls: Add helper for DigitallySigned validation

[Denis Kenzior]

To support additional signature algorithms, move the logic
that validates DigitallySigned structure to a helper function.
---
 ell/tls-suites.c | 87 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 60 insertions(+), 27 deletions(-)

diff --git a/ell/tls-suites.c b/ell/tls-suites.c
index 1c1ca078b3d8..d5d2ec8f741f 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -40,6 +40,57 @@
 #include "ecdh.h"
 #include "missing.h"
 
+enum signature_algorithm {
+	SIGNATURE_ALGORITHM_ANONYMOUS = 0,
+	SIGNATURE_ALGORITHM_RSA = 1,
+	SIGNATURE_ALGORITHM_DSA = 2,
+	SIGNATURE_ALGORITHM_ECDSA = 3,
+};
+
+/*
+ * Sanitize DigitallySigned struct input, making sure the lengths
+ * are valid and correspond to what we expect.
+ *
+ * Returns: start of the opaque portion
+ */
+static const uint8_t *validate_digitally_signed(struct l_tls *tls,
+					const uint8_t *in, size_t in_len,
+					enum signature_algorithm expected_alg,
+					uint16_t *opaque_len)
+{
+	size_t offset = 2;
+	uint16_t len;
+
+	if (tls->negotiated_version < L_TLS_V12)
+		offset = 0;
+
+	if (in_len < offset + 2)
+		goto size_error;
+
+	len = l_get_be16(in + offset);
+	if (len != in_len - offset - 2)
+		goto size_error;
+
+	if (tls->negotiated_version >= L_TLS_V12) {
+		if (in[1] != expected_alg) {
+			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
+					"Unknown signature algorithm %i",
+					in[1]);
+
+			return NULL;
+		}
+	}
+
+	*opaque_len = len;
+	return in + offset + 2;
+
+size_error:
+	TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0, "Signature msg too "
+			"short (%zi) or signature length doesn't match",
+			in_len);
+	return NULL;
+}
+
 static bool tls_rsa_validate_cert_key(struct l_cert *cert)
 {
 	return l_cert_get_pubkey_type(cert) == L_CERT_KEY_RSA;
@@ -112,29 +163,20 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t in_len,
 	enum l_checksum_type sign_checksum_type;
 	uint8_t expected[HANDSHAKE_HASH_MAX_SIZE + 36];
 	size_t expected_len;
-	unsigned int offset;
+	const uint8_t *opaque;
+	uint16_t opaque_len;
 	bool success;
 
-	/* 2 bytes for SignatureAndHashAlgorithm if version >= 1.2 */
-	offset = 2;
-	if (tls->negotiated_version < L_TLS_V12)
-		offset = 0;
-
-	if (in_len < offset + 2 ||
-			(size_t) l_get_be16(in + offset) + offset + 2 !=
-			in_len) {
-		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0, "Signature msg too "
-				"short (%zi) or signature length doesn't match",
-				in_len);
-
+	opaque = validate_digitally_signed(tls, in, in_len,
+				SIGNATURE_ALGORITHM_RSA, &opaque_len);
+	if (!opaque)
 		return false;
-	}
 
 	/* Only the default hash type supported */
-	if (in_len != offset + 2 + tls->peer_pubkey_size) {
+	if (opaque_len != tls->peer_pubkey_size) {
 		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
-				"Signature length %zi not equal %zi", in_len,
-				offset + 2 + tls->peer_pubkey_size);
+				"Signature length %hu not equal %zi",
+				opaque_len, tls->peer_pubkey_size);
 
 		return false;
 	}
@@ -142,15 +184,6 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t in_len,
 	if (tls->negotiated_version >= L_TLS_V12) {
 		enum handshake_hash_type hash;
 
-		/* Only RSA supported */
-		if (in[1] != 1 /* RSA_sign */) {
-			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
-					"Unknown signature algorithm %i",
-					in[1]);
-
-			return false;
-		}
-
 		for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
 			if (tls_handshake_hash_data[hash].tls_id == in[0])
 				break;
@@ -203,7 +236,7 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t in_len,
 	}
 
 	success = l_key_verify(tls->peer_pubkey, L_KEY_RSA_PKCS1_V1_5,
-				sign_checksum_type, expected, in + offset + 2,
+				sign_checksum_type, expected, opaque,
 				expected_len, tls->peer_pubkey_size);
 
 	if (!success)
-- 
2.35.1

[PATCH 6/9] tls: Add helper to find hash function by id

[Denis Kenzior]

Instead of open-coding a loop to map the hash id from
SignatureAndHashAlgorithm structure to a supported hash function.
---
 ell/tls-suites.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/ell/tls-suites.c b/ell/tls-suites.c
index d5d2ec8f741f..bc6a756422b3 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -47,6 +47,17 @@ enum signature_algorithm {
 	SIGNATURE_ALGORITHM_ECDSA = 3,
 };
 
+static enum handshake_hash_type find_hash_by_id(uint8_t id)
+{
+	enum handshake_hash_type hash;
+
+	for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
+		if (tls_handshake_hash_data[hash].tls_id == id)
+			break;
+
+	return hash;
+}
+
 /*
  * Sanitize DigitallySigned struct input, making sure the lengths
  * are valid and correspond to what we expect.
@@ -182,11 +193,7 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t in_len,
 	}
 
 	if (tls->negotiated_version >= L_TLS_V12) {
-		enum handshake_hash_type hash;
-
-		for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
-			if (tls_handshake_hash_data[hash].tls_id == in[0])
-				break;
+		enum handshake_hash_type hash = find_hash_by_id(in[0]);
 
 		if (hash == __HANDSHAKE_HASH_COUNT) {
 			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
-- 
2.35.1

[PATCH 7/9] tls-suites: Add ECDSA suites from RFC 8422

[Denis Kenzior]

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

diff --git a/ell/tls-suites.c b/ell/tls-suites.c
index bc6a756422b3..34141ab7fa56 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -262,6 +262,81 @@ static struct tls_signature_algorithm tls_rsa_signature = {
 	.verify = tls_rsa_verify,
 };
 
+static bool tls_ecdsa_validate_cert_key(struct l_cert *cert)
+{
+	return l_cert_get_pubkey_type(cert) == L_CERT_KEY_ECC;
+}
+
+static bool tls_ecdsa_verify(struct l_tls *tls,
+				const uint8_t *in, size_t in_len,
+				tls_get_hash_t get_hash,
+				const uint8_t *data, size_t data_len)
+{
+	/* RFC 8422, Section 5.10: "SHA-1 is used in TLS 1.1 and earlier" */
+	enum handshake_hash_type hash = HANDSHAKE_HASH_SHA1;
+	enum l_checksum_type sign_checksum_type;
+	const uint8_t *opaque;
+	uint16_t opaque_len;
+	uint8_t expected[HANDSHAKE_HASH_MAX_SIZE];
+	size_t expected_len;
+	bool success;
+
+	opaque = validate_digitally_signed(tls, in, in_len,
+				SIGNATURE_ALGORITHM_ECDSA, &opaque_len);
+	if (!opaque)
+		return false;
+
+	if (tls->negotiated_version >= L_TLS_V12) {
+		hash = find_hash_by_id(in[0]);
+		if (hash == __HANDSHAKE_HASH_COUNT) {
+			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
+					"Unknown hash type %i", in[0]);
+			return false;
+		}
+
+		/* Hash should match the curve, refer to RFC 5480, Section 4 */
+		switch (tls->peer_pubkey_size) {
+		case 32:
+			if (hash != HANDSHAKE_HASH_SHA256 &&
+					hash != HANDSHAKE_HASH_SHA384)
+				goto bad_hash;
+
+			break;
+		case 48:
+			if (hash != HANDSHAKE_HASH_SHA384)
+				goto bad_hash;
+
+			break;
+		bad_hash:
+		default:
+			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
+					"Invalid hash %i",
+					in[0]);
+		}
+	}
+
+	get_hash(tls, hash, data, data_len, expected, &expected_len);
+	sign_checksum_type = tls_handshake_hash_data[hash].l_id;
+
+	success = l_key_verify(tls->peer_pubkey, L_KEY_ECDSA_X962,
+				sign_checksum_type, expected, opaque,
+				expected_len, opaque_len);
+
+	if (!success)
+		TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
+				"Peer signature verification failed");
+	else
+		TLS_DEBUG("Peer signature verified");
+
+	return success;
+}
+
+static struct tls_signature_algorithm tls_ecdsa_signature = {
+	.id = 3, /* SignatureAlgorithm.ecdsa */
+	.validate_cert_key_type = tls_ecdsa_validate_cert_key,
+	.verify = tls_ecdsa_verify,
+};
+
 static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
 {
 	uint8_t buf[1024 + 32];
@@ -1350,11 +1425,52 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 	.prf_hmac = L_CHECKSUM_SHA384,
 	.signature = &tls_rsa_signature,
 	.key_xchg = &tls_ecdhe,
+}, tls_ecdhe_ecdsa_with_3des_ede_cbc_sha = {
+	.id = { 0xc0, 0x08 },
+	.name = "TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_3des_ede,
+	.mac = &tls_sha,
+	.signature = &tls_ecdsa_signature,
+	.key_xchg = &tls_ecdhe,
+}, tls_ecdhe_ecdsa_with_aes_128_cbc_sha = {
+	.id = { 0xc0, 0x09 },
+	.name = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128,
+	.mac = &tls_sha,
+	.signature = &tls_ecdsa_signature,
+	.key_xchg = &tls_ecdhe,
+}, tls_ecdhe_ecdsa_with_aes_256_cbc_sha = {
+	.id = { 0xc0, 0x0a },
+	.name = "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256,
+	.mac = &tls_sha,
+	.signature = &tls_ecdsa_signature,
+	.key_xchg = &tls_ecdhe,
+}, tls_ecdhe_ecdsa_with_aes_128_gcm_sha256 = {
+	.id = { 0xc0, 0x2b },
+	.name = "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+	.verify_data_length = 12,
+	.encryption = &tls_aes128_gcm,
+	.signature = &tls_ecdsa_signature,
+	.key_xchg = &tls_ecdhe,
+}, tls_ecdhe_ecdsa_with_aes_256_gcm_sha384 = {
+	.id = { 0xc0, 0x2c },
+	.name = "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+	.verify_data_length = 12,
+	.encryption = &tls_aes256_gcm,
+	.prf_hmac = L_CHECKSUM_SHA384,
+	.signature = &tls_ecdsa_signature,
+	.key_xchg = &tls_ecdhe,
 };
 
 struct tls_cipher_suite *tls_cipher_suite_pref[] = {
 	&tls_ecdhe_rsa_with_aes_256_cbc_sha,
+	&tls_ecdhe_ecdsa_with_aes_256_cbc_sha,
 	&tls_ecdhe_rsa_with_aes_128_cbc_sha,
+	&tls_ecdhe_ecdsa_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,
@@ -1367,11 +1483,14 @@ struct tls_cipher_suite *tls_cipher_suite_pref[] = {
 	&tls_rsa_with_aes_128_cbc_sha256,
 	&tls_ecdhe_rsa_with_aes_256_gcm_sha384,
 	&tls_ecdhe_rsa_with_aes_128_gcm_sha256,
+	&tls_ecdhe_ecdsa_with_aes_256_gcm_sha384,
+	&tls_ecdhe_ecdsa_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_ecdhe_ecdsa_with_3des_ede_cbc_sha,
 	&tls_dhe_rsa_with_3des_ede_cbc_sha,
 	&tls_rsa_with_3des_ede_cbc_sha,
 	NULL,
-- 
2.35.1

[PATCH 8/9] useful: Add maxsize()

[Denis Kenzior]

Similar to minsize(), but for finding the maximum of two sizes
---
 ell/useful.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/ell/useful.h b/ell/useful.h
index 791fa2006494..efd91d78b3f2 100644
--- a/ell/useful.h
+++ b/ell/useful.h
@@ -38,6 +38,14 @@ static inline size_t minsize(size_t a, size_t b)
 	return b;
 }
 
+static inline size_t maxsize(size_t a, size_t b)
+{
+	if (a >= b)
+		return a;
+
+	return b;
+}
+
 static inline void set_bit(void *addr, unsigned int bit)
 {
 	unsigned char *field = addr;
-- 
2.35.1

[PATCH 9/9] tls: Do not set verify_data_length unless needed

[Denis Kenzior]

All current TLS cipher suites use a verify_data_length of 12.  In fact,
according to RFC 5246, most cipher suites are expected to be 12 bytes
unless specified otherwise.  Use this fact to simplify the cipher suite
definition: initialization of verify_data_length is no longer necessary
unless the length is greater than 12 bytes.

While here, also update struct tls_cipher_suite to use a size_t member
for verify_data_length instead of an int.
---
 ell/tls-private.h |  2 +-
 ell/tls-suites.c  | 26 --------------------------
 ell/tls.c         | 29 +++++++++++++++++++++--------
 3 files changed, 22 insertions(+), 35 deletions(-)

diff --git a/ell/tls-private.h b/ell/tls-private.h
index 8ceeb68df40b..8941e90d03ca 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -112,7 +112,7 @@ struct tls_mac_algorithm {
 struct tls_cipher_suite {
 	uint8_t id[2];
 	const char *name;
-	int verify_data_length;
+	size_t verify_data_length;
 
 	struct tls_bulk_encryption_algorithm *encryption;
 	struct tls_signature_algorithm *signature;
diff --git a/ell/tls-suites.c b/ell/tls-suites.c
index 34141ab7fa56..ee4e7ee6c310 100644
--- a/ell/tls-suites.c
+++ b/ell/tls-suites.c
@@ -1262,7 +1262,6 @@ static struct tls_mac_algorithm tls_sha = {
 static struct tls_cipher_suite 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,
 	.signature = &tls_rsa_signature,
@@ -1270,7 +1269,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1278,7 +1276,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1286,7 +1283,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1294,7 +1290,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1302,7 +1297,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1310,7 +1304,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1318,7 +1311,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1326,7 +1318,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1334,7 +1325,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1342,14 +1332,12 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
 	.key_xchg = &tls_rsa_key_xchg,
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1357,14 +1345,12 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
 	.key_xchg = &tls_dhe,
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1372,7 +1358,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1380,7 +1365,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1388,7 +1372,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1396,7 +1379,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1404,7 +1386,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
@@ -1413,14 +1394,12 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, 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,
 	.signature = &tls_rsa_signature,
 	.key_xchg = &tls_ecdhe,
 }, 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,
 	.signature = &tls_rsa_signature,
@@ -1428,7 +1407,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, tls_ecdhe_ecdsa_with_3des_ede_cbc_sha = {
 	.id = { 0xc0, 0x08 },
 	.name = "TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA",
-	.verify_data_length = 12,
 	.encryption = &tls_3des_ede,
 	.mac = &tls_sha,
 	.signature = &tls_ecdsa_signature,
@@ -1436,7 +1414,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, tls_ecdhe_ecdsa_with_aes_128_cbc_sha = {
 	.id = { 0xc0, 0x09 },
 	.name = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
-	.verify_data_length = 12,
 	.encryption = &tls_aes128,
 	.mac = &tls_sha,
 	.signature = &tls_ecdsa_signature,
@@ -1444,7 +1421,6 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, tls_ecdhe_ecdsa_with_aes_256_cbc_sha = {
 	.id = { 0xc0, 0x0a },
 	.name = "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
-	.verify_data_length = 12,
 	.encryption = &tls_aes256,
 	.mac = &tls_sha,
 	.signature = &tls_ecdsa_signature,
@@ -1452,14 +1428,12 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
 }, tls_ecdhe_ecdsa_with_aes_128_gcm_sha256 = {
 	.id = { 0xc0, 0x2b },
 	.name = "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
-	.verify_data_length = 12,
 	.encryption = &tls_aes128_gcm,
 	.signature = &tls_ecdsa_signature,
 	.key_xchg = &tls_ecdhe,
 }, tls_ecdhe_ecdsa_with_aes_256_gcm_sha384 = {
 	.id = { 0xc0, 0x2c },
 	.name = "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
-	.verify_data_length = 12,
 	.encryption = &tls_aes256_gcm,
 	.prf_hmac = L_CHECKSUM_SHA384,
 	.signature = &tls_ecdsa_signature,
diff --git a/ell/tls.c b/ell/tls.c
index 75b9d45c6523..0bf647919478 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -1370,11 +1370,25 @@ static void tls_send_change_cipher_spec(struct l_tls *tls)
 	tls_tx_record(tls, TLS_CT_CHANGE_CIPHER_SPEC, &buf, 1);
 }
 
+static size_t tls_verify_data_length(struct l_tls *tls, unsigned int index)
+{
+	/*
+	 * RFC 5246, Section 7.4.9:
+	 *
+	 * In previous versions of TLS, the verify_data was always 12 octets
+	 * long.  In the current version of TLS, it depends on the cipher
+	 * suite.  Any cipher suite which does not explicitly specify
+	 * verify_data_length has a verify_data_length equal to 12.
+	 */
+	return maxsize(tls->cipher_suite[index]->verify_data_length, 12);
+}
+
 static void tls_send_finished(struct l_tls *tls)
 {
 	uint8_t buf[512];
 	uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
 	uint8_t seed[HANDSHAKE_HASH_MAX_SIZE * 2];
+	size_t vdl = tls_verify_data_length(tls, 1);
 	size_t seed_len;
 
 	if (tls->negotiated_version >= L_TLS_V12) {
@@ -1391,8 +1405,8 @@ static void tls_send_finished(struct l_tls *tls)
 				tls->server ? "server finished" :
 				"client finished",
 				seed, seed_len,
-				ptr, tls->cipher_suite[1]->verify_data_length);
-	ptr += tls->cipher_suite[1]->verify_data_length;
+				ptr, vdl);
+	ptr += vdl;
 
 	tls_tx_handshake(tls, TLS_FINISHED, buf, ptr - buf);
 }
@@ -1400,14 +1414,14 @@ static void tls_send_finished(struct l_tls *tls)
 static bool tls_verify_finished(struct l_tls *tls, const uint8_t *received,
 				size_t len)
 {
-	uint8_t expected[tls->cipher_suite[0]->verify_data_length];
+	size_t vdl = tls_verify_data_length(tls, 0);
+	uint8_t expected[vdl];
 	uint8_t *seed;
 	size_t seed_len;
 
-	if (len != (size_t) tls->cipher_suite[0]->verify_data_length) {
+	if (len != vdl) {
 		TLS_DISCONNECT(TLS_ALERT_DECODE_ERROR, 0,
-				"TLS_FINISHED length not %i",
-				tls->cipher_suite[0]->verify_data_length);
+				"TLS_FINISHED length not %zu", vdl);
 
 		return false;
 	}
@@ -1428,8 +1442,7 @@ static bool tls_verify_finished(struct l_tls *tls, const uint8_t *received,
 				tls->server ? "client finished" :
 				"server finished",
 				seed, seed_len,
-				expected,
-				tls->cipher_suite[0]->verify_data_length);
+				expected, vdl);
 
 	if (memcmp(received, expected, len)) {
 		TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
-- 
2.35.1

Re: [PATCH 4/9] tls: Support peer certificates that use ECDSA

[Mat Martineau]

On Mon, 18 Jul 2022, Denis Kenzior wrote:

> ---
> ell/tls.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/ell/tls.c b/ell/tls.c
> index b2f7411f3b36..75b9d45c6523 100644
> --- a/ell/tls.c
> +++ b/ell/tls.c
> @@ -1899,6 +1899,8 @@ static void tls_handle_certificate(struct l_tls *tls,
> 	bool dummy;
> 	const char *error_str;
> 	char *subject_str;
> +	enum l_key_cipher_type format_type;
> +	enum l_checksum_type checksum_type;
>
> 	if (len < 3)
> 		goto decode_error;
> @@ -2028,9 +2030,23 @@ static void tls_handle_certificate(struct l_tls *tls,
> 		return;
> 	}
>
> -	if (!l_key_get_info(tls->peer_pubkey, L_KEY_RSA_PKCS1_V1_5,
> -					L_CHECKSUM_NONE, &tls->peer_pubkey_size,
> -					&dummy)) {
> +	switch (l_cert_get_pubkey_type(tls->peer_cert)) {
> +	case L_CERT_KEY_RSA:
> +		format_type = L_KEY_RSA_PKCS1_V1_5;
> +		checksum_type = L_CHECKSUM_NONE;
> +		break;
> +	case L_CERT_KEY_ECC:
> +		format_type = L_KEY_ECDSA_X962;
> +		checksum_type = L_CHECKSUM_SHA1;
> +		break;
> +	case L_CERT_KEY_UNKNOWN:

Hi Denis,

I needed to add

 	default:

here to get it to build. Details below.

> +		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> +				"Unknown public key type");
> +		return;
> +	}
> +
> +	if (!l_key_get_info(tls->peer_pubkey, format_type, checksum_type,
> +				&tls->peer_pubkey_size, &dummy)) {

The ell (standalone, bootstrap-configure) build fails here with 
gcc12/Fedora36:

ell/tls.c:2061:14: error: 'checksum_type' may be used uninitialized [-Werror=maybe-uninitialized]
ell/tls.c:2061:14: error: 'format_type' may be used uninitialized [-Werror=maybe-uninitialized]

Apparently gcc12 can't track that -Werror=switch-enum is in use in 
combination with -Werror=maybe-uninitialized, and doesn't understand that 
the switch statement above does guarantee initialization.

> 		TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
> 				"Can't l_key_get_info for peer public key");
>
> -- 
> 2.35.1
>
>
>

--
Mat Martineau
Intel

Re: [PATCH 7/9] tls-suites: Add ECDSA suites from RFC 8422

[Mat Martineau]

On Mon, 18 Jul 2022, Denis Kenzior wrote:

> ---
> ell/tls-suites.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 119 insertions(+)
>
> diff --git a/ell/tls-suites.c b/ell/tls-suites.c
> index bc6a756422b3..34141ab7fa56 100644
> --- a/ell/tls-suites.c
> +++ b/ell/tls-suites.c
> @@ -262,6 +262,81 @@ static struct tls_signature_algorithm tls_rsa_signature = {
> 	.verify = tls_rsa_verify,
> };
>
> +static bool tls_ecdsa_validate_cert_key(struct l_cert *cert)
> +{
> +	return l_cert_get_pubkey_type(cert) == L_CERT_KEY_ECC;
> +}
> +
> +static bool tls_ecdsa_verify(struct l_tls *tls,
> +				const uint8_t *in, size_t in_len,
> +				tls_get_hash_t get_hash,
> +				const uint8_t *data, size_t data_len)
> +{
> +	/* RFC 8422, Section 5.10: "SHA-1 is used in TLS 1.1 and earlier" */
> +	enum handshake_hash_type hash = HANDSHAKE_HASH_SHA1;
> +	enum l_checksum_type sign_checksum_type;
> +	const uint8_t *opaque;
> +	uint16_t opaque_len;
> +	uint8_t expected[HANDSHAKE_HASH_MAX_SIZE];
> +	size_t expected_len;
> +	bool success;
> +
> +	opaque = validate_digitally_signed(tls, in, in_len,
> +				SIGNATURE_ALGORITHM_ECDSA, &opaque_len);
> +	if (!opaque)
> +		return false;
> +
> +	if (tls->negotiated_version >= L_TLS_V12) {
> +		hash = find_hash_by_id(in[0]);
> +		if (hash == __HANDSHAKE_HASH_COUNT) {
> +			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
> +					"Unknown hash type %i", in[0]);
> +			return false;
> +		}
> +
> +		/* Hash should match the curve, refer to RFC 5480, Section 4 */
> +		switch (tls->peer_pubkey_size) {
> +		case 32:
> +			if (hash != HANDSHAKE_HASH_SHA256 &&
> +					hash != HANDSHAKE_HASH_SHA384)
> +				goto bad_hash;
> +
> +			break;
> +		case 48:
> +			if (hash != HANDSHAKE_HASH_SHA384)
> +				goto bad_hash;
> +
> +			break;
> +		bad_hash:
> +		default:
> +			TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
> +					"Invalid hash %i",
> +					in[0]);
> +		}
> +	}
> +
> +	get_hash(tls, hash, data, data_len, expected, &expected_len);
> +	sign_checksum_type = tls_handshake_hash_data[hash].l_id;
> +
> +	success = l_key_verify(tls->peer_pubkey, L_KEY_ECDSA_X962,
> +				sign_checksum_type, expected, opaque,
> +				expected_len, opaque_len);
> +
> +	if (!success)
> +		TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
> +				"Peer signature verification failed");
> +	else
> +		TLS_DEBUG("Peer signature verified");
> +
> +	return success;
> +}
> +
> +static struct tls_signature_algorithm tls_ecdsa_signature = {
> +	.id = 3, /* SignatureAlgorithm.ecdsa */
> +	.validate_cert_key_type = tls_ecdsa_validate_cert_key,
> +	.verify = tls_ecdsa_verify,
> +};
> +
> static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
> {
> 	uint8_t buf[1024 + 32];
> @@ -1350,11 +1425,52 @@ static struct tls_cipher_suite tls_rsa_with_3des_ede_cbc_sha = {
> 	.prf_hmac = L_CHECKSUM_SHA384,
> 	.signature = &tls_rsa_signature,
> 	.key_xchg = &tls_ecdhe,
> +}, tls_ecdhe_ecdsa_with_3des_ede_cbc_sha = {
> +	.id = { 0xc0, 0x08 },
> +	.name = "TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA",
> +	.verify_data_length = 12,
> +	.encryption = &tls_3des_ede,
> +	.mac = &tls_sha,
> +	.signature = &tls_ecdsa_signature,
> +	.key_xchg = &tls_ecdhe,
> +}, tls_ecdhe_ecdsa_with_aes_128_cbc_sha = {
> +	.id = { 0xc0, 0x09 },
> +	.name = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
> +	.verify_data_length = 12,
> +	.encryption = &tls_aes128,
> +	.mac = &tls_sha,
> +	.signature = &tls_ecdsa_signature,
> +	.key_xchg = &tls_ecdhe,
> +}, tls_ecdhe_ecdsa_with_aes_256_cbc_sha = {
> +	.id = { 0xc0, 0x0a },
> +	.name = "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
> +	.verify_data_length = 12,
> +	.encryption = &tls_aes256,
> +	.mac = &tls_sha,
> +	.signature = &tls_ecdsa_signature,
> +	.key_xchg = &tls_ecdhe,
> +}, tls_ecdhe_ecdsa_with_aes_128_gcm_sha256 = {
> +	.id = { 0xc0, 0x2b },
> +	.name = "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
> +	.verify_data_length = 12,
> +	.encryption = &tls_aes128_gcm,
> +	.signature = &tls_ecdsa_signature,
> +	.key_xchg = &tls_ecdhe,
> +}, tls_ecdhe_ecdsa_with_aes_256_gcm_sha384 = {
> +	.id = { 0xc0, 0x2c },
> +	.name = "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
> +	.verify_data_length = 12,
> +	.encryption = &tls_aes256_gcm,
> +	.prf_hmac = L_CHECKSUM_SHA384,
> +	.signature = &tls_ecdsa_signature,
> +	.key_xchg = &tls_ecdhe,
> };

These new suites fail in unit/test-tls with the 5.18.11-200.fc36.x86_64 
kernel (latest Fedora 36):

TEST: TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
test-tls: unit/test-tls.c:661: test_tls_with_ver: Assertion `!!l_tls_start(s[1].tls) == !test->expect_client_start_fail' failed.

I started commenting out each failed test, also verified the failure with 
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA and 
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384. Didn't try the last couple cases 
(GCM_SHA256 and 3DES).

>
> struct tls_cipher_suite *tls_cipher_suite_pref[] = {
> 	&tls_ecdhe_rsa_with_aes_256_cbc_sha,
> +	&tls_ecdhe_ecdsa_with_aes_256_cbc_sha,
> 	&tls_ecdhe_rsa_with_aes_128_cbc_sha,
> +	&tls_ecdhe_ecdsa_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,
> @@ -1367,11 +1483,14 @@ struct tls_cipher_suite *tls_cipher_suite_pref[] = {
> 	&tls_rsa_with_aes_128_cbc_sha256,
> 	&tls_ecdhe_rsa_with_aes_256_gcm_sha384,
> 	&tls_ecdhe_rsa_with_aes_128_gcm_sha256,
> +	&tls_ecdhe_ecdsa_with_aes_256_gcm_sha384,
> +	&tls_ecdhe_ecdsa_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_ecdhe_ecdsa_with_3des_ede_cbc_sha,
> 	&tls_dhe_rsa_with_3des_ede_cbc_sha,
> 	&tls_rsa_with_3des_ede_cbc_sha,
> 	NULL,
> -- 
> 2.35.1
>
>
>

--
Mat Martineau
Intel

Re: [PATCH 4/9] tls: Support peer certificates that use ECDSA

[Denis Kenzior]

Hi Mat,

>      default:

Indeed.

> here to get it to build. Details below.
> 
>> +        TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
>> +                "Unknown public key type");
>> +        return;
>> +    }
>> +
>> +    if (!l_key_get_info(tls->peer_pubkey, format_type, checksum_type,
>> +                &tls->peer_pubkey_size, &dummy)) {
> 
> The ell (standalone, bootstrap-configure) build fails here with gcc12/Fedora36:
> 
> ell/tls.c:2061:14: error: 'checksum_type' may be used uninitialized 
> [-Werror=maybe-uninitialized]
> ell/tls.c:2061:14: error: 'format_type' may be used uninitialized 
> [-Werror=maybe-uninitialized]
> 
> Apparently gcc12 can't track that -Werror=switch-enum is in use in combination 
> with -Werror=maybe-uninitialized, and doesn't understand that the switch 
> statement above does guarantee initialization.
> 

GCC seems a bit silly here.  Not sure adding 'default:' is any better since we 
use these warnings defensively in case a new enumeration is added and not handled.

Anyhow, I fixed this slightly differently in v2 out shortly.  Thanks for testing.

Regards,
-Denis