[RFC v2 0/2] Proposal for seamless handling of TPM based RSA keys in openssl

classic Classic list List threaded Threaded
55 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[RFC v2 0/2] Proposal for seamless handling of TPM based RSA keys in openssl

James Bottomley
One of the principle problems of using TPM based keys is that there's
no easy way of integrating them with standard file based keys.  This
proposal adds a generic method for handling file based engine keys that
can be loaded as PEM files.  Integration into the PEM loader requires a
BIO based engine API callback which the first patch adds.  The second
patch checks to see if the key can be loaded by any of the present
engines.  Note that this requires that any engine which is to be used
must be present and initialised via openssl.cnf.

I'll also post to this list the patch to openssl_tpm_engine that makes
use if this infrastructure so the integration of the whole can be seen.
 It should also be noted that gnutls has had this functionality since
2012.

The patch was done against 1.0.2h for easier testing and you can try it
and the openssl_tpm_engine out (if you run openSUSE) here:

https://build.opensuse.org/project/show/home:jejb1:Tumbleweed

James

---
v2: make bio callback explicit vi new method rather than overloading
keyid

---

James Bottomley (2):
  engine: add new bio based method for loading engine keys
  pem: load engine keys

 crypto/engine/eng_int.h  |  1 +
 crypto/engine/eng_pkey.c | 38 ++++++++++++++++++++++++++++++++++++++
 crypto/engine/engine.h   | 17 +++++++++++++++++
 crypto/pem/pem_pkey.c    |  4 ++++
 4 files changed, 60 insertions(+)


--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

[RFC v2 1/2] engine: add new bio based method for loading engine keys

James Bottomley
Some engines have a PEM format for their keys, so add a mechanism
whereby these keys can be read in to EVP_PKEY structures backed by the
engine methods.  The expectation is that each engine that wants to use
this will define its own unique guard tags for the PEM file.

Signed-off-by: James Bottomley <[hidden email]>
---
 crypto/engine/eng_int.h  |  1 +
 crypto/engine/eng_pkey.c | 38 ++++++++++++++++++++++++++++++++++++++
 crypto/engine/engine.h   | 17 +++++++++++++++++
 3 files changed, 56 insertions(+)

diff --git a/crypto/engine/eng_int.h b/crypto/engine/eng_int.h
index 46f163b..1d182c8 100644
--- a/crypto/engine/eng_int.h
+++ b/crypto/engine/eng_int.h
@@ -197,6 +197,7 @@ struct engine_st {
     ENGINE_CTRL_FUNC_PTR ctrl;
     ENGINE_LOAD_KEY_PTR load_privkey;
     ENGINE_LOAD_KEY_PTR load_pubkey;
+    ENGINE_LOAD_KEY_BIO_PTR load_privkey_bio;
     ENGINE_SSL_CLIENT_CERT_PTR load_ssl_client_cert;
     const ENGINE_CMD_DEFN *cmd_defns;
     int flags;
diff --git a/crypto/engine/eng_pkey.c b/crypto/engine/eng_pkey.c
index 23580d9..0eb7d6e 100644
--- a/crypto/engine/eng_pkey.c
+++ b/crypto/engine/eng_pkey.c
@@ -64,6 +64,13 @@ int ENGINE_set_load_privkey_function(ENGINE *e,
     return 1;
 }
 
+int ENGINE_set_load_privkey_bio_function(ENGINE *e,
+ ENGINE_LOAD_KEY_BIO_PTR load_f)
+{
+    e->load_privkey_bio = load_f;
+    return 1;
+}
+
 int ENGINE_set_load_pubkey_function(ENGINE *e, ENGINE_LOAD_KEY_PTR loadpub_f)
 {
     e->load_pubkey = loadpub_f;
@@ -88,6 +95,11 @@ ENGINE_LOAD_KEY_PTR ENGINE_get_load_pubkey_function(const ENGINE *e)
     return e->load_pubkey;
 }
 
+ENGINE_LOAD_KEY_BIO_PTR ENGINE_get_load_privkey_bio_function(const ENGINE *e)
+{
+    return e->load_privkey_bio;
+}
+
 ENGINE_SSL_CLIENT_CERT_PTR ENGINE_get_ssl_client_cert_function(const ENGINE
                                                                *e)
 {
@@ -184,3 +196,29 @@ int ENGINE_load_ssl_client_cert(ENGINE *e, SSL *s,
     return e->load_ssl_client_cert(e, s, ca_dn, pcert, ppkey, pother,
                                    ui_method, callback_data);
 }
+
+int ENGINE_find_engine_load_key(ENGINE **e, EVP_PKEY **pkey, BIO *bio,
+                                pem_password_cb *cb, void *cb_data)
+{
+    ENGINE *ep;
+    int ret = 0;
+
+    for (ep = ENGINE_get_first(); ep != NULL; ep = ENGINE_get_next(ep)) {
+        if (!ep->load_privkey_bio)
+            continue;
+        if (ep->load_privkey_bio(ep, pkey, bio, cb, cb_data) == 1) {
+            ret = 1;
+            break;
+        }
+
+ /* reset the bio and clear any error */
+ (void)BIO_reset(bio);
+ ERR_clear_error();
+    }
+    if (e)
+        *e = ep;
+    else if (ep)
+        ENGINE_free(ep);
+
+    return ret;
+}
diff --git a/crypto/engine/engine.h b/crypto/engine/engine.h
index bd7b591..022be41 100644
--- a/crypto/engine/engine.h
+++ b/crypto/engine/engine.h
@@ -97,6 +97,7 @@
 # include <openssl/symhacks.h>
 
 # include <openssl/x509.h>
+# include <openssl/pem.h>
 
 #ifdef  __cplusplus
 extern "C" {
@@ -338,6 +339,11 @@ typedef int (*ENGINE_CTRL_FUNC_PTR) (ENGINE *, int, long, void *,
 typedef EVP_PKEY *(*ENGINE_LOAD_KEY_PTR)(ENGINE *, const char *,
                                          UI_METHOD *ui_method,
                                          void *callback_data);
+
+/* Load key from bio if engine recognises the pem guards */
+typedef int (*ENGINE_LOAD_KEY_BIO_PTR)(ENGINE *, EVP_PKEY **,  BIO *,
+       pem_password_cb *pwd_callback,
+       void *callback_data);
 typedef int (*ENGINE_SSL_CLIENT_CERT_PTR) (ENGINE *, SSL *ssl,
                                            STACK_OF(X509_NAME) *ca_dn,
                                            X509 **pcert, EVP_PKEY **pkey,
@@ -565,6 +571,8 @@ int ENGINE_set_finish_function(ENGINE *e, ENGINE_GEN_INT_FUNC_PTR finish_f);
 int ENGINE_set_ctrl_function(ENGINE *e, ENGINE_CTRL_FUNC_PTR ctrl_f);
 int ENGINE_set_load_privkey_function(ENGINE *e,
                                      ENGINE_LOAD_KEY_PTR loadpriv_f);
+int ENGINE_set_load_privkey_bio_function(ENGINE *e,
+ ENGINE_LOAD_KEY_BIO_PTR loadpriv_f);
 int ENGINE_set_load_pubkey_function(ENGINE *e, ENGINE_LOAD_KEY_PTR loadpub_f);
 int ENGINE_set_load_ssl_client_cert_function(ENGINE *e,
                                              ENGINE_SSL_CLIENT_CERT_PTR
@@ -611,6 +619,7 @@ ENGINE_GEN_INT_FUNC_PTR ENGINE_get_finish_function(const ENGINE *e);
 ENGINE_CTRL_FUNC_PTR ENGINE_get_ctrl_function(const ENGINE *e);
 ENGINE_LOAD_KEY_PTR ENGINE_get_load_privkey_function(const ENGINE *e);
 ENGINE_LOAD_KEY_PTR ENGINE_get_load_pubkey_function(const ENGINE *e);
+ENGINE_LOAD_KEY_BIO_PTR ENGINE_get_load_privkey_bio_function(const ENGINE *e);
 ENGINE_SSL_CLIENT_CERT_PTR ENGINE_get_ssl_client_cert_function(const ENGINE
                                                                *e);
 ENGINE_CIPHERS_PTR ENGINE_get_ciphers(const ENGINE *e);
@@ -671,6 +680,14 @@ int ENGINE_load_ssl_client_cert(ENGINE *e, SSL *s,
                                 UI_METHOD *ui_method, void *callback_data);
 
 /*
+ * Given a bio, this method iterates over all present engines to
+ * see if any can handle it.  It's functionality depends on the engine
+ * implementing e->load_privkey_bio.
+ */
+int ENGINE_find_engine_load_key(ENGINE **e, EVP_PKEY **pkey, BIO *bio,
+                                pem_password_cb *cb, void *cb_data);
+
+/*
  * This returns a pointer for the current ENGINE structure that is (by
  * default) performing any RSA operations. The value returned is an
  * incremented reference, so it should be free'd (ENGINE_finish) before it is
--
2.6.6

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

[RFC v2 2/2] pem: load engine keys

James Bottomley
In reply to this post by James Bottomley
Before trying to process the PEM file, hand it to each of the loaded
engines to see if they recognise the PEM guards.  This uses the new
bio based load key callback, so the engine must be loaded and
implement this callback to be considered.

Signed-off-by: James Bottomley <[hidden email]>
---
 crypto/pem/pem_pkey.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/crypto/pem/pem_pkey.c b/crypto/pem/pem_pkey.c
index 04d6319..e3737f0 100644
--- a/crypto/pem/pem_pkey.c
+++ b/crypto/pem/pem_pkey.c
@@ -85,6 +85,10 @@ EVP_PKEY *PEM_read_bio_PrivateKey(BIO *bp, EVP_PKEY **x, pem_password_cb *cb,
     int slen;
     EVP_PKEY *ret = NULL;
 
+    /* first check to see if an engine can load the PEM */
+    if (ENGINE_find_engine_load_key(NULL, &ret, (const char *)bp, cb, u) == 1)
+        return ret;
+
     if (!PEM_bytes_read_bio(&data, &len, &nm, PEM_STRING_EVP_PKEY, bp, cb, u))
         return NULL;
     p = data;
--
2.6.6

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

Salz, Rich
Thanks for working to improve openssl.

It is probably easier for you to do a GitHub pull request and then have discussion here, pointing to that PR.
And also, before any of this code could be used, we'll need the appropriate CLA.
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

James Bottomley
In reply to this post by James Bottomley
> Thanks for working to improve openssl.

You're welcome.

> It is probably easier for you to do a GitHub pull request and then
> have discussion here, pointing to that PR.

Actually, being a kernel developer, email is far easier.  I'll send a
pull request when everyone's OK with the mechanism, plus it will need
tests and other things.

> And also, before any of this code could be used, we'll need the
> appropriate CLA.

Groan ... since you're changing licences, I don't suppose you'd
consider moving to a DCO model.  It's a whole lot less bureaucratic and
much easier to keep track of.

James

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

Salz, Rich

> Actually, being a kernel developer, email is far easier.  I'll send a pull request
> when everyone's OK with the mechanism, plus it will need tests and other
> things.

Well... okay.  I don't know how the community will react.  But I *do* know that the team prefers things as PR's.


> Groan ... since you're changing licences, I don't suppose you'd consider
> moving to a DCO model.

Sorry, no.  Legal advice and best practices.
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

James Bottomley
On Wed, 2016-11-30 at 16:04 +0000, Salz, Rich wrote:
> > Groan ... since you're changing licences, I don't suppose you'd
> > consider moving to a DCO model.
>
> Sorry, no.  Legal advice and best practices.

Interesting: whose legal advice?  I assumed you were talking to the
SFLC and I thought they were mostly pre-programmed not to recommend
CLAs after their FSF experience.  I'm asking professionally because the
knee jerk reaction of most lawyers is to recommend a CLA and there's a
group programme (run by the LF) to help them kick the habit.

Plus the DCO is industry best practice: even OpenStack is adopting it
after a long struggle.

James

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

Salz, Rich
 
> Plus the DCO is industry best practice: even OpenStack is adopting it after a
> long struggle.

Great.  Good for them.

This is what we're doing.

:)

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

James Bottomley
On Wed, 2016-11-30 at 17:59 +0000, Salz, Rich wrote:

>  
> > Plus the DCO is industry best practice: even OpenStack is adopting
> > it after a
> > long struggle.
>
> Great.  Good for them.
>
> This is what we're doing.
>
> :)

OK, so where is the foundation charter and who are your lawyers?

James

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

Salz, Rich
> OK, so where is the foundation charter and who are your lawyers?

Wow, this seems to have taken a turn to the unfriendly.  I apologize if I added to that.  Sometimes a smiley doesn't wipe out all bad impressions.

The OpenSSL Software Foundation is incorporated in the the state of Delaware, United States, as a non-profit corporation. It does not qualify as a tax-exempt charitable organisation under Section 501(c)(3) of the U.S. Internal Revenue Code.   You can email [hidden email] with questions.
But do note that openssl open source project itself is not governed by those entities, but rather by the collection of individuals known as the development team.  You can find more information by clicking on the "policies" and "community" tab on the website.  

Hope this helps.
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

James Bottomley
On Wed, 2016-11-30 at 19:32 +0000, Salz, Rich wrote:
> > OK, so where is the foundation charter and who are your lawyers?
>
> Wow, this seems to have taken a turn to the unfriendly.  I apologize
> if I added to that.  Sometimes a smiley doesn't wipe out all bad
> impressions.

No, it's standard if you insist on the CLA route:  If you sign a CLA to
an organization, you have to understand what the organization does
before you understand what you're actually committing to: the actual
commitment isn't within the four corners of the CLA.  Your current ICLA
gives effectively an unlimited perpetual licence to do anything with
regard to sublicensing so the scope of that grant is actually governed
by the bylaws and restrictions of the foundation itself because it is
the grant recipient.  For instance, the old OpenStack ICLA was fairly
similar to yours so you had to dig into their bylaws to understand what
they were actually committing to do with the code (basically sublicense
it to all comers under ASL-2).  The structure of the organisation
matters a lot: unlimited grant to a corporate entity under a CLA is
usually a bad idea because they often have nefarious plans to take your
code private via a dual licence, or they might mean well, but their
intentions become nullified if they get taken over.  Foundations are
usually better because their charter often restricts what they can
actually do with the code and what happens to the grant in the event of
dissolution or takeover, which is why reading and understanding the
charter (and possibly the bylaws) is important.

Usually I don't have to ask, all of this is simply available on the web
most of the time.  It's just the openssl foundation doesn't appear to
have any of this online.

I suspect IBM will need to sign a CCLA ... they'll definitely need to
know who your lawyers are.

> The OpenSSL Software Foundation is incorporated in the the state of
> Delaware, United States, as a non-profit corporation. It does not
> qualify as a tax-exempt charitable organisation under Section
> 501(c)(3) of the U.S. Internal Revenue Code.   You can email
> [hidden email] with questions.
> But do note that openssl open source project itself is not governed
> by those entities, but rather by the collection of individuals known
> as the development team.  You can find more information by clicking
> on the "policies" and "community" tab on the website.  

I did check those links ... they don't have any governance information
about the actual openssl foundation that I can find.

James

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

Salz, Rich
> I suspect IBM will need to sign a CCLA ... they'll definitely need to know who
> your lawyers are.

We have a CCLA from IBM; contact Christopher Barrett.

> I did check those links ... they don't have any governance information about
> the actual openssl foundation that I can find.

If you want particular details, email the info address I gave you.  And again, that organization does not specify how the openssl project works.
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 0/2] Proposal for seamless handling of TPM based RSA keys in openssl

Blumenthal, Uri - 0553 - MITLL
In reply to this post by James Bottomley
On 11/30/16, 10:24 AM, "openssl-dev on behalf of James Bottomley" <[hidden email] on behalf of [hidden email]> wrote:

    > One of the principle problems of using TPM based keys is that there's
    > no easy way of integrating them with standard file based keys.

Why should token- and/or TPM-based keys be integrated with file-based keys? OpenSSL and its engines need/should accept URI pointing at the keys. Pointing them at files containing some proprietary reference to keys that are kept in hardware does not seem to make sense.

So why is it better to say “…engine –key /some/weird/path/weird-file.pem” than “…engine –key pkcs11:id=02” (or such)?

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 0/2] Proposal for seamless handling of TPM based RSA keys in openssl

James Bottomley
On Wed, 2016-11-30 at 21:18 +0000, Blumenthal, Uri - 0553 - MITLL
wrote:

> On 11/30/16, 10:24 AM, "openssl-dev on behalf of James Bottomley" <
> [hidden email] on behalf of
> [hidden email]> wrote:
>
>     > One of the principle problems of using TPM based keys is that
> there's
>     > no easy way of integrating them with standard file based keys.
>
> Why should token- and/or TPM-based keys be integrated with file-based
> keys? OpenSSL and its engines need/should accept URI pointing at the
> keys. Pointing them at files containing some proprietary reference to
> keys that are kept in hardware does not seem to make sense.
>
> So why is it better to say “…engine –key /some/weird/path/weird
> -file.pem” than “…engine –key pkcs11:id=02” (or such)?
There appears to be some confusion here.  pkcs11 is a representation
for defined tokens.  However, for TPM, there's also file representation
of an unloaded key (it has to be parented or "wrapped" to one of the
loaded storage keys, usually the SRK).  There is no URI reference
(other than a file one) because any key so described may exist only in
the file and don't have to be part of the tpm token infrastructure.  To
make use of it, the engine first has to load the key into the TPM.
 It's actually a simpler way of handling keys than loading them into
the TPM pkcs11 token infrastructure and naming them by slot and key

The point here is that because there's a pem file representation of the
key, it can be used anywhere a PEM file can be *without* having to tell
openssl what the engine is (the PEM guards being unique to the key
type).

James


--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 0/2] Proposal for seamless handling of TPM based RSA keys in openssl

Blumenthal, Uri - 0553 - MITLL
    >> So why is it better to say “…engine –key /some/weird/path/weird
    >> -file.pem” than “…engine –key pkcs11:id=02” (or such)?
    >
    > There appears to be some confusion here.  pkcs11 is a representation
    > for defined tokens.

Well, I did not mean *specifically* pkcs11 – just as an example of something that currently works.


    > However, for TPM, there's also file representation
    > of an unloaded key (it has to be parented or "wrapped" to one of the
    > loaded storage keys, usually the SRK).

So this PEM wrapping is needed just to load keys into TPM? How do you refer to those keys when they are already loaded?


    > The point here is that because there's a pem file representation of the
    > key, it can be used anywhere a PEM file can be *without* having to tell
    > openssl what the engine is (the PEM guards being unique to the key
    > type).
   
Well, I think I can see your point (except for the above question), but frankly I don’t like this approach very much.
   

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 0/2] Proposal for seamless handling of TPM based RSA keys in openssl

James Bottomley
On Wed, 2016-11-30 at 21:41 +0000, Blumenthal, Uri - 0553 - MITLL
wrote:

>     >> So why is it better to say “…engine –key
> /some/weird/path/weird
>     >> -file.pem” than “…engine –key pkcs11:id=02” (or such)?
>     >
>     > There appears to be some confusion here.  pkcs11 is a
> representation
>     > for defined tokens.
>
> Well, I did not mean *specifically* pkcs11 – just as an example of
> something that currently works.
>
>
>     > However, for TPM, there's also file representation
>     > of an unloaded key (it has to be parented or "wrapped" to one
> of the
>     > loaded storage keys, usually the SRK).
>
> So this PEM wrapping is needed just to load keys into TPM? How do you
> refer to those keys when they are already loaded?
The keys are TPM volatile, so they're effectively unloaded as soon as
you close the TPM connection.  The engine code to use them is here

https://sourceforge.net/p/trousers/openssl_tpm_engine

the TPM connection is from engine init to engine finish and once the
key is loaded, it stays in the TPM until engine finish.  create_tpm_key
is what takes a standard RSA key and wraps it to the SRK as a volatile
key for the TPM.

>
>     > The point here is that because there's a pem file
> representation of the
>     > key, it can be used anywhere a PEM file can be *without* having
> to tell
>     > openssl what the engine is (the PEM guards being unique to the
> key
>     > type).
>    
> Well, I think I can see your point (except for the above question),
> but frankly I don’t like this approach very much.
It's already a mechanism used by gnutls to load TPM keys, so it makes
sense from a compatibility point of view that it should just work for
openssl as well.  It doesn't preclude using a pkcs11 token approach as
well, it's just a different mechanism for handling keys.

I like it because it's easier than trying to set up and use TPM token,
which seems to be very hard without admin privileges (the PEM file
approach can be used by any user of the system provided the SRK has a
well known authority).

James

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

Richard Levitte - VMS Whacker-2
In reply to this post by James Bottomley
This patch doesn't fit the rest...

Generally speaking, I am unsure about your solution. It seems like hack to fit a specific case where something more general could be of greater service to others as well.

Cheers
Richard

On November 30, 2016 4:27:49 PM GMT+01:00, James Bottomley <[hidden email]> wrote:

>Before trying to process the PEM file, hand it to each of the loaded
>engines to see if they recognise the PEM guards.  This uses the new
>bio based load key callback, so the engine must be loaded and
>implement this callback to be considered.
>
>Signed-off-by: James Bottomley <[hidden email]>
>---
> crypto/pem/pem_pkey.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/crypto/pem/pem_pkey.c b/crypto/pem/pem_pkey.c
>index 04d6319..e3737f0 100644
>--- a/crypto/pem/pem_pkey.c
>+++ b/crypto/pem/pem_pkey.c
>@@ -85,6 +85,10 @@ EVP_PKEY *PEM_read_bio_PrivateKey(BIO *bp, EVP_PKEY
>**x, pem_password_cb *cb,
>     int slen;
>     EVP_PKEY *ret = NULL;
>
>+    /* first check to see if an engine can load the PEM */
>+    if (ENGINE_find_engine_load_key(NULL, &ret, (const char *)bp, cb,
>u) == 1)
>+        return ret;
>+
>if (!PEM_bytes_read_bio(&data, &len, &nm, PEM_STRING_EVP_PKEY, bp, cb,
>u))
>         return NULL;
>     p = data;

--
[hidden email]
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

James Bottomley
On Thu, 2016-12-01 at 00:22 +0100, Richard Levitte wrote:
> This patch doesn't fit the rest...

I'm not quite sure I follow why.  To allow engines to load PEM encoded
engine keys in place of machine processed ones, the hook into the
loader has to be in somewhere.  This seems to be the most generic place
to put the hook.

> Generally speaking, I am unsure about your solution. It seems like
> hack to fit a specific case where something more general could be of
> greater service to others as well.

Well, the more adaptable patch set was the previous one that overloaded
the meaning of key_id.  This one has a specific bio mechanism for
loading PEM files, so it only really works for engines that have PEM
representable unloaded keys (which, to be fair, is almost all of them,
since even the USB crypto keys have a wrapped format).

I've tried to make it as generic as possible, but I am conditioned to
think to my use case: TPM keys.  If you give an example of another use
case, it will help me see where it should be more generic.

James


> Cheers
> Richard
>
> On November 30, 2016 4:27:49 PM GMT+01:00, James Bottomley <
> [hidden email]> wrote:
> > Before trying to process the PEM file, hand it to each of the
> > loaded
> > engines to see if they recognise the PEM guards.  This uses the new
> > bio based load key callback, so the engine must be loaded and
> > implement this callback to be considered.
> >
> > Signed-off-by: James Bottomley <[hidden email]>
> > ---
> > crypto/pem/pem_pkey.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/crypto/pem/pem_pkey.c b/crypto/pem/pem_pkey.c
> > index 04d6319..e3737f0 100644
> > --- a/crypto/pem/pem_pkey.c
> > +++ b/crypto/pem/pem_pkey.c
> > @@ -85,6 +85,10 @@ EVP_PKEY *PEM_read_bio_PrivateKey(BIO *bp,
> > EVP_PKEY
> > **x, pem_password_cb *cb,
> >     int slen;
> >     EVP_PKEY *ret = NULL;
> >
> > +    /* first check to see if an engine can load the PEM */
> > +    if (ENGINE_find_engine_load_key(NULL, &ret, (const char *)bp,
> > cb,
> > u) == 1)
> > +        return ret;
> > +
> > if (!PEM_bytes_read_bio(&data, &len, &nm, PEM_STRING_EVP_PKEY, bp,
> > cb,
> > u))
> >         return NULL;
> >     p = data;
>
> --
> [hidden email]

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

Richard Levitte - VMS Whacker-2


James Bottomley <[hidden email]> skrev: (1 december 2016 00:42:09 CET)
>On Thu, 2016-12-01 at 00:22 +0100, Richard Levitte wrote:
>> This patch doesn't fit the rest...
>
>I'm not quite sure I follow why.

It casts bp to const char *. That was for your earlier implementation, wasn't it? It doesn't fit the latest incarnation of your API.

>> Generally speaking, I am unsure about your solution. It seems like
>> hack to fit a specific case where something more general could be of
>> greater service to others as well.
>
>Well, the more adaptable patch set was the previous one that overloaded
>the meaning of key_id.  This one has a specific bio mechanism for
>loading PEM files, so it only really works for engines that have PEM
>representable unloaded keys (which, to be fair, is almost all of them,
>since even the USB crypto keys have a wrapped format).
>
>I've tried to make it as generic as possible, but I am conditioned to
>think to my use case: TPM keys.  If you give an example of another use
>case, it will help me see where it should be more generic.

Among other things, I'd rather not encourage an API that inherently makes the engine<->libcrypto tie even tighter. Also, it puts a requirement on the engine to parse PEM, which is unnecessary.

Also, we already have thoughts on loading keys by uri references, and there may be new ideas and formats coming in the future. All this is tied together and solving it one small hack at a time is sub-optimal in the long run.

I'll repeat myself again, please have a look at the STORE stuff I'm working on. TPM files could very well serve as a first use case.

>James
>
>
>> Cheers
>> Richard
>>
>> On November 30, 2016 4:27:49 PM GMT+01:00, James Bottomley <
>> [hidden email]> wrote:
>> > Before trying to process the PEM file, hand it to each of the
>> > loaded
>> > engines to see if they recognise the PEM guards.  This uses the new
>> > bio based load key callback, so the engine must be loaded and
>> > implement this callback to be considered.
>> >
>> > Signed-off-by: James Bottomley <[hidden email]>
>> > ---
>> > crypto/pem/pem_pkey.c | 4 ++++
>> > 1 file changed, 4 insertions(+)
>> >
>> > diff --git a/crypto/pem/pem_pkey.c b/crypto/pem/pem_pkey.c
>> > index 04d6319..e3737f0 100644
>> > --- a/crypto/pem/pem_pkey.c
>> > +++ b/crypto/pem/pem_pkey.c
>> > @@ -85,6 +85,10 @@ EVP_PKEY *PEM_read_bio_PrivateKey(BIO *bp,
>> > EVP_PKEY
>> > **x, pem_password_cb *cb,
>> >     int slen;
>> >     EVP_PKEY *ret = NULL;
>> >
>> > +    /* first check to see if an engine can load the PEM */
>> > +    if (ENGINE_find_engine_load_key(NULL, &ret, (const char *)bp,
>> > cb,
>> > u) == 1)
>> > +        return ret;
>> > +
>> > if (!PEM_bytes_read_bio(&data, &len, &nm, PEM_STRING_EVP_PKEY, bp,
>> > cb,
>> > u))
>> >         return NULL;
>> >     p = data;
>>
>> --
>> [hidden email]

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC v2 2/2] pem: load engine keys

James Bottomley
On Thu, 2016-12-01 at 01:38 +0100, Richard Levitte wrote:

>
> James Bottomley <[hidden email]> skrev: (1
> december 2016 00:42:09 CET)
> > On Thu, 2016-12-01 at 00:22 +0100, Richard Levitte wrote:
> > > This patch doesn't fit the rest...
> >
> > I'm not quite sure I follow why.
>
> It casts bp to const char *. That was for your earlier
> implementation, wasn't it? It doesn't fit the latest incarnation of
> your API.

Good catch: a leftover I missed the warning for in the spray of other
compile information.  Apparently the debug-linux-x86_64 target doesn't
have a -Werror ... I've added it so I should pick a problem like this
up easily.

> > > Generally speaking, I am unsure about your solution. It seems
> > > like hack to fit a specific case where something more general
> > > could be of greater service to others as well.
> >
> > Well, the more adaptable patch set was the previous one that
> > overloaded the meaning of key_id.  This one has a specific bio
> > mechanism for loading PEM files, so it only really works for
> > engines that have PEM representable unloaded keys (which, to be
> > fair, is almost all of them, since even the USB crypto keys have a
> > wrapped format).
> >
> > I've tried to make it as generic as possible, but I am conditioned
> > to think to my use case: TPM keys.  If you give an example of
> > another use case, it will help me see where it should be more
> > generic.
>
> Among other things, I'd rather not encourage an API that inherently
> makes the engine<->libcrypto tie even tighter. Also, it puts a
> requirement on the engine to parse PEM, which is unnecessary.
>
> Also, we already have thoughts on loading keys by uri references, and
> there may be new ideas and formats coming in the future. All this is
> tied together and solving it one small hack at a time is sub-optimal
> in the long run.
>
> I'll repeat myself again, please have a look at the STORE stuff I'm
> working on. TPM files could very well serve as a first use case.

That's this new pull request:

https://github.com/openssl/openssl/pull/2011/files

?

Just so I understand you, you mean register a store handler from the
engine for the TPM BLOB PEM files so that a reference to the file via
the store can use the PEM file as an EVP_PKEY?

That will work, I'm just not clear from the current pull request how
the store would be integrated with the existing PEM file handler ...
Will every application have to be modified to use the new store, or
will you hook it like I did for the engine keys?

The think I'm looking for is to have a TPM PEM key file just work in
place of a standard RSA private key file.

James


> > James
> >
> >
> > > Cheers
> > > Richard
> > >
> > > On November 30, 2016 4:27:49 PM GMT+01:00, James Bottomley <
> > > [hidden email]> wrote:
> > > > Before trying to process the PEM file, hand it to each of the
> > > > loaded
> > > > engines to see if they recognise the PEM guards.  This uses the
> > > > new
> > > > bio based load key callback, so the engine must be loaded and
> > > > implement this callback to be considered.
> > > >
> > > > Signed-off-by: James Bottomley <[hidden email]>
> > > > ---
> > > > crypto/pem/pem_pkey.c | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/crypto/pem/pem_pkey.c b/crypto/pem/pem_pkey.c
> > > > index 04d6319..e3737f0 100644
> > > > --- a/crypto/pem/pem_pkey.c
> > > > +++ b/crypto/pem/pem_pkey.c
> > > > @@ -85,6 +85,10 @@ EVP_PKEY *PEM_read_bio_PrivateKey(BIO *bp,
> > > > EVP_PKEY
> > > > **x, pem_password_cb *cb,
> > > >     int slen;
> > > >     EVP_PKEY *ret = NULL;
> > > >
> > > > +    /* first check to see if an engine can load the PEM */
> > > > +    if (ENGINE_find_engine_load_key(NULL, &ret, (const char
> > > > *)bp,
> > > > cb,
> > > > u) == 1)
> > > > +        return ret;
> > > > +
> > > > if (!PEM_bytes_read_bio(&data, &len, &nm, PEM_STRING_EVP_PKEY,
> > > > bp,
> > > > cb,
> > > > u))
> > > >         return NULL;
> > > >     p = data;
> > >
> > > --
> > > [hidden email]
>
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
123