[PATCH v2 0/2] crypto: validate inputs for gcm and aes

[PATCH v2 0/2] crypto: validate inputs for gcm and aes

[Iuliana Prodan]

Added inline helper functions to check authsize and assoclen for
gcm, rfc4106 and rfc4543.  
Added, also, inline helper function to check key length for AES algorithms.
These are used in the generic implementation of gcm/rfc4106/rfc4543
and aes.

Changes since v1:
- rename helper functions with crypto_ prefix;
- update aes after it was moved to lib/crypto.

Iuliana Prodan (2):
  crypto: gcm - helper functions for assoclen/authsize check
  crypto: aes - helper function to validate key length for AES
    algorithms

 crypto/gcm.c         | 41 ++++++++++++++-------------------------
 include/crypto/aes.h | 17 ++++++++++++++++
 include/crypto/gcm.h | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/crypto/aes.c     |  8 ++++----
 4 files changed, 91 insertions(+), 30 deletions(-)

-- 
2.1.0

[PATCH v2 1/2] crypto: gcm - helper functions for assoclen/authsize check

[Iuliana Prodan]

Added inline helper functions to check authsize and assoclen for
gcm, rfc4106 and rfc4543.
These are used in the generic implementation of gcm, rfc4106 and
rfc4543.

Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
---
 crypto/gcm.c         | 41 ++++++++++++++-------------------------
 include/crypto/gcm.h | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 26 deletions(-)

diff --git a/crypto/gcm.c b/crypto/gcm.c
index f254e2d..2f3b50f 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -152,20 +152,7 @@ static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
 static int crypto_gcm_setauthsize(struct crypto_aead *tfm,
 				  unsigned int authsize)
 {
-	switch (authsize) {
-	case 4:
-	case 8:
-	case 12:
-	case 13:
-	case 14:
-	case 15:
-	case 16:
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	return 0;
+	return crypto_gcm_check_authsize(authsize);
 }
 
 static void crypto_gcm_init_common(struct aead_request *req)
@@ -762,15 +749,11 @@ static int crypto_rfc4106_setauthsize(struct crypto_aead *parent,
 				      unsigned int authsize)
 {
 	struct crypto_rfc4106_ctx *ctx = crypto_aead_ctx(parent);
+	int err;
 
-	switch (authsize) {
-	case 8:
-	case 12:
-	case 16:
-		break;
-	default:
-		return -EINVAL;
-	}
+	err = crypto_rfc4106_check_authsize(authsize);
+	if (err)
+		return err;
 
 	return crypto_aead_setauthsize(ctx->child, authsize);
 }
@@ -818,8 +801,11 @@ static struct aead_request *crypto_rfc4106_crypt(struct aead_request *req)
 
 static int crypto_rfc4106_encrypt(struct aead_request *req)
 {
-	if (req->assoclen != 16 && req->assoclen != 20)
-		return -EINVAL;
+	int err;
+
+	err = crypto_ipsec_check_assoclen(req->assoclen);
+	if (err)
+		return err;
 
 	req = crypto_rfc4106_crypt(req);
 
@@ -828,8 +814,11 @@ static int crypto_rfc4106_encrypt(struct aead_request *req)
 
 static int crypto_rfc4106_decrypt(struct aead_request *req)
 {
-	if (req->assoclen != 16 && req->assoclen != 20)
-		return -EINVAL;
+	int err;
+
+	err = crypto_ipsec_check_assoclen(req->assoclen);
+	if (err)
+		return err;
 
 	req = crypto_rfc4106_crypt(req);
 
diff --git a/include/crypto/gcm.h b/include/crypto/gcm.h
index c50e057..0a2f21e 100644
--- a/include/crypto/gcm.h
+++ b/include/crypto/gcm.h
@@ -1,8 +1,63 @@
 #ifndef _CRYPTO_GCM_H
 #define _CRYPTO_GCM_H
 
+#include <uapi/asm-generic/errno-base.h>
+
 #define GCM_AES_IV_SIZE 12
 #define GCM_RFC4106_IV_SIZE 8
 #define GCM_RFC4543_IV_SIZE 8
 
+/*
+ * validate authentication tag for GCM
+ */
+static inline int crypto_gcm_check_authsize(unsigned int authsize)
+{
+	switch (authsize) {
+	case 4:
+	case 8:
+	case 12:
+	case 13:
+	case 14:
+	case 15:
+	case 16:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
+ * validate authentication tag for RFC4106
+ */
+static inline int crypto_rfc4106_check_authsize(unsigned int authsize)
+{
+	switch (authsize) {
+	case 8:
+	case 12:
+	case 16:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
+ * validate assoclen for RFC4106/RFC4543
+ */
+static inline int crypto_ipsec_check_assoclen(unsigned int assoclen)
+{
+	switch (assoclen) {
+	case 16:
+	case 20:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
 #endif
-- 
2.1.0

[PATCH v2 2/2] crypto: aes - helper function to validate key length for AES algorithms

[Iuliana Prodan]

Add inline helper function to check key length for AES algorithms.
The key can be 128, 192 or 256 bits size.
This function is used in the generic aes implementation.

Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
---
 include/crypto/aes.h | 17 +++++++++++++++++
 lib/crypto/aes.c     |  8 ++++----
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/include/crypto/aes.h b/include/crypto/aes.h
index 8e0f4cf..8ee07a8 100644
--- a/include/crypto/aes.h
+++ b/include/crypto/aes.h
@@ -31,6 +31,23 @@ struct crypto_aes_ctx {
 extern const u32 crypto_ft_tab[4][256] ____cacheline_aligned;
 extern const u32 crypto_it_tab[4][256] ____cacheline_aligned;
 
+/*
+ * validate key length for AES algorithms
+ */
+static inline int crypto_aes_check_keylen(unsigned int keylen)
+{
+	switch (keylen) {
+	case AES_KEYSIZE_128:
+	case AES_KEYSIZE_192:
+	case AES_KEYSIZE_256:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int crypto_aes_set_key(struct crypto_tfm *tfm, const u8 *in_key,
 		unsigned int key_len);
 
diff --git a/lib/crypto/aes.c b/lib/crypto/aes.c
index 4e100af..3407b01 100644
--- a/lib/crypto/aes.c
+++ b/lib/crypto/aes.c
@@ -187,11 +187,11 @@ int aes_expandkey(struct crypto_aes_ctx *ctx, const u8 *in_key,
 {
 	u32 kwords = key_len / sizeof(u32);
 	u32 rc, i, j;
+	int err;
 
-	if (key_len != AES_KEYSIZE_128 &&
-	    key_len != AES_KEYSIZE_192 &&
-	    key_len != AES_KEYSIZE_256)
-		return -EINVAL;
+	err = crypto_aes_check_keylen(key_len);
+	if (err)
+		return err;
 
 	ctx->key_length = key_len;
 
-- 
2.1.0

Re: [PATCH v2 1/2] crypto: gcm - helper functions for assoclen/authsize check

[Horia Geanta]

On 7/30/2019 1:33 PM, Iuliana Prodan wrote:
> Added inline helper functions to check authsize and assoclen for
> gcm, rfc4106 and rfc4543.
> These are used in the generic implementation of gcm, rfc4106 and
> rfc4543.
> 
> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Reviewed-by: Horia Geantă <horia.geanta@nxp.com>

Thanks,
Horia

Re: [PATCH v2 2/2] crypto: aes - helper function to validate key length for AES algorithms

[Horia Geanta]

On 7/30/2019 1:33 PM, Iuliana Prodan wrote:
> Add inline helper function to check key length for AES algorithms.
> The key can be 128, 192 or 256 bits size.
> This function is used in the generic aes implementation.
> 
> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Reviewed-by: Horia Geantă <horia.geanta@nxp.com>

Thanks,
Horia

Re: [PATCH v2 1/2] crypto: gcm - helper functions for assoclen/authsize check

[Horia Geanta]

On 7/30/2019 1:33 PM, Iuliana Prodan wrote:
> --- a/include/crypto/gcm.h
> +++ b/include/crypto/gcm.h
> @@ -1,8 +1,63 @@
>  #ifndef _CRYPTO_GCM_H
>  #define _CRYPTO_GCM_H
>  
> +#include <uapi/asm-generic/errno-base.h>
> +
This is new in v2 and I missed it initially.

If needed, <linux/errno.h> should be used instead.

Horia

Re: [PATCH v2 2/2] crypto: aes - helper function to validate key length for AES algorithms

[Ard Biesheuvel]

On Tue, 30 Jul 2019 at 13:33, Iuliana Prodan <iuliana.prodan@nxp.com> wrote:
>
> Add inline helper function to check key length for AES algorithms.
> The key can be 128, 192 or 256 bits size.
> This function is used in the generic aes implementation.
>
> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
> ---
>  include/crypto/aes.h | 17 +++++++++++++++++
>  lib/crypto/aes.c     |  8 ++++----
>  2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/include/crypto/aes.h b/include/crypto/aes.h
> index 8e0f4cf..8ee07a8 100644
> --- a/include/crypto/aes.h
> +++ b/include/crypto/aes.h
> @@ -31,6 +31,23 @@ struct crypto_aes_ctx {
>  extern const u32 crypto_ft_tab[4][256] ____cacheline_aligned;
>  extern const u32 crypto_it_tab[4][256] ____cacheline_aligned;
>
> +/*
> + * validate key length for AES algorithms
> + */
> +static inline int crypto_aes_check_keylen(unsigned int keylen)

Please rename this to aes_check_keylen()

> +{
> +       switch (keylen) {
> +       case AES_KEYSIZE_128:
> +       case AES_KEYSIZE_192:
> +       case AES_KEYSIZE_256:
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
>  int crypto_aes_set_key(struct crypto_tfm *tfm, const u8 *in_key,
>                 unsigned int key_len);
>
> diff --git a/lib/crypto/aes.c b/lib/crypto/aes.c
> index 4e100af..3407b01 100644
> --- a/lib/crypto/aes.c
> +++ b/lib/crypto/aes.c
> @@ -187,11 +187,11 @@ int aes_expandkey(struct crypto_aes_ctx *ctx, const u8 *in_key,
>  {
>         u32 kwords = key_len / sizeof(u32);
>         u32 rc, i, j;
> +       int err;
>
> -       if (key_len != AES_KEYSIZE_128 &&
> -           key_len != AES_KEYSIZE_192 &&
> -           key_len != AES_KEYSIZE_256)
> -               return -EINVAL;
> +       err = crypto_aes_check_keylen(key_len);
> +       if (err)
> +               return err;
>
>         ctx->key_length = key_len;
>
> --
> 2.1.0
>

Re: [PATCH v2 2/2] crypto: aes - helper function to validate key length for AES algorithms

[Iuliana Prodan]

On 7/31/2019 8:33 AM, Ard Biesheuvel wrote:
> On Tue, 30 Jul 2019 at 13:33, Iuliana Prodan <iuliana.prodan@nxp.com> wrote:
>>
>> Add inline helper function to check key length for AES algorithms.
>> The key can be 128, 192 or 256 bits size.
>> This function is used in the generic aes implementation.
>>
>> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
>> ---
>>   include/crypto/aes.h | 17 +++++++++++++++++
>>   lib/crypto/aes.c     |  8 ++++----
>>   2 files changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/crypto/aes.h b/include/crypto/aes.h
>> index 8e0f4cf..8ee07a8 100644
>> --- a/include/crypto/aes.h
>> +++ b/include/crypto/aes.h
>> @@ -31,6 +31,23 @@ struct crypto_aes_ctx {
>>   extern const u32 crypto_ft_tab[4][256] ____cacheline_aligned;
>>   extern const u32 crypto_it_tab[4][256] ____cacheline_aligned;
>>
>> +/*
>> + * validate key length for AES algorithms
>> + */
>> +static inline int crypto_aes_check_keylen(unsigned int keylen)
> 
> Please rename this to aes_check_keylen()
> 
I just renamed it to crypto_, the first version was check_aes_keylen
- see https://patchwork.kernel.org/patch/11058869/.
I think is better to keep the helper functions with crypto_, as most of 
these type of functions, in crypto, have this prefix.

>> +{
>> +       switch (keylen) {
>> +       case AES_KEYSIZE_128:
>> +       case AES_KEYSIZE_192:
>> +       case AES_KEYSIZE_256:
>> +               break;
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>   int crypto_aes_set_key(struct crypto_tfm *tfm, const u8 *in_key,
>>                  unsigned int key_len);
>>
>> diff --git a/lib/crypto/aes.c b/lib/crypto/aes.c
>> index 4e100af..3407b01 100644
>> --- a/lib/crypto/aes.c
>> +++ b/lib/crypto/aes.c
>> @@ -187,11 +187,11 @@ int aes_expandkey(struct crypto_aes_ctx *ctx, const u8 *in_key,
>>   {
>>          u32 kwords = key_len / sizeof(u32);
>>          u32 rc, i, j;
>> +       int err;
>>
>> -       if (key_len != AES_KEYSIZE_128 &&
>> -           key_len != AES_KEYSIZE_192 &&
>> -           key_len != AES_KEYSIZE_256)
>> -               return -EINVAL;
>> +       err = crypto_aes_check_keylen(key_len);
>> +       if (err)
>> +               return err;
>>
>>          ctx->key_length = key_len;
>>
>> --
>> 2.1.0
>>
> 

Re: [PATCH v2 2/2] crypto: aes - helper function to validate key length for AES algorithms

[Ard Biesheuvel]

On Wed, 31 Jul 2019 at 11:35, Iuliana Prodan <iuliana.prodan@nxp.com> wrote:
>
> On 7/31/2019 8:33 AM, Ard Biesheuvel wrote:
> > On Tue, 30 Jul 2019 at 13:33, Iuliana Prodan <iuliana.prodan@nxp.com> wrote:
> >>
> >> Add inline helper function to check key length for AES algorithms.
> >> The key can be 128, 192 or 256 bits size.
> >> This function is used in the generic aes implementation.
> >>
> >> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
> >> ---
> >>   include/crypto/aes.h | 17 +++++++++++++++++
> >>   lib/crypto/aes.c     |  8 ++++----
> >>   2 files changed, 21 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/include/crypto/aes.h b/include/crypto/aes.h
> >> index 8e0f4cf..8ee07a8 100644
> >> --- a/include/crypto/aes.h
> >> +++ b/include/crypto/aes.h
> >> @@ -31,6 +31,23 @@ struct crypto_aes_ctx {
> >>   extern const u32 crypto_ft_tab[4][256] ____cacheline_aligned;
> >>   extern const u32 crypto_it_tab[4][256] ____cacheline_aligned;
> >>
> >> +/*
> >> + * validate key length for AES algorithms
> >> + */
> >> +static inline int crypto_aes_check_keylen(unsigned int keylen)
> >
> > Please rename this to aes_check_keylen()
> >
> I just renamed it to crypto_, the first version was check_aes_keylen
> - see https://patchwork.kernel.org/patch/11058869/.
> I think is better to keep the helper functions with crypto_, as most of
> these type of functions, in crypto, have this prefix.
>

The AES library consists of

aes_encrypt
aes_decrypt
aes_expandkey

and has no dependencies on the crypto API, which is why I omitted the
crypto_ prefix from the identifiers. Please do the same for this
function.
.


> >> +{
> >> +       switch (keylen) {
> >> +       case AES_KEYSIZE_128:
> >> +       case AES_KEYSIZE_192:
> >> +       case AES_KEYSIZE_256:
> >> +               break;
> >> +       default:
> >> +               return -EINVAL;
> >> +       }
> >> +
> >> +       return 0;
> >> +}
> >> +
> >>   int crypto_aes_set_key(struct crypto_tfm *tfm, const u8 *in_key,
> >>                  unsigned int key_len);
> >>
> >> diff --git a/lib/crypto/aes.c b/lib/crypto/aes.c
> >> index 4e100af..3407b01 100644
> >> --- a/lib/crypto/aes.c
> >> +++ b/lib/crypto/aes.c
> >> @@ -187,11 +187,11 @@ int aes_expandkey(struct crypto_aes_ctx *ctx, const u8 *in_key,
> >>   {
> >>          u32 kwords = key_len / sizeof(u32);
> >>          u32 rc, i, j;
> >> +       int err;
> >>
> >> -       if (key_len != AES_KEYSIZE_128 &&
> >> -           key_len != AES_KEYSIZE_192 &&
> >> -           key_len != AES_KEYSIZE_256)
> >> -               return -EINVAL;
> >> +       err = crypto_aes_check_keylen(key_len);
> >> +       if (err)
> >> +               return err;
> >>
> >>          ctx->key_length = key_len;
> >>
> >> --
> >> 2.1.0
> >>
> >
>