All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cipher: add AES-CMAC hashing support
@ 2019-05-29  0:49 Brian Gix
  2019-05-29  1:32 ` Denis Kenzior
  0 siblings, 1 reply; 15+ messages in thread
From: Brian Gix @ 2019-05-29  0:49 UTC (permalink / raw)
  To: ell

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

The algorythm is supported in the kernel, and it is used heavily by
Bluetooth Mesh, and is the one Mesh required algorythm missing from ELL.
---
 ell/cipher.c       | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ell/cipher.h       | 16 ++++++++++
 unit/test-cipher.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+)

diff --git a/ell/cipher.c b/ell/cipher.c
index 7b909df..2f2e482 100644
--- a/ell/cipher.c
+++ b/ell/cipher.c
@@ -90,6 +90,7 @@ struct af_alg_iv {
 
 static uint32_t supported_ciphers;
 static uint32_t supported_aead_ciphers;
+static uint32_t supported_hash_ciphers;
 
 struct l_cipher {
 	int type;
@@ -103,6 +104,11 @@ struct l_aead_cipher {
 	int decrypt_sk;
 };
 
+struct l_hash_cipher {
+	int type;
+	int encrypt_sk;
+};
+
 static int create_alg(const char *alg_type, const char *alg_name,
 			const void *key, size_t key_length, size_t tag_length)
 {
@@ -248,6 +254,42 @@ error_free:
 	return NULL;
 }
 
+static const char *hash_cipher_type_to_name(enum l_hash_cipher_type type)
+{
+	switch (type) {
+	case L_HASH_CIPHER_AES_CMAC:
+		return "cmac(aes)";
+	}
+
+	return NULL;
+}
+
+LIB_EXPORT struct l_hash_cipher *l_hash_cipher_new(enum l_hash_cipher_type type,
+							const void *key,
+							size_t key_length)
+{
+	struct l_hash_cipher *cipher;
+	const char *uninitialized_var(alg_name);
+
+	if (unlikely(!key))
+		return NULL;
+
+	if (type != L_HASH_CIPHER_AES_CMAC)
+		return NULL;
+
+	cipher = l_new(struct l_hash_cipher, 1);
+	cipher->type = type;
+	alg_name = hash_cipher_type_to_name(type);
+
+	cipher->encrypt_sk = create_alg("hash", alg_name, key, key_length,
+					0);
+	if (cipher->encrypt_sk >= 0)
+		return cipher;
+
+	l_free(cipher);
+	return NULL;
+}
+
 LIB_EXPORT void l_cipher_free(struct l_cipher *cipher)
 {
 	if (unlikely(!cipher))
@@ -270,6 +312,16 @@ LIB_EXPORT void l_aead_cipher_free(struct l_aead_cipher *cipher)
 	l_free(cipher);
 }
 
+LIB_EXPORT void l_hash_cipher_free(struct l_hash_cipher *cipher)
+{
+	if (unlikely(!cipher))
+		return;
+
+	close(cipher->encrypt_sk);
+
+	l_free(cipher);
+}
+
 static ssize_t operate_cipher(int sk, __u32 operation,
 				const void *in, size_t in_len,
 				const void *ad, size_t ad_len,
@@ -610,6 +662,20 @@ LIB_EXPORT bool l_aead_cipher_decrypt(struct l_aead_cipher *cipher,
 			(ssize_t)out_len;
 }
 
+LIB_EXPORT bool l_hash_cipher_encrypt(struct l_hash_cipher *cipher,
+					const void *in, size_t in_len,
+					void *out, size_t out_len)
+{
+	if (unlikely(!cipher))
+		return false;
+
+	if (unlikely(!in) || unlikely(!out))
+		return false;
+
+	return operate_cipher(cipher->encrypt_sk, ALG_OP_ENCRYPT, in, in_len,
+				NULL, 0, NULL, 0, out, out_len) >= 0;
+}
+
 static void init_supported()
 {
 	static bool initialized = false;
@@ -617,6 +683,7 @@ static void init_supported()
 	int sk;
 	enum l_cipher_type c;
 	enum l_aead_cipher_type a;
+	enum l_hash_cipher_type h;
 
 	if (likely(initialized))
 		return;
@@ -651,6 +718,17 @@ static void init_supported()
 		supported_aead_ciphers |= 1 << a;
 	}
 
+	strcpy((char *) salg.salg_type, "hash");
+
+	for (h = L_HASH_CIPHER_AES_CMAC; h <= L_HASH_CIPHER_AES_CMAC; h++) {
+		strcpy((char *) salg.salg_name, hash_cipher_type_to_name(h));
+
+		if (bind(sk, (struct sockaddr *) &salg, sizeof(salg)) < 0)
+			continue;
+
+		supported_hash_ciphers |= 1 << h;
+	}
+
 	close(sk);
 }
 
@@ -673,3 +751,13 @@ LIB_EXPORT bool l_aead_cipher_is_supported(enum l_aead_cipher_type type)
 
 	return supported_aead_ciphers & (1 << type);
 }
+
+LIB_EXPORT bool l_hash_cipher_is_supported(enum l_hash_cipher_type type)
+{
+	if (type != L_HASH_CIPHER_AES_CMAC)
+		return false;
+
+	init_supported();
+
+	return supported_hash_ciphers & (1 << type);
+}
diff --git a/ell/cipher.h b/ell/cipher.h
index 84f2988..a18fbf0 100644
--- a/ell/cipher.h
+++ b/ell/cipher.h
@@ -84,8 +84,24 @@ bool l_aead_cipher_decrypt(struct l_aead_cipher *cipher,
 				const void *nonce, size_t nonce_len,
 				void *out, size_t out_len);
 
+struct l_hash_cipher;
+
+enum l_hash_cipher_type {
+	L_HASH_CIPHER_AES_CMAC = 0,
+};
+
+struct l_hash_cipher *l_hash_cipher_new(enum l_hash_cipher_type type,
+					const void *key, size_t key_length);
+
+void l_hash_cipher_free(struct l_hash_cipher *cipher);
+
+bool l_hash_cipher_encrypt(struct l_hash_cipher *cipher,
+					const void *in, size_t len,
+					void *out, size_t out_len);
+
 bool l_cipher_is_supported(enum l_cipher_type type);
 bool l_aead_cipher_is_supported(enum l_aead_cipher_type type);
+bool l_hash_cipher_is_supported(enum l_hash_cipher_type type);
 
 #ifdef __cplusplus
 }
diff --git a/unit/test-cipher.c b/unit/test-cipher.c
index 3bb7602..7d08f39 100644
--- a/unit/test-cipher.c
+++ b/unit/test-cipher.c
@@ -358,6 +358,75 @@ static void test_aead(const void *data)
 	l_free(tag);
 }
 
+struct hash_test_vector {
+	enum l_hash_cipher_type type;
+	char *plaintext;
+	char *key;
+	char *ciphertext;
+};
+
+/* Hash AES_CMAC tests based on Bluetooth Mesh published sample data */
+static const struct hash_test_vector hash_test1 = {
+	.type = L_HASH_CIPHER_AES_CMAC,
+	.plaintext = "74657374",
+	.key = "00000000000000000000000000000000",
+	.ciphertext = "b73cefbd641ef2ea598c2b6efb62f79c",
+};
+
+static const struct hash_test_vector hash_test2 = {
+	.type = L_HASH_CIPHER_AES_CMAC,
+	.plaintext = "f7a2a44f8e8a8029064f173ddc1e2b00",
+	.key = "4f90480c1871bfbffd16971f4d8d10b1",
+	.ciphertext = "2ea6467aa3378c4c545eda62935b9b86",
+};
+
+static void test_hash(const void *data)
+{
+	struct l_hash_cipher *cipher;
+	char *encbuf;
+	size_t encbuflen;
+	char *decbuf;
+	size_t decbuflen;
+	int r;
+	bool success;
+	const struct hash_test_vector *tv = data;
+
+	size_t ptlen;
+	uint8_t *pt = l_util_from_hexstring(tv->plaintext, &ptlen) ?:
+		(uint8_t[]) {};
+	size_t keylen;
+	uint8_t *key = l_util_from_hexstring(tv->key, &keylen);
+	size_t ctlen;
+	uint8_t *ct = l_util_from_hexstring(tv->ciphertext, &ctlen) ?:
+		(uint8_t[]) {};
+
+	encbuflen = ctlen;
+	encbuf = alloca(encbuflen);
+	memset(encbuf, 0, encbuflen);
+	decbuflen = ptlen;
+	decbuf = alloca(decbuflen);
+	memset(decbuf, 0, decbuflen);
+
+	cipher = l_hash_cipher_new(tv->type, key, keylen);
+	assert(cipher);
+
+	success = l_hash_cipher_encrypt(cipher, pt, ptlen, encbuf, encbuflen);
+	assert(success);
+
+	r = memcmp(encbuf, ct, ctlen);
+	assert(!r);
+
+	l_hash_cipher_free(cipher);
+
+	if (ptlen)
+		l_free(pt);
+
+	l_free(key);
+
+	if (ctlen)
+		l_free(ct);
+}
+
 int main(int argc, char *argv[])
 {
 	l_test_init(&argc, &argv);
@@ -388,5 +457,10 @@ int main(int argc, char *argv[])
 		l_test_add("aes_gcm test 6", test_aead, &gcm_test6);
 	}
 
+	if (l_hash_cipher_is_supported(L_HASH_CIPHER_AES_CMAC)) {
+		l_test_add("hash_aes_cmac test 1", test_hash, &hash_test1);
+		l_test_add("hash_aes_cmac test 2", test_hash, &hash_test2);
+	}
+
 	return l_test_run();
 }
-- 
2.14.5


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] cipher: add AES-CMAC hashing support
  2019-05-29  0:49 [PATCH] cipher: add AES-CMAC hashing support Brian Gix
@ 2019-05-29  1:32 ` Denis Kenzior
  2019-05-29  3:04   ` Gix, Brian
  0 siblings, 1 reply; 15+ messages in thread
From: Denis Kenzior @ 2019-05-29  1:32 UTC (permalink / raw)
  To: ell

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

Hi Brian,

On 05/28/2019 07:49 PM, Brian Gix wrote:
> The algorythm is supported in the kernel, and it is used heavily by
> Bluetooth Mesh, and is the one Mesh required algorythm missing from ELL.
> ---
>   ell/cipher.c       | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ell/cipher.h       | 16 ++++++++++
>   unit/test-cipher.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 178 insertions(+)
> 

Please be aware of our patch submission guidelines found in HACKING. 
E.g. the unit test should be in a separate commit.

Have you checked whether l_checksum_new_cmac_aes() satisfies your needs?

Regards,
-Denis

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] cipher: add AES-CMAC hashing support
  2019-05-29  1:32 ` Denis Kenzior
@ 2019-05-29  3:04   ` Gix, Brian
  2019-05-29  3:25     ` Gix, Brian
  0 siblings, 1 reply; 15+ messages in thread
From: Gix, Brian @ 2019-05-29  3:04 UTC (permalink / raw)
  To: ell

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

Hi Denis,

> On May 28, 2019, at 6:32 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> 
> Hi Brian,
> 
>> On 05/28/2019 07:49 PM, Brian Gix wrote:
>> The algorythm is supported in the kernel, and it is used heavily by
>> Bluetooth Mesh, and is the one Mesh required algorythm missing from ELL.
>> ---
>>  ell/cipher.c       | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  ell/cipher.h       | 16 ++++++++++
>>  unit/test-cipher.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 178 insertions(+)
> 
> Please be aware of our patch submission guidelines found in HACKING. E.g. the unit test should be in a separate commit.
> 
> Have you checked whether l_checksum_new_cmac_aes() satisfies your needs?

I didn’t see that. I will check it out, and if it works, I will withdraw my patch altogether. If it does not work, I will resubmit as a patch-set.

> 
> Regards,
> -Denis

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] cipher: add AES-CMAC hashing support
  2019-05-29  3:04   ` Gix, Brian
@ 2019-05-29  3:25     ` Gix, Brian
  2019-05-29 10:52       ` =?unknown-8bit?q?Micha=C5=82?= Lowas-Rzechonek
  0 siblings, 1 reply; 15+ messages in thread
From: Gix, Brian @ 2019-05-29  3:25 UTC (permalink / raw)
  To: ell

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

Hi Denis,

It does work, Thanks.

I will probably do one patch anyway, to add a couple tests to test-checksum.c, because currently a unit test
for l_checksum_new_cmac_aes() is conspicuous in it's absence.


On Wed, 2019-05-29 at 03:04 +0000, Gix, Brian wrote:
> Hi Denis,
> 
> > On May 28, 2019, at 6:32 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> > 
> > Hi Brian,
> > 
> > > On 05/28/2019 07:49 PM, Brian Gix wrote:
> > > The algorythm is supported in the kernel, and it is used heavily by
> > > Bluetooth Mesh, and is the one Mesh required algorythm missing from ELL.
> > > ---
> > >  ell/cipher.c       | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  ell/cipher.h       | 16 ++++++++++
> > >  unit/test-cipher.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 178 insertions(+)
> > 
> > Please be aware of our patch submission guidelines found in HACKING. E.g. the unit test should be in a
> > separate commit.
> > 
> > Have you checked whether l_checksum_new_cmac_aes() satisfies your needs?
> 
> I didn’t see that. I will check it out, and if it works, I will withdraw my patch altogether. If it does not
> work, I will resubmit as a patch-set.
> 
> > 
> > Regards,
> > -Denis
> 
> _______________________________________________
> ell mailing list
> ell(a)lists.01.org
> https://lists.01.org/mailman/listinfo/ell

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] cipher: add AES-CMAC hashing support
  2019-05-29  3:25     ` Gix, Brian
@ 2019-05-29 10:52       ` =?unknown-8bit?q?Micha=C5=82?= Lowas-Rzechonek
  2019-05-29 15:29         ` Gix, Brian
  2019-05-29 16:14         ` Denis Kenzior
  0 siblings, 2 replies; 15+ messages in thread
From: =?unknown-8bit?q?Micha=C5=82?= Lowas-Rzechonek @ 2019-05-29 10:52 UTC (permalink / raw)
  To: ell

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

Hi Brian,

On 05/29, Gix, Brian wrote:
> > > > The algorythm is supported in the kernel, and it is used heavily by
> > > > Bluetooth Mesh, and is the one Mesh required algorythm missing from ELL.

Is this about mesh_crypto_aes_ccm_encrypt and
mesh_crypto_aes_ccm_decrypt functions in mesh/crypto.c?

If so, they can be implemented with l_aead_cipher_* API, without any
changes to ELL, see below.

There is a slight problem, though - this works only on newer kernels
(IIRC, 4.9+), and some vendors (particularly Freescale i.MX6) do not
provide them :(

What's worse, my experiments show what the kernel does *not* report an
error while bind()ing the algo socket, but the results are wrong :(



bool mesh_crypto_aes_ccm_encrypt(const uint8_t nonce[NONCE_LEN],
		const uint8_t key[KEY_LEN],
		const uint8_t *aad, uint16_t aad_len,
		const uint8_t *msg, uint16_t msg_len,
		uint8_t *out_msg,
		void *out_mic, size_t mic_size)
{
	bool result;
	struct l_aead_cipher *cipher = l_aead_cipher_new(L_AEAD_CIPHER_AES_CCM,
			&key[0], KEY_LEN, mic_size);

	if (!cipher)
		return false;

	if (!out_msg)
		return false;

	result = l_aead_cipher_encrypt(cipher, (void *)msg, msg_len,
			aad, aad_len, &nonce[0], NONCE_LEN,
			(void *)out_msg, msg_len + mic_size);

	if (out_mic) {
		switch (mic_size) {
		case 4:
			*(uint32_t *)out_mic = l_get_be32(out_msg + msg_len);
			break;
		case 8:
			*(uint64_t *)out_mic = l_get_be64(out_msg + msg_len);
			break;
		default:
			/* Unsupported MIC size */
			return false;
		}
	}

	l_aead_cipher_free(cipher);
	return result;
}

bool mesh_crypto_aes_ccm_decrypt(const uint8_t nonce[13], const uint8_t key[16],
				const uint8_t *aad, uint16_t aad_len,
				const uint8_t *enc_msg, uint16_t enc_msg_len,
				uint8_t *out_msg,
				void *out_mic, size_t mic_size)
{
	bool result;
	struct l_aead_cipher *cipher = l_aead_cipher_new(L_AEAD_CIPHER_AES_CCM,
			&key[0], KEY_LEN, mic_size);

	if (!cipher)
		return false;

	result = l_aead_cipher_decrypt(cipher, (void *)enc_msg, enc_msg_len,
			aad, aad_len, &nonce[0], NONCE_LEN,
			(void *)out_msg, enc_msg_len - mic_size);

	if (out_mic) {
		switch (mic_size) {
		case 4:
			*(uint32_t *)out_mic = l_get_be32(out_msg +
					enc_msg_len - mic_size);
			break;
		case 8:
			*(uint64_t *)out_mic = l_get_be64(out_msg +
					enc_msg_len - mic_size);
			break;
		default:
			/* Unsupported MIC size */
			return false;
		}
	}

	l_aead_cipher_free(cipher);
	return result;
}

-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] cipher: add AES-CMAC hashing support
  2019-05-29 10:52       ` =?unknown-8bit?q?Micha=C5=82?= Lowas-Rzechonek
@ 2019-05-29 15:29         ` Gix, Brian
  2019-05-29 19:26           ` michal.lowas-rzechonek
  2019-05-29 16:14         ` Denis Kenzior
  1 sibling, 1 reply; 15+ messages in thread
From: Gix, Brian @ 2019-05-29 15:29 UTC (permalink / raw)
  To: ell

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

Hi Michał,

On Wed, 2019-05-29 at 12:52 +0200, Michał Lowas-Rzechonek wrote:
> Hi Brian,
> 
> On 05/29, Gix, Brian wrote:
> > > > > The algorythm is supported in the kernel, and it is used heavily by
> > > > > Bluetooth Mesh, and is the one Mesh required algorythm missing from ELL.
> 
> Is this about mesh_crypto_aes_ccm_encrypt and
> mesh_crypto_aes_ccm_decrypt functions in mesh/crypto.c?

The code you copy pasted below *is* the usage of AES_CCM that we use in mesh, but it is *not* the topic of the
patch I posted here to ELL yesterday. AES-CMAC is the algorythm we use in Mesh to generate all of the various
Keys and IDs from master Net and App keys....  It is a simple one-directional hashing function, and as I also
learned yesterday, is already supported in ELLs checksum.c code.


> 
> If so, they can be implemented with l_aead_cipher_* API, without any
> changes to ELL, see below.
> 
> There is a slight problem, though - this works only on newer kernels
> (IIRC, 4.9+), and some vendors (particularly Freescale i.MX6) do not
> provide them :(


This is definitely a question worth answering. We are aware that pre-4.9 kernels did not correctly support
AES_CCM, so switching the mesh daemon's crypto to pure ELL supported kernel based encryption and decryption
will mean that the daemon is not supported on very old kernels.


As far as Freescale platforms go, I do not know how to answer that question.  At some point we do need to move
on from older platforms so that the rest are not "held back". But I do not know if that time is yet now.

I would really like to get Marcel's opinion on this.


> 
> What's worse, my experiments show what the kernel does *not* report an
> error while bind()ing the algo socket, but the results are wrong :(
> 
> 
> 
> bool mesh_crypto_aes_ccm_encrypt(const uint8_t nonce[NONCE_LEN],
> 		const uint8_t key[KEY_LEN],
> 		const uint8_t *aad, uint16_t aad_len,
> 		const uint8_t *msg, uint16_t msg_len,
> 		uint8_t *out_msg,
> 		void *out_mic, size_t mic_size)
> {
> 	bool result;
> 	struct l_aead_cipher *cipher = l_aead_cipher_new(L_AEAD_CIPHER_AES_CCM,
> 			&key[0], KEY_LEN, mic_size);
> 
> 	if (!cipher)
> 		return false;
> 
> 	if (!out_msg)
> 		return false;
> 
> 	result = l_aead_cipher_encrypt(cipher, (void *)msg, msg_len,
> 			aad, aad_len, &nonce[0], NONCE_LEN,
> 			(void *)out_msg, msg_len + mic_size);
> 
> 	if (out_mic) {
> 		switch (mic_size) {
> 		case 4:
> 			*(uint32_t *)out_mic = l_get_be32(out_msg + msg_len);
> 			break;
> 		case 8:
> 			*(uint64_t *)out_mic = l_get_be64(out_msg + msg_len);
> 			break;
> 		default:
> 			/* Unsupported MIC size */
> 			return false;
> 		}
> 	}
> 
> 	l_aead_cipher_free(cipher);
> 	return result;
> }
> 
> bool mesh_crypto_aes_ccm_decrypt(const uint8_t nonce[13], const uint8_t key[16],
> 				const uint8_t *aad, uint16_t aad_len,
> 				const uint8_t *enc_msg, uint16_t enc_msg_len,
> 				uint8_t *out_msg,
> 				void *out_mic, size_t mic_size)
> {
> 	bool result;
> 	struct l_aead_cipher *cipher = l_aead_cipher_new(L_AEAD_CIPHER_AES_CCM,
> 			&key[0], KEY_LEN, mic_size);
> 
> 	if (!cipher)
> 		return false;
> 
> 	result = l_aead_cipher_decrypt(cipher, (void *)enc_msg, enc_msg_len,
> 			aad, aad_len, &nonce[0], NONCE_LEN,
> 			(void *)out_msg, enc_msg_len - mic_size);
> 
> 	if (out_mic) {
> 		switch (mic_size) {
> 		case 4:
> 			*(uint32_t *)out_mic = l_get_be32(out_msg +
> 					enc_msg_len - mic_size);
> 			break;
> 		case 8:
> 			*(uint64_t *)out_mic = l_get_be64(out_msg +
> 					enc_msg_len - mic_size);
> 			break;
> 		default:
> 			/* Unsupported MIC size */
> 			return false;
> 		}
> 	}
> 
> 	l_aead_cipher_free(cipher);
> 	return result;
> }
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] cipher: add AES-CMAC hashing support
  2019-05-29 10:52       ` =?unknown-8bit?q?Micha=C5=82?= Lowas-Rzechonek
  2019-05-29 15:29         ` Gix, Brian
@ 2019-05-29 16:14         ` Denis Kenzior
  2019-05-31 17:28           ` ELL building broken for 32bit systems Gix, Brian
  1 sibling, 1 reply; 15+ messages in thread
From: Denis Kenzior @ 2019-05-29 16:14 UTC (permalink / raw)
  To: ell

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

Hi Michał,

> Is this about mesh_crypto_aes_ccm_encrypt and
> mesh_crypto_aes_ccm_decrypt functions in mesh/crypto.c?

No, this is different.  But *yikes*.  BlueZ implements AEAD cipher in 
terms of ecb(aes)?  That is highly inefficient due to syscall overhead.

> 
> If so, they can be implemented with l_aead_cipher_* API, without any
> changes to ELL, see below.

That would be much better, yes.

> 
> There is a slight problem, though - this works only on newer kernels
> (IIRC, 4.9+), and some vendors (particularly Freescale i.MX6) do not
> provide them :(
> 
> What's worse, my experiments show what the kernel does *not* report an
> error while bind()ing the algo socket, but the results are wrong :(

I think there was a bug in AEAD ciphers, the reported length from one of 
the operations was incorrect.  I believe Mat fixed this and ell deals 
with this correctly.  See 08be47564b3c49404e5cb6061c0ede94b0a9e783.

But still, 4.9 is a freaking old kernel.  Worst case have the vendor 
port the fixed aead implementation into their tree.

If this isn't available, you can always hack in a fallback in bluez to 
keep the same ecb(aes) behavior, but that's just painful :)

Regards,
-Denis

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] cipher: add AES-CMAC hashing support
  2019-05-29 15:29         ` Gix, Brian
@ 2019-05-29 19:26           ` michal.lowas-rzechonek
  2019-05-29 20:07             ` Gix, Brian
  0 siblings, 1 reply; 15+ messages in thread
From: michal.lowas-rzechonek @ 2019-05-29 19:26 UTC (permalink / raw)
  To: ell

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

Hi Brian, Denis,

On 05/29, Gix, Brian wrote:
> > Is this about mesh_crypto_aes_ccm_encrypt and
> > mesh_crypto_aes_ccm_decrypt functions in mesh/crypto.c?
> 
> The code you copy pasted below *is* the usage of AES_CCM that we use in mesh,

Um, it's not - mesh_crypto_aes_ccm_* functions seem to implement AES_CCM
on top of ECB, in user space. As Denis mentioned, this is somewhat
inefficient, but at least it works on older kernels.

I was thinking about submitting a patch to change that, but then I'd be
shooting myself in the foot, because my target platform runs kernel 4.4
if I recall correctly ;)

> but it is *not* the topic of the patch I posted here to ELL yesterday.
> AES-CMAC is the algorithm we use in Mesh to generate all of the
> various Keys and IDs from master Net and App keys....

Ah, right, it's the K* family of functions. Sorry, I confused CCM and
CMAC modes.

regards
-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] cipher: add AES-CMAC hashing support
  2019-05-29 19:26           ` michal.lowas-rzechonek
@ 2019-05-29 20:07             ` Gix, Brian
  0 siblings, 0 replies; 15+ messages in thread
From: Gix, Brian @ 2019-05-29 20:07 UTC (permalink / raw)
  To: ell

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

Hi Michal,


On Wed, 2019-05-29 at 21:26 +0200, michal.lowas-rzechonek(a)silvair.com wrote:
> Hi Brian, Denis,
> 
> On 05/29, Gix, Brian wrote:
> > > Is this about mesh_crypto_aes_ccm_encrypt and
> > > mesh_crypto_aes_ccm_decrypt functions in mesh/crypto.c?
> > 
> > The code you copy pasted below *is* the usage of AES_CCM that we use in mesh,
> 
> Um, it's not - mesh_crypto_aes_ccm_* functions seem to implement AES_CCM
> on top of ECB, in user space. As Denis mentioned, this is somewhat
> inefficient, but at least it works on older kernels.

Yes, I understand now...  You wrote the code snippet, that looked an awful lot like the patch I was in the
middle of writing for mesh_crypto_aes_ccm_*....

my best guess right now is that we will probably decide to go ahead and patch mesh such that it assumes an up-
to-date kernel version, and a working underlying architecture.

It will then be the responsibility of the vendor (you guys) to patch mesh such that it continues to work on
your older kernels and perhaps architectures.  That way future kernels and platforms are not perpetually "held
back" to support kernels that should be obsoleted or fixed. 

> 
> I was thinking about submitting a patch to change that, but then I'd be
> shooting myself in the foot, because my target platform runs kernel 4.4
> if I recall correctly ;)
> 
> > but it is *not* the topic of the patch I posted here to ELL yesterday.
> > AES-CMAC is the algorithm we use in Mesh to generate all of the
> > various Keys and IDs from master Net and App keys....
> 
> Ah, right, it's the K* family of functions. Sorry, I confused CCM and
> CMAC modes.
> 
> regards

Best Reagrds,
Brian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* ELL building broken for 32bit systems
  2019-05-29 16:14         ` Denis Kenzior
@ 2019-05-31 17:28           ` Gix, Brian
  2019-05-31 17:41             ` Gix, Brian
  2019-05-31 19:53             ` Denis Kenzior
  0 siblings, 2 replies; 15+ messages in thread
From: Gix, Brian @ 2019-05-31 17:28 UTC (permalink / raw)
  To: ell

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


As part of my Bluetooth Mesh change-over from user-space crypto to ELL
provided cipher and checksum routines,  I have been building on all the
platforms I have available to me, including Fedora-30 running on 32bit
i686 architecture.

Here are the issues I encountered:

There is an unchecked build dependancy on "xxd", which appears to be
provided by the vim-common package.  This probably affects all
platforms, but my 32-bit F30 installation is deliberately kept
"minimal", so that is where it showed up.


After installing vim-common, there is a build error that is unique to
the 32bit F30 (it works fine on 64 bit systems).  When doing a full
build (and when doing a build of *only* unit/test-cipher) I get the
following error when building ell/genl.c:

[ell]$ make -k unit/test-cipher
  CC       ell/genl.lo
In file included from /usr/include/string.h:494,
                 from ell/util.h:26,
                 from ell/genl.c:32:
In function ‘strncpy’,
    inlined from ‘family_add_mcast’ at ell/genl.c:240:2,
    inlined from ‘family_add_mcast.isra.0’ at ell/genl.c:224:13:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’
specified bound 16 equals destination size [-Werror=stringop-
truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
(__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~
In function ‘strncpy’,
    inlined from ‘get_family_callback’ at ell/genl.c:1049:4:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’
specified bound 16 equals destination size [-Werror=stringop-
truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
(__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~
In function ‘strncpy’,
    inlined from ‘family_alloc’ at ell/genl.c:164:2:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’
specified bound 16 equals destination size [-Werror=stringop-
truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
(__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile:1780: ell/genl.lo] Error 1
make: Target 'unit/test-cipher' not remade because of errors.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3250 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: ELL building broken for 32bit systems
  2019-05-31 17:28           ` ELL building broken for 32bit systems Gix, Brian
@ 2019-05-31 17:41             ` Gix, Brian
  2019-05-31 19:53             ` Denis Kenzior
  1 sibling, 0 replies; 15+ messages in thread
From: Gix, Brian @ 2019-05-31 17:41 UTC (permalink / raw)
  To: ell

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

On Fri, 2019-05-31 at 17:28 +0000, Gix, Brian wrote:
> As part of my Bluetooth Mesh change-over from user-space crypto to
> ELL
> provided cipher and checksum routines,  I have been building on all
> the
> platforms I have available to me, including Fedora-30 running on
> 32bit
> i686 architecture.
> 
> Here are the issues I encountered:
> 
> There is an unchecked build dependancy on "xxd", which appears to be
> provided by the vim-common package.  This probably affects all
> platforms, but my 32-bit F30 installation is deliberately kept
> "minimal", so that is where it showed up.


Actually for this one, xxd is checked for, but it doesn't cause a
failure of ./bootstrap-configure, even though it correctly determines
that xxd is missing.

> 
> 
> After installing vim-common, there is a build error that is unique to
> the 32bit F30 (it works fine on 64 bit systems).  When doing a full
> build (and when doing a build of *only* unit/test-cipher) I get the
> following error when building ell/genl.c:
> 
> [ell]$ make -k unit/test-cipher
>   CC       ell/genl.lo
> In file included from /usr/include/string.h:494,
>                  from ell/util.h:26,
>                  from ell/genl.c:32:
> In function ‘strncpy’,
>     inlined from ‘family_add_mcast’ at ell/genl.c:240:2,
>     inlined from ‘family_add_mcast.isra.0’ at ell/genl.c:224:13:
> /usr/include/bits/string_fortified.h:106:10: error:
> ‘__builtin_strncpy’
> specified bound 16 equals destination size [-Werror=stringop-
> truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
> (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~
> ~~~~~~~~
> In function ‘strncpy’,
>     inlined from ‘get_family_callback’ at ell/genl.c:1049:4:
> /usr/include/bits/string_fortified.h:106:10: error:
> ‘__builtin_strncpy’
> specified bound 16 equals destination size [-Werror=stringop-
> truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
> (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~
> ~~~~~~~~
> In function ‘strncpy’,
>     inlined from ‘family_alloc’ at ell/genl.c:164:2:
> /usr/include/bits/string_fortified.h:106:10: error:
> ‘__builtin_strncpy’
> specified bound 16 equals destination size [-Werror=stringop-
> truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
> (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~
> ~~~~~~~~
> cc1: all warnings being treated as errors
> make: *** [Makefile:1780: ell/genl.lo] Error 1
> make: Target 'unit/test-cipher' not remade because of errors.
> _______________________________________________
> ell mailing list
> ell(a)lists.01.org
> https://lists.01.org/mailman/listinfo/ell

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3250 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: ELL building broken for 32bit systems
  2019-05-31 17:28           ` ELL building broken for 32bit systems Gix, Brian
  2019-05-31 17:41             ` Gix, Brian
@ 2019-05-31 19:53             ` Denis Kenzior
  2019-05-31 20:28               ` Gix, Brian
  1 sibling, 1 reply; 15+ messages in thread
From: Denis Kenzior @ 2019-05-31 19:53 UTC (permalink / raw)
  To: ell

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

Hi Brian,

> build (and when doing a build of *only* unit/test-cipher) I get the
> following error when building ell/genl.c:
> 

Uhh, no idea why only test-cipher, but okay...

> [ell]$ make -k unit/test-cipher
>    CC       ell/genl.lo
> In file included from /usr/include/string.h:494,
>                   from ell/util.h:26,
>                   from ell/genl.c:32:
> In function ‘strncpy’,
>      inlined from ‘family_add_mcast’ at ell/genl.c:240:2,
>      inlined from ‘family_add_mcast.isra.0’ at ell/genl.c:224:13:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’
> specified bound 16 equals destination size [-Werror=stringop-
> truncation]
>    106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
> (__dest));
>        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~
> In function ‘strncpy’,
>      inlined from ‘get_family_callback’ at ell/genl.c:1049:4:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’
> specified bound 16 equals destination size [-Werror=stringop-
> truncation]
>    106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
> (__dest));
>        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~
> In function ‘strncpy’,
>      inlined from ‘family_alloc’ at ell/genl.c:164:2:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’
> specified bound 16 equals destination size [-Werror=stringop-
> truncation]
>    106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
> (__dest));
>        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~

What version are you using?  Upstream ell doesn't even use strncpy in 
genl.c anymore.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: ELL building broken for 32bit systems
  2019-05-31 19:53             ` Denis Kenzior
@ 2019-05-31 20:28               ` Gix, Brian
  2019-05-31 20:44                 ` Denis Kenzior
  0 siblings, 1 reply; 15+ messages in thread
From: Gix, Brian @ 2019-05-31 20:28 UTC (permalink / raw)
  To: ell

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

Hi Denis,

> 
> Hi Brian,
> 
> > build (and when doing a build of *only* unit/test-cipher) I get the
> > following error when building ell/genl.c:
> >
> 
> Uhh, no idea why only test-cipher, but okay...

Sorry... I was trying to short-cut my own needs to be able to recommend the running of test-cipher in the README file of Bluetooth mesh to determine if the known "old kernel" AEAD problem existed...  But yes, this is an ELL wide problem.

> 
> > [ell]$ make -k unit/test-cipher
> >    CC       ell/genl.lo
> > In file included from /usr/include/string.h:494,
> >                   from ell/util.h:26,
> >                   from ell/genl.c:32:
> > In function ‘strncpy’,
> >      inlined from ‘family_add_mcast’ at ell/genl.c:240:2,
> >      inlined from ‘family_add_mcast.isra.0’ at ell/genl.c:224:13:
> > /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’
> > specified bound 16 equals destination size [-Werror=stringop-
> > truncation]
> >    106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
> > (__dest));
> >        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ~~~~~~~~
> > In function ‘strncpy’,
> >      inlined from ‘get_family_callback’ at ell/genl.c:1049:4:
> > /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’
> > specified bound 16 equals destination size [-Werror=stringop-
> > truncation]
> >    106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
> > (__dest));
> >        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ~~~~~~~~
> > In function ‘strncpy’,
> >      inlined from ‘family_alloc’ at ell/genl.c:164:2:
> > /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’
> > specified bound 16 equals destination size [-Werror=stringop-
> > truncation]
> >    106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos
> > (__dest));
> >        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ~~~~~~~~
> 
> What version are you using?  Upstream ell doesn't even use strncpy in genl.c
> anymore.

This is unmodified Fedora-30 for i686 32-bit.  I tried chasing down the nesting rabbit-hole to find exactly where the strncpy's were coming from, and came up empty... I can only assume that some inlined function in string.h resolves to some version of strncpy on 32-bit fedora...   perhaps relating to the strlcpy calls.

For what it's worth, this does not appear to be *new*  per se...  Checking out the first labeled ELL version from Jan-2019 causes the same errors.

I do know that F30 introduced a number of alignment warnings (which are thrown as errors) that did not exist in F29.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: ELL building broken for 32bit systems
  2019-05-31 20:28               ` Gix, Brian
@ 2019-05-31 20:44                 ` Denis Kenzior
  2019-05-31 23:12                   ` Gix, Brian
  0 siblings, 1 reply; 15+ messages in thread
From: Denis Kenzior @ 2019-05-31 20:44 UTC (permalink / raw)
  To: ell

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

Hi Brian,

>> What version are you using?  Upstream ell doesn't even use strncpy in genl.c
>> anymore.
> 
> This is unmodified Fedora-30 for i686 32-bit.  I tried chasing down the nesting rabbit-hole to find exactly where the strncpy's were coming from, and came up empty... I can only assume that some inlined function in string.h resolves to some version of strncpy on 32-bit fedora...   perhaps relating to the strlcpy calls.

No, I meant what version of ell?  The issue comes from gcc9 which is 
much more strict about checking strncpy use than before.  The problem is 
that we used to use something like:

#define FOO_BUFFER_SIZE
char buf[FOO_BUFFER_SIZE];

strncpy(buf, some_string, sizeof(buf));

Which would leave the buffer non-null terminated.  Not really a concern 
since all the strings came from the kernel and the chances of things 
ending up badly were small, but still...

The fix is to either use sizeof(buf) - 1 and null-terminate the string, 
or just use l_strlcpy.

Upstream ell is down to about 4 occurrences of strncpy, none of them in 
genl.  So if you are having this problem with some previous release of 
ell, then you should simply disable this warning.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: ELL building broken for 32bit systems
  2019-05-31 20:44                 ` Denis Kenzior
@ 2019-05-31 23:12                   ` Gix, Brian
  0 siblings, 0 replies; 15+ messages in thread
From: Gix, Brian @ 2019-05-31 23:12 UTC (permalink / raw)
  To: ell

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

Hi Denis,

On Fri, 2019-05-31 at 15:44 -0500, Denis Kenzior wrote:
> No, I meant what version of ell?  The issue comes from gcc9 which
> > > is 
> much more strict about checking strncpy use than before.  The problem
> is 
> that we used to use something like:
> 
> 


Damnit, that is my bad.  My VM version of the git repo was wedged,
and stuck on an early Apirl-2019 version of ELL. So, I am no longer
getting the build warning=error for genl.c as of the (now) tip:

> commit 21306874a2318eb17cdd23ec1cb0ed1a193d5971 (HEAD -> master,
origin/master, origin/HEAD)
> Author: Tim Kourt <tim.a.kourt@linux.intel.com>
> Date:   Thu May 30 13:20:48 2019 -0700

There *are* now three unit test assertions happening which I didn't
notice before, although they are in an area that doesn't affect me
personally.

These failures happen on both 64 and 32 bit Fedora 30:

[ell]$ unit/test-pem
TEST: pem/invalid header/test 1
TEST: pem/invalid header/test 2
TEST: pem/empty
TEST: pem/empty label
TEST: pem/v1 MD5AndDES encrypted Private Key
test-pem: unit/test-pem.c:111: test_encrypted_pkey: Assertion `pkey1'
failed.
Aborted (core dumped)
[ell]$ unit/test-tls
Running sysctl kernel.keys.maxkeys=2000 is recommended
TEST: TLS 1.0 PRF
TEST: TLS 1.2 PRF with SHA256
TEST: TLS 1.2 PRF with SHA384
TEST: TLS 1.2 PRF with SHA512
TEST: Certificate chains
TEST: TLS connection no auth
test-tls: unit/test-tls.c:550: test_tls_with_ver: Assertion `auth_ok'
failed.
Aborted (core dumped)
[ell]$ unit/test-key
TEST: unsupported
TEST: user key
TEST: Diffie-Hellman 1
TEST: Diffie-Hellman 2
TEST: Diffie-Hellman 3
TEST: simple keyring
TEST: trusted keyring
TEST: trust chain
TEST: key crypto
test-key: unit/test-key.c:572: test_key_crypto: Assertion `privkey'
failed.
Aborted (core dumped)


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3250 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-05-31 23:12 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29  0:49 [PATCH] cipher: add AES-CMAC hashing support Brian Gix
2019-05-29  1:32 ` Denis Kenzior
2019-05-29  3:04   ` Gix, Brian
2019-05-29  3:25     ` Gix, Brian
2019-05-29 10:52       ` =?unknown-8bit?q?Micha=C5=82?= Lowas-Rzechonek
2019-05-29 15:29         ` Gix, Brian
2019-05-29 19:26           ` michal.lowas-rzechonek
2019-05-29 20:07             ` Gix, Brian
2019-05-29 16:14         ` Denis Kenzior
2019-05-31 17:28           ` ELL building broken for 32bit systems Gix, Brian
2019-05-31 17:41             ` Gix, Brian
2019-05-31 19:53             ` Denis Kenzior
2019-05-31 20:28               ` Gix, Brian
2019-05-31 20:44                 ` Denis Kenzior
2019-05-31 23:12                   ` Gix, Brian

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.