Re: Limited usefulness of RSA set key function

2015-08-02 Thread Stephan Mueller
Am Sonntag, 2. August 2015, 21:16:47 schrieb Marcel Holtmann:

Hi Marcel,

>Hi Tadeusz,
>
>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 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.
>
>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.
>
>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.
>
>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.

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: Proposal for adding setpubkey callback to akcipher_alg

2015-08-02 Thread Stephan Mueller
Am Sonntag, 2. August 2015, 22:28:33 schrieb Marcel Holtmann:

Hi Marcel,

>Hi Tadeusz,
>
>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.

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.

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.

Thus, I do not currently understand your request. May I ask you to give more 
explanation why the use of BER is insufficient?


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


Proposal for adding setpubkey callback to akcipher_alg

2015-08-02 Thread Marcel Holtmann
Hi Tadeusz,

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 key 
data compared to just the public key data, then it is useful to have these 
independent. That way we can use standard formats for the keys and do not have 
to have a Linux kernel specific key format.

My definition would be that setkey sets the private and public key. And the 
setpubkey only sets the public key. So depending on which format of keys you 
have, you call the proper function and it will do the rest for you. At least 
for RSA this solves the problem that I described in my previous email and we 
could use RSA standard ASN.1 formats for each of the key files.

For obvious reasons, when you only call setpubkey, then only encrypt and verify 
will work. However if you call setkey, then you can sign, verify, encrypt and 
decrypt.

When exposing akcipher via AF_ALG, I would also propose to add a ALG_SET_PUBKEY 
so that userspace can clearly tell the kernel which part of the keys it has. 
This would map nicely and we then know which ASN.1 decoder to call instead of 
having to guess what format userspace provided. In case of RSA, the user 
already selected RSA as cipher. So it either has RSA Public Key and would use 
ALG_SET_PUBKEY or it has RSA Private Key and would use ALG_SET_KEY. Since the 
key formats do not describe themselves, I think this is the cleaner solution 
from an API point of view.

On a side note, that the ASN.1 decoder accepts a key with two integers even 
while the format describes three integers seems like a bug in the decoder and 
not a feature. If the third integer is not marked as optional, the decoder 
should just fail the parsing.

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


Limited usefulness of RSA set key function

2015-08-02 Thread Marcel Holtmann
Hi Tadeusz,

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 })
}

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.

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.

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.

Any ideas on how we can fix this to allow a sensible userspace API?

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: fix spelling mistake in dev_err error message

2015-08-02 Thread Colin King
From: Colin Ian King 

Trival change, fix spelling mistake 'aquire' -> 'acquire' in
dev_err message.

Signed-off-by: Colin Ian King 
---
 drivers/crypto/img-hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/img-hash.c b/drivers/crypto/img-hash.c
index ad47d0d..68e8aa9 100644
--- a/drivers/crypto/img-hash.c
+++ b/drivers/crypto/img-hash.c
@@ -334,7 +334,7 @@ static int img_hash_dma_init(struct img_hash_dev *hdev)
 
hdev->dma_lch = dma_request_slave_channel(hdev->dev, "tx");
if (!hdev->dma_lch) {
-   dev_err(hdev->dev, "Couldn't aquire a slave DMA channel.\n");
+   dev_err(hdev->dev, "Couldn't acquire a slave DMA channel.\n");
return -EBUSY;
}
dma_conf.direction = DMA_MEM_TO_DEV;
-- 
2.5.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