commit c4d17faf81d8cfe4cf943ba11be03413c58f4d44 Author: George Kadianakis desnacked@riseup.net Date: Thu Jul 6 14:15:23 2017 +0300
Explicit length checks in circuit_init_cpath_crypto(). --- src/or/circuitbuild.c | 23 ++++++++++++++++++----- src/or/circuitbuild.h | 5 +++-- src/or/command.c | 3 ++- src/or/cpuworker.c | 2 +- src/or/hs_circuit.c | 6 ++++-- src/or/rendservice.c | 4 +++- 6 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index fdcf3e7..0dd33fc 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -46,6 +46,7 @@ #include "crypto.h" #include "directory.h" #include "entrynodes.h" +#include "hs_ntor.h" #include "main.h" #include "microdesc.h" #include "networkstatus.h" @@ -1331,7 +1332,7 @@ circuit_extend(cell_t *cell, circuit_t *circ) * service circuits and <b>key_data</b> must be at least * HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN bytes in length. * - * If <b>is_hs_v3</b> is not set, key_data must contain CPATH_KEY_MATERIAL + * If <b>is_hs_v3</b> is not set, key_data must contain CPATH_KEY_MATERIAL_LEN * bytes, which are used as follows: * - 20 to initialize f_digest * - 20 to initialize b_digest @@ -1339,9 +1340,12 @@ circuit_extend(cell_t *cell, circuit_t *circ) * - 16 to key b_crypto * * (If 'reverse' is true, then f_XX and b_XX are swapped.) + * + * Return 0 if init was successful, else -1 if it failed. */ int -circuit_init_cpath_crypto(crypt_path_t *cpath, const char *key_data, +circuit_init_cpath_crypto(crypt_path_t *cpath, + const char *key_data, size_t key_data_len, int reverse, int is_hs_v3) { crypto_digest_t *tmp_digest; @@ -1354,6 +1358,13 @@ circuit_init_cpath_crypto(crypt_path_t *cpath, const char *key_data, tor_assert(!(cpath->f_crypto || cpath->b_crypto || cpath->f_digest || cpath->b_digest));
+ /* Basic key size validation */ + if (is_hs_v3 && BUG(key_data_len != HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN)) { + return -1; + } else if (!is_hs_v3 && BUG(key_data_len != CPATH_KEY_MATERIAL_LEN)) { + return -1; + } + /* If we are using this cpath for next gen onion services use SHA3-256, otherwise use good ol' SHA1 */ if (is_hs_v3) { @@ -1450,7 +1461,7 @@ circuit_finish_handshake(origin_circuit_t *circ,
onion_handshake_state_release(&hop->handshake_state);
- if (circuit_init_cpath_crypto(hop, keys, 0, 0)<0) { + if (circuit_init_cpath_crypto(hop, keys, sizeof(keys), 0, 0)<0) { return -END_CIRC_REASON_TORPROTOCOL; }
@@ -1517,12 +1528,14 @@ circuit_truncated(origin_circuit_t *circ, crypt_path_t *layer, int reason) int onionskin_answer(or_circuit_t *circ, const created_cell_t *created_cell, - const char *keys, + const char *keys, size_t keys_len, const uint8_t *rend_circ_nonce) { cell_t cell; crypt_path_t *tmp_cpath;
+ tor_assert(keys_len == CPATH_KEY_MATERIAL_LEN); + if (created_cell_format(&cell, created_cell) < 0) { log_warn(LD_BUG,"couldn't format created cell (type=%d, len=%d)", (int)created_cell->cell_type, (int)created_cell->handshake_len); @@ -1538,7 +1551,7 @@ onionskin_answer(or_circuit_t *circ, log_debug(LD_CIRC,"init digest forward 0x%.8x, backward 0x%.8x.", (unsigned int)get_uint32(keys), (unsigned int)get_uint32(keys+20)); - if (circuit_init_cpath_crypto(tmp_cpath, keys, 0, 0)<0) { + if (circuit_init_cpath_crypto(tmp_cpath, keys, keys_len, 0, 0)<0) { log_warn(LD_BUG,"Circuit initialization failed"); tor_free(tmp_cpath); return -1; diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 6910b3a..62a6367 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -31,7 +31,8 @@ int circuit_timeout_want_to_count_circ(origin_circuit_t *circ); int circuit_send_next_onion_skin(origin_circuit_t *circ); void circuit_note_clock_jumped(int seconds_elapsed); int circuit_extend(cell_t *cell, circuit_t *circ); -int circuit_init_cpath_crypto(crypt_path_t *cpath, const char *key_data, +int circuit_init_cpath_crypto(crypt_path_t *cpath, + const char *key_data, size_t key_data_len, int reverse, int is_hs_v3); struct created_cell_t; int circuit_finish_handshake(origin_circuit_t *circ, @@ -40,7 +41,7 @@ int circuit_truncated(origin_circuit_t *circ, crypt_path_t *layer, int reason); int onionskin_answer(or_circuit_t *circ, const struct created_cell_t *created_cell, - const char *keys, + const char *keys, size_t keys_len, const uint8_t *rend_circ_nonce); MOCK_DECL(int, circuit_all_predicted_ports_handled, (time_t now, int *need_uptime, diff --git a/src/or/command.c b/src/or/command.c index c667cbb..2c82984 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -381,7 +381,8 @@ command_process_create_cell(cell_t *cell, channel_t *chan) created_cell.handshake_len = len;
if (onionskin_answer(circ, &created_cell, - (const char *)keys, rend_circ_nonce)<0) { + (const char *)keys, sizeof(keys), + rend_circ_nonce)<0) { log_warn(LD_OR,"Failed to reply to CREATE_FAST cell. Closing."); circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); return; diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c index 06d45f9..f786d79 100644 --- a/src/or/cpuworker.c +++ b/src/or/cpuworker.c @@ -372,7 +372,7 @@ cpuworker_onion_handshake_replyfn(void *work_)
if (onionskin_answer(circ, &rpl.created_cell, - (const char*)rpl.keys, + (const char*)rpl.keys, sizeof(rpl.keys), rpl.rend_auth_material) < 0) { log_warn(LD_OR,"onionskin_answer failed. Closing."); circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 1729080..42c5dcb 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -60,7 +60,7 @@ create_rend_cpath(const uint8_t *ntor_key_seed, int is_service_side) cpath = tor_malloc_zero(sizeof(crypt_path_t)); cpath->magic = CRYPT_PATH_MAGIC;
- if (circuit_init_cpath_crypto(cpath, (char*)keys, + if (circuit_init_cpath_crypto(cpath, (char*)keys, sizeof(keys), is_service_side, 1) < 0) { tor_free(cpath); goto err; @@ -96,7 +96,9 @@ create_rend_cpath_legacy(origin_circuit_t *circ, const uint8_t *rend_cell_body) goto err; } /* ... and set up cpath. */ - if (circuit_init_cpath_crypto(hop, keys+DIGEST_LEN, 0, 0)<0) + if (circuit_init_cpath_crypto(hop, + keys+DIGEST_LEN, sizeof(keys)-DIGEST_LEN, + 0, 0) < 0) goto err;
/* Check whether the digest is right... */ diff --git a/src/or/rendservice.c b/src/or/rendservice.c index d389a87..b8e704e 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -2195,7 +2195,9 @@ rend_service_receive_introduction(origin_circuit_t *circuit,
cpath->rend_dh_handshake_state = dh; dh = NULL; - if (circuit_init_cpath_crypto(cpath,keys+DIGEST_LEN,1, 0)<0) + if (circuit_init_cpath_crypto(cpath, + keys+DIGEST_LEN, sizeof(keys)-DIGEST_LEN, + 1, 0)<0) goto err; memcpy(cpath->rend_circ_nonce, keys, DIGEST_LEN);