[openssl-dev] [RFC v2 2/2] pem: load engine keys

[Richard Levitte]

James Bottomley <James.Bottomley at HansenPartnership.com> skrev: (1 december 2016 07:36:26 CET)
>On Thu, 2016-12-01 at 01:38 +0100, Richard Levitte wrote:
>> 
>> James Bottomley <James.Bottomley at HansenPartnership.com> 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.

The debug config targets create binaries that can easily be run through a debugger, nothing more. What you're looking for is the configuration option --strict-warnings. 

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

Yes. 

>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?

Essentially, yes. 

>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?

Generally speaking, I was thinking that applications would move to use the STORE API. 

>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.

I get your point. That could be med as a call after the PEM_bytes_read_bio call. At that point, the necessary data for an internal store loading are available. It's still a hack, but if you must... 

>
>James
>
>
>> > James
>> > 
>> > 
>> > > Cheers 
>> > > Richard 
>> > > 
>> > > On November 30, 2016 4:27:49 PM GMT+01:00, James Bottomley <
>> > > James.Bottomley at HansenPartnership.com> 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 <jejb at linux.vnet.ibm.com>
>> > > > ---
>> > > > 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;
>> > > 
>> > > -- 
>> > > levitte at openssl.org 
>> 
>> -- 
>> Sent from my Android device with K-9 Mail. Please excuse my brevity.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.