From e043b96887be4aaa103de8e4d31f8fc9008ae75d Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 16 Dec 2016 12:04:57 -0500 Subject: [PATCH 1/4] trunnel: Move ESTABLISH_INTRO cell sig_len after the end_sig_fields Signed-off-by: David Goulet --- src/trunnel/hs/cell_establish_intro.c | 12 ++++++------ src/trunnel/hs/cell_establish_intro.h | 8 ++++---- src/trunnel/hs/cell_establish_intro.trunnel | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/trunnel/hs/cell_establish_intro.c b/src/trunnel/hs/cell_establish_intro.c index 02094fe2bf..633bd7c214 100644 --- a/src/trunnel/hs/cell_establish_intro.c +++ b/src/trunnel/hs/cell_establish_intro.c @@ -231,6 +231,11 @@ hs_cell_establish_intro_getconstarray_handshake_mac(const hs_cell_establish_intr { return (const uint8_t *)hs_cell_establish_intro_getarray_handshake_mac((hs_cell_establish_intro_t*)inp); } +const uint8_t * +hs_cell_establish_intro_get_end_sig_fields(const hs_cell_establish_intro_t *inp) +{ + return inp->end_sig_fields; +} uint16_t hs_cell_establish_intro_get_sig_len(const hs_cell_establish_intro_t *inp) { @@ -242,11 +247,6 @@ hs_cell_establish_intro_set_sig_len(hs_cell_establish_intro_t *inp, uint16_t val inp->sig_len = val; return 0; } -const uint8_t * -hs_cell_establish_intro_get_end_sig_fields(const hs_cell_establish_intro_t *inp) -{ - return inp->end_sig_fields; -} size_t hs_cell_establish_intro_getlen_sig(const hs_cell_establish_intro_t *inp) { @@ -518,12 +518,12 @@ hs_cell_establish_intro_parse_into(hs_cell_establish_intro_t *obj, const uint8_t CHECK_REMAINING(TRUNNEL_SHA3_256_LEN, truncated); memcpy(obj->handshake_mac, ptr, TRUNNEL_SHA3_256_LEN); remaining -= TRUNNEL_SHA3_256_LEN; ptr += TRUNNEL_SHA3_256_LEN; + obj->end_sig_fields = ptr; /* Parse u16 sig_len */ CHECK_REMAINING(2, truncated); obj->sig_len = trunnel_ntohs(trunnel_get_uint16(ptr)); remaining -= 2; ptr += 2; - obj->end_sig_fields = ptr; /* Parse u8 sig[sig_len] */ CHECK_REMAINING(obj->sig_len, truncated); diff --git a/src/trunnel/hs/cell_establish_intro.h b/src/trunnel/hs/cell_establish_intro.h index a9166f10d8..725d47cd85 100644 --- a/src/trunnel/hs/cell_establish_intro.h +++ b/src/trunnel/hs/cell_establish_intro.h @@ -19,8 +19,8 @@ struct hs_cell_establish_intro_st { struct cell_extension_st *extensions; const uint8_t *end_mac_fields; uint8_t handshake_mac[TRUNNEL_SHA3_256_LEN]; - uint16_t sig_len; const uint8_t *end_sig_fields; + uint16_t sig_len; TRUNNEL_DYNARRAY_HEAD(, uint8_t) sig; uint8_t trunnel_error_code_; }; @@ -170,6 +170,9 @@ uint8_t * hs_cell_establish_intro_getarray_handshake_mac(hs_cell_establish_intro * a const pointer */ const uint8_t * hs_cell_establish_intro_getconstarray_handshake_mac(const hs_cell_establish_intro_t *inp); +/** Return the position for end_sig_fields when we parsed this object + */ +const uint8_t * hs_cell_establish_intro_get_end_sig_fields(const hs_cell_establish_intro_t *inp); /** Return the value of the sig_len field of the * hs_cell_establish_intro_t in 'inp' */ @@ -179,9 +182,6 @@ uint16_t hs_cell_establish_intro_get_sig_len(const hs_cell_establish_intro_t *in * return -1 and set the error code on 'inp' on failure. */ int hs_cell_establish_intro_set_sig_len(hs_cell_establish_intro_t *inp, uint16_t val); -/** Return the position for end_sig_fields when we parsed this object - */ -const uint8_t * hs_cell_establish_intro_get_end_sig_fields(const hs_cell_establish_intro_t *inp); /** Return the length of the dynamic array holding the sig field of * the hs_cell_establish_intro_t in 'inp'. */ diff --git a/src/trunnel/hs/cell_establish_intro.trunnel b/src/trunnel/hs/cell_establish_intro.trunnel index 4f9e8f7e08..33a133bf67 100644 --- a/src/trunnel/hs/cell_establish_intro.trunnel +++ b/src/trunnel/hs/cell_establish_intro.trunnel @@ -26,9 +26,9 @@ struct hs_cell_establish_intro { u8 handshake_mac[TRUNNEL_SHA3_256_LEN]; /* Signature */ - u16 sig_len; /* Indicate the end of the handshake authentication data. */ @ptr end_sig_fields; + u16 sig_len; u8 sig[sig_len]; }; From b0ccb6bfa505dc072c7af87261f6ac04e5e106db Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 16 Dec 2016 12:09:22 -0500 Subject: [PATCH 2/4] hs: Add an extra safety check on ESTABLISH_INTRO sig len Signed-off-by: David Goulet --- src/or/hs_intropoint.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/or/hs_intropoint.c b/src/or/hs_intropoint.c index bfc7ec3876..7bd80d5c9d 100644 --- a/src/or/hs_intropoint.c +++ b/src/or/hs_intropoint.c @@ -70,7 +70,11 @@ verify_establish_intro_cell(const hs_cell_establish_intro_t *cell, ed25519_signature_t sig_struct; const uint8_t *sig_array = hs_cell_establish_intro_getconstarray_sig(cell); - if (hs_cell_establish_intro_getlen_sig(cell) != sizeof(sig_struct.sig)) { + /* Make sure the signature length is of the right size. For EXTRA safety, + * we check both the size of the array and the length which must be the + * same. Safety first!*/ + if (hs_cell_establish_intro_getlen_sig(cell) != sizeof(sig_struct.sig) || + hs_cell_establish_intro_get_sig_len(cell) != sizeof(sig_struct.sig)) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "ESTABLISH_INTRO sig len is invalid"); return -1; From db0e926849954b36f3eeba274308f2c9bb8292f1 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 16 Dec 2016 12:12:37 -0500 Subject: [PATCH 3/4] hs: Remove a useless cast in verify_establish_intro_cell() Signed-off-by: David Goulet --- src/or/hs_intropoint.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/hs_intropoint.c b/src/or/hs_intropoint.c index 7bd80d5c9d..b37c9a1b9f 100644 --- a/src/or/hs_intropoint.c +++ b/src/or/hs_intropoint.c @@ -87,7 +87,7 @@ verify_establish_intro_cell(const hs_cell_establish_intro_t *cell, const size_t sig_msg_len = cell->end_sig_fields - msg; int sig_mismatch = ed25519_checksig_prefixed(&sig_struct, - (uint8_t*) msg, sig_msg_len, + msg, sig_msg_len, ESTABLISH_INTRO_SIG_PREFIX, &auth_key); if (sig_mismatch) { From e76b072defa5f5d6f5429d4df9afdd1335591658 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 16 Dec 2016 12:18:05 -0500 Subject: [PATCH 4/4] test: fix the generate ESTABLISH_INTRO v3 cell The "sig_len" fields was moved below the "end_sig_fields" in the trunnel specification so when signing the cell content, the function generating such a cell needed to be adjust. Closes #20991 Signed-off-by: David Goulet --- src/or/hs_service.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 3b5a3e7853..7b12b63065 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -152,8 +152,9 @@ generate_establish_intro_cell(const uint8_t *circuit_key_material, tor_assert(encoded_len > ED25519_SIG_LEN); if (ed25519_sign_prefixed(&sig, - (uint8_t*) cell_bytes_tmp, - encoded_len - ED25519_SIG_LEN, + cell_bytes_tmp, + encoded_len - + (ED25519_SIG_LEN + sizeof(cell->sig_len)), ESTABLISH_INTRO_SIG_PREFIX, &key_struct)) { log_warn(LD_BUG, "Unable to gen signature for ESTABLISH_INTRO cell.");