| Message ID | 20150123223357.15316.72597.stgit@tstruk-mobl1 (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
Am Freitag, 23. Januar 2015, 14:33:57 schrieb Tadeusz Struk: Hi Tadeusz, > Changed the __driver-gcm-aes-aesni to be a proper aead algorithm. > > Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com> > --- > arch/x86/crypto/aesni-intel_glue.c | 53 > ++++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 14 > deletions(-) > > diff --git a/arch/x86/crypto/aesni-intel_glue.c > b/arch/x86/crypto/aesni-intel_glue.c index 947c6bf..5544ad9 100644 > --- a/arch/x86/crypto/aesni-intel_glue.c > +++ b/arch/x86/crypto/aesni-intel_glue.c > @@ -890,15 +890,12 @@ out_free_ablkcipher: > return ret; > } > > -static int rfc4106_set_key(struct crypto_aead *parent, const u8 *key, > - unsigned int key_len) > +static int common_rfc4106_set_key(struct crypto_aead *aead, const u8 *key, > + unsigned int key_len) > { > int ret = 0; > - struct crypto_tfm *tfm = crypto_aead_tfm(parent); > - struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(parent); > - struct crypto_aead *cryptd_child = cryptd_aead_child(ctx->cryptd_tfm); > - struct aesni_rfc4106_gcm_ctx *child_ctx = > - aesni_rfc4106_gcm_ctx_get(cryptd_child); > + struct crypto_tfm *tfm = crypto_aead_tfm(aead); > + struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(aead); > u8 *new_key_align, *new_key_mem = NULL; > > if (key_len < 4) { > @@ -943,20 +940,29 @@ static int rfc4106_set_key(struct crypto_aead *parent, > const u8 *key, goto exit; > } > ret = rfc4106_set_hash_subkey(ctx->hash_subkey, key, key_len); > - memcpy(child_ctx, ctx, sizeof(*ctx)); > exit: > kfree(new_key_mem); > return ret; > } > > -/* This is the Integrity Check Value (aka the authentication tag length and > can - * be 8, 12 or 16 bytes long. */ > -static int rfc4106_set_authsize(struct crypto_aead *parent, > - unsigned int authsize) > +static int rfc4106_set_key(struct crypto_aead *parent, const u8 *key, > + unsigned int key_len) > { > struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(parent); > struct crypto_aead *cryptd_child = cryptd_aead_child(ctx->cryptd_tfm); > + struct aesni_rfc4106_gcm_ctx *child_ctx = > + aesni_rfc4106_gcm_ctx_get(cryptd_child); > + int ret; > > + ret = common_rfc4106_set_key(parent, key, key_len); Shouldn't that one be crypto_aead_setkey, i.e using the regular crypto API instead of internal calls? > + if (!ret) > + memcpy(child_ctx, ctx, sizeof(*ctx)); > + return ret; > +} > + > +static int common_rfc4106_set_authsize(struct crypto_aead *aead, > + unsigned int authsize) > +{ > switch (authsize) { > case 8: > case 12: > @@ -965,11 +971,26 @@ static int rfc4106_set_authsize(struct crypto_aead > *parent, default: > return -EINVAL; > } > - crypto_aead_crt(parent)->authsize = authsize; > - crypto_aead_crt(cryptd_child)->authsize = authsize; > + crypto_aead_crt(aead)->authsize = authsize; > return 0; > } > > +/* This is the Integrity Check Value (aka the authentication tag length and > can + * be 8, 12 or 16 bytes long. */ > +static int rfc4106_set_authsize(struct crypto_aead *parent, > + unsigned int authsize) > +{ > + struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(parent); > + struct crypto_aead *cryptd_child = cryptd_aead_child(ctx->cryptd_tfm); > + int ret; > + > + ret = common_rfc4106_set_authsize(parent, authsize); Same here, shouldn't that one be crypto_aead_setauthsize? > + if (!ret) > + ret = common_rfc4106_set_authsize(cryptd_child, authsize); > + > + return ret; > +} > + > static int rfc4106_encrypt(struct aead_request *req) > { > int ret; > @@ -1366,8 +1387,12 @@ static struct crypto_alg aesni_algs[] = { { > .cra_module = THIS_MODULE, > .cra_u = { > .aead = { > + .setkey = common_rfc4106_set_key, > + .setauthsize = common_rfc4106_set_authsize, > .encrypt = __driver_rfc4106_encrypt, > .decrypt = __driver_rfc4106_decrypt, > + .ivsize = 8, > + .maxauthsize = 16, > }, > }, > }, { > > -- > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Stephan, On 01/25/2015 12:58 AM, Stephan Mueller wrote: >> +static int rfc4106_set_key(struct crypto_aead *parent, const u8 *key, >> > + unsigned int key_len) >> > { >> > struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(parent); >> > struct crypto_aead *cryptd_child = cryptd_aead_child(ctx->cryptd_tfm); >> > + struct aesni_rfc4106_gcm_ctx *child_ctx = >> > + aesni_rfc4106_gcm_ctx_get(cryptd_child); >> > + int ret; >> > >> > + ret = common_rfc4106_set_key(parent, key, key_len); > Shouldn't that one be crypto_aead_setkey, i.e using the regular crypto API > instead of internal calls? > No, I don't think so. I think that would create an infinite loop. >> +static int rfc4106_set_authsize(struct crypto_aead *parent, >> > + unsigned int authsize) >> > +{ >> > + struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(parent); >> > + struct crypto_aead *cryptd_child = cryptd_aead_child(ctx->cryptd_tfm); >> > + int ret; >> > + >> > + ret = common_rfc4106_set_authsize(parent, authsize); > Same here, shouldn't that one be crypto_aead_setauthsize? > Same here. Thanks, Tadeusz -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 25, 2015 at 08:26:50AM -0800, Tadeusz Struk wrote: > Hi Stephan, > On 01/25/2015 12:58 AM, Stephan Mueller wrote: > >> +static int rfc4106_set_key(struct crypto_aead *parent, const u8 *key, > >> > + unsigned int key_len) > >> > { > >> > struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(parent); > >> > struct crypto_aead *cryptd_child = cryptd_aead_child(ctx->cryptd_tfm); > >> > + struct aesni_rfc4106_gcm_ctx *child_ctx = > >> > + aesni_rfc4106_gcm_ctx_get(cryptd_child); > >> > + int ret; > >> > > >> > + ret = common_rfc4106_set_key(parent, key, key_len); > > Shouldn't that one be crypto_aead_setkey, i.e using the regular crypto API > > instead of internal calls? > > No, I don't think so. I think that would create an infinite loop. So why does it work for ablk_helper but not for aead? Cheers,
On 01/25/2015 04:10 PM, Herbert Xu wrote: > On Sun, Jan 25, 2015 at 08:26:50AM -0800, Tadeusz Struk wrote: >> > Hi Stephan, >> > On 01/25/2015 12:58 AM, Stephan Mueller wrote: >>>> > >> +static int rfc4106_set_key(struct crypto_aead *parent, const u8 *key, >>>>> > >> > + unsigned int key_len) >>>>> > >> > { >>>>> > >> > struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(parent); >>>>> > >> > struct crypto_aead *cryptd_child = cryptd_aead_child(ctx->cryptd_tfm); >>>>> > >> > + struct aesni_rfc4106_gcm_ctx *child_ctx = >>>>> > >> > + aesni_rfc4106_gcm_ctx_get(cryptd_child); >>>>> > >> > + int ret; >>>>> > >> > >>>>> > >> > + ret = common_rfc4106_set_key(parent, key, key_len); >>> > > Shouldn't that one be crypto_aead_setkey, i.e using the regular crypto API >>> > > instead of internal calls? >> > >> > No, I don't think so. I think that would create an infinite loop. > So why does it work for ablk_helper but not for aead? Here we have two instances of crypto_aead algorithm, one the rfc4106(gcm(aes)), whose setkey points to rfc4106_set_key(), and the internal helper __gcm-aes-aesni (wrapped in by the cryptd interface), whose setkey points to common_rfc4106_set_key(). If we would call crypto_aead_setkey() on the parent from rfc4106_set_key() then we would invoke the same rfc4106_set_key() function. It would be ok to call the crypto_aead_setkey() on the child, but what's the point? What we really want to do is to setup the context (authsize and key) for both the top level rfc4106(gcm(aes)) and the helper __gcm-aes-aesni. We can do it by calling the internal function directly or by the regular crypto API crypto_aead_setkey()/set_authsize() on the child, but I don't see any difference or benefit of it. Hope that make sense. Thanks, Tadeusz -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Montag, 26. Januar 2015, 08:58:33 schrieb Tadeusz Struk: Hi Tadeusz, > On 01/25/2015 04:10 PM, Herbert Xu wrote: > > On Sun, Jan 25, 2015 at 08:26:50AM -0800, Tadeusz Struk wrote: > >> > Hi Stephan, > >> > > >> > On 01/25/2015 12:58 AM, Stephan Mueller wrote: > >>>> > >> +static int rfc4106_set_key(struct crypto_aead *parent, const u8 > >>>> > >> *key, > >>>> > >> > >>>>> > >> > + unsigned int key_len) > >>>>> > >> > > >>>>> > >> > { > >>>>> > >> > > >>>>> > >> > struct aesni_rfc4106_gcm_ctx *ctx = > >>>>> > >> > aesni_rfc4106_gcm_ctx_get(parent); > >>>>> > >> > struct crypto_aead *cryptd_child = > >>>>> > >> > cryptd_aead_child(ctx->cryptd_tfm); > >>>>> > >> > > >>>>> > >> > + struct aesni_rfc4106_gcm_ctx *child_ctx = > >>>>> > >> > + aesni_rfc4106_gcm_ctx_get(cryptd_child); > >>>>> > >> > + int ret; > >>>>> > >> > > >>>>> > >> > + ret = common_rfc4106_set_key(parent, key, key_len); > >>> > > > >>> > > Shouldn't that one be crypto_aead_setkey, i.e using the regular > >>> > > crypto API > >>> > > instead of internal calls? > >> > > >> > No, I don't think so. I think that would create an infinite loop. > > > > So why does it work for ablk_helper but not for aead? > > Here we have two instances of crypto_aead algorithm, one the > rfc4106(gcm(aes)), whose setkey points to rfc4106_set_key(), and the > internal helper __gcm-aes-aesni (wrapped in by the cryptd interface), > whose setkey points to common_rfc4106_set_key(). If we would call > crypto_aead_setkey() on the parent from rfc4106_set_key() then we would > invoke the same rfc4106_set_key() function. It would be ok to call the > crypto_aead_setkey() on the child, but what's the point? The point is to maintain an onion style framework that is coherent. All other ciphers implement it (look at the generic gcm.c). > What we really want to do is to setup the context (authsize and key) for > both the top level rfc4106(gcm(aes)) and the helper __gcm-aes-aesni. We > can do it by calling the internal function directly or by the regular > crypto API crypto_aead_setkey()/set_authsize() on the child, but I don't > see any difference or benefit of it. Then, why do we register the internal __driver "cipher" at all. On the one hand, the kernel crypto API is used for some aspects but not for others. Hm... > Hope that make sense. > Thanks, > Tadeusz > -- > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/26/2015 11:20 AM, Stephan Mueller wrote: >> > Here we have two instances of crypto_aead algorithm, one the >> > rfc4106(gcm(aes)), whose setkey points to rfc4106_set_key(), and the >> > internal helper __gcm-aes-aesni (wrapped in by the cryptd interface), >> > whose setkey points to common_rfc4106_set_key(). If we would call >> > crypto_aead_setkey() on the parent from rfc4106_set_key() then we would >> > invoke the same rfc4106_set_key() function. It would be ok to call the >> > crypto_aead_setkey() on the child, but what's the point? > The point is to maintain an onion style framework that is coherent. All other > ciphers implement it (look at the generic gcm.c). > Hi Stephan, Ok, to keep it consistent is a good enough reason. I'll send v2 soon. Thanks, Tadeusz -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c index 947c6bf..5544ad9 100644 --- a/arch/x86/crypto/aesni-intel_glue.c +++ b/arch/x86/crypto/aesni-intel_glue.c @@ -890,15 +890,12 @@ out_free_ablkcipher: return ret; } -static int rfc4106_set_key(struct crypto_aead *parent, const u8 *key, - unsigned int key_len) +static int common_rfc4106_set_key(struct crypto_aead *aead, const u8 *key, + unsigned int key_len) { int ret = 0; - struct crypto_tfm *tfm = crypto_aead_tfm(parent); - struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(parent); - struct crypto_aead *cryptd_child = cryptd_aead_child(ctx->cryptd_tfm); - struct aesni_rfc4106_gcm_ctx *child_ctx = - aesni_rfc4106_gcm_ctx_get(cryptd_child); + struct crypto_tfm *tfm = crypto_aead_tfm(aead); + struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(aead); u8 *new_key_align, *new_key_mem = NULL; if (key_len < 4) { @@ -943,20 +940,29 @@ static int rfc4106_set_key(struct crypto_aead *parent, const u8 *key, goto exit; } ret = rfc4106_set_hash_subkey(ctx->hash_subkey, key, key_len); - memcpy(child_ctx, ctx, sizeof(*ctx)); exit: kfree(new_key_mem); return ret; } -/* This is the Integrity Check Value (aka the authentication tag length and can - * be 8, 12 or 16 bytes long. */ -static int rfc4106_set_authsize(struct crypto_aead *parent, - unsigned int authsize) +static int rfc4106_set_key(struct crypto_aead *parent, const u8 *key, + unsigned int key_len) { struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(parent); struct crypto_aead *cryptd_child = cryptd_aead_child(ctx->cryptd_tfm); + struct aesni_rfc4106_gcm_ctx *child_ctx = + aesni_rfc4106_gcm_ctx_get(cryptd_child); + int ret; + ret = common_rfc4106_set_key(parent, key, key_len); + if (!ret) + memcpy(child_ctx, ctx, sizeof(*ctx)); + return ret; +} + +static int common_rfc4106_set_authsize(struct crypto_aead *aead, + unsigned int authsize) +{ switch (authsize) { case 8: case 12: @@ -965,11 +971,26 @@ static int rfc4106_set_authsize(struct crypto_aead *parent, default: return -EINVAL; } - crypto_aead_crt(parent)->authsize = authsize; - crypto_aead_crt(cryptd_child)->authsize = authsize; + crypto_aead_crt(aead)->authsize = authsize; return 0; } +/* This is the Integrity Check Value (aka the authentication tag length and can + * be 8, 12 or 16 bytes long. */ +static int rfc4106_set_authsize(struct crypto_aead *parent, + unsigned int authsize) +{ + struct aesni_rfc4106_gcm_ctx *ctx = aesni_rfc4106_gcm_ctx_get(parent); + struct crypto_aead *cryptd_child = cryptd_aead_child(ctx->cryptd_tfm); + int ret; + + ret = common_rfc4106_set_authsize(parent, authsize); + if (!ret) + ret = common_rfc4106_set_authsize(cryptd_child, authsize); + + return ret; +} + static int rfc4106_encrypt(struct aead_request *req) { int ret; @@ -1366,8 +1387,12 @@ static struct crypto_alg aesni_algs[] = { { .cra_module = THIS_MODULE, .cra_u = { .aead = { + .setkey = common_rfc4106_set_key, + .setauthsize = common_rfc4106_set_authsize, .encrypt = __driver_rfc4106_encrypt, .decrypt = __driver_rfc4106_decrypt, + .ivsize = 8, + .maxauthsize = 16, }, }, }, {
Changed the __driver-gcm-aes-aesni to be a proper aead algorithm. Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com> --- arch/x86/crypto/aesni-intel_glue.c | 53 ++++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 14 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html