diff --git a/Recipe b/Recipe index 802eebe8..a84c646a 100644 --- a/Recipe +++ b/Recipe @@ -343,13 +343,14 @@ pageant : [G] winpgnt pageant sshrsa sshpubk sshdes ARITH sshmd5 version + tree234 MISC sshaes sshsha winsecur winpgntc aqsync sshdss sshsh256 + sshsh512 winutils sshecc winmisc winmiscs winhelp conf pageant.res + sshauxcrypt sshhmac wincapi winnps winnpc winhsock errsock winnet - + winhandl callback be_misc winselgui winhandl sshsha3 LIBS + + winhandl callback be_misc winselgui winhandl sshsha3 sshblake2 + + sshargon2 LIBS puttygen : [G] winpgen KEYGEN SSHPRIME sshdes ARITH sshmd5 version + sshrand winnoise sshsha winstore MISC winctrls sshrsa sshdss winmisc + sshpubk sshaes sshsh256 sshsh512 IMPORT winutils puttygen.res + tree234 notiming winhelp winnojmp CONF LIBS wintime sshecc sshprng - + sshauxcrypt sshhmac winsecur winmiscs sshsha3 + + sshauxcrypt sshhmac winsecur winmiscs sshsha3 sshblake2 sshargon2 pterm : [X] GTKTERM uxmisc misc ldisc settings uxpty uxsel BE_NONE uxstore + uxsignal CHARSET cmdline uxpterm version time xpmpterm xpmptcfg @@ -368,7 +369,7 @@ PUTTYGEN_UNIX = KEYGEN SSHPRIME sshdes ARITH sshmd5 version sshprng + sshrand uxnoise sshsha MISC sshrsa sshdss uxcons uxstore uxmisc + sshpubk sshaes sshsh256 sshsh512 IMPORT puttygen.res time tree234 + uxgen notiming CONF sshecc sshsha3 uxnogtk sshauxcrypt sshhmac - + uxpoll uxutils + + uxpoll uxutils sshblake2 sshargon2 puttygen : [U] cmdgen PUTTYGEN_UNIX cgtest : [UT] cgtest PUTTYGEN_UNIX @@ -381,7 +382,8 @@ pageant : [X] uxpgnt uxagentc aqsync pageant sshrsa sshpubk sshdes ARITH + sshmd5 version tree234 misc sshaes sshsha sshdss sshsh256 sshsh512 + sshecc CONF uxsignal nocproxy nogss be_none x11fwd ux_x11 uxcons + gtkask gtkmisc nullplug logging UXMISC uxagentsock utils memory - + sshauxcrypt sshhmac sshprng uxnoise uxcliloop sshsha3 + + sshauxcrypt sshhmac sshprng uxnoise uxcliloop sshsha3 sshblake2 + + sshargon2 ptermapp : [XT] GTKTERM uxmisc misc ldisc settings uxpty uxsel BE_NONE uxstore + uxsignal CHARSET uxpterm version time xpmpterm xpmptcfg diff --git a/cmdgen.c b/cmdgen.c index cd469fa0..69073386 100644 --- a/cmdgen.c +++ b/cmdgen.c @@ -1029,7 +1029,8 @@ int main(int argc, char **argv) } } else { assert(ssh2key); - ret = ppk_save_f(outfilename, ssh2key, new_passphrase); + ret = ppk_save_f(outfilename, ssh2key, new_passphrase, + &ppk_save_default_parameters); if (!ret) { fprintf(stderr, "puttygen: unable to save SSH-2 private key\n"); RETURN(1); diff --git a/doc/pubkeyfmt.but b/doc/pubkeyfmt.but index d3cea441..d1c92d6d 100644 --- a/doc/pubkeyfmt.but +++ b/doc/pubkeyfmt.but @@ -71,7 +71,7 @@ about what's stored in it and how. They look like this: \e bbbbbbbbbbbbbbbbbb \s{version} is a decimal number giving the version number of the file -format itself. The current file format version is 2. +format itself. The current file format version is 3. \s{algorithm-name} is the SSH protocol identifier for the public key algorithm that this key is used for (such as \cq{ssh-dss} or @@ -103,6 +103,35 @@ base64 data is split across multiple lines. But if you remove the newlines from the middle of this section, the resulting base64 blob is in the right format to go in an \c{authorized_keys} line. +If the key is encrypted (i.e. \s{encryption-type} is not \cq{none}), +then the next thing that appears is a sequence of lines specifying how +the keys for encrypting the file are to be derived from the +passphrase: + +\c Key-Derivation: argon2-flavour +\e bbbbbbbbbbbbbb +\c Argon2-Memory: decimal-integer +\e bbbbbbbbbbbbbbb +\c Argon2-Passes: decimal-integer +\e bbbbbbbbbbbbbbb +\c Argon2-Parallelism: decimal-integer +\e bbbbbbbbbbbbbbb +\c Argon2-Salt: hex-string +\e bbbbbbbbbb + +\s{argon2-flavour} is one of the identifiers \cq{Argon2d}, +\cq{Argon2i} or \cq{Argon2id}, all describing variants of the Argon2 +password-hashing function. + +The three integer values are used as parameters for Argon2, which +allows you to configure the amount of memory used (in Kb), the number +of passes of the algorithm to run (to tune its running time), and the +degree of parallelism required by the hash function. The salt is +decoded into a sequence of binary bytes and used as an additional +input to Argon2. (It is chosen randomly when the key file is written, +so that a guessing attack can't be mounted in parallel against +multiple key files.) + The next part of the file gives the private key. This is base64-encoded in the same way: @@ -119,8 +148,8 @@ shown above: plain text. \b If \s{encryption-type} is \cq{aes256-cbc}, then this data is -encrypted using AES, with a 256-bit key length, in the CBC cipher mode -with an all-zero initialisation vector. The key is derived from the +encrypted using AES, with a 256-bit key length, in the CBC cipher +mode. The key and initialisation vector are derived from the passphrase: see \k{ppk-keys}. \lcont{ @@ -144,8 +173,9 @@ The final thing in the key file is the MAC: \c Private-MAC: hex-mac-data \e bbbbbbbbbbbb -\s{hex-mac-data} is a hexadecimal-encoded value, generated using the -HMAC-SHA-1 algorithm with the following binary data as input: +\s{hex-mac-data} is a hexadecimal-encoded value, 64 digits long (i.e. +32 bytes), generated using the HMAC-SHA-256 algorithm with the +following binary data as input: \b \cw{string}: the \s{algorithm-name} header field. @@ -245,30 +275,97 @@ public point. \H{ppk-keys} Key derivation -When a key file is encrypted, the encryption key is derived from the -passphrase by means of generating a sequence of hashes, concatenating -them, and taking an appropriate-length prefix of the resulting -sequence. +When a key file is encrypted, there are three pieces of key material +that need to be computed from the passphrase: -Each hash in the sequence is a SHA-1 hash of the following data: +\b the key for the symmetric cipher used to encrypt the private key -\b \cw{uint32}: a sequence number. This is 0 in the first hash, and -increments by 1 each time after that. +\b the initialisation vector for that cipher encryption + +\b the key for the MAC. + +If \s{encryption-type} is \cq{aes256-cbc}, then the symmetric cipher +key is 32 bytes long, and the initialisation vector is 16 bytes (one +cipher block). The length of the MAC key is also chosen to be 32 +bytes. + +If \s{encryption-type} is \cq{none}, then all three of these pieces of +data have zero length. (The MAC is still generated and checked in the +key file format, but it has a zero-length key.) + +If the amount of key material required is not zero, then the +passphrase is fed to the Argon2 key derivation function, in whichever +mode is described in the \cq{Key-Derivation} header in the key file, +with parameters derived from the various +\q{\cw{Argon2-}\e{Parameter}\cw{:}} headers. + +(If the key is unencrypted, then all those headers are omitted, and +Argon2 is not run at all.) + +Argon2 takes two extra string inputs in addition to the passphrase and +the salt: a secret key, and some \q{associated data}. In PPK's use of +Argon2, these are both set to the empty string. + +The \q{tag length} parameter to Argon2 (i.e. the amount of data it is +asked to output) is set to the sum of the lengths of all of the data +items required, i.e. (cipher key length + IV length + MAC key length). +The output data is interpreted as the concatenation of the cipher key, +the IV and the MAC key, in that order. + +So, for \cq{aes256-cbc}, the tag length will be 32+16+32\_=\_80 bytes; +of the 80 bytes of output data, the first 32 bytes are used as the +256-bit AES key, the next 16 as the CBC IV, and the final 32 bytes as +the HMAC-SHA-256 key. + +\H{ppk-old} Older versions of the PPK format + +\S{ppk-v2} Version 2 + +PPK version 2 was used by PuTTY 0.52 to 0.74 inclusive. + +In PPK version 2, the MAC algorithm used was HMAC-SHA-1 (so the +\cw{Private-MAC} line contained only 40 hex digits). + +The \cq{Key-Derivation:} header and all the +\q{\cw{Argon2-}\e{Parameter}\cw{:}} headers were absent. Instead of +using Argon2, the key material for encrypting the private blob was +derived from the passphrase in a totally different way, as follows. + +The cipher key for \cq{aes256-cbc} was constructed by generating two +SHA-1 hashes, concatenating them, and taking the first 32 bytes of the +result. (So you'd get all 20 bytes of the first hash output, and the +first 12 of the second). Each hash preimage was as follows: + +\b \cw{uint32}: a sequence number. This is 0 in the first hash, and 1 +in the second. (The idea was to extend this mechanism to further +hashes by continuing to increment the sequence number, if future +changes required even longer keys.) \b the passphrase, without any prefix length field. -The MAC key is also derived from the passphrase. It is a single SHA-1 -hash of the following data: +In PPK v2, the CBC initialisation vector was all zeroes. + +The MAC key was 20 bytes long, and was a single SHA-1 hash of the +following data: \b the fixed string \cq{putty-private-key-file-mac-key}, without any prefix length field. -\b the passphrase, without any prefix length field. (If the key file -is unencrypted, the MAC is still computed in the same way, and the -passphrase is taken to be the empty string for the purpose of deriving -the MAC key.) +\b the passphrase, without any prefix length field. (If the key is +stored unencrypted, the passphrase was taken to be the empty string +for these purposes.) -\H{ppk-v1} PPK version 1 +\S{ppk-v1} Version 1 + +PPK version 1 was a badly designed format, only used during initial +development, and not recommended for production use. + +PPK version 1 was never used by a released version of PuTTY. It was +only emitted by some early development snapshots between version 0.51 +(which did not support SSH-2 public keys at all) and 0.52 (which +already used version 2 of this file format). I \e{hope} there are no +PPK v1 files in use anywhere. But just in case, the old badly designed +format is documented here anyway. In PPK version 1, the input to the MAC does not include any of the header fields or the public key. It is simply the private key data @@ -309,8 +406,3 @@ In an \e{unencrypted} version 1 key file, the MAC is replaced by a plain SHA-1 hash of the private key data. This is indicated by the \cq{Private-MAC:} header being replaced with \cq{Private-Hash:} instead. - -PPK version 1 is not recommended for use! It was only emitted in some -early development snapshots between version 0.51 (which did not -support SSH-2 public keys at all) and 0.52 (which already used version -2 of this file format). diff --git a/ssh.h b/ssh.h index b25288a8..d09e003a 100644 --- a/ssh.h +++ b/ssh.h @@ -1234,9 +1234,22 @@ int rsa1_load_s(BinarySource *src, RSAKey *key, int rsa1_load_f(const Filename *filename, RSAKey *key, const char *passphrase, const char **errorstr); -strbuf *ppk_save_sb(ssh2_userkey *key, const char *passphrase); +typedef struct ppk_save_parameters { + Argon2Flavour argon2_flavour; + uint32_t argon2_mem; /* in Kb */ + bool argon2_passes_auto; + union { + uint32_t argon2_passes; /* if auto == false */ + uint32_t argon2_milliseconds; /* if auto == true */ + }; + uint32_t argon2_parallelism; +} ppk_save_parameters; +extern const ppk_save_parameters ppk_save_default_parameters; + +strbuf *ppk_save_sb(ssh2_userkey *key, const char *passphrase, + const ppk_save_parameters *params); bool ppk_save_f(const Filename *filename, ssh2_userkey *key, - const char *passphrase); + const char *passphrase, const ppk_save_parameters *params); strbuf *rsa1_save_sb(RSAKey *key, const char *passphrase); bool rsa1_save_f(const Filename *filename, RSAKey *key, const char *passphrase); diff --git a/sshpubk.c b/sshpubk.c index be9569c1..e1f9e921 100644 --- a/sshpubk.c +++ b/sshpubk.c @@ -10,6 +10,7 @@ #include #include #include +#include #include "putty.h" #include "mpint.h" @@ -595,11 +596,42 @@ static const struct ppk_cipher ppk_cipher_aes256_cbc = { "aes256-cbc", 16, 32, 1 static void ssh2_ppk_derive_keys( unsigned fmt_version, const struct ppk_cipher *ciphertype, ptrlen passphrase, strbuf *storage, ptrlen *cipherkey, ptrlen *cipheriv, - ptrlen *mackey) + ptrlen *mackey, ptrlen passphrase_salt, ppk_save_parameters *params) { size_t mac_keylen; switch (fmt_version) { + case 3: { + if (ciphertype->keylen == 0) { + mac_keylen = 0; + break; + } + ptrlen empty = PTRLEN_LITERAL(""); + + mac_keylen = 32; + + uint32_t taglen = ciphertype->keylen + ciphertype->ivlen + mac_keylen; + + if (params->argon2_passes_auto) { + uint32_t passes; + + argon2_choose_passes( + params->argon2_flavour, params->argon2_mem, + params->argon2_milliseconds, &passes, + params->argon2_parallelism, taglen, + passphrase, passphrase_salt, empty, empty, storage); + + params->argon2_passes_auto = false; + params->argon2_passes = passes; + } else { + argon2(params->argon2_flavour, params->argon2_mem, + params->argon2_passes, params->argon2_parallelism, taglen, + passphrase, passphrase_salt, empty, empty, storage); + } + + break; + } + case 2: case 1: { /* Counter-mode iteration to generate cipher key data. */ @@ -645,6 +677,18 @@ static int userkey_parse_line_counter(const char *text) return -1; } +static bool str_to_uint32_t(const char *s, uint32_t *out) +{ + char *endptr; + unsigned long converted = strtoul(s, &endptr, 10); + if (*s && !*endptr && converted <= ~(uint32_t)0) { + *out = converted; + return true; + } else { + return false; + } +} + ssh2_userkey *ppk_load_s(BinarySource *src, const char *passphrase, const char **errorstr) { @@ -652,12 +696,14 @@ ssh2_userkey *ppk_load_s(BinarySource *src, const char *passphrase, const ssh_keyalg *alg; ssh2_userkey *ret; strbuf *public_blob, *private_blob, *cipher_mac_keys_blob; + strbuf *passphrase_salt = strbuf_new(); ptrlen cipherkey, cipheriv, mackey; const struct ppk_cipher *ciphertype; int i; bool is_mac; unsigned fmt_version; const char *error = NULL; + ppk_save_parameters params; ret = NULL; /* return NULL for most errors */ encryption = comment = mac = NULL; @@ -668,7 +714,9 @@ ssh2_userkey *ppk_load_s(BinarySource *src, const char *passphrase, error = "no header line found in key file"; goto error; } - if (0 == strcmp(header, "PuTTY-User-Key-File-2")) { + if (0 == strcmp(header, "PuTTY-User-Key-File-3")) { + fmt_version = 3; + } else if (0 == strcmp(header, "PuTTY-User-Key-File-2")) { fmt_version = 2; } else if (0 == strcmp(header, "PuTTY-User-Key-File-1")) { /* this is an old key file; warn and then continue */ @@ -713,6 +761,9 @@ ssh2_userkey *ppk_load_s(BinarySource *src, const char *passphrase, if ((comment = read_body(src)) == NULL) goto error; + memset(¶ms, 0, sizeof(params)); /* in particular, sets + * passes_auto=false */ + /* Read the Public-Lines header line and the public blob. */ if (!read_header(src, header) || 0 != strcmp(header, "Public-Lines")) goto error; @@ -726,6 +777,75 @@ ssh2_userkey *ppk_load_s(BinarySource *src, const char *passphrase, if (!read_blob(src, i, BinarySink_UPCAST(public_blob))) goto error; + if (fmt_version >= 3 && ciphertype->keylen != 0) { + /* Read Argon2 key derivation parameters. */ + if (!read_header(src, header) || 0 != strcmp(header, "Key-Derivation")) + goto error; + if ((b = read_body(src)) == NULL) + goto error; + if (!strcmp(b, "Argon2d")) { + params.argon2_flavour = Argon2d; + } else if (!strcmp(b, "Argon2i")) { + params.argon2_flavour = Argon2i; + } else if (!strcmp(b, "Argon2id")) { + params.argon2_flavour = Argon2id; + } else { + sfree(b); + goto error; + } + sfree(b); + + if (!read_header(src, header) || 0 != strcmp(header, "Argon2-Memory")) + goto error; + if ((b = read_body(src)) == NULL) + goto error; + if (!str_to_uint32_t(b, ¶ms.argon2_mem)) { + sfree(b); + goto error; + } + sfree(b); + + if (!read_header(src, header) || 0 != strcmp(header, "Argon2-Passes")) + goto error; + if ((b = read_body(src)) == NULL) + goto error; + if (!str_to_uint32_t(b, ¶ms.argon2_passes)) { + sfree(b); + goto error; + } + sfree(b); + + if (!read_header(src, header) || + 0 != strcmp(header, "Argon2-Parallelism")) + goto error; + if ((b = read_body(src)) == NULL) + goto error; + if (!str_to_uint32_t(b, ¶ms.argon2_parallelism)) { + sfree(b); + goto error; + } + sfree(b); + + if (!read_header(src, header) || 0 != strcmp(header, "Argon2-Salt")) + goto error; + if ((b = read_body(src)) == NULL) + goto error; + for (size_t i = 0; b[i]; i += 2) { + if (isxdigit((unsigned char)b[i]) && b[i+1] && + isxdigit((unsigned char)b[i+1])) { + char s[3]; + s[0] = b[i]; + s[1] = b[i+1]; + s[2] = '\0'; + put_byte(passphrase_salt, strtoul(s, NULL, 16)); + } else { + sfree(b); + goto error; + } + } + sfree(b); + } + /* Read the Private-Lines header line and the Private blob. */ if (!read_header(src, header) || 0 != strcmp(header, "Private-Lines")) goto error; @@ -756,7 +876,8 @@ ssh2_userkey *ppk_load_s(BinarySource *src, const char *passphrase, cipher_mac_keys_blob = strbuf_new(); ssh2_ppk_derive_keys(fmt_version, ciphertype, ptrlen_from_asciz(passphrase ? passphrase : ""), - cipher_mac_keys_blob, &cipherkey, &cipheriv, &mackey); + cipher_mac_keys_blob, &cipherkey, &cipheriv, &mackey, + ptrlen_from_strbuf(passphrase_salt), ¶ms); /* * Decrypt the private blob. @@ -772,12 +893,13 @@ ssh2_userkey *ppk_load_s(BinarySource *src, const char *passphrase, * Verify the MAC. */ { - unsigned char binary[20]; + unsigned char binary[32]; char realmac[sizeof(binary) * 2 + 1]; strbuf *macdata; bool free_macdata; - const ssh2_macalg *mac_alg = &ssh_hmac_sha1; + const ssh2_macalg *mac_alg = + fmt_version <= 2 ? &ssh_hmac_sha1 : &ssh_hmac_sha256; if (fmt_version == 1) { /* MAC (or hash) only covers the private blob. */ @@ -861,6 +983,7 @@ ssh2_userkey *ppk_load_s(BinarySource *src, const char *passphrase, strbuf_free(private_blob); if (cipher_mac_keys_blob) strbuf_free(cipher_mac_keys_blob); + strbuf_free(passphrase_salt); if (errorstr) *errorstr = error; return ret; @@ -1107,7 +1230,8 @@ bool ppk_loadpub_s(BinarySource *src, char **algorithm, BinarySink *bs, /* Read the first header line which contains the key type. */ if (!read_header(src, header) - || (0 != strcmp(header, "PuTTY-User-Key-File-2") && + || (0 != strcmp(header, "PuTTY-User-Key-File-3") && + 0 != strcmp(header, "PuTTY-User-Key-File-2") && 0 != strcmp(header, "PuTTY-User-Key-File-1"))) { if (0 == strncmp(header, "PuTTY-User-Key-File-", 20)) error = "PuTTY key format too new"; @@ -1194,7 +1318,8 @@ bool ppk_encrypted_s(BinarySource *src, char **commentptr) *commentptr = NULL; if (!read_header(src, header) - || (0 != strcmp(header, "PuTTY-User-Key-File-2") && + || (0 != strcmp(header, "PuTTY-User-Key-File-3") && + 0 != strcmp(header, "PuTTY-User-Key-File-2") && 0 != strcmp(header, "PuTTY-User-Key-File-1"))) { return false; } @@ -1284,7 +1409,54 @@ void base64_encode(FILE *fp, const unsigned char *data, int datalen, int cpl) base64_encode_s(BinarySink_UPCAST(&ss), data, datalen, cpl); } -strbuf *ppk_save_sb(ssh2_userkey *key, const char *passphrase) +const ppk_save_parameters ppk_save_default_parameters = { + /* + * The Argon2 spec recommends the hybrid variant Argon2id, where + * you don't have a good reason to go with the pure Argon2d or + * Argon2i. + */ + .argon2_flavour = Argon2id, + + /* + * Memory requirement for hashing a password: I don't want to set + * this to some truly huge thing like a gigabyte, because for all + * I know people might perfectly reasonably be running PuTTY on + * machines that don't _have_ a gigabyte spare to hash a private + * key passphrase in the legitimate use cases. + * + * I've picked 8 MB as an amount of memory that isn't unreasonable + * to expect a desktop client machine to have, but is also large + * compared to the memory requirements of the PPK v2 password hash + * (which was plain SHA-1), so it still imposes a limit on + * parallel attacks on someone's key file. + */ + .argon2_mem = 8192, /* require 8 Mb memory */ + + /* + * Automatically scale the number of Argon2 passes so that the + * overall time taken is about 1/10 second. (Again, I could crank + * this up to a larger time and _most_ people might be OK with it, + * but for the moment, I'm trying to err on the side of not + * stopping anyone from using the tools at all.) + */ + .argon2_passes_auto = true, + .argon2_milliseconds = 100, + + /* + * PuTTY's own Argon2 implementation is single-threaded. So we + * might as well set parallelism to 1, which requires that + * attackers' implementations must also be effectively + * single-threaded, and they don't get any benefit from using + * multiple cores on the same hash attempt. (Of course they can + * still use multiple cores for _separate_ hash attempts, but at + * least they don't get a speed advantage over us in computing + * even one hash.) + */ + .argon2_parallelism = 1, +}; + +strbuf *ppk_save_sb(ssh2_userkey *key, const char *passphrase, + const ppk_save_parameters *params_orig) { strbuf *pub_blob, *priv_blob, *cipher_mac_keys_blob; unsigned char *priv_blob_encrypted; @@ -1294,7 +1466,7 @@ strbuf *ppk_save_sb(ssh2_userkey *key, const char *passphrase) const char *cipherstr; ptrlen cipherkey, cipheriv, mackey; const struct ppk_cipher *ciphertype; - unsigned char priv_mac[20]; + unsigned char priv_mac[32]; /* * Fetch the key component blobs. @@ -1328,10 +1500,20 @@ strbuf *ppk_save_sb(ssh2_userkey *key, const char *passphrase) memcpy(priv_blob_encrypted + priv_blob->len, priv_mac, priv_encrypted_len - priv_blob->len); + /* Copy the save parameters, so that when derive_keys chooses the + * number of Argon2 passes, it can write the result back to our + * copy for us to retrieve. */ + ppk_save_parameters params = *params_orig; + + /* Invent a salt for the password hash. */ + strbuf *passphrase_salt = strbuf_new(); + random_read(strbuf_append(passphrase_salt, 16), 16); + cipher_mac_keys_blob = strbuf_new(); - ssh2_ppk_derive_keys(2, ciphertype, + ssh2_ppk_derive_keys(3, ciphertype, ptrlen_from_asciz(passphrase ? passphrase : ""), - cipher_mac_keys_blob, &cipherkey, &cipheriv, &mackey); + cipher_mac_keys_blob, &cipherkey, &cipheriv, &mackey, + ptrlen_from_strbuf(passphrase_salt), ¶ms); /* Now create the MAC. */ { @@ -1344,7 +1526,7 @@ strbuf *ppk_save_sb(ssh2_userkey *key, const char *passphrase) put_string(macdata, pub_blob->s, pub_blob->len); put_string(macdata, priv_blob_encrypted, priv_encrypted_len); - mac_simple(&ssh_hmac_sha1, mackey, + mac_simple(&ssh_hmac_sha256, mackey, ptrlen_from_strbuf(macdata), priv_mac); strbuf_free(macdata); } @@ -1356,20 +1538,35 @@ strbuf *ppk_save_sb(ssh2_userkey *key, const char *passphrase) } strbuf *out = strbuf_new_nm(); - strbuf_catf(out, "PuTTY-User-Key-File-2: %s\n", ssh_key_ssh_id(key->key)); + strbuf_catf(out, "PuTTY-User-Key-File-3: %s\n", ssh_key_ssh_id(key->key)); strbuf_catf(out, "Encryption: %s\n", cipherstr); strbuf_catf(out, "Comment: %s\n", key->comment); strbuf_catf(out, "Public-Lines: %d\n", base64_lines(pub_blob->len)); base64_encode_s(BinarySink_UPCAST(out), pub_blob->u, pub_blob->len, 64); + if (ciphertype->keylen != 0) { + strbuf_catf(out, "Key-Derivation: %s\n", + params.argon2_flavour == Argon2d ? "Argon2d" : + params.argon2_flavour == Argon2i ? "Argon2i" : "Argon2id"); + strbuf_catf(out, "Argon2-Memory: %"PRIu32"\n", params.argon2_mem); + assert(!params.argon2_passes_auto); + strbuf_catf(out, "Argon2-Passes: %"PRIu32"\n", params.argon2_passes); + strbuf_catf(out, "Argon2-Parallelism: %"PRIu32"\n", + params.argon2_parallelism); + strbuf_catf(out, "Argon2-Salt: "); + for (size_t i = 0; i < passphrase_salt->len; i++) + strbuf_catf(out, "%02x", passphrase_salt->u[i]); + strbuf_catf(out, "\n"); + } strbuf_catf(out, "Private-Lines: %d\n", base64_lines(priv_encrypted_len)); base64_encode_s(BinarySink_UPCAST(out), priv_blob_encrypted, priv_encrypted_len, 64); strbuf_catf(out, "Private-MAC: "); - for (i = 0; i < 20; i++) + for (i = 0; i < 32; i++) strbuf_catf(out, "%02x", priv_mac[i]); strbuf_catf(out, "\n"); strbuf_free(cipher_mac_keys_blob); + strbuf_free(passphrase_salt); strbuf_free(pub_blob); strbuf_free(priv_blob); smemclr(priv_blob_encrypted, priv_encrypted_len); @@ -1378,13 +1575,13 @@ strbuf *ppk_save_sb(ssh2_userkey *key, const char *passphrase) } bool ppk_save_f(const Filename *filename, ssh2_userkey *key, - const char *passphrase) + const char *passphrase, const ppk_save_parameters *params) { FILE *fp = f_open(filename, "wb", true); if (!fp) return false; - strbuf *buf = ppk_save_sb(key, passphrase); + strbuf *buf = ppk_save_sb(key, passphrase, params); bool toret = fwrite(buf->s, 1, buf->len, fp) == buf->len; if (fclose(fp)) toret = false; diff --git a/test/cryptsuite.py b/test/cryptsuite.py index e39e0d0d..09ee5e8d 100755 --- a/test/cryptsuite.py +++ b/test/cryptsuite.py @@ -2109,7 +2109,7 @@ culpa qui officia deserunt mollit anim id est laborum. def testPPKLoadSave(self): # Stability test of PPK load/save functions. input_clear_key = b"""\ -PuTTY-User-Key-File-2: ssh-ed25519 +PuTTY-User-Key-File-3: ssh-ed25519 Encryption: none Comment: ed25519-key-20200105 Public-Lines: 2 @@ -2117,18 +2117,23 @@ AAAAC3NzaC1lZDI1NTE5AAAAIHJCszOHaI9X/yGLtjn22f0hO6VPMQDVtctkym6F JH1W Private-Lines: 1 AAAAIGvvIpl8jyqn8Xufkw6v3FnEGtXF3KWw55AP3/AGEBpY -Private-MAC: 2a629acfcfbe28488a1ba9b6948c36406bc28422 +Private-MAC: 816c84093fc4877e8411b8e5139c5ce35d8387a2630ff087214911d67417a54d """ input_encrypted_key = b"""\ -PuTTY-User-Key-File-2: ssh-ed25519 +PuTTY-User-Key-File-3: ssh-ed25519 Encryption: aes256-cbc Comment: ed25519-key-20200105 Public-Lines: 2 AAAAC3NzaC1lZDI1NTE5AAAAIHJCszOHaI9X/yGLtjn22f0hO6VPMQDVtctkym6F JH1W +Key-Derivation: Argon2id +Argon2-Memory: 8192 +Argon2-Passes: 13 +Argon2-Parallelism: 1 +Argon2-Salt: 37c3911bfefc8c1d11ec579627d2b3d9 Private-Lines: 1 -4/jKlTgC652oa9HLVGrMjHZw7tj0sKRuZaJPOuLhGTvb25Jzpcqpbi+Uf+y+uo+Z -Private-MAC: 5b1f6f4cc43eb0060d2c3e181bc0129343adba2b +amviz4sVUBN64jLO3gt4HGXJosUArghc4Soi7aVVLb2Tir5Baj0OQClorycuaPRd +Private-MAC: 6f5e588e475e55434106ec2c3569695b03f423228b44993a9e97d52ffe7be5a8 """ algorithm = b'ssh-ed25519' comment = b'ed25519-key-20200105' @@ -2153,12 +2158,66 @@ Private-MAC: 5b1f6f4cc43eb0060d2c3e181bc0129343adba2b self.assertEqual((c, e), (comment, None)) k2, c, e = ppk_load_s(input_encrypted_key, pp) self.assertEqual((c, e), (comment, None)) + privblob = ssh_key_private_blob(k1) + self.assertEqual(ssh_key_private_blob(k2), privblob) - self.assertEqual(ppk_save_sb(k1, comment, None), input_clear_key) - self.assertEqual(ppk_save_sb(k2, comment, None), input_clear_key) + salt = unhex('37c3911bfefc8c1d11ec579627d2b3d9') + with queued_specific_random_data(salt): + self.assertEqual(ppk_save_sb(k1, comment, None, 'id', 8192, 13, 1), + input_clear_key) + with queued_specific_random_data(salt): + self.assertEqual(ppk_save_sb(k2, comment, None, 'id', 8192, 13, 1), + input_clear_key) - self.assertEqual(ppk_save_sb(k1, comment, pp), input_encrypted_key) - self.assertEqual(ppk_save_sb(k2, comment, pp), input_encrypted_key) + with queued_specific_random_data(salt): + self.assertEqual(ppk_save_sb(k1, comment, pp, 'id', 8192, 13, 1), + input_encrypted_key) + with queued_specific_random_data(salt): + self.assertEqual(ppk_save_sb(k2, comment, pp, 'id', 8192, 13, 1), + input_encrypted_key) + + # And check we can still handle v2 key files. + v2_clear_key = b"""\ +PuTTY-User-Key-File-2: ssh-ed25519 +Encryption: none +Comment: ed25519-key-20200105 +Public-Lines: 2 +AAAAC3NzaC1lZDI1NTE5AAAAIHJCszOHaI9X/yGLtjn22f0hO6VPMQDVtctkym6F +JH1W +Private-Lines: 1 +AAAAIGvvIpl8jyqn8Xufkw6v3FnEGtXF3KWw55AP3/AGEBpY +Private-MAC: 2a629acfcfbe28488a1ba9b6948c36406bc28422 +""" + v2_encrypted_key = b"""\ +PuTTY-User-Key-File-2: ssh-ed25519 +Encryption: aes256-cbc +Comment: ed25519-key-20200105 +Public-Lines: 2 +AAAAC3NzaC1lZDI1NTE5AAAAIHJCszOHaI9X/yGLtjn22f0hO6VPMQDVtctkym6F +JH1W +Private-Lines: 1 +4/jKlTgC652oa9HLVGrMjHZw7tj0sKRuZaJPOuLhGTvb25Jzpcqpbi+Uf+y+uo+Z +Private-MAC: 5b1f6f4cc43eb0060d2c3e181bc0129343adba2b +""" + + self.assertEqual(ppk_encrypted_s(v2_clear_key), (False, comment)) + self.assertEqual(ppk_encrypted_s(v2_encrypted_key), (True, comment)) + self.assertEqual(ppk_encrypted_s("not a key file"), (False, None)) + + self.assertEqual(ppk_loadpub_s(v2_clear_key), + (True, algorithm, public_blob, comment, None)) + self.assertEqual(ppk_loadpub_s(v2_encrypted_key), + (True, algorithm, public_blob, comment, None)) + self.assertEqual(ppk_loadpub_s("not a key file"), + (False, None, b'', None, + b'not a PuTTY SSH-2 private key')) + + k1, c, e = ppk_load_s(v2_clear_key, None) + self.assertEqual((c, e), (comment, None)) + k2, c, e = ppk_load_s(v2_encrypted_key, pp) + self.assertEqual((c, e), (comment, None)) + self.assertEqual(ssh_key_private_blob(k1), privblob) + self.assertEqual(ssh_key_private_blob(k2), privblob) def testRSA1LoadSave(self): # Stability test of SSH-1 RSA key-file load/save functions. diff --git a/testcrypt.c b/testcrypt.c index f76417bc..8db09227 100644 --- a/testcrypt.c +++ b/testcrypt.c @@ -49,6 +49,10 @@ static NORETURN PRINTF_LIKE(1, 2) void fatal_error(const char *p, ...) void out_of_memory(void) { fatal_error("out of memory"); } +/* For platforms where getticks is defined within this code base */ +unsigned long (getticks)(void) +{ unreachable("this is a stub needed to link, and should never be called"); } + FILE *f_open(const struct Filename *fn, char const *mode, bool private) { unreachable("f_open should never be called by this test program"); } @@ -1141,12 +1145,24 @@ int rsa1_load_s_wrapper(BinarySource *src, RSAKey *rsa, char **comment, #define rsa1_load_s rsa1_load_s_wrapper strbuf *ppk_save_sb_wrapper(ssh_key *key, const char *comment, - const char *passphrase) + const char *passphrase, Argon2Flavour flavour, + uint32_t mem, uint32_t passes, uint32_t parallel) { + /* + * For repeatable testing purposes, we never want a timing-dependent + * choice of password hashing parameters, so this is easy. + */ + ppk_save_parameters save_params; + save_params.argon2_flavour = flavour; + save_params.argon2_mem = mem; + save_params.argon2_passes_auto = false; + save_params.argon2_passes = passes; + save_params.argon2_parallelism = parallel; + ssh2_userkey uk; uk.key = key; uk.comment = dupstr(comment); - strbuf *toret = ppk_save_sb(&uk, passphrase); + strbuf *toret = ppk_save_sb(&uk, passphrase, &save_params); sfree(uk.comment); return toret; } diff --git a/testcrypt.h b/testcrypt.h index 12e248b8..5918f179 100644 --- a/testcrypt.h +++ b/testcrypt.h @@ -258,7 +258,7 @@ FUNC5(boolean, ppk_loadpub_s, val_string_binarysource, out_opt_val_string_asciz, FUNC4(int, rsa1_loadpub_s, val_string_binarysource, out_val_string_binarysink, out_opt_val_string_asciz, out_opt_val_string_asciz_const) FUNC4(opt_val_key, ppk_load_s, val_string_binarysource, out_opt_val_string_asciz, opt_val_string_asciz, out_opt_val_string_asciz_const) FUNC5(int, rsa1_load_s, val_string_binarysource, val_rsa, out_opt_val_string_asciz, opt_val_string_asciz, out_opt_val_string_asciz_const) -FUNC3(val_string, ppk_save_sb, val_key, opt_val_string_asciz, opt_val_string_asciz) +FUNC7(val_string, ppk_save_sb, val_key, opt_val_string_asciz, opt_val_string_asciz, argon2flavour, uint, uint, uint) FUNC3(val_string, rsa1_save_sb, val_rsa, opt_val_string_asciz, opt_val_string_asciz) /* diff --git a/testsc.c b/testsc.c index 6f9ee003..f04f718c 100644 --- a/testsc.c +++ b/testsc.c @@ -97,6 +97,9 @@ FILE *f_open(const Filename *filename, char const *mode, bool is_private) { unreachable("this is a stub needed to link, and should never be called"); } void old_keyfile_warning(void) { unreachable("this is a stub needed to link, and should never be called"); } +/* For platforms where getticks is defined within this code base */ +unsigned long (getticks)(void) +{ unreachable("this is a stub needed to link, and should never be called"); } /* * A simple deterministic PRNG, without any of the Fortuna diff --git a/windows/winpgen.c b/windows/winpgen.c index 3cf1a1e9..c2a8f8d0 100644 --- a/windows/winpgen.c +++ b/windows/winpgen.c @@ -1468,7 +1468,8 @@ static INT_PTR CALLBACK MainDlgProc(HWND hwnd, UINT msg, *passphrase ? passphrase : NULL); else ret = ppk_save_f(fn, &state->ssh2key, - *passphrase ? passphrase : NULL); + *passphrase ? passphrase : NULL, + &ppk_save_default_parameters); filename_free(fn); } else { Filename *fn = filename_from_str(filename);