Engine keys are private key files which are only understood by openssl external engines. ?The problem is they can't be loaded with the usual openssl methods, they have to be loaded via ENGINE_load_private_key(). ?Because they're files, they fit well into openssh pub/private file structure, so they're not very appropriately handled by the pkcs11 interface because it assumes the private keys are already present in some token (so there's no way to pass in a file name). The candidate I have for this is openssh private keys based in the trusted platform module (either tpm 1.2 or tpm 2.0 ... we have engines for both). This is an example of my tpm2 based private/public ssh-key: jejb at jarvis:~> ssh-add -o tpm2 /home/jejb/.ssh/id_rsa Enter engine key passphrase: jejb at jarvis:~> ssh-add -l 2048 SHA256:ZAv7jrI2bB2VBgk5jHA1g/fe4rVX1GqyCdPwF0ELU9k jejb at mulgrave (RSA) jejb at jarvis:~> cat .ssh/id_rsa -----BEGIN TSS2 KEY BLOB----- MIICAAYFZ4EFCgKgAwEBAKEHAgUAgQAAAaKCARwEggEYARYAAQALAAIEQAAAABAA EAgAAAAAAAEAp0rC+B6BvAl6ySgNggwBSYvkcvBFGIC5bs1/s0NtYkoZ5QnpafHY 7qHvqvvell9lRk58UDyRgGFFzSFvq0/YN4PJoG4ywMWHnOABzoIq/y5RAQgER6OE lrnvS9on9J5epy7tT7nQkCCjO+Oz3849UeFX1m6DFOJuir5GhYD968CZGfemjRyO LLEQ4dxF75Q/74seg2cdE+1BA6Q94nHOmcgNpJsprRgoNfpwFkU1iJrJ8oURUyCb 1cPsS+4kehgaCCTQ/Nkqcz9d2feaMLi5ukBj1qMOV4KdxS4KIfins1O6l2Yde1oe QDUW4AjWSQ3OHwwYVzvqDWZmlPG3NAFvEQSByADGACDZDFvuD6MUiE9vmkRZ2wYt kZVJPBp817eKWmDnkII71QAQ5oxhXbJERo+p5KeQWlf/CCtxK8NRbK/FKAwg0/Uu HoSDBT+gQ8S62pLD+qBDyXO4eFj0moxCX+shqlvo34rig2WF+Y/5NNhZg+2COdXt 45b/MO9jUPsXjCDPUm1f1NaVztK45/Wddk4m2lZ9TCDU7MOK140pcB5Ewricw61b st+nmmUtgIPUxxWc3uMcVATjJP2EWrFnmdRgbKXcVT8olHa+ -----END TSS2 KEY BLOB----- Where openssl_tpm2_engine is available here: git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engin e.git/ --- James Bottomley (2): ? Add support for openssl engine based keys ? engine: add "any" engine mechanism and make it the default ?Makefile.in??|???4 +- ?authfd.c?????|??45 +++++++++++++++ ?authfd.h?????|???7 +++ ?ssh-add.c????|??41 +++++++++++-- ?ssh-agent.c??|??82 ++++++++++++++++++++++++++ ?ssh-engine.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ?ssh-engine.h |??10 ++++ ?7 files changed, 367 insertions(+), 7 deletions(-) ?create mode 100644 ssh-engine.c ?create mode 100644 ssh-engine.h
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. Signed-off-by: James Bottomley <James.Bottomley at HansenPartnership.com> --- Makefile.in | 4 +- authfd.c | 45 +++++++++++++++++ authfd.h | 7 +++ ssh-add.c | 35 ++++++++++++-- ssh-agent.c | 82 +++++++++++++++++++++++++++++++ ssh-engine.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ssh-engine.h | 10 ++++ 7 files changed, 332 insertions(+), 5 deletions(-) create mode 100644 ssh-engine.c create mode 100644 ssh-engine.h diff --git a/Makefile.in b/Makefile.in index c52ce191f..42725fcbd 100644 --- a/Makefile.in +++ b/Makefile.in @@ -172,8 +172,8 @@ scp$(EXEEXT): $(LIBCOMPAT) libssh.a scp.o progressmeter.o ssh-add$(EXEEXT): $(LIBCOMPAT) libssh.a ssh-add.o $(LD) -o $@ ssh-add.o $(LDFLAGS) -lssh -lopenbsd-compat $(LIBS) -ssh-agent$(EXEEXT): $(LIBCOMPAT) libssh.a ssh-agent.o ssh-pkcs11-client.o - $(LD) -o $@ ssh-agent.o ssh-pkcs11-client.o $(LDFLAGS) -lssh -lopenbsd-compat $(LIBS) +ssh-agent$(EXEEXT): $(LIBCOMPAT) libssh.a ssh-agent.o ssh-pkcs11-client.o ssh-engine.o + $(LD) -o $@ ssh-agent.o ssh-pkcs11-client.o ssh-engine.o $(LDFLAGS) -lssh -lopenbsd-compat $(LIBS) ssh-keygen$(EXEEXT): $(LIBCOMPAT) libssh.a ssh-keygen.o $(LD) -o $@ ssh-keygen.o $(LDFLAGS) -lssh -lopenbsd-compat $(LIBS) diff --git a/authfd.c b/authfd.c index a460fa350..0a0fcfafc 100644 --- a/authfd.c +++ b/authfd.c @@ -514,6 +514,51 @@ ssh_remove_identity(int sock, struct sshkey *key) } /* + * Add an engine based identity + */ +int +ssh_add_engine_key(int sock, const char *file, const char *engine, + const char *pin, u_int lifetime, u_int confirm) +{ + 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, engine); + 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); + 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; +} + + +/* * 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 43abf85da..aa433644a 100644 --- a/authfd.h +++ b/authfd.h @@ -36,6 +36,9 @@ 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 *engine, + const char *pin, u_int lifetime, u_int confirm); + int ssh_decrypt_challenge(int sock, struct sshkey* key, BIGNUM *challenge, u_char session_id[16], u_char response[16]); int ssh_agent_sign(int sock, const struct sshkey *key, @@ -75,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 diff --git a/ssh-add.c b/ssh-add.c index 2afd48330..d85bcc7ec 100644 --- a/ssh-add.c +++ b/ssh-add.c @@ -337,6 +337,27 @@ add_file(int agent_fd, const char *filename, int key_only, int qflag) } static int +add_engine_key(int agent_fd, const char *file, const char *engine) +{ + int ret; + char *pin = NULL; + + ret = ssh_add_engine_key(agent_fd, file, engine, NULL, lifetime, confirm); + 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, engine, pin, lifetime, confirm); + } + if (ret != SSH_AGENT_SUCCESS) { + fprintf(stderr, "failed to add engine key: %s\n", ssh_err(ret)); + } + if (pin) + free (pin); + return ret; +} + +static int update_card(int agent_fd, int add, const char *id) { char *pin = NULL; @@ -462,6 +483,7 @@ usage(void) fprintf(stderr, " -s pkcs11 Add keys from PKCS#11 provider.\n"); fprintf(stderr, " -e pkcs11 Remove keys provided by PKCS#11 provider.\n"); fprintf(stderr, " -q Be quiet after a successful operation.\n"); + fprintf(stderr, " -o engine key file is to be processed by specified openssl engine\n"); } int @@ -470,7 +492,7 @@ main(int argc, char **argv) extern char *optarg; extern int optind; int agent_fd; - char *pkcs11provider = NULL; + char *pkcs11provider = NULL, *opensslengine = NULL; int r, i, ch, deleting = 0, ret = 0, key_only = 0; int xflag = 0, lflag = 0, Dflag = 0, qflag = 0; @@ -500,7 +522,7 @@ main(int argc, char **argv) exit(2); } - while ((ch = getopt(argc, argv, "klLcdDxXE:e:qs:t:")) != -1) { + while ((ch = getopt(argc, argv, "klLcdDxXE:e:qs:t:o:")) != -1) { switch (ch) { case 'E': fingerprint_hash = ssh_digest_alg_by_name(optarg); @@ -548,6 +570,9 @@ main(int argc, char **argv) case 'q': qflag = 1; break; + case 'o': + opensslengine = optarg; + break; default: usage(); ret = 1; @@ -573,7 +598,11 @@ main(int argc, char **argv) argc -= optind; argv += optind; - if (pkcs11provider != NULL) { + if (opensslengine != NULL) { + if (add_engine_key(agent_fd, argv[0], opensslengine) < 0) + ret = 1; + goto done; + } else if (pkcs11provider != NULL) { if (update_card(agent_fd, !deleting, pkcs11provider) == -1) ret = 1; goto done; diff --git a/ssh-agent.c b/ssh-agent.c index 0c6c36592..6b8834737 100644 --- a/ssh-agent.c +++ b/ssh-agent.c @@ -85,11 +85,16 @@ #include "digest.h" #include "ssherr.h" #include "match.h" +#include "authfile.h" #ifdef ENABLE_PKCS11 #include "ssh-pkcs11.h" #endif +#ifdef USE_OPENSSL_ENGINE +#include "ssh-engine.h" +#endif + #ifndef DEFAULT_PKCS11_WHITELIST # define DEFAULT_PKCS11_WHITELIST "/usr/lib*/*,/usr/local/lib*/*" #endif @@ -525,6 +530,77 @@ no_identities(SocketEntry *e) sshbuf_free(msg); } +#ifdef USE_OPENSSL_ENGINE +static void +process_add_engine_key(SocketEntry *e) +{ + char *engine, *pin, *file, *comment; + int r, confirm = 0; + u_int seconds; + time_t death = 0; + u_char type; + struct sshkey *k, *kp; + Identity *id; + + if ((r = sshbuf_get_cstring(e->request, &engine, NULL)) != 0 || + (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(engine, file, pin, &k)) < 0) + goto send; + + if (sshkey_load_public(file, &kp, &comment) < 0) + comment = xstrdup(file); + else + sshkey_free(kp); + + if (lookup_identity(k) == NULL) { + id = xcalloc(1, sizeof(Identity)); + id->key = k; + id->provider = xstrdup(engine); + id->comment = comment; + id->death = death; + id->confirm = confirm; + TAILQ_INSERT_TAIL(&idtab->idlist, id, next); + idtab->nentries++; + r = SSH_AGENT_SUCCESS; + } else { + sshkey_free(k); + } + +send: + free(pin); + free(engine); + 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) @@ -730,6 +806,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..f4673e4ee --- /dev/null +++ b/ssh-engine.c @@ -0,0 +1,154 @@ +#include "includes.h" + +#include <string.h> + +#include <openssl/engine.h> +#include <openssl/evp.h> +#include <openssl/ui.h> + +#include "log.h" +#include "ssh-engine.h" +#include "sshkey.h" +#include "ssherr.h" +#include "xmalloc.h" + +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; +} + +int +engine_process_add(char *engine, char *file, char *pin, + struct sshkey **k) +{ + EVP_PKEY *pk; + ENGINE *e; + struct sshkey *key; + int ret; + UI_METHOD *ui; + EVP_PKEY_CTX *ctx; + char hash[SHA256_DIGEST_LENGTH], result[1024]; + size_t siglen; + struct ui_data d; + + verbose("%s: add provider=%s, key=%s", __func__, engine, file); + + ret = SSH_ERR_INTERNAL_ERROR; + e = ENGINE_by_id(engine); + if (!e) { + verbose("%s: failed to get engine %s", __func__, engine); + ERR_print_errors_fp(stderr); + return ret; + } + + 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; + pk = ENGINE_load_private_key(e, file, ui, &d); + ENGINE_finish(e); + + if (d.ret == -2) + return SSH_ERR_KEY_WRONG_PASSPHRASE; + + 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) { + 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 = SSH_ERR_ALLOC_FAIL; + + key = sshkey_new(KEY_UNSPEC); + key->flags |= SSHKEY_FLAG_EXT; + if (!key) + goto err_free_pkey; + + switch (EVP_PKEY_id(pk)) { + case EVP_PKEY_RSA: + key->type = KEY_RSA; + key->rsa = EVP_PKEY_get1_RSA(pk); + break; + case EVP_PKEY_DSA: + key->type = KEY_DSA; + key->dsa = EVP_PKEY_get1_DSA(pk); + break; +#ifdef OPENSSL_HAS_ECC + case EVP_PKEY_EC: + key->type = KEY_ECDSA; + key->ecdsa = EVP_PKEY_get1_EC_KEY(pk); + key->ecdsa_nid = sshkey_ecdsa_key_to_nid(key->ecdsa); + if (key->ecdsa_nid == -1 || + sshkey_curve_nid_to_name(key->ecdsa_nid) == NULL) + goto err_free_sshkey; + break; +#endif + default: + verbose("%s: Unrecognised key type %d\n", __func__, EVP_PKEY_id(pk)); + ret = SSH_ERR_INVALID_FORMAT; + goto err_free_sshkey; + } + *k = key; + key = NULL; + ret = 1; + err_free_sshkey: + if (key) + sshkey_free(key); + err_free_pkey: + EVP_PKEY_free(pk); + verbose("%s: returning %d", __func__, ret); + return ret; +} diff --git a/ssh-engine.h b/ssh-engine.h new file mode 100644 index 000000000..b5e85f44d --- /dev/null +++ b/ssh-engine.h @@ -0,0 +1,10 @@ +#ifndef _ENGINE_H +#define _ENGINE_H + +#include "sshkey.h" + +int +engine_process_add(char *engine, char *file, char *pin, + struct sshkey **k); + +#endif -- 2.12.3
James Bottomley
2017-Oct-26 07:45 UTC
[RFC 2/2] engine: add "any" engine mechanism and make it the default
If openssl cannot load the private key, chances are it's an engine key (or a corrupt file), so try processing it via any engine first before concluding corruption. Signed-off-by: James Bottomley <James.Bottomley at HansenPartnership.com> --- ssh-add.c | 6 ++++-- ssh-engine.c | 51 +++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/ssh-add.c b/ssh-add.c index d85bcc7ec..b8706866a 100644 --- a/ssh-add.c +++ b/ssh-add.c @@ -459,8 +459,10 @@ do_file(int agent_fd, int deleting, int key_only, char *file, int qflag) if (delete_file(agent_fd, file, key_only, qflag) == -1) return -1; } else { - if (add_file(agent_fd, file, key_only, qflag) == -1) - return -1; + if (add_file(agent_fd, file, key_only, qflag) == -1) { + if (add_engine_key(agent_fd, file, "any") < 0) + return -1; + } } return 0; } diff --git a/ssh-engine.c b/ssh-engine.c index f4673e4ee..53fd1a004 100644 --- a/ssh-engine.c +++ b/ssh-engine.c @@ -37,29 +37,23 @@ ui_read(UI *ui, UI_STRING *uis) return d->ret; } -int -engine_process_add(char *engine, char *file, char *pin, - struct sshkey **k) +static int +engine_process_add_internal(ENGINE *e, char *file, char *pin, + struct sshkey **k) { EVP_PKEY *pk; - ENGINE *e; struct sshkey *key; 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; - e = ENGINE_by_id(engine); - if (!e) { - verbose("%s: failed to get engine %s", __func__, engine); - ERR_print_errors_fp(stderr); - return ret; - } ui = UI_create_method("ssh-agent password writer"); if (!ui) { @@ -152,3 +146,40 @@ engine_process_add(char *engine, char *file, char *pin, verbose("%s: returning %d", __func__, ret); return ret; } + +int +engine_process_add(char *engine, char *file, char *pin, + struct sshkey **k) +{ + ENGINE *e; + + if (strcmp(engine, "any") != 0) { + int ret; + + e = ENGINE_by_id(engine); + if (!e) { + verbose("%s: failed to get engine %s", __func__, engine); + ERR_print_errors_fp(stderr); + return SSH_ERR_INTERNAL_ERROR; + } + ret = engine_process_add_internal(e, file, pin, k); + ENGINE_free(e); + return ret; + } + + /* this is the any engine case */ + + 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 == 1 || ret == SSH_ERR_KEY_WRONG_PASSPHRASE) + return ret; + } + + return SSH_ERR_INTERNAL_ERROR; +} -- 2.12.3
On Thu, 26 Oct 2017, 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.What sort of keys do you have in mind here that can't be represented via PKCS#11? -d
Possibly Parallel Threads
- [RFC 1/2] Add support for openssl engine based keys
- [RFC 1/2] Add support for openssl engine based keys
- [RFC 1/2] Add support for openssl engine based keys
- [PATCH 1/2] Add support for openssl engine based keys
- [PATCH v2 0/2] Add openssl engine keys with provider upgrade path