Loïc
2020-Apr-25 00:58 UTC
[PATCH 1/3] Add private key protection information extraction to ssh-keygen
Add private key protection information extraction to shh-keygen using -v option on top of -y option which is already parsing the private key. Technically, the passphrase isn't necessary to do this, but it is the most logical thing to do for me. Adding this to -l option is not appropriate because fingerprinting is using the .pub file when available. An other idea is to add a new option, I can do it if you prefer. Also, I'm laking information for information extraction from PEM and PKCS8 file format, I'm OK to have a pointer to implement this correctly. This patch is also adding a regression test for the functionnality. --- ?authfile.c??????????????????????????? |? 16 ++-- ?authfile.h??????????????????????????? |?? 7 +- ?regress/Makefile????????????????????? |?? 3 +- ?regress/keygen-private-information.sh |? 81 +++++++++++++++++++++ ?ssh-keygen.c????????????????????????? |? 44 +++++++---- ?ssh-keysign.c???????????????????????? |?? 2 +- ?sshconnect2.c???????????????????????? |?? 2 +- ?sshd.c??????????????????????????????? |?? 2 +- ?sshkey.c????????????????????????????? | 101 +++++++++++++++++++++++--- ?sshkey.h????????????????????????????? |? 14 +++- ?10 files changed, 234 insertions(+), 38 deletions(-) ?create mode 100644 regress/keygen-private-information.sh diff --git a/authfile.c b/authfile.c index 35ccf576c2b5..6c79369ebfc1 100644 --- a/authfile.c +++ b/authfile.c @@ -116,7 +116,7 @@ sshkey_perm_ok(int fd, const char *filename) ? ?int ?sshkey_load_private_type(int type, const char *filename, const char *passphrase, -??? struct sshkey **keyp, char **commentp) +??? struct sshkey **keyp, char **commentp, struct sshkey_vault **vault_infop) ?{ ??? ?int fd, r; ? @@ -124,6 +124,8 @@ sshkey_load_private_type(int type, const char *filename, const char *passphrase, ??? ??? ?*keyp = NULL; ??? ?if (commentp != NULL) ??? ??? ?*commentp = NULL; +?? ?if (vault_infop != NULL) +?? ??? ?*vault_infop = NULL; ? ??? ?if ((fd = open(filename, O_RDONLY)) == -1) ??? ??? ?return SSH_ERR_SYSTEM_ERROR; @@ -132,7 +134,7 @@ sshkey_load_private_type(int type, const char *filename, const char *passphrase, ??? ?if (r != 0) ??? ??? ?goto out; ? -?? ?r = sshkey_load_private_type_fd(fd, type, passphrase, keyp, commentp); +?? ?r = sshkey_load_private_type_fd(fd, type, passphrase, keyp, commentp, vault_infop); ??? ?if (r == 0 && keyp && *keyp) ??? ??? ?r = sshkey_set_filename(*keyp, filename); ? out: @@ -142,15 +144,15 @@ sshkey_load_private_type(int type, const char *filename, const char *passphrase, ? ?int ?sshkey_load_private(const char *filename, const char *passphrase, -??? struct sshkey **keyp, char **commentp) +??? struct sshkey **keyp, char **commentp, struct sshkey_vault **vault_infop) ?{ ??? ?return sshkey_load_private_type(KEY_UNSPEC, filename, passphrase, -?? ???? keyp, commentp); +?? ???? keyp, commentp, vault_infop); ?} ? ?int ?sshkey_load_private_type_fd(int fd, int type, const char *passphrase, -??? struct sshkey **keyp, char **commentp) +??? struct sshkey **keyp, char **commentp, struct sshkey_vault **vault_infop) ?{ ??? ?struct sshbuf *buffer = NULL; ??? ?int r; @@ -159,7 +161,7 @@ sshkey_load_private_type_fd(int fd, int type, const char *passphrase, ??? ??? ?*keyp = NULL; ??? ?if ((r = sshbuf_load_fd(fd, &buffer)) != 0 || ??? ???? (r = sshkey_parse_private_fileblob_type(buffer, type, -?? ???? passphrase, keyp, commentp)) != 0) +?? ???? passphrase, keyp, commentp, vault_infop)) != 0) ??? ??? ?goto out; ? ??? ?/* success */ @@ -334,7 +336,7 @@ sshkey_load_private_cert(int type, const char *filename, const char *passphrase, ??? ?} ? ??? ?if ((r = sshkey_load_private_type(type, filename, -?? ???? passphrase, &key, NULL)) != 0 || +?? ???? passphrase, &key, NULL, NULL)) != 0 || ??? ???? (r = sshkey_load_cert(filename, &cert)) != 0) ??? ??? ?goto out; ? diff --git a/authfile.h b/authfile.h index 1db067a813a1..85f78ac78edf 100644 --- a/authfile.h +++ b/authfile.h @@ -29,6 +29,7 @@ ? ?struct sshbuf; ?struct sshkey; +struct sshkey_vault; ? ?/* XXX document these */ ?/* XXX some of these could probably be merged/retired */ @@ -37,13 +38,13 @@ int sshkey_save_private(struct sshkey *, const char *, ???? const char *, const char *, int, const char *, int); ?int sshkey_load_cert(const char *, struct sshkey **); ?int sshkey_load_public(const char *, struct sshkey **, char **); -int sshkey_load_private(const char *, const char *, struct sshkey **, char **); +int sshkey_load_private(const char *, const char *, struct sshkey **, char **, struct sshkey_vault **); ?int sshkey_load_private_cert(int, const char *, const char *, ???? struct sshkey **); ?int sshkey_load_private_type(int, const char *, const char *, -??? struct sshkey **, char **); +??? struct sshkey **, char **, struct sshkey_vault **); ?int sshkey_load_private_type_fd(int fd, int type, const char *passphrase, -??? struct sshkey **keyp, char **commentp); +??? struct sshkey **keyp, char **commentp, struct sshkey_vault **vault_infop); ?int sshkey_perm_ok(int, const char *); ?int sshkey_in_file(struct sshkey *, const char *, int, int); ?int sshkey_check_revoked(struct sshkey *key, const char *revoked_keys_file); diff --git a/regress/Makefile b/regress/Makefile index 62794d25fc42..ae6f4dd09edc 100644 --- a/regress/Makefile +++ b/regress/Makefile @@ -92,7 +92,8 @@ LTESTS= ?? ?connect \ ??? ??? ?allow-deny-users \ ??? ??? ?authinfo \ ??? ??? ?sshsig \ -?? ??? ?keygen-comment +?? ??? ?keygen-comment \ +??????? keygen-private-information ? ? ?INTEROP_TESTS=?? ?putty-transfer putty-ciphers putty-kex conch-ciphers diff --git a/regress/keygen-private-information.sh b/regress/keygen-private-information.sh new file mode 100644 index 000000000000..a9959e919fd1 --- /dev/null +++ b/regress/keygen-private-information.sh @@ -0,0 +1,81 @@ +#??? Placed in the Public Domain. + +tid="Test information extraction from private key" + +check_private_key () { +?? ?file="$1" +?? ?format="$2" +?? ?comment="$3" +?? ?secret="$4" +?? ?rounds="$5" + +?? ?# construct expected output in $exp file +?? ?exp=$OBJ/$t-expected +?? ?# default format is RFC4716 +?? ?test -z "$format" && format="RFC4716" +?? ?# Currently PKCS8 is detected as PEM, should be fixed in ssh-keygen +?? ?test "$format" = "PKCS8" && format="PEM" +?? ?cat > $exp << EOF +$comment +Key protection details: +File format: $format +EOF +?? ?if [ -z "$secret" -o "$format" = "PEM" ]; then +?? ??? ?# For PEM format, passphrase is not detected yet, should be fixed in ssh-keygen +?? ??? ?echo "no passphrase" >> $exp +?? ?else +?? ??? ?cat >> $exp << EOF +cipher: aes256-ctr +kdf: bcrypt +rounds: $rounds +EOF +?? ?fi + +?? ?if ! ${SSHKEYGEN} -yv -P "${secret}" -f $file > $OBJ/$t-pub ; then +?? ??? ?fail "ssh-keygen -y failed for $t-key" +?? ?fi +?? ?if ! sed '1 s/[^ ]* [^ ]* \?//' $OBJ/$t-pub > $OBJ/$t-tmp ; then +?? ??? ?fail "sed failed for $t-key" +?? ?fi +?? ?if ! cmp $OBJ/$t-tmp $exp > /dev/null 2>&1; then +?? ??? ?fail "ssh-keygen -yv output is not the expected value for $t-key" +?? ??? ?diff $exp $OBJ/$t-tmp +?? ?fi +?? ?rm -f $OBJ/$t-pub $OBJ/$t-tmp $exp +} + +for fmt in '' PKCS8 PEM ; do +?? ?for secret in '' 'secret1'; do +?? ??? ?rounds_list="0" +?? ??? ?test -n "$secret" -a -z "$fmt" && rounds_list="2 16" +?? ??? ?for rounds in $rounds_list; do +?? ??? ??? ?for t in $SSH_KEYTYPES; do +?? ??? ??? ??? ?trace "generating $t key in '$fmt' format with '$secret' passphrase and '$rounds' rounds" +?? ??? ??? ??? ?rm -f $OBJ/$t-key* +?? ??? ??? ??? ?oldfmt="" +?? ??? ??? ??? ?case "$fmt" in +?? ??? ??? ??? ?PKCS8|PEM) oldfmt=1 ;; +?? ??? ??? ??? ?esac +?? ??? ??? ??? ?# Some key types like ssh-ed25519 and *@openssh.com are never +?? ??? ??? ??? ?# stored in old formats. +?? ??? ??? ??? ?case "$t" in +?? ??? ??? ??? ?ssh-ed25519|*openssh.com) test -z "$oldfmt" || continue ;; +?? ??? ??? ??? ?esac +?? ??? ??? ??? ?comment="foo bar" +?? ??? ??? ??? ?fmtarg="" +?? ??? ??? ??? ?test -z "$fmt" || fmtarg="-m $fmt" +?? ??? ??? ??? ?test "$rounds" = "0" || roundarg="-a $rounds" +?? ??? ??? ??? ?${SSHKEYGEN} $fmtarg $roundarg -N "${secret}" -C "${comment}" \ +?? ??? ??? ??? ???? -t $t -f $OBJ/$t-key >/dev/null 2>&1 || \ +?? ??? ??? ??? ??? ?fatal "keygen of $t in format $fmt failed" +?? ??? ??? ??? ?rm -f $OBJ/$t-key.pub # .pub file not used, remove it to be sure it is not used +?? ??? ??? ??? ?if [ ! -z "$oldfmt" ] ; then +?? ??? ??? ??? ??? ?# Comment cannot be recovered from old format keys. +?? ??? ??? ??? ??? ?comment="" +?? ??? ??? ??? ?fi +?? ??? ??? ??? ?check_private_key $OBJ/$t-key "${fmt}" "${comment}" "${secret}" "${rounds}" +?? ??? ??? ??? ?rm -f $OBJ/$t-key* +?? ??? ??? ?done +?? ??? ?done +?? ?done +done diff --git a/ssh-keygen.c b/ssh-keygen.c index d50ca5f28c51..6dd17c48be5e 100644 --- a/ssh-keygen.c +++ b/ssh-keygen.c @@ -310,7 +310,7 @@ ask_filename(struct passwd *pw, const char *prompt) ?} ? ?static struct sshkey * -load_identity(const char *filename, char **commentp) +load_identity(const char *filename, char **commentp, struct sshkey_vault **vault_infop) ?{ ??? ?char *pass; ??? ?struct sshkey *prv; @@ -318,7 +318,9 @@ load_identity(const char *filename, char **commentp) ? ??? ?if (commentp != NULL) ??? ??? ?*commentp = NULL; -?? ?if ((r = sshkey_load_private(filename, "", &prv, commentp)) == 0) +?? ?if (vault_infop != NULL) +?? ??? ?*vault_infop = NULL; +?? ?if ((r = sshkey_load_private(filename, "", &prv, commentp, vault_infop)) == 0) ??? ??? ?return prv; ??? ?if (r != SSH_ERR_KEY_WRONG_PASSPHRASE) ??? ??? ?fatal("Load key \"%s\": %s", filename, ssh_err(r)); @@ -326,7 +328,7 @@ load_identity(const char *filename, char **commentp) ??? ??? ?pass = xstrdup(identity_passphrase); ??? ?else ??? ??? ?pass = read_passphrase("Enter passphrase: ", RP_ALLOW_STDIN); -?? ?r = sshkey_load_private(filename, pass, &prv, commentp); +?? ?r = sshkey_load_private(filename, pass, &prv, commentp, vault_infop); ??? ?freezero(pass, strlen(pass)); ??? ?if (r != 0) ??? ??? ?fatal("Load key \"%s\": %s", filename, ssh_err(r)); @@ -429,7 +431,7 @@ do_convert_to(struct passwd *pw) ??? ?if (stat(identity_file, &st) == -1) ??? ??? ?fatal("%s: %s: %s", __progname, identity_file, strerror(errno)); ??? ?if ((r = sshkey_load_public(identity_file, &k, NULL)) != 0) -?? ??? ?k = load_identity(identity_file, NULL); +?? ??? ?k = load_identity(identity_file, NULL, NULL); ??? ?switch (convert_format) { ??? ?case FMT_RFC4716: ??? ??? ?do_convert_to_ssh2(pw, k); @@ -806,19 +808,33 @@ do_print_public(struct passwd *pw) ??? ?struct stat st; ??? ?int r; ??? ?char *comment = NULL; +?? ?struct sshkey_vault *vault_info = NULL; ? ??? ?if (!have_identity) ??? ??? ?ask_filename(pw, "Enter file in which the key is"); ??? ?if (stat(identity_file, &st) == -1) ??? ??? ?fatal("%s: %s", identity_file, strerror(errno)); -?? ?prv = load_identity(identity_file, &comment); +?? ?prv = load_identity(identity_file, &comment, &vault_info); ??? ?if ((r = sshkey_write(prv, stdout)) != 0) ??? ??? ?error("sshkey_write failed: %s", ssh_err(r)); ??? ?sshkey_free(prv); ??? ?if (comment != NULL && *comment != '\0') -?? ??? ?fprintf(stdout, " %s", comment); -?? ?fprintf(stdout, "\n"); +?? ??? ?printf(" %s", comment); +?? ?printf("\n"); +?? ?if (log_level_get() >= SYSLOG_LEVEL_VERBOSE) { +?? ??? ?printf("Key protection details:\n"); +?? ??? ?printf("File format: %s\n", sshkey_format_name(vault_info->format)); +?? ??? ?if ( (vault_info->ciphername == NULL || strcmp(vault_info->ciphername, "none") == 0) +?? ??? ?? || (vault_info->kdfname == NULL || strcmp(vault_info->kdfname, "none") == 0)) { +?? ??? ??? ?printf("no passphrase\n"); +?? ??? ?} else { +?? ??? ??? ?printf("cipher: %s\n", vault_info->ciphername); +?? ??? ??? ?printf("kdf: %s\n", vault_info->kdfname); +?? ??? ??? ?printf("rounds: %d\n", vault_info->rounds); +?? ??? ?} +?? ?} ??? ?free(comment); +?? ?sshkey_vault_free(vault_info); ??? ?exit(0); ?} ? @@ -920,7 +936,7 @@ fingerprint_private(const char *path) ??? ?if (pubkey == NULL || comment == NULL || *comment == '\0') { ??? ??? ?free(comment); ??? ??? ?if ((r = sshkey_load_private(path, NULL, -?? ??? ???? &privkey, &comment)) != 0) +?? ??? ???? &privkey, &comment, NULL)) != 0) ??? ??? ??? ?debug("load private \"%s\": %s", path, ssh_err(r)); ??? ?} ??? ?if (pubkey == NULL && privkey == NULL) @@ -1416,7 +1432,7 @@ do_change_passphrase(struct passwd *pw) ??? ?if (stat(identity_file, &st) == -1) ??? ??? ?fatal("%s: %s", identity_file, strerror(errno)); ??? ?/* Try to load the file with empty passphrase. */ -?? ?r = sshkey_load_private(identity_file, "", &private, &comment); +?? ?r = sshkey_load_private(identity_file, "", &private, &comment, NULL); ??? ?if (r == SSH_ERR_KEY_WRONG_PASSPHRASE) { ??? ??? ?if (identity_passphrase) ??? ??? ??? ?old_passphrase = xstrdup(identity_passphrase); @@ -1425,7 +1441,7 @@ do_change_passphrase(struct passwd *pw) ??? ??? ??? ???? read_passphrase("Enter old passphrase: ", ??? ??? ??? ???? RP_ALLOW_STDIN); ??? ??? ?r = sshkey_load_private(identity_file, old_passphrase, -?? ??? ???? &private, &comment); +?? ??? ???? &private, &comment, NULL); ??? ??? ?freezero(old_passphrase, strlen(old_passphrase)); ??? ??? ?if (r != 0) ??? ??? ??? ?goto badkey; @@ -1525,7 +1541,7 @@ do_change_comment(struct passwd *pw, const char *identity_comment) ??? ?if (stat(identity_file, &st) == -1) ??? ??? ?fatal("%s: %s", identity_file, strerror(errno)); ??? ?if ((r = sshkey_load_private(identity_file, "", -?? ???? &private, &comment)) == 0) +?? ???? &private, &comment, NULL)) == 0) ??? ??? ?passphrase = xstrdup(""); ??? ?else if (r != SSH_ERR_KEY_WRONG_PASSPHRASE) ??? ??? ?fatal("Cannot load private key \"%s\": %s.", @@ -1540,7 +1556,7 @@ do_change_comment(struct passwd *pw, const char *identity_comment) ??? ??? ??? ???? RP_ALLOW_STDIN); ??? ??? ?/* Try to load using the passphrase. */ ??? ??? ?if ((r = sshkey_load_private(identity_file, passphrase, -?? ??? ???? &private, &comment)) != 0) { +?? ??? ???? &private, &comment, NULL)) != 0) { ??? ??? ??? ?freezero(passphrase, strlen(passphrase)); ??? ??? ??? ?fatal("Cannot load private key \"%s\": %s.", ??? ??? ??? ???? identity_file, ssh_err(r)); @@ -1785,7 +1801,7 @@ do_ca_sign(struct passwd *pw, const char *ca_key_path, int prefer_agent, ??? ??? ?ca->flags |= SSHKEY_FLAG_EXT; ??? ?} else { ??? ??? ?/* CA key is assumed to be a private key on the filesystem */ -?? ??? ?ca = load_identity(tmp, NULL); +?? ??? ?ca = load_identity(tmp, NULL, NULL); ??? ?} ??? ?free(tmp); ? @@ -2494,7 +2510,7 @@ load_sign_key(const char *keypath, const struct sshkey *pubkey) ??? ??? ?debug("%s: %s looks like a public key, using private key " ??? ??? ???? "path %s instead", __func__, keypath, privpath); ??? ?} -?? ?if ((privkey = load_identity(privpath, NULL)) == NULL) { +?? ?if ((privkey = load_identity(privpath, NULL, NULL)) == NULL) { ??? ??? ?error("Couldn't load identity %s", keypath); ??? ??? ?goto done; ??? ?} diff --git a/ssh-keysign.c b/ssh-keysign.c index 3e3ea3e1481d..c9c20483b9a5 100644 --- a/ssh-keysign.c +++ b/ssh-keysign.c @@ -225,7 +225,7 @@ main(int argc, char **argv) ??? ??? ?if (key_fd[i] == -1) ??? ??? ??? ?continue; ??? ??? ?r = sshkey_load_private_type_fd(key_fd[i], KEY_UNSPEC, -?? ??? ???? NULL, &key, NULL); +?? ??? ???? NULL, &key, NULL, NULL); ??? ??? ?close(key_fd[i]); ??? ??? ?if (r != 0) ??? ??? ??? ?debug("parse key %d: %s", i, ssh_err(r)); diff --git a/sshconnect2.c b/sshconnect2.c index 1a6545edf026..7947f2da6584 100644 --- a/sshconnect2.c +++ b/sshconnect2.c @@ -1472,7 +1472,7 @@ load_identity_file(Identity *id) ??? ??? ??? ?} ??? ??? ?} ??? ??? ?switch ((r = sshkey_load_private_type(KEY_UNSPEC, id->filename, -?? ??? ???? passphrase, &private, &comment))) { +?? ??? ???? passphrase, &private, &comment, NULL))) { ??? ??? ?case 0: ??? ??? ??? ?break; ??? ??? ?case SSH_ERR_KEY_WRONG_PASSPHRASE: diff --git a/sshd.c b/sshd.c index 6f8f11a3bdac..42c19089a225 100644 --- a/sshd.c +++ b/sshd.c @@ -1789,7 +1789,7 @@ main(int ac, char **av) ??? ??? ?if (options.host_key_files[i] == NULL) ??? ??? ??? ?continue; ??? ??? ?if ((r = sshkey_load_private(options.host_key_files[i], "", -?? ??? ???? &key, NULL)) != 0 && r != SSH_ERR_SYSTEM_ERROR) +?? ??? ???? &key, NULL, NULL)) != 0 && r != SSH_ERR_SYSTEM_ERROR) ??? ??? ??? ?do_log2(ll, "Unable to load host key \"%s\": %s", ??? ??? ??? ???? options.host_key_files[i], ssh_err(r)); ??? ??? ?if (sshkey_is_sk(key) && diff --git a/sshkey.c b/sshkey.c index 1571e3d93878..4c1948a3752e 100644 --- a/sshkey.c +++ b/sshkey.c @@ -93,6 +93,26 @@ int?? ?sshkey_private_serialize_opt(struct sshkey *key, ?static int sshkey_from_blob_internal(struct sshbuf *buf, ???? struct sshkey **keyp, int allow_cert); ? +/* Supported format types */ +const char * +sshkey_format_name(enum sshkey_private_format format) { +?? ?const char *format_str; +?? ?switch (format) { +?? ?case SSHKEY_PRIVATE_OPENSSH: +?? ??? ?format_str = "RFC4716"; +?? ??? ?break; +?? ?case SSHKEY_PRIVATE_PKCS8: +?? ??? ?format_str = "PKCS8"; +?? ??? ?break; +?? ?case SSHKEY_PRIVATE_PEM: +?? ??? ?format_str = "PEM"; +?? ??? ?break; +?? ?default: +?? ??? ?format_str = "unknown"; +?? ?} +?? ?return format_str; +} + ?/* Supported key types */ ?struct keytype { ??? ?const char *name; @@ -679,6 +699,29 @@ sshkey_free(struct sshkey *k) ??? ?freezero(k, sizeof(*k)); ?} ? +struct sshkey_vault * +sshkey_vault_new() +{ +?? ?struct sshkey_vault *k = calloc(1, sizeof(*k)); +?? ?if (k == NULL) +?? ??? ?return NULL; +?? ?k->format = SSHKEY_PRIVATE_OPENSSH; +?? ?k->ciphername = NULL; +?? ?k->kdfname = NULL; +?? ?k->rounds = -1; +?? ?return k; +} + +void +sshkey_vault_free(struct sshkey_vault *k) +{ +?? ?if (k == NULL) +?? ??? ?return; +?? ?free(k->kdfname); +?? ?free(k); +?? ?return; +} + ?static int ?cert_compare(struct sshkey_cert *a, struct sshkey_cert *b) ?{ @@ -4029,7 +4072,7 @@ private2_uudecode(struct sshbuf *blob, struct sshbuf **decodedp) ? ?static int ?private2_decrypt(struct sshbuf *decoded, const char *passphrase, -??? struct sshbuf **decryptedp, struct sshkey **pubkeyp) +??? struct sshbuf **decryptedp, struct sshkey **pubkeyp, struct sshkey_vault **vault_infop) ?{ ??? ?char *ciphername = NULL, *kdfname = NULL; ??? ?const struct sshcipher *cipher = NULL; @@ -4038,12 +4081,21 @@ private2_decrypt(struct sshbuf *decoded, const char *passphrase, ??? ?struct sshbuf *kdf = NULL, *decrypted = NULL; ??? ?struct sshcipher_ctx *ciphercontext = NULL; ??? ?struct sshkey *pubkey = NULL; +?? ?struct sshkey_vault *vault_info = NULL; ??? ?u_char *key = NULL, *salt = NULL, *dp; ??? ?u_int blocksize, rounds, nkeys, encrypted_len, check1, check2; ? ??? ?if (decoded == NULL || decryptedp == NULL || pubkeyp == NULL) ??? ??? ?return SSH_ERR_INVALID_ARGUMENT; ? +?? ?if (vault_infop != NULL) { +?? ??? ?*vault_infop = NULL; +?? ?} +?? ?if ((vault_info = sshkey_vault_new()) == NULL) { +?? ??? ?r = SSH_ERR_ALLOC_FAIL; +?? ??? ?goto out; +?? ?} + ??? ?*decryptedp = NULL; ??? ?*pubkeyp = NULL; ? @@ -4074,10 +4126,18 @@ private2_decrypt(struct sshbuf *decoded, const char *passphrase, ??? ??? ?r = SSH_ERR_KEY_UNKNOWN_CIPHER; ??? ??? ?goto out; ??? ?} +?? ?if ((vault_info->ciphername = strdup(ciphername)) == NULL) { +?? ??? ?r = SSH_ERR_ALLOC_FAIL; +?? ??? ?goto out; +?? ?} ??? ?if (strcmp(kdfname, "none") != 0 && strcmp(kdfname, "bcrypt") != 0) { ??? ??? ?r = SSH_ERR_KEY_UNKNOWN_CIPHER; ??? ??? ?goto out; ??? ?} +?? ?if ((vault_info->kdfname = strdup(kdfname)) == NULL) { +?? ??? ?r = SSH_ERR_ALLOC_FAIL; +?? ??? ?goto out; +?? ?} ??? ?if (strcmp(kdfname, "none") == 0 && strcmp(ciphername, "none") != 0) { ??? ??? ?r = SSH_ERR_INVALID_FORMAT; ??? ??? ?goto out; @@ -4108,6 +4168,7 @@ private2_decrypt(struct sshbuf *decoded, const char *passphrase, ??? ??? ?if ((r = sshbuf_get_string(kdf, &salt, &slen)) != 0 || ??? ??? ???? (r = sshbuf_get_u32(kdf, &rounds)) != 0) ??? ??? ??? ?goto out; +?? ??? ?vault_info->rounds = rounds; ??? ??? ?if (bcrypt_pbkdf(passphrase, strlen(passphrase), salt, slen, ??? ??? ???? key, keylen + ivlen, rounds) < 0) { ??? ??? ??? ?r = SSH_ERR_INVALID_FORMAT; @@ -4155,6 +4216,10 @@ private2_decrypt(struct sshbuf *decoded, const char *passphrase, ??? ?decrypted = NULL; ??? ?*pubkeyp = pubkey; ??? ?pubkey = NULL; +?? ?if (vault_infop != NULL) { +?? ??? ?*vault_infop = vault_info; +?? ??? ?vault_info = NULL; +?? ?} ??? ?r = 0; ? out: ??? ?cipher_free(ciphercontext); @@ -4171,6 +4236,7 @@ private2_decrypt(struct sshbuf *decoded, const char *passphrase, ??? ?} ??? ?sshbuf_free(kdf); ??? ?sshbuf_free(decrypted); +?? ?sshkey_vault_free(vault_info); ??? ?return r; ?} ? @@ -4201,7 +4267,7 @@ private2_check_padding(struct sshbuf *decrypted) ? ?static int ?sshkey_parse_private2(struct sshbuf *blob, int type, const char *passphrase, -??? struct sshkey **keyp, char **commentp) +??? struct sshkey **keyp, char **commentp, struct sshkey_vault **vault_infop) ?{ ??? ?char *comment = NULL; ??? ?int r = SSH_ERR_INTERNAL_ERROR; @@ -4216,7 +4282,7 @@ sshkey_parse_private2(struct sshbuf *blob, int type, const char *passphrase, ??? ?/* Undo base64 encoding and decrypt the private section */ ??? ?if ((r = private2_uudecode(blob, &decoded)) != 0 || ??? ???? (r = private2_decrypt(decoded, passphrase, -?? ???? &decrypted, &pubkey)) != 0) +?? ???? &decrypted, &pubkey, vault_infop)) != 0) ??? ??? ?goto out; ? ??? ?if (type != KEY_UNSPEC && @@ -4521,15 +4587,18 @@ pem_passphrase_cb(char *buf, int size, int rwflag, void *u) ? ?static int ?sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type, -??? const char *passphrase, struct sshkey **keyp) +??? const char *passphrase, struct sshkey **keyp, struct sshkey_vault **vault_infop) ?{ ??? ?EVP_PKEY *pk = NULL; ??? ?struct sshkey *prv = NULL; +?? ?struct sshkey_vault *vault_info = NULL; ??? ?BIO *bio = NULL; ??? ?int r; ? ??? ?if (keyp != NULL) ??? ??? ?*keyp = NULL; +?? ?if (vault_infop != NULL) +?? ??? ?*vault_infop = NULL; ? ??? ?if ((bio = BIO_new(BIO_s_mem())) == NULL || sshbuf_len(blob) > INT_MAX) ??? ??? ?return SSH_ERR_ALLOC_FAIL; @@ -4538,6 +4607,13 @@ sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type, ??? ??? ?r = SSH_ERR_ALLOC_FAIL; ??? ??? ?goto out; ??? ?} +?? ?if ((vault_info = sshkey_vault_new()) == NULL) { +?? ??? ?r = SSH_ERR_ALLOC_FAIL; +?? ??? ?goto out; +?? ?} +?? ?// TODO: identify correctly PEM and PKCS8 format +?? ?vault_info->format = SSHKEY_PRIVATE_PEM; +?? ?// TODO: put the correct ciphername, kdfname and round if a passphrase is used ? ??? ?clear_libcrypto_errors(); ??? ?if ((pk = PEM_read_bio_PrivateKey(bio, NULL, pem_passphrase_cb, @@ -4614,17 +4690,22 @@ sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type, ??? ??? ?*keyp = prv; ??? ??? ?prv = NULL; ??? ?} +?? ?if (vault_infop != NULL) { +?? ??? ?*vault_infop = vault_info; +?? ??? ?vault_info = NULL; +?? ?} ? out: ??? ?BIO_free(bio); ??? ?EVP_PKEY_free(pk); ??? ?sshkey_free(prv); +?? ?sshkey_vault_free(vault_info); ??? ?return r; ?} ?#endif /* WITH_OPENSSL */ ? ?int ?sshkey_parse_private_fileblob_type(struct sshbuf *blob, int type, -??? const char *passphrase, struct sshkey **keyp, char **commentp) +??? const char *passphrase, struct sshkey **keyp, char **commentp, struct sshkey_vault **vault_infop) ?{ ??? ?int r = SSH_ERR_INTERNAL_ERROR; ? @@ -4632,22 +4713,24 @@ sshkey_parse_private_fileblob_type(struct sshbuf *blob, int type, ??? ??? ?*keyp = NULL; ??? ?if (commentp != NULL) ??? ??? ?*commentp = NULL; +?? ?if (vault_infop != NULL) +?? ??? ?*vault_infop = NULL; ? ??? ?switch (type) { ??? ?case KEY_ED25519: ??? ?case KEY_XMSS: ??? ??? ?/* No fallback for new-format-only keys */ ??? ??? ?return sshkey_parse_private2(blob, type, passphrase, -?? ??? ???? keyp, commentp); +?? ??? ???? keyp, commentp, vault_infop); ??? ?default: ??? ??? ?r = sshkey_parse_private2(blob, type, passphrase, keyp, -?? ??? ???? commentp); +?? ??? ???? commentp, vault_infop); ??? ??? ?/* Only fallback to PEM parser if a format error occurred. */ ??? ??? ?if (r != SSH_ERR_INVALID_FORMAT) ??? ??? ??? ?return r; ?#ifdef WITH_OPENSSL ??? ??? ?return sshkey_parse_private_pem_fileblob(blob, type, -?? ??? ???? passphrase, keyp); +?? ??? ???? passphrase, keyp, vault_infop); ?#else ??? ??? ?return SSH_ERR_INVALID_FORMAT; ?#endif /* WITH_OPENSSL */ @@ -4664,7 +4747,7 @@ sshkey_parse_private_fileblob(struct sshbuf *buffer, const char *passphrase, ??? ??? ?*commentp = NULL; ? ??? ?return sshkey_parse_private_fileblob_type(buffer, KEY_UNSPEC, -?? ???? passphrase, keyp, commentp); +?? ???? passphrase, keyp, commentp, NULL); ?} ? ?void diff --git a/sshkey.h b/sshkey.h index 9c1d4f6372f6..7b7de828d9ce 100644 --- a/sshkey.h +++ b/sshkey.h @@ -99,6 +99,8 @@ enum sshkey_private_format { ??? ?SSHKEY_PRIVATE_PEM = 1, ??? ?SSHKEY_PRIVATE_PKCS8 = 2, ?}; +const char * +sshkey_format_name(enum sshkey_private_format); ? ?/* key is stored in external hardware */ ?#define SSHKEY_FLAG_EXT?? ??? ?0x0001 @@ -162,6 +164,16 @@ struct sshkey_sig_details { ??? ?uint8_t sk_flags;?? ?/* U2F signature flags; see ssh-sk.h */ ?}; ? +/* Key storage parameters in private key file */ +struct sshkey_vault { +?? ?enum sshkey_private_format format; +?? ?char *ciphername; +?? ?char *kdfname; +?? ?int rounds; +}; +struct sshkey_vault?? ?*sshkey_vault_new(); +void?? ??? ?sshkey_vault_free(struct sshkey_vault *); + ?struct sshkey?? ?*sshkey_new(int); ?void?? ??? ? sshkey_free(struct sshkey *); ?int?? ??? ? sshkey_equal_public(const struct sshkey *, @@ -258,7 +270,7 @@ int?? ?sshkey_private_to_fileblob(struct sshkey *key, struct sshbuf *blob, ?int?? ?sshkey_parse_private_fileblob(struct sshbuf *buffer, ???? const char *passphrase, struct sshkey **keyp, char **commentp); ?int?? ?sshkey_parse_private_fileblob_type(struct sshbuf *blob, int type, -??? const char *passphrase, struct sshkey **keyp, char **commentp); +??? const char *passphrase, struct sshkey **keyp, char **commentp, struct sshkey_vault **vault_infop); ?int?? ?sshkey_parse_pubkey_from_private_fileblob_type(struct sshbuf *blob, ???? int type, struct sshkey **pubkeyp); ? -- 2.17.1
On 25/04/2020 at 02:58, Lo?c wrote :> Add private key protection information extraction to shh-keygen using -v > option on top of -y option which is already parsing the private key. > > Technically, the passphrase isn't necessary to do this, but it is the > most logical thing to do for me. > > Adding this to -l option is not appropriate because fingerprinting is > using the .pub file when available. > > An other idea is to add a new option, I can do it if you prefer. > > Also, I'm laking information for information extraction from PEM and > PKCS8 file format, I'm OK to have a pointer to implement this correctly. > > This patch is also adding a regression test for the functionnality. > > --- > > ?authfile.c??????????????????????????? |? 16 ++-- > ?authfile.h??????????????????????????? |?? 7 +- > ?regress/Makefile????????????????????? |?? 3 +- > ?regress/keygen-private-information.sh |? 81 +++++++++++++++++++++ > ?ssh-keygen.c????????????????????????? |? 44 +++++++---- > ?ssh-keysign.c???????????????????????? |?? 2 +- > ?sshconnect2.c???????????????????????? |?? 2 +- > ?sshd.c??????????????????????????????? |?? 2 +- > ?sshkey.c????????????????????????????? | 101 +++++++++++++++++++++++--- > ?sshkey.h????????????????????????????? |? 14 +++- > ?10 files changed, 234 insertions(+), 38 deletions(-) > ?create mode 100644 regress/keygen-private-information.sh >In since I discovered the -Z option, I'm adding here a regression test for this option, the patch below applies on top on the upper one I'm replying to. Hope it is useful. --- ?regress/keygen-private-information.sh | 82 ++++++++++++++++----------- ?1 file changed, 50 insertions(+), 32 deletions(-) diff --git a/regress/keygen-private-information.sh b/regress/keygen-private-information.sh index a9959e919fd1..ddf74eb95c3c 100644 --- a/regress/keygen-private-information.sh +++ b/regress/keygen-private-information.sh @@ -7,7 +7,8 @@ check_private_key () { ???? format="$2" ???? comment="$3" ???? secret="$4" -??? rounds="$5" +??? cipher="$5" +??? rounds="$6" ? ???? # construct expected output in $exp file ???? exp=$OBJ/$t-expected @@ -25,7 +26,7 @@ EOF ???? ??? echo "no passphrase" >> $exp ???? else ???? ??? cat >> $exp << EOF -cipher: aes256-ctr +cipher: $cipher ?kdf: bcrypt ?rounds: $rounds ?EOF @@ -44,37 +45,54 @@ EOF ???? rm -f $OBJ/$t-pub $OBJ/$t-tmp $exp ?} ? -for fmt in '' PKCS8 PEM ; do +for fmt in '' RFC4716 PKCS8 PEM ; do ???? for secret in '' 'secret1'; do -??? ??? rounds_list="0" -??? ??? test -n "$secret" -a -z "$fmt" && rounds_list="2 16" -??? ??? for rounds in $rounds_list; do -??? ??? ??? for t in $SSH_KEYTYPES; do -??? ??? ??? ??? trace "generating $t key in '$fmt' format with '$secret' passphrase and '$rounds' rounds" -??? ??? ??? ??? rm -f $OBJ/$t-key* -??? ??? ??? ??? oldfmt="" -??? ??? ??? ??? case "$fmt" in -??? ??? ??? ??? PKCS8|PEM) oldfmt=1 ;; -??? ??? ??? ??? esac -??? ??? ??? ??? # Some key types like ssh-ed25519 and *@openssh.com are never -??? ??? ??? ??? # stored in old formats. -??? ??? ??? ??? case "$t" in -??? ??? ??? ??? ssh-ed25519|*openssh.com) test -z "$oldfmt" || continue ;; -??? ??? ??? ??? esac -??? ??? ??? ??? comment="foo bar" -??? ??? ??? ??? fmtarg="" -??? ??? ??? ??? test -z "$fmt" || fmtarg="-m $fmt" -??? ??? ??? ??? test "$rounds" = "0" || roundarg="-a $rounds" -??? ??? ??? ??? ${SSHKEYGEN} $fmtarg $roundarg -N "${secret}" -C "${comment}" \ -??? ??? ??? ??? ??? -t $t -f $OBJ/$t-key >/dev/null 2>&1 || \ -??? ??? ??? ??? ??? fatal "keygen of $t in format $fmt failed" -??? ??? ??? ??? rm -f $OBJ/$t-key.pub # .pub file not used, remove it to be sure it is not used -??? ??? ??? ??? if [ ! -z "$oldfmt" ] ; then -??? ??? ??? ??? ??? # Comment cannot be recovered from old format keys. -??? ??? ??? ??? ??? comment="" -??? ??? ??? ??? fi -??? ??? ??? ??? check_private_key $OBJ/$t-key "${fmt}" "${comment}" "${secret}" "${rounds}" -??? ??? ??? ??? rm -f $OBJ/$t-key* +??? ??? cipher_list="default" +??? ??? test -n "$secret" -a -z "$fmt" && cipher_list=`${SSH} -Q cipher` +??? ??? for cipher in $cipher_list; do +??? ??? ??? rounds_list="default" +??? ??? ??? test -n "$secret" -a -z "$fmt" && rounds_list="2 16" +??? ??? ??? for rounds in $rounds_list; do +??? ??? ??? ??? for t in $SSH_KEYTYPES; do +??? ??? ??? ??? ??? trace "generating $t key in '$fmt' format with '$secret' passphrase, '$cipher' cipher and '$rounds' rounds" +??? ??? ??? ??? ??? rm -f $OBJ/$t-key* +??? ??? ??? ??? ??? oldfmt="" +??? ??? ??? ??? ??? case "$fmt" in +??? ??? ??? ??? ??? PKCS8|PEM) oldfmt=1 ;; +??? ??? ??? ??? ??? esac +??? ??? ??? ??? ??? # Some key types like ssh-ed25519 and *@openssh.com are never +??? ??? ??? ??? ??? # stored in old formats. +??? ??? ??? ??? ??? case "$t" in +??? ??? ??? ??? ??? ssh-ed25519|*openssh.com) test -z "$oldfmt" || continue ;; +??? ??? ??? ??? ??? esac +??? ??? ??? ??? ??? comment="foo bar" +??? ??? ??? ??? ??? fmtarg="" +??? ??? ??? ??? ??? test -z "$fmt" || fmtarg="-m $fmt" +??? ??? ??? ??? ??? if test "$rounds" = "default" ; then +??? ??? ??? ??? ??? ??? rounds=16; +??? ??? ??? ??? ??? ??? roundarg="" +??? ??? ??? ??? ??? else +??? ??? ??? ??? ??? ??? roundarg="-a $rounds"; +??? ??? ??? ??? ??? fi +??? ??? ??? ??? ??? if test "$cipher" = "default" ; then +??? ??? ??? ??? ??? ??? cipher="aes256-ctr" ; +??? ??? ??? ??? ??? ??? cipherarg="" +??? ??? ??? ??? ??? else +??? ??? ??? ??? ??? ??? cipherarg="-Z $cipher"; +??? ??? ??? ??? ??? fi +??? ??? ??? ??? ??? ${SSHKEYGEN} $fmtarg $cipherarg $roundarg \ +??? ??? ??? ??? ??? ??? -N "${secret}" -C "${comment}" \ +??? ??? ??? ??? ??? ??? -t $t -f $OBJ/$t-key >/dev/null 2>&1 || \ +??? ??? ??? ??? ??? ??? fatal "keygen of $t in format $fmt failed" +??? ??? ??? ??? ??? rm -f $OBJ/$t-key.pub # .pub file not used, remove it to be sure it is not used +??? ??? ??? ??? ??? if [ ! -z "$oldfmt" ] ; then +??? ??? ??? ??? ??? ??? # Comment cannot be recovered from old format keys. +??? ??? ??? ??? ??? ??? comment="" +??? ??? ??? ??? ??? fi +??? ??? ??? ??? ??? check_private_key $OBJ/$t-key "${fmt}" "${comment}" \ +??? ??? ??? ??? ??? ??? "${secret}" "${cipher}" "${rounds}" +??? ??? ??? ??? ??? rm -f $OBJ/$t-key* +??? ??? ??? ??? done ???? ??? ??? done ???? ??? done ???? done -- 2.17.1
Loïc
2020-Apr-25 21:48 UTC
[PATCH 5/3] ssh-keygen: -Z cipher can be "none" test it in regression and, report it correctly in -yv option
On 25/04/2020 at 23:35, Lo?c wrote :> On 25/04/2020 at 02:58, Lo?c wrote : >> Add private key protection information extraction to shh-keygen using -v >> option on top of -y option which is already parsing the private key. >> >> Technically, the passphrase isn't necessary to do this, but it is the >> most logical thing to do for me. >> >> Adding this to -l option is not appropriate because fingerprinting is >> using the .pub file when available. >> >> An other idea is to add a new option, I can do it if you prefer. >> >> Also, I'm laking information for information extraction from PEM and >> PKCS8 file format, I'm OK to have a pointer to implement this correctly. >> >> This patch is also adding a regression test for the functionnality. >> >> --- >> >> ?authfile.c??????????????????????????? |? 16 ++-- >> ?authfile.h??????????????????????????? |?? 7 +- >> ?regress/Makefile????????????????????? |?? 3 +- >> ?regress/keygen-private-information.sh |? 81 +++++++++++++++++++++ >> ?ssh-keygen.c????????????????????????? |? 44 +++++++---- >> ?ssh-keysign.c???????????????????????? |?? 2 +- >> ?sshconnect2.c???????????????????????? |?? 2 +- >> ?sshd.c??????????????????????????????? |?? 2 +- >> ?sshkey.c????????????????????????????? | 101 +++++++++++++++++++++++--- >> ?sshkey.h????????????????????????????? |? 14 +++- >> ?10 files changed, 234 insertions(+), 38 deletions(-) >> ?create mode 100644 regress/keygen-private-information.sh >> > In since I discovered the -Z option, I'm adding here a regression test > for this option, the patch below applies on top on the upper one I'm > replying to. >In fact "none" cypher is allowed here is a patch to test it in regression and report it correctly in -yv option --- ?regress/keygen-private-information.sh | 2 +- ?ssh-keygen.c????????????????????????? | 3 +-- ?2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/regress/keygen-private-information.sh b/regress/keygen-private-information.sh index ddf74eb95c3c..22ad6429a079 100644 --- a/regress/keygen-private-information.sh +++ b/regress/keygen-private-information.sh @@ -48,7 +48,7 @@ EOF ?for fmt in '' RFC4716 PKCS8 PEM ; do ???? for secret in '' 'secret1'; do ???? ??? cipher_list="default" -??? ??? test -n "$secret" -a -z "$fmt" && cipher_list=`${SSH} -Q cipher` +??? ??? test -n "$secret" -a -z "$fmt" && cipher_list=`${SSH} -Q cipher`" none" ???? ??? for cipher in $cipher_list; do ???? ??? ??? rounds_list="default" ???? ??? ??? test -n "$secret" -a -z "$fmt" && rounds_list="2 16" diff --git a/ssh-keygen.c b/ssh-keygen.c index a848edc33b5d..030b12e5b897 100644 --- a/ssh-keygen.c +++ b/ssh-keygen.c @@ -824,8 +824,7 @@ do_print_public(struct passwd *pw) ???? if (log_level_get() >= SYSLOG_LEVEL_VERBOSE) { ???? ??? printf("Key protection details:\n"); ???? ??? printf("File format: %s\n", sshkey_format_name(vault_info->format)); -??? ??? if ( (vault_info->ciphername == NULL || strcmp(vault_info->ciphername, "none") == 0) -??? ??? ? || (vault_info->kdfname == NULL || strcmp(vault_info->kdfname, "none") == 0)) { +??? ??? if (vault_info->kdfname == NULL || strcmp(vault_info->kdfname, "none") == 0) { ???? ??? ??? printf("no passphrase\n"); ???? ??? } else { ???? ??? ??? printf("cipher: %s\n", vault_info->ciphername); -- 2.17.1
Seemingly Similar Threads
- [PATCH v2] Remove sshkey_load_private()
- [PATCH] regression of comment extraction in private key file without passphrase
- [Bug 2207] New: Potential NULL deference, found using coverity
- [Bug 3068] New: Duplicate code in sshkey_load_private() function
- [patch] Make passphrase-protected SSHv1 keys work again