Re: Proposal for adding setpubkey callback to akcipher_alg

2015-08-04 Thread Marcel Holtmann
Hi Herbert,

 We already have an interface that can handle asymmetric keys and it is easy 
 to extend with new key formats and key types. So lets use that. I can 
 clearly see that after RSA, we get DSA, ECDH etc. So having a simple way to 
 handle these key formats is a good idea. That infrastructure is already in 
 place and easy to extend if needed. Especially with the background that some 
 keys might be actually in hardware or compiled into the kernel, the current 
 asymmetric key interface has the right abstraction. It is also the right 
 abstraction to deal with crypto hardware like TPM or even UEFI.
 
 The crypto API akcipher interface is never going to be used for TPM
 or UEFI.  This is a purely algorithmic interface intended for
 hardware acceleration devices.  If your key is embedded into the
 hardware or otherwise hidden then this is not the interface for you.

I think it actually is the correct interface. And it will still stay a purely 
algorithmic interface. It is just that the algorithm is bound to specific 
hardware with a specific key. I really do not understand your distinction here.

Seriously where is the difference. Lets say you have AES-CCM offload in one PCI 
card and AES-ECB offload in another PCI card. The main job of skcipher here is 
to pick the right engine. So RSA-key1 in one PCI card compared to RSA-key2 in 
another PCI card is pretty much the same concept. So really explain to me where 
you are deriving your difference from.

If the hardware can offload the operation for you, then that is what it is 
doing. Not everything is about speed. Some things are actually about security. 
And treating akcipher the same as skcipher is not going to work. They are two 
different concepts and they are used differently.

Why would you advocate that we duplicate akcipher abstraction once for hardware 
acceleration and once for security key storage operation. At the end of the day 
it is the same abstraction. As I mentioned before, for skcipher the 
acceleration aspects are clear first and foremost. However just extending that 
reasoning blindly to akcipher is short sighted. I think we have a great 
opportunity to create a really powerful and simple API for asymmetric 
cryptography and hardware assisted asymmetric cryptography.

And if you think about how RSA for example is used these days, you spent more 
time loading the certificate and the keys, then you actually spent in the 
cipher operation. It is just the stepping stone for creating the session key 
that you are using with skcipher like AES.

So I do not want to waste time setting up my keys over and over again for a 
single RSA operation. I want to set them up quickly and then run my RSA cipher 
operation. I also want to set them up securely where my private key is protect 
and not copied for every single instance.

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

2015-08-04 Thread Herbert Xu
On Tue, Aug 04, 2015 at 09:33:27PM -0700, Marcel Holtmann wrote:

 I think it actually is the correct interface. And it will still stay a purely 
 algorithmic interface. It is just that the algorithm is bound to specific 
 hardware with a specific key. I really do not understand your distinction 
 here.

The crypto API provides a platform where you can provide hardware
accelerated implementations for pure software algorithms.  Hardware
keys are fundamentally incompatible with that as you cannot do it
in software.

Cheers,
-- 
Email: Herbert Xu herb...@gondor.apana.org.au
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

2015-08-04 Thread Herbert Xu
On Mon, Aug 03, 2015 at 12:25:31AM -0700, Marcel Holtmann wrote:
 
 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.

I don't have a problem with a setpubkey/setprivkey split interface.

However, I'm totally against importing MPI keys which is just silly.
The BER-encoded keys are just raw integers.  Most of the hardware
out there take raw integers.  So it makes no sense to have our
interface take MPIs instead of raw integers, as this would mean
converting into MPIs and then straight back into raw integers
for hardware devices.

Cheers,
-- 
Email: Herbert Xu herb...@gondor.apana.org.au
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

2015-08-04 Thread Marcel Holtmann
Hi Herbert,

 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.
 
 I don't have a problem with a setpubkey/setprivkey split interface.
 
 However, I'm totally against importing MPI keys which is just silly.
 The BER-encoded keys are just raw integers.  Most of the hardware
 out there take raw integers.  So it makes no sense to have our
 interface take MPIs instead of raw integers, as this would mean
 converting into MPIs and then straight back into raw integers
 for hardware devices.


after looking further into this, the whole akcipher setkey should not operate 
on BER-encoded key blobs nor MPIs. Instead it should take a struct key as input 
and reference it.

For skcipher the key as binary blob thing works nicely, but for akcipher this 
is not a good interface. I prefer to have one interface for loading 
public/private keys and not have 2 or more different ways of loading and 
parsing these keys. That will get messy really quick. They are just magnitude 
more complicated than skcipher keys since they come in more complicated 
packaging.

We already have an interface that can handle asymmetric keys and it is easy to 
extend with new key formats and key types. So lets use that. I can clearly see 
that after RSA, we get DSA, ECDH etc. So having a simple way to handle these 
key formats is a good idea. That infrastructure is already in place and easy to 
extend if needed. Especially with the background that some keys might be 
actually in hardware or compiled into the kernel, the current asymmetric key 
interface has the right abstraction. It is also the right abstraction to deal 
with crypto hardware like TPM or even UEFI.

The nice advantage is that keys do not have to be copied around. The struct key 
supports referencing keys and it also ties nicely into process and user 
permissions. So essentially you reference your key for akcipher by struct key 
or from userspace indirectly via key_serial_t. No need to duplicate the key for 
each instance of the akcipher. There is only a single key. Especially when 
handling private keys, I prefer to keep them nicely in one place and not spread 
copies around kernel memory. Duplicating sensitive key material is a bad 
security practice.

It also avoids having to duplicate the ASN.1 parsing over and over again (I 
found 3 places that parse RSA keys at the moment). Have the asymmetric key 
interface parse the key once and stop duplicating this on every single attempt. 
This is especially helpful if your key comes in form of a certificate. We can 
validate the certificate and provide a key reference. More important we can 
also validate it against the kernel system keyring or a hardware provided 
keyring or certificate.

I really want to avoid that we are getting into the business of converting from 
format a to format b to format c because everybody invents their own ones. That 
is a bad API for me. And lets face it, when it comes to public key 
cryptography, there are standards we should follow. If the Linux kernel starts 
making up yet another format and interface, then we are doing a pretty bad job.

With the support for different sub-keytypes, we can really easily optimize the 
handling of struct key for the actual hardware in question. So instead of using 
an interim BER format or MPI or whatever, the keys can be parsed right into the 
correct format of the hardware that will eventually run the offload. This will 
be done once and we can avoid all the extra work.

Further more the important part for me is that when keys are actually bound to 
hardware, meaning we will never see the actual private key in kernel memory, we 
can still reference them. The selection of a hardware provided private key with 
a RSA cipher then can automatically select the right implementation for usage 
of that key. No point in trying a software RSA cipher if the key is only in 
hardware.

If the hardware supports RSA offload and has a private key storage of its own, 
why should that be treated any different than AES hardware engine. It is just 
that its usage is limited to its keys. However the interface of akcipher should 
handle this. If it does not, then we boxed ourselves into a corner and end up 
inventing new interfaces for these kinds of hardware. TPM and UEFI are such 
devices / services already out there. For me they are no different than 
skcipher offload. It is just that they are bound to a key and not just an 
instruction or hardware block.

The keys subsystem with the asymmetric key type should be responsible for 
loading and managing the keys. The crypto subsystem with akcipher 

Re: Proposal for adding setpubkey callback to akcipher_alg

2015-08-03 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


Re: Proposal for adding setpubkey callback to akcipher_alg

2015-08-03 Thread Marcel Holtmann
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 sense whatsoever.
 
 Fully agreed. Thus, a BER encoder is on my agenda for libkcapi.

You are missing my point here. You should not need such a thing. The RSA keys 
are already provided in two well defined DER 

Re: Proposal for adding setpubkey callback to akcipher_alg

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


Re: Proposal for adding setpubkey callback to akcipher_alg

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