commit b94e7de7dbf31a9b9bfe2d376013e279f15500f8 Author: Nick Mathewson nickm@torproject.org Date: Thu Jul 19 17:09:23 2018 -0400
Refactor crypto_rsa to use pem module.
This cleans up a lot of junk from crypto_rsa_openssl, and will save us duplicated code in crypto_rsa_nss (when it exists).
(Actually, it already exists, but I am going to use git rebase so that this commit precedes the creation of crypto_rsa_nss.) --- src/lib/crypt_ops/crypto_rsa.c | 253 ++++++++++++++++++++++++++ src/lib/crypt_ops/crypto_rsa.h | 16 +- src/lib/crypt_ops/crypto_rsa_openssl.c | 320 +++++---------------------------- src/test/test_crypto.c | 3 +- src/test/testing_rsakeys.c | 3 +- 5 files changed, 317 insertions(+), 278 deletions(-)
diff --git a/src/lib/crypt_ops/crypto_rsa.c b/src/lib/crypt_ops/crypto_rsa.c index 328fd732d..0f80bc967 100644 --- a/src/lib/crypt_ops/crypto_rsa.c +++ b/src/lib/crypt_ops/crypto_rsa.c @@ -23,8 +23,12 @@
#include "lib/log/log.h" #include "lib/encoding/binascii.h" +#include "lib/encoding/pem.h"
#include <string.h> +#ifdef HAVE_SYS_STAT_H +#include <sys/stat.h> +#endif
/** Return the number of bytes added by padding method <b>padding</b>. */ @@ -388,3 +392,252 @@ crypto_pk_get_common_digests(crypto_pk_t *pk, common_digests_t *digests_out) tor_free(buf); return rv; } + +static const char RSA_PUBLIC_TAG[] = "RSA PUBLIC KEY"; +static const char RSA_PRIVATE_TAG[] = "RSA PRIVATE KEY"; + +/** PEM-encode the public key portion of <b>env</b> and write it to a + * newly allocated string. On success, set *<b>dest</b> to the new + * string, *<b>len</b> to the string's length, and return 0. On + * failure, return -1. + */ +int +crypto_pk_write_public_key_to_string(crypto_pk_t *env, + char **dest, size_t *len) +{ + size_t buflen = crypto_pk_keysize(env) * 3; + char *buf = tor_malloc(buflen); + char *result = NULL; + size_t resultlen = 0; + int rv = -1; + + int n = crypto_pk_asn1_encode(env, buf, buflen); + if (n < 0) + goto done; + + resultlen = pem_encoded_size(n, RSA_PUBLIC_TAG); + result = tor_malloc(resultlen); + if (pem_encode(result, resultlen, + (const unsigned char *)buf, n, RSA_PUBLIC_TAG) < 0) { + goto done; + } + + *dest = result; + *len = resultlen; + rv = 0; + + done: + if (rv < 0 && result) { + memwipe(result, 0, resultlen); + tor_free(result); + } + memwipe(buf, 0, buflen); + tor_free(buf); + return rv; +} + +/** PEM-encode the private key portion of <b>env</b> and write it to a + * newly allocated string. On success, set *<b>dest</b> to the new + * string, *<b>len</b> to the string's length, and return 0. On + * failure, return -1. + */ +int +crypto_pk_write_private_key_to_string(crypto_pk_t *env, + char **dest, size_t *len) +{ + size_t buflen = crypto_pk_keysize(env) * 16; + char *buf = tor_malloc(buflen); + char *result = NULL; + size_t resultlen = 0; + int rv = -1; + + int n = crypto_pk_asn1_encode_private(env, buf, buflen); + if (n < 0) + goto done; + + resultlen = pem_encoded_size(n, RSA_PRIVATE_TAG); + result = tor_malloc(resultlen); + if (pem_encode(result, resultlen, + (const unsigned char *)buf, n, RSA_PRIVATE_TAG) < 0) + goto done; + + *dest = result; + *len = resultlen; + rv = 0; + done: + if (rv < 0 && result) { + memwipe(result, 0, resultlen); + tor_free(result); + } + memwipe(buf, 0, buflen); + tor_free(buf); + return rv; +} + +/** Read a PEM-encoded public key from the first <b>len</b> characters of + * <b>src</b>, and store the result in <b>env</b>. Return 0 on success, -1 on + * failure. + */ +int +crypto_pk_read_public_key_from_string(crypto_pk_t *env, + const char *src, size_t len) +{ + if (len == (size_t)-1) + len = strlen(src); + + size_t buflen = len; + uint8_t *buf = tor_malloc(buflen); + int rv = -1; + + int n = pem_decode(buf, buflen, src, len, RSA_PUBLIC_TAG); + if (n < 0) + goto done; + + crypto_pk_t *pk = crypto_pk_asn1_decode((const char*)buf, n); + if (! pk) + goto done; + + crypto_pk_assign_public(env, pk); + crypto_pk_free(pk); + rv = 0; + + done: + memwipe(buf, 0, buflen); + tor_free(buf); + return rv; +} + +/** Read a PEM-encoded private key from the <b>len</b>-byte string <b>s</b> + * into <b>env</b>. Return 0 on success, -1 on failure. If len is -1, + * the string is nul-terminated. + */ +int +crypto_pk_read_private_key_from_string(crypto_pk_t *env, + const char *s, ssize_t len) +{ + if (len == -1) + len = strlen(s); + + size_t buflen = len; + uint8_t *buf = tor_malloc(buflen); + int rv = -1; + + int n = pem_decode(buf, buflen, s, len, RSA_PRIVATE_TAG); + if (n < 0) { + goto done; + } + + crypto_pk_t *pk = crypto_pk_asn1_decode_private((const char *)buf, n); + if (! pk) + goto done; + + crypto_pk_assign_private(env, pk); + crypto_pk_free(pk); + rv = 0; + + done: + memwipe(buf, 0, buflen); + tor_free(buf); + return rv; +} + +/** Read a PEM-encoded private key from the file named by + * <b>keyfile</b> into <b>env</b>. Return 0 on success, -1 on failure. + */ +int +crypto_pk_read_private_key_from_filename(crypto_pk_t *env, + const char *keyfile) +{ + struct stat st; + char *buf = read_file_to_str(keyfile, 0, &st); + if (!buf) + return -1; + + int rv = crypto_pk_read_private_key_from_string(env, buf, st.st_size); + memwipe(buf, 0, st.st_size); + tor_free(buf); + return rv; +} + +/** Write the private key from <b>env</b> into the file named by <b>fname</b>, + * PEM-encoded. Return 0 on success, -1 on failure. + */ +int +crypto_pk_write_private_key_to_filename(crypto_pk_t *env, + const char *fname) +{ + char *s = NULL; + size_t n = 0; + + if (crypto_pk_write_private_key_to_string(env, &s, &n) < 0) + return -1; + + int rv = write_bytes_to_file(fname, s, n, 0); + memwipe(s, 0, n); + tor_free(s); + return rv; +} + +/** Given a crypto_pk_t <b>pk</b>, allocate a new buffer containing the + * Base64 encoding of the DER representation of the private key as a NUL + * terminated string, and return it via <b>priv_out</b>. Return 0 on + * success, -1 on failure. + * + * It is the caller's responsibility to sanitize and free the resulting buffer. + */ +int +crypto_pk_base64_encode_private(const crypto_pk_t *pk, char **priv_out) +{ + size_t buflen = crypto_pk_keysize(pk)*16; + char *buf = tor_malloc(buflen); + char *result = NULL; + size_t reslen = 0; + bool ok = false; + + int n = crypto_pk_asn1_encode_private(pk, buf, buflen); + + if (n < 0) + goto done; + + reslen = base64_encode_size(n, 0)+1; + result = tor_malloc(reslen); + if (base64_encode(result, reslen, buf, n, 0) < 0) + goto done; + + ok = true; + + done: + memwipe(buf, 0, buflen); + tor_free(buf); + if (result && ! ok) { + memwipe(result, 0, reslen); + tor_free(result); + } + *priv_out = result; + return ok ? 0 : -1; +} + +/** Given a string containing the Base64 encoded DER representation of the + * private key <b>str</b>, decode and return the result on success, or NULL + * on failure. + */ +crypto_pk_t * +crypto_pk_base64_decode_private(const char *str, size_t len) +{ + crypto_pk_t *pk = NULL; + + char *der = tor_malloc_zero(len + 1); + int der_len = base64_decode(der, len, str, len); + if (der_len <= 0) { + log_warn(LD_CRYPTO, "Stored RSA private key seems corrupted (base64)."); + goto out; + } + + pk = crypto_pk_asn1_decode_private(der, der_len); + + out: + memwipe(der, 0, len+1); + tor_free(der); + + return pk; +} diff --git a/src/lib/crypt_ops/crypto_rsa.h b/src/lib/crypt_ops/crypto_rsa.h index 4a5d92c6b..bcb2b51dd 100644 --- a/src/lib/crypt_ops/crypto_rsa.h +++ b/src/lib/crypt_ops/crypto_rsa.h @@ -93,6 +93,9 @@ int crypto_pk_private_sign(const crypto_pk_t *env, char *to, size_t tolen, const char *from, size_t fromlen); 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); 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); @@ -107,6 +110,14 @@ int crypto_pk_get_common_digests(crypto_pk_t *pk, int crypto_pk_base64_encode_private(const crypto_pk_t *pk, char **priv_out); crypto_pk_t *crypto_pk_base64_decode_private(const char *str, size_t len);
+#ifdef TOR_UNIT_TESTS +#ifdef ENABLE_NSS +struct SECItemStr; +STATIC int secitem_uint_cmp(const struct SECItemStr *a, + const struct SECItemStr *b); +#endif +#endif + #ifdef ENABLE_OPENSSL /* Prototypes for private functions only used by tortls.c, crypto.c, and the * unit tests. */ @@ -118,8 +129,7 @@ MOCK_DECL(struct evp_pkey_st *, crypto_pk_get_openssl_evp_pkey_,( crypto_pk_t *env,int private)); #endif
-#ifdef TOR_UNIT_TESTS -void crypto_pk_assign_(crypto_pk_t *dest, const crypto_pk_t *src); -#endif +void crypto_pk_assign_public(crypto_pk_t *dest, const crypto_pk_t *src); +void crypto_pk_assign_private(crypto_pk_t *dest, const crypto_pk_t *src);
#endif diff --git a/src/lib/crypt_ops/crypto_rsa_openssl.c b/src/lib/crypt_ops/crypto_rsa_openssl.c index d1b56c3b6..9a3729a87 100644 --- a/src/lib/crypt_ops/crypto_rsa_openssl.c +++ b/src/lib/crypt_ops/crypto_rsa_openssl.c @@ -183,216 +183,6 @@ crypto_pk_generate_key_with_bits,(crypto_pk_t *env, int bits)) return 0; }
-/** A PEM callback that always reports a failure to get a password */ -static int -pem_no_password_cb(char *buf, int size, int rwflag, void *u) -{ - (void)buf; - (void)size; - (void)rwflag; - (void)u; - return -1; -} - -/** Read a PEM-encoded private key from the <b>len</b>-byte string <b>s</b> - * into <b>env</b>. Return 0 on success, -1 on failure. If len is -1, - * the string is nul-terminated. - */ -int -crypto_pk_read_private_key_from_string(crypto_pk_t *env, - const char *s, ssize_t len) -{ - BIO *b; - - tor_assert(env); - tor_assert(s); - tor_assert(len < INT_MAX && len < SSIZE_T_CEILING); - - /* Create a read-only memory BIO, backed by the string 's' */ - b = BIO_new_mem_buf((char*)s, (int)len); - if (!b) - return -1; - - if (env->key) - RSA_free(env->key); - - env->key = PEM_read_bio_RSAPrivateKey(b,NULL,pem_no_password_cb,NULL); - - BIO_free(b); - - if (!env->key) { - crypto_openssl_log_errors(LOG_WARN, "Error parsing private key"); - return -1; - } - return 0; -} - -/** Read a PEM-encoded private key from the file named by - * <b>keyfile</b> into <b>env</b>. Return 0 on success, -1 on failure. - */ -int -crypto_pk_read_private_key_from_filename(crypto_pk_t *env, - const char *keyfile) -{ - char *contents; - int r; - - /* Read the file into a string. */ - contents = read_file_to_str(keyfile, 0, NULL); - if (!contents) { - log_warn(LD_CRYPTO, "Error reading private key from "%s"", keyfile); - return -1; - } - - /* Try to parse it. */ - r = crypto_pk_read_private_key_from_string(env, contents, -1); - memwipe(contents, 0, strlen(contents)); - tor_free(contents); - if (r) - return -1; /* read_private_key_from_string already warned, so we don't.*/ - - /* Make sure it's valid. */ - if (crypto_pk_check_key(env) <= 0) - return -1; - - return 0; -} - -/** Helper function to implement crypto_pk_write_*_key_to_string. Return 0 on - * success, -1 on failure. */ -static int -crypto_pk_write_key_to_string_impl(crypto_pk_t *env, char **dest, - size_t *len, int is_public) -{ - BUF_MEM *buf; - BIO *b; - int r; - - tor_assert(env); - tor_assert(env->key); - tor_assert(dest); - - b = BIO_new(BIO_s_mem()); /* Create a memory BIO */ - if (!b) - return -1; - - /* Now you can treat b as if it were a file. Just use the - * PEM_*_bio_* functions instead of the non-bio variants. - */ - if (is_public) - r = PEM_write_bio_RSAPublicKey(b, env->key); - else - r = PEM_write_bio_RSAPrivateKey(b, env->key, NULL,NULL,0,NULL,NULL); - - if (!r) { - crypto_openssl_log_errors(LOG_WARN, "writing RSA key to string"); - BIO_free(b); - return -1; - } - - BIO_get_mem_ptr(b, &buf); - - *dest = tor_malloc(buf->length+1); - memcpy(*dest, buf->data, buf->length); - (*dest)[buf->length] = 0; /* nul terminate it */ - *len = buf->length; - - BIO_free(b); - - return 0; -} - -/** PEM-encode the public key portion of <b>env</b> and write it to a - * newly allocated string. On success, set *<b>dest</b> to the new - * string, *<b>len</b> to the string's length, and return 0. On - * failure, return -1. - */ -int -crypto_pk_write_public_key_to_string(crypto_pk_t *env, char **dest, - size_t *len) -{ - return crypto_pk_write_key_to_string_impl(env, dest, len, 1); -} - -/** PEM-encode the private key portion of <b>env</b> and write it to a - * newly allocated string. On success, set *<b>dest</b> to the new - * string, *<b>len</b> to the string's length, and return 0. On - * failure, return -1. - */ -int -crypto_pk_write_private_key_to_string(crypto_pk_t *env, char **dest, - size_t *len) -{ - return crypto_pk_write_key_to_string_impl(env, dest, len, 0); -} - -/** Read a PEM-encoded public key from the first <b>len</b> characters of - * <b>src</b>, and store the result in <b>env</b>. Return 0 on success, -1 on - * failure. - */ -int -crypto_pk_read_public_key_from_string(crypto_pk_t *env, const char *src, - size_t len) -{ - BIO *b; - - tor_assert(env); - tor_assert(src); - tor_assert(len<INT_MAX); - - b = BIO_new(BIO_s_mem()); /* Create a memory BIO */ - if (!b) - return -1; - - BIO_write(b, src, (int)len); - - if (env->key) - RSA_free(env->key); - env->key = PEM_read_bio_RSAPublicKey(b, NULL, pem_no_password_cb, NULL); - BIO_free(b); - if (!env->key) { - crypto_openssl_log_errors(LOG_WARN, "reading public key from string"); - return -1; - } - - return 0; -} - -/** Write the private key from <b>env</b> into the file named by <b>fname</b>, - * PEM-encoded. Return 0 on success, -1 on failure. - */ -int -crypto_pk_write_private_key_to_filename(crypto_pk_t *env, - const char *fname) -{ - BIO *bio; - char *cp; - long len; - char *s; - int r; - - tor_assert(crypto_pk_key_is_private(env)); - - if (!(bio = BIO_new(BIO_s_mem()))) - return -1; - if (PEM_write_bio_RSAPrivateKey(bio, env->key, NULL,NULL,0,NULL,NULL) - == 0) { - crypto_openssl_log_errors(LOG_WARN, "writing private key"); - BIO_free(bio); - return -1; - } - len = BIO_get_mem_data(bio, &cp); - tor_assert(len >= 0); - s = tor_malloc(len+1); - memcpy(s, cp, len); - s[len]='\0'; - r = write_str_to_file(fname, s, 0); - BIO_free(bio); - memwipe(s, 0, strlen(s)); - tor_free(s); - return r; -} - /** Return true iff <b>env</b> has a valid key. */ int @@ -512,11 +302,11 @@ crypto_pk_dup_key(crypto_pk_t *env) return env; }
-#ifdef TOR_UNIT_TESTS -/** For testing: replace dest with src. (Dest must have a refcount - * of 1) */ +/** Replace dest with src (private key only). (Dest must have a refcount + * of 1) + */ void -crypto_pk_assign_(crypto_pk_t *dest, const crypto_pk_t *src) +crypto_pk_assign_private(crypto_pk_t *dest, const crypto_pk_t *src) { tor_assert(dest); tor_assert(dest->refs == 1); @@ -524,7 +314,19 @@ crypto_pk_assign_(crypto_pk_t *dest, const crypto_pk_t *src) RSA_free(dest->key); dest->key = RSAPrivateKey_dup(src->key); } -#endif /* defined(TOR_UNIT_TESTS) */ + +/** Replace dest with src (public key only). (Dest must have a refcount + * of 1) + */ +void +crypto_pk_assign_public(crypto_pk_t *dest, const crypto_pk_t *src) +{ + tor_assert(dest); + tor_assert(dest->refs == 1); + tor_assert(src); + RSA_free(dest->key); + dest->key = RSAPublicKey_dup(src->key); +}
/** Make a real honest-to-goodness copy of <b>env</b>, and return it. * Returns NULL on failure. */ @@ -733,74 +535,48 @@ crypto_pk_asn1_decode(const char *str, size_t len) return crypto_new_pk_from_openssl_rsa_(rsa); }
-/** Given a crypto_pk_t <b>pk</b>, allocate a new buffer containing the - * Base64 encoding of the DER representation of the private key as a NUL - * terminated string, and return it via <b>priv_out</b>. Return 0 on - * success, -1 on failure. - * - * It is the caller's responsibility to sanitize and free the resulting buffer. +/** ASN.1-encode the private portion of <b>pk</b> into <b>dest</b>. + * Return -1 on error, or the number of characters used on success. */ int -crypto_pk_base64_encode_private(const crypto_pk_t *pk, char **priv_out) +crypto_pk_asn1_encode_private(const crypto_pk_t *pk, char *dest, + size_t dest_len) { - unsigned char *der = NULL; - int der_len; - int ret = -1; - - *priv_out = NULL; + int len; + unsigned char *buf = NULL;
- der_len = i2d_RSAPrivateKey(pk->key, &der); - if (der_len < 0 || der == NULL) - return ret; + len = i2d_RSAPrivateKey(pk->key, &buf); + if (len < 0 || buf == NULL) + return -1;
- size_t priv_len = base64_encode_size(der_len, 0) + 1; - char *priv = tor_malloc_zero(priv_len); - if (base64_encode(priv, priv_len, (char *)der, der_len, 0) >= 0) { - *priv_out = priv; - ret = 0; - } else { - tor_free(priv); + if ((size_t)len > dest_len || dest_len > SIZE_T_CEILING) { + OPENSSL_free(buf); + return -1; } - - memwipe(der, 0, der_len); - OPENSSL_free(der); - return ret; + /* We don't encode directly into 'dest', because that would be illegal + * type-punning. (C99 is smarter than me, C99 is smarter than me...) + */ + memcpy(dest,buf,len); + OPENSSL_free(buf); + return len; }
-/** Given a string containing the Base64 encoded DER representation of the - * private key <b>str</b>, decode and return the result on success, or NULL - * on failure. +/** Decode an ASN.1-encoded private key from <b>str</b>; return the result on + * success and NULL on failure. */ crypto_pk_t * -crypto_pk_base64_decode_private(const char *str, size_t len) +crypto_pk_asn1_decode_private(const char *str, size_t len) { - crypto_pk_t *pk = NULL; - - char *der = tor_malloc_zero(len + 1); - int der_len = base64_decode(der, len, str, len); - if (der_len <= 0) { - log_warn(LD_CRYPTO, "Stored RSA private key seems corrupted (base64)."); - goto out; - } - - const unsigned char *dp = (unsigned char*)der; /* Shut the compiler up. */ - RSA *rsa = d2i_RSAPrivateKey(NULL, &dp, der_len); + RSA *rsa; + unsigned char *buf; + const unsigned char *cp; + cp = buf = tor_malloc(len); + memcpy(buf,str,len); + rsa = d2i_RSAPrivateKey(NULL, &cp, len); + tor_free(buf); if (!rsa) { - crypto_openssl_log_errors(LOG_WARN, "decoding private key"); - goto out; - } - - pk = crypto_new_pk_from_openssl_rsa_(rsa); - - /* Make sure it's valid. */ - if (crypto_pk_check_key(pk) <= 0) { - crypto_pk_free(pk); - pk = NULL; - goto out; + crypto_openssl_log_errors(LOG_WARN,"decoding public key"); + return NULL; } - - out: - memwipe(der, 0, len + 1); - tor_free(der); - return pk; + return crypto_new_pk_from_openssl_rsa_(rsa); } diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index 81d43ff13..90fb8d468 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -1519,7 +1519,8 @@ test_crypto_digests(void *arg) (void)arg; k = crypto_pk_new(); tt_assert(k); - r = crypto_pk_read_private_key_from_string(k, AUTHORITY_SIGNKEY_3, -1); + r = crypto_pk_read_private_key_from_string(k, AUTHORITY_SIGNKEY_3, + strlen(AUTHORITY_SIGNKEY_3)); tt_assert(!r);
r = crypto_pk_get_digest(k, digest); diff --git a/src/test/testing_rsakeys.c b/src/test/testing_rsakeys.c index a8c9ce4ce..c8062b82d 100644 --- a/src/test/testing_rsakeys.c +++ b/src/test/testing_rsakeys.c @@ -490,7 +490,7 @@ crypto_pk_generate_key_with_bits__get_cached(crypto_pk_t *env, int bits) { if (bits == 1024 || bits == 2048) { crypto_pk_t *newkey = pk_generate_internal(bits); - crypto_pk_assign_(env, newkey); + crypto_pk_assign_private(env, newkey); crypto_pk_free(newkey); } else { return crypto_pk_generate_key_with_bits__real(env, bits); @@ -544,4 +544,3 @@ init_pregenerated_keys(void) crypto_pk_generate_key_with_bits__get_cached); #endif /* defined(USE_PREGENERATED_RSA_KEYS) */ } -