cipher: Only do the RFC3610 IV formatting for CCM

PATCH Applied Andrew Zaborowski 3 participants 18 messages Nov 14, 2018 – Nov 15, 2018

Share & tools

Andrew Zaborowski

author

2018-11-14 13:18 UTC #0 lore raw

PATCH 1/12 cipher: Only do the RFC3610 IV formatting for CCM

In preparation to add the AES GCM mode, move the CCM-specific IV formatting code from operate_cipher to the callers (only aead encrypt and decrypt), make operate_cipher accept the IV buffer directly. Check that the nonce is >= 7 bytes.

Zero the "counter" part of the IV to avoid sending uninitialized bytes to the kernel (the kernel will zero them again though)

---
 ell/cipher.c | 91 +++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 61 insertions(+), 30 deletions(-)

ell/cipher.c +61 ~~-30~~

index 79cecdb..af4bc10 100644
@@ -266,28 +266,11 @@ LIB_EXPORT void l_aead_cipher_free(struct l_aead_cipher *cipher)
 	l_free(cipher);
 }
 
-static ssize_t build_iv(const void *nonce, uint8_t nonce_len, uint8_t *iv,
-			uint8_t iv_len)
-{
-	const size_t iv_overhead = 2;
-
-	if (nonce_len + iv_overhead > iv_len)
-		return -EINVAL;
-
-	iv[0] = iv_len - iv_overhead - nonce_len;
-	memcpy(iv + 1, nonce, nonce_len);
-
-	/* Assumes that remaining bytes in iv were already zeroed out */
-
-	return iv_len;
-}
-
 static ssize_t operate_cipher(int sk, __u32 operation,
 				const void *in, size_t in_len,
 				const void *ad, size_t ad_len,
-				const void *nonce, size_t nonce_len,
-				void *out, size_t out_len,
-				size_t iv_len)
+				const void *iv, size_t iv_len,
+				void *out, size_t out_len)
 {
 	char *c_msg_buf;
 	size_t c_msg_size;

@@ -298,7 +281,7 @@ static ssize_t operate_cipher(int sk, __u32 operation,
 
 	c_msg_size = CMSG_SPACE(sizeof(operation));
 	c_msg_size += ad_len ? CMSG_SPACE(sizeof(uint32_t)) : 0;
-	c_msg_size += (nonce && iv_len) ?
+	c_msg_size += iv_len ?
 		CMSG_SPACE(sizeof(struct af_alg_iv) + iv_len) : 0;
 
 	c_msg_buf = alloca(c_msg_size);

@@ -338,7 +321,7 @@ static ssize_t operate_cipher(int sk, __u32 operation,
 		msg.msg_iovlen = 1;
 	}
 
-	if (nonce && iv_len) {
+	if (iv_len) {
 		struct af_alg_iv *algiv;
 
 		c_msg = CMSG_NXTHDR(&msg, c_msg);

@@ -348,9 +331,7 @@ static ssize_t operate_cipher(int sk, __u32 operation,
 
 		algiv = (void *)CMSG_DATA(c_msg);
 		algiv->ivlen = iv_len;
-		result = build_iv(nonce, nonce_len, &algiv->iv[0], iv_len);
-		if (result < 0)
-			return result;
+		memcpy(algiv->iv, iv, iv_len);
 	}
 
 	result = sendmsg(sk, &msg, 0);

@@ -444,7 +425,7 @@ LIB_EXPORT bool l_cipher_encrypt(struct l_cipher *cipher,
 		return false;
 
 	return operate_cipher(cipher->encrypt_sk, ALG_OP_ENCRYPT, in, len,
-				NULL, 0, NULL, 0, out, len, 0) >= 0;
+				NULL, 0, NULL, 0, out, len) >= 0;
 }
 
 LIB_EXPORT bool l_cipher_encryptv(struct l_cipher *cipher,

@@ -471,7 +452,7 @@ LIB_EXPORT bool l_cipher_decrypt(struct l_cipher *cipher,
 		return false;
 
 	return operate_cipher(cipher->decrypt_sk, ALG_OP_DECRYPT, in, len,
-				NULL, 0, NULL, 0, out, len, 0) >= 0;
+				NULL, 0, NULL, 0, out, len) >= 0;
 }
 
 LIB_EXPORT bool l_cipher_decryptv(struct l_cipher *cipher,

@@ -534,21 +515,54 @@ static size_t l_aead_cipher_get_ivlen(struct l_aead_cipher *cipher)
 	return 0;
 }
 
+/* RFC3610 Section 2.3 */
+static ssize_t build_ccm_iv(const void *nonce, uint8_t nonce_len, uint8_t *iv)
+{
+	const size_t iv_overhead = 2;
+	const size_t iv_len = 16;
+
+	if (unlikely(nonce_len + iv_overhead > iv_len || nonce_len < 7))
+		return -EINVAL;
+
+	iv[0] = iv_len - iv_overhead - nonce_len;
+	memcpy(iv + 1, nonce, nonce_len);
+	memset(iv + 1 + nonce_len, 0, iv_len - 1 - nonce_len);
+
+	return iv_len;
+}
+
 LIB_EXPORT bool l_aead_cipher_encrypt(struct l_aead_cipher *cipher,
 					const void *in, size_t in_len,
 					const void *ad, size_t ad_len,
 					const void *nonce, size_t nonce_len,
 					void *out, size_t out_len)
 {
+	uint8_t ccm_iv[16];
+	const uint8_t *iv;
+	ssize_t iv_len;
+
 	if (unlikely(!cipher))
 		return false;
 
 	if (unlikely(!in) || unlikely(!out))
 		return false;
 
+	if (cipher->type == L_AEAD_CIPHER_AES_CCM) {
+		iv_len = build_ccm_iv(nonce, nonce_len, ccm_iv);
+		if (unlikely(iv_len < 0))
+			return false;
+
+		iv = ccm_iv;
+	} else {
+		if (unlikely(nonce_len != l_aead_cipher_get_ivlen(cipher)))
+			return false;
+
+		iv = nonce;
+		iv_len = nonce_len;
+	}
+
 	return operate_cipher(cipher->encrypt_sk, ALG_OP_ENCRYPT, in, in_len,
-				ad, ad_len, nonce, nonce_len, out, out_len,
-				l_aead_cipher_get_ivlen(cipher)) ==
+				ad, ad_len, iv, iv_len, out, out_len) ==
 			(ssize_t)out_len;
 }

@@ -558,15 +572,32 @@ LIB_EXPORT bool l_aead_cipher_decrypt(struct l_aead_cipher *cipher,
 					const void *nonce, size_t nonce_len,
 					void *out, size_t out_len)
 {
+	uint8_t ccm_iv[16];
+	const uint8_t *iv;
+	ssize_t iv_len;
+
 	if (unlikely(!cipher))
 		return false;
 
 	if (unlikely(!in) || unlikely(!out))
 		return false;
 
+	if (cipher->type == L_AEAD_CIPHER_AES_CCM) {
+		iv_len = build_ccm_iv(nonce, nonce_len, ccm_iv);
+		if (unlikely(iv_len < 0))
+			return false;
+
+		iv = ccm_iv;
+	} else {
+		if (unlikely(nonce_len != l_aead_cipher_get_ivlen(cipher)))
+			return false;
+
+		iv = nonce;
+		iv_len = nonce_len;
+	}
+
 	return operate_cipher(cipher->decrypt_sk, ALG_OP_DECRYPT, in, in_len,
-				ad, ad_len, nonce, nonce_len, out, out_len,
-				l_aead_cipher_get_ivlen(cipher)) ==
+				ad, ad_len, iv, iv_len, out, out_len) ==
 			(ssize_t)out_len;
 }

Signature · 1 line

2.19.1

Andrew Zaborowski

author

2018-11-14 13:18 UTC #1 lore raw

PATCH 2/12 cipher: Add gcm(aes) AEAD cipher

---
 ell/cipher.c | 10 +++++++---
 ell/cipher.h |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

ell/cipher.c +7 -3

index af4bc10..0f5804a 100644
@@ -202,6 +202,8 @@ static const char *aead_cipher_type_to_name(enum l_aead_cipher_type type)
 	switch (type) {
 	case L_AEAD_CIPHER_AES_CCM:
 		return "ccm(aes)";
+	case L_AEAD_CIPHER_AES_GCM:
+		return "gcm(aes)";
 	}
 
 	return NULL;

@@ -218,7 +220,7 @@ LIB_EXPORT struct l_aead_cipher *l_aead_cipher_new(enum l_aead_cipher_type type,
 	if (unlikely(!key))
 		return NULL;
 
-	if (type != L_AEAD_CIPHER_AES_CCM)
+	if (type != L_AEAD_CIPHER_AES_CCM && type != L_AEAD_CIPHER_AES_GCM)
 		return NULL;
 
 	cipher = l_new(struct l_aead_cipher, 1);

@@ -510,6 +512,8 @@ static size_t l_aead_cipher_get_ivlen(struct l_aead_cipher *cipher)
 	switch (cipher->type) {
 	case L_AEAD_CIPHER_AES_CCM:
 		return 16;
+	case L_AEAD_CIPHER_AES_GCM:
+		return 12;
 	}
 
 	return 0;

@@ -633,7 +637,7 @@ static void init_supported()
 
 	strcpy((char *) salg.salg_type, "aead");
 
-	for (a = L_AEAD_CIPHER_AES_CCM; a <= L_AEAD_CIPHER_AES_CCM; a++) {
+	for (a = L_AEAD_CIPHER_AES_CCM; a <= L_AEAD_CIPHER_AES_GCM; a++) {
 		strcpy((char *) salg.salg_name, aead_cipher_type_to_name@);
 
 		if (bind(sk, (struct sockaddr *) &salg, sizeof(salg)) < 0)

@@ -657,7 +661,7 @@ LIB_EXPORT bool l_cipher_is_supported(enum l_cipher_type type)
 
 LIB_EXPORT bool l_aead_cipher_is_supported(enum l_aead_cipher_type type)
 {
-	if (type != L_AEAD_CIPHER_AES_CCM)
+	if (type != L_AEAD_CIPHER_AES_CCM && type != L_AEAD_CIPHER_AES_GCM)
 		return false;
 
 	init_supported();

ell/cipher.h +1 -0

index d787f33..946b9ad 100644
@@ -62,6 +62,7 @@ struct l_aead_cipher;
 
 enum l_aead_cipher_type {
 	L_AEAD_CIPHER_AES_CCM = 0,
+	L_AEAD_CIPHER_AES_GCM,
 };
 
 struct l_aead_cipher *l_aead_cipher_new(enum l_aead_cipher_type type,

Signature · 1 line

2.19.1

Denis Kenzior

Applied

2018-11-14 21:17 UTC #2 lore raw

Re: [PATCH 02/12] cipher: Add gcm(aes) AEAD cipher

Hi Andrew,

Quoted 5 lines · Andrew Zaborowski

---
  ell/cipher.c | 10 +++++++---
  ell/cipher.h |  1 +
  2 files changed, 8 insertions(+), 3 deletions(-)

Applied, thanks.

Regards,
-Denis

Andrew Zaborowski

author

2018-11-14 13:18 UTC #3 lore raw

PATCH 3/12 unit: Add gcm(aes) test vectors

In reply to Andrew Zaborowski

---
 unit/test-cipher.c | 127 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 114 insertions(+), 13 deletions(-)

unit/test-cipher.c +114 ~~-13~~

index 4765c3e..fe9ae72 100644
@@ -124,7 +124,8 @@ static void test_arc4(const void *data)
 	l_cipher_free(cipher);
 }
 
-struct ccm_test_vector {
+struct aead_test_vector {
+	enum l_aead_cipher_type type;
 	char *aad;
 	char *plaintext;
 	char *key;

@@ -133,7 +134,8 @@ struct ccm_test_vector {
 	char *tag;
 };
 
-static const struct ccm_test_vector ccm_long_nonce = {
+static const struct aead_test_vector ccm_long_nonce = {
+	.type = L_AEAD_CIPHER_AES_CCM,
 	.aad =
 	"333b6b8fda49c6e671bad05c7e2cafa88bd47f9b0aef1a358bc87d04f26f6c82",
 	.plaintext =

@@ -145,7 +147,8 @@ static const struct ccm_test_vector ccm_long_nonce = {
 	.tag = "e0a03b982c5afc8a937373d7d2b0e7a3"
 };
 
-static const struct ccm_test_vector ccm_short_nonce = {
+static const struct aead_test_vector ccm_short_nonce = {
+	.type = L_AEAD_CIPHER_AES_CCM,
 	.plaintext =
 	"a3b3fdf26d213f83c5f656b00f77253b68959c188767d584914887602c787595",
 	.aad =

@@ -157,7 +160,8 @@ static const struct ccm_test_vector ccm_short_nonce = {
 	.tag = "ee007aafe91135c39855ebf3db96d7ff"
 };
 
-static const struct ccm_test_vector ccm_no_aad = {
+static const struct aead_test_vector ccm_no_aad = {
+	.type = L_AEAD_CIPHER_AES_CCM,
 	.plaintext =
 	"90795fffab99cffdeee5cadafe448ea4df74c480f9d7e1e481ee49adeee2732a",
 	.aad = "",

@@ -168,7 +172,94 @@ static const struct ccm_test_vector ccm_no_aad = {
 	.tag = "efd1dc938802cd845a16f32a60eabd0f"
 };
 
-static void test_aes_ccm(const void *data)
+/* https://tools.ietf.org/html/draft-mcgrew-gcm-test-01 */
+
+static const struct aead_test_vector gcm_test1 = {
+	.type = L_AEAD_CIPHER_AES_GCM,
+	.aad = "000043218765432100000000",
+	.plaintext =
+	"45000048699a000080114db7c0a80102c0a801010a9bf15638d3010000010000"
+	"00000000045f736970045f756470037369700963796265726369747902646b00"
+	"0021000101020201",
+	/* 128-bit key */
+	.key = "4c80cdefbb5d10da906ac73c3613a634",
+	.nonce = "2e443b684956ed7e3b244cfe",
+	.ciphertext =
+	"fecf537e729d5b07dc30df528dd22b768d1b98736696a6fd348509fa13ceac34"
+	"cfa2436f14a3f3cf65925bf1f4a13c5d15b21e1884f5ff6247aeabb786b93bce"
+	"61bc17d768fd9732",
+	.tag = "459018148f6cbe722fd04796562dfdb4",
+};
+
+static const struct aead_test_vector gcm_test2 = {
+	.type = L_AEAD_CIPHER_AES_GCM,
+	.aad = "0000a5f80000000a",
+	.plaintext =
+	"45000028a4ad4000400678800a01038f0a010612802306b8cb712602dd6bb03e"
+	"501016d075680001",
+	/* 192-bit key */
+	.key = "feffe9928665731c6d6a8f9467308308feffe9928665731c",
+	.nonce = "cafebabefacedbaddecaf888",
+	.ciphertext =
+	"a5b1f8066029aea40e598b8122de02420938b3ab33f828e687b8858b5bfbdbd0"
+	"315b27452144cc77",
+	.tag = "95457b9652037f5318027b5b4cd7a636",
+};
+
+static const struct aead_test_vector gcm_test3 = {
+	.type = L_AEAD_CIPHER_AES_GCM,
+	.aad = "4a2cbfe300000002",
+	.plaintext =
+	"4500003069a6400080062690c0a801029389155e0a9e008b2dc57ee000000000"
+	"7002400020bf0000020405b40101040201020201",
+	/* 256-bit key */
+	.key =
+	"abbccddef00112233445566778899aababbccddef00112233445566778899aab",
+	.nonce = "112233440102030405060708",
+	.ciphertext =
+	"ff425c9b724599df7a3bcd510194e00d6a78107f1b0b1cbf06efae9d65a5d763"
+	"748a637985771d347f0545659f14e99def842d8e",
+	.tag = "b335f4eecfdbf831824b4c4915956c96",
+};
+
+static const struct aead_test_vector gcm_test4 = {
+	.type = L_AEAD_CIPHER_AES_GCM,
+	.aad = "0000000000000001",
+	.plaintext =
+	"4500003c99c500008001cb7a40679318010101010800075c0200440061626364"
+	"65666768696a6b6c6d6e6f707172737475767761626364656667686901020201",
+	.key = "00000000000000000000000000000000",
+	.nonce = "000000000000000000000000",
+	.ciphertext =
+	"4688daf2f973a392732909c331d56d60f694abaa414b5e7ff5fdcdfff5e9a284"
+	"456476492719ffb64de7d9dca1e1d894bc3bd57873ed4d181d19d4d5c8c18af3",
+	.tag = "f821d496eeb096e98ad2b69e4799c71d",
+};
+
+static const struct aead_test_vector gcm_test5 = {
+	.type = L_AEAD_CIPHER_AES_GCM,
+	.aad = "335467aeffffffff",
+	.plaintext = "01020201",
+	.key = "7d773d00c144c525ac619d18c84a3f47",
+	.nonce = "d966426743457e9182443bc6",
+	.ciphertext = "437f866b",
+	.tag = "cb3f699fe9b0822bac961c4504bef270",
+};
+
+static const struct aead_test_vector gcm_test6 = {
+	.type = L_AEAD_CIPHER_AES_GCM,
+	.aad =
+	"0000432100000007000000000000000045000030da3a00008001df3bc0a80005"
+	"c0a800010800c6cd020007006162636465666768696a6b6c6d6e6f7071727374"
+	"01020201",
+	.plaintext = "",
+	.key = "4c80cdefbb5d10da906ac73c3613a634",
+	.nonce = "22433c640000000000000000",
+	.ciphertext = "",
+	.tag = "f2a9a836e155106aa8dcd618e4099aaa",
+};
+
+static void test_aead(const void *data)
 {
 	struct l_aead_cipher *cipher;
 	char *encbuf;

@@ -177,10 +268,11 @@ static void test_aes_ccm(const void *data)
 	size_t decbuflen;
 	int r;
 	bool success;
-	const struct ccm_test_vector *tv = data;
+	const struct aead_test_vector *tv = data;
 
 	size_t ptlen;
-	uint8_t *pt = l_util_from_hexstring(tv->plaintext, &ptlen);
+	uint8_t *pt = l_util_from_hexstring(tv->plaintext, &ptlen) ?:
+		(uint8_t[]) {};
 	size_t aadlen;
 	uint8_t *aad = l_util_from_hexstring(tv->aad, &aadlen);
 	size_t keylen;

@@ -199,7 +291,7 @@ static void test_aes_ccm(const void *data)
 	decbuf = alloca(decbuflen);
 	memset(decbuf, 0, decbuflen);
 
-	cipher = l_aead_cipher_new(L_AEAD_CIPHER_AES_CCM, key, keylen, taglen);
+	cipher = l_aead_cipher_new(tv->type, key, keylen, taglen);
 	assert(cipher);
 
 	success = l_aead_cipher_encrypt(cipher, pt, ptlen, aad, aadlen,

@@ -225,7 +317,8 @@ static void test_aes_ccm(const void *data)
 	assert(!r);
 
 	l_aead_cipher_free(cipher);
-	l_free(pt);
+	if (ptlen)
+		l_free(pt);
 	l_free(key);
 	l_free(aad);
 	l_free(nonce);

@@ -246,10 +339,18 @@ int main(int argc, char *argv[])
 		l_test_add("arc4", test_arc4, NULL);
 
 	if (l_aead_cipher_is_supported(L_AEAD_CIPHER_AES_CCM)) {
-		l_test_add("aes_ccm long nonce", test_aes_ccm, &ccm_long_nonce);
-		l_test_add("aes_ccm short nonce", test_aes_ccm,
-							&ccm_short_nonce);
-		l_test_add("aes_ccm no AAD", test_aes_ccm, &ccm_no_aad);
+		l_test_add("aes_ccm long nonce", test_aead, &ccm_long_nonce);
+		l_test_add("aes_ccm short nonce", test_aead, &ccm_short_nonce);
+		l_test_add("aes_ccm no AAD", test_aead, &ccm_no_aad);
+	}
+
+	if (l_aead_cipher_is_supported(L_AEAD_CIPHER_AES_GCM)) {
+		l_test_add("aes_gcm test 1", test_aead, &gcm_test1);
+		l_test_add("aes_gcm test 2", test_aead, &gcm_test2);
+		l_test_add("aes_gcm test 3", test_aead, &gcm_test3);
+		l_test_add("aes_gcm test 4", test_aead, &gcm_test4);
+		l_test_add("aes_gcm test 5", test_aead, &gcm_test5);
+		l_test_add("aes_gcm test 6", test_aead, &gcm_test6);
 	}
 
 	return l_test_run();

Signature · 1 line

2.19.1

Andrew Zaborowski

author

2018-11-14 13:18 UTC #4 lore raw

PATCH 4/12 tls: Fix functions that return error strings

In reply to Andrew Zaborowski

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

ell/tls.c +3 -3

index 4b37785..9dc97ff 100644
@@ -204,7 +204,7 @@ static bool tls_change_cipher_spec(struct l_tls *tls, bool txrx,
 	struct tls_bulk_encryption_algorithm *enc;
 	struct tls_mac_algorithm *mac;
 	int key_offset;
-	char error_buf[50];
+	static char error_buf[200];
 
 	if (tls->cipher[txrx]) {
 		l_cipher_free(tls->cipher[txrx]);

@@ -533,7 +533,7 @@ enum tls_handshake_type {
 
 static const char *tls_handshake_type_to_str(enum tls_handshake_type type)
 {
-	static char buf[40];
+	static char buf[100];
 
 	switch (type) {
 	SWITCH_ENUM_TO_STR(TLS_HELLO_REQUEST)

@@ -2529,7 +2529,7 @@ LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
 
 const char *tls_handshake_state_to_str(enum tls_handshake_state state)
 {
-	static char buf[40];
+	static char buf[100];
 
 	switch (state) {
 	SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_HELLO)

Signature · 1 line

2.19.1

Denis Kenzior

Applied

2018-11-14 21:07 UTC #5 lore raw

Re: [PATCH 04/12] tls: Fix functions that return error strings

Hi Andrew,

Quoted 4 lines · Andrew Zaborowski

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

Applied, thanks.

Regards,
-Denis

Andrew Zaborowski

author

2018-11-14 13:18 UTC #6 lore raw

PATCH 5/12 tls: Add AEAD cipher support

In reply to Andrew Zaborowski

---
 ell/tls-private.h | 15 +++++++--
 ell/tls-record.c  | 82 +++++++++++++++++++++++++++++++++++++++++++++--
 ell/tls.c         | 65 ++++++++++++++++++++++++++++++-------
 3 files changed, 147 insertions(+), 15 deletions(-)

ell/tls-private.h +13 -2

index 4f680a0..a562846 100644
@@ -38,10 +38,15 @@ enum tls_cipher_type {
 
 struct tls_bulk_encryption_algorithm {
 	enum tls_cipher_type cipher_type;
-	enum l_cipher_type l_id;
+	union {
+		enum l_cipher_type l_id;
+		enum l_aead_cipher_type l_aead_id;
+	};
 	size_t key_length;
 	size_t iv_length;
+	size_t fixed_iv_length;
 	size_t block_length;
+	size_t auth_tag_length;
 };
 
 struct tls_hash_algorithm {

@@ -192,11 +197,17 @@ struct l_tls {
 
 	enum tls_cipher_type cipher_type[2];
 	struct tls_cipher_suite *cipher_suite[2];
-	struct l_cipher *cipher[2];
+	union {
+		struct l_cipher *cipher[2];
+		struct l_aead_cipher *aead_cipher[2];
+	};
 	struct l_checksum *mac[2];
 	size_t mac_length[2];
 	size_t block_length[2];
 	size_t record_iv_length[2];
+	size_t fixed_iv_length[2];
+	uint8_t fixed_iv[2][32];
+	size_t auth_tag_length[2];
 	uint64_t seq_num[2];
 	/*
 	 * Some of the key and IV parts of the "current" state are kept

ell/tls-record.c +80 -2

106 lines not on this page · raw message

ell/tls.c +54 ~~-11~~

115 lines not on this page · raw message

Signature · 1 line

2.19.1

Andrew Zaborowski

author

2018-11-14 13:18 UTC #7 lore raw

PATCH 6/12 tls: Add TLS_RSA_WITH_AES_128_GCM_SHA256 cipher suite

In reply to Andrew Zaborowski

Defined in RFC5288. This cipher suite is preferred by a lot of servers over the block mode AES suites (but the same cipher with ECDHE key exchange is liked even more.)

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

ell/tls.c +14 -0

index fd26306..fab4ce6 100644
@@ -413,6 +413,13 @@ static struct tls_bulk_encryption_algorithm tls_rc4 = {
 	.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,
 };
 
 static struct tls_mac_algorithm tls_md5 = {

@@ -462,6 +469,13 @@ static struct tls_cipher_suite tls_cipher_suite_pref[] = {
 		.mac = &tls_sha256,
 		.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",

Signature · 1 line

2.19.1

Andrew Zaborowski

author

2018-11-14 13:18 UTC #8 lore raw

PATCH 7/12 tls: Add support for PRFs other than P_SHA256

In reply to Andrew Zaborowski

Add the ability for the cipher suite to define a PRF hash other than SHA256 but without allowing PRFs other than the RFC5246 TLS PRF (called P_hash) for now. All our cipher suites until recently were specified in RFC5256 and so all used P_SHA256. Other RFCs define cipher suites that use different hash algorithms for the TLS PRF but I haven't seen a definition of a completely different PRF so far. We already have unit tests for P_SHA256, P_SHA238 and P_SHA512.

l_tls_prf_get_bytes and tls_prf_get_bytes don't take the hash type parameter anymore, this is backwards incompatible and TLS-based EAP method implementations need to be updated.

---
 ell/tls-private.h |  8 ++---
 ell/tls.c         | 89 ++++++++++++++++++++++++++++-------------------
 ell/tls.h         |  4 +--
 3 files changed, 58 insertions(+), 43 deletions(-)

ell/tls-private.h +4 -4

index a562846..4f98cfe 100644
@@ -91,6 +91,7 @@ struct tls_cipher_suite {
 	struct tls_bulk_encryption_algorithm *encryption;
 	struct tls_key_exchange_algorithm *key_xchg;
 	struct tls_mac_algorithm *mac;
+	enum l_checksum_type prf_hmac;
 };
 
 struct tls_compression_method {

@@ -123,8 +124,8 @@ enum tls_content_type {
  * Finished message.  If we're sent a hash of a different type (in TLS 1.2+)
  * and need to verify we'll give up.
  * SHA1 and MD5 are explicitly required by versions < 1.2 and 1.2 requires
- * that the Finished hash is the same as used for the PRF, which in all
- * our supported cipher suites is defined to be SHA256.
+ * that the Finished hash is the same as used for the PRF so we need to
+ * keep@least the hashes our supported cipher suites specify for the PRF.
  */
 enum handshake_hash_type {
 	HANDSHAKE_HASH_SHA256,

@@ -134,8 +135,6 @@ enum handshake_hash_type {
 };
 #define HANDSHAKE_HASH_MAX_SIZE	32
 
-#define HANDSHAKE_HASH_TLS12 HANDSHAKE_HASH_SHA256
-
 struct l_tls {
 	bool server;

@@ -178,6 +177,7 @@ struct l_tls {
 	struct l_key *peer_pubkey;
 	size_t peer_pubkey_size;
 	enum handshake_hash_type signature_hash;
+	const struct tls_hash_algorithm *prf_hmac;
 
 	/* SecurityParameters current and pending */

ell/tls.c +53 ~~-36~~

196 lines not on this page · raw message

ell/tls.h +1 -3

index 893fd63..e1a5a21 100644
@@ -108,9 +108,7 @@ const char *l_tls_alert_to_str(enum l_tls_alert_desc desc);
 
 enum l_checksum_type;
 
-bool l_tls_prf_get_bytes(struct l_tls *tls,
-				enum l_checksum_type type,
-				size_t hash_len, bool use_master_secret,
+bool l_tls_prf_get_bytes(struct l_tls *tls, bool use_master_secret,
 				const char *label, uint8_t *buf, size_t len);
 
 bool l_tls_set_debug(struct l_tls *tls, l_tls_debug_cb_t function,

Signature · 1 line

2.19.1

Denis Kenzior

2018-11-14 21:11 UTC #9 lore raw

Re: [PATCH 07/12] tls: Add support for PRFs other than P_SHA256

Hi Andrew,

Quoted 11 lines · Andrew Zaborowski

Add the ability for the cipher suite to define a PRF hash other than SHA256 but without allowing PRFs other than the RFC5246 TLS PRF (called P_hash) for now. All our cipher suites until recently were specified in RFC5256 and so all used P_SHA256. Other RFCs define cipher suites that use different hash algorithms for the TLS PRF but I haven't seen a definition of a completely different PRF so far. We already have unit tests for P_SHA256, P_SHA238 and P_SHA512.
l_tls_prf_get_bytes and tls_prf_get_bytes don't take the hash type parameter anymore, this is backwards incompatible and TLS-based EAP method implementations need to be updated.

Are you sending patches for these?

Quoted 104 lines

---
  ell/tls-private.h |  8 ++---
  ell/tls.c         | 89 ++++++++++++++++++++++++++++-------------------
  ell/tls.h         |  4 +--
  3 files changed, 58 insertions(+), 43 deletions(-)

ell/tls-private.h +4 -4

index a562846..4f98cfe 100644
@@ -91,6 +91,7 @@ struct tls_cipher_suite {
  	struct tls_bulk_encryption_algorithm *encryption;
  	struct tls_key_exchange_algorithm *key_xchg;
  	struct tls_mac_algorithm *mac;
+	enum l_checksum_type prf_hmac;
  };
 
  struct tls_compression_method {

@@ -123,8 +124,8 @@ enum tls_content_type {
   * Finished message.  If we're sent a hash of a different type (in TLS 1.2+)
   * and need to verify we'll give up.
   * SHA1 and MD5 are explicitly required by versions < 1.2 and 1.2 requires
- * that the Finished hash is the same as used for the PRF, which in all
- * our supported cipher suites is defined to be SHA256.
+ * that the Finished hash is the same as used for the PRF so we need to
+ * keep at least the hashes our supported cipher suites specify for the PRF.
   */
  enum handshake_hash_type {
  	HANDSHAKE_HASH_SHA256,

@@ -134,8 +135,6 @@ enum handshake_hash_type {
  };
  #define HANDSHAKE_HASH_MAX_SIZE	32
 
-#define HANDSHAKE_HASH_TLS12 HANDSHAKE_HASH_SHA256
-
  struct l_tls {
  	bool server;

@@ -178,6 +177,7 @@ struct l_tls {
  	struct l_key *peer_pubkey;
  	size_t peer_pubkey_size;
  	enum handshake_hash_type signature_hash;
+	const struct tls_hash_algorithm *prf_hmac;
 
  	/* SecurityParameters current and pending */

ell/tls.c +10 ~~-11~~

index fab4ce6..93a3bd5 100644
@@ -114,39 +114,35 @@ void tls12_prf(enum l_checksum_type type, size_t hash_len,
  }
 
  void tls_prf_get_bytes(struct l_tls *tls,
-				enum l_checksum_type type, size_t hash_len,
  				const void *secret, size_t secret_len,
  				const char *label,
  				const void *seed, size_t seed_len,
  				uint8_t *buf, size_t len)
  {
  	if (tls->negotiated_version >= TLS_V12)
-		tls12_prf(type, hash_len, secret, secret_len, label,
+		tls12_prf(tls->prf_hmac->l_id, tls->prf_hmac->length,
+				secret, secret_len, label,
  				seed, seed_len, buf, len);
  	else
  		tls10_prf(secret, secret_len, label, seed, seed_len, buf, len);
  }
 
-LIB_EXPORT bool l_tls_prf_get_bytes(struct l_tls *tls,
-				enum l_checksum_type type,
-				size_t hash_len, bool use_master_secret,
+LIB_EXPORT bool l_tls_prf_get_bytes(struct l_tls *tls, bool use_master_secret,
  				const char *label, uint8_t *buf, size_t len)
  {
  	uint8_t seed[64];
 
-	if (unlikely(!tls))
+	if (unlikely(!tls || !tls->prf_hmac))
  		return false;
 
  	memcpy(seed +  0, tls->pending.client_random, 32);
  	memcpy(seed + 32, tls->pending.server_random, 32);
 
  	if (use_master_secret)
-		tls_prf_get_bytes(tls, type, hash_len,
-						tls->pending.master_secret, 48,
-						label, seed, 64, buf, len);
+		tls_prf_get_bytes(tls, tls->pending.master_secret, 48,
+					label, seed, 64, buf, len);
  	else
-		tls_prf_get_bytes(tls, type, hash_len, "", 0,
-						label, seed, 64, buf, len);
+		tls_prf_get_bytes(tls, "", 0, label, seed, 64, buf, len);
 
  	memset(seed, 0, 64);

@@ -568,6 +564,21 @@ err:
  	return false;
  }
 
+static void tls_set_prf_hmac(struct l_tls *tls)
+{
+	enum handshake_hash_type hash;

ell/tls.c err: · line 579

Quoted 10 lines

ell/tls.c +10 -0

+
+	if (tls->pending.cipher_suite->prf_hmac == L_CHECKSUM_NONE) {
+		tls->prf_hmac = &tls_handshake_hash_data[HANDSHAKE_HASH_SHA256];
+		return;
+	}
+
+	for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
+		if (tls_handshake_hash_data[hash].l_id ==
+				tls->pending.cipher_suite->prf_hmac)
+			tls->prf_hmac = &tls_handshake_hash_data[hash];

No break?

And also no return value? If this is guaranteed to succeed, then a comment might be needed.

Quoted 150 lines

ell/tls.c +33 ~~-25~~

+}
+
  enum tls_handshake_type {
  	TLS_HELLO_REQUEST	= 0,
  	TLS_CLIENT_HELLO	= 1,

@@ -972,8 +983,7 @@ static void tls_generate_master_secret(struct l_tls *tls,
  	memcpy(seed +  0, tls->pending.client_random, 32);
  	memcpy(seed + 32, tls->pending.server_random, 32);
 
-	tls_prf_get_bytes(tls, L_CHECKSUM_SHA256, 32,
-				pre_master_secret, pre_master_secret_len,
+	tls_prf_get_bytes(tls, pre_master_secret, pre_master_secret_len,
  				"master secret", seed, 64,
  				tls->pending.master_secret, 48);

@@ -1003,8 +1013,7 @@ static void tls_generate_master_secret(struct l_tls *tls,
  	memcpy(seed +  0, tls->pending.server_random, 32);
  	memcpy(seed + 32, tls->pending.client_random, 32);
 
-	tls_prf_get_bytes(tls, L_CHECKSUM_SHA256, 32,
-				tls->pending.master_secret, 48,
+	tls_prf_get_bytes(tls, tls->pending.master_secret, 48,
  				"key expansion", seed, 64,
  				tls->pending.key_block, key_block_size);

@@ -1267,10 +1276,10 @@ static bool tls_send_certificate_verify(struct l_tls *tls)
  	if (sign_len < 0)
  		return false;
 
-	/* Stop maintaining handshake message hashes other than SHA256. */
+	/* Stop maintaining handshake message hashes other than the PRF hash */
  	if (tls->negotiated_version >= TLS_V12)
  		for (i = 0; i < __HANDSHAKE_HASH_COUNT; i++)
-			if (i != HANDSHAKE_HASH_SHA256)
+			if (&tls_handshake_hash_data[i] != tls->prf_hmac)
  				tls_drop_handshake_hash(tls, i);
 
  	tls_tx_handshake(tls, TLS_CERTIFICATE_VERIFY, buf,

@@ -1294,23 +1303,22 @@ static void tls_send_finished(struct l_tls *tls)
  	size_t seed_len;
 
  	if (tls->negotiated_version >= TLS_V12) {
-		/*
-		 * Same hash type as that used for the PRF, i.e. SHA256
-		 * unless an exotic cipher suite was negotiated that
-		 * dictates a different hash for the PRF and for the
-		 * Finished hash.  We don't support any such ciphers so
-		 * it's always SHA256.
-		 */
-		tls_get_handshake_hash(tls, HANDSHAKE_HASH_TLS12, seed);
-		seed_len = tls_handshake_hash_data[HANDSHAKE_HASH_TLS12].length;
+		/* Same hash type as that used for the PRF (usually SHA256) */
+		enum handshake_hash_type hash;
+
+		for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
+			if (&tls_handshake_hash_data[hash] == tls->prf_hmac)
+				break;
+
+		tls_get_handshake_hash(tls, hash, seed);
+		seed_len = tls_handshake_hash_data[hash].length;
  	} else {
  		tls_get_handshake_hash(tls, HANDSHAKE_HASH_MD5, seed + 0);
  		tls_get_handshake_hash(tls, HANDSHAKE_HASH_SHA1, seed + 16);
  		seed_len = 36;
  	}
 
-	tls_prf_get_bytes(tls, L_CHECKSUM_SHA256, 32,
-				tls->pending.master_secret, 48,
+	tls_prf_get_bytes(tls, tls->pending.master_secret, 48,
  				tls->server ? "server finished" :
  				"client finished",
  				seed, seed_len,

@@ -1336,8 +1344,14 @@ static bool tls_verify_finished(struct l_tls *tls, const uint8_t *received,
  	}
 
  	if (tls->negotiated_version >= TLS_V12) {
-		seed = tls->prev_digest[HANDSHAKE_HASH_TLS12];
-		seed_len = tls_handshake_hash_data[HANDSHAKE_HASH_TLS12].length;
+		enum handshake_hash_type hash;
+
+		for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
+			if (&tls_handshake_hash_data[hash] == tls->prf_hmac)
+				break;
+
+		seed = tls->prev_digest[hash];
+		seed_len = tls_handshake_hash_data[hash].length;
  	} else {
  		seed = alloca(36);
  		memcpy(seed + 0, tls->prev_digest[HANDSHAKE_HASH_MD5], 16);

@@ -1345,8 +1359,7 @@ static bool tls_verify_finished(struct l_tls *tls, const uint8_t *received,
  		seed_len = 36;
  	}
 
-	tls_prf_get_bytes(tls, L_CHECKSUM_SHA256, 32,
-				tls->pending.master_secret, 48,
+	tls_prf_get_bytes(tls, tls->pending.master_secret, 48,
  				tls->server ? "client finished" :
  				"server finished",
  				seed, seed_len,

@@ -1488,6 +1501,8 @@ static void tls_handle_client_hello(struct l_tls *tls,
  		return;
  	}
 
+	tls_set_prf_hmac(tls);
+
  	TLS_DEBUG("Negotiated %s", tls->pending.cipher_suite->name);
 
  	/* Select a compression method */

@@ -1598,6 +1613,8 @@ static void tls_handle_server_hello(struct l_tls *tls,
  		return;
  	}
 
+	tls_set_prf_hmac(tls);
+
  	TLS_DEBUG("Negotiated %s", tls->pending.cipher_suite->name);
 
  	tls->pending.compression_method =

@@ -1839,11 +1856,11 @@ static void tls_handle_certificate_request(struct l_tls *tls,
 
  		/*
  		 * We can now safely stop maintaining handshake message
-		 * hashes other than SHA256 and the one selected for
+		 * hashes other than the PRF hash and the one selected for
  		 * signing.
  		 */
  		for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
-			if (hash != HANDSHAKE_HASH_SHA256 &&
+			if (&tls_handshake_hash_data[hash] != tls->prf_hmac &&
  					hash != tls->signature_hash)
  				tls_drop_handshake_hash(tls, hash);
  	}

@@ -1991,10 +2008,10 @@ static void tls_handle_certificate_verify(struct l_tls *tls,
  						tls_get_prev_digest_by_id))
  		return;
 
-	/* Stop maintaining handshake message hashes other than SHA256. */
+	/* Stop maintaining handshake message hashes other than the PRF hash */
  	if (tls->negotiated_version >= TLS_V12)
  		for (i = 0; i < __HANDSHAKE_HASH_COUNT; i++)
-			if (i != HANDSHAKE_HASH_SHA256)
+			if (&tls_handshake_hash_data[i] != tls->prf_hmac)
  				tls_drop_handshake_hash(tls, i);
 
  	/*

ell/tls.h +0 -0

index 893fd63..e1a5a21 100644

ell/tls.h const char *l_tls_alert_to_str(enum l_tls_alert_desc desc); · line 114

Quoted 10 lines

ell/tls.h +1 -3

@@ -108,9 +108,7 @@ const char *l_tls_alert_to_str(enum l_tls_alert_desc desc);
 
  enum l_checksum_type;
 
-bool l_tls_prf_get_bytes(struct l_tls *tls,
-				enum l_checksum_type type,
-				size_t hash_len, bool use_master_secret,
+bool l_tls_prf_get_bytes(struct l_tls *tls, bool use_master_secret,
  				const char *label, uint8_t *buf, size_t len);
 
  bool l_tls_set_debug(struct l_tls *tls, l_tls_debug_cb_t function,

Regards,
-Denis

Andrew Zaborowski

author

2018-11-14 13:18 UTC #10 lore raw

PATCH 8/12 tls: Add TLS_RSA_WITH_AES_256_GCM_SHA384 cipher suite

In reply to Andrew Zaborowski

Defined in RFC5288. This cipher suite is preferred by some servers over TLS_RSA_WITH_AES_128_GCM_SHA256 but not by others.

---
 ell/tls-private.h |  3 ++-
 ell/tls.c         | 16 ++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

ell/tls-private.h +2 -1

index 4f98cfe..5b76e73 100644
@@ -128,12 +128,13 @@ enum tls_content_type {
  * keep at least the hashes our supported cipher suites specify for the PRF.
  */
 enum handshake_hash_type {
+	HANDSHAKE_HASH_SHA384,
 	HANDSHAKE_HASH_SHA256,
 	HANDSHAKE_HASH_MD5,
 	HANDSHAKE_HASH_SHA1,
 	__HANDSHAKE_HASH_COUNT,
 };
-#define HANDSHAKE_HASH_MAX_SIZE	32
+#define HANDSHAKE_HASH_MAX_SIZE	48
 
 struct l_tls {
 	bool server;

ell/tls.c +16 -0

index 93a3bd5..0645897 100644
@@ -416,6 +416,13 @@ static struct tls_bulk_encryption_algorithm tls_rc4 = {
 	.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 = {

@@ -465,6 +472,14 @@ static struct tls_cipher_suite tls_cipher_suite_pref[] = {
 		.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",

@@ -530,6 +545,7 @@ static struct tls_compression_method *tls_find_compression_method(
 }
 
 static 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" },
 	[HANDSHAKE_HASH_SHA1]	= { 2, L_CHECKSUM_SHA1, 20, "SHA1" },

Signature · 1 line

2.19.1

Andrew Zaborowski

author

2018-11-14 13:18 UTC #11 lore raw

PATCH 9/12 tls: Make tls_prf_get_bytes static

In reply to Andrew Zaborowski

tls_prf_get_bytes is not used outside tls.c and everyone can use l_tls_prf_get_bytes()

---
 ell/tls-private.h | 7 -------
 ell/tls.c         | 2 +-
 2 files changed, 1 insertion(+), 8 deletions(-)

ell/tls-private.h +0 -7

index 5b76e73..0d8592a 100644
@@ -265,13 +265,6 @@ void tls_cert_free_certchain(struct tls_cert *cert);
 
 enum tls_cert_key_type tls_cert_get_pubkey_type(struct tls_cert *cert);
 
-void tls_prf_get_bytes(struct l_tls *tls,
-				enum l_checksum_type type, size_t hash_len,
-				const void *secret, size_t secret_len,
-				const char *label,
-				const void *seed, size_t seed_len,
-				uint8_t *buf, size_t len);
-
 #define TLS_DEBUG(fmt, args...)	\
 	l_util_debug(tls->debug_handler, tls->debug_data, "%s:%i " fmt,	\
 			__func__, __LINE__, ## args)

ell/tls.c +1 -1

index 0645897..534d22c 100644
@@ -113,7 +113,7 @@ void tls12_prf(enum l_checksum_type type, size_t hash_len,
 	l_checksum_free(hmac);
 }
 
-void tls_prf_get_bytes(struct l_tls *tls,
+static void tls_prf_get_bytes(struct l_tls *tls,
 				const void *secret, size_t secret_len,
 				const char *label,
 				const void *seed, size_t seed_len,

Signature · 1 line

2.19.1

Andrew Zaborowski

author

2018-11-14 13:19 UTC #12 lore raw

PATCH 10/12 tls: Basic cipher/hash support checking

In reply to Andrew Zaborowski

Use our _is_supported APIs to check each cipher suites compatibility with the running kernel to try to avoid proposing cipher suites we can't handle in the negotiation messages (this isn't perfect yet) and to try to fail early on configuration errors and hopefully with an error message (this can still be improved too.)

---
 ell/tls.c | 159 +++++++++++++++++++++++++++++++++++++++++++++---------
 ell/tls.h |   2 +-
 2 files changed, 136 insertions(+), 25 deletions(-)

ell/tls.c +135 ~~-24~~

index 534d22c..4124e1c 100644
@@ -513,6 +513,88 @@ static struct tls_cipher_suite tls_cipher_suite_pref[] = {
 	},
 };
 
+static bool tls_cipher_suite_is_compatible(struct l_tls *tls,
+					const struct tls_cipher_suite *suite,
+					const char **error)
+{
+	static char error_buf[200];
+
+	if (suite->encryption &&
+			suite->encryption->cipher_type == TLS_CIPHER_AEAD) {
+		if (tls->negotiated_version &&
+				tls->negotiated_version < TLS_V12) {
+			if (error) {
+				*error = error_buf;
+				snprintf(error_buf, sizeof(error_buf),
+						"Cipher suite %s uses an AEAD "
+						"cipher but TLS < 1.2 was "
+						"negotiated", suite->name);
+			}
+
+			return false;
+		}
+
+		if (!l_aead_cipher_is_supported(suite->encryption->l_aead_id)) {
+			if (error) {
+				*error = error_buf;
+				snprintf(error_buf, sizeof(error_buf),
+						"Cipher suite %s's AEAD cipher "
+						"algorithm not supported by "
+						"the kernel", suite->name);
+			}
+
+			return false;
+		}
+	} else if (suite->encryption) { /* Block or stream cipher */
+		if (!l_cipher_is_supported(suite->encryption->l_id)) {
+			if (error) {
+				*error = error_buf;
+				snprintf(error_buf, sizeof(error_buf),
+						"Cipher suite %s's block/stream"
+						" cipher algorithm not "
+						"supported by the kernel",
+						suite->name);
+			}
+
+			return false;
+		}
+	}
+
+	if (suite->mac &&
+			!l_checksum_is_supported(suite->mac->hmac_type, true)) {
+		if (error) {
+			*error = error_buf;
+			snprintf(error_buf, sizeof(error_buf),
+					"Cipher suite %s's HMAC algorithm not "
+					"supported by the kernel", suite->name);
+		}
+
+		return false;
+	}
+
+	if ((tls->negotiated_version && tls->negotiated_version < TLS_V12 &&
+			(!l_checksum_is_supported(L_CHECKSUM_MD5, true) ||
+			 !l_checksum_is_supported(L_CHECKSUM_SHA1, true))) ||
+			(tls->negotiated_version >= TLS_V12 &&
+			 !l_checksum_is_supported(
+					suite->prf_hmac != L_CHECKSUM_NONE ?
+					suite->prf_hmac : L_CHECKSUM_SHA256,
+					true))) {
+		if (error) {
+			*error = error_buf;
+			snprintf(error_buf, sizeof(error_buf),
+					"Cipher suite %s's PRF algorithm not "
+					"supported by the kernel", suite->name);
+		}
+
+		return false;
+	}
+
+	/* TODO: Check suite->key_xchg->validate_cert_key_type(tls->cert) */
+
+	return true;
+}
+
 static struct tls_cipher_suite *tls_find_cipher_suite(const uint8_t *id)
 {
 	int i;

@@ -755,11 +837,12 @@ static void tls_tx_handshake(struct l_tls *tls, int type, uint8_t *buf,
 	tls_tx_record(tls, TLS_CT_HANDSHAKE, buf, length);
 }
 
-static void tls_send_client_hello(struct l_tls *tls)
+static bool tls_send_client_hello(struct l_tls *tls)
 {
 	uint8_t buf[128 + 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;
 
 	/* Fill in the Client Hello body */

@@ -774,27 +857,38 @@ static void tls_send_client_hello(struct l_tls *tls)
 	*ptr++ = 0; /* No SessionID */
 
 	/*
-	 * We can list all supported key exchange mechanisms regardless of the
-	 * certificate type we are actually presenting (if any).
-	 *
-	 * TODO: perhaps scan /proc/crypto for supported ciphers so we don't
-	 * include ones that will cause an internal error later in the
-	 * handshake.  We can add camellia whan this is done.
+	 * FIXME: We do need to filter the cipher suites by key exchange
+	 * mechanism compatibility with the certificate but we don't normally
+	 * have the certificate at this point because we're called from
+	 * l_tls_new.  We also don't know the TLS version that's going to
+	 * be negotiated yet.
 	 */
-	l_put_be16(L_ARRAY_SIZE(tls_cipher_suite_pref) * 2, ptr);
+	len_ptr = ptr;
 	ptr += 2;
 
 	for (i = 0; i < (int) L_ARRAY_SIZE(tls_cipher_suite_pref); i++) {
+		if (!tls_cipher_suite_is_compatible(tls,
+						&tls_cipher_suite_pref[i],
+						NULL))
+			continue;
+
 		*ptr++ = tls_cipher_suite_pref[i].id[0];
 		*ptr++ = tls_cipher_suite_pref[i].id[1];
 	}
 
+	if (ptr == len_ptr + 2) {
+		TLS_DEBUG("No compatible cipher suites, check kernel config");
+		return false;
+	}
+
+	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++)
 		*ptr++ = tls_compression_pref[i].id;
 
 	tls_tx_handshake(tls, TLS_CLIENT_HELLO, buf, ptr - buf);
+	return true;
 }
 
 static void tls_send_server_hello(struct l_tls *tls)

@@ -1495,17 +1589,13 @@ static void tls_handle_client_hello(struct l_tls *tls,
 
 	/* Select a cipher suite according to client's preference list */
 	while (cipher_suites_size) {
-		/*
-		 * TODO: filter supported cipher suites by the certificate/key
-		 * type that was submitted by tls_set_auth_data() if any.
-		 * Perhaps just call cipher_suite->verify_cert_type() on each
-		 * cipher suite passing a pre-parsed certificate ASN.1 struct.
-		 */
-		tls->pending.cipher_suite =
-					tls_find_cipher_suite(cipher_suites);
+		struct tls_cipher_suite *suite =
+			tls_find_cipher_suite(cipher_suites);
 
-		if (tls->pending.cipher_suite)
-			break;
+		if (suite && tls_cipher_suite_is_compatible(tls, suite, NULL)) {
+			tls->pending.cipher_suite = suite;
+ 			break;
+		}
 
 		cipher_suites += 2;
 		cipher_suites_size -= 2;

@@ -1513,7 +1603,8 @@ static void tls_handle_client_hello(struct l_tls *tls,
 
 	if (!cipher_suites_size) {
 		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
-				"No common cipher suites");
+				"No common cipher suites matching negotiated "
+				"TLS version and our certificate's key type");
 		return;
 	}

@@ -1575,6 +1666,7 @@ static void tls_handle_server_hello(struct l_tls *tls,
 					const uint8_t *buf, size_t len)
 {
 	uint8_t session_id_size, cipher_suite_id[2], compression_method_id;
+	const char *error;
 	int i;
 
 	/* Do we have enough for ProtocolVersion + Random + SessionID len ? */

@@ -1629,6 +1721,14 @@ static void tls_handle_server_hello(struct l_tls *tls,
 		return;
 	}
 
+	if (!tls_cipher_suite_is_compatible(tls, tls->pending.cipher_suite,
+						&error)) {
+		TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+				"Selected cipher suite not compatible: %s",
+				error);
+		return;
+	}
+
 	tls_set_prf_hmac(tls);
 
 	TLS_DEBUG("Negotiated %s", tls->pending.cipher_suite->name);

@@ -2285,6 +2385,9 @@ LIB_EXPORT struct l_tls *l_tls_new(bool server,
 {
 	struct l_tls *tls;
 
+	if (!l_key_is_supported(L_KEY_FEATURE_CRYPTO))
+		return NULL;
+
 	tls = l_new(struct l_tls, 1);
 	tls->server = server;
 	tls->rx = app_data_handler;

@@ -2297,13 +2400,12 @@ LIB_EXPORT struct l_tls *l_tls_new(bool server,
 
 	/* If we're the client, start the handshake right away */
 	if (!tls->server) {
-		if (!tls_init_handshake_hash(tls)) {
+		if (!tls_init_handshake_hash(tls) ||
+				!tls_send_client_hello(tls)) {
 			l_free(tls);
 
 			return NULL;
 		}
-
-		tls_send_client_hello(tls);
 	}
 
 	TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO);

@@ -2490,7 +2592,7 @@ LIB_EXPORT void l_tls_close(struct l_tls *tls)
 	TLS_DISCONNECT(TLS_ALERT_CLOSE_NOTIFY, 0, "Closing session");
 }
 
-LIB_EXPORT void l_tls_set_cacert(struct l_tls *tls, const char *ca_cert_path)
+LIB_EXPORT bool l_tls_set_cacert(struct l_tls *tls, const char *ca_cert_path)
 {
 	TLS_DEBUG("ca-cert-path=%s", ca_cert_path);

@@ -2499,8 +2601,17 @@ LIB_EXPORT void l_tls_set_cacert(struct l_tls *tls, const char *ca_cert_path)
 		tls->ca_cert_path = NULL;
 	}
 
-	if (ca_cert_path)
+	if (ca_cert_path) {
+		if (!l_key_is_supported(L_KEY_FEATURE_RESTRICT)) {
+			TLS_DEBUG("keyctl restrict support missing, "
+					"check kernel configuration");
+			return false;
+		}
+
 		tls->ca_cert_path = l_strdup(ca_cert_path);
+	}
+
+	return true;
 }
 
 LIB_EXPORT bool l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,

ell/tls.h +1 -1

index e1a5a21..133e763 100644
@@ -87,7 +87,7 @@ void l_tls_write(struct l_tls *tls, const uint8_t *data, size_t len);
 void l_tls_handle_rx(struct l_tls *tls, const uint8_t *data, size_t len);
 
 /* If peer is to be authenticated, supply the CA certificate */
-void l_tls_set_cacert(struct l_tls *tls, const char *ca_cert_path);
+bool l_tls_set_cacert(struct l_tls *tls, const char *ca_cert_path);
 
 /*
  * If we are to be authenticated, supply our certificate, private key and

Signature · 1 line

2.19.1

Andrew Zaborowski

author

2018-11-14 13:19 UTC #13 lore raw

PATCH 11/12 examples: Print TLS debug info if TLS_DEBUG set

In reply to Andrew Zaborowski

---
 examples/https-client-test.c | 7 +++++++
 examples/https-server-test.c | 7 +++++++
 2 files changed, 14 insertions(+)

examples/https-client-test.c +7 -0

index 7135346..e0188ee 100644
@@ -113,6 +113,11 @@ static void https_tls_ready(const char *peer_identity, void *user_data)
 	l_tls_write(tls, buf, l);
 }
 
+static void https_tls_debug_cb(const char *str, void *user_data)
+{
+	l_info("%s", str);
+}
+
 int main(int argc, char *argv[])
 {
 	struct hostent *he;

@@ -171,6 +176,8 @@ int main(int argc, char *argv[])
 
 	tls = l_tls_new(false, https_new_data, https_tls_write,
 			https_tls_ready, https_tls_disconnected, NULL);
+	if (getenv("TLS_DEBUG"))
+		l_tls_set_debug(tls, https_tls_debug_cb, NULL, NULL);
 	if (argc > 2)
 		l_tls_set_cacert(tls, argv[2]);
 	if (argc > 5)

examples/https-server-test.c +7 -0

index 2539277..faaf50d 100644
@@ -107,6 +107,11 @@ static void https_tls_ready(const char *peer_identity, void *user_data)
 		printf("Client not authenticated\n");
 }
 
+static void https_tls_debug_cb(const char *str, void *user_data)
+{
+	l_info("%s", str);
+}
+
 int main(int argc, char *argv[])
 {
 	struct sockaddr_in addr;

@@ -162,6 +167,8 @@ int main(int argc, char *argv[])
 
 	tls = l_tls_new(true, https_new_data, https_tls_write,
 			https_tls_ready, https_tls_disconnected, NULL);
+	if (getenv("TLS_DEBUG"))
+		l_tls_set_debug(tls, https_tls_debug_cb, NULL, NULL);
 	auth_ok = l_tls_set_auth_data(tls, argv[1], argv[2], argv[3]);
 	l_tls_set_cacert(tls, argc > 4 ? argv[4] : NULL);

Signature · 1 line

2.19.1

Denis Kenzior

2018-11-14 21:15 UTC #14 lore raw

Re: [PATCH 11/12] examples: Print TLS debug info if TLS_DEBUG set

Quoted 5 lines · Andrew Zaborowski

---
  examples/https-client-test.c | 7 +++++++
  examples/https-server-test.c | 7 +++++++
  2 files changed, 14 insertions(+)

Patch 11 & 12 applied, thanks.

Regards,
-Denis

Andrew Zaborowski

author

2018-11-14 13:19 UTC #15 lore raw

PATCH 12/12 unit: Print TLS debug info if TLS_DEBUG set

In reply to Andrew Zaborowski

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

unit/test-tls.c +10 -0

index d969ee7..005aede 100644
@@ -364,6 +364,11 @@ static void tls_test_disconnected(enum l_tls_alert_desc reason, bool remote,
 	assert(false);
 }
 
+static void tls_debug_cb(const char *str, void *user_data)
+{
+	l_info("%s %s", (const char *) user_data, str);
+}
+
 static void test_tls_test(const void *data)
 {
 	bool auth_ok;

@@ -399,6 +404,11 @@ static void test_tls_test(const void *data)
 	assert(s[0].tls);
 	assert(s[1].tls);
 
+	if (getenv("TLS_SERVER_DEBUG"))
+		l_tls_set_debug(s[0].tls, tls_debug_cb, "server", NULL);
+	if (getenv("TLS_DEBUG"))
+		l_tls_set_debug(s[1].tls, tls_debug_cb, "client", NULL);
+
 	auth_ok = l_tls_set_auth_data(s[0].tls, test->server_cert_path,
 					test->server_key_path,
 					test->server_key_passphrase);

Signature · 1 line

2.19.1

Mat Martineau

2018-11-14 21:13 UTC #16 lore raw

In reply to Andrew Zaborowski

Hi Andrew -

Quoted 94 lines · Andrew Zaborowski

Zero the "counter" part of the IV to avoid sending uninitialized bytes to the kernel (the kernel will zero them again though)
---
ell/cipher.c | 91 +++++++++++++++++++++++++++++++++++-----------------

1 file changed, 61 insertions(+), 30 deletions(-)

ell/cipher.c +7 ~~-26~~

index 79cecdb..af4bc10 100644
@@ -266,28 +266,11 @@ LIB_EXPORT void l_aead_cipher_free(struct l_aead_cipher *cipher)
 	l_free(cipher);
 }
 
-static ssize_t build_iv(const void *nonce, uint8_t nonce_len, uint8_t *iv,
-			uint8_t iv_len)
-{
-	const size_t iv_overhead = 2;
-
-	if (nonce_len + iv_overhead > iv_len)
-		return -EINVAL;
-
-	iv[0] = iv_len - iv_overhead - nonce_len;
-	memcpy(iv + 1, nonce, nonce_len);
-
-	/* Assumes that remaining bytes in iv were already zeroed out */
-
-	return iv_len;
-}
-
 static ssize_t operate_cipher(int sk, __u32 operation,
 				const void *in, size_t in_len,
 				const void *ad, size_t ad_len,
-				const void *nonce, size_t nonce_len,
-				void *out, size_t out_len,
-				size_t iv_len)
+				const void *iv, size_t iv_len,
+				void *out, size_t out_len)
 {
 	char *c_msg_buf;
 	size_t c_msg_size;

@@ -298,7 +281,7 @@ static ssize_t operate_cipher(int sk, __u32 operation,
 
 	c_msg_size = CMSG_SPACE(sizeof(operation));
 	c_msg_size += ad_len ? CMSG_SPACE(sizeof(uint32_t)) : 0;
-	c_msg_size += (nonce && iv_len) ?
+	c_msg_size += iv_len ?
 		CMSG_SPACE(sizeof(struct af_alg_iv) + iv_len) : 0;
 
 	c_msg_buf = alloca(c_msg_size);

@@ -338,7 +321,7 @@ static ssize_t operate_cipher(int sk, __u32 operation,
 		msg.msg_iovlen = 1;
 	}
 
-	if (nonce && iv_len) {
+	if (iv_len) {
 		struct af_alg_iv *algiv;
 
 		c_msg = CMSG_NXTHDR(&msg, c_msg);

@@ -348,9 +331,7 @@ static ssize_t operate_cipher(int sk, __u32 operation,
 
 		algiv = (void *)CMSG_DATA(c_msg);
 		algiv->ivlen = iv_len;
-		result = build_iv(nonce, nonce_len, &algiv->iv[0], iv_len);
-		if (result < 0)
-			return result;
+		memcpy(algiv->iv, iv, iv_len);
 	}
 
 	result = sendmsg(sk, &msg, 0);

@@ -444,7 +425,7 @@ LIB_EXPORT bool l_cipher_encrypt(struct l_cipher *cipher,
 		return false;
 
 	return operate_cipher(cipher->encrypt_sk, ALG_OP_ENCRYPT, in, len,
-				NULL, 0, NULL, 0, out, len, 0) >= 0;
+				NULL, 0, NULL, 0, out, len) >= 0;
 }
 
 LIB_EXPORT bool l_cipher_encryptv(struct l_cipher *cipher,

@@ -471,7 +452,7 @@ LIB_EXPORT bool l_cipher_decrypt(struct l_cipher *cipher,
 		return false;
 
 	return operate_cipher(cipher->decrypt_sk, ALG_OP_DECRYPT, in, len,
-				NULL, 0, NULL, 0, out, len, 0) >= 0;
+				NULL, 0, NULL, 0, out, len) >= 0;
 }
 
 LIB_EXPORT bool l_cipher_decryptv(struct l_cipher *cipher,

ell/cipher.c static size_t l_aead_cipher_get_ivlen(struct l_aead_cipher *cipher) · line 522

Quoted 8 lines

ell/cipher.c +5 -0

@@ -534,21 +515,54 @@ static size_t l_aead_cipher_get_ivlen(struct l_aead_cipher *cipher)
 	return 0;
 }
 
+/* RFC3610 Section 2.3 */
+static ssize_t build_ccm_iv(const void *nonce, uint8_t nonce_len, uint8_t *iv)
+{
+	const size_t iv_overhead = 2;
+	const size_t iv_len = 16;

This depends on the length of the iv array being 16, but all we get from the caller is a uint8_t pointer. How about passing in iv_len for a more robust length check?

Other than that, the cipher patches look good. I also did a quick review of the rest of the series and didn't spot any issues.

Thanks,

Mat

Quoted 91 lines

ell/cipher.c +49 -4

+
+	if (unlikely(nonce_len + iv_overhead > iv_len || nonce_len < 7))
+		return -EINVAL;
+
+	iv[0] = iv_len - iv_overhead - nonce_len;
+	memcpy(iv + 1, nonce, nonce_len);
+	memset(iv + 1 + nonce_len, 0, iv_len - 1 - nonce_len);
+
+	return iv_len;
+}
+
 LIB_EXPORT bool l_aead_cipher_encrypt(struct l_aead_cipher *cipher,
 					const void *in, size_t in_len,
 					const void *ad, size_t ad_len,
 					const void *nonce, size_t nonce_len,
 					void *out, size_t out_len)
 {
+	uint8_t ccm_iv[16];
+	const uint8_t *iv;
+	ssize_t iv_len;
+
 	if (unlikely(!cipher))
 		return false;
 
 	if (unlikely(!in) || unlikely(!out))
 		return false;
 
+	if (cipher->type == L_AEAD_CIPHER_AES_CCM) {
+		iv_len = build_ccm_iv(nonce, nonce_len, ccm_iv);
+		if (unlikely(iv_len < 0))
+			return false;
+
+		iv = ccm_iv;
+	} else {
+		if (unlikely(nonce_len != l_aead_cipher_get_ivlen(cipher)))
+			return false;
+
+		iv = nonce;
+		iv_len = nonce_len;
+	}
+
 	return operate_cipher(cipher->encrypt_sk, ALG_OP_ENCRYPT, in, in_len,
-				ad, ad_len, nonce, nonce_len, out, out_len,
-				l_aead_cipher_get_ivlen(cipher)) ==
+				ad, ad_len, iv, iv_len, out, out_len) ==
 			(ssize_t)out_len;
 }

@@ -558,15 +572,32 @@ LIB_EXPORT bool l_aead_cipher_decrypt(struct l_aead_cipher *cipher,
 					const void *nonce, size_t nonce_len,
 					void *out, size_t out_len)
 {
+	uint8_t ccm_iv[16];
+	const uint8_t *iv;
+	ssize_t iv_len;
+
 	if (unlikely(!cipher))
 		return false;
 
 	if (unlikely(!in) || unlikely(!out))
 		return false;
 
+	if (cipher->type == L_AEAD_CIPHER_AES_CCM) {
+		iv_len = build_ccm_iv(nonce, nonce_len, ccm_iv);
+		if (unlikely(iv_len < 0))
+			return false;
+
+		iv = ccm_iv;
+	} else {
+		if (unlikely(nonce_len != l_aead_cipher_get_ivlen(cipher)))
+			return false;
+
+		iv = nonce;
+		iv_len = nonce_len;
+	}
+
 	return operate_cipher(cipher->decrypt_sk, ALG_OP_DECRYPT, in, in_len,
-				ad, ad_len, nonce, nonce_len, out, out_len,
-				l_aead_cipher_get_ivlen(cipher)) ==
+				ad, ad_len, iv, iv_len, out, out_len) ==
 			(ssize_t)out_len;
 }

Signature · 6 lines

2.19.1

_______________________________________________
ell mailing list
ell(a)lists.01.org
https://lists.01.org/mailman/listinfo/ell

--
Mat Martineau
Intel OTC

Andrew Zaborowski

author

2018-11-15 11:46 UTC #17 lore raw

Hi Mat,

Quoted 10 lines · Mat Martineau

ell/cipher.c static size_t l_aead_cipher_get_ivlen(struct l_aead_cipher *cipher) · line 522
Quoted 5 lines · Andrew Zaborowski
ell/cipher.c +5 -0
+/* RFC3610 Section 2.3 */
+static ssize_t build_ccm_iv(const void *nonce, uint8_t nonce_len, uint8_t *iv)
+{
+     const size_t iv_overhead = 2;
+     const size_t iv_len = 16;
This depends on the length of the iv array being 16, but all we get from the caller is a uint8_t pointer. How about passing in iv_len for a more robust length check?

I think we can check that the provided buffer is big enough but does it make sense to use it for the calculations? Since this is now called build_ccm_iv it can assume the cipher mode is CCM and the buffer needs 16 bytes.

The RFC doesn't suggest there may be different IV sizes:

      Octet Number   Contents
      ------------   ---------
      0              Flags
      1 ... 15-L     Nonce N
      16-L ... 15    Counter i

Quoted 3 lines

Other than that, the cipher patches look good. I also did a quick review of the rest of the series and didn't spot any issues.

Thank you!