Skip to contentSkip to conversation

crypto: gcmaes - Provide minimal library implementation

PATCH In review Ard Biesheuvel 2 participants 3 messages Oct 14, 2022 – Oct 18, 2022

Subsystems (3)
x86 Crypto API Crypto library
lore
Share & tools
lore

Ard Biesheuvel

author maintainer
#0 lore raw

PATCH v2 1/1 crypto: gcmaes - Provide minimal library implementation

Implement a minimal library version of GCM based on the existing library implementations of AES and multiplication in GF(2^128). Using these primitives, GCM can be implemented in a straight-forward manner.

GCM has a couple of sharp edges, i.e., the amount of input data processed with the same initialization vector (IV) should be capped to protect the counter from 32-bit rollover (or carry), and the size of the authentication tag should be fixed for a given key. [0]

The former concern is addressed trivially, given that the function call API uses 32-bit signed types for the input lengths. It is still up to the caller to avoid IV reuse in general, but this is not something we can police at the implementation level.

As for the latter concern, let's make the authentication tag size part of the key schedule, and only permit it to be configured as part of the key expansion routine.

Note that table based AES implementations are susceptible to known plaintext timing attacks on the encryption key. The AES library already attempts to mitigate this to some extent, but given that the counter mode encryption used by GCM operates exclusively on known plaintext by construction (the IV and therefore the initial counter value are known to an attacker), let's take some extra care to mitigate this, by calling the AES library with interrupts disabled.

[0] https://nvlpubs.nist.gov/nistpubs/legacy/sp/nistspecialpublication800-38d.pdf

Link https://lore.kernel.org/all/[email protected]/ open here
Signed-off-by Ard Biesheuvel <[email protected]>
Tested-by Nikunj A Dadhania <[email protected]>

---
v2: - rename gcm to gcmaes to reflect that GCM is also used in
combination with other symmetric ciphers (Jason)
- add Nikunj's Tested-by

 arch/x86/crypto/aesni-intel_glue.c |  24 +-
 include/crypto/gcm.h               |  66 ++
 lib/crypto/Kconfig                 |   6 +
 lib/crypto/Makefile                |   3 +
 lib/crypto/gcmaes.c                | 720 ++++++++++++++++++++
 5 files changed, 807 insertions(+), 12 deletions(-)
arch/x86/crypto/aesni-intel_glue.c +12 -12
index a5b0cb3efeba..0733f218783e 100644
@@ -778,8 +778,8 @@ static int gcmaes_crypt_by_sg(bool enc, struct aead_request *req,
 	return 0;
 }
 
-static int gcmaes_encrypt(struct aead_request *req, unsigned int assoclen,
-			  u8 *hash_subkey, u8 *iv, void *aes_ctx)
+static int gcmaesni_encrypt(struct aead_request *req, unsigned int assoclen,
+			    u8 *hash_subkey, u8 *iv, void *aes_ctx)
 {
 	struct crypto_aead *tfm = crypto_aead_reqtfm(req);
 	unsigned long auth_tag_len = crypto_aead_authsize(tfm);

@@ -797,8 +797,8 @@ static int gcmaes_encrypt(struct aead_request *req, unsigned int assoclen,
 	return 0;
 }
 
-static int gcmaes_decrypt(struct aead_request *req, unsigned int assoclen,
-			  u8 *hash_subkey, u8 *iv, void *aes_ctx)
+static int gcmaesni_decrypt(struct aead_request *req, unsigned int assoclen,
+			    u8 *hash_subkey, u8 *iv, void *aes_ctx)
 {
 	struct crypto_aead *tfm = crypto_aead_reqtfm(req);
 	unsigned long auth_tag_len = crypto_aead_authsize(tfm);

@@ -847,8 +847,8 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
 		*(iv+4+i) = req->iv[i];
 	*((__be32 *)(iv+12)) = counter;
 
-	return gcmaes_encrypt(req, req->assoclen - 8, ctx->hash_subkey, iv,
-			      aes_ctx);
+	return gcmaesni_encrypt(req, req->assoclen - 8, ctx->hash_subkey, iv,
+				aes_ctx);
 }
 
 static int helper_rfc4106_decrypt(struct aead_request *req)

@@ -875,8 +875,8 @@ static int helper_rfc4106_decrypt(struct aead_request *req)
 		*(iv+4+i) = req->iv[i];
 	*((__be32 *)(iv+12)) = counter;
 
-	return gcmaes_decrypt(req, req->assoclen - 8, ctx->hash_subkey, iv,
-			      aes_ctx);
+	return gcmaesni_decrypt(req, req->assoclen - 8, ctx->hash_subkey, iv,
+				aes_ctx);
 }
 #endif
 

@@ -1167,8 +1167,8 @@ static int generic_gcmaes_encrypt(struct aead_request *req)
 	memcpy(iv, req->iv, 12);
 	*((__be32 *)(iv+12)) = counter;
 
-	return gcmaes_encrypt(req, req->assoclen, ctx->hash_subkey, iv,
-			      aes_ctx);
+	return gcmaesni_encrypt(req, req->assoclen, ctx->hash_subkey, iv,
+				aes_ctx);
 }
 
 static int generic_gcmaes_decrypt(struct aead_request *req)

@@ -1183,8 +1183,8 @@ static int generic_gcmaes_decrypt(struct aead_request *req)
 	memcpy(iv, req->iv, 12);
 	*((__be32 *)(iv+12)) = counter;
 
-	return gcmaes_decrypt(req, req->assoclen, ctx->hash_subkey, iv,
-			      aes_ctx);
+	return gcmaesni_decrypt(req, req->assoclen, ctx->hash_subkey, iv,
+				aes_ctx);
 }
 
 static struct aead_alg aesni_aeads[] = { {
include/crypto/gcm.h +66 -0

76 lines not on this page · raw message

lib/crypto/Kconfig +6 -0
index 7e9683e9f5c6..974721d4b081 100644
@@ -8,6 +8,12 @@ config CRYPTO_LIB_UTILS
 config CRYPTO_LIB_AES
 	tristate
 
+config CRYPTO_LIB_GCMAES
+	tristate
+	select CRYPTO_GF128MUL
+	select CRYPTO_LIB_AES
+	select CRYPTO_LIB_UTILS
+
 config CRYPTO_LIB_ARC4
 	tristate
lib/crypto/Makefile +3 -0
index c852f067ab06..f1b0b2d2d596 100644
@@ -10,6 +10,9 @@ obj-$(CONFIG_CRYPTO_LIB_CHACHA_GENERIC)		+= libchacha.o
 obj-$(CONFIG_CRYPTO_LIB_AES)			+= libaes.o
 libaes-y					:= aes.o
 
+obj-$(CONFIG_CRYPTO_LIB_GCMAES)			+= libgcmaes.o
+libgcmaes-y					:= gcmaes.o
+
 obj-$(CONFIG_CRYPTO_LIB_ARC4)			+= libarc4.o
 libarc4-y					:= arc4.o
lib/crypto/gcmaes.c new file +720 -0

720 lines not on this page · raw message

Signature · 1 line
2.35.1
maintainer
#1 lore raw
Quoted 7 lines · Ard Biesheuvel

Note that table based AES implementations are susceptible to known plaintext timing attacks on the encryption key. The AES library already attempts to mitigate this to some extent, but given that the counter mode encryption used by GCM operates exclusively on known plaintext by construction (the IV and therefore the initial counter value are known to an attacker), let's take some extra care to mitigate this, by calling the AES library with interrupts disabled.

Note that crypto/gf128mul.c has no mitigations against timing attacks. I take it that is something that needs to be tolerated here?

Quoted 29 lines
include/crypto/gcm.h +14 -0
index 9d7eff04f224..dfbc381df5ae 100644
@@ -3,6 +3,9 @@
 
 #include <linux/errno.h>
 
+#include <crypto/aes.h>
+#include <crypto/gf128mul.h>
+
 #define GCM_AES_IV_SIZE 12
 #define GCM_RFC4106_IV_SIZE 8
 #define GCM_RFC4543_IV_SIZE 8

@@ -60,4 +63,67 @@ static inline int crypto_ipsec_check_assoclen(unsigned int assoclen)
 
 	return 0;
 }
+
+struct gcmaes_ctx {
+	be128			ghash_key;
+	struct crypto_aes_ctx	aes_ctx;
+	unsigned int		authsize;
+};
+
+/**
+ * gcmaes_expandkey - Expands the AES and GHASH keys for the GCM-AES key
+ * 		      schedule
+ *

include/crypto/gcm.h static inline int crypto_ipsec_check_assoclen(unsigned int assoclen) · line 86

Quoted 10 lines
include/crypto/gcm.h +10 -0
+ * @ctx:	The data structure that will hold the GCM-AES key schedule
+ * @key:	The AES encryption input key
+ * @keysize:	The length in bytes of the input key
+ * @authsize:	The size in bytes of the GCM authentication tag
+ *
+ * Returns 0 on success, or -EINVAL if @keysize or @authsize contain values
+ * that are not permitted by the GCM specification.
+ */
+int gcmaes_expandkey(struct gcmaes_ctx *ctx, const u8 *key,
+		     unsigned int keysize, unsigned int authsize);

These comments are duplicated in the .c file too. They should be in just one place, probably the .c file since that approach is more common in the kernel.

Also, this seems to be intended to be a kerneldoc comment, but the return value isn't documented in the correct format. It needs to be "Return:". Try this:

$ ./scripts/kernel-doc -v -none lib/crypto/gcmaes.c
lib/crypto/gcmaes.c:35: info: Scanning doc for function gcmaes_expandkey
lib/crypto/gcmaes.c:48: warning: No description found for return value of 'gcmaes_expandkey'
lib/crypto/gcmaes.c:114: info: Scanning doc for function gcmaes_encrypt
lib/crypto/gcmaes.c:142: info: Scanning doc for function gcmaes_decrypt
lib/crypto/gcmaes.c:162: warning: No description found for return value of 'gcmaes_decrypt'

include/crypto/gcm.h static inline int crypto_ipsec_check_assoclen(unsigned int assoclen) · line 91

Quoted 5 lines
include/crypto/gcm.h +5 -0
+config CRYPTO_LIB_GCMAES
+	tristate
+	select CRYPTO_GF128MUL
+	select CRYPTO_LIB_AES
+	select CRYPTO_LIB_UTILS

Doesn't this mean that crypto/gf128mul.c needs to be moved into lib/crypto/?

- Eric

author maintainer
#2 lore raw
Quoted 10 lines · Eric Biggers
Quoted 7 lines · Ard Biesheuvel

Note that table based AES implementations are susceptible to known plaintext timing attacks on the encryption key. The AES library already attempts to mitigate this to some extent, but given that the counter mode encryption used by GCM operates exclusively on known plaintext by construction (the IV and therefore the initial counter value are known to an attacker), let's take some extra care to mitigate this, by calling the AES library with interrupts disabled.

Quoted 3 lines

Note that crypto/gf128mul.c has no mitigations against timing attacks. I take it that is something that needs to be tolerated here?

Ah good point - I misremembered and thought that the 'slow' version without the expanded key uses not table lookups at all.

I can add a patch to address this, it doesn't seem all that difficult to remove the table lookups and data or key dependent conditionals.

Quoted 34 lines
Quoted 34 lines
include/crypto/gcm.h +19 -0
index 9d7eff04f224..dfbc381df5ae 100644
@@ -3,6 +3,9 @@
 
 #include <linux/errno.h>
 
+#include <crypto/aes.h>
+#include <crypto/gf128mul.h>
+
 #define GCM_AES_IV_SIZE 12
 #define GCM_RFC4106_IV_SIZE 8
 #define GCM_RFC4543_IV_SIZE 8

@@ -60,4 +63,67 @@ static inline int crypto_ipsec_check_assoclen(unsigned int assoclen)
 
      return 0;
 }
+
+struct gcmaes_ctx {
+     be128                   ghash_key;
+     struct crypto_aes_ctx   aes_ctx;
+     unsigned int            authsize;
+};
+
+/**
+ * gcmaes_expandkey - Expands the AES and GHASH keys for the GCM-AES key
+ *                 schedule
+ *
+ * @ctx:     The data structure that will hold the GCM-AES key schedule
+ * @key:     The AES encryption input key
+ * @keysize: The length in bytes of the input key
+ * @authsize:        The size in bytes of the GCM authentication tag
+ *
Quoted 9 lines

include/crypto/gcm.h static inline int crypto_ipsec_check_assoclen(unsigned int assoclen) · line 86

Quoted 5 lines
include/crypto/gcm.h +5 -0
+ * Returns 0 on success, or -EINVAL if @keysize or @authsize contain values
+ * that are not permitted by the GCM specification.
+ */
+int gcmaes_expandkey(struct gcmaes_ctx *ctx, const u8 *key,
+                  unsigned int keysize, unsigned int authsize);

These comments are duplicated in the .c file too. They should be in just one place, probably the .c file since that approach is more common in the kernel.

OK

Quoted 10 lines

Also, this seems to be intended to be a kerneldoc comment, but the return value isn't documented in the correct format. It needs to be "Return:". Try this:

$ ./scripts/kernel-doc -v -none lib/crypto/gcmaes.c
lib/crypto/gcmaes.c:35: info: Scanning doc for function gcmaes_expandkey
lib/crypto/gcmaes.c:48: warning: No description found for return value of 'gcmaes_expandkey'
lib/crypto/gcmaes.c:114: info: Scanning doc for function gcmaes_encrypt
lib/crypto/gcmaes.c:142: info: Scanning doc for function gcmaes_decrypt
lib/crypto/gcmaes.c:162: warning: No description found for return value of 'gcmaes_decrypt'

OK, will fix.

Quoted 8 lines

include/crypto/gcm.h static inline int crypto_ipsec_check_assoclen(unsigned int assoclen) · line 91

Quoted 5 lines
include/crypto/gcm.h +5 -0
+config CRYPTO_LIB_GCMAES
+     tristate
+     select CRYPTO_GF128MUL
+     select CRYPTO_LIB_AES
+     select CRYPTO_LIB_UTILS

Doesn't this mean that crypto/gf128mul.c needs to be moved into lib/crypto/?

Probably, I'll address that as well in v3.

Thanks,
Ard.