Re: [openssl-dev] [RFC 1/2] engine: add new flag based method for loading engine keys

2016-11-18 Thread David Woodhouse
On Thu, 2016-11-17 at 09:33 +0200, Roumen Petrov wrote:
> David Woodhouse wrote:
> > > The assumption in all the current engine code is that key_id can be
> > > passed as something like a file name.
> 
> This is mostly documentation issue.
> Usually OpenSSL man pages use filename for , but actually it is 
> just a string and engine is responsible how to process

Right. In engine_pkcs11 it's a RFC7512 PKCS#11 URI and not a filename.

> > >   There are some new users that
> > > actually want to pass a BIO, so add a new load_key method for
> > > engines
> > > that takes a flag value.
> 
> Engine could use some URN formats for . For instance if  
> starts with file:/ engile could try to load from filesystem.

Note that GnuTLS has a URN format for keys stored in the TPM. See
output of 'tpmtool --list' for example. The TPM engine should probably
accept those.

But this doesn't help with the case where we *have* the actual
(wrapped) key data in memory already — unless you pass in a string
which is a base64-encoded form of that, which is kind of horrid.

-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


Re: [openssl-dev] [RFC 1/2] engine: add new flag based method for loading engine keys

2016-11-17 Thread Roumen Petrov

David Woodhouse wrote:

The assumption in all the current engine code is that key_id can be
passed as something like a file name.

This is mostly documentation issue.
Usually OpenSSL man pages use filename for , but actually it is 
just a string and engine is responsible how to process



  There are some new users that
actually want to pass a BIO, so add a new load_key method for engines
that takes a flag value.
Engine could use some URN formats for . For instance if  
starts with file:/ engile could try to load from filesystem.



The first defined flag is
ENGINE_LOAD_KEY_FLAG_BIO which means that the key_id is actually a bio
pointer.
I'm not sure that is good idea to pass pointers between loadable 
modules. It could be used if there is no alternative. In this case URN 
format for   could inform engine how to load key.


[SNIP]

Regadrs,
Roumen Petrov
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


Re: [openssl-dev] [RFC 1/2] engine: add new flag based method for loading engine keys

2016-11-16 Thread Dr. Stephen Henson
On Wed, Nov 16, 2016, James Bottomley wrote:

> The assumption in all the current engine code is that key_id can be
> passed as something like a file name.

Well no it's a null terminated string whose meaning is engine specific. In
some cases it is a key ID, in others it is a more complex string indicating
multiple parameters.

Steve.
--
Dr Stephen N. Henson. OpenSSL project core developer.
Commercial tech support now available see: http://www.openssl.org
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


Re: [openssl-dev] [RFC 1/2] engine: add new flag based method for loading engine keys

2016-11-16 Thread Salz, Rich
It is a heck of a lot easier for everyone if you make pull requests and not 
just mail big patches.  Can you do that?
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


[openssl-dev] [RFC 1/2] engine: add new flag based method for loading engine keys

2016-11-16 Thread James Bottomley
The assumption in all the current engine code is that key_id can be
passed as something like a file name.  There are some new users that
actually want to pass a BIO, so add a new load_key method for engines
that takes a flag value.  The first defined flag is
ENGINE_LOAD_KEY_FLAG_BIO which means that the key_id is actually a bio
pointer.

Signed-off-by: James Bottomley 
---
 crypto/engine/eng_int.h  |  1 +
 crypto/engine/eng_pkey.c | 38 ++
 crypto/engine/engine.h   | 26 ++
 3 files changed, 65 insertions(+)

diff --git a/crypto/engine/eng_int.h b/crypto/engine/eng_int.h
index 46f163b..b65cc41 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_FLAGS_PTR load_key_flags;
 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..124426f 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_key_flags_function(ENGINE *e,
+  ENGINE_LOAD_KEY_FLAGS_PTR load_f)
+{
+e->load_key_flags = 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_FLAGS_PTR ENGINE_get_load_key_flags_function(const ENGINE *e)
+{
+return e->load_key_flags;
+}
+
 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, const char 
*key_id,
+pem_password_cb *cb, void *cb_data,
+unsigned int flags)
+{
+ENGINE *ep;
+int ret = 0;
+
+for (ep = ENGINE_get_first(); ep != NULL; ep = ENGINE_get_next(ep)) {
+if (!ep->load_key_flags)
+continue;
+if (ep->load_key_flags(ep, pkey, key_id, cb, cb_data, flags) == 1) {
+ret = 1;
+break;
+}
+   if (flags & ENGINE_LOAD_KEY_FLAG_BIO)
+(void)BIO_reset((BIO *)key_id);
+   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..49f6a55 100644
--- a/crypto/engine/engine.h
+++ b/crypto/engine/engine.h
@@ -97,6 +97,7 @@
 # include 
 
 # include 
+# include 
 
 #ifdef  __cplusplus
 extern "C" {
@@ -338,6 +339,19 @@ 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);
+
+/*
+ * This flag signals that the const char *key_id (3rd argument) actually
+ * points to a SSL BIO structure
+ */
+#define ENGINE_LOAD_KEY_FLAG_BIO   0x01
+
+/* Replacement load_key with flags and return code */
+typedef int (*ENGINE_LOAD_KEY_FLAGS_PTR)(ENGINE *, EVP_PKEY **,
+const char *,
+pem_password_cb *pwd_callback,
+void *callback_data,
+unsigned int flags);
 typedef int (*ENGINE_SSL_CLIENT_CERT_PTR) (ENGINE *, SSL *ssl,
STACK_OF(X509_NAME) *ca_dn,
X509 **pcert, EVP_PKEY **pkey,
@@ -565,6 +579,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_key_flags_function(ENGINE *e,
+  ENGINE_LOAD_KEY_FLAGS_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 +627,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_fu

Re: [openssl-dev] [RFC 1/2] engine: add new flag based method for loading engine keys

2016-11-14 Thread David Woodhouse

> The assumption in all the current engine code is that key_id can be
> passed as something like a file name.  There are some new users that
> actually want to pass a BIO, so add a new load_key method for engines
> that takes a flag value.  The first defined flag is
> ENGINE_LOAD_KEY_FLAG_BIO which means that the key_id is actually a bio
> pointer.

I like that this also fixes the UI callback horridness discussed at
http://git.infradead.org/users/dwmw2/openconnect.git/blob/b8d39711:/openssl.c#l423

I like it even more that I can completely remove all mention of the TPM
and the special case to load the engine, and just rely on OpenSSL to Do
The Right Thing when I feed it a PEM file containing -BEGIN TSS KEY
BLOB-, just like GnuTLS does.

-- 
dwmw2

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