From ac9a470c641fd3ba826cdad07b1a7a495c00acba Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 17 Jul 2018 09:16:11 -0400 Subject: [PATCH] Extract the shared part of crypto_dh_compute_secret. --- src/lib/crypt_ops/crypto_dh.c | 47 +++++++++++++++++++++++++++ src/lib/crypt_ops/crypto_dh.h | 7 +++- src/lib/crypt_ops/crypto_dh_openssl.c | 34 ++++++++----------- 3 files changed, 66 insertions(+), 22 deletions(-) diff --git a/src/lib/crypt_ops/crypto_dh.c b/src/lib/crypt_ops/crypto_dh.c index 8bb66379fd..6f763e37a1 100644 --- a/src/lib/crypt_ops/crypto_dh.c +++ b/src/lib/crypt_ops/crypto_dh.c @@ -42,3 +42,50 @@ const char OAKLEY_PRIME_2[] = "302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9" "A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE6" "49286651ECE65381FFFFFFFFFFFFFFFF"; + +/** Given a DH key exchange object, and our peer's value of g^y (as a + * pubkey_len-byte value in pubkey) generate + * secret_bytes_out bytes of shared key material and write them + * to secret_out. Return the number of bytes generated on success, + * or -1 on failure. + * + * (We generate key material by computing + * SHA1( g^xy || "\x00" ) || SHA1( g^xy || "\x01" ) || ... + * where || is concatenation.) + */ +ssize_t +crypto_dh_compute_secret(int severity, crypto_dh_t *dh, + const char *pubkey, size_t pubkey_len, + char *secret_out, size_t secret_bytes_out) +{ + tor_assert(secret_bytes_out/DIGEST_LEN <= 255); + + unsigned char *secret_tmp = NULL; + size_t secret_len=0, secret_tmp_len=0; + secret_tmp_len = crypto_dh_get_bytes(dh); + secret_tmp = tor_malloc(secret_tmp_len); + + ssize_t result = crypto_dh_handshake(severity, dh, pubkey, pubkey_len, + secret_tmp, secret_tmp_len); + if (result < 0) + goto error; + + secret_len = result; + if (crypto_expand_key_material_TAP(secret_tmp, secret_len, + (uint8_t*)secret_out, secret_bytes_out)<0) + goto error; + secret_len = secret_bytes_out; + + goto done; + error: + result = -1; + done: + if (secret_tmp) { + memwipe(secret_tmp, 0, secret_tmp_len); + tor_free(secret_tmp); + } + if (result < 0) + return result; + else + return secret_len; +} diff --git a/src/lib/crypt_ops/crypto_dh.h b/src/lib/crypt_ops/crypto_dh.h index 2d25ba8c85..f8e4e4f43e 100644 --- a/src/lib/crypt_ops/crypto_dh.h +++ b/src/lib/crypt_ops/crypto_dh.h @@ -40,7 +40,11 @@ ssize_t crypto_dh_compute_secret(int severity, crypto_dh_t *dh, void crypto_dh_free_(crypto_dh_t *dh); #define crypto_dh_free(dh) FREE_AND_NULL(crypto_dh_t, crypto_dh_free_, (dh)) -/* Crypto DH free */ +ssize_t crypto_dh_handshake(int severity, crypto_dh_t *dh, + const char *pubkey, size_t pubkey_len, + unsigned char *secret_out, + size_t secret_bytes_out); + void crypto_dh_free_all(void); /* Prototypes for private functions only used by tortls.c, crypto.c, and the @@ -48,4 +52,5 @@ void crypto_dh_free_all(void); struct dh_st; struct dh_st *crypto_dh_get_dh_(crypto_dh_t *dh); + #endif /* !defined(TOR_CRYPTO_DH_H) */ diff --git a/src/lib/crypt_ops/crypto_dh_openssl.c b/src/lib/crypt_ops/crypto_dh_openssl.c index 4457b18144..395058d92a 100644 --- a/src/lib/crypt_ops/crypto_dh_openssl.c +++ b/src/lib/crypt_ops/crypto_dh_openssl.c @@ -405,27 +405,29 @@ tor_check_dh_key(int severity, const BIGNUM *bn) /** Given a DH key exchange object, and our peer's value of g^y (as a * pubkey_len-byte value in pubkey) generate - * secret_bytes_out bytes of shared key material and write them - * to secret_out. Return the number of bytes generated on success, + * g^xy as a big-endian integer in secret_out. + * Return the number of bytes generated on success, * or -1 on failure. * - * (We generate key material by computing - * SHA1( g^xy || "\x00" ) || SHA1( g^xy || "\x01" ) || ... - * where || is concatenation.) + * This function MUST validate that g^y is actually in the group. */ ssize_t -crypto_dh_compute_secret(int severity, crypto_dh_t *dh, - const char *pubkey, size_t pubkey_len, - char *secret_out, size_t secret_bytes_out) +crypto_dh_handshake(int severity, crypto_dh_t *dh, + const char *pubkey, size_t pubkey_len, + unsigned char *secret_out, size_t secret_bytes_out) { - char *secret_tmp = NULL; BIGNUM *pubkey_bn = NULL; - size_t secret_len=0, secret_tmp_len=0; + size_t secret_len=0; int result=0; + tor_assert(dh); tor_assert(secret_bytes_out/DIGEST_LEN <= 255); tor_assert(pubkey_len < INT_MAX); + if (BUG(crypto_dh_get_bytes(dh) > (int)secret_bytes_out)) { + goto error; + } + if (!(pubkey_bn = BN_bin2bn((const unsigned char*)pubkey, (int)pubkey_len, NULL))) goto error; @@ -434,18 +436,12 @@ crypto_dh_compute_secret(int severity, crypto_dh_t *dh, log_fn(severity, LD_CRYPTO,"Rejected invalid g^x"); goto error; } - secret_tmp_len = crypto_dh_get_bytes(dh); - secret_tmp = tor_malloc(secret_tmp_len); - result = DH_compute_key((unsigned char*)secret_tmp, pubkey_bn, dh->dh); + result = DH_compute_key(secret_out, pubkey_bn, dh->dh); if (result < 0) { log_warn(LD_CRYPTO,"DH_compute_key() failed."); goto error; } secret_len = result; - if (crypto_expand_key_material_TAP((uint8_t*)secret_tmp, secret_len, - (uint8_t*)secret_out, secret_bytes_out)<0) - goto error; - secret_len = secret_bytes_out; goto done; error: @@ -454,10 +450,6 @@ crypto_dh_compute_secret(int severity, crypto_dh_t *dh, crypto_openssl_log_errors(LOG_WARN, "completing DH handshake"); if (pubkey_bn) BN_clear_free(pubkey_bn); - if (secret_tmp) { - memwipe(secret_tmp, 0, secret_tmp_len); - tor_free(secret_tmp); - } if (result < 0) return result; else