[PATCH v3] crypto: talitos: Remove zero_entry static initializer
Compiling the talitos driver with my GCC 4.3.1 e500v2 cross-compiler
resulted in a failed build due to the anonymous union/structures
introduced in this commit:
crypto: talitos - enhanced talitos_desc struct for SEC1
The build error was:
drivers/crypto/talitos.h:56: error: unknown field 'len' specified in
initializer
drivers/crypto/talitos.h:56: warning: missing braces around initializer
drivers/crypto/talitos.h:56: warning: (near initialization for
'zero_entry.')
drivers/crypto/talitos.h:57: error: unknown field 'j_extent' specified in
initializer
drivers/crypto/talitos.h:58: error: unknown field 'eptr' specified in
initializer
drivers/crypto/talitos.h:58: warning: excess elements in struct initializer
drivers/crypto/talitos.h:58: warning: (near initialization for 'zero_entry')
make[2]: *** [drivers/crypto/talitos.o] Error 1
make[1]: *** [drivers/crypto] Error 2
make: *** [drivers] Error 2
This patch eliminates the errors by relying on the C standard's
implicit assignment of zero to static variables.
Signed-off-by: Aaron Sierra
---
drivers/crypto/talitos.h | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index 314daf5..163cfe7 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -52,12 +52,7 @@ struct talitos_ptr {
__be32 ptr; /* address */
};
-static const struct talitos_ptr zero_entry = {
- .len = 0,
- .j_extent = 0,
- .eptr = 0,
- .ptr = 0
-};
+static const struct talitos_ptr zero_entry;
/* descriptor */
struct talitos_desc {
--
1.9.1
--
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 v2] crypto: talitos: Remove zero_entry static initializer
- Original Message - > From: "Herbert Xu" > Sent: Monday, August 3, 2015 6:31:20 PM > > On Mon, Aug 03, 2015 at 11:14:37AM -0500, Aaron Sierra wrote: > > Compiling the talitos driver with my GCC 4.3.1 e500v2 cross-compiler > > resulted in a failed build due to the anonymous union/structures > > introduced in this commit: > > > > crypto: talitos - enhanced talitos_desc struct for SEC1 > > > > The build error was: > > > > drivers/crypto/talitos.h:56: error: unknown field 'len' specified in > > initializer > > drivers/crypto/talitos.h:56: warning: missing braces around initializer > > drivers/crypto/talitos.h:56: warning: (near initialization for > > 'zero_entry.') > > drivers/crypto/talitos.h:57: error: unknown field 'j_extent' specified in > > initializer > > drivers/crypto/talitos.h:58: error: unknown field 'eptr' specified in > > initializer > > drivers/crypto/talitos.h:58: warning: excess elements in struct > > initializer > > drivers/crypto/talitos.h:58: warning: (near initialization for > > 'zero_entry') > > make[2]: *** [drivers/crypto/talitos.o] Error 1 > > make[1]: *** [drivers/crypto] Error 2 > > make: *** [drivers] Error 2 > > > > This patch eliminates the errors by moving the static constant > > zero_entry to the talitos_private structure. As a member of that > > structure, zero_entry gets initialized for free (and compatibly) by > > the kzalloc() during device probe. > > Why? A static variable is always zeroed. Please just get rid > of the initialisation and it will work. Thanks for elaborating. I'll make the change. -Aaron -- 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 v2] crypto: talitos: Remove zero_entry static initializer
On Mon, Aug 03, 2015 at 11:14:37AM -0500, Aaron Sierra wrote: > Compiling the talitos driver with my GCC 4.3.1 e500v2 cross-compiler > resulted in a failed build due to the anonymous union/structures > introduced in this commit: > > crypto: talitos - enhanced talitos_desc struct for SEC1 > > The build error was: > > drivers/crypto/talitos.h:56: error: unknown field 'len' specified in > initializer > drivers/crypto/talitos.h:56: warning: missing braces around initializer > drivers/crypto/talitos.h:56: warning: (near initialization for > 'zero_entry.') > drivers/crypto/talitos.h:57: error: unknown field 'j_extent' specified in > initializer > drivers/crypto/talitos.h:58: error: unknown field 'eptr' specified in > initializer > drivers/crypto/talitos.h:58: warning: excess elements in struct initializer > drivers/crypto/talitos.h:58: warning: (near initialization for 'zero_entry') > make[2]: *** [drivers/crypto/talitos.o] Error 1 > make[1]: *** [drivers/crypto] Error 2 > make: *** [drivers] Error 2 > > This patch eliminates the errors by moving the static constant > zero_entry to the talitos_private structure. As a member of that > structure, zero_entry gets initialized for free (and compatibly) by > the kzalloc() during device probe. Why? A static variable is always zeroed. Please just get rid of the initialisation and it will work. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- 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: Limited usefulness of RSA set key function
On 08/03/2015 11:20 AM, Marcel Holtmann wrote: > actually I think this reasoning needs to be revisited. When I look at this, > this makes no sense whatsoever. The end result is that we have keys in > multiple formats in the kernel and have to convert between them or parse them > again. > > If you do not want to use struct public_key, then lets go for struct key as I > proposed in my other response. That should in the end be able to represent > hardware keys as well. There is really no good reason to move things around > and parse them again or create new formats out of it. > > My take is that we want to store a key once in a single location and single > location only. Storing the same key in different formats in different > locations is just a plain bad idea. Keep it secure and locked in one place. > The approach was not to use any of the existing public key stuff. To be consistent with all the other crypto API tfm needs to "own" the key. The existing struct key is not controlled by the tfm and the key can be gone without tfm knowing. We can change this approach, but this is not my call, but Herbert's. Herbert, any comments? > I hope someone is looking at lib/digsig.c as well and gets this all cleaned > up into a single implementation. However for this to happen, we really need > to get this key stuff figured out. Right now this looks to me like the left > hand does not know what the right hand is doing. Yes, your are right, but crypto/public_key/rsa.c and lib/digsig.c have been added long before akcipher. Now we are trying to get all this fixed, but we need to have stable API before any of it will be converted. Thanks, T -- 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: Limited usefulness of RSA set key function
Hi Tadeusz, >> I already have patches for that actually. The question is just which >> approach to take? >> >> My current proposal is to separate the current crypto_akcipher_setkey into >> two functions. Use the crypto_akcipher_setkey for loading combined private >> and public key formats and crypto_akcipher_setpubkey for just loading the >> public key only format. > > I would prefer to have one setkey function if possible. > >> >> One other option is actually for crypto_akcipher_setkey to use struct >> public_key from include/crypto/public_key.h. I mean we have this all >> defined. Why do we operate on binary blobs for the keys in the first place. >> For example if the key comes from a certificate in the keyring, lets just >> use that data. Most likely the asymmetric key type already decoded it nicely >> for us. No need to do this again. > > That was the first proposal to use this struct, but it was rejected. Looks > like MPIs are not acceptable on the API. > I think it make sense now as it will only be useful for the SW > implementation. For hardware the easiest way is to take > it in this form. actually I think this reasoning needs to be revisited. When I look at this, this makes no sense whatsoever. The end result is that we have keys in multiple formats in the kernel and have to convert between them or parse them again. If you do not want to use struct public_key, then lets go for struct key as I proposed in my other response. That should in the end be able to represent hardware keys as well. There is really no good reason to move things around and parse them again or create new formats out of it. My take is that we want to store a key once in a single location and single location only. Storing the same key in different formats in different locations is just a plain bad idea. Keep it secure and locked in one place. >> I mean there is more than RSA out there and this would allow us to express >> struct public_key_algorithm in addition to the allowed applications of said >> key as well. This would properly also allow us to combine the >> crypto/asymmetric_keys/rsa.c and crypto/rsa.c so that we only have a single >> RSA implementation. Not to mention that we also have lib/digsig.c in the >> kernel. > > The crypto/asymmetric_keys/ will be converted to the new api and > crypto/asymmetric_keys/rsa.c removed soon. I hope someone is looking at lib/digsig.c as well and gets this all cleaned up into a single implementation. However for this to happen, we really need to get this key stuff figured out. Right now this looks to me like the left hand does not know what the right hand is doing. Regards Marcel -- 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: Limited usefulness of RSA set key function
Hi Tadeusz, >>> as you can clearly see. There are two formats defined here. There is no >>> single ASN.1 structure that can decode both of these. >>> >>> It is what it is, RSA Public Key and RSA Private Key formats are two >>> different key formats. And OpenSSL also treats it like this. You can >>> extract the public key from a private key (same way you can extract it from >>> a certificate), but you can not create a private key structure that only >>> contains the public key. >>> >>> For RSA we need to support the two formats as listed above. To make this >>> really easy from an API point of view, I would have setkey and setpubkey >>> function. And also expose them as ALG_SET_KEY and ALG_SET_PUBKEY socket >>> options for AF_ALG. >> >> I'll have a look what will be the easiest way to get the openSSL generated & >> unmodified private key working. > > I already have patches for that actually. The question is just which approach > to take? > > My current proposal is to separate the current crypto_akcipher_setkey into > two functions. Use the crypto_akcipher_setkey for loading combined private > and public key formats and crypto_akcipher_setpubkey for just loading the > public key only format. > > One other option is actually for crypto_akcipher_setkey to use struct > public_key from include/crypto/public_key.h. I mean we have this all defined. > Why do we operate on binary blobs for the keys in the first place. For > example if the key comes from a certificate in the keyring, lets just use > that data. Most likely the asymmetric key type already decoded it nicely for > us. No need to do this again. actually taking this one step further, why don't we integrate directly with the keys. int crypto_akcipher_setkey(struct crypto_akcipher *tfm, struct key *key) Or maybe key_ref_t or key_serial_t depending on what is the preferred method of referencing keys. So for every key loaded via the asymmetric key type, we can a have nice clean way to get the key from various different parts of the kernel and it is also reference counted. It is also by definition cipher agnostic. So this would work for DSA, ECDH or whatever comes next. The only check needed is that the provided key is of type asymmetric and of subtype public key. This means there is no need to parse it twice or store an extra copy of it or invent some new reference counting around struct public_key. And then it would be dead simple to introduce ALG_SET_KEY_ID that takes a key_serial_t for usage with AF_ALG. I have implemented ALG_SET_KEY_ID for skcipher and it works great. However for akcipher this would show even more benefits. Especially when certificates and public keys come from the system keyring or via TPM / UEFI. Regards Marcel -- 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: Limited usefulness of RSA set key function
On 08/03/2015 10:39 AM, Marcel Holtmann wrote: > I already have patches for that actually. The question is just which approach > to take? > > My current proposal is to separate the current crypto_akcipher_setkey into > two functions. Use the crypto_akcipher_setkey for loading combined private > and public key formats and crypto_akcipher_setpubkey for just loading the > public key only format. I would prefer to have one setkey function if possible. > > One other option is actually for crypto_akcipher_setkey to use struct > public_key from include/crypto/public_key.h. I mean we have this all defined. > Why do we operate on binary blobs for the keys in the first place. For > example if the key comes from a certificate in the keyring, lets just use > that data. Most likely the asymmetric key type already decoded it nicely for > us. No need to do this again. That was the first proposal to use this struct, but it was rejected. Looks like MPIs are not acceptable on the API. I think it make sense now as it will only be useful for the SW implementation. For hardware the easiest way is to take it in this form. > > I mean there is more than RSA out there and this would allow us to express > struct public_key_algorithm in addition to the allowed applications of said > key as well. This would properly also allow us to combine the > crypto/asymmetric_keys/rsa.c and crypto/rsa.c so that we only have a single > RSA implementation. Not to mention that we also have lib/digsig.c in the > kernel. The crypto/asymmetric_keys/ will be converted to the new api and crypto/asymmetric_keys/rsa.c removed soon. -- 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: Limited usefulness of RSA set key function
Hi Tadeusz, >> as you can clearly see. There are two formats defined here. There is no >> single ASN.1 structure that can decode both of these. >> >> It is what it is, RSA Public Key and RSA Private Key formats are two >> different key formats. And OpenSSL also treats it like this. You can extract >> the public key from a private key (same way you can extract it from a >> certificate), but you can not create a private key structure that only >> contains the public key. >> >> For RSA we need to support the two formats as listed above. To make this >> really easy from an API point of view, I would have setkey and setpubkey >> function. And also expose them as ALG_SET_KEY and ALG_SET_PUBKEY socket >> options for AF_ALG. > > I'll have a look what will be the easiest way to get the openSSL generated & > unmodified private key working. I already have patches for that actually. The question is just which approach to take? My current proposal is to separate the current crypto_akcipher_setkey into two functions. Use the crypto_akcipher_setkey for loading combined private and public key formats and crypto_akcipher_setpubkey for just loading the public key only format. One other option is actually for crypto_akcipher_setkey to use struct public_key from include/crypto/public_key.h. I mean we have this all defined. Why do we operate on binary blobs for the keys in the first place. For example if the key comes from a certificate in the keyring, lets just use that data. Most likely the asymmetric key type already decoded it nicely for us. No need to do this again. I mean there is more than RSA out there and this would allow us to express struct public_key_algorithm in addition to the allowed applications of said key as well. This would properly also allow us to combine the crypto/asymmetric_keys/rsa.c and crypto/rsa.c so that we only have a single RSA implementation. Not to mention that we also have lib/digsig.c in the kernel. Regards Marcel -- 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: Limited usefulness of RSA set key function
Hi Marcel, On 08/03/2015 12:30 AM, Marcel Holtmann wrote: > as you can clearly see. There are two formats defined here. There is no > single ASN.1 structure that can decode both of these. > > It is what it is, RSA Public Key and RSA Private Key formats are two > different key formats. And OpenSSL also treats it like this. You can extract > the public key from a private key (same way you can extract it from a > certificate), but you can not create a private key structure that only > contains the public key. > > For RSA we need to support the two formats as listed above. To make this > really easy from an API point of view, I would have setkey and setpubkey > function. And also expose them as ALG_SET_KEY and ALG_SET_PUBKEY socket > options for AF_ALG. I'll have a look what will be the easiest way to get the openSSL generated & unmodified private key working. Thanks, T -- 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 6/8] crypto: qat - Convert to new AEAD interface
On 08/01/2015 06:23 PM, Herbert Xu wrote:
> On Fri, Jul 31, 2015 at 01:39:59PM -0700, Tadeusz Struk wrote:
>> On 07/30/2015 02:53 AM, Herbert Xu wrote:
>>> -}, {
>>> + .init = qat_alg_aead_sha512_init,
>>> + .exit = qat_alg_aead_exit,
>>> + .setkey = qat_alg_aead_setkey,
>>> + .decrypt = qat_alg_aead_dec,
>>> + .encrypt = qat_alg_aead_enc,
>>> + .ivsize = AES_BLOCK_SIZE,
>>> + .maxauthsize = SHA512_DIGEST_SIZE,
>>
>> Hi Herbert,
>> crypto_aead_encrypt() and crypto_aead_decrypt() work fine, but
>> crypto_aead_givencrypt(),
>> which is still supported on the API causes a crash. Should we also disable
>> all the
>> crypto_aead_giv* calls? I use it in my internal tests.
>
> givencrypt will be removed once all drivers are converted across.
> So it must not be used.
Hi Herbert,
There is one problem that I missed before.
We can shutdown a qat device and bring it back up via ioct.
Before we shut the last device we also unregister the algorithms.
Then we can bring them up again, but it fails without resetting the flags.
--- 8< ---
QAT algorithms can be registered and unregistered without
removing the qat module via ioctl, therefore they need to
have their flags reset before each register.
diff --git a/drivers/crypto/qat/qat_common/qat_algs.c
b/drivers/crypto/qat/qat_common/qat_algs.c
index 45420a6..1411e4c 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -1183,16 +1183,22 @@ static struct crypto_alg qat_algs[] = { {
int qat_algs_register(void)
{
- int ret = 0;
+ int ret = 0, i;
mutex_lock(&algs_lock);
if (++active_devs != 1)
goto unlock;
+ for (i = 0; i < ARRAY_SIZE(qat_algs); i++)
+ qat_algs[i].cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER |
CRYPTO_ALG_ASYNC;
+
ret = crypto_register_algs(qat_algs, ARRAY_SIZE(qat_algs));
if (ret)
goto unlock;
+ for (i = 0; i < ARRAY_SIZE(qat_aeads); i++)
+ qat_aeads[i].base.cra_flags = CRYPTO_ALG_ASYNC |
CRYPTO_ALG_AEAD_NEW;
+
ret = crypto_register_aeads(qat_aeads, ARRAY_SIZE(qat_aeads));
if (ret)
goto unreg_algs;
--
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
[PATCH v2] crypto: talitos: Remove zero_entry static initializer
Compiling the talitos driver with my GCC 4.3.1 e500v2 cross-compiler
resulted in a failed build due to the anonymous union/structures
introduced in this commit:
crypto: talitos - enhanced talitos_desc struct for SEC1
The build error was:
drivers/crypto/talitos.h:56: error: unknown field 'len' specified in
initializer
drivers/crypto/talitos.h:56: warning: missing braces around initializer
drivers/crypto/talitos.h:56: warning: (near initialization for
'zero_entry.')
drivers/crypto/talitos.h:57: error: unknown field 'j_extent' specified in
initializer
drivers/crypto/talitos.h:58: error: unknown field 'eptr' specified in
initializer
drivers/crypto/talitos.h:58: warning: excess elements in struct initializer
drivers/crypto/talitos.h:58: warning: (near initialization for 'zero_entry')
make[2]: *** [drivers/crypto/talitos.o] Error 1
make[1]: *** [drivers/crypto] Error 2
make: *** [drivers] Error 2
This patch eliminates the errors by moving the static constant
zero_entry to the talitos_private structure. As a member of that
structure, zero_entry gets initialized for free (and compatibly) by
the kzalloc() during device probe.
Signed-off-by: Aaron Sierra
---
drivers/crypto/talitos.c | 14 +++---
drivers/crypto/talitos.h | 8 +---
2 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 83aca95..5347570 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1663,7 +1663,7 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
bool is_sec1 = has_ftr_sec1(priv);
/* first DWORD empty */
- desc->ptr[0] = zero_entry;
+ desc->ptr[0] = priv->zero_entry;
/* cipher iv */
to_talitos_ptr(&desc->ptr[1], edesc->iv_dma, is_sec1);
@@ -1693,7 +1693,7 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
DMA_FROM_DEVICE);
/* last DWORD empty */
- desc->ptr[6] = zero_entry;
+ desc->ptr[6] = priv->zero_entry;
ret = talitos_submit(dev, ctx->ch, desc, callback, areq);
if (ret != -EINPROGRESS) {
@@ -1833,7 +1833,7 @@ static int common_nonsnoop_hash(struct talitos_edesc
*edesc,
bool is_sec1 = has_ftr_sec1(priv);
/* first DWORD empty */
- desc->ptr[0] = zero_entry;
+ desc->ptr[0] = priv->zero_entry;
/* hash context in */
if (!req_ctx->first || req_ctx->swinit) {
@@ -1843,7 +1843,7 @@ static int common_nonsnoop_hash(struct talitos_edesc
*edesc,
DMA_TO_DEVICE);
req_ctx->swinit = 0;
} else {
- desc->ptr[1] = zero_entry;
+ desc->ptr[1] = priv->zero_entry;
/* Indicate next op is not the first. */
req_ctx->first = 0;
}
@@ -1853,7 +1853,7 @@ static int common_nonsnoop_hash(struct talitos_edesc
*edesc,
map_single_talitos_ptr(dev, &desc->ptr[2], ctx->keylen,
(char *)&ctx->key, DMA_TO_DEVICE);
else
- desc->ptr[2] = zero_entry;
+ desc->ptr[2] = priv->zero_entry;
/*
* data in
@@ -1862,7 +1862,7 @@ static int common_nonsnoop_hash(struct talitos_edesc
*edesc,
DMA_TO_DEVICE, &desc->ptr[3]);
/* fifth DWORD empty */
- desc->ptr[4] = zero_entry;
+ desc->ptr[4] = priv->zero_entry;
/* hash/HMAC out -or- hash context out */
if (req_ctx->last)
@@ -1875,7 +1875,7 @@ static int common_nonsnoop_hash(struct talitos_edesc
*edesc,
req_ctx->hw_context, DMA_FROM_DEVICE);
/* last DWORD empty */
- desc->ptr[6] = zero_entry;
+ desc->ptr[6] = priv->zero_entry;
if (is_sec1 && from_talitos_ptr_len(&desc->ptr[3], true) == 0)
talitos_handle_buggy_hash(ctx, edesc, &desc->ptr[3]);
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index 314daf5..153c56a 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -52,13 +52,6 @@ struct talitos_ptr {
__be32 ptr; /* address */
};
-static const struct talitos_ptr zero_entry = {
- .len = 0,
- .j_extent = 0,
- .eptr = 0,
- .ptr = 0
-};
-
/* descriptor */
struct talitos_desc {
__be32 hdr; /* header high bits */
@@ -142,6 +135,7 @@ struct talitos_private {
unsigned int fifo_len;
struct talitos_channel *chan;
+ const struct talitos_ptr zero_entry;
/* next channel to be assigned next incoming descriptor */
atomic_t last_chan cacheline_aligned;
--
1.9.1
--
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: Limited usefulness of RSA set key function
Hi Stephan,
>> It does not. The RSA Private Key has a different format.
>>
>> RSAPrivateKey ::= SEQUENCE {
>> version Version,
>> modulus INTEGER, -- n
>> publicExponentINTEGER, -- e
>> privateExponent INTEGER, -- d
>> prime1INTEGER, -- p
>> prime2INTEGER, -- q
>> exponent1 INTEGER, -- d mod (p-1)
>> exponent2 INTEGER, -- d mod (q-1)
>> coefficient INTEGER, -- (inverse of q) mod p
>> }
>>
>> And honestly that the RSA Public Key magically matches seems more luck then
>> clear intention.
>>
>> RSAPublicKey ::= SEQUENCE {
>> modulus INTEGER, -- n
>> publicExponentINTEGER -- e
>> }
>
> I think here we may have the issue: the ASN.1 structure the kernel uses
> should
> be changed to implement that commonly used ASN.1 structure. If this change
> would allow a DER to be used, I think we have the solution.
as you can clearly see. There are two formats defined here. There is no single
ASN.1 structure that can decode both of these.
It is what it is, RSA Public Key and RSA Private Key formats are two different
key formats. And OpenSSL also treats it like this. You can extract the public
key from a private key (same way you can extract it from a certificate), but
you can not create a private key structure that only contains the public key.
For RSA we need to support the two formats as listed above. To make this really
easy from an API point of view, I would have setkey and setpubkey function. And
also expose them as ALG_SET_KEY and ALG_SET_PUBKEY socket options for AF_ALG.
Regards
Marcel
--
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: Proposal for adding setpubkey callback to akcipher_alg
Hi Stephan,
I think we need to split the akcipher_alg setkey callback into a setkey
and
setpubkey.
diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
index 69d163e39101..ca93952b6d19 100644
--- a/include/crypto/akcipher.h
+++ b/include/crypto/akcipher.h
@@ -91,6 +91,8 @@ struct akcipher_alg {
int (*decrypt)(struct akcipher_request *req);
int (*setkey)(struct crypto_akcipher *tfm, const void *key,
unsigned int keylen);
+ int (*setpubkey)(struct crypto_akcipher *tfm, const void *key,
+unsigned int keylen);
int (*init)(struct crypto_akcipher *tfm);
void (*exit)(struct crypto_akcipher *tfm);
If the cipher actually uses two different formats for the public +
private
>>>
>>> The public key is n + e.
>>>
>>> The private key is n + d.
>>
>> for RSA Public Key it is just n and e. However for RSA Private Key it is n
>> and e and d and also version, primes etc. So the RSA Public Key contains a
>> sequence of 2 integers and the RSA Private Key contains a sequence of 9
>> integers.
>>> Both are encoded in the BER structure the current API requires. It is
>>> perfectly valid to provide only n + e when you do public key operations.
>>
>> And from an API perspective that is fully wrong from my point of view. We
>> just invented another format that is not in any standard. The two standard
>> key formats for RSA are RSA Private Key and RSA Public Key. These are the
>> ones we should support.
>>
>> The format with n plus e and optionally d is total Linux invention as far as
>> I can tell. And I do not want this exposed to userspace.
>>
>> For a clean separation I think splitting this into setkey for the RSA
>> Private Key and setpubkey for the RSA Public Key is pretty obvious choice.
>
> Please define exactly what your pubkey and your privkey contains. Even when I
> think of the DER keys from OpenSSL, we once have n+e and once n+e+d (see
> asn1dump), no?
RSA Private Key is n + e + d (including 6 other fields). RSA Public Key is n +
e (no other fields).
So for RSA you would make setkey to take RSA Private Key and setpubkey to take
RSA Public Key. Meaning you only have to use one of them since if you have the
private key, you always have the public key.
This real difference here is that you can provide the key in two different key
formats. As explained RSA uses two different format.
>>> Please see in the testmgr.h for the 2048 bit key test vector (i.e. the one
>>> with public_key_vec = true). The BER structure has nice comments from
>>> Tadeusz to indicate it only contains n and e without d.
>>
>> And it is totally made up format. Why would you force conversion of a RSA
>
> BER is a made up implementation? I do not think so:
> https://en.wikipedia.org/wiki/Basic_Encoding_Rules
>
> Or do you say that the kernel's implementation of BER is broken?
BER is an encoding format. It does NOT define a key format. You can use BER to
define a key format, but that still means that our defined format that is
currently used for setkey with RSA is made up. It is Linux specific.
The standards for key formats for RSA are RSA Public Key and RSA Private Key.
>> Public Key or RSA Private Key in DER format into this format. This Linux
>> only input format makes it just complicated for no reason. It is also not
>> documented anywhere as I can tell. I had to dig this out of the code and
>> rsakey.asn1.
>>
>> As mentioned above, splitting this into two functions makes this simpler.
>> For all intense and purposes this is akcipher so we always either have
>> public/private key pair or we just have the public key. And at least with
>> RSA they are defined as two independent formats.
>
> I can see that user space (e.g. libkcapi) has such two functions. But
> currently I do not see such distinction necessary in the kernel as mentioned
> above.
Then how do you tell the two key formats apart? Try one, fail, try the other? I
do not like these things. Just tell the kernel clearly what format you have.
Simple and easy.
>> Since the parsing of the key data is not a generic handling, I do not see a
>> good enough reason to invent new formats. Use the format the cipher you
>> implement already has defined.
>>> Thus, I do not currently understand your request. May I ask you to give
>>> more explanation why the use of BER is insufficient?
>>
>> Tell me how you create this Linux specific BER encoded key. I would like
>> someone to provide the magic OpenSSL conversion command line to get this.
>
> Again: there is no DER to BER converter that I am aware of. Agreed, that
> should be there. But currently I do not see that the kernel should do that.
For all intense and purposes DER is valid BER. Why are discussing this?
>> Hand crafting such keys when there is a standard format for RSA Private Key
>> and RSA Public Key makes no s
Re: Limited usefulness of RSA set key function
Am Montag, 3. August 2015, 00:14:28 schrieb Marcel Holtmann:
Hi Marcel,
>
> It does not. The RSA Private Key has a different format.
>
> RSAPrivateKey ::= SEQUENCE {
> version Version,
> modulus INTEGER, -- n
> publicExponentINTEGER, -- e
> privateExponent INTEGER, -- d
> prime1INTEGER, -- p
> prime2INTEGER, -- q
> exponent1 INTEGER, -- d mod (p-1)
> exponent2 INTEGER, -- d mod (q-1)
> coefficient INTEGER, -- (inverse of q) mod p
> }
>
> And honestly that the RSA Public Key magically matches seems more luck then
> clear intention.
>
> RSAPublicKey ::= SEQUENCE {
> modulus INTEGER, -- n
> publicExponentINTEGER -- e
> }
I think here we may have the issue: the ASN.1 structure the kernel uses should
be changed to implement that commonly used ASN.1 structure. If this change
would allow a DER to be used, I think we have the solution.
--
Ciao
Stephan
--
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
Crypto Fixes for 4.2
Hi Linus: This push fixes the following issues: * A bogus BUG_ON in ixp4xx that can be triggered by a dst buffer that is an SG list. * The error handling in hwrngd may cause a crash in case of an error. * Fix a race condition in qat registration when multiple devices are present. Please pull from git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git Herbert Xu (1): crypto: ixp4xx - Remove bogus BUG_ON on scattered dst buffer Martin Schwidefsky (1): hwrng: core - correct error check of kthread_run call Tadeusz Struk (1): crypto: qat - Fix invalid synchronization between register/unregister sym algs drivers/char/hw_random/core.c|2 +- drivers/crypto/ixp4xx_crypto.c |1 - drivers/crypto/qat/qat_common/qat_algs.c | 24 3 files changed, 17 insertions(+), 10 deletions(-) Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- 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: Proposal for adding setpubkey callback to akcipher_alg
Am Montag, 3. August 2015, 00:03:03 schrieb Marcel Holtmann:
Hi Marcel,
> Hi Stephan,
>
> >> I think we need to split the akcipher_alg setkey callback into a setkey
> >> and
> >> setpubkey.
> >>
> >> diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
> >> index 69d163e39101..ca93952b6d19 100644
> >> --- a/include/crypto/akcipher.h
> >> +++ b/include/crypto/akcipher.h
> >> @@ -91,6 +91,8 @@ struct akcipher_alg {
> >>
> >> int (*decrypt)(struct akcipher_request *req);
> >> int (*setkey)(struct crypto_akcipher *tfm, const void *key,
> >>
> >> unsigned int keylen);
> >>
> >> + int (*setpubkey)(struct crypto_akcipher *tfm, const void *key,
> >> +unsigned int keylen);
> >>
> >> int (*init)(struct crypto_akcipher *tfm);
> >> void (*exit)(struct crypto_akcipher *tfm);
> >>
> >> If the cipher actually uses two different formats for the public +
> >> private
> >
> > The public key is n + e.
> >
> > The private key is n + d.
>
> for RSA Public Key it is just n and e. However for RSA Private Key it is n
> and e and d and also version, primes etc. So the RSA Public Key contains a
> sequence of 2 integers and the RSA Private Key contains a sequence of 9
> integers.
> > Both are encoded in the BER structure the current API requires. It is
> > perfectly valid to provide only n + e when you do public key operations.
>
> And from an API perspective that is fully wrong from my point of view. We
> just invented another format that is not in any standard. The two standard
> key formats for RSA are RSA Private Key and RSA Public Key. These are the
> ones we should support.
>
> The format with n plus e and optionally d is total Linux invention as far as
> I can tell. And I do not want this exposed to userspace.
>
> For a clean separation I think splitting this into setkey for the RSA
> Private Key and setpubkey for the RSA Public Key is pretty obvious choice.
Please define exactly what your pubkey and your privkey contains. Even when I
think of the DER keys from OpenSSL, we once have n+e and once n+e+d (see
asn1dump), no?
> > Please see in the testmgr.h for the 2048 bit key test vector (i.e. the one
> > with public_key_vec = true). The BER structure has nice comments from
> > Tadeusz to indicate it only contains n and e without d.
>
> And it is totally made up format. Why would you force conversion of a RSA
BER is a made up implementation? I do not think so:
https://en.wikipedia.org/wiki/Basic_Encoding_Rules
Or do you say that the kernel's implementation of BER is broken?
> Public Key or RSA Private Key in DER format into this format. This Linux
> only input format makes it just complicated for no reason. It is also not
> documented anywhere as I can tell. I had to dig this out of the code and
> rsakey.asn1.
>
> As mentioned above, splitting this into two functions makes this simpler.
> For all intense and purposes this is akcipher so we always either have
> public/private key pair or we just have the public key. And at least with
> RSA they are defined as two independent formats.
I can see that user space (e.g. libkcapi) has such two functions. But
currently I do not see such distinction necessary in the kernel as mentioned
above.
>
> Since the parsing of the key data is not a generic handling, I do not see a
> good enough reason to invent new formats. Use the format the cipher you
> implement already has defined.
> > Thus, I do not currently understand your request. May I ask you to give
> > more explanation why the use of BER is insufficient?
>
> Tell me how you create this Linux specific BER encoded key. I would like
> someone to provide the magic OpenSSL conversion command line to get this.
Again: there is no DER to BER converter that I am aware of. Agreed, that
should be there. But currently I do not see that the kernel should do that.
> Hand crafting such keys when there is a standard format for RSA Private Key
> and RSA Public Key makes no sense whatsoever.
Fully agreed. Thus, a BER encoder is on my agenda for libkcapi.
--
Ciao
Stephan
--
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: Limited usefulness of RSA set key function
Hi Stephan,
>> I have been working with the AF_ALG patches for akcipher lately and I find
>> the RSA set key function way too limited. Especially the fact that it uses a
>> format that I can not find a single reference / standard for worries me.
>>
>> RsaKey ::= SEQUENCE {
>> n INTEGER ({ rsa_get_n }),
>> e INTEGER ({ rsa_get_e }),
>> d INTEGER ({ rsa_get_d })
>> }
>
> Note, the kernel uses BER encoding.
so what? DER is still valid BER.
>> So where is this format coming from? I can find the RSA Public Key format
>> which is a sequence of n and e. If you have a DER encoded RSA public key,
>> then you can use it to encrypt and verify. So that is okay.
>>
>> However if you have a standard Public Key that OpenSSL would create by
>> default, then things do not work since that is actually a more complicated
>> DER encoded format. However in the end, I would expect that we could also
>> load such a key here. The RSA set key function should auto detect it and
>> extract the right information if it is marked as rsaEncryption type.
>
> Do you propose to add a DER parser to the kernel? I feel that the BER parser
> is complex enough. If somebody has a DER key, user space should have the code
> to convert it to BER.
Why would you do that? DER is still valid BER.
>> My biggest concern however is that this does not reassemble the RSA Private
>> Key at all. That key format is a sequence of 9 integers starting with a
>> version. So logically I would expect that I can just set a RSA Private Key
>> and then utilize the encrypt and decrypt features of the RSA cipher.
>
> Why do you think this is not possible? testmgr.h seems to exactly do that.
It does not. The RSA Private Key has a different format.
RSAPrivateKey ::= SEQUENCE {
version Version,
modulus INTEGER, -- n
publicExponentINTEGER, -- e
privateExponent INTEGER, -- d
prime1INTEGER, -- p
prime2INTEGER, -- q
exponent1 INTEGER, -- d mod (p-1)
exponent2 INTEGER, -- d mod (q-1)
coefficient INTEGER, -- (inverse of q) mod p
}
And honestly that the RSA Public Key magically matches seems more luck then
clear intention.
RSAPublicKey ::= SEQUENCE {
modulus INTEGER, -- n
publicExponentINTEGER -- e
}
So what you have in testmgr.h is neither RSA Private Key nor RSA Public Key. It
is a brand new format that is sadly Linux specific.
>> When it comes to exposing RSA via AF_ALG and akcipher, I really want standard
>> format for the set key operation. Asking userspace to construct this Linux
>> kernel only key format is not helpful. You want to be able to just load the
>> RSA Private Key in DER format and be done with it.
>
> I am all for it. BER is a standard, is it not? Yes, I have not yet seen a
> converter for DER to BER (and I have not searched for one either). But I feel
> DER should be left to user space.
DER is valid BER. However just saying the key is BER has no meaning whatsoever.
It does not define an actual key format.
What I would like to see is support for RSA Private Key (DER encoded) and RSA
Public Key (DER encoded) when using the RSA cipher. That makes sense to me.
Everything else is just insanity since we force the user to convert well known
formats into new ones for no apparent reason.
>> Any ideas on how we can fix this to allow a sensible userspace API?
>
> I actually planned to add a BER encoder to my libkcapi. I feel that should be
> right place to provide that code, including a potential DER to BER converter.
No idea what DER to BER would give you since DER is always valid BER. I see no
need for BER or DER encoders if the keys and also certificates are already
available in DER format your system (or can be easily converted from PEM to DER
with standard tools).
Seriously, I want to take the RSA Private Key that comes out of OpenSSL and use
it. I want to be able to extract the RSA Public Key out of a certificate and
use it. Plain and simple. No other gimmicks, conversions or tricks to play.
Using existing key format standards is really the key here.
Regards
Marcel
--
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
[PATCH] crypto: documentation - AEAD API conversion
The AEAD API changes are now reflected in the crypto API doc book. Signed-off-by: Stephan Mueller --- Documentation/DocBook/crypto-API.tmpl | 4 ++-- include/crypto/aead.h | 24 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/Documentation/DocBook/crypto-API.tmpl b/Documentation/DocBook/crypto-API.tmpl index 0992531..8e17a41 100644 --- a/Documentation/DocBook/crypto-API.tmpl +++ b/Documentation/DocBook/crypto-API.tmpl @@ -585,7 +585,7 @@ kernel crypto API| IPSEC Layer +---+| | |(1) | aead| <--- esp_output -| (seqniv) | ---+ +| (seqiv) | ---+ +---+| | (2) +---+| @@ -1687,7 +1687,7 @@ read(opfd, out, outlen); !Pinclude/linux/crypto.h Block Cipher Algorithm Definitions !Finclude/linux/crypto.h crypto_alg !Finclude/linux/crypto.h ablkcipher_alg -!Finclude/linux/crypto.h aead_alg +!Finclude/crypto/aead.h aead_alg !Finclude/linux/crypto.h blkcipher_alg !Finclude/linux/crypto.h cipher_alg !Finclude/crypto/rng.h rng_alg diff --git a/include/crypto/aead.h b/include/crypto/aead.h index 7169ad0..14e35364 100644 --- a/include/crypto/aead.h +++ b/include/crypto/aead.h @@ -45,6 +45,30 @@ * a breach in the integrity of the message. In essence, that -EBADMSG error * code is the key bonus an AEAD cipher has over "standard" block chaining * modes. + * + * Memory Structure: + * + * To support the needs of the most prominent user of AEAD ciphers, namely + * IPSEC, the AEAD ciphers have a special memory layout the caller must adhere + * to. + * + * The scatter list pointing to the input data must contain: + * + * * for RFC4106 ciphers, the concatenation of + * associated authentication data || IV || plaintext or ciphertext. Note, the + * same IV (buffer) is also set with the aead_request_set_crypt call. Note, + * the API call of aead_request_set_ad must provide the length of the AAD and + * the IV. The API call of aead_request_set_crypt only points to the size of + * the input plaintext or ciphertext. + * + * * for "normal" AEAD ciphers, the concatenation of + * associated authentication data || plaintext or ciphertext. + * + * It is important to note that if multiple scatter gather list entries form + * the input data mentioned above, the first entry must not point to a NULL + * buffer. If there is any potential where the AAD buffer can be NULL, the + * calling code must contain a precaution to ensure that this does not result + * in the first scatter gather list entry pointing to a NULL buffer. */ /** -- 2.1.0 -- 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: Proposal for adding setpubkey callback to akcipher_alg
Hi Stephan,
>> I think we need to split the akcipher_alg setkey callback into a setkey and
>> setpubkey.
>>
>> diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
>> index 69d163e39101..ca93952b6d19 100644
>> --- a/include/crypto/akcipher.h
>> +++ b/include/crypto/akcipher.h
>> @@ -91,6 +91,8 @@ struct akcipher_alg {
>> int (*decrypt)(struct akcipher_request *req);
>> int (*setkey)(struct crypto_akcipher *tfm, const void *key,
>> unsigned int keylen);
>> + int (*setpubkey)(struct crypto_akcipher *tfm, const void *key,
>> +unsigned int keylen);
>> int (*init)(struct crypto_akcipher *tfm);
>> void (*exit)(struct crypto_akcipher *tfm);
>>
>> If the cipher actually uses two different formats for the public + private
>
> The public key is n + e.
>
> The private key is n + d.
for RSA Public Key it is just n and e. However for RSA Private Key it is n and
e and d and also version, primes etc. So the RSA Public Key contains a sequence
of 2 integers and the RSA Private Key contains a sequence of 9 integers.
> Both are encoded in the BER structure the current API requires. It is
> perfectly valid to provide only n + e when you do public key operations.
And from an API perspective that is fully wrong from my point of view. We just
invented another format that is not in any standard. The two standard key
formats for RSA are RSA Private Key and RSA Public Key. These are the ones we
should support.
The format with n plus e and optionally d is total Linux invention as far as I
can tell. And I do not want this exposed to userspace.
For a clean separation I think splitting this into setkey for the RSA Private
Key and setpubkey for the RSA Public Key is pretty obvious choice.
> Please see in the testmgr.h for the 2048 bit key test vector (i.e. the one
> with public_key_vec = true). The BER structure has nice comments from Tadeusz
> to indicate it only contains n and e without d.
And it is totally made up format. Why would you force conversion of a RSA
Public Key or RSA Private Key in DER format into this format. This Linux only
input format makes it just complicated for no reason. It is also not documented
anywhere as I can tell. I had to dig this out of the code and rsakey.asn1.
As mentioned above, splitting this into two functions makes this simpler. For
all intense and purposes this is akcipher so we always either have
public/private key pair or we just have the public key. And at least with RSA
they are defined as two independent formats.
Since the parsing of the key data is not a generic handling, I do not see a
good enough reason to invent new formats. Use the format the cipher you
implement already has defined.
> Thus, I do not currently understand your request. May I ask you to give more
> explanation why the use of BER is insufficient?
Tell me how you create this Linux specific BER encoded key. I would like
someone to provide the magic OpenSSL conversion command line to get this. Hand
crafting such keys when there is a standard format for RSA Private Key and RSA
Public Key makes no sense whatsoever.
Regards
Marcel
--
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
