diff --git a/misc.h b/misc.h index e07d58e4..ff101d83 100644 --- a/misc.h +++ b/misc.h @@ -170,6 +170,8 @@ int string_length_for_printf(size_t); * string. */ #define PTRLEN_LITERAL(stringlit) \ TYPECHECK("" stringlit "", make_ptrlen(stringlit, sizeof(stringlit)-1)) +/* Make a ptrlen out of a constant byte array. */ +#define PTRLEN_FROM_CONST_BYTES(a) make_ptrlen(a, sizeof(a)) /* Wipe sensitive data out of memory that's about to be freed. Simpler * than memset because we don't need the fill char parameter; also diff --git a/pageant.c b/pageant.c index 0ad6e4a4..446790a0 100644 --- a/pageant.c +++ b/pageant.c @@ -349,6 +349,15 @@ void pageant_handle_msg(BinarySink *bs, return; } + char *invalid = ssh_key_invalid(key->key, flags); + if (invalid) { + char *msg = dupprintf("key invalid: %s", invalid); + pageant_failure_msg(bs, msg, logctx, logfn); + sfree(msg); + sfree(invalid); + return; + } + signature = strbuf_new(); ssh_key_sign(key->key, sigdata, flags, BinarySink_UPCAST(signature)); diff --git a/ssh.h b/ssh.h index 87ec3a8f..4445bed1 100644 --- a/ssh.h +++ b/ssh.h @@ -706,6 +706,7 @@ struct ssh_keyalg { /* Methods that operate on an existing ssh_key */ void (*freekey) (ssh_key *key); + char *(*invalid) (ssh_key *key, unsigned flags); void (*sign) (ssh_key *key, ptrlen data, unsigned flags, BinarySink *); bool (*verify) (ssh_key *key, ptrlen sig, ptrlen data); void (*public_blob)(ssh_key *key, BinarySink *); @@ -728,6 +729,7 @@ struct ssh_keyalg { #define ssh_key_new_priv_openssh(alg, bs) ((alg)->new_priv_openssh(alg, bs)) #define ssh_key_free(key) ((key)->vt->freekey(key)) +#define ssh_key_invalid(key, flags) ((key)->vt->invalid(key, flags)) #define ssh_key_sign(key, data, flags, bs) \ ((key)->vt->sign(key, data, flags, bs)) #define ssh_key_verify(key, sig, data) ((key)->vt->verify(key, sig, data)) diff --git a/ssh2userauth.c b/ssh2userauth.c index 9d026118..6193c7bf 100644 --- a/ssh2userauth.c +++ b/ssh2userauth.c @@ -850,9 +850,26 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) ppl_printf("Unable to load private key (%s)\r\n", error); key = NULL; + s->suppress_wait_for_response_packet = true; break; /* try something else */ } } + + /* FIXME: if we ever support variable signature + * flags, this is somewhere they'll need to be + * put */ + char *invalid = ssh_key_invalid(key->key, 0); + if (invalid) { + ppl_printf("Cannot use this private key (%s)\r\n", + invalid); + ssh_key_free(key->key); + sfree(key->comment); + sfree(key); + sfree(invalid); + key = NULL; + s->suppress_wait_for_response_packet = true; + break; /* try something else */ + } } if (key) { diff --git a/sshdss.c b/sshdss.c index 2a66c977..d7325420 100644 --- a/sshdss.c +++ b/sshdss.c @@ -83,6 +83,12 @@ static char *dss_cache_str(ssh_key *key) return strbuf_to_str(sb); } +static char *dss_invalid(ssh_key *key, unsigned flags) +{ + /* No validity criterion will stop us from using a DSA key at all */ + return NULL; +} + static bool dss_verify(ssh_key *key, ptrlen sig, ptrlen data) { struct dss_key *dss = container_of(key, struct dss_key, sshk); @@ -459,6 +465,7 @@ const ssh_keyalg ssh_dss = { dss_new_priv_openssh, dss_freekey, + dss_invalid, dss_sign, dss_verify, dss_public_blob, diff --git a/sshecc.c b/sshecc.c index 98daf972..6cefde41 100644 --- a/sshecc.c +++ b/sshecc.c @@ -549,6 +549,13 @@ static void eddsa_freekey(ssh_key *key) sfree(ek); } +static char *ec_signkey_invalid(ssh_key *key, unsigned flags) +{ + /* All validity criteria for both ECDSA and EdDSA were checked + * when we loaded the key in the first place */ + return NULL; +} + static ssh_key *ecdsa_new_pub(const ssh_keyalg *alg, ptrlen data) { const struct ecsign_extra *extra = @@ -1126,6 +1133,7 @@ const ssh_keyalg ssh_ecdsa_ed25519 = { eddsa_new_priv_openssh, eddsa_freekey, + ec_signkey_invalid, eddsa_sign, eddsa_verify, eddsa_public_blob, @@ -1155,6 +1163,7 @@ const ssh_keyalg ssh_ecdsa_nistp256 = { ecdsa_new_priv_openssh, ecdsa_freekey, + ec_signkey_invalid, ecdsa_sign, ecdsa_verify, ecdsa_public_blob, @@ -1184,6 +1193,7 @@ const ssh_keyalg ssh_ecdsa_nistp384 = { ecdsa_new_priv_openssh, ecdsa_freekey, + ec_signkey_invalid, ecdsa_sign, ecdsa_verify, ecdsa_public_blob, @@ -1213,6 +1223,7 @@ const ssh_keyalg ssh_ecdsa_nistp521 = { ecdsa_new_priv_openssh, ecdsa_freekey, + ec_signkey_invalid, ecdsa_sign, ecdsa_verify, ecdsa_public_blob, diff --git a/sshrsa.c b/sshrsa.c index 7cc02e5c..062dd1ee 100644 --- a/sshrsa.c +++ b/sshrsa.c @@ -551,75 +551,109 @@ static int rsa2_pubkey_bits(const ssh_keyalg *self, ptrlen pub) return ret; } -/* - * This is the magic ASN.1/DER prefix that goes in the decoded - * signature, between the string of FFs and the actual SHA hash - * value. The meaning of it is: - * - * 00 -- this marks the end of the FFs; not part of the ASN.1 bit itself - * - * 30 21 -- a constructed SEQUENCE of length 0x21 - * 30 09 -- a constructed sub-SEQUENCE of length 9 - * 06 05 -- an object identifier, length 5 - * 2B 0E 03 02 1A -- object id { 1 3 14 3 2 26 } - * (the 1,3 comes from 0x2B = 43 = 40*1+3) - * 05 00 -- NULL - * 04 14 -- a primitive OCTET STRING of length 0x14 - * [0x14 bytes of hash data follows] - * - * The object id in the middle there is listed as `id-sha1' in - * ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1/pkcs-1v2-1d2.asn (the - * ASN module for PKCS #1) and its expanded form is as follows: - * - * id-sha1 OBJECT IDENTIFIER ::= { - * iso(1) identified-organization(3) oiw(14) secsig(3) - * algorithms(2) 26 } - */ -static const unsigned char sha1_asn1_prefix[] = { - 0x00, 0x30, 0x21, 0x30, 0x09, 0x06, 0x05, 0x2B, - 0x0E, 0x03, 0x02, 0x1A, 0x05, 0x00, 0x04, 0x14, -}; +static inline const ssh_hashalg *rsa2_hash_alg_for_flags( + unsigned flags, const char **protocol_id_out) +{ + const ssh_hashalg *halg; + const char *protocol_id; -/* - * Two more similar pieces of ASN.1 used for signatures using SHA-256 - * and SHA-512, in the same format but differing only in various - * length fields and OID. - */ -static const unsigned char sha256_asn1_prefix[] = { - 0x00, 0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, 0x60, - 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, - 0x05, 0x00, 0x04, 0x20, -}; -static const unsigned char sha512_asn1_prefix[] = { - 0x00, 0x30, 0x51, 0x30, 0x0d, 0x06, 0x09, 0x60, - 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03, - 0x05, 0x00, 0x04, 0x40, -}; + if (flags & SSH_AGENT_RSA_SHA2_256) { + halg = &ssh_sha256; + protocol_id = "rsa-sha2-256"; + } else if (flags & SSH_AGENT_RSA_SHA2_512) { + halg = &ssh_sha512; + protocol_id = "rsa-sha2-512"; + } else { + halg = &ssh_sha1; + protocol_id = "ssh-rsa"; + } -#define SHA1_ASN1_PREFIX_LEN sizeof(sha1_asn1_prefix) + if (protocol_id_out) + *protocol_id_out = protocol_id; + + return halg; +} + +static inline ptrlen rsa_pkcs1_prefix_for_hash(const ssh_hashalg *halg) +{ + if (halg == &ssh_sha1) { + /* + * This is the magic ASN.1/DER prefix that goes in the decoded + * signature, between the string of FFs and the actual SHA-1 + * hash value. The meaning of it is: + * + * 00 -- this marks the end of the FFs; not part of the ASN.1 + * bit itself + * + * 30 21 -- a constructed SEQUENCE of length 0x21 + * 30 09 -- a constructed sub-SEQUENCE of length 9 + * 06 05 -- an object identifier, length 5 + * 2B 0E 03 02 1A -- object id { 1 3 14 3 2 26 } + * (the 1,3 comes from 0x2B = 43 = 40*1+3) + * 05 00 -- NULL + * 04 14 -- a primitive OCTET STRING of length 0x14 + * [0x14 bytes of hash data follows] + * + * The object id in the middle there is listed as `id-sha1' in + * ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1/pkcs-1v2-1d2.asn + * (the ASN module for PKCS #1) and its expanded form is as + * follows: + * + * id-sha1 OBJECT IDENTIFIER ::= { + * iso(1) identified-organization(3) oiw(14) secsig(3) + * algorithms(2) 26 } + */ + static const unsigned char sha1_asn1_prefix[] = { + 0x00, 0x30, 0x21, 0x30, 0x09, 0x06, 0x05, 0x2B, + 0x0E, 0x03, 0x02, 0x1A, 0x05, 0x00, 0x04, 0x14, + }; + return PTRLEN_FROM_CONST_BYTES(sha1_asn1_prefix); + } + + if (halg == &ssh_sha256) { + /* + * A similar piece of ASN.1 used for signatures using SHA-256, + * in the same format but differing only in various length + * fields and OID. + */ + static const unsigned char sha256_asn1_prefix[] = { + 0x00, 0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, 0x60, + 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, + 0x05, 0x00, 0x04, 0x20, + }; + return PTRLEN_FROM_CONST_BYTES(sha256_asn1_prefix); + } + + if (halg == &ssh_sha512) { + /* + * And one more for SHA-512. + */ + static const unsigned char sha512_asn1_prefix[] = { + 0x00, 0x30, 0x51, 0x30, 0x0d, 0x06, 0x09, 0x60, + 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03, + 0x05, 0x00, 0x04, 0x40, + }; + return PTRLEN_FROM_CONST_BYTES(sha512_asn1_prefix); + } + + unreachable("bad hash algorithm for RSA PKCS#1"); +} + +static inline size_t rsa_pkcs1_length_of_fixed_parts(const ssh_hashalg *halg) +{ + ptrlen asn1_prefix = rsa_pkcs1_prefix_for_hash(halg); + return halg->hlen + asn1_prefix.len + 2; +} static unsigned char *rsa_pkcs1_signature_string( size_t nbytes, const ssh_hashalg *halg, ptrlen data) { - const unsigned char *asn1_prefix; - unsigned asn1_prefix_size; - - if (halg == &ssh_sha256) { - asn1_prefix = sha256_asn1_prefix; - asn1_prefix_size = sizeof(sha256_asn1_prefix); - } else if (halg == &ssh_sha512) { - asn1_prefix = sha512_asn1_prefix; - asn1_prefix_size = sizeof(sha512_asn1_prefix); - } else { - assert(halg == &ssh_sha1); - asn1_prefix = sha1_asn1_prefix; - asn1_prefix_size = sizeof(sha1_asn1_prefix); - } - - size_t fixed_parts = halg->hlen + asn1_prefix_size + 2; + size_t fixed_parts = rsa_pkcs1_length_of_fixed_parts(halg); assert(nbytes >= fixed_parts); size_t padding = nbytes - fixed_parts; + ptrlen asn1_prefix = rsa_pkcs1_prefix_for_hash(halg); + unsigned char *bytes = snewn(nbytes, unsigned char); bytes[0] = 0; @@ -627,11 +661,11 @@ static unsigned char *rsa_pkcs1_signature_string( memset(bytes + 2, 0xFF, padding); - memcpy(bytes + 2 + padding, asn1_prefix, asn1_prefix_size); + memcpy(bytes + 2 + padding, asn1_prefix.ptr, asn1_prefix.len); ssh_hash *h = ssh_hash_new(halg); put_datapl(h, data); - ssh_hash_final(h, bytes + 2 + padding + asn1_prefix_size); + ssh_hash_final(h, bytes + 2 + padding + asn1_prefix.len); return bytes; } @@ -643,6 +677,15 @@ static bool rsa2_verify(ssh_key *key, ptrlen sig, ptrlen data) ptrlen type, in_pl; mp_int *in, *out; + /* If we need to support variable flags on verify, this is where they go */ + const ssh_hashalg *halg = rsa2_hash_alg_for_flags(0, NULL); + + /* Start by making sure the key is even long enough to encode a + * signature. If not, everything fails to verify. */ + size_t nbytes = (mp_get_nbits(rsa->modulus) + 7) / 8; + if (nbytes < rsa_pkcs1_length_of_fixed_parts(halg)) + return false; + BinarySource_BARE_INIT_PL(src, sig); type = get_string(src); /* @@ -665,8 +708,7 @@ static bool rsa2_verify(ssh_key *key, ptrlen sig, ptrlen data) unsigned diff = 0; - size_t nbytes = (mp_get_nbits(rsa->modulus) + 7) / 8; - unsigned char *bytes = rsa_pkcs1_signature_string(nbytes, &ssh_sha1, data); + unsigned char *bytes = rsa_pkcs1_signature_string(nbytes, halg, data); for (size_t i = 0; i < nbytes; i++) diff |= bytes[nbytes-1 - i] ^ mp_get_byte(out, i); smemclr(bytes, nbytes); @@ -686,16 +728,7 @@ static void rsa2_sign(ssh_key *key, ptrlen data, const ssh_hashalg *halg; const char *sign_alg_name; - if (flags & SSH_AGENT_RSA_SHA2_256) { - halg = &ssh_sha256; - sign_alg_name = "rsa-sha2-256"; - } else if (flags & SSH_AGENT_RSA_SHA2_512) { - halg = &ssh_sha512; - sign_alg_name = "rsa-sha2-512"; - } else { - halg = &ssh_sha1; - sign_alg_name = "ssh-rsa"; - } + halg = rsa2_hash_alg_for_flags(flags, &sign_alg_name); nbytes = (mp_get_nbits(rsa->modulus) + 7) / 8; @@ -716,12 +749,28 @@ static void rsa2_sign(ssh_key *key, ptrlen data, mp_free(out); } +char *rsa2_invalid(ssh_key *key, unsigned flags) +{ + RSAKey *rsa = container_of(key, RSAKey, sshk); + size_t bits = mp_get_nbits(rsa->modulus), nbytes = (bits + 7) / 8; + const char *sign_alg_name; + const ssh_hashalg *halg = rsa2_hash_alg_for_flags(flags, &sign_alg_name); + if (nbytes < rsa_pkcs1_length_of_fixed_parts(halg)) { + return dupprintf( + "%zu-bit RSA key is too short to generate %s signatures", + bits, sign_alg_name); + } + + return NULL; +} + const ssh_keyalg ssh_rsa = { rsa2_new_pub, rsa2_new_priv, rsa2_new_priv_openssh, rsa2_freekey, + rsa2_invalid, rsa2_sign, rsa2_verify, rsa2_public_blob, diff --git a/testcrypt.c b/testcrypt.c index da228985..0421747b 100644 --- a/testcrypt.c +++ b/testcrypt.c @@ -506,6 +506,14 @@ static void return_val_string_asciz(strbuf *out, char *s) return_val_string(out, sb); } +static void return_opt_val_string_asciz(strbuf *out, char *s) +{ + if (!s) + strbuf_catf(out, "NULL\n"); + else + return_val_string_asciz(out, s); +} + static void return_opt_val_cipher(strbuf *out, ssh_cipher *c) { if (!c) diff --git a/testcrypt.h b/testcrypt.h index bc7c6772..620ab4b9 100644 --- a/testcrypt.h +++ b/testcrypt.h @@ -147,6 +147,7 @@ FUNC1(val_string, ssh2_mac_genresult, val_mac) FUNC2(val_key, ssh_key_new_pub, keyalg, val_string_ptrlen) FUNC3(val_key, ssh_key_new_priv, keyalg, val_string_ptrlen, val_string_ptrlen) FUNC2(val_key, ssh_key_new_priv_openssh, keyalg, val_string_binarysource) +FUNC2(opt_val_string_asciz, ssh_key_invalid, val_key, uint) FUNC4(void, ssh_key_sign, val_key, val_string_ptrlen, uint, out_val_string_binarysink) FUNC3(boolean, ssh_key_verify, val_key, val_string_ptrlen, val_string_ptrlen) FUNC2(void, ssh_key_public_blob, val_key, out_val_string_binarysink) diff --git a/unix/uxserver.c b/unix/uxserver.c index 4fe96eff..12a9e6a5 100644 --- a/unix/uxserver.c +++ b/unix/uxserver.c @@ -416,6 +416,12 @@ int main(int argc, char **argv) "%s\n", appname, val, error); exit(1); } + char *invalid = ssh_key_invalid(uk->key, 0); + if (invalid) { + fprintf(stderr, "%s: host key '%s' is unusable: " + "%s\n", appname, val, invalid); + exit(1); + } key = uk->key; sfree(uk->comment); sfree(uk);