From 5b0f32a10008f0f487eca670373cc6a7e58a6b89 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Thu, 13 Dec 2018 18:16:07 +0000 Subject: [PATCH] Centralise RSA PKCS1 signature formatting. There was no point in rsa2_sign and rsa2_verify having mirrored versions of the same code to construct the cleartext of the RSA signature integer, just because one is building it and the other is checking it. Much more sensible to have a single function that builds it, and then rsa2_verify can compare the received integer against that while rsa2_sign encodes it into an output integer. --- sshrsa.c | 101 +++++++++++++++++++++++++++---------------------------- 1 file changed, 49 insertions(+), 52 deletions(-) diff --git a/sshrsa.c b/sshrsa.c index bf2f7bef..043cdd3c 100644 --- a/sshrsa.c +++ b/sshrsa.c @@ -718,15 +718,51 @@ static const unsigned char sha512_asn1_prefix[] = { #define SHA1_ASN1_PREFIX_LEN sizeof(sha1_asn1_prefix) +static unsigned char *rsa_pkcs1_signature_string( + size_t nbytes, const struct 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; + assert(nbytes >= fixed_parts); + size_t padding = nbytes - fixed_parts; + + unsigned char *bytes = snewn(nbytes, unsigned char); + + bytes[0] = 0; + bytes[1] = 1; + + memset(bytes + 2, 0xFF, padding); + + memcpy(bytes + 2 + padding, asn1_prefix, asn1_prefix_size); + + ssh_hash *h = ssh_hash_new(halg); + put_data(h, data.ptr, data.len); + ssh_hash_final(h, bytes + 2 + padding + asn1_prefix_size); + + return bytes; +} + static bool rsa2_verify(ssh_key *key, ptrlen sig, ptrlen data) { struct RSAKey *rsa = container_of(key, struct RSAKey, sshk); BinarySource src[1]; ptrlen type, in_pl; Bignum in, out; - int bytes, i, j; bool toret; - unsigned char hash[20]; BinarySource_BARE_INIT(src, sig.ptr, sig.len); type = get_string(src); @@ -750,29 +786,13 @@ static bool rsa2_verify(ssh_key *key, ptrlen sig, ptrlen data) toret = true; - bytes = (bignum_bitcount(rsa->modulus)+7) / 8; - /* Top (partial) byte should be zero. */ - if (bignum_byte(out, bytes - 1) != 0) - toret = false; - /* First whole byte should be 1. */ - if (bignum_byte(out, bytes - 2) != 1) - toret = false; - /* Most of the rest should be FF. */ - for (i = bytes - 3; i >= 20 + SHA1_ASN1_PREFIX_LEN; i--) { - if (bignum_byte(out, i) != 0xFF) + size_t nbytes = (bignum_bitcount(rsa->modulus) + 7) / 8; + unsigned char *bytes = rsa_pkcs1_signature_string(nbytes, &ssh_sha1, data); + for (size_t i = 0; i < nbytes; i++) + if (bytes[nbytes-1 - i] != bignum_byte(out, i)) toret = false; - } - /* Then we expect to see the sha1_asn1_prefix. */ - for (i = 20 + SHA1_ASN1_PREFIX_LEN - 1, j = 0; i >= 20; i--, j++) { - if (bignum_byte(out, i) != sha1_asn1_prefix[j]) - toret = false; - } - /* Finally, we expect to see the SHA-1 hash of the signed data. */ - SHA_Simple(data.ptr, data.len, hash); - for (i = 19, j = 0; i >= 0; i--, j++) { - if (bignum_byte(out, i) != hash[j]) - toret = false; - } + smemclr(bytes, nbytes); + sfree(bytes); freebn(out); return toret; @@ -784,50 +804,27 @@ static void rsa2_sign(ssh_key *key, const void *data, int datalen, struct RSAKey *rsa = container_of(key, struct RSAKey, sshk); unsigned char *bytes; int nbytes; - unsigned char hash[64]; Bignum in, out; - int i, j; const struct ssh_hashalg *halg; - ssh_hash *h; - const unsigned char *asn1_prefix; - unsigned asn1_prefix_size; const char *sign_alg_name; if (flags & SSH_AGENT_RSA_SHA2_256) { halg = &ssh_sha256; - asn1_prefix = sha256_asn1_prefix; - asn1_prefix_size = sizeof(sha256_asn1_prefix); sign_alg_name = "rsa-sha2-256"; } else if (flags & SSH_AGENT_RSA_SHA2_512) { halg = &ssh_sha512; - asn1_prefix = sha512_asn1_prefix; - asn1_prefix_size = sizeof(sha512_asn1_prefix); sign_alg_name = "rsa-sha2-512"; } else { halg = &ssh_sha1; - asn1_prefix = sha1_asn1_prefix; - asn1_prefix_size = sizeof(sha1_asn1_prefix); sign_alg_name = "ssh-rsa"; } - h = ssh_hash_new(halg); - put_data(h, data, datalen); - ssh_hash_final(h, hash); - - nbytes = (bignum_bitcount(rsa->modulus) - 1) / 8; - assert(1 <= nbytes - halg->hlen - asn1_prefix_size); - bytes = snewn(nbytes, unsigned char); - - bytes[0] = 1; - for (i = 1; i < nbytes - halg->hlen - asn1_prefix_size; i++) - bytes[i] = 0xFF; - for (i = nbytes - halg->hlen - asn1_prefix_size, j = 0; - i < nbytes - halg->hlen; i++, j++) - bytes[i] = asn1_prefix[j]; - for (i = nbytes - halg->hlen, j = 0; i < nbytes; i++, j++) - bytes[i] = hash[j]; + nbytes = (bignum_bitcount(rsa->modulus) + 7) / 8; + bytes = rsa_pkcs1_signature_string( + nbytes, halg, make_ptrlen(data, datalen)); in = bignum_from_bytes(bytes, nbytes); + smemclr(bytes, nbytes); sfree(bytes); out = rsa_privkey_op(in, rsa); @@ -836,7 +833,7 @@ static void rsa2_sign(ssh_key *key, const void *data, int datalen, put_stringz(bs, sign_alg_name); nbytes = (bignum_bitcount(out) + 7) / 8; put_uint32(bs, nbytes); - for (i = 0; i < nbytes; i++) + for (size_t i = 0; i < nbytes; i++) put_byte(bs, bignum_byte(out, nbytes - 1 - i)); freebn(out);