Re: Limited usefulness of RSA set key function

2015-08-03 Thread Stephan Mueller
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


Re: Limited usefulness of RSA set key function

2015-08-03 Thread Marcel Holtmann
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: Limited usefulness of RSA set key function

2015-08-03 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: Limited usefulness of RSA set key function

2015-08-03 Thread Marcel Holtmann
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


Re: Limited usefulness of RSA set key function

2015-08-03 Thread Tadeusz Struk
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

2015-08-03 Thread Marcel Holtmann
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

2015-08-03 Thread Tadeusz Struk
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

2015-08-03 Thread Tadeusz Struk
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: Limited usefulness of RSA set key function

2015-08-03 Thread Marcel Holtmann
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

2015-08-03 Thread Marcel Holtmann
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


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