commit 6c8c9731915a796c8a1bc11f376793459db1200f Author: Nick Mathewson <nickm@torproject.org> Date: Mon Jul 24 14:32:59 2017 -0400 Rename the hybrid_encrypt/decrypt functions; label them as dangerous We need to keep these around for TAP and old-style hidden services, but they're obsolete, and we shouldn't encourage anyone to use them. So I've added "obsolete" to their names, and a comment explaining what the problem is. Closes ticket 23026. --- changes/bug23026 | 4 ++++ src/common/crypto.c | 15 +++++++++++---- src/common/crypto.h | 4 ++-- src/or/onion_tap.c | 4 ++-- src/or/rendclient.c | 4 ++-- src/or/rendservice.c | 2 +- src/test/test.c | 3 ++- src/test/test_crypto.c | 4 ++-- src/test/test_introduce.c | 4 ++-- 9 files changed, 28 insertions(+), 16 deletions(-) diff --git a/changes/bug23026 b/changes/bug23026 new file mode 100644 index 000000000..b00745cfa --- /dev/null +++ b/changes/bug23026 @@ -0,0 +1,4 @@ + o Code simplification and refactoring: + - Rename the obsolete malleable hybrid_encrypt functions used in + TAP and old hidden services to indicate that they aren't suitable + for new protocols or formats. Closes ticket 23026. diff --git a/src/common/crypto.c b/src/common/crypto.c index 875b4eeb5..c258f239a 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -1238,9 +1238,12 @@ crypto_pk_private_sign_digest(crypto_pk_t *env, char *to, size_t tolen, * - The beginning of the source data prefixed with a 16-byte symmetric key, * padded and encrypted with the public key; followed by the rest of * the source data encrypted in AES-CTR mode with the symmetric key. + * + * NOTE that this format does not authenticate the symmetrically encrypted + * part of the data, and SHOULD NOT BE USED for new protocols. */ int -crypto_pk_public_hybrid_encrypt(crypto_pk_t *env, +crypto_pk_obsolete_public_hybrid_encrypt(crypto_pk_t *env, char *to, size_t tolen, const char *from, size_t fromlen, @@ -1302,10 +1305,14 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_t *env, return -1; } -/** Invert crypto_pk_public_hybrid_encrypt. Returns the number of bytes - * written on success, -1 on failure. */ +/** Invert crypto_pk_obsolete_public_hybrid_encrypt. Returns the number of + * bytes written on success, -1 on failure. + * + * NOTE that this format does not authenticate the symmetrically encrypted + * part of the data, and SHOULD NOT BE USED for new protocols. + */ int -crypto_pk_private_hybrid_decrypt(crypto_pk_t *env, +crypto_pk_obsolete_private_hybrid_decrypt(crypto_pk_t *env, char *to, size_t tolen, const char *from, diff --git a/src/common/crypto.h b/src/common/crypto.h index 5951321c0..cde241dd3 100644 --- a/src/common/crypto.h +++ b/src/common/crypto.h @@ -197,11 +197,11 @@ int crypto_pk_private_sign(const crypto_pk_t *env, char *to, size_t tolen, const char *from, size_t fromlen); int crypto_pk_private_sign_digest(crypto_pk_t *env, char *to, size_t tolen, const char *from, size_t fromlen); -int crypto_pk_public_hybrid_encrypt(crypto_pk_t *env, char *to, +int crypto_pk_obsolete_public_hybrid_encrypt(crypto_pk_t *env, char *to, size_t tolen, const char *from, size_t fromlen, int padding, int force); -int crypto_pk_private_hybrid_decrypt(crypto_pk_t *env, char *to, +int crypto_pk_obsolete_private_hybrid_decrypt(crypto_pk_t *env, char *to, size_t tolen, const char *from, size_t fromlen, int padding, int warnOnFailure); diff --git a/src/or/onion_tap.c b/src/or/onion_tap.c index 928fcaabf..c71fa236e 100644 --- a/src/or/onion_tap.c +++ b/src/or/onion_tap.c @@ -73,7 +73,7 @@ onion_skin_TAP_create(crypto_pk_t *dest_router_key, goto err; /* set meeting point, meeting cookie, etc here. Leave zero for now. */ - if (crypto_pk_public_hybrid_encrypt(dest_router_key, onion_skin_out, + if (crypto_pk_obsolete_public_hybrid_encrypt(dest_router_key, onion_skin_out, TAP_ONIONSKIN_CHALLENGE_LEN, challenge, DH_KEY_LEN, PK_PKCS1_OAEP_PADDING, 1)<0) @@ -122,7 +122,7 @@ onion_skin_TAP_server_handshake( k = i==0?private_key:prev_private_key; if (!k) break; - len = crypto_pk_private_hybrid_decrypt(k, challenge, + len = crypto_pk_obsolete_private_hybrid_decrypt(k, challenge, TAP_ONIONSKIN_CHALLENGE_LEN, onion_skin, TAP_ONIONSKIN_CHALLENGE_LEN, diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 441c371ba..e47e1ef63 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -286,9 +286,9 @@ rend_client_send_introduction(origin_circuit_t *introcirc, goto perm_err; } - /*XXX maybe give crypto_pk_public_hybrid_encrypt a max_len arg, + /*XXX maybe give crypto_pk_obsolete_public_hybrid_encrypt a max_len arg, * to avoid buffer overflows? */ - r = crypto_pk_public_hybrid_encrypt(intro_key, payload+DIGEST_LEN, + r = crypto_pk_obsolete_public_hybrid_encrypt(intro_key, payload+DIGEST_LEN, sizeof(payload)-DIGEST_LEN, tmp, (int)(dh_offset+DH_KEY_LEN), diff --git a/src/or/rendservice.c b/src/or/rendservice.c index b5ef72e4f..98ed1100e 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -2732,7 +2732,7 @@ rend_service_decrypt_intro( /* Decrypt the encrypted part */ result = - crypto_pk_private_hybrid_decrypt( + crypto_pk_obsolete_private_hybrid_decrypt( key, (char *)buf, sizeof(buf), (const char *)(intro->ciphertext), intro->ciphertext_len, PK_PKCS1_OAEP_PADDING, 1); diff --git a/src/test/test.c b/src/test/test.c index b6b11ce94..c5c394900 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -142,7 +142,8 @@ test_bad_onion_handshake(void *arg) /* Server: Case 1: the encrypted data is degenerate. */ memset(junk_buf, 0, sizeof(junk_buf)); - crypto_pk_public_hybrid_encrypt(pk, junk_buf2, TAP_ONIONSKIN_CHALLENGE_LEN, + crypto_pk_obsolete_public_hybrid_encrypt(pk, + junk_buf2, TAP_ONIONSKIN_CHALLENGE_LEN, junk_buf, DH_KEY_LEN, PK_PKCS1_OAEP_PADDING, 1); tt_int_op(-1, OP_EQ, onion_skin_TAP_server_handshake(junk_buf2, pk, NULL, diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index 4d9651db9..11200b4e9 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -1252,10 +1252,10 @@ test_crypto_pk(void *arg) for (i = 85; i < 140; ++i) { memset(data2,0,1024); memset(data3,0,1024); - len = crypto_pk_public_hybrid_encrypt(pk1,data2,sizeof(data2), + len = crypto_pk_obsolete_public_hybrid_encrypt(pk1,data2,sizeof(data2), data1,i,PK_PKCS1_OAEP_PADDING,0); tt_int_op(len, OP_GE, 0); - len = crypto_pk_private_hybrid_decrypt(pk1,data3,sizeof(data3), + len = crypto_pk_obsolete_private_hybrid_decrypt(pk1,data3,sizeof(data3), data2,len,PK_PKCS1_OAEP_PADDING,1); tt_int_op(len,OP_EQ, i); tt_mem_op(data1,OP_EQ, data3,i); diff --git a/src/test/test_introduce.c b/src/test/test_introduce.c index cfb8d83b1..5a2463047 100644 --- a/src/test/test_introduce.c +++ b/src/test/test_introduce.c @@ -355,7 +355,7 @@ make_intro_from_plaintext( /* * Figure out an upper bound on how big the ciphertext will be - * (see crypto_pk_public_hybrid_encrypt()) + * (see crypto_pk_obsolete_public_hybrid_encrypt()) */ ciphertext_size = PKCS1_OAEP_PADDING_OVERHEAD; ciphertext_size += crypto_pk_keysize(key); @@ -372,7 +372,7 @@ make_intro_from_plaintext( tt_assert(r >= 0); /* Do encryption */ - r = crypto_pk_public_hybrid_encrypt( + r = crypto_pk_obsolete_public_hybrid_encrypt( key, cell + DIGEST_LEN, ciphertext_size, buf, len, PK_PKCS1_OAEP_PADDING, 0);