mirror of
https://gitlab.torproject.org/tpo/core/tor.git
synced 2024-11-27 22:03:31 +01:00
When parsing, reject >1024-bit RSA private keys sooner.
Private-key validation is fairly expensive for long keys in openssl, so we need to avoid it sooner.
This commit is contained in:
parent
41d52e9cd8
commit
9e1085c924
@ -389,7 +389,8 @@ get_next_token(memarea_t *area,
|
||||
RET_ERR("Couldn't parse public key.");
|
||||
} else if (!strcmp(tok->object_type, "RSA PRIVATE KEY")) { /* private key */
|
||||
tok->key = crypto_pk_new();
|
||||
if (crypto_pk_read_private_key_from_string(tok->key, obstart, eol-obstart))
|
||||
if (crypto_pk_read_private_key1024_from_string(tok->key,
|
||||
obstart, eol-obstart))
|
||||
RET_ERR("Couldn't parse private key.");
|
||||
} else { /* If it's something else, try to base64-decode it */
|
||||
int r;
|
||||
|
@ -490,7 +490,7 @@ crypto_pk_write_private_key_to_string(crypto_pk_t *env,
|
||||
static int
|
||||
crypto_pk_read_from_string_generic(crypto_pk_t *env, const char *src,
|
||||
size_t len, int severity,
|
||||
bool private_key)
|
||||
bool private_key, int max_bits)
|
||||
{
|
||||
if (len == (size_t)-1) // "-1" indicates "use the length of the string."
|
||||
len = strlen(src);
|
||||
@ -510,7 +510,7 @@ crypto_pk_read_from_string_generic(crypto_pk_t *env, const char *src,
|
||||
}
|
||||
|
||||
crypto_pk_t *pk = private_key
|
||||
? crypto_pk_asn1_decode_private((const char*)buf, n)
|
||||
? crypto_pk_asn1_decode_private((const char*)buf, n, max_bits)
|
||||
: crypto_pk_asn1_decode((const char*)buf, n);
|
||||
if (! pk) {
|
||||
log_fn(severity, LD_CRYPTO,
|
||||
@ -539,7 +539,8 @@ int
|
||||
crypto_pk_read_public_key_from_string(crypto_pk_t *env,
|
||||
const char *src, size_t len)
|
||||
{
|
||||
return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, false);
|
||||
return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, false,
|
||||
-1);
|
||||
}
|
||||
|
||||
/** Read a PEM-encoded private key from the <b>len</b>-byte string <b>src</b>
|
||||
@ -550,7 +551,21 @@ int
|
||||
crypto_pk_read_private_key_from_string(crypto_pk_t *env,
|
||||
const char *src, ssize_t len)
|
||||
{
|
||||
return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, true);
|
||||
return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, true,
|
||||
-1);
|
||||
}
|
||||
|
||||
/**
|
||||
* As crypto_pk_read_private_key_from_string(), but reject any key
|
||||
* with a modulus longer than 1024 bits before doing any expensive
|
||||
* validation on it.
|
||||
*/
|
||||
int
|
||||
crypto_pk_read_private_key1024_from_string(crypto_pk_t *env,
|
||||
const char *src, ssize_t len)
|
||||
{
|
||||
return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, true,
|
||||
1024);
|
||||
}
|
||||
|
||||
/** If a file is longer than this, we won't try to decode its private key */
|
||||
@ -578,7 +593,7 @@ crypto_pk_read_private_key_from_filename(crypto_pk_t *env,
|
||||
}
|
||||
|
||||
int rv = crypto_pk_read_from_string_generic(env, buf, (ssize_t)st.st_size,
|
||||
LOG_WARN, true);
|
||||
LOG_WARN, true, -1);
|
||||
if (rv < 0) {
|
||||
log_warn(LD_CRYPTO, "Unable to decode private key from file %s",
|
||||
escaped(keyfile));
|
||||
@ -662,7 +677,7 @@ crypto_pk_base64_decode_private(const char *str, size_t len)
|
||||
goto out;
|
||||
}
|
||||
|
||||
pk = crypto_pk_asn1_decode_private(der, der_len);
|
||||
pk = crypto_pk_asn1_decode_private(der, der_len, -1);
|
||||
|
||||
out:
|
||||
memwipe(der, 0, len+1);
|
||||
|
@ -61,6 +61,8 @@ int crypto_pk_read_public_key_from_string(crypto_pk_t *env,
|
||||
const char *src, size_t len);
|
||||
int crypto_pk_read_private_key_from_string(crypto_pk_t *env,
|
||||
const char *s, ssize_t len);
|
||||
int crypto_pk_read_private_key1024_from_string(crypto_pk_t *env,
|
||||
const char *src, ssize_t len);
|
||||
int crypto_pk_write_private_key_to_filename(crypto_pk_t *env,
|
||||
const char *fname);
|
||||
|
||||
@ -95,7 +97,8 @@ int crypto_pk_asn1_encode(const crypto_pk_t *pk, char *dest, size_t dest_len);
|
||||
crypto_pk_t *crypto_pk_asn1_decode(const char *str, size_t len);
|
||||
int crypto_pk_asn1_encode_private(const crypto_pk_t *pk,
|
||||
char *dest, size_t dest_len);
|
||||
crypto_pk_t *crypto_pk_asn1_decode_private(const char *str, size_t len);
|
||||
crypto_pk_t *crypto_pk_asn1_decode_private(const char *str, size_t len,
|
||||
int max_bits);
|
||||
int crypto_pk_get_fingerprint(crypto_pk_t *pk, char *fp_out,int add_space);
|
||||
int crypto_pk_get_hashed_fingerprint(crypto_pk_t *pk, char *fp_out);
|
||||
void crypto_add_spaces_to_fp(char *out, size_t outlen, const char *in);
|
||||
|
@ -679,9 +679,12 @@ crypto_pk_asn1_encode_private(const crypto_pk_t *pk,
|
||||
/** Given a buffer containing the DER representation of the
|
||||
* private key <b>str</b>, decode and return the result on success, or NULL
|
||||
* on failure.
|
||||
*
|
||||
* If <b>max_bits</b> is nonnegative, reject any key longer than max_bits
|
||||
* without performing any expensive validation on it.
|
||||
*/
|
||||
crypto_pk_t *
|
||||
crypto_pk_asn1_decode_private(const char *str, size_t len)
|
||||
crypto_pk_asn1_decode_private(const char *str, size_t len, int max_bits)
|
||||
{
|
||||
tor_assert(str);
|
||||
tor_assert(len < INT_MAX);
|
||||
@ -731,6 +734,15 @@ crypto_pk_asn1_decode_private(const char *str, size_t len)
|
||||
output = NULL;
|
||||
}
|
||||
|
||||
if (output) {
|
||||
const int bits = SECKEY_PublicKeyStrengthInBits(output->pubkey);
|
||||
if (max_bits > 0 && bits > max_bits) {
|
||||
log_info(LD_CRYPTO, "Private key longer than expected.");
|
||||
crypto_pk_free(output);
|
||||
output = NULL;
|
||||
}
|
||||
}
|
||||
|
||||
if (slot)
|
||||
PK11_FreeSlot(slot);
|
||||
|
||||
|
@ -566,9 +566,12 @@ crypto_pk_asn1_encode_private(const crypto_pk_t *pk, char *dest,
|
||||
|
||||
/** Decode an ASN.1-encoded private key from <b>str</b>; return the result on
|
||||
* success and NULL on failure.
|
||||
*
|
||||
* If <b>max_bits</b> is nonnegative, reject any key longer than max_bits
|
||||
* without performing any expensive validation on it.
|
||||
*/
|
||||
crypto_pk_t *
|
||||
crypto_pk_asn1_decode_private(const char *str, size_t len)
|
||||
crypto_pk_asn1_decode_private(const char *str, size_t len, int max_bits)
|
||||
{
|
||||
RSA *rsa;
|
||||
unsigned char *buf;
|
||||
@ -578,7 +581,11 @@ crypto_pk_asn1_decode_private(const char *str, size_t len)
|
||||
rsa = d2i_RSAPrivateKey(NULL, &cp, len);
|
||||
tor_free(buf);
|
||||
if (!rsa) {
|
||||
crypto_openssl_log_errors(LOG_WARN,"decoding public key");
|
||||
crypto_openssl_log_errors(LOG_WARN,"decoding private key");
|
||||
return NULL;
|
||||
}
|
||||
if (max_bits >= 0 && RSA_bits(rsa) > max_bits) {
|
||||
log_info(LD_CRYPTO, "Private key longer than expected.");
|
||||
return NULL;
|
||||
}
|
||||
crypto_pk_t *result = crypto_new_pk_from_openssl_rsa_(rsa);
|
||||
|
Loading…
Reference in New Issue
Block a user