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

Richard Levitte levitte at openssl.org
Tue Dec 6 21:30:57 UTC 2016


In message <1481043672.4406.22.camel at HansenPartnership.com> on Tue, 06 Dec 2016 09:01:12 -0800, James Bottomley <James.Bottomley at HansenPartnership.com> said:

James.Bottomley> On Tue, 2016-12-06 at 17:47 +0100, Richard Levitte wrote:
James.Bottomley> > In message <1481042048.4406.14.camel at HansenPartnership.com> on Tue,
James.Bottomley> > 06 Dec 2016 08:34:08 -0800, James Bottomley <
James.Bottomley> > James.Bottomley at HansenPartnership.com> said:
James.Bottomley> > 
James.Bottomley> > James.Bottomley> On Tue, 2016-12-06 at 15:12 +0100, Richard Levitte
James.Bottomley> > wrote:
James.Bottomley> > James.Bottomley> > In message <
James.Bottomley> > 1480697558.2410.33.camel at HansenPartnership.com> on Fri,
James.Bottomley> > James.Bottomley> > 02 Dec 2016 08:52:38 -0800, James Bottomley <
James.Bottomley> > James.Bottomley> > James.Bottomley at HansenPartnership.com> said:
James.Bottomley> > James.Bottomley> > 
James.Bottomley> > James.Bottomley> > When I made that argument, I hadn't thought and
James.Bottomley> > felt things through
James.Bottomley> > James.Bottomley> > entirely.  Truth be told, I'm feeling very uneasy
James.Bottomley> > handing over the
James.Bottomley> > James.Bottomley> > reading and parsing of the PEM file to an engine. 
James.Bottomley> > However, handing
James.Bottomley> > James.Bottomley> > over the decoded data and leaving it to the engine
James.Bottomley> > to do something
James.Bottomley> > James.Bottomley> > sensible with it, no issues at all.
James.Bottomley> > James.Bottomley> 
James.Bottomley> > James.Bottomley> OK, can I pick on this a bit (I'll look at the store
James.Bottomley> > stuff later and
James.Bottomley> > James.Bottomley> reply after I've played with it)?  What is it that
James.Bottomley> > you imagine handing
James.Bottomley> > James.Bottomley> to the engine?  Some type of buffer that contains
James.Bottomley> > the full PEM
James.Bottomley> > James.Bottomley> representation?
James.Bottomley> > 
James.Bottomley> > In my STORE branch, you will find this in
James.Bottomley> > include/openssl/store_file.h:
James.Bottomley> > 
James.Bottomley> >     /*
James.Bottomley> >      * The try_decode function is called to check if the blob of data
James.Bottomley> > can
James.Bottomley> >      * be used by this handler, and if it can, decodes it into a
James.Bottomley> > supported
James.Bottomley> >      * OpenSSL and returns a STORE_INFO with the recorded data.
James.Bottomley> >      * Input:
James.Bottomley> >      *    pem_name:     If this blob comes from a PEM file, this
James.Bottomley> > holds
James.Bottomley> >      *                  the PEM name.  If it comes from another type
James.Bottomley> > of
James.Bottomley> >      *                  file, this is NULL.
James.Bottomley> >      *    blob:         The blob of data to match with what this
James.Bottomley> > handler
James.Bottomley> >      *                  can use.
James.Bottomley> >      *    len:          The length of the blob.
James.Bottomley> >      *    handler_ctx:  For a handler marked repeatable, this pointer
James.Bottomley> > can
James.Bottomley> >      *                  be used to create a context for the handler. 
James.Bottomley> > IT IS
James.Bottomley> >      *                  THE HANDLER'S RESPONSIBILITY TO CREATE AND
James.Bottomley> > DESTROY
James.Bottomley> >      *                  THIS CONTEXT APPROPRIATELY, i.e. create on
James.Bottomley> > first call
James.Bottomley> >      *                  and destroy when about to return NULL.
James.Bottomley> >      *    password_ui:  Application UI method for getting a password.
James.Bottomley> >      *    password_ui_data:
James.Bottomley> >      *                  Application data to be passed to password_ui
James.Bottomley> > when
James.Bottomley> >      *                  it's called.
James.Bottomley> >      * Output:
James.Bottomley> >      *    a STORE_INFO
James.Bottomley> >      */
James.Bottomley> >     typedef STORE_INFO *(*STORE_FILE_try_decode_fn)(const char
James.Bottomley> > *pem_name,
James.Bottomley> >                                                     const unsigned
James.Bottomley> > char *blob,
James.Bottomley> >                                                     size_t len,
James.Bottomley> >                                                     void
James.Bottomley> > **handler_ctx,
James.Bottomley> >                                                     const UI_METHOD
James.Bottomley> > *password_ui,
James.Bottomley> >                                                     void
James.Bottomley> > *password_ui_data);
James.Bottomley> > 
James.Bottomley> > This is the central function that tries to see if it can decode
James.Bottomley> > whatever's thrown at it.  As you can see, it gets handed the PEM name
James.Bottomley> > if it's called as part of a PEM read.  The |blob| is the decoded *and
James.Bottomley> > decrypted* data.
James.Bottomley> > 
James.Bottomley> > James.Bottomley> The reason I chose a BIO is that it's the basic
James.Bottomley> > abstract data handler
James.Bottomley> > James.Bottomley> for openssl. I can hand a buffer to the engine,
James.Bottomley> > sure, but I'd need to
James.Bottomley> > James.Bottomley> transform it to a BIO again (because it would need
James.Bottomley> > PEM parsing and all
James.Bottomley> > James.Bottomley> the PEM parsers speak BIOs), so it feels suboptimal.
James.Bottomley> > 
James.Bottomley> > With a try_decode function, I really do not see why there's a need to
James.Bottomley> > deal with the BIO...
James.Bottomley> 
James.Bottomley> OK, I get what you want.  There are two problems with this.  The first
James.Bottomley> is easy: I'm eventually going to need the BIO header as well because
James.Bottomley> TPM2 keys are going to need to describe external things that aren't in
James.Bottomley> the key representation (and even TPM1 keys might need to say who their
James.Bottomley> parent key is).  However, this is fixable by simply expanding your
James.Bottomley> prototype above to have const char *pem_name and const char *pem_header
James.Bottomley> ... is that OK?
James.Bottomley> 
James.Bottomley> The next problem is that this is slightly harder simply to insert into
James.Bottomley> the PEM code.  The BIO parsing is done in PEM_bytes_read_bio() not
James.Bottomley>  PEM_read_bio_PrivateKey().  The easy way to cope with this would be to
James.Bottomley> move PEM parsing into the ENGINE_find_engine_load_key() function and
James.Bottomley> then hand the name, header, blob to the engine.  The disadvantage of
James.Bottomley> this is that we'll end up pulling the PEM apart twice: once in
James.Bottomley> ENGINE_find_engine_load_key() and then again in PEM_bytes_read_bio(). 
James.Bottomley>  Is this OK?  If it is I can code up a v3.  If not, I can think about
James.Bottomley> how we might integrate this into PEM_bytes_read_bio().

Oh....

I think I aired some thoughts on using PEM headers a very long while
ago, but that never came into fruition, among others because I ended
up doubting that it would be the best way in the long run.

These days, the use of PEM headers is considered old and kinda sorta
deprecated, even though OpenSSL still produces encrypted private key
PEM files that uses headers for the encryption metadata.  It seems
that PKCS#8 is prefered "out there".

So I have to wonder, is PEM really the right way to go for this?
Would it be just as possible to wrap a TSS key with a PKCS#8
container, and use the associated attributes for the external data?
Just a thought, though...  I can't do more than throw around ideas,
considering how little I know about TPM.

That being said, it should certainly be easy enough to change the
appropriate places to make sure headers are available as well, and I
have zero issues adding a header parameter to the try_decode
prototype and associated functions.

One thing I didn't think of earlier is that PEM_bytes_read_bio()
checks the pem name against a known set, *or* in the private key case,
that the pem name ends with " PRIVATE KEY" (which "TSS KEY BLOB" does
not), so some kind of refactoring is needed to accomodate the
store_file_load() call either way.
(quite frankly, I'm slowly realising that the STORE_FILE_HANDLER code
can replace quite a lot of the discovery code in the PEM module, so
refactoring could be in order either way)

Cheers,
Richard

-- 
Richard Levitte         levitte at openssl.org
OpenSSL Project         http://www.openssl.org/~levitte/


More information about the openssl-dev mailing list