RE: [PATCH 2/2] crypto: caam - add support for rfc4106(gcm(aes))

2014-10-10 Thread tudor.amba...@freescale.com

On Thu, 9 Oct 2014 17:54:10 +0300
Tudor Ambarus tudor.amba...@freescale.com wrote:

 +static int rfc4106_set_sh_desc(struct crypto_aead *aead)
...
 + /*
 +  * Job Descriptor and Shared Descriptors
 +  * must all fit into the 64-word Descriptor h/w Buffer
 +  */
 + if (DESC_RFC4106_DEC_LEN + DESC_JOB_IO_LEN +
 + ctx-enckeylen = CAAM_DESC_BYTES_MAX)
 + key_fit_inline = true;

we need to reset encrypt descriptor's keys_fit_inline setting to
false before doing this.

Also, the singular of keys_fit_inline is key_fits_inline, but
I'd prefer we not gratuitously rename the variable from the rest of
the driver's keys_fit_inline for consistency's sake, thanks.
[TA] Agreed.

 + /*
 +  * Job Descriptor and Shared Descriptors
 +  * must all fit into the 64-word Descriptor h/w Buffer
 +  */
 + if (DESC_RFC4106_GIVENC_LEN + DESC_JOB_IO_LEN +
 + ctx-split_key_pad_len + ctx-enckeylen =
 + CAAM_DESC_BYTES_MAX)
 + key_fit_inline = true;

we need to reset the variable here too.
[TA] Agreed.

 +static int rfc4106_setauthsize(struct crypto_aead *authenc,
 +unsigned int authsize)
 +{
 + struct caam_ctx *ctx = crypto_aead_ctx(authenc);
 +
 + switch (authsize) {
 + case 8:
 + case 12:
 + case 16:
 + break;
 + default:
 + return -EINVAL;
 + }

the h/w can handle more authsizes than that, so we
shouldn't be blocking it from doing so here.

[TA] rfc4106 says that Implementations MUST support a full-length 16-octet 
ICV, and MAY support 8 or 12 octet ICVs, and MUST NOT support other ICV 
lengths.
Do we want to support other ICV lengths?

 @@ -2601,6 +2986,23 @@ static struct caam_alg_template driver_algs[] = {
  OP_ALG_AAI_HMAC_PRECOMP,
   .alg_op = OP_ALG_ALGSEL_SHA512 | OP_ALG_AAI_HMAC,
   },
 + {
 + .name = rfc4106(gcm(aes)),
 + .driver_name = rfc4106-gcm-aes-caam,
 + .blocksize = 1,
 + .type = CRYPTO_ALG_TYPE_AEAD,
 + .template_aead = {
 + .setkey = rfc4106_setkey,
 + .setauthsize = rfc4106_setauthsize,
 + .encrypt = aead_encrypt,
 + .decrypt = aead_decrypt,
 + .givencrypt = aead_givencrypt,
 + .geniv = built-in,
 + .ivsize = 8,
 + .maxauthsize = 16,

AES_BLOCK_SIZE

[TA] I don't think we should change the blocksize value to AES_BLOCK_SIZE.

Thank you,
Tudor

Thanks,

Kim
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] crypto: caam - add support for rfc4106(gcm(aes))

2014-10-10 Thread Kim Phillips
On Fri, 10 Oct 2014 05:10:55 -0500
Ambarus Tudor-Dan-B38632 tudor.amba...@freescale.com wrote:

 On Thu, 9 Oct 2014 17:54:10 +0300
 Tudor Ambarus tudor.amba...@freescale.com wrote:
  +static int rfc4106_setauthsize(struct crypto_aead *authenc,
  +  unsigned int authsize)
  +{
  +   struct caam_ctx *ctx = crypto_aead_ctx(authenc);
  +
  +   switch (authsize) {
  +   case 8:
  +   case 12:
  +   case 16:
  +   break;
  +   default:
  +   return -EINVAL;
  +   }
 
 the h/w can handle more authsizes than that, so we
 shouldn't be blocking it from doing so here.
 
 [TA] rfc4106 says that Implementations MUST support a full-length 16-octet 
 ICV, and MAY support 8 or 12 octet ICVs, and MUST NOT support other ICV 
 lengths.
 Do we want to support other ICV lengths?

how much the linux kernel wants to enforce the RFC is up to the
higher levels in its crypto API and other front end interfaces such
as XFRM. So those checks for RFC compliance should be performed
before they reach the implementation (driver).

Meanwhile, drivers themselves should accurately represent the h/w
capabilities, whether they meet and/or exceed the RFC's constraints
or not, IMO.  This way, if/when the RFC gets extended to become
open to a wider range of values, the driver needs no changes.

  @@ -2601,6 +2986,23 @@ static struct caam_alg_template driver_algs[] = {
 OP_ALG_AAI_HMAC_PRECOMP,
  .alg_op = OP_ALG_ALGSEL_SHA512 | OP_ALG_AAI_HMAC,
  },
  +   {
  +   .name = rfc4106(gcm(aes)),
  +   .driver_name = rfc4106-gcm-aes-caam,
  +   .blocksize = 1,
  +   .type = CRYPTO_ALG_TYPE_AEAD,
  +   .template_aead = {
  +   .setkey = rfc4106_setkey,
  +   .setauthsize = rfc4106_setauthsize,
  +   .encrypt = aead_encrypt,
  +   .decrypt = aead_decrypt,
  +   .givencrypt = aead_givencrypt,
  +   .geniv = built-in,
  +   .ivsize = 8,
  +   .maxauthsize = 16,
 
 AES_BLOCK_SIZE
 
 [TA] I don't think we should change the blocksize value to AES_BLOCK_SIZE.

sorry, I meant just .maxauthsize = AES_BLOCK_SIZE.

Thanks,

Kim
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] crypto: caam - add support for rfc4106(gcm(aes))

2014-10-09 Thread Kim Phillips
On Thu, 9 Oct 2014 17:54:10 +0300
Tudor Ambarus tudor.amba...@freescale.com wrote:

 +static int rfc4106_set_sh_desc(struct crypto_aead *aead)
...
 + /*
 +  * Job Descriptor and Shared Descriptors
 +  * must all fit into the 64-word Descriptor h/w Buffer
 +  */
 + if (DESC_RFC4106_DEC_LEN + DESC_JOB_IO_LEN +
 + ctx-enckeylen = CAAM_DESC_BYTES_MAX)
 + key_fit_inline = true;

we need to reset encrypt descriptor's keys_fit_inline setting to
false before doing this.

Also, the singular of keys_fit_inline is key_fits_inline, but
I'd prefer we not gratuitously rename the variable from the rest of
the driver's keys_fit_inline for consistency's sake, thanks.

 + /*
 +  * Job Descriptor and Shared Descriptors
 +  * must all fit into the 64-word Descriptor h/w Buffer
 +  */
 + if (DESC_RFC4106_GIVENC_LEN + DESC_JOB_IO_LEN +
 + ctx-split_key_pad_len + ctx-enckeylen =
 + CAAM_DESC_BYTES_MAX)
 + key_fit_inline = true;

we need to reset the variable here too.

 +static int rfc4106_setauthsize(struct crypto_aead *authenc,
 +unsigned int authsize)
 +{
 + struct caam_ctx *ctx = crypto_aead_ctx(authenc);
 +
 + switch (authsize) {
 + case 8:
 + case 12:
 + case 16:
 + break;
 + default:
 + return -EINVAL;
 + }

the h/w can handle more authsizes than that, so we
shouldn't be blocking it from doing so here.

 @@ -2601,6 +2986,23 @@ static struct caam_alg_template driver_algs[] = {
  OP_ALG_AAI_HMAC_PRECOMP,
   .alg_op = OP_ALG_ALGSEL_SHA512 | OP_ALG_AAI_HMAC,
   },
 + {
 + .name = rfc4106(gcm(aes)),
 + .driver_name = rfc4106-gcm-aes-caam,
 + .blocksize = 1,
 + .type = CRYPTO_ALG_TYPE_AEAD,
 + .template_aead = {
 + .setkey = rfc4106_setkey,
 + .setauthsize = rfc4106_setauthsize,
 + .encrypt = aead_encrypt,
 + .decrypt = aead_decrypt,
 + .givencrypt = aead_givencrypt,
 + .geniv = built-in,
 + .ivsize = 8,
 + .maxauthsize = 16,

AES_BLOCK_SIZE

Thanks,

Kim
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html