1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00
putty-source/sshpubk.c

1950 lines
56 KiB
C
Raw Normal View History

/*
* Generic SSH public-key handling operations. In particular,
* reading of SSH public-key files, and also the generic `sign'
* operation for SSH-2 (which checks the type of the key and
* dispatches to the appropriate key-type specific function).
*/
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <stdlib.h>
#include <assert.h>
#include <ctype.h>
#include "putty.h"
Complete rewrite of PuTTY's bignum library. The old 'Bignum' data type is gone completely, and so is sshbn.c. In its place is a new thing called 'mp_int', handled by an entirely new library module mpint.c, with API differences both large and small. The main aim of this change is that the new library should be free of timing- and cache-related side channels. I've written the code so that it _should_ - assuming I haven't made any mistakes - do all of its work without either control flow or memory addressing depending on the data words of the input numbers. (Though, being an _arbitrary_ precision library, it does have to at least depend on the sizes of the numbers - but there's a 'formal' size that can vary separately from the actual magnitude of the represented integer, so if you want to keep it secret that your number is actually small, it should work fine to have a very long mp_int and just happen to store 23 in it.) So I've done all my conditionalisation by means of computing both answers and doing bit-masking to swap the right one into place, and all loops over the words of an mp_int go up to the formal size rather than the actual size. I haven't actually tested the constant-time property in any rigorous way yet (I'm still considering the best way to do it). But this code is surely at the very least a big improvement on the old version, even if I later find a few more things to fix. I've also completely rewritten the low-level elliptic curve arithmetic from sshecc.c; the new ecc.c is closer to being an adjunct of mpint.c than it is to the SSH end of the code. The new elliptic curve code keeps all coordinates in Montgomery-multiplication transformed form to speed up all the multiplications mod the same prime, and only converts them back when you ask for the affine coordinates. Also, I adopted extended coordinates for the Edwards curve implementation. sshecc.c has also had a near-total rewrite in the course of switching it over to the new system. While I was there, I've separated ECDSA and EdDSA more completely - they now have separate vtables, instead of a single vtable in which nearly every function had a big if statement in it - and also made the externally exposed types for an ECDSA key and an ECDH context different. A minor new feature: since the new arithmetic code includes a modular square root function, we can now support the compressed point representation for the NIST curves. We seem to have been getting along fine without that so far, but it seemed a shame not to put it in, since it was suddenly easy. In sshrsa.c, one major change is that I've removed the RSA blinding step in rsa_privkey_op, in which we randomise the ciphertext before doing the decryption. The purpose of that was to avoid timing leaks giving away the plaintext - but the new arithmetic code should take that in its stride in the course of also being careful enough to avoid leaking the _private key_, which RSA blinding had no way to do anything about in any case. Apart from those specific points, most of the rest of the changes are more or less mechanical, just changing type names and translating code into the new API.
2018-12-31 13:53:41 +00:00
#include "mpint.h"
#include "ssh.h"
#include "misc.h"
/*
* Fairly arbitrary size limit on any public or private key blob.
* Chosen to match AGENT_MAX_MSGLEN, on the basis that any key too
* large to transfer over the ssh-agent protocol is probably too large
* to be useful in general.
*
* MAX_KEY_BLOB_LINES is the corresponding limit on the Public-Lines
* or Private-Lines header field in a key file.
*/
#define MAX_KEY_BLOB_SIZE 262144
#define MAX_KEY_BLOB_LINES (MAX_KEY_BLOB_SIZE / 48)
/*
* Corresponding limit on the size of a key _file_ itself, based on
* base64-encoding the key blob and then adding a few Kb for
* surrounding metadata.
*/
#define MAX_KEY_FILE_SIZE (MAX_KEY_BLOB_SIZE * 4 / 3 + 4096)
static const ptrlen rsa1_signature =
PTRLEN_DECL_LITERAL("SSH PRIVATE KEY FILE FORMAT 1.1\n\0");
#define BASE64_TOINT(x) ( (x)-'A'<26 ? (x)-'A'+0 :\
(x)-'a'<26 ? (x)-'a'+26 :\
(x)-'0'<10 ? (x)-'0'+52 :\
(x)=='+' ? 62 : \
(x)=='/' ? 63 : 0 )
LoadedFile *lf_new(size_t max_size)
{
LoadedFile *lf = snew_plus(LoadedFile, max_size);
lf->data = snew_plus_get_aux(lf);
lf->len = 0;
lf->max_size = max_size;
return lf;
}
void lf_free(LoadedFile *lf)
{
smemclr(lf->data, lf->max_size);
smemclr(lf, sizeof(LoadedFile));
sfree(lf);
}
LoadFileStatus lf_load_fp(LoadedFile *lf, FILE *fp)
{
lf->len = 0;
while (lf->len < lf->max_size) {
size_t retd = fread(lf->data + lf->len, 1, lf->max_size - lf->len, fp);
if (ferror(fp))
return LF_ERROR;
if (retd == 0)
break;
lf->len += retd;
}
LoadFileStatus status = LF_OK;
if (lf->len == lf->max_size) {
/* The file might be too long to fit in our fixed-size
* structure. Try reading one more byte, to check. */
if (fgetc(fp) != EOF)
status = LF_TOO_BIG;
}
BinarySource_INIT(lf, lf->data, lf->len);
return status;
}
LoadFileStatus lf_load(LoadedFile *lf, const Filename *filename)
{
FILE *fp = f_open(filename, "rb", false);
if (!fp)
return LF_ERROR;
LoadFileStatus status = lf_load_fp(lf, fp);
fclose(fp);
return status;
}
static inline bool lf_load_keyfile_helper(LoadFileStatus status,
const char **errptr)
{
const char *error;
switch (status) {
case LF_OK:
return true;
case LF_TOO_BIG:
error = "file is too large to be a key file";
break;
case LF_ERROR:
error = strerror(errno);
break;
default:
unreachable("bad status value in lf_load_keyfile_helper");
}
if (errptr)
*errptr = error;
return false;
}
LoadedFile *lf_load_keyfile(const Filename *filename, const char **errptr)
{
LoadedFile *lf = lf_new(MAX_KEY_FILE_SIZE);
if (!lf_load_keyfile_helper(lf_load(lf, filename), errptr)) {
lf_free(lf);
return NULL;
}
return lf;
}
LoadedFile *lf_load_keyfile_fp(FILE *fp, const char **errptr)
{
LoadedFile *lf = lf_new(MAX_KEY_FILE_SIZE);
if (!lf_load_keyfile_helper(lf_load_fp(lf, fp), errptr)) {
lf_free(lf);
return NULL;
}
return lf;
}
static bool expect_signature(BinarySource *src, ptrlen realsig)
{
ptrlen thissig = get_data(src, realsig.len);
return !get_err(src) && ptrlen_eq_ptrlen(realsig, thissig);
}
static int rsa1_load_s_internal(BinarySource *src, RSAKey *key, bool pub_only,
char **commentptr, const char *passphrase,
const char **error)
{
strbuf *buf = NULL;
int ciphertype;
int ret = 0;
ptrlen comment;
*error = "not an SSH-1 RSA file";
if (!expect_signature(src, rsa1_signature))
goto end;
*error = "file format error";
/* One byte giving encryption type, and one reserved uint32. */
ciphertype = get_byte(src);
if (ciphertype != 0 && ciphertype != SSH1_CIPHER_3DES)
goto end;
if (get_uint32(src) != 0)
goto end; /* reserved field nonzero, panic! */
/* Now the serious stuff. An ordinary SSH-1 public key. */
get_rsa_ssh1_pub(src, key, RSA_SSH1_MODULUS_FIRST);
/* Next, the comment field. */
comment = get_string(src);
if (commentptr)
*commentptr = mkstr(comment);
if (key)
key->comment = mkstr(comment);
if (pub_only) {
ret = 1;
goto end;
}
if (!key) {
ret = ciphertype != 0;
*error = NULL;
goto end;
}
/*
* Decrypt remainder of buffer.
*/
if (ciphertype) {
size_t enclen = get_avail(src);
if (enclen & 7)
goto end;
buf = strbuf_dup_nm(get_data(src, enclen));
unsigned char keybuf[16];
hash_simple(&ssh_md5, ptrlen_from_asciz(passphrase), keybuf);
des3_decrypt_pubkey(keybuf, buf->u, enclen);
smemclr(keybuf, sizeof(keybuf)); /* burn the evidence */
BinarySource_BARE_INIT_PL(src, ptrlen_from_strbuf(buf));
}
/*
* We are now in the secret part of the key. The first four
* bytes should be of the form a, b, a, b.
*/
{
int b0a = get_byte(src);
int b1a = get_byte(src);
int b0b = get_byte(src);
int b1b = get_byte(src);
if (b0a != b0b || b1a != b1b) {
*error = "wrong passphrase";
ret = -1;
goto end;
}
}
/*
* After that, we have one further bignum which is our
* decryption exponent, and then the three auxiliary values
* (iqmp, q, p).
*/
get_rsa_ssh1_priv(src, key);
key->iqmp = get_mp_ssh1(src);
key->q = get_mp_ssh1(src);
key->p = get_mp_ssh1(src);
if (!rsa_verify(key)) {
*error = "rsa_verify failed";
freersakey(key);
ret = 0;
} else {
*error = NULL;
ret = 1;
}
end:
if (buf)
strbuf_free(buf);
return ret;
}
int rsa1_load_s(BinarySource *src, RSAKey *key,
const char *passphrase, const char **errstr)
{
return rsa1_load_s_internal(src, key, false, NULL, passphrase, errstr);
}
int rsa1_load_f(const Filename *filename, RSAKey *key,
const char *passphrase, const char **errstr)
{
LoadedFile *lf = lf_load_keyfile(filename, errstr);
if (!lf)
return false;
int toret = rsa1_load_s(BinarySource_UPCAST(lf), key, passphrase, errstr);
lf_free(lf);
return toret;
}
/*
* See whether an RSA key is encrypted. Return its comment field as
* well.
*/
bool rsa1_encrypted_s(BinarySource *src, char **comment)
{
const char *dummy;
return rsa1_load_s_internal(src, NULL, false, comment, NULL, &dummy) == 1;
}
bool rsa1_encrypted_f(const Filename *filename, char **comment)
{
LoadedFile *lf = lf_load_keyfile(filename, NULL);
if (!lf)
return false; /* couldn't even open the file */
bool toret = rsa1_encrypted_s(BinarySource_UPCAST(lf), comment);
lf_free(lf);
return toret;
}
/*
* Read the public part of an SSH-1 RSA key from a file (public or
* private), and generate its public blob in exponent-first order.
*/
int rsa1_loadpub_s(BinarySource *src, BinarySink *bs,
char **commentptr, const char **errorstr)
{
RSAKey key;
int ret;
const char *error = NULL;
/* Default return if we fail. */
Convert a lot of 'int' variables to 'bool'. My normal habit these days, in new code, is to treat int and bool as _almost_ completely separate types. I'm still willing to use C's implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine, no need to spell it out as blob.len != 0), but generally, if a variable is going to be conceptually a boolean, I like to declare it bool and assign to it using 'true' or 'false' rather than 0 or 1. PuTTY is an exception, because it predates the C99 bool, and I've stuck to its existing coding style even when adding new code to it. But it's been annoying me more and more, so now that I've decided C99 bool is an acceptable thing to require from our toolchain in the first place, here's a quite thorough trawl through the source doing 'boolification'. Many variables and function parameters are now typed as bool rather than int; many assignments of 0 or 1 to those variables are now spelled 'true' or 'false'. I managed this thorough conversion with the help of a custom clang plugin that I wrote to trawl the AST and apply heuristics to point out where things might want changing. So I've even managed to do a decent job on parts of the code I haven't looked at in years! To make the plugin's work easier, I pushed platform front ends generally in the direction of using standard 'bool' in preference to platform-specific boolean types like Windows BOOL or GTK's gboolean; I've left the platform booleans in places they _have_ to be for the platform APIs to work right, but variables only used by my own code have been converted wherever I found them. In a few places there are int values that look very like booleans in _most_ of the places they're used, but have a rarely-used third value, or a distinction between different nonzero values that most users don't care about. In these cases, I've _removed_ uses of 'true' and 'false' for the return values, to emphasise that there's something more subtle going on than a simple boolean answer: - the 'multisel' field in dialog.h's list box structure, for which the GTK front end in particular recognises a difference between 1 and 2 but nearly everything else treats as boolean - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you something about the specific location of the urgent pointer, but most clients only care about 0 vs 'something nonzero' - the return value of wc_match, where -1 indicates a syntax error in the wildcard. - the return values from SSH-1 RSA-key loading functions, which use -1 for 'wrong passphrase' and 0 for all other failures (so any caller which already knows it's not loading an _encrypted private_ key can treat them as boolean) - term->esc_query, and the 'query' parameter in toggle_mode in terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h, but can also hold -1 for some other intervening character that we don't support. In a few places there's an integer that I haven't turned into a bool even though it really _can_ only take values 0 or 1 (and, as above, tried to make the call sites consistent in not calling those values true and false), on the grounds that I thought it would make it more confusing to imply that the 0 value was in some sense 'negative' or bad and the 1 positive or good: - the return value of plug_accepting uses the POSIXish convention of 0=success and nonzero=error; I think if I made it bool then I'd also want to reverse its sense, and that's a job for a separate piece of work. - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1 represent the default and alternate screens. There's no obvious reason why one of those should be considered 'true' or 'positive' or 'success' - they're just indices - so I've left it as int. ssh_scp_recv had particularly confusing semantics for its previous int return value: its call sites used '<= 0' to check for error, but it never actually returned a negative number, just 0 or 1. Now the function and its call sites agree that it's a bool. In a couple of places I've renamed variables called 'ret', because I don't like that name any more - it's unclear whether it means the return value (in preparation) for the _containing_ function or the return value received from a subroutine call, and occasionally I've accidentally used the same variable for both and introduced a bug. So where one of those got in my way, I've renamed it to 'toret' or 'retd' (the latter short for 'returned') in line with my usual modern practice, but I haven't done a thorough job of finding all of them. Finally, one amusing side effect of doing this is that I've had to separate quite a few chained assignments. It used to be perfectly fine to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a the 'true' defined by stdbool.h, that idiom provokes a warning from gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
ret = 0;
bool is_privkey_file = expect_signature(src, rsa1_signature);
BinarySource_REWIND(src);
if (is_privkey_file) {
/*
* Load just the public half from an SSH-1 private key file.
*/
memset(&key, 0, sizeof(key));
if (rsa1_load_s_internal(src, &key, true, commentptr, NULL, &error)) {
rsa_ssh1_public_blob(bs, &key, RSA_SSH1_EXPONENT_FIRST);
freersakey(&key);
ret = 1;
}
} else {
/*
* Try interpreting the file as an SSH-1 public key.
*/
char *line, *p, *bitsp, *expp, *modp, *commentp;
line = mkstr(get_chomped_line(src));
p = line;
bitsp = p;
p += strspn(p, "0123456789");
if (*p != ' ')
goto not_public_either;
*p++ = '\0';
expp = p;
p += strspn(p, "0123456789");
if (*p != ' ')
goto not_public_either;
*p++ = '\0';
modp = p;
p += strspn(p, "0123456789");
if (*p) {
if (*p != ' ')
goto not_public_either;
*p++ = '\0';
commentp = p;
} else {
commentp = NULL;
}
memset(&key, 0, sizeof(key));
Complete rewrite of PuTTY's bignum library. The old 'Bignum' data type is gone completely, and so is sshbn.c. In its place is a new thing called 'mp_int', handled by an entirely new library module mpint.c, with API differences both large and small. The main aim of this change is that the new library should be free of timing- and cache-related side channels. I've written the code so that it _should_ - assuming I haven't made any mistakes - do all of its work without either control flow or memory addressing depending on the data words of the input numbers. (Though, being an _arbitrary_ precision library, it does have to at least depend on the sizes of the numbers - but there's a 'formal' size that can vary separately from the actual magnitude of the represented integer, so if you want to keep it secret that your number is actually small, it should work fine to have a very long mp_int and just happen to store 23 in it.) So I've done all my conditionalisation by means of computing both answers and doing bit-masking to swap the right one into place, and all loops over the words of an mp_int go up to the formal size rather than the actual size. I haven't actually tested the constant-time property in any rigorous way yet (I'm still considering the best way to do it). But this code is surely at the very least a big improvement on the old version, even if I later find a few more things to fix. I've also completely rewritten the low-level elliptic curve arithmetic from sshecc.c; the new ecc.c is closer to being an adjunct of mpint.c than it is to the SSH end of the code. The new elliptic curve code keeps all coordinates in Montgomery-multiplication transformed form to speed up all the multiplications mod the same prime, and only converts them back when you ask for the affine coordinates. Also, I adopted extended coordinates for the Edwards curve implementation. sshecc.c has also had a near-total rewrite in the course of switching it over to the new system. While I was there, I've separated ECDSA and EdDSA more completely - they now have separate vtables, instead of a single vtable in which nearly every function had a big if statement in it - and also made the externally exposed types for an ECDSA key and an ECDH context different. A minor new feature: since the new arithmetic code includes a modular square root function, we can now support the compressed point representation for the NIST curves. We seem to have been getting along fine without that so far, but it seemed a shame not to put it in, since it was suddenly easy. In sshrsa.c, one major change is that I've removed the RSA blinding step in rsa_privkey_op, in which we randomise the ciphertext before doing the decryption. The purpose of that was to avoid timing leaks giving away the plaintext - but the new arithmetic code should take that in its stride in the course of also being careful enough to avoid leaking the _private key_, which RSA blinding had no way to do anything about in any case. Apart from those specific points, most of the rest of the changes are more or less mechanical, just changing type names and translating code into the new API.
2018-12-31 13:53:41 +00:00
key.exponent = mp_from_decimal(expp);
key.modulus = mp_from_decimal(modp);
if (atoi(bitsp) != mp_get_nbits(key.modulus)) {
mp_free(key.exponent);
mp_free(key.modulus);
sfree(line);
error = "key bit count does not match in SSH-1 public key file";
goto end;
}
if (commentptr)
*commentptr = commentp ? dupstr(commentp) : NULL;
rsa_ssh1_public_blob(bs, &key, RSA_SSH1_EXPONENT_FIRST);
freersakey(&key);
sfree(line);
Convert a lot of 'int' variables to 'bool'. My normal habit these days, in new code, is to treat int and bool as _almost_ completely separate types. I'm still willing to use C's implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine, no need to spell it out as blob.len != 0), but generally, if a variable is going to be conceptually a boolean, I like to declare it bool and assign to it using 'true' or 'false' rather than 0 or 1. PuTTY is an exception, because it predates the C99 bool, and I've stuck to its existing coding style even when adding new code to it. But it's been annoying me more and more, so now that I've decided C99 bool is an acceptable thing to require from our toolchain in the first place, here's a quite thorough trawl through the source doing 'boolification'. Many variables and function parameters are now typed as bool rather than int; many assignments of 0 or 1 to those variables are now spelled 'true' or 'false'. I managed this thorough conversion with the help of a custom clang plugin that I wrote to trawl the AST and apply heuristics to point out where things might want changing. So I've even managed to do a decent job on parts of the code I haven't looked at in years! To make the plugin's work easier, I pushed platform front ends generally in the direction of using standard 'bool' in preference to platform-specific boolean types like Windows BOOL or GTK's gboolean; I've left the platform booleans in places they _have_ to be for the platform APIs to work right, but variables only used by my own code have been converted wherever I found them. In a few places there are int values that look very like booleans in _most_ of the places they're used, but have a rarely-used third value, or a distinction between different nonzero values that most users don't care about. In these cases, I've _removed_ uses of 'true' and 'false' for the return values, to emphasise that there's something more subtle going on than a simple boolean answer: - the 'multisel' field in dialog.h's list box structure, for which the GTK front end in particular recognises a difference between 1 and 2 but nearly everything else treats as boolean - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you something about the specific location of the urgent pointer, but most clients only care about 0 vs 'something nonzero' - the return value of wc_match, where -1 indicates a syntax error in the wildcard. - the return values from SSH-1 RSA-key loading functions, which use -1 for 'wrong passphrase' and 0 for all other failures (so any caller which already knows it's not loading an _encrypted private_ key can treat them as boolean) - term->esc_query, and the 'query' parameter in toggle_mode in terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h, but can also hold -1 for some other intervening character that we don't support. In a few places there's an integer that I haven't turned into a bool even though it really _can_ only take values 0 or 1 (and, as above, tried to make the call sites consistent in not calling those values true and false), on the grounds that I thought it would make it more confusing to imply that the 0 value was in some sense 'negative' or bad and the 1 positive or good: - the return value of plug_accepting uses the POSIXish convention of 0=success and nonzero=error; I think if I made it bool then I'd also want to reverse its sense, and that's a job for a separate piece of work. - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1 represent the default and alternate screens. There's no obvious reason why one of those should be considered 'true' or 'positive' or 'success' - they're just indices - so I've left it as int. ssh_scp_recv had particularly confusing semantics for its previous int return value: its call sites used '<= 0' to check for error, but it never actually returned a negative number, just 0 or 1. Now the function and its call sites agree that it's a bool. In a couple of places I've renamed variables called 'ret', because I don't like that name any more - it's unclear whether it means the return value (in preparation) for the _containing_ function or the return value received from a subroutine call, and occasionally I've accidentally used the same variable for both and introduced a bug. So where one of those got in my way, I've renamed it to 'toret' or 'retd' (the latter short for 'returned') in line with my usual modern practice, but I haven't done a thorough job of finding all of them. Finally, one amusing side effect of doing this is that I've had to separate quite a few chained assignments. It used to be perfectly fine to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a the 'true' defined by stdbool.h, that idiom provokes a warning from gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
return 1;
not_public_either:
sfree(line);
error = "not an SSH-1 RSA file";
}
end:
if ((ret != 1) && errorstr)
*errorstr = error;
return ret;
}
int rsa1_loadpub_f(const Filename *filename, BinarySink *bs,
char **commentptr, const char **errorstr)
{
LoadedFile *lf = lf_load_keyfile(filename, errorstr);
if (!lf)
return 0;
int toret = rsa1_loadpub_s(BinarySource_UPCAST(lf), bs,
commentptr, errorstr);
lf_free(lf);
return toret;
}
strbuf *rsa1_save_sb(RSAKey *key, const char *passphrase)
{
strbuf *buf = strbuf_new_nm();
int estart;
/*
* The public part of the key.
*/
put_datapl(buf, rsa1_signature);
put_byte(buf, passphrase ? SSH1_CIPHER_3DES : 0); /* encryption type */
put_uint32(buf, 0); /* reserved */
rsa_ssh1_public_blob(BinarySink_UPCAST(buf), key,
RSA_SSH1_MODULUS_FIRST);
put_stringz(buf, NULLTOEMPTY(key->comment));
/*
* The encrypted portion starts here.
*/
estart = buf->len;
/*
* Two bytes, then the same two bytes repeated.
*/
{
uint8_t bytes[2];
random_read(bytes, 2);
put_data(buf, bytes, 2);
put_data(buf, bytes, 2);
}
/*
* Four more bignums: the decryption exponent, then iqmp, then
* q, then p.
*/
put_mp_ssh1(buf, key->private_exponent);
put_mp_ssh1(buf, key->iqmp);
put_mp_ssh1(buf, key->q);
put_mp_ssh1(buf, key->p);
/*
* Now write zeros until the encrypted portion is a multiple of
* 8 bytes.
*/
put_padding(buf, (estart - buf->len) & 7, 0);
/*
* Now encrypt the encrypted portion.
*/
if (passphrase) {
unsigned char keybuf[16];
hash_simple(&ssh_md5, ptrlen_from_asciz(passphrase), keybuf);
des3_encrypt_pubkey(keybuf, buf->u + estart, buf->len - estart);
smemclr(keybuf, sizeof(keybuf)); /* burn the evidence */
}
return buf;
}
/*
* Save an RSA key file. Return true on success.
*/
bool rsa1_save_f(const Filename *filename, RSAKey *key, const char *passphrase)
{
FILE *fp = f_open(filename, "wb", true);
if (!fp)
return false;
strbuf *buf = rsa1_save_sb(key, passphrase);
bool toret = fwrite(buf->s, 1, buf->len, fp) == buf->len;
if (fclose(fp))
toret = false;
strbuf_free(buf);
return toret;
}
/* ----------------------------------------------------------------------
* SSH-2 private key load/store functions.
*
* PuTTY's own file format for SSH-2 keys is given in doc/ppk.but, aka
* the "PPK file format" appendix in the PuTTY manual.
*/
static bool read_header(BinarySource *src, char *header)
{
int len = 39;
int c;
while (1) {
c = get_byte(src);
if (c == '\n' || c == '\r' || get_err(src))
return false; /* failure */
if (c == ':') {
c = get_byte(src);
if (c != ' ')
return false;
*header = '\0';
return true; /* success! */
}
if (len == 0)
return false; /* failure */
*header++ = c;
len--;
}
Convert a lot of 'int' variables to 'bool'. My normal habit these days, in new code, is to treat int and bool as _almost_ completely separate types. I'm still willing to use C's implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine, no need to spell it out as blob.len != 0), but generally, if a variable is going to be conceptually a boolean, I like to declare it bool and assign to it using 'true' or 'false' rather than 0 or 1. PuTTY is an exception, because it predates the C99 bool, and I've stuck to its existing coding style even when adding new code to it. But it's been annoying me more and more, so now that I've decided C99 bool is an acceptable thing to require from our toolchain in the first place, here's a quite thorough trawl through the source doing 'boolification'. Many variables and function parameters are now typed as bool rather than int; many assignments of 0 or 1 to those variables are now spelled 'true' or 'false'. I managed this thorough conversion with the help of a custom clang plugin that I wrote to trawl the AST and apply heuristics to point out where things might want changing. So I've even managed to do a decent job on parts of the code I haven't looked at in years! To make the plugin's work easier, I pushed platform front ends generally in the direction of using standard 'bool' in preference to platform-specific boolean types like Windows BOOL or GTK's gboolean; I've left the platform booleans in places they _have_ to be for the platform APIs to work right, but variables only used by my own code have been converted wherever I found them. In a few places there are int values that look very like booleans in _most_ of the places they're used, but have a rarely-used third value, or a distinction between different nonzero values that most users don't care about. In these cases, I've _removed_ uses of 'true' and 'false' for the return values, to emphasise that there's something more subtle going on than a simple boolean answer: - the 'multisel' field in dialog.h's list box structure, for which the GTK front end in particular recognises a difference between 1 and 2 but nearly everything else treats as boolean - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you something about the specific location of the urgent pointer, but most clients only care about 0 vs 'something nonzero' - the return value of wc_match, where -1 indicates a syntax error in the wildcard. - the return values from SSH-1 RSA-key loading functions, which use -1 for 'wrong passphrase' and 0 for all other failures (so any caller which already knows it's not loading an _encrypted private_ key can treat them as boolean) - term->esc_query, and the 'query' parameter in toggle_mode in terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h, but can also hold -1 for some other intervening character that we don't support. In a few places there's an integer that I haven't turned into a bool even though it really _can_ only take values 0 or 1 (and, as above, tried to make the call sites consistent in not calling those values true and false), on the grounds that I thought it would make it more confusing to imply that the 0 value was in some sense 'negative' or bad and the 1 positive or good: - the return value of plug_accepting uses the POSIXish convention of 0=success and nonzero=error; I think if I made it bool then I'd also want to reverse its sense, and that's a job for a separate piece of work. - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1 represent the default and alternate screens. There's no obvious reason why one of those should be considered 'true' or 'positive' or 'success' - they're just indices - so I've left it as int. ssh_scp_recv had particularly confusing semantics for its previous int return value: its call sites used '<= 0' to check for error, but it never actually returned a negative number, just 0 or 1. Now the function and its call sites agree that it's a bool. In a couple of places I've renamed variables called 'ret', because I don't like that name any more - it's unclear whether it means the return value (in preparation) for the _containing_ function or the return value received from a subroutine call, and occasionally I've accidentally used the same variable for both and introduced a bug. So where one of those got in my way, I've renamed it to 'toret' or 'retd' (the latter short for 'returned') in line with my usual modern practice, but I haven't done a thorough job of finding all of them. Finally, one amusing side effect of doing this is that I've had to separate quite a few chained assignments. It used to be perfectly fine to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a the 'true' defined by stdbool.h, that idiom provokes a warning from gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
return false; /* failure */
}
static char *read_body(BinarySource *src)
{
strbuf *buf = strbuf_new_nm();
while (1) {
int c = get_byte(src);
if (c == '\r' || c == '\n' || get_err(src)) {
if (!get_err(src)) {
c = get_byte(src);
if (c != '\r' && c != '\n' && !get_err(src))
src->pos--;
}
return strbuf_to_str(buf);
}
put_byte(buf, c);
}
}
static bool read_blob(BinarySource *src, int nlines, BinarySink *bs)
{
unsigned char *blob;
char *line;
int linelen;
int i, j, k;
/* We expect at most 64 base64 characters, ie 48 real bytes, per line. */
assert(nlines < MAX_KEY_BLOB_LINES);
blob = snewn(48 * nlines, unsigned char);
for (i = 0; i < nlines; i++) {
line = read_body(src);
if (!line) {
sfree(blob);
return false;
}
linelen = strlen(line);
if (linelen % 4 != 0 || linelen > 64) {
sfree(blob);
sfree(line);
return false;
}
for (j = 0; j < linelen; j += 4) {
unsigned char decoded[3];
k = base64_decode_atom(line + j, decoded);
if (!k) {
sfree(line);
sfree(blob);
return false;
}
put_data(bs, decoded, k);
}
sfree(line);
}
sfree(blob);
return true;
}
/*
* Magic error return value for when the passphrase is wrong.
*/
ssh2_userkey ssh2_wrong_passphrase = { NULL, NULL };
const ssh_keyalg *const all_keyalgs[] = {
&ssh_rsa,
&ssh_rsa_sha256,
&ssh_rsa_sha512,
&ssh_dsa,
&ssh_ecdsa_nistp256,
&ssh_ecdsa_nistp384,
&ssh_ecdsa_nistp521,
&ssh_ecdsa_ed25519,
&ssh_ecdsa_ed448,
};
const size_t n_keyalgs = lenof(all_keyalgs);
const ssh_keyalg *find_pubkey_alg_len(ptrlen name)
{
for (size_t i = 0; i < n_keyalgs; i++)
if (ptrlen_eq_string(name, all_keyalgs[i]->ssh_id))
return all_keyalgs[i];
return NULL;
}
Invent a struct type for polymorphic SSH key data. During last week's work, I made a mistake in which I got the arguments backwards in one of the key-blob-generating functions - mistakenly swapped the 'void *' key instance with the 'BinarySink *' output destination - and I didn't spot the mistake until run time, because in C you can implicitly convert both to and from void * and so there was no compile-time failure of type checking. Now that I've introduced the FROMFIELD macro that downcasts a pointer to one field of a structure to retrieve a pointer to the whole structure, I think I might start using that more widely to indicate this kind of polymorphic subtyping. So now all the public-key functions in the struct ssh_signkey vtable handle their data instance in the form of a pointer to a subfield of a new zero-sized structure type 'ssh_key', which outside the key implementations indicates 'this is some kind of key instance but it could be of any type'; they downcast that pointer internally using FROMFIELD in place of the previous ordinary C cast, and return one by returning &foo->sshk for whatever foo they've just made up. The sshk member is not at the beginning of the structure, which means all those FROMFIELDs and &key->sshk are actually adding and subtracting an offset. Of course I could have put the member at the start anyway, but I had the idea that it's actually a feature _not_ to have the two types start at the same address, because it means you should notice earlier rather than later if you absentmindedly cast from one to the other directly rather than by the approved method (in particular, if you accidentally assign one through a void * and back without even _noticing_ you perpetrated a cast). In particular, this enforces that you can't sfree() the thing even once without realising you should instead of called the right freekey function. (I found several bugs by this method during initial testing, so I think it's already proved its worth!) While I'm here, I've also renamed the vtable structure ssh_signkey to ssh_keyalg, because it was a confusing name anyway - it describes the _algorithm_ for handling all keys of that type, not a specific key. So ssh_keyalg is the collection of code, and ssh_key is one instance of the data it handles.
2018-05-27 07:32:21 +00:00
const ssh_keyalg *find_pubkey_alg(const char *name)
{
return find_pubkey_alg_len(ptrlen_from_asciz(name));
}
ptrlen pubkey_blob_to_alg_name(ptrlen blob)
{
BinarySource src[1];
BinarySource_BARE_INIT_PL(src, blob);
return get_string(src);
}
const ssh_keyalg *pubkey_blob_to_alg(ptrlen blob)
{
return find_pubkey_alg_len(pubkey_blob_to_alg_name(blob));
}
struct ppk_cipher {
const char *name;
size_t blocklen, keylen, ivlen;
};
static const struct ppk_cipher ppk_cipher_none = { "none", 1, 0, 0 };
static const struct ppk_cipher ppk_cipher_aes256_cbc = { "aes256-cbc", 16, 32, 16 };
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 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. */
for (unsigned ctr = 0; ctr * 20 < ciphertype->keylen; ctr++) {
ssh_hash *h = ssh_hash_new(&ssh_sha1);
put_uint32(h, ctr);
put_datapl(h, passphrase);
ssh_hash_final(h, strbuf_append(storage, 20));
}
strbuf_shrink_to(storage, ciphertype->keylen);
/* In this version of the format, the CBC IV was always all 0. */
put_padding(storage, ciphertype->ivlen, 0);
/* Completely separate hash for the MAC key. */
ssh_hash *h = ssh_hash_new(&ssh_sha1);
mac_keylen = ssh_hash_alg(h)->hlen;
put_datapl(h, PTRLEN_LITERAL("putty-private-key-file-mac-key"));
put_datapl(h, passphrase);
ssh_hash_final(h, strbuf_append(storage, mac_keylen));
break;
}
default:
unreachable("bad format version in ssh2_ppk_derive_keys");
}
BinarySource src[1];
BinarySource_BARE_INIT_PL(src, ptrlen_from_strbuf(storage));
*cipherkey = get_data(src, ciphertype->keylen);
*cipheriv = get_data(src, ciphertype->ivlen);
*mackey = get_data(src, mac_keylen);
}
static int userkey_parse_line_counter(const char *text)
{
char *endptr;
unsigned long ul = strtoul(text, &endptr, 10);
if (*text && !*endptr && ul < MAX_KEY_BLOB_LINES)
return ul;
else
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)
{
char header[40], *b, *encryption, *comment, *mac;
Invent a struct type for polymorphic SSH key data. During last week's work, I made a mistake in which I got the arguments backwards in one of the key-blob-generating functions - mistakenly swapped the 'void *' key instance with the 'BinarySink *' output destination - and I didn't spot the mistake until run time, because in C you can implicitly convert both to and from void * and so there was no compile-time failure of type checking. Now that I've introduced the FROMFIELD macro that downcasts a pointer to one field of a structure to retrieve a pointer to the whole structure, I think I might start using that more widely to indicate this kind of polymorphic subtyping. So now all the public-key functions in the struct ssh_signkey vtable handle their data instance in the form of a pointer to a subfield of a new zero-sized structure type 'ssh_key', which outside the key implementations indicates 'this is some kind of key instance but it could be of any type'; they downcast that pointer internally using FROMFIELD in place of the previous ordinary C cast, and return one by returning &foo->sshk for whatever foo they've just made up. The sshk member is not at the beginning of the structure, which means all those FROMFIELDs and &key->sshk are actually adding and subtracting an offset. Of course I could have put the member at the start anyway, but I had the idea that it's actually a feature _not_ to have the two types start at the same address, because it means you should notice earlier rather than later if you absentmindedly cast from one to the other directly rather than by the approved method (in particular, if you accidentally assign one through a void * and back without even _noticing_ you perpetrated a cast). In particular, this enforces that you can't sfree() the thing even once without realising you should instead of called the right freekey function. (I found several bugs by this method during initial testing, so I think it's already proved its worth!) While I'm here, I've also renamed the vtable structure ssh_signkey to ssh_keyalg, because it was a confusing name anyway - it describes the _algorithm_ for handling all keys of that type, not a specific key. So ssh_keyalg is the collection of code, and ssh_key is one instance of the data it handles.
2018-05-27 07:32:21 +00:00
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;
Convert a lot of 'int' variables to 'bool'. My normal habit these days, in new code, is to treat int and bool as _almost_ completely separate types. I'm still willing to use C's implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine, no need to spell it out as blob.len != 0), but generally, if a variable is going to be conceptually a boolean, I like to declare it bool and assign to it using 'true' or 'false' rather than 0 or 1. PuTTY is an exception, because it predates the C99 bool, and I've stuck to its existing coding style even when adding new code to it. But it's been annoying me more and more, so now that I've decided C99 bool is an acceptable thing to require from our toolchain in the first place, here's a quite thorough trawl through the source doing 'boolification'. Many variables and function parameters are now typed as bool rather than int; many assignments of 0 or 1 to those variables are now spelled 'true' or 'false'. I managed this thorough conversion with the help of a custom clang plugin that I wrote to trawl the AST and apply heuristics to point out where things might want changing. So I've even managed to do a decent job on parts of the code I haven't looked at in years! To make the plugin's work easier, I pushed platform front ends generally in the direction of using standard 'bool' in preference to platform-specific boolean types like Windows BOOL or GTK's gboolean; I've left the platform booleans in places they _have_ to be for the platform APIs to work right, but variables only used by my own code have been converted wherever I found them. In a few places there are int values that look very like booleans in _most_ of the places they're used, but have a rarely-used third value, or a distinction between different nonzero values that most users don't care about. In these cases, I've _removed_ uses of 'true' and 'false' for the return values, to emphasise that there's something more subtle going on than a simple boolean answer: - the 'multisel' field in dialog.h's list box structure, for which the GTK front end in particular recognises a difference between 1 and 2 but nearly everything else treats as boolean - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you something about the specific location of the urgent pointer, but most clients only care about 0 vs 'something nonzero' - the return value of wc_match, where -1 indicates a syntax error in the wildcard. - the return values from SSH-1 RSA-key loading functions, which use -1 for 'wrong passphrase' and 0 for all other failures (so any caller which already knows it's not loading an _encrypted private_ key can treat them as boolean) - term->esc_query, and the 'query' parameter in toggle_mode in terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h, but can also hold -1 for some other intervening character that we don't support. In a few places there's an integer that I haven't turned into a bool even though it really _can_ only take values 0 or 1 (and, as above, tried to make the call sites consistent in not calling those values true and false), on the grounds that I thought it would make it more confusing to imply that the 0 value was in some sense 'negative' or bad and the 1 positive or good: - the return value of plug_accepting uses the POSIXish convention of 0=success and nonzero=error; I think if I made it bool then I'd also want to reverse its sense, and that's a job for a separate piece of work. - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1 represent the default and alternate screens. There's no obvious reason why one of those should be considered 'true' or 'positive' or 'success' - they're just indices - so I've left it as int. ssh_scp_recv had particularly confusing semantics for its previous int return value: its call sites used '<= 0' to check for error, but it never actually returned a negative number, just 0 or 1. Now the function and its call sites agree that it's a bool. In a couple of places I've renamed variables called 'ret', because I don't like that name any more - it's unclear whether it means the return value (in preparation) for the _containing_ function or the return value received from a subroutine call, and occasionally I've accidentally used the same variable for both and introduced a bug. So where one of those got in my way, I've renamed it to 'toret' or 'retd' (the latter short for 'returned') in line with my usual modern practice, but I haven't done a thorough job of finding all of them. Finally, one amusing side effect of doing this is that I've had to separate quite a few chained assignments. It used to be perfectly fine to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a the 'true' defined by stdbool.h, that idiom provokes a warning from gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
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;
public_blob = private_blob = cipher_mac_keys_blob = NULL;
/* Read the first header line which contains the key type. */
if (!read_header(src, header)) {
error = "no header line found in key file";
goto error;
}
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 */
old_keyfile_warning();
fmt_version = 1;
} else if (0 == strncmp(header, "PuTTY-User-Key-File-", 20)) {
/* this is a key file FROM THE FUTURE; refuse it, but with a
* more specific error message than the generic one below */
error = "PuTTY key format too new";
goto error;
} else {
error = "not a PuTTY SSH-2 private key";
goto error;
}
error = "file format error";
if ((b = read_body(src)) == NULL)
goto error;
/* Select key algorithm structure. */
alg = find_pubkey_alg(b);
if (!alg) {
sfree(b);
goto error;
}
sfree(b);
/* Read the Encryption header line. */
if (!read_header(src, header) || 0 != strcmp(header, "Encryption"))
goto error;
if ((encryption = read_body(src)) == NULL)
goto error;
if (!strcmp(encryption, "aes256-cbc")) {
ciphertype = &ppk_cipher_aes256_cbc;
} else if (!strcmp(encryption, "none")) {
ciphertype = &ppk_cipher_none;
} else {
goto error;
}
/* Read the Comment header line. */
if (!read_header(src, header) || 0 != strcmp(header, "Comment"))
goto error;
if ((comment = read_body(src)) == NULL)
goto error;
memset(&params, 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;
if ((b = read_body(src)) == NULL)
goto error;
i = userkey_parse_line_counter(b);
sfree(b);
if (i < 0)
goto error;
public_blob = strbuf_new();
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, &params.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, &params.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, &params.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;
if ((b = read_body(src)) == NULL)
goto error;
i = userkey_parse_line_counter(b);
sfree(b);
if (i < 0)
goto error;
private_blob = strbuf_new_nm();
if (!read_blob(src, i, BinarySink_UPCAST(private_blob)))
goto error;
/* Read the Private-MAC or Private-Hash header line. */
if (!read_header(src, header))
goto error;
if (0 == strcmp(header, "Private-MAC")) {
if ((mac = read_body(src)) == NULL)
goto error;
is_mac = true;
} else if (0 == strcmp(header, "Private-Hash") && fmt_version == 1) {
if ((mac = read_body(src)) == NULL)
goto error;
is_mac = false;
} else
goto error;
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,
ptrlen_from_strbuf(passphrase_salt), &params);
/*
* Decrypt the private blob.
*/
if (private_blob->len % ciphertype->blocklen)
goto error;
if (ciphertype == &ppk_cipher_aes256_cbc) {
aes256_decrypt_pubkey(cipherkey.ptr, cipheriv.ptr,
private_blob->u, private_blob->len);
}
/*
* Verify the MAC.
*/
{
unsigned char binary[32];
char realmac[sizeof(binary) * 2 + 1];
strbuf *macdata;
Convert a lot of 'int' variables to 'bool'. My normal habit these days, in new code, is to treat int and bool as _almost_ completely separate types. I'm still willing to use C's implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine, no need to spell it out as blob.len != 0), but generally, if a variable is going to be conceptually a boolean, I like to declare it bool and assign to it using 'true' or 'false' rather than 0 or 1. PuTTY is an exception, because it predates the C99 bool, and I've stuck to its existing coding style even when adding new code to it. But it's been annoying me more and more, so now that I've decided C99 bool is an acceptable thing to require from our toolchain in the first place, here's a quite thorough trawl through the source doing 'boolification'. Many variables and function parameters are now typed as bool rather than int; many assignments of 0 or 1 to those variables are now spelled 'true' or 'false'. I managed this thorough conversion with the help of a custom clang plugin that I wrote to trawl the AST and apply heuristics to point out where things might want changing. So I've even managed to do a decent job on parts of the code I haven't looked at in years! To make the plugin's work easier, I pushed platform front ends generally in the direction of using standard 'bool' in preference to platform-specific boolean types like Windows BOOL or GTK's gboolean; I've left the platform booleans in places they _have_ to be for the platform APIs to work right, but variables only used by my own code have been converted wherever I found them. In a few places there are int values that look very like booleans in _most_ of the places they're used, but have a rarely-used third value, or a distinction between different nonzero values that most users don't care about. In these cases, I've _removed_ uses of 'true' and 'false' for the return values, to emphasise that there's something more subtle going on than a simple boolean answer: - the 'multisel' field in dialog.h's list box structure, for which the GTK front end in particular recognises a difference between 1 and 2 but nearly everything else treats as boolean - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you something about the specific location of the urgent pointer, but most clients only care about 0 vs 'something nonzero' - the return value of wc_match, where -1 indicates a syntax error in the wildcard. - the return values from SSH-1 RSA-key loading functions, which use -1 for 'wrong passphrase' and 0 for all other failures (so any caller which already knows it's not loading an _encrypted private_ key can treat them as boolean) - term->esc_query, and the 'query' parameter in toggle_mode in terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h, but can also hold -1 for some other intervening character that we don't support. In a few places there's an integer that I haven't turned into a bool even though it really _can_ only take values 0 or 1 (and, as above, tried to make the call sites consistent in not calling those values true and false), on the grounds that I thought it would make it more confusing to imply that the 0 value was in some sense 'negative' or bad and the 1 positive or good: - the return value of plug_accepting uses the POSIXish convention of 0=success and nonzero=error; I think if I made it bool then I'd also want to reverse its sense, and that's a job for a separate piece of work. - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1 represent the default and alternate screens. There's no obvious reason why one of those should be considered 'true' or 'positive' or 'success' - they're just indices - so I've left it as int. ssh_scp_recv had particularly confusing semantics for its previous int return value: its call sites used '<= 0' to check for error, but it never actually returned a negative number, just 0 or 1. Now the function and its call sites agree that it's a bool. In a couple of places I've renamed variables called 'ret', because I don't like that name any more - it's unclear whether it means the return value (in preparation) for the _containing_ function or the return value received from a subroutine call, and occasionally I've accidentally used the same variable for both and introduced a bug. So where one of those got in my way, I've renamed it to 'toret' or 'retd' (the latter short for 'returned') in line with my usual modern practice, but I haven't done a thorough job of finding all of them. Finally, one amusing side effect of doing this is that I've had to separate quite a few chained assignments. It used to be perfectly fine to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a the 'true' defined by stdbool.h, that idiom provokes a warning from gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
bool free_macdata;
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. */
macdata = private_blob;
free_macdata = false;
} else {
macdata = strbuf_new_nm();
put_stringz(macdata, alg->ssh_id);
put_stringz(macdata, encryption);
put_stringz(macdata, comment);
put_string(macdata, public_blob->s,
public_blob->len);
put_string(macdata, private_blob->s,
private_blob->len);
free_macdata = true;
}
if (is_mac) {
ssh2_mac *mac;
mac = ssh2_mac_new(mac_alg, NULL);
ssh2_mac_setkey(mac, mackey);
ssh2_mac_start(mac);
put_data(mac, macdata->s, macdata->len);
ssh2_mac_genresult(mac, binary);
ssh2_mac_free(mac);
} else {
hash_simple(&ssh_sha1, ptrlen_from_strbuf(macdata), binary);
}
if (free_macdata)
strbuf_free(macdata);
for (i = 0; i < mac_alg->len; i++)
sprintf(realmac + 2 * i, "%02x", binary[i]);
if (strcmp(mac, realmac)) {
/* An incorrect MAC is an unconditional Error if the key is
* unencrypted. Otherwise, it means Wrong Passphrase. */
if (ciphertype->keylen != 0) {
error = "wrong passphrase";
ret = SSH2_WRONG_PASSPHRASE;
} else {
error = "MAC failed";
ret = NULL;
}
goto error;
}
}
/*
* Create and return the key.
*/
ret = snew(ssh2_userkey);
ret->comment = comment;
comment = NULL;
ret->key = ssh_key_new_priv(
alg, ptrlen_from_strbuf(public_blob),
ptrlen_from_strbuf(private_blob));
if (!ret->key) {
sfree(ret);
ret = NULL;
error = "createkey failed";
goto error;
}
error = NULL;
/*
* Error processing.
*/
error:
if (comment)
sfree(comment);
if (encryption)
sfree(encryption);
if (mac)
sfree(mac);
if (public_blob)
strbuf_free(public_blob);
if (private_blob)
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;
}
ssh2_userkey *ppk_load_f(const Filename *filename, const char *passphrase,
const char **errorstr)
{
LoadedFile *lf = lf_load_keyfile(filename, errorstr);
ssh2_userkey *toret;
if (lf) {
toret = ppk_load_s(BinarySource_UPCAST(lf), passphrase, errorstr);
lf_free(lf);
} else {
toret = NULL;
*errorstr = "can't open file";
}
return toret;
}
static bool rfc4716_loadpub(BinarySource *src, char **algorithm,
BinarySink *bs,
char **commentptr, const char **errorstr)
{
const char *error;
char *line, *colon, *value;
char *comment = NULL;
strbuf *pubblob = NULL;
char base64in[4];
unsigned char base64out[3];
int base64bytes;
int alglen;
line = mkstr(get_chomped_line(src));
if (!line || 0 != strcmp(line, "---- BEGIN SSH2 PUBLIC KEY ----")) {
error = "invalid begin line in SSH-2 public key file";
goto error;
}
sfree(line); line = NULL;
while (1) {
line = mkstr(get_chomped_line(src));
if (!line) {
error = "truncated SSH-2 public key file";
goto error;
}
colon = strstr(line, ": ");
if (!colon)
break;
*colon = '\0';
value = colon + 2;
if (!strcmp(line, "Comment")) {
char *p, *q;
/* Remove containing double quotes, if present */
p = value;
if (*p == '"' && p[strlen(p)-1] == '"') {
p[strlen(p)-1] = '\0';
p++;
}
/* Remove \-escaping, not in RFC4716 but seen in the wild
* in practice. */
for (q = line; *p; p++) {
if (*p == '\\' && p[1])
p++;
*q++ = *p;
}
*q = '\0';
sfree(comment); /* *just* in case of multiple Comment headers */
comment = dupstr(line);
} else if (!strcmp(line, "Subject") ||
!strncmp(line, "x-", 2)) {
/* Headers we recognise and ignore. Do nothing. */
} else {
error = "unrecognised header in SSH-2 public key file";
goto error;
}
sfree(line); line = NULL;
}
/*
* Now line contains the initial line of base64 data. Loop round
* while it still does contain base64.
*/
pubblob = strbuf_new();
base64bytes = 0;
while (line && line[0] != '-') {
char *p;
for (p = line; *p; p++) {
base64in[base64bytes++] = *p;
if (base64bytes == 4) {
int n = base64_decode_atom(base64in, base64out);
put_data(pubblob, base64out, n);
base64bytes = 0;
}
}
sfree(line); line = NULL;
line = mkstr(get_chomped_line(src));
}
/*
* Finally, check the END line makes sense.
*/
if (!line || 0 != strcmp(line, "---- END SSH2 PUBLIC KEY ----")) {
error = "invalid end line in SSH-2 public key file";
goto error;
}
sfree(line); line = NULL;
/*
* OK, we now have a public blob and optionally a comment. We must
* return the key algorithm string too, so look for that at the
* start of the public blob.
*/
if (pubblob->len < 4) {
error = "not enough data in SSH-2 public key file";
goto error;
}
alglen = toint(GET_32BIT_MSB_FIRST(pubblob->u));
if (alglen < 0 || alglen > pubblob->len-4) {
error = "invalid algorithm prefix in SSH-2 public key file";
goto error;
}
if (algorithm)
*algorithm = dupprintf("%.*s", alglen, pubblob->s+4);
if (commentptr)
*commentptr = comment;
else
sfree(comment);
put_datapl(bs, ptrlen_from_strbuf(pubblob));
strbuf_free(pubblob);
return true;
error:
sfree(line);
sfree(comment);
if (pubblob)
strbuf_free(pubblob);
if (errorstr)
*errorstr = error;
return false;
}
static bool openssh_loadpub(BinarySource *src, char **algorithm,
BinarySink *bs,
char **commentptr, const char **errorstr)
{
const char *error;
char *line, *base64;
char *comment = NULL;
unsigned char *pubblob = NULL;
int pubbloblen, pubblobsize;
int alglen;
line = mkstr(get_chomped_line(src));
base64 = strchr(line, ' ');
if (!base64) {
error = "no key blob in OpenSSH public key file";
goto error;
}
*base64++ = '\0';
comment = strchr(base64, ' ');
if (comment) {
*comment++ = '\0';
comment = dupstr(comment);
}
pubblobsize = strlen(base64) / 4 * 3;
pubblob = snewn(pubblobsize, unsigned char);
pubbloblen = 0;
while (!memchr(base64, '\0', 4)) {
assert(pubbloblen + 3 <= pubblobsize);
pubbloblen += base64_decode_atom(base64, pubblob + pubbloblen);
base64 += 4;
}
if (*base64) {
error = "invalid length for base64 data in OpenSSH public key file";
goto error;
}
/*
* Sanity check: the first word on the line should be the key
* algorithm, and should match the encoded string at the start of
* the public blob.
*/
alglen = strlen(line);
if (pubbloblen < alglen + 4 ||
GET_32BIT_MSB_FIRST(pubblob) != alglen ||
0 != memcmp(pubblob + 4, line, alglen)) {
error = "key algorithms do not match in OpenSSH public key file";
goto error;
}
/*
* Done.
*/
if (algorithm)
*algorithm = dupstr(line);
if (commentptr)
*commentptr = comment;
else
sfree(comment);
sfree(line);
put_data(bs, pubblob, pubbloblen);
sfree(pubblob);
return true;
error:
sfree(line);
sfree(comment);
sfree(pubblob);
if (errorstr)
*errorstr = error;
return false;
}
bool ppk_loadpub_s(BinarySource *src, char **algorithm, BinarySink *bs,
char **commentptr, const char **errorstr)
{
char header[40], *b;
Invent a struct type for polymorphic SSH key data. During last week's work, I made a mistake in which I got the arguments backwards in one of the key-blob-generating functions - mistakenly swapped the 'void *' key instance with the 'BinarySink *' output destination - and I didn't spot the mistake until run time, because in C you can implicitly convert both to and from void * and so there was no compile-time failure of type checking. Now that I've introduced the FROMFIELD macro that downcasts a pointer to one field of a structure to retrieve a pointer to the whole structure, I think I might start using that more widely to indicate this kind of polymorphic subtyping. So now all the public-key functions in the struct ssh_signkey vtable handle their data instance in the form of a pointer to a subfield of a new zero-sized structure type 'ssh_key', which outside the key implementations indicates 'this is some kind of key instance but it could be of any type'; they downcast that pointer internally using FROMFIELD in place of the previous ordinary C cast, and return one by returning &foo->sshk for whatever foo they've just made up. The sshk member is not at the beginning of the structure, which means all those FROMFIELDs and &key->sshk are actually adding and subtracting an offset. Of course I could have put the member at the start anyway, but I had the idea that it's actually a feature _not_ to have the two types start at the same address, because it means you should notice earlier rather than later if you absentmindedly cast from one to the other directly rather than by the approved method (in particular, if you accidentally assign one through a void * and back without even _noticing_ you perpetrated a cast). In particular, this enforces that you can't sfree() the thing even once without realising you should instead of called the right freekey function. (I found several bugs by this method during initial testing, so I think it's already proved its worth!) While I'm here, I've also renamed the vtable structure ssh_signkey to ssh_keyalg, because it was a confusing name anyway - it describes the _algorithm_ for handling all keys of that type, not a specific key. So ssh_keyalg is the collection of code, and ssh_key is one instance of the data it handles.
2018-05-27 07:32:21 +00:00
const ssh_keyalg *alg;
int type, i;
const char *error = NULL;
char *comment = NULL;
/* Initially, check if this is a public-only key file. Sometimes
* we'll be asked to read a public blob from one of those. */
type = key_type_s(src);
if (type == SSH_KEYTYPE_SSH2_PUBLIC_RFC4716) {
bool ret = rfc4716_loadpub(src, algorithm, bs, commentptr, errorstr);
return ret;
} else if (type == SSH_KEYTYPE_SSH2_PUBLIC_OPENSSH) {
bool ret = openssh_loadpub(src, algorithm, bs, commentptr, errorstr);
return ret;
} else if (type != SSH_KEYTYPE_SSH2) {
error = "not a public key or a PuTTY SSH-2 private key";
goto error;
}
/* Read the first header line which contains the key type. */
if (!read_header(src, header)
|| (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";
else
error = "not a public key or a PuTTY SSH-2 private key";
goto error;
}
error = "file format error";
if ((b = read_body(src)) == NULL)
goto error;
/* Select key algorithm structure. */
alg = find_pubkey_alg(b);
sfree(b);
if (!alg) {
goto error;
}
/* Read the Encryption header line. */
if (!read_header(src, header) || 0 != strcmp(header, "Encryption"))
goto error;
if ((b = read_body(src)) == NULL)
goto error;
sfree(b); /* we don't care */
/* Read the Comment header line. */
if (!read_header(src, header) || 0 != strcmp(header, "Comment"))
goto error;
if ((comment = read_body(src)) == NULL)
goto error;
if (commentptr)
*commentptr = comment;
else
sfree(comment);
/* Read the Public-Lines header line and the public blob. */
if (!read_header(src, header) || 0 != strcmp(header, "Public-Lines"))
goto error;
if ((b = read_body(src)) == NULL)
goto error;
i = userkey_parse_line_counter(b);
sfree(b);
if (i < 0)
goto error;
if (!read_blob(src, i, bs))
goto error;
if (algorithm)
*algorithm = dupstr(alg->ssh_id);
return true;
/*
* Error processing.
*/
error:
if (errorstr)
*errorstr = error;
if (comment && commentptr) {
sfree(comment);
*commentptr = NULL;
}
return false;
}
bool ppk_loadpub_f(const Filename *filename, char **algorithm, BinarySink *bs,
char **commentptr, const char **errorstr)
{
LoadedFile *lf = lf_load_keyfile(filename, errorstr);
if (!lf)
return false;
bool toret = ppk_loadpub_s(BinarySource_UPCAST(lf), algorithm, bs,
commentptr, errorstr);
lf_free(lf);
return toret;
}
bool ppk_encrypted_s(BinarySource *src, char **commentptr)
{
char header[40], *b, *comment;
Convert a lot of 'int' variables to 'bool'. My normal habit these days, in new code, is to treat int and bool as _almost_ completely separate types. I'm still willing to use C's implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine, no need to spell it out as blob.len != 0), but generally, if a variable is going to be conceptually a boolean, I like to declare it bool and assign to it using 'true' or 'false' rather than 0 or 1. PuTTY is an exception, because it predates the C99 bool, and I've stuck to its existing coding style even when adding new code to it. But it's been annoying me more and more, so now that I've decided C99 bool is an acceptable thing to require from our toolchain in the first place, here's a quite thorough trawl through the source doing 'boolification'. Many variables and function parameters are now typed as bool rather than int; many assignments of 0 or 1 to those variables are now spelled 'true' or 'false'. I managed this thorough conversion with the help of a custom clang plugin that I wrote to trawl the AST and apply heuristics to point out where things might want changing. So I've even managed to do a decent job on parts of the code I haven't looked at in years! To make the plugin's work easier, I pushed platform front ends generally in the direction of using standard 'bool' in preference to platform-specific boolean types like Windows BOOL or GTK's gboolean; I've left the platform booleans in places they _have_ to be for the platform APIs to work right, but variables only used by my own code have been converted wherever I found them. In a few places there are int values that look very like booleans in _most_ of the places they're used, but have a rarely-used third value, or a distinction between different nonzero values that most users don't care about. In these cases, I've _removed_ uses of 'true' and 'false' for the return values, to emphasise that there's something more subtle going on than a simple boolean answer: - the 'multisel' field in dialog.h's list box structure, for which the GTK front end in particular recognises a difference between 1 and 2 but nearly everything else treats as boolean - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you something about the specific location of the urgent pointer, but most clients only care about 0 vs 'something nonzero' - the return value of wc_match, where -1 indicates a syntax error in the wildcard. - the return values from SSH-1 RSA-key loading functions, which use -1 for 'wrong passphrase' and 0 for all other failures (so any caller which already knows it's not loading an _encrypted private_ key can treat them as boolean) - term->esc_query, and the 'query' parameter in toggle_mode in terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h, but can also hold -1 for some other intervening character that we don't support. In a few places there's an integer that I haven't turned into a bool even though it really _can_ only take values 0 or 1 (and, as above, tried to make the call sites consistent in not calling those values true and false), on the grounds that I thought it would make it more confusing to imply that the 0 value was in some sense 'negative' or bad and the 1 positive or good: - the return value of plug_accepting uses the POSIXish convention of 0=success and nonzero=error; I think if I made it bool then I'd also want to reverse its sense, and that's a job for a separate piece of work. - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1 represent the default and alternate screens. There's no obvious reason why one of those should be considered 'true' or 'positive' or 'success' - they're just indices - so I've left it as int. ssh_scp_recv had particularly confusing semantics for its previous int return value: its call sites used '<= 0' to check for error, but it never actually returned a negative number, just 0 or 1. Now the function and its call sites agree that it's a bool. In a couple of places I've renamed variables called 'ret', because I don't like that name any more - it's unclear whether it means the return value (in preparation) for the _containing_ function or the return value received from a subroutine call, and occasionally I've accidentally used the same variable for both and introduced a bug. So where one of those got in my way, I've renamed it to 'toret' or 'retd' (the latter short for 'returned') in line with my usual modern practice, but I haven't done a thorough job of finding all of them. Finally, one amusing side effect of doing this is that I've had to separate quite a few chained assignments. It used to be perfectly fine to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a the 'true' defined by stdbool.h, that idiom provokes a warning from gcc: 'suggest parentheses around assignment used as truth value'!
2018-11-02 19:23:19 +00:00
bool ret;
if (commentptr)
*commentptr = NULL;
if (!read_header(src, header)
|| (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;
}
if ((b = read_body(src)) == NULL) {
return false;
}
sfree(b); /* we don't care about key type here */
/* Read the Encryption header line. */
if (!read_header(src, header) || 0 != strcmp(header, "Encryption")) {
return false;
}
if ((b = read_body(src)) == NULL) {
return false;
}
/* Read the Comment header line. */
if (!read_header(src, header) || 0 != strcmp(header, "Comment")) {
sfree(b);
return true;
}
if ((comment = read_body(src)) == NULL) {
sfree(b);
return true;
}
if (commentptr)
*commentptr = comment;
else
sfree(comment);
if (!strcmp(b, "aes256-cbc"))
ret = true;
else
ret = false;
sfree(b);
return ret;
}
bool ppk_encrypted_f(const Filename *filename, char **commentptr)
{
LoadedFile *lf = lf_load_keyfile(filename, NULL);
if (!lf) {
if (commentptr)
*commentptr = NULL;
return false;
}
bool toret = ppk_encrypted_s(BinarySource_UPCAST(lf), commentptr);
lf_free(lf);
return toret;
}
int base64_lines(int datalen)
{
/* When encoding, we use 64 chars/line, which equals 48 real chars. */
return (datalen + 47) / 48;
}
static void base64_encode_s(BinarySink *bs, const unsigned char *data,
int datalen, int cpl)
{
int linelen = 0;
char out[4];
int n, i;
while (datalen > 0) {
n = (datalen < 3 ? datalen : 3);
base64_encode_atom(data, n, out);
data += n;
datalen -= n;
for (i = 0; i < 4; i++) {
if (linelen >= cpl) {
linelen = 0;
put_byte(bs, '\n');
}
put_byte(bs, out[i]);
linelen++;
}
}
put_byte(bs, '\n');
}
void base64_encode(FILE *fp, const unsigned char *data, int datalen, int cpl)
{
stdio_sink ss;
stdio_sink_init(&ss, fp);
base64_encode_s(BinarySink_UPCAST(&ss), data, datalen, cpl);
}
const ppk_save_parameters ppk_save_default_parameters = {
.fmt_version = 3,
/*
* 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;
int priv_encrypted_len;
int cipherblk;
int i;
const char *cipherstr;
ptrlen cipherkey, cipheriv, mackey;
const struct ppk_cipher *ciphertype;
unsigned char priv_mac[32];
/*
* Fetch the key component blobs.
*/
pub_blob = strbuf_new();
ssh_key_public_blob(key->key, BinarySink_UPCAST(pub_blob));
priv_blob = strbuf_new_nm();
ssh_key_private_blob(key->key, BinarySink_UPCAST(priv_blob));
/*
* Determine encryption details, and encrypt the private blob.
*/
if (passphrase) {
cipherstr = "aes256-cbc";
cipherblk = 16;
ciphertype = &ppk_cipher_aes256_cbc;
} else {
cipherstr = "none";
cipherblk = 1;
ciphertype = &ppk_cipher_none;
}
priv_encrypted_len = priv_blob->len + cipherblk - 1;
priv_encrypted_len -= priv_encrypted_len % cipherblk;
priv_blob_encrypted = snewn(priv_encrypted_len, unsigned char);
memset(priv_blob_encrypted, 0, priv_encrypted_len);
memcpy(priv_blob_encrypted, priv_blob->u, priv_blob->len);
/* Create padding based on the SHA hash of the unpadded blob. This prevents
* too easy a known-plaintext attack on the last block. */
hash_simple(&ssh_sha1, ptrlen_from_strbuf(priv_blob), priv_mac);
assert(priv_encrypted_len - priv_blob->len < 20);
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;
strbuf *passphrase_salt = strbuf_new();
if (params.fmt_version == 3) {
/* Invent a salt for the password hash. */
if (params.salt)
put_data(passphrase_salt, params.salt, params.saltlen);
else
random_read(strbuf_append(passphrase_salt, 16), 16);
}
cipher_mac_keys_blob = strbuf_new();
ssh2_ppk_derive_keys(params.fmt_version, ciphertype,
ptrlen_from_asciz(passphrase ? passphrase : ""),
cipher_mac_keys_blob, &cipherkey, &cipheriv, &mackey,
ptrlen_from_strbuf(passphrase_salt), &params);
const ssh2_macalg *macalg = (params.fmt_version == 2 ?
&ssh_hmac_sha1 : &ssh_hmac_sha256);
/* Now create the MAC. */
{
strbuf *macdata;
macdata = strbuf_new_nm();
put_stringz(macdata, ssh_key_ssh_id(key->key));
put_stringz(macdata, cipherstr);
put_stringz(macdata, key->comment);
put_string(macdata, pub_blob->s, pub_blob->len);
put_string(macdata, priv_blob_encrypted, priv_encrypted_len);
mac_simple(macalg, mackey, ptrlen_from_strbuf(macdata), priv_mac);
strbuf_free(macdata);
}
if (passphrase) {
assert(cipherkey.len == 32);
aes256_encrypt_pubkey(cipherkey.ptr, cipheriv.ptr,
priv_blob_encrypted, priv_encrypted_len);
}
strbuf *out = strbuf_new_nm();
put_fmt(out, "PuTTY-User-Key-File-%u: %s\n",
params.fmt_version, ssh_key_ssh_id(key->key));
put_fmt(out, "Encryption: %s\n", cipherstr);
put_fmt(out, "Comment: %s\n", key->comment);
put_fmt(out, "Public-Lines: %d\n", base64_lines(pub_blob->len));
base64_encode_s(BinarySink_UPCAST(out), pub_blob->u, pub_blob->len, 64);
if (params.fmt_version == 3 && ciphertype->keylen != 0) {
put_fmt(out, "Key-Derivation: %s\n",
params.argon2_flavour == Argon2d ? "Argon2d" :
params.argon2_flavour == Argon2i ? "Argon2i" : "Argon2id");
put_fmt(out, "Argon2-Memory: %"PRIu32"\n", params.argon2_mem);
assert(!params.argon2_passes_auto);
put_fmt(out, "Argon2-Passes: %"PRIu32"\n", params.argon2_passes);
put_fmt(out, "Argon2-Parallelism: %"PRIu32"\n",
params.argon2_parallelism);
put_fmt(out, "Argon2-Salt: ");
for (size_t i = 0; i < passphrase_salt->len; i++)
put_fmt(out, "%02x", passphrase_salt->u[i]);
put_fmt(out, "\n");
}
put_fmt(out, "Private-Lines: %d\n", base64_lines(priv_encrypted_len));
base64_encode_s(BinarySink_UPCAST(out),
priv_blob_encrypted, priv_encrypted_len, 64);
put_fmt(out, "Private-MAC: ");
for (i = 0; i < macalg->len; i++)
put_fmt(out, "%02x", priv_mac[i]);
put_fmt(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);
sfree(priv_blob_encrypted);
return out;
}
bool ppk_save_f(const Filename *filename, ssh2_userkey *key,
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, params);
bool toret = fwrite(buf->s, 1, buf->len, fp) == buf->len;
if (fclose(fp))
toret = false;
strbuf_free(buf);
return toret;
}
/* ----------------------------------------------------------------------
* Output public keys.
*/
char *ssh1_pubkey_str(RSAKey *key)
{
char *buffer;
char *dec1, *dec2;
Complete rewrite of PuTTY's bignum library. The old 'Bignum' data type is gone completely, and so is sshbn.c. In its place is a new thing called 'mp_int', handled by an entirely new library module mpint.c, with API differences both large and small. The main aim of this change is that the new library should be free of timing- and cache-related side channels. I've written the code so that it _should_ - assuming I haven't made any mistakes - do all of its work without either control flow or memory addressing depending on the data words of the input numbers. (Though, being an _arbitrary_ precision library, it does have to at least depend on the sizes of the numbers - but there's a 'formal' size that can vary separately from the actual magnitude of the represented integer, so if you want to keep it secret that your number is actually small, it should work fine to have a very long mp_int and just happen to store 23 in it.) So I've done all my conditionalisation by means of computing both answers and doing bit-masking to swap the right one into place, and all loops over the words of an mp_int go up to the formal size rather than the actual size. I haven't actually tested the constant-time property in any rigorous way yet (I'm still considering the best way to do it). But this code is surely at the very least a big improvement on the old version, even if I later find a few more things to fix. I've also completely rewritten the low-level elliptic curve arithmetic from sshecc.c; the new ecc.c is closer to being an adjunct of mpint.c than it is to the SSH end of the code. The new elliptic curve code keeps all coordinates in Montgomery-multiplication transformed form to speed up all the multiplications mod the same prime, and only converts them back when you ask for the affine coordinates. Also, I adopted extended coordinates for the Edwards curve implementation. sshecc.c has also had a near-total rewrite in the course of switching it over to the new system. While I was there, I've separated ECDSA and EdDSA more completely - they now have separate vtables, instead of a single vtable in which nearly every function had a big if statement in it - and also made the externally exposed types for an ECDSA key and an ECDH context different. A minor new feature: since the new arithmetic code includes a modular square root function, we can now support the compressed point representation for the NIST curves. We seem to have been getting along fine without that so far, but it seemed a shame not to put it in, since it was suddenly easy. In sshrsa.c, one major change is that I've removed the RSA blinding step in rsa_privkey_op, in which we randomise the ciphertext before doing the decryption. The purpose of that was to avoid timing leaks giving away the plaintext - but the new arithmetic code should take that in its stride in the course of also being careful enough to avoid leaking the _private key_, which RSA blinding had no way to do anything about in any case. Apart from those specific points, most of the rest of the changes are more or less mechanical, just changing type names and translating code into the new API.
2018-12-31 13:53:41 +00:00
dec1 = mp_get_decimal(key->exponent);
dec2 = mp_get_decimal(key->modulus);
buffer = dupprintf("%"SIZEu" %s %s%s%s", mp_get_nbits(key->modulus),
dec1, dec2, key->comment ? " " : "",
key->comment ? key->comment : "");
sfree(dec1);
sfree(dec2);
return buffer;
}
void ssh1_write_pubkey(FILE *fp, RSAKey *key)
{
char *buffer = ssh1_pubkey_str(key);
fprintf(fp, "%s\n", buffer);
sfree(buffer);
}
static char *ssh2_pubkey_openssh_str_internal(const char *comment,
const void *v_pub_blob,
int pub_len)
{
const unsigned char *ssh2blob = (const unsigned char *)v_pub_blob;
ptrlen alg;
char *buffer, *p;
int i;
{
BinarySource src[1];
BinarySource_BARE_INIT(src, ssh2blob, pub_len);
alg = get_string(src);
if (get_err(src)) {
const char *replacement_str = "INVALID-ALGORITHM";
alg.ptr = replacement_str;
alg.len = strlen(replacement_str);
}
}
buffer = snewn(alg.len +
4 * ((pub_len+2) / 3) +
(comment ? strlen(comment) : 0) + 3, char);
p = buffer + sprintf(buffer, "%.*s ", PTRLEN_PRINTF(alg));
i = 0;
while (i < pub_len) {
int n = (pub_len - i < 3 ? pub_len - i : 3);
base64_encode_atom(ssh2blob + i, n, p);
i += n;
p += 4;
}
if (comment) {
*p++ = ' ';
strcpy(p, comment);
} else
*p++ = '\0';
return buffer;
}
char *ssh2_pubkey_openssh_str(ssh2_userkey *key)
{
strbuf *blob;
char *ret;
blob = strbuf_new();
ssh_key_public_blob(key->key, BinarySink_UPCAST(blob));
ret = ssh2_pubkey_openssh_str_internal(
key->comment, blob->s, blob->len);
strbuf_free(blob);
return ret;
}
void ssh2_write_pubkey(FILE *fp, const char *comment,
const void *v_pub_blob, int pub_len,
int keytype)
{
unsigned char *pub_blob = (unsigned char *)v_pub_blob;
if (keytype == SSH_KEYTYPE_SSH2_PUBLIC_RFC4716) {
const char *p;
int i, column;
fprintf(fp, "---- BEGIN SSH2 PUBLIC KEY ----\n");
if (comment) {
fprintf(fp, "Comment: \"");
for (p = comment; *p; p++) {
if (*p == '\\' || *p == '\"')
fputc('\\', fp);
fputc(*p, fp);
}
fprintf(fp, "\"\n");
}
i = 0;
column = 0;
while (i < pub_len) {
char buf[5];
int n = (pub_len - i < 3 ? pub_len - i : 3);
base64_encode_atom(pub_blob + i, n, buf);
i += n;
buf[4] = '\0';
fputs(buf, fp);
if (++column >= 16) {
fputc('\n', fp);
column = 0;
}
}
if (column > 0)
fputc('\n', fp);
fprintf(fp, "---- END SSH2 PUBLIC KEY ----\n");
} else if (keytype == SSH_KEYTYPE_SSH2_PUBLIC_OPENSSH) {
char *buffer = ssh2_pubkey_openssh_str_internal(comment,
v_pub_blob, pub_len);
fprintf(fp, "%s\n", buffer);
sfree(buffer);
} else {
unreachable("Bad key type in ssh2_write_pubkey");
}
}
/* ----------------------------------------------------------------------
* Utility functions to compute SSH-2 fingerprints in a uniform way.
*/
static void ssh2_fingerprint_blob_md5(ptrlen blob, strbuf *sb)
{
unsigned char digest[16];
hash_simple(&ssh_md5, blob, digest);
for (unsigned i = 0; i < 16; i++)
put_fmt(sb, "%02x%s", digest[i], i==15 ? "" : ":");
}
static void ssh2_fingerprint_blob_sha256(ptrlen blob, strbuf *sb)
{
unsigned char digest[32];
hash_simple(&ssh_sha256, blob, digest);
put_datapl(sb, PTRLEN_LITERAL("SHA256:"));
for (unsigned i = 0; i < 32; i += 3) {
char buf[5];
unsigned len = 32-i;
if (len > 3)
len = 3;
base64_encode_atom(digest + i, len, buf);
put_data(sb, buf, 4);
}
strbuf_chomp(sb, '=');
}
char *ssh2_fingerprint_blob(ptrlen blob, FingerprintType fptype)
{
strbuf *sb = strbuf_new();
/*
* Identify the key algorithm, if possible.
*
* If we can't do that, then we have a seriously confused key
* blob, in which case we return only the hash.
*/
BinarySource src[1];
BinarySource_BARE_INIT_PL(src, blob);
ptrlen algname = get_string(src);
if (!get_err(src)) {
const ssh_keyalg *alg = find_pubkey_alg_len(algname);
if (alg) {
int bits = ssh_key_public_bits(alg, blob);
put_fmt(sb, "%.*s %d ", PTRLEN_PRINTF(algname), bits);
} else {
put_fmt(sb, "%.*s ", PTRLEN_PRINTF(algname));
}
}
switch (fptype) {
case SSH_FPTYPE_MD5:
ssh2_fingerprint_blob_md5(blob, sb);
break;
case SSH_FPTYPE_SHA256:
ssh2_fingerprint_blob_sha256(blob, sb);
break;
}
return strbuf_to_str(sb);
}
char **ssh2_all_fingerprints_for_blob(ptrlen blob)
{
char **fps = snewn(SSH_N_FPTYPES, char *);
for (unsigned i = 0; i < SSH_N_FPTYPES; i++)
fps[i] = ssh2_fingerprint_blob(blob, i);
return fps;
}
char *ssh2_fingerprint(ssh_key *data, FingerprintType fptype)
{
strbuf *blob = strbuf_new();
ssh_key_public_blob(data, BinarySink_UPCAST(blob));
char *ret = ssh2_fingerprint_blob(ptrlen_from_strbuf(blob), fptype);
strbuf_free(blob);
return ret;
}
char **ssh2_all_fingerprints(ssh_key *data)
{
strbuf *blob = strbuf_new();
ssh_key_public_blob(data, BinarySink_UPCAST(blob));
char **ret = ssh2_all_fingerprints_for_blob(ptrlen_from_strbuf(blob));
strbuf_free(blob);
return ret;
}
void ssh2_free_all_fingerprints(char **fps)
{
for (unsigned i = 0; i < SSH_N_FPTYPES; i++)
sfree(fps[i]);
sfree(fps);
}
/* ----------------------------------------------------------------------
* Determine the type of a private key file.
*/
static int key_type_s_internal(BinarySource *src)
{
static const ptrlen public_std_sig =
PTRLEN_DECL_LITERAL("---- BEGIN SSH2 PUBLIC KEY");
static const ptrlen putty2_sig =
PTRLEN_DECL_LITERAL("PuTTY-User-Key-File-");
static const ptrlen sshcom_sig =
PTRLEN_DECL_LITERAL("---- BEGIN SSH2 ENCRYPTED PRIVAT");
static const ptrlen openssh_new_sig =
PTRLEN_DECL_LITERAL("-----BEGIN OPENSSH PRIVATE KEY");
static const ptrlen openssh_sig =
PTRLEN_DECL_LITERAL("-----BEGIN ");
if (BinarySource_REWIND(src), expect_signature(src, rsa1_signature))
return SSH_KEYTYPE_SSH1;
if (BinarySource_REWIND(src), expect_signature(src, public_std_sig))
return SSH_KEYTYPE_SSH2_PUBLIC_RFC4716;
if (BinarySource_REWIND(src), expect_signature(src, putty2_sig))
return SSH_KEYTYPE_SSH2;
if (BinarySource_REWIND(src), expect_signature(src, openssh_new_sig))
return SSH_KEYTYPE_OPENSSH_NEW;
if (BinarySource_REWIND(src), expect_signature(src, openssh_sig))
return SSH_KEYTYPE_OPENSSH_PEM;
if (BinarySource_REWIND(src), expect_signature(src, sshcom_sig))
return SSH_KEYTYPE_SSHCOM;
BinarySource_REWIND(src);
if (get_chars(src, "0123456789").len > 0 && get_chars(src, " ").len == 1 &&
get_chars(src, "0123456789").len > 0 && get_chars(src, " ").len == 1 &&
get_chars(src, "0123456789").len > 0 &&
get_nonchars(src, " \n").len == 0)
return SSH_KEYTYPE_SSH1_PUBLIC;
BinarySource_REWIND(src);
if (find_pubkey_alg_len(get_nonchars(src, " \n")) > 0 &&
get_chars(src, " ").len == 1 &&
get_chars(src, "0123456789ABCDEFGHIJKLMNOPQRSTUV"
"WXYZabcdefghijklmnopqrstuvwxyz+/=").len > 0 &&
get_nonchars(src, " \n").len == 0)
return SSH_KEYTYPE_SSH2_PUBLIC_OPENSSH;
return SSH_KEYTYPE_UNKNOWN; /* unrecognised or EOF */
}
int key_type_s(BinarySource *src)
{
int toret = key_type_s_internal(src);
BinarySource_REWIND(src);
return toret;
}
int key_type(const Filename *filename)
{
LoadedFile *lf = lf_new(1024);
if (lf_load(lf, filename) == LF_ERROR) {
lf_free(lf);
return SSH_KEYTYPE_UNOPENABLE;
}
int toret = key_type_s(BinarySource_UPCAST(lf));
lf_free(lf);
return toret;
}
/*
* Convert the type word to a string, for `wrong type' error
* messages.
*/
const char *key_type_to_str(int type)
{
switch (type) {
case SSH_KEYTYPE_UNOPENABLE:
return "unable to open file";
case SSH_KEYTYPE_UNKNOWN:
return "not a recognised key file format";
case SSH_KEYTYPE_SSH1_PUBLIC:
return "SSH-1 public key";
case SSH_KEYTYPE_SSH2_PUBLIC_RFC4716:
return "SSH-2 public key (RFC 4716 format)";
case SSH_KEYTYPE_SSH2_PUBLIC_OPENSSH:
return "SSH-2 public key (OpenSSH format)";
case SSH_KEYTYPE_SSH1:
return "SSH-1 private key";
case SSH_KEYTYPE_SSH2:
return "PuTTY SSH-2 private key";
case SSH_KEYTYPE_OPENSSH_PEM:
return "OpenSSH SSH-2 private key (old PEM format)";
case SSH_KEYTYPE_OPENSSH_NEW:
return "OpenSSH SSH-2 private key (new format)";
case SSH_KEYTYPE_SSHCOM:
return "ssh.com SSH-2 private key";
/*
* This function is called with a key type derived from
* looking at an actual key file, so the output-only type
* OPENSSH_AUTO should never get here, and is much an INTERNAL
* ERROR as a code we don't even understand.
*/
case SSH_KEYTYPE_OPENSSH_AUTO:
unreachable("OPENSSH_AUTO should never reach key_type_to_str");
default:
unreachable("bad key type in key_type_to_str");
}
}