Mailing List Archive

[PATCH v2 2/2] Add support for openssl engine based keys
Engine keys are keys whose file format is understood by a specific
engine rather than by openssl itself. Since these keys are file
based, the pkcs11 interface isn't appropriate for them because they
don't actually represent tokens. The current most useful engine for
openssh keys are the TPM engines, which allow all private keys to be
stored in a form only the TPM hardware can decode, making them
impossible to steal. The design of engine keys is that while they
have to be loaded using a different openssl API, they can be used as
standard openssl EVP_PKEYs. The only difficulty for openssh is that
the private components can't be serialised, so a new agent command is
introduced which interprets a message giving the location of the file
and invokes the correct engine API from with ssh-agent.

Since engine keys are drop in replacements for openssl keys, any ssh
key that can be converted to openssl form can also be converted to an
engine key. To convert an existing ssh key to engine form first
convert to PKCS8 and then pass that output into the engine conversion
command. So to convert a private key stored in file rsa to TPM engine
format, you do

ssh-keygen -p -m PKCS8 -f rsa
create_tpm2_key -w rsa rsa.tpm

Then to use the TPM key simply mv rsa.tpm rsa and openssh will be able
to use this key using the -o option to specify the engine:

ssh -i rsa user@host

Note that engines usually have specific limits on the type of keys
they accept (so TPM 2.0 usually only does 2048 bits for RSA and NIST
elliptic curves) so not all existing openssh keys can be converted to
engine keys.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
Makefile.in | 2 +-
authfd.c | 44 ++++++++++++++
authfd.h | 6 ++
ssh-add.c | 36 ++++++++++++
ssh-agent.c | 74 ++++++++++++++++++++++++
ssh-engine.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++
ssh-engine.h | 9 +++
7 files changed, 329 insertions(+), 1 deletion(-)
create mode 100644 ssh-engine.c
create mode 100644 ssh-engine.h

diff --git a/Makefile.in b/Makefile.in
index c9e4294d3..3260d2186 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -136,7 +136,7 @@ SCP_OBJS= scp.o progressmeter.o

SSHADD_OBJS= ssh-add.o $(SKOBJS)

-SSHAGENT_OBJS= ssh-agent.o ssh-pkcs11-client.o $(SKOBJS)
+SSHAGENT_OBJS= ssh-agent.o ssh-pkcs11-client.o ssh-engine.o $(SKOBJS)

SSHKEYGEN_OBJS= ssh-keygen.o sshsig.o $(SKOBJS)

diff --git a/authfd.c b/authfd.c
index 4b647a628..6022a24ec 100644
--- a/authfd.c
+++ b/authfd.c
@@ -567,6 +567,50 @@ ssh_remove_identity(int sock, struct sshkey *key)
return r;
}

+/*
+ * Add an engine based identity
+ */
+#ifdef USE_OPENSSL_ENGINE
+int
+ssh_add_engine_key(int sock, const char *file, const char *pin,
+ u_int lifetime, u_int confirm, u_int maxsign)
+{
+ struct sshbuf *msg;
+ int r, constrained = (lifetime || confirm);
+ u_char type = constrained ? SSH_AGENTC_ADD_ENGINE_KEY_CONSTRAINED :
+ SSH_AGENTC_ADD_ENGINE_KEY;
+
+ msg = sshbuf_new();
+ if (!msg)
+ return SSH_ERR_ALLOC_FAIL;
+ r = sshbuf_put_u8(msg, type);
+ if (r)
+ goto out;
+ r = sshbuf_put_cstring(msg, file);
+ if (r)
+ goto out;
+ r = sshbuf_put_cstring(msg, pin);
+ if (r)
+ goto out;
+ if (constrained) {
+ r = encode_constraints(msg, lifetime, confirm, maxsign, NULL);
+ if (r)
+ goto out;
+ }
+ r = ssh_request_reply(sock, msg, msg);
+ if (r)
+ goto out;
+ r = sshbuf_get_u8(msg, &type);
+ if (r)
+ goto out;
+ r = (signed char)type;
+ out:
+ sshbuf_free(msg);
+ return r;
+}
+#endif
+
+
/*
* Add/remove an token-based identity from the authentication server.
* This call is intended only for use by ssh-add(1) and like applications.
diff --git a/authfd.h b/authfd.h
index c3bf6259a..1bf42b4e6 100644
--- a/authfd.h
+++ b/authfd.h
@@ -38,6 +38,8 @@ int ssh_remove_identity(int sock, struct sshkey *key);
int ssh_update_card(int sock, int add, const char *reader_id,
const char *pin, u_int life, u_int confirm);
int ssh_remove_all_identities(int sock, int version);
+int ssh_add_engine_key(int sock, const char *file, const char *pin,
+ u_int lifetime, u_int confirm, u_int maxsign);

int ssh_agent_sign(int sock, const struct sshkey *key,
u_char **sigp, size_t *lenp,
@@ -76,6 +78,10 @@ int ssh_agent_sign(int sock, const struct sshkey *key,
#define SSH2_AGENTC_ADD_ID_CONSTRAINED 25
#define SSH_AGENTC_ADD_SMARTCARD_KEY_CONSTRAINED 26

+/* engine keys */
+#define SSH_AGENTC_ADD_ENGINE_KEY 27
+#define SSH_AGENTC_ADD_ENGINE_KEY_CONSTRAINED 28
+
#define SSH_AGENT_CONSTRAIN_LIFETIME 1
#define SSH_AGENT_CONSTRAIN_CONFIRM 2
#define SSH_AGENT_CONSTRAIN_MAXSIGN 3
diff --git a/ssh-add.c b/ssh-add.c
index a40198ab5..b0ce393c7 100644
--- a/ssh-add.c
+++ b/ssh-add.c
@@ -110,6 +110,31 @@ clear_pass(void)
}
}

+#ifdef USE_OPENSSL_ENGINE
+static int
+add_engine_key(int agent_fd, const char *file)
+{
+ int ret;
+ char *pin = NULL;
+
+ ret = ssh_add_engine_key(agent_fd, file, NULL, lifetime, confirm, maxsign);
+ if (ret == SSH_ERR_KEY_WRONG_PASSPHRASE) {
+ pin = read_passphrase("Enter engine key passphrase:", RP_ALLOW_STDIN);
+ if (!pin)
+ return -1;
+ ret = ssh_add_engine_key(agent_fd, file, pin, lifetime, confirm, maxsign);
+ }
+ if (ret != SSH_AGENT_SUCCESS) {
+ fprintf(stderr, "failed to add engine key: %s\n", ssh_err(ret));
+ } else {
+ fprintf(stderr, "Engine Identity added: %s\n", file);
+ }
+ if (pin)
+ free (pin);
+ return ret;
+}
+#endif
+
static int
delete_file(int agent_fd, const char *filename, int key_only, int qflag)
{
@@ -235,6 +260,17 @@ add_file(int agent_fd, const char *filename, int key_only, int qflag,
/* At first, try empty passphrase */
if ((r = sshkey_parse_private_fileblob(keyblob, "", &private,
&comment)) != 0 && r != SSH_ERR_KEY_WRONG_PASSPHRASE) {
+#ifdef USE_OPENSSL_ENGINE
+ int n_r = add_engine_key(agent_fd, filename);
+
+ if (n_r == SSH_AGENT_SUCCESS) {
+ clear_pass();
+ sshbuf_free(keyblob);
+ return 0;
+ } else if (n_r != SSH_ERR_INTERNAL_ERROR) {
+ r = n_r;
+ }
+#endif
fprintf(stderr, "Error loading key \"%s\": %s\n",
filename, ssh_err(r));
goto fail_load;
diff --git a/ssh-agent.c b/ssh-agent.c
index e081413b8..665f7a47e 100644
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -92,6 +92,10 @@
#include "ssh-pkcs11.h"
#include "sk-api.h"

+#ifdef USE_OPENSSL_ENGINE
+#include "ssh-engine.h"
+#endif
+
#ifndef DEFAULT_PROVIDER_WHITELIST
# define DEFAULT_PROVIDER_WHITELIST "/usr/lib*/*,/usr/local/lib*/*"
#endif
@@ -639,6 +643,70 @@ no_identities(SocketEntry *e)
sshbuf_free(msg);
}

+#ifdef USE_OPENSSL_ENGINE
+static void
+process_add_engine_key(SocketEntry *e)
+{
+ char *pin, *file, *comment;
+ int r, confirm = 0;
+ u_int seconds;
+ time_t death = 0;
+ u_char type;
+ struct sshkey *k;
+ Identity *id;
+
+ if ((r = sshbuf_get_cstring(e->request, &file, NULL)) != 0 ||
+ (r = sshbuf_get_cstring(e->request, &pin, NULL)) != 0)
+ fatal("%s: buffer error: %s", __func__, ssh_err(r));
+
+ while (sshbuf_len(e->request)) {
+ if ((r = sshbuf_get_u8(e->request, &type)) != 0)
+ fatal("%s: buffer error: %s", __func__, ssh_err(r));
+ switch (type) {
+ case SSH_AGENT_CONSTRAIN_LIFETIME:
+ if ((r = sshbuf_get_u32(e->request, &seconds)) != 0)
+ fatal("%s: buffer error: %s",
+ __func__, ssh_err(r));
+ death = monotime() + seconds;
+ break;
+ case SSH_AGENT_CONSTRAIN_CONFIRM:
+ confirm = 1;
+ break;
+ default:
+ error("%s: Unknown constraint type %d", __func__, type);
+ goto send;
+ }
+ }
+ if (lifetime && !death)
+ death = monotime() + lifetime;
+
+ if ((r = engine_process_add(file, pin, &k, &comment)) < 0)
+ goto send;
+
+ r = SSH_AGENT_SUCCESS;
+ if (lookup_identity(k) == NULL) {
+ id = xcalloc(1, sizeof(Identity));
+ id->key = k;
+ id->comment = comment;
+ id->death = death;
+ id->confirm = confirm;
+ TAILQ_INSERT_TAIL(&idtab->idlist, id, next);
+ idtab->nentries++;
+ } else {
+ /* key is already present, just return success */
+ sshkey_free(k);
+ }
+
+send:
+ free(pin);
+ free(file);
+ /* open code send_status because need to return actual error */
+ if (sshbuf_put_u32(e->output, 1) != 0 ||
+ sshbuf_put_u8(e->output, r) != 0)
+ fatal("%s: buffer error", __func__);
+}
+#endif /* USE_OPENSSL_ENGINE */
+
#ifdef ENABLE_PKCS11
static void
process_add_smartcard_key(SocketEntry *e)
@@ -859,6 +927,12 @@ process_message(u_int socknum)
process_remove_smartcard_key(e);
break;
#endif /* ENABLE_PKCS11 */
+#ifdef USE_OPENSSL_ENGINE
+ case SSH_AGENTC_ADD_ENGINE_KEY:
+ case SSH_AGENTC_ADD_ENGINE_KEY_CONSTRAINED:
+ process_add_engine_key(e);
+ break;
+#endif /* USE_OPENSSL_ENGINE */
default:
/* Unknown message. Respond with failure. */
error("Unknown message %d", type);
diff --git a/ssh-engine.c b/ssh-engine.c
new file mode 100644
index 000000000..2263c8e7d
--- /dev/null
+++ b/ssh-engine.c
@@ -0,0 +1,159 @@
+#include "includes.h"
+
+#include <string.h>
+
+#include <openssl/engine.h>
+#include <openssl/evp.h>
+#include <openssl/ui.h>
+
+#include "authfile.h"
+#include "log.h"
+#include "ssh-engine.h"
+#include "sshkey.h"
+#include "ssherr.h"
+#include "xmalloc.h"
+
+#ifdef USE_OPENSSL_ENGINE
+
+struct ui_data {
+ char *passphrase;
+ int ret;
+};
+
+static int
+ui_read(UI *ui, UI_STRING *uis)
+{
+ struct ui_data *d = UI_get0_user_data(ui);
+ d->ret = 0;
+
+ if (UI_get_string_type(uis) == UIT_PROMPT) {
+ if (d->passphrase == NULL || d->passphrase[0] == '\0') {
+ /* we sent no passphrase but get asked for one
+ * send an interrupt event to avoid DA implications */
+ d->ret = -2;
+ } else {
+ UI_set_result(ui, uis, d->passphrase);
+ d->ret = 1;
+ }
+ }
+
+ return d->ret;
+}
+
+static int
+engine_process_add_internal(ENGINE *e, char *file, char *pin,
+ struct sshkey **k)
+{
+ EVP_PKEY *pk;
+ int ret;
+ UI_METHOD *ui;
+ EVP_PKEY_CTX *ctx;
+ char hash[SHA256_DIGEST_LENGTH], result[1024];
+ size_t siglen;
+ const char *engine = ENGINE_get_name(e);
+ struct ui_data d;
+
+ verbose("%s: add provider=%s, key=%s", __func__, engine, file);
+
+ ret = SSH_ERR_INTERNAL_ERROR;
+
+ ui = UI_create_method("ssh-agent password writer");
+ if (!ui) {
+ verbose("%s: failed to create UI method", __func__);
+ ERR_print_errors_fp(stderr);
+ return ret;
+ }
+ UI_method_set_reader(ui, ui_read);
+
+ if (!ENGINE_init(e)) {
+ verbose("%s: failed to init engine %s", __func__, engine);
+ ERR_print_errors_fp(stderr);
+ return ret;
+ }
+
+ d.passphrase = pin;
+ d.ret = 0;
+ pk = ENGINE_load_private_key(e, file, ui, &d);
+ ENGINE_finish(e);
+
+ if (d.ret == -2) {
+ ret = SSH_ERR_KEY_WRONG_PASSPHRASE;
+ ERR_clear_error();
+ goto err_free_pkey;
+ }
+
+ if (!pk) {
+ verbose("%s: engine returned no key", __func__);
+ ERR_print_errors_fp(stderr);
+ return ret;
+ }
+
+ /* here's a nasty problem: most engines don't tell us the password
+ * was wrong until we try to use the key, so do a test to see */
+ ctx = EVP_PKEY_CTX_new(pk, NULL);
+ if (!ctx) {
+ verbose("%s: openssl context allocation failed", __func__);
+ ERR_print_errors_fp(stderr);
+ goto err_free_pkey;
+ }
+
+ EVP_PKEY_sign_init(ctx);
+
+ siglen=sizeof(result);
+ ret = EVP_PKEY_sign(ctx, result, &siglen, hash, sizeof(hash));
+ EVP_PKEY_CTX_free(ctx);
+
+ if (ret != 1 || siglen == 0) {
+ verbose("%s: trial signature failed with %d", __func__, ret);
+ ERR_print_errors_fp(stderr);
+ ret = SSH_ERR_KEY_WRONG_PASSPHRASE;
+ goto err_free_pkey;
+ }
+
+ ret = sshkey_openssl_private_key(KEY_UNSPEC, pk, k, 1);
+
+ err_free_pkey:
+ EVP_PKEY_free(pk);
+ verbose("%s: returning %d", __func__, ret);
+ return ret;
+}
+
+static void
+engine_get_comment(char *file, char **comment)
+{
+ struct sshkey *kp;
+
+ if (!comment)
+ return;
+
+ if (sshkey_load_public(file, &kp, comment) < 0)
+ *comment = xstrdup(file);
+ else
+ sshkey_free(kp);
+}
+
+int
+engine_process_add(char *file, char *pin, struct sshkey **k, char **comment)
+{
+ ENGINE *e;
+
+ for (e = ENGINE_get_first(); e != NULL; e = ENGINE_get_next(e)) {
+ int ret;
+
+ if (!ENGINE_get_load_privkey_function(e))
+ continue;
+
+ ret = engine_process_add_internal(e, file, pin, k);
+
+ if (ret == SSH_ERR_KEY_WRONG_PASSPHRASE) {
+ return ret;
+ } else if (ret == 0) {
+ engine_get_comment(file, comment);
+ return ret;
+ }
+ }
+
+ return SSH_ERR_INTERNAL_ERROR;
+}
+
+#endif
diff --git a/ssh-engine.h b/ssh-engine.h
new file mode 100644
index 000000000..a1310dc90
--- /dev/null
+++ b/ssh-engine.h
@@ -0,0 +1,9 @@
+#ifndef _ENGINE_H
+#define _ENGINE_H
+
+#include "sshkey.h"
+
+int
+engine_process_add(char *file, char *pin, struct sshkey **k, char **comment);
+
+#endif
--
2.26.2

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: [PATCH v2 2/2] Add support for openssl engine based keys [ In reply to ]
Hello,
thank you for the patch. It looks good on a fast read-through.

The ssh-agent protocol modification you propose would require addition
to the draft specification [1]. This is why I used in my initial
implementation of PKCS#11 URI few years back the same messages as are
used today, SSH_AGENTC_ADD_SMARTCARD_KEY, but used PKCS#11 URI instead
of the "opaque id" as described in the specs, but I understand that it
might be cumbersome in this case.

Reading also through the rest of the patches, I see you added support
for pkcs11 uris only to the ssh-agent interface. When we should support
this, we should support also input to ssh (through command-line and
configuration) and ssh-keygen (to get public keys from engine) too.

Just out of curiosity, do the keys in agent work when after removing
and reinserting smart card? The current implementation does not handle
this at all [3] causing headaches for anyone interested in using this.

[1] https://tools.ietf.org/html/draft-miller-ssh-agent-04
[2] https://github.com/Jakuje/openssh-portable/commits/jjelen-pkcs11
[3] https://bugzilla.mindrot.org/show_bug.cgi?id=2890

Regards,
Jakub

On Tue, 2020-06-09 at 12:36 -0700, James Bottomley wrote:
> Engine keys are keys whose file format is understood by a specific
> engine rather than by openssl itself. Since these keys are file
> based, the pkcs11 interface isn't appropriate for them because they
> don't actually represent tokens. The current most useful engine for
> openssh keys are the TPM engines, which allow all private keys to be
> stored in a form only the TPM hardware can decode, making them
> impossible to steal. The design of engine keys is that while they
> have to be loaded using a different openssl API, they can be used as
> standard openssl EVP_PKEYs. The only difficulty for openssh is that
> the private components can't be serialised, so a new agent command is
> introduced which interprets a message giving the location of the file
> and invokes the correct engine API from with ssh-agent.
>
> Since engine keys are drop in replacements for openssl keys, any ssh
> key that can be converted to openssl form can also be converted to an
> engine key. To convert an existing ssh key to engine form first
> convert to PKCS8 and then pass that output into the engine conversion
> command. So to convert a private key stored in file rsa to TPM
> engine
> format, you do
>
> ssh-keygen -p -m PKCS8 -f rsa
> create_tpm2_key -w rsa rsa.tpm
>
> Then to use the TPM key simply mv rsa.tpm rsa and openssh will be
> able
> to use this key using the -o option to specify the engine:
>
> ssh -i rsa user@host
>
> Note that engines usually have specific limits on the type of keys
> they accept (so TPM 2.0 usually only does 2048 bits for RSA and NIST
> elliptic curves) so not all existing openssh keys can be converted to
> engine keys.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com
> >
> ---
> Makefile.in | 2 +-
> authfd.c | 44 ++++++++++++++
> authfd.h | 6 ++
> ssh-add.c | 36 ++++++++++++
> ssh-agent.c | 74 ++++++++++++++++++++++++
> ssh-engine.c | 159
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> ssh-engine.h | 9 +++
> 7 files changed, 329 insertions(+), 1 deletion(-)
> create mode 100644 ssh-engine.c
> create mode 100644 ssh-engine.h
>
> diff --git a/Makefile.in b/Makefile.in
> index c9e4294d3..3260d2186 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -136,7 +136,7 @@ SCP_OBJS= scp.o progressmeter.o
>
> SSHADD_OBJS= ssh-add.o $(SKOBJS)
>
> -SSHAGENT_OBJS= ssh-agent.o ssh-pkcs11-client.o $(SKOBJS)
> +SSHAGENT_OBJS= ssh-agent.o ssh-pkcs11-client.o ssh-engine.o
> $(SKOBJS)
>
> SSHKEYGEN_OBJS= ssh-keygen.o sshsig.o $(SKOBJS)
>
> diff --git a/authfd.c b/authfd.c
> index 4b647a628..6022a24ec 100644
> --- a/authfd.c
> +++ b/authfd.c
> @@ -567,6 +567,50 @@ ssh_remove_identity(int sock, struct sshkey
> *key)
> return r;
> }
>
> +/*
> + * Add an engine based identity
> + */
> +#ifdef USE_OPENSSL_ENGINE
> +int
> +ssh_add_engine_key(int sock, const char *file, const char *pin,
> + u_int lifetime, u_int confirm, u_int maxsign)
> +{
> + struct sshbuf *msg;
> + int r, constrained = (lifetime || confirm);
> + u_char type = constrained ?
> SSH_AGENTC_ADD_ENGINE_KEY_CONSTRAINED :
> + SSH_AGENTC_ADD_ENGINE_KEY;
> +
> + msg = sshbuf_new();
> + if (!msg)
> + return SSH_ERR_ALLOC_FAIL;
> + r = sshbuf_put_u8(msg, type);
> + if (r)
> + goto out;
> + r = sshbuf_put_cstring(msg, file);
> + if (r)
> + goto out;
> + r = sshbuf_put_cstring(msg, pin);
> + if (r)
> + goto out;
> + if (constrained) {
> + r = encode_constraints(msg, lifetime, confirm, maxsign,
> NULL);
> + if (r)
> + goto out;
> + }
> + r = ssh_request_reply(sock, msg, msg);
> + if (r)
> + goto out;
> + r = sshbuf_get_u8(msg, &type);
> + if (r)
> + goto out;
> + r = (signed char)type;
> + out:
> + sshbuf_free(msg);
> + return r;
> +}
> +#endif
> +
> +
> /*
> * Add/remove an token-based identity from the authentication
> server.
> * This call is intended only for use by ssh-add(1) and like
> applications.
> diff --git a/authfd.h b/authfd.h
> index c3bf6259a..1bf42b4e6 100644
> --- a/authfd.h
> +++ b/authfd.h
> @@ -38,6 +38,8 @@ int ssh_remove_identity(int sock, struct sshkey
> *key);
> int ssh_update_card(int sock, int add, const char *reader_id,
> const char *pin, u_int life, u_int confirm);
> int ssh_remove_all_identities(int sock, int version);
> +int ssh_add_engine_key(int sock, const char *file, const char *pin,
> + u_int lifetime, u_int confirm, u_int
> maxsign);
>
> int ssh_agent_sign(int sock, const struct sshkey *key,
> u_char **sigp, size_t *lenp,
> @@ -76,6 +78,10 @@ int ssh_agent_sign(int sock, const struct
> sshkey *key,
> #define SSH2_AGENTC_ADD_ID_CONSTRAINED 25
> #define SSH_AGENTC_ADD_SMARTCARD_KEY_CONSTRAINED 26
>
> +/* engine keys */
> +#define SSH_AGENTC_ADD_ENGINE_KEY 27
> +#define SSH_AGENTC_ADD_ENGINE_KEY_CONSTRAINED 28
> +
> #define SSH_AGENT_CONSTRAIN_LIFETIME 1
> #define SSH_AGENT_CONSTRAIN_CONFIRM 2
> #define SSH_AGENT_CONSTRAIN_MAXSIGN 3
> diff --git a/ssh-add.c b/ssh-add.c
> index a40198ab5..b0ce393c7 100644
> --- a/ssh-add.c
> +++ b/ssh-add.c
> @@ -110,6 +110,31 @@ clear_pass(void)
> }
> }
>
> +#ifdef USE_OPENSSL_ENGINE
> +static int
> +add_engine_key(int agent_fd, const char *file)
> +{
> + int ret;
> + char *pin = NULL;
> +
> + ret = ssh_add_engine_key(agent_fd, file, NULL, lifetime,
> confirm, maxsign);
> + if (ret == SSH_ERR_KEY_WRONG_PASSPHRASE) {
> + pin = read_passphrase("Enter engine key passphrase:",
> RP_ALLOW_STDIN);
> + if (!pin)
> + return -1;
> + ret = ssh_add_engine_key(agent_fd, file, pin, lifetime,
> confirm, maxsign);
> + }
> + if (ret != SSH_AGENT_SUCCESS) {
> + fprintf(stderr, "failed to add engine key: %s\n",
> ssh_err(ret));
> + } else {
> + fprintf(stderr, "Engine Identity added: %s\n", file);
> + }
> + if (pin)
> + free (pin);
> + return ret;
> +}
> +#endif
> +
> static int
> delete_file(int agent_fd, const char *filename, int key_only, int
> qflag)
> {
> @@ -235,6 +260,17 @@ add_file(int agent_fd, const char *filename, int
> key_only, int qflag,
> /* At first, try empty passphrase */
> if ((r = sshkey_parse_private_fileblob(keyblob, "", &private,
> &comment)) != 0 && r != SSH_ERR_KEY_WRONG_PASSPHRASE) {
> +#ifdef USE_OPENSSL_ENGINE
> + int n_r = add_engine_key(agent_fd, filename);
> +
> + if (n_r == SSH_AGENT_SUCCESS) {
> + clear_pass();
> + sshbuf_free(keyblob);
> + return 0;
> + } else if (n_r != SSH_ERR_INTERNAL_ERROR) {
> + r = n_r;
> + }
> +#endif
> fprintf(stderr, "Error loading key \"%s\": %s\n",
> filename, ssh_err(r));
> goto fail_load;
> diff --git a/ssh-agent.c b/ssh-agent.c
> index e081413b8..665f7a47e 100644
> --- a/ssh-agent.c
> +++ b/ssh-agent.c
> @@ -92,6 +92,10 @@
> #include "ssh-pkcs11.h"
> #include "sk-api.h"
>
> +#ifdef USE_OPENSSL_ENGINE
> +#include "ssh-engine.h"
> +#endif
> +
> #ifndef DEFAULT_PROVIDER_WHITELIST
> # define DEFAULT_PROVIDER_WHITELIST "/usr/lib*/*,/usr/local/lib*/*"
> #endif
> @@ -639,6 +643,70 @@ no_identities(SocketEntry *e)
> sshbuf_free(msg);
> }
>
> +#ifdef USE_OPENSSL_ENGINE
> +static void
> +process_add_engine_key(SocketEntry *e)
> +{
> + char *pin, *file, *comment;
> + int r, confirm = 0;
> + u_int seconds;
> + time_t death = 0;
> + u_char type;
> + struct sshkey *k;
> + Identity *id;
> +
> + if ((r = sshbuf_get_cstring(e->request, &file, NULL)) != 0 ||
> + (r = sshbuf_get_cstring(e->request, &pin, NULL)) != 0)
> + fatal("%s: buffer error: %s", __func__, ssh_err(r));
> +
> + while (sshbuf_len(e->request)) {
> + if ((r = sshbuf_get_u8(e->request, &type)) != 0)
> + fatal("%s: buffer error: %s", __func__,
> ssh_err(r));
> + switch (type) {
> + case SSH_AGENT_CONSTRAIN_LIFETIME:
> + if ((r = sshbuf_get_u32(e->request, &seconds))
> != 0)
> + fatal("%s: buffer error: %s",
> + __func__, ssh_err(r));
> + death = monotime() + seconds;
> + break;
> + case SSH_AGENT_CONSTRAIN_CONFIRM:
> + confirm = 1;
> + break;
> + default:
> + error("%s: Unknown constraint type %d",
> __func__, type);
> + goto send;
> + }
> + }
> + if (lifetime && !death)
> + death = monotime() + lifetime;
> +
> + if ((r = engine_process_add(file, pin, &k, &comment)) < 0)
> + goto send;
> +
> + r = SSH_AGENT_SUCCESS;
> + if (lookup_identity(k) == NULL) {
> + id = xcalloc(1, sizeof(Identity));
> + id->key = k;
> + id->comment = comment;
> + id->death = death;
> + id->confirm = confirm;
> + TAILQ_INSERT_TAIL(&idtab->idlist, id, next);
> + idtab->nentries++;
> + } else {
> + /* key is already present, just return success */
> + sshkey_free(k);
> + }
> +
> +send:
> + free(pin);
> + free(file);
> + /* open code send_status because need to return actual error */
> + if (sshbuf_put_u32(e->output, 1) != 0 ||
> + sshbuf_put_u8(e->output, r) != 0)
> + fatal("%s: buffer error", __func__);
> +}
> +#endif /* USE_OPENSSL_ENGINE */
> +
> #ifdef ENABLE_PKCS11
> static void
> process_add_smartcard_key(SocketEntry *e)
> @@ -859,6 +927,12 @@ process_message(u_int socknum)
> process_remove_smartcard_key(e);
> break;
> #endif /* ENABLE_PKCS11 */
> +#ifdef USE_OPENSSL_ENGINE
> + case SSH_AGENTC_ADD_ENGINE_KEY:
> + case SSH_AGENTC_ADD_ENGINE_KEY_CONSTRAINED:
> + process_add_engine_key(e);
> + break;
> +#endif /* USE_OPENSSL_ENGINE */
> default:
> /* Unknown message. Respond with failure. */
> error("Unknown message %d", type);
> diff --git a/ssh-engine.c b/ssh-engine.c
> new file mode 100644
> index 000000000..2263c8e7d
> --- /dev/null
> +++ b/ssh-engine.c
> @@ -0,0 +1,159 @@
> +#include "includes.h"
> +
> +#include <string.h>
> +
> +#include <openssl/engine.h>
> +#include <openssl/evp.h>
> +#include <openssl/ui.h>
> +
> +#include "authfile.h"
> +#include "log.h"
> +#include "ssh-engine.h"
> +#include "sshkey.h"
> +#include "ssherr.h"
> +#include "xmalloc.h"
> +
> +#ifdef USE_OPENSSL_ENGINE
> +
> +struct ui_data {
> + char *passphrase;
> + int ret;
> +};
> +
> +static int
> +ui_read(UI *ui, UI_STRING *uis)
> +{
> + struct ui_data *d = UI_get0_user_data(ui);
> + d->ret = 0;
> +
> + if (UI_get_string_type(uis) == UIT_PROMPT) {
> + if (d->passphrase == NULL || d->passphrase[0] == '\0')
> {
> + /* we sent no passphrase but get asked for one
> + * send an interrupt event to avoid DA
> implications */
> + d->ret = -2;
> + } else {
> + UI_set_result(ui, uis, d->passphrase);
> + d->ret = 1;
> + }
> + }
> +
> + return d->ret;
> +}
> +
> +static int
> +engine_process_add_internal(ENGINE *e, char *file, char *pin,
> + struct sshkey **k)
> +{
> + EVP_PKEY *pk;
> + int ret;
> + UI_METHOD *ui;
> + EVP_PKEY_CTX *ctx;
> + char hash[SHA256_DIGEST_LENGTH], result[1024];
> + size_t siglen;
> + const char *engine = ENGINE_get_name(e);
> + struct ui_data d;
> +
> + verbose("%s: add provider=%s, key=%s", __func__, engine, file);
> +
> + ret = SSH_ERR_INTERNAL_ERROR;
> +
> + ui = UI_create_method("ssh-agent password writer");
> + if (!ui) {
> + verbose("%s: failed to create UI method", __func__);
> + ERR_print_errors_fp(stderr);
> + return ret;
> + }
> + UI_method_set_reader(ui, ui_read);
> +
> + if (!ENGINE_init(e)) {
> + verbose("%s: failed to init engine %s", __func__,
> engine);
> + ERR_print_errors_fp(stderr);
> + return ret;
> + }
> +
> + d.passphrase = pin;
> + d.ret = 0;
> + pk = ENGINE_load_private_key(e, file, ui, &d);
> + ENGINE_finish(e);
> +
> + if (d.ret == -2) {
> + ret = SSH_ERR_KEY_WRONG_PASSPHRASE;
> + ERR_clear_error();
> + goto err_free_pkey;
> + }
> +
> + if (!pk) {
> + verbose("%s: engine returned no key", __func__);
> + ERR_print_errors_fp(stderr);
> + return ret;
> + }
> +
> + /* here's a nasty problem: most engines don't tell us the
> password
> + * was wrong until we try to use the key, so do a test to see
> */
> + ctx = EVP_PKEY_CTX_new(pk, NULL);
> + if (!ctx) {
> + verbose("%s: openssl context allocation failed",
> __func__);
> + ERR_print_errors_fp(stderr);
> + goto err_free_pkey;
> + }
> +
> + EVP_PKEY_sign_init(ctx);
> +
> + siglen=sizeof(result);
> + ret = EVP_PKEY_sign(ctx, result, &siglen, hash, sizeof(hash));
> + EVP_PKEY_CTX_free(ctx);
> +
> + if (ret != 1 || siglen == 0) {
> + verbose("%s: trial signature failed with %d", __func__,
> ret);
> + ERR_print_errors_fp(stderr);
> + ret = SSH_ERR_KEY_WRONG_PASSPHRASE;
> + goto err_free_pkey;
> + }
> +
> + ret = sshkey_openssl_private_key(KEY_UNSPEC, pk, k, 1);
> +
> + err_free_pkey:
> + EVP_PKEY_free(pk);
> + verbose("%s: returning %d", __func__, ret);
> + return ret;
> +}
> +
> +static void
> +engine_get_comment(char *file, char **comment)
> +{
> + struct sshkey *kp;
> +
> + if (!comment)
> + return;
> +
> + if (sshkey_load_public(file, &kp, comment) < 0)
> + *comment = xstrdup(file);
> + else
> + sshkey_free(kp);
> +}
> +
> +int
> +engine_process_add(char *file, char *pin, struct sshkey **k, char
> **comment)
> +{
> + ENGINE *e;
> +
> + for (e = ENGINE_get_first(); e != NULL; e = ENGINE_get_next(e))
> {
> + int ret;
> +
> + if (!ENGINE_get_load_privkey_function(e))
> + continue;
> +
> + ret = engine_process_add_internal(e, file, pin, k);
> +
> + if (ret == SSH_ERR_KEY_WRONG_PASSPHRASE) {
> + return ret;
> + } else if (ret == 0) {
> + engine_get_comment(file, comment);
> + return ret;
> + }
> + }
> +
> + return SSH_ERR_INTERNAL_ERROR;
> +}
> +
> +#endif
> diff --git a/ssh-engine.h b/ssh-engine.h
> new file mode 100644
> index 000000000..a1310dc90
> --- /dev/null
> +++ b/ssh-engine.h
> @@ -0,0 +1,9 @@
> +#ifndef _ENGINE_H
> +#define _ENGINE_H
> +
> +#include "sshkey.h"
> +
> +int
> +engine_process_add(char *file, char *pin, struct sshkey **k, char
> **comment);
> +
> +#endif
--
Jakub Jelen
Senior Software Engineer
Security Technologies
Red Hat, Inc.

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: [PATCH v2 2/2] Add support for openssl engine based keys [ In reply to ]
On Wed, 2020-06-10 at 21:23 +0200, Jakub Jelen wrote:
> Hello,
> thank you for the patch. It looks good on a fast read-through.
>
> The ssh-agent protocol modification you propose would require
> addition to the draft specification [1]. This is why I used in my
> initial implementation of PKCS#11 URI few years back the same
> messages as are used today, SSH_AGENTC_ADD_SMARTCARD_KEY, but used
> PKCS#11 URI instead of the "opaque id" as described in the specs, but
> I understand that it might be cumbersome in this case.

I can certainly propose that addition. Given that this scheme would
likely be used for the provider API beyond the engine one, should it
have a more generic name?

> Reading also through the rest of the patches, I see you added support
> for pkcs11 uris only to the ssh-agent interface. When we should
> support this, we should support also input to ssh (through command-
> line and configuration) and ssh-keygen (to get public keys from
> engine) too.

OK, I'll look at that. My use case was always taking an existing key
and moving it to an engine, but creating an engine key and using it is
also a completely valid use case even if I don't do it.

> Just out of curiosity, do the keys in agent work when after removing
> and reinserting smart card? The current implementation does not
> handle this at all [3] causing headaches for anyone interested in
> using this.

Um, I have to admit that I don't actually have any actual pkcs#11
cards. I do all my stuff using either full emulators, like softhsm2 or
a shim which can make any engine key appear as a pkcs#11 one:

https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl-pkcs11-export.git/

However, I do believe this is solved in p11 kit which means the
solution should propagate to the engine. However, the engine does use
a global context, which the agent keeps for its lifetime so there may
be other problems.

James


> [1] https://tools.ietf.org/html/draft-miller-ssh-agent-04
> [2] https://github.com/Jakuje/openssh-portable/commits/jjelen-pkcs11
> [3] https://bugzilla.mindrot.org/show_bug.cgi?id=2890
>
> Regards,
> Jakub
>
> On Tue, 2020-06-09 at 12:36 -0700, James Bottomley wrote:
> > Engine keys are keys whose file format is understood by a specific
> > engine rather than by openssl itself. Since these keys are file
> > based, the pkcs11 interface isn't appropriate for them because they
> > don't actually represent tokens. The current most useful engine
> > for
> > openssh keys are the TPM engines, which allow all private keys to
> > be
> > stored in a form only the TPM hardware can decode, making them
> > impossible to steal. The design of engine keys is that while they
> > have to be loaded using a different openssl API, they can be used
> > as
> > standard openssl EVP_PKEYs. The only difficulty for openssh is
> > that
> > the private components can't be serialised, so a new agent command
> > is
> > introduced which interprets a message giving the location of the
> > file
> > and invokes the correct engine API from with ssh-agent.
> >
> > Since engine keys are drop in replacements for openssl keys, any
> > ssh
> > key that can be converted to openssl form can also be converted to
> > an
> > engine key. To convert an existing ssh key to engine form first
> > convert to PKCS8 and then pass that output into the engine
> > conversion
> > command. So to convert a private key stored in file rsa to TPM
> > engine
> > format, you do
> >
> > ssh-keygen -p -m PKCS8 -f rsa
> > create_tpm2_key -w rsa rsa.tpm
> >
> > Then to use the TPM key simply mv rsa.tpm rsa and openssh will be
> > able
> > to use this key using the -o option to specify the engine:
> >
> > ssh -i rsa user@host
> >
> > Note that engines usually have specific limits on the type of keys
> > they accept (so TPM 2.0 usually only does 2048 bits for RSA and
> > NIST
> > elliptic curves) so not all existing openssh keys can be converted
> > to
> > engine keys.
> >
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.c
> > om
> > >
> >
> > ---
> > Makefile.in | 2 +-
> > authfd.c | 44 ++++++++++++++
> > authfd.h | 6 ++
> > ssh-add.c | 36 ++++++++++++
> > ssh-agent.c | 74 ++++++++++++++++++++++++
> > ssh-engine.c | 159
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> > ssh-engine.h | 9 +++
> > 7 files changed, 329 insertions(+), 1 deletion(-)
> > create mode 100644 ssh-engine.c
> > create mode 100644 ssh-engine.h
> >
> > diff --git a/Makefile.in b/Makefile.in
> > index c9e4294d3..3260d2186 100644
> > --- a/Makefile.in
> > +++ b/Makefile.in
> > @@ -136,7 +136,7 @@ SCP_OBJS= scp.o progressmeter.o
> >
> > SSHADD_OBJS= ssh-add.o $(SKOBJS)
> >
> > -SSHAGENT_OBJS= ssh-agent.o ssh-pkcs11-client.o $(SKOBJS)
> > +SSHAGENT_OBJS= ssh-agent.o ssh-pkcs11-client.o ssh-engine.o
> > $(SKOBJS)
> >
> > SSHKEYGEN_OBJS= ssh-keygen.o sshsig.o $(SKOBJS)
> >
> > diff --git a/authfd.c b/authfd.c
> > index 4b647a628..6022a24ec 100644
> > --- a/authfd.c
> > +++ b/authfd.c
> > @@ -567,6 +567,50 @@ ssh_remove_identity(int sock, struct sshkey
> > *key)
> > return r;
> > }
> >
> > +/*
> > + * Add an engine based identity
> > + */
> > +#ifdef USE_OPENSSL_ENGINE
> > +int
> > +ssh_add_engine_key(int sock, const char *file, const char *pin,
> > + u_int lifetime, u_int confirm, u_int maxsign)
> > +{
> > + struct sshbuf *msg;
> > + int r, constrained = (lifetime || confirm);
> > + u_char type = constrained ?
> > SSH_AGENTC_ADD_ENGINE_KEY_CONSTRAINED :
> > + SSH_AGENTC_ADD_ENGINE_KEY;
> > +
> > + msg = sshbuf_new();
> > + if (!msg)
> > + return SSH_ERR_ALLOC_FAIL;
> > + r = sshbuf_put_u8(msg, type);
> > + if (r)
> > + goto out;
> > + r = sshbuf_put_cstring(msg, file);
> > + if (r)
> > + goto out;
> > + r = sshbuf_put_cstring(msg, pin);
> > + if (r)
> > + goto out;
> > + if (constrained) {
> > + r = encode_constraints(msg, lifetime, confirm,
> > maxsign,
> > NULL);
> > + if (r)
> > + goto out;
> > + }
> > + r = ssh_request_reply(sock, msg, msg);
> > + if (r)
> > + goto out;
> > + r = sshbuf_get_u8(msg, &type);
> > + if (r)
> > + goto out;
> > + r = (signed char)type;
> > + out:
> > + sshbuf_free(msg);
> > + return r;
> > +}
> > +#endif
> > +
> > +
> > /*
> > * Add/remove an token-based identity from the authentication
> > server.
> > * This call is intended only for use by ssh-add(1) and like
> > applications.
> > diff --git a/authfd.h b/authfd.h
> > index c3bf6259a..1bf42b4e6 100644
> > --- a/authfd.h
> > +++ b/authfd.h
> > @@ -38,6 +38,8 @@ int ssh_remove_identity(int sock, struct
> > sshkey
> > *key);
> > int ssh_update_card(int sock, int add, const char
> > *reader_id,
> > const char *pin, u_int life, u_int confirm);
> > int ssh_remove_all_identities(int sock, int version);
> > +int ssh_add_engine_key(int sock, const char *file, const
> > char *pin,
> > + u_int lifetime, u_int confirm, u_int
> > maxsign);
> >
> > int ssh_agent_sign(int sock, const struct sshkey *key,
> > u_char **sigp, size_t *lenp,
> > @@ -76,6 +78,10 @@ int ssh_agent_sign(int sock, const struct
> > sshkey *key,
> > #define SSH2_AGENTC_ADD_ID_CONSTRAINED 25
> > #define SSH_AGENTC_ADD_SMARTCARD_KEY_CONSTRAINED 26
> >
> > +/* engine keys */
> > +#define SSH_AGENTC_ADD_ENGINE_KEY 27
> > +#define SSH_AGENTC_ADD_ENGINE_KEY_CONSTRAINED 28
> > +
> > #define SSH_AGENT_CONSTRAIN_LIFETIME 1
> > #define SSH_AGENT_CONSTRAIN_CONFIRM 2
> > #define SSH_AGENT_CONSTRAIN_MAXSIGN 3
> > diff --git a/ssh-add.c b/ssh-add.c
> > index a40198ab5..b0ce393c7 100644
> > --- a/ssh-add.c
> > +++ b/ssh-add.c
> > @@ -110,6 +110,31 @@ clear_pass(void)
> > }
> > }
> >
> > +#ifdef USE_OPENSSL_ENGINE
> > +static int
> > +add_engine_key(int agent_fd, const char *file)
> > +{
> > + int ret;
> > + char *pin = NULL;
> > +
> > + ret = ssh_add_engine_key(agent_fd, file, NULL, lifetime,
> > confirm, maxsign);
> > + if (ret == SSH_ERR_KEY_WRONG_PASSPHRASE) {
> > + pin = read_passphrase("Enter engine key
> > passphrase:",
> > RP_ALLOW_STDIN);
> > + if (!pin)
> > + return -1;
> > + ret = ssh_add_engine_key(agent_fd, file, pin,
> > lifetime,
> > confirm, maxsign);
> > + }
> > + if (ret != SSH_AGENT_SUCCESS) {
> > + fprintf(stderr, "failed to add engine key: %s\n",
> > ssh_err(ret));
> > + } else {
> > + fprintf(stderr, "Engine Identity added: %s\n",
> > file);
> > + }
> > + if (pin)
> > + free (pin);
> > + return ret;
> > +}
> > +#endif
> > +
> > static int
> > delete_file(int agent_fd, const char *filename, int key_only, int
> > qflag)
> > {
> > @@ -235,6 +260,17 @@ add_file(int agent_fd, const char *filename,
> > int
> > key_only, int qflag,
> > /* At first, try empty passphrase */
> > if ((r = sshkey_parse_private_fileblob(keyblob, "",
> > &private,
> > &comment)) != 0 && r != SSH_ERR_KEY_WRONG_PASSPHRASE)
> > {
> > +#ifdef USE_OPENSSL_ENGINE
> > + int n_r = add_engine_key(agent_fd, filename);
> > +
> > + if (n_r == SSH_AGENT_SUCCESS) {
> > + clear_pass();
> > + sshbuf_free(keyblob);
> > + return 0;
> > + } else if (n_r != SSH_ERR_INTERNAL_ERROR) {
> > + r = n_r;
> > + }
> > +#endif
> > fprintf(stderr, "Error loading key \"%s\": %s\n",
> > filename, ssh_err(r));
> > goto fail_load;
> > diff --git a/ssh-agent.c b/ssh-agent.c
> > index e081413b8..665f7a47e 100644
> > --- a/ssh-agent.c
> > +++ b/ssh-agent.c
> > @@ -92,6 +92,10 @@
> > #include "ssh-pkcs11.h"
> > #include "sk-api.h"
> >
> > +#ifdef USE_OPENSSL_ENGINE
> > +#include "ssh-engine.h"
> > +#endif
> > +
> > #ifndef DEFAULT_PROVIDER_WHITELIST
> > # define DEFAULT_PROVIDER_WHITELIST
> > "/usr/lib*/*,/usr/local/lib*/*"
> > #endif
> > @@ -639,6 +643,70 @@ no_identities(SocketEntry *e)
> > sshbuf_free(msg);
> > }
> >
> > +#ifdef USE_OPENSSL_ENGINE
> > +static void
> > +process_add_engine_key(SocketEntry *e)
> > +{
> > + char *pin, *file, *comment;
> > + int r, confirm = 0;
> > + u_int seconds;
> > + time_t death = 0;
> > + u_char type;
> > + struct sshkey *k;
> > + Identity *id;
> > +
> > + if ((r = sshbuf_get_cstring(e->request, &file, NULL)) != 0
> > ||
> > + (r = sshbuf_get_cstring(e->request, &pin, NULL)) != 0)
> > + fatal("%s: buffer error: %s", __func__,
> > ssh_err(r));
> > +
> > + while (sshbuf_len(e->request)) {
> > + if ((r = sshbuf_get_u8(e->request, &type)) != 0)
> > + fatal("%s: buffer error: %s", __func__,
> > ssh_err(r));
> > + switch (type) {
> > + case SSH_AGENT_CONSTRAIN_LIFETIME:
> > + if ((r = sshbuf_get_u32(e->request,
> > &seconds))
> > != 0)
> > + fatal("%s: buffer error: %s",
> > + __func__, ssh_err(r));
> > + death = monotime() + seconds;
> > + break;
> > + case SSH_AGENT_CONSTRAIN_CONFIRM:
> > + confirm = 1;
> > + break;
> > + default:
> > + error("%s: Unknown constraint type %d",
> > __func__, type);
> > + goto send;
> > + }
> > + }
> > + if (lifetime && !death)
> > + death = monotime() + lifetime;
> > +
> > + if ((r = engine_process_add(file, pin, &k, &comment)) < 0)
> > + goto send;
> > +
> > + r = SSH_AGENT_SUCCESS;
> > + if (lookup_identity(k) == NULL) {
> > + id = xcalloc(1, sizeof(Identity));
> > + id->key = k;
> > + id->comment = comment;
> > + id->death = death;
> > + id->confirm = confirm;
> > + TAILQ_INSERT_TAIL(&idtab->idlist, id, next);
> > + idtab->nentries++;
> > + } else {
> > + /* key is already present, just return success */
> > + sshkey_free(k);
> > + }
> > +
> > +send:
> > + free(pin);
> > + free(file);
> > + /* open code send_status because need to return actual
> > error */
> > + if (sshbuf_put_u32(e->output, 1) != 0 ||
> > + sshbuf_put_u8(e->output, r) != 0)
> > + fatal("%s: buffer error", __func__);
> > +}
> > +#endif /* USE_OPENSSL_ENGINE */
> > +
> > #ifdef ENABLE_PKCS11
> > static void
> > process_add_smartcard_key(SocketEntry *e)
> > @@ -859,6 +927,12 @@ process_message(u_int socknum)
> > process_remove_smartcard_key(e);
> > break;
> > #endif /* ENABLE_PKCS11 */
> > +#ifdef USE_OPENSSL_ENGINE
> > + case SSH_AGENTC_ADD_ENGINE_KEY:
> > + case SSH_AGENTC_ADD_ENGINE_KEY_CONSTRAINED:
> > + process_add_engine_key(e);
> > + break;
> > +#endif /* USE_OPENSSL_ENGINE */
> > default:
> > /* Unknown message. Respond with failure. */
> > error("Unknown message %d", type);
> > diff --git a/ssh-engine.c b/ssh-engine.c
> > new file mode 100644
> > index 000000000..2263c8e7d
> > --- /dev/null
> > +++ b/ssh-engine.c
> > @@ -0,0 +1,159 @@
> > +#include "includes.h"
> > +
> > +#include <string.h>
> > +
> > +#include <openssl/engine.h>
> > +#include <openssl/evp.h>
> > +#include <openssl/ui.h>
> > +
> > +#include "authfile.h"
> > +#include "log.h"
> > +#include "ssh-engine.h"
> > +#include "sshkey.h"
> > +#include "ssherr.h"
> > +#include "xmalloc.h"
> > +
> > +#ifdef USE_OPENSSL_ENGINE
> > +
> > +struct ui_data {
> > + char *passphrase;
> > + int ret;
> > +};
> > +
> > +static int
> > +ui_read(UI *ui, UI_STRING *uis)
> > +{
> > + struct ui_data *d = UI_get0_user_data(ui);
> > + d->ret = 0;
> > +
> > + if (UI_get_string_type(uis) == UIT_PROMPT) {
> > + if (d->passphrase == NULL || d->passphrase[0] ==
> > '\0')
> > {
> > + /* we sent no passphrase but get asked for
> > one
> > + * send an interrupt event to avoid DA
> > implications */
> > + d->ret = -2;
> > + } else {
> > + UI_set_result(ui, uis, d->passphrase);
> > + d->ret = 1;
> > + }
> > + }
> > +
> > + return d->ret;
> > +}
> > +
> > +static int
> > +engine_process_add_internal(ENGINE *e, char *file, char *pin,
> > + struct sshkey **k)
> > +{
> > + EVP_PKEY *pk;
> > + int ret;
> > + UI_METHOD *ui;
> > + EVP_PKEY_CTX *ctx;
> > + char hash[SHA256_DIGEST_LENGTH], result[1024];
> > + size_t siglen;
> > + const char *engine = ENGINE_get_name(e);
> > + struct ui_data d;
> > +
> > + verbose("%s: add provider=%s, key=%s", __func__, engine,
> > file);
> > +
> > + ret = SSH_ERR_INTERNAL_ERROR;
> > +
> > + ui = UI_create_method("ssh-agent password writer");
> > + if (!ui) {
> > + verbose("%s: failed to create UI method",
> > __func__);
> > + ERR_print_errors_fp(stderr);
> > + return ret;
> > + }
> > + UI_method_set_reader(ui, ui_read);
> > +
> > + if (!ENGINE_init(e)) {
> > + verbose("%s: failed to init engine %s", __func__,
> > engine);
> > + ERR_print_errors_fp(stderr);
> > + return ret;
> > + }
> > +
> > + d.passphrase = pin;
> > + d.ret = 0;
> > + pk = ENGINE_load_private_key(e, file, ui, &d);
> > + ENGINE_finish(e);
> > +
> > + if (d.ret == -2) {
> > + ret = SSH_ERR_KEY_WRONG_PASSPHRASE;
> > + ERR_clear_error();
> > + goto err_free_pkey;
> > + }
> > +
> > + if (!pk) {
> > + verbose("%s: engine returned no key", __func__);
> > + ERR_print_errors_fp(stderr);
> > + return ret;
> > + }
> > +
> > + /* here's a nasty problem: most engines don't tell us the
> > password
> > + * was wrong until we try to use the key, so do a test to
> > see
> > */
> > + ctx = EVP_PKEY_CTX_new(pk, NULL);
> > + if (!ctx) {
> > + verbose("%s: openssl context allocation failed",
> > __func__);
> > + ERR_print_errors_fp(stderr);
> > + goto err_free_pkey;
> > + }
> > +
> > + EVP_PKEY_sign_init(ctx);
> > +
> > + siglen=sizeof(result);
> > + ret = EVP_PKEY_sign(ctx, result, &siglen, hash,
> > sizeof(hash));
> > + EVP_PKEY_CTX_free(ctx);
> > +
> > + if (ret != 1 || siglen == 0) {
> > + verbose("%s: trial signature failed with %d",
> > __func__,
> > ret);
> > + ERR_print_errors_fp(stderr);
> > + ret = SSH_ERR_KEY_WRONG_PASSPHRASE;
> > + goto err_free_pkey;
> > + }
> > +
> > + ret = sshkey_openssl_private_key(KEY_UNSPEC, pk, k, 1);
> > +
> > + err_free_pkey:
> > + EVP_PKEY_free(pk);
> > + verbose("%s: returning %d", __func__, ret);
> > + return ret;
> > +}
> > +
> > +static void
> > +engine_get_comment(char *file, char **comment)
> > +{
> > + struct sshkey *kp;
> > +
> > + if (!comment)
> > + return;
> > +
> > + if (sshkey_load_public(file, &kp, comment) < 0)
> > + *comment = xstrdup(file);
> > + else
> > + sshkey_free(kp);
> > +}
> > +
> > +int
> > +engine_process_add(char *file, char *pin, struct sshkey **k, char
> > **comment)
> > +{
> > + ENGINE *e;
> > +
> > + for (e = ENGINE_get_first(); e != NULL; e =
> > ENGINE_get_next(e))
> > {
> > + int ret;
> > +
> > + if (!ENGINE_get_load_privkey_function(e))
> > + continue;
> > +
> > + ret = engine_process_add_internal(e, file, pin,
> > k);
> > +
> > + if (ret == SSH_ERR_KEY_WRONG_PASSPHRASE) {
> > + return ret;
> > + } else if (ret == 0) {
> > + engine_get_comment(file, comment);
> > + return ret;
> > + }
> > + }
> > +
> > + return SSH_ERR_INTERNAL_ERROR;
> > +}
> > +
> > +#endif
> > diff --git a/ssh-engine.h b/ssh-engine.h
> > new file mode 100644
> > index 000000000..a1310dc90
> > --- /dev/null
> > +++ b/ssh-engine.h
> > @@ -0,0 +1,9 @@
> > +#ifndef _ENGINE_H
> > +#define _ENGINE_H
> > +
> > +#include "sshkey.h"
> > +
> > +int
> > +engine_process_add(char *file, char *pin, struct sshkey **k, char
> > **comment);
> > +
> > +#endif

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev