May 2019 - tor-commits - lists.torproject.org

[tor/master] prop289: Add SENDME trunnel declaration
by asn＠torproject.org 02 May '19

02 May '19

commit eef78ac0b07096a6925ae42e8b5d526304fa54a8 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Jan 8 11:13:37 2019 -0500 prop289: Add SENDME trunnel declaration Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/trunnel/include.am | 3 + src/trunnel/sendme.c | 492 +++++++++++++++++++++++++++++++++++++++++++++ src/trunnel/sendme.h | 170 ++++++++++++++++ src/trunnel/sendme.trunnel | 18 ++ 4 files changed, 683 insertions(+) diff --git a/src/trunnel/include.am b/src/trunnel/include.am index 4f4f1d362..82e7a6695 100644 --- a/src/trunnel/include.am +++ b/src/trunnel/include.am @@ -11,6 +11,7 @@ TRUNNELINPUTS = \ src/trunnel/link_handshake.trunnel \ src/trunnel/pwbox.trunnel \ src/trunnel/channelpadding_negotiation.trunnel \ + src/trunnel/sendme.trunnel \ src/trunnel/socks5.trunnel \ src/trunnel/circpad_negotiation.trunnel @@ -24,6 +25,7 @@ TRUNNELSOURCES = \ src/trunnel/hs/cell_introduce1.c \ src/trunnel/hs/cell_rendezvous.c \ src/trunnel/channelpadding_negotiation.c \ + src/trunnel/sendme.c \ src/trunnel/socks5.c \ src/trunnel/netinfo.c \ src/trunnel/circpad_negotiation.c @@ -40,6 +42,7 @@ TRUNNELHEADERS = \ src/trunnel/hs/cell_introduce1.h \ src/trunnel/hs/cell_rendezvous.h \ src/trunnel/channelpadding_negotiation.h \ + src/trunnel/sendme.h \ src/trunnel/socks5.h \ src/trunnel/netinfo.h \ src/trunnel/circpad_negotiation.h diff --git a/src/trunnel/sendme.c b/src/trunnel/sendme.c new file mode 100644 index 000000000..08f9ed5e9 --- /dev/null +++ b/src/trunnel/sendme.c @@ -0,0 +1,492 @@ +/* sendme.c -- generated by Trunnel v1.5.2. + * https://gitweb.torproject.org/trunnel.git + * You probably shouldn't edit this file. + */ +#include <stdlib.h> +#include "trunnel-impl.h" + +#include "sendme.h" + +#define TRUNNEL_SET_ERROR_CODE(obj) \ + do { \ + (obj)->trunnel_error_code_ = 1; \ + } while (0) + +#if defined(__COVERITY__) || defined(__clang_analyzer__) +/* If we're running a static analysis tool, we don't want it to complain + * that some of our remaining-bytes checks are dead-code. */ +int sendme_deadcode_dummy__ = 0; +#define OR_DEADCODE_DUMMY || sendme_deadcode_dummy__ +#else +#define OR_DEADCODE_DUMMY +#endif + +#define CHECK_REMAINING(nbytes, label) \ + do { \ + if (remaining < (nbytes) OR_DEADCODE_DUMMY) { \ + goto label; \ + } \ + } while (0) + +sendme_cell_t * +sendme_cell_new(void) +{ + sendme_cell_t *val = trunnel_calloc(1, sizeof(sendme_cell_t)); + if (NULL == val) + return NULL; + return val; +} + +/** Release all storage held inside 'obj', but do not free 'obj'. + */ +static void +sendme_cell_clear(sendme_cell_t *obj) +{ + (void) obj; + TRUNNEL_DYNARRAY_WIPE(&obj->data); + TRUNNEL_DYNARRAY_CLEAR(&obj->data); +} + +void +sendme_cell_free(sendme_cell_t *obj) +{ + if (obj == NULL) + return; + sendme_cell_clear(obj); + trunnel_memwipe(obj, sizeof(sendme_cell_t)); + trunnel_free_(obj); +} + +uint8_t +sendme_cell_get_version(const sendme_cell_t *inp) +{ + return inp->version; +} +int +sendme_cell_set_version(sendme_cell_t *inp, uint8_t val) +{ + if (! ((val == 0 || val == 1))) { + TRUNNEL_SET_ERROR_CODE(inp); + return -1; + } + inp->version = val; + return 0; +} +uint16_t +sendme_cell_get_data_len(const sendme_cell_t *inp) +{ + return inp->data_len; +} +int +sendme_cell_set_data_len(sendme_cell_t *inp, uint16_t val) +{ + inp->data_len = val; + return 0; +} +size_t +sendme_cell_getlen_data(const sendme_cell_t *inp) +{ + return TRUNNEL_DYNARRAY_LEN(&inp->data); +} + +uint8_t +sendme_cell_get_data(sendme_cell_t *inp, size_t idx) +{ + return TRUNNEL_DYNARRAY_GET(&inp->data, idx); +} + +uint8_t +sendme_cell_getconst_data(const sendme_cell_t *inp, size_t idx) +{ + return sendme_cell_get_data((sendme_cell_t*)inp, idx); +} +int +sendme_cell_set_data(sendme_cell_t *inp, size_t idx, uint8_t elt) +{ + TRUNNEL_DYNARRAY_SET(&inp->data, idx, elt); + return 0; +} +int +sendme_cell_add_data(sendme_cell_t *inp, uint8_t elt) +{ +#if SIZE_MAX >= UINT16_MAX + if (inp->data.n_ == UINT16_MAX) + goto trunnel_alloc_failed; +#endif + TRUNNEL_DYNARRAY_ADD(uint8_t, &inp->data, elt, {}); + return 0; + trunnel_alloc_failed: + TRUNNEL_SET_ERROR_CODE(inp); + return -1; +} + +uint8_t * +sendme_cell_getarray_data(sendme_cell_t *inp) +{ + return inp->data.elts_; +} +const uint8_t * +sendme_cell_getconstarray_data(const sendme_cell_t *inp) +{ + return (const uint8_t *)sendme_cell_getarray_data((sendme_cell_t*)inp); +} +int +sendme_cell_setlen_data(sendme_cell_t *inp, size_t newlen) +{ + uint8_t *newptr; +#if UINT16_MAX < SIZE_MAX + if (newlen > UINT16_MAX) + goto trunnel_alloc_failed; +#endif + newptr = trunnel_dynarray_setlen(&inp->data.allocated_, + &inp->data.n_, inp->data.elts_, newlen, + sizeof(inp->data.elts_[0]), (trunnel_free_fn_t) NULL, + &inp->trunnel_error_code_); + if (newlen != 0 && newptr == NULL) + goto trunnel_alloc_failed; + inp->data.elts_ = newptr; + return 0; + trunnel_alloc_failed: + TRUNNEL_SET_ERROR_CODE(inp); + return -1; +} +const char * +sendme_cell_check(const sendme_cell_t *obj) +{ + if (obj == NULL) + return "Object was NULL"; + if (obj->trunnel_error_code_) + return "A set function failed on this object"; + if (! (obj->version == 0 || obj->version == 1)) + return "Integer out of bounds"; + if (TRUNNEL_DYNARRAY_LEN(&obj->data) != obj->data_len) + return "Length mismatch for data"; + return NULL; +} + +ssize_t +sendme_cell_encoded_len(const sendme_cell_t *obj) +{ + ssize_t result = 0; + + if (NULL != sendme_cell_check(obj)) + return -1; + + + /* Length of u8 version IN [0, 1] */ + result += 1; + + /* Length of u16 data_len */ + result += 2; + + /* Length of u8 data[data_len] */ + result += TRUNNEL_DYNARRAY_LEN(&obj->data); + return result; +} +int +sendme_cell_clear_errors(sendme_cell_t *obj) +{ + int r = obj->trunnel_error_code_; + obj->trunnel_error_code_ = 0; + return r; +} +ssize_t +sendme_cell_encode(uint8_t *output, const size_t avail, const sendme_cell_t *obj) +{ + ssize_t result = 0; + size_t written = 0; + uint8_t *ptr = output; + const char *msg; +#ifdef TRUNNEL_CHECK_ENCODED_LEN + const ssize_t encoded_len = sendme_cell_encoded_len(obj); +#endif + + if (NULL != (msg = sendme_cell_check(obj))) + goto check_failed; + +#ifdef TRUNNEL_CHECK_ENCODED_LEN + trunnel_assert(encoded_len >= 0); +#endif + + /* Encode u8 version IN [0, 1] */ + trunnel_assert(written <= avail); + if (avail - written < 1) + goto truncated; + trunnel_set_uint8(ptr, (obj->version)); + written += 1; ptr += 1; + + /* Encode u16 data_len */ + trunnel_assert(written <= avail); + if (avail - written < 2) + goto truncated; + trunnel_set_uint16(ptr, trunnel_htons(obj->data_len)); + written += 2; ptr += 2; + + /* Encode u8 data[data_len] */ + { + size_t elt_len = TRUNNEL_DYNARRAY_LEN(&obj->data); + trunnel_assert(obj->data_len == elt_len); + trunnel_assert(written <= avail); + if (avail - written < elt_len) + goto truncated; + if (elt_len) + memcpy(ptr, obj->data.elts_, elt_len); + written += elt_len; ptr += elt_len; + } + + + trunnel_assert(ptr == output + written); +#ifdef TRUNNEL_CHECK_ENCODED_LEN + { + trunnel_assert(encoded_len >= 0); + trunnel_assert((size_t)encoded_len == written); + } + +#endif + + return written; + + truncated: + result = -2; + goto fail; + check_failed: + (void)msg; + result = -1; + goto fail; + fail: + trunnel_assert(result < 0); + return result; +} + +/** As sendme_cell_parse(), but do not allocate the output object. + */ +static ssize_t +sendme_cell_parse_into(sendme_cell_t *obj, const uint8_t *input, const size_t len_in) +{ + const uint8_t *ptr = input; + size_t remaining = len_in; + ssize_t result = 0; + (void)result; + + /* Parse u8 version IN [0, 1] */ + CHECK_REMAINING(1, truncated); + obj->version = (trunnel_get_uint8(ptr)); + remaining -= 1; ptr += 1; + if (! (obj->version == 0 || obj->version == 1)) + goto fail; + + /* Parse u16 data_len */ + CHECK_REMAINING(2, truncated); + obj->data_len = trunnel_ntohs(trunnel_get_uint16(ptr)); + remaining -= 2; ptr += 2; + + /* Parse u8 data[data_len] */ + CHECK_REMAINING(obj->data_len, truncated); + TRUNNEL_DYNARRAY_EXPAND(uint8_t, &obj->data, obj->data_len, {}); + obj->data.n_ = obj->data_len; + if (obj->data_len) + memcpy(obj->data.elts_, ptr, obj->data_len); + ptr += obj->data_len; remaining -= obj->data_len; + trunnel_assert(ptr + remaining == input + len_in); + return len_in - remaining; + + truncated: + return -2; + trunnel_alloc_failed: + return -1; + fail: + result = -1; + return result; +} + +ssize_t +sendme_cell_parse(sendme_cell_t **output, const uint8_t *input, const size_t len_in) +{ + ssize_t result; + *output = sendme_cell_new(); + if (NULL == *output) + return -1; + result = sendme_cell_parse_into(*output, input, len_in); + if (result < 0) { + sendme_cell_free(*output); + *output = NULL; + } + return result; +} +sendme_data_v1_t * +sendme_data_v1_new(void) +{ + sendme_data_v1_t *val = trunnel_calloc(1, sizeof(sendme_data_v1_t)); + if (NULL == val) + return NULL; + return val; +} + +/** Release all storage held inside 'obj', but do not free 'obj'. + */ +static void +sendme_data_v1_clear(sendme_data_v1_t *obj) +{ + (void) obj; +} + +void +sendme_data_v1_free(sendme_data_v1_t *obj) +{ + if (obj == NULL) + return; + sendme_data_v1_clear(obj); + trunnel_memwipe(obj, sizeof(sendme_data_v1_t)); + trunnel_free_(obj); +} + +size_t +sendme_data_v1_getlen_digest(const sendme_data_v1_t *inp) +{ + (void)inp; return 4; +} + +uint8_t +sendme_data_v1_get_digest(sendme_data_v1_t *inp, size_t idx) +{ + trunnel_assert(idx < 4); + return inp->digest[idx]; +} + +uint8_t +sendme_data_v1_getconst_digest(const sendme_data_v1_t *inp, size_t idx) +{ + return sendme_data_v1_get_digest((sendme_data_v1_t*)inp, idx); +} +int +sendme_data_v1_set_digest(sendme_data_v1_t *inp, size_t idx, uint8_t elt) +{ + trunnel_assert(idx < 4); + inp->digest[idx] = elt; + return 0; +} + +uint8_t * +sendme_data_v1_getarray_digest(sendme_data_v1_t *inp) +{ + return inp->digest; +} +const uint8_t * +sendme_data_v1_getconstarray_digest(const sendme_data_v1_t *inp) +{ + return (const uint8_t *)sendme_data_v1_getarray_digest((sendme_data_v1_t*)inp); +} +const char * +sendme_data_v1_check(const sendme_data_v1_t *obj) +{ + if (obj == NULL) + return "Object was NULL"; + if (obj->trunnel_error_code_) + return "A set function failed on this object"; + return NULL; +} + +ssize_t +sendme_data_v1_encoded_len(const sendme_data_v1_t *obj) +{ + ssize_t result = 0; + + if (NULL != sendme_data_v1_check(obj)) + return -1; + + + /* Length of u8 digest[4] */ + result += 4; + return result; +} +int +sendme_data_v1_clear_errors(sendme_data_v1_t *obj) +{ + int r = obj->trunnel_error_code_; + obj->trunnel_error_code_ = 0; + return r; +} +ssize_t +sendme_data_v1_encode(uint8_t *output, const size_t avail, const sendme_data_v1_t *obj) +{ + ssize_t result = 0; + size_t written = 0; + uint8_t *ptr = output; + const char *msg; +#ifdef TRUNNEL_CHECK_ENCODED_LEN + const ssize_t encoded_len = sendme_data_v1_encoded_len(obj); +#endif + + if (NULL != (msg = sendme_data_v1_check(obj))) + goto check_failed; + +#ifdef TRUNNEL_CHECK_ENCODED_LEN + trunnel_assert(encoded_len >= 0); +#endif + + /* Encode u8 digest[4] */ + trunnel_assert(written <= avail); + if (avail - written < 4) + goto truncated; + memcpy(ptr, obj->digest, 4); + written += 4; ptr += 4; + + + trunnel_assert(ptr == output + written); +#ifdef TRUNNEL_CHECK_ENCODED_LEN + { + trunnel_assert(encoded_len >= 0); + trunnel_assert((size_t)encoded_len == written); + } + +#endif + + return written; + + truncated: + result = -2; + goto fail; + check_failed: + (void)msg; + result = -1; + goto fail; + fail: + trunnel_assert(result < 0); + return result; +} + +/** As sendme_data_v1_parse(), but do not allocate the output object. + */ +static ssize_t +sendme_data_v1_parse_into(sendme_data_v1_t *obj, const uint8_t *input, const size_t len_in) +{ + const uint8_t *ptr = input; + size_t remaining = len_in; + ssize_t result = 0; + (void)result; + + /* Parse u8 digest[4] */ + CHECK_REMAINING(4, truncated); + memcpy(obj->digest, ptr, 4); + remaining -= 4; ptr += 4; + trunnel_assert(ptr + remaining == input + len_in); + return len_in - remaining; + + truncated: + return -2; +} + +ssize_t +sendme_data_v1_parse(sendme_data_v1_t **output, const uint8_t *input, const size_t len_in) +{ + ssize_t result; + *output = sendme_data_v1_new(); + if (NULL == *output) + return -1; + result = sendme_data_v1_parse_into(*output, input, len_in); + if (result < 0) { + sendme_data_v1_free(*output); + *output = NULL; + } + return result; +} diff --git a/src/trunnel/sendme.h b/src/trunnel/sendme.h new file mode 100644 index 000000000..8207cb56f --- /dev/null +++ b/src/trunnel/sendme.h @@ -0,0 +1,170 @@ +/* sendme.h -- generated by Trunnel v1.5.2. + * https://gitweb.torproject.org/trunnel.git + * You probably shouldn't edit this file. + */ +#ifndef TRUNNEL_SENDME_H +#define TRUNNEL_SENDME_H + +#include <stdint.h> +#include "trunnel.h" + +#if !defined(TRUNNEL_OPAQUE) && !defined(TRUNNEL_OPAQUE_SENDME_CELL) +struct sendme_cell_st { + uint8_t version; + uint16_t data_len; + TRUNNEL_DYNARRAY_HEAD(, uint8_t) data; + uint8_t trunnel_error_code_; +}; +#endif +typedef struct sendme_cell_st sendme_cell_t; +#if !defined(TRUNNEL_OPAQUE) && !defined(TRUNNEL_OPAQUE_SENDME_DATA_V1) +struct sendme_data_v1_st { + uint8_t digest[4]; + uint8_t trunnel_error_code_; +}; +#endif +typedef struct sendme_data_v1_st sendme_data_v1_t; +/** Return a newly allocated sendme_cell with all elements set to + * zero. + */ +sendme_cell_t *sendme_cell_new(void); +/** Release all storage held by the sendme_cell in 'victim'. (Do + * nothing if 'victim' is NULL.) + */ +void sendme_cell_free(sendme_cell_t *victim); +/** Try to parse a sendme_cell from the buffer in 'input', using up to + * 'len_in' bytes from the input buffer. On success, return the number + * of bytes consumed and set *output to the newly allocated + * sendme_cell_t. On failure, return -2 if the input appears + * truncated, and -1 if the input is otherwise invalid. + */ +ssize_t sendme_cell_parse(sendme_cell_t **output, const uint8_t *input, const size_t len_in); +/** Return the number of bytes we expect to need to encode the + * sendme_cell in 'obj'. On failure, return a negative value. Note + * that this value may be an overestimate, and can even be an + * underestimate for certain unencodeable objects. + */ +ssize_t sendme_cell_encoded_len(const sendme_cell_t *obj); +/** Try to encode the sendme_cell from 'input' into the buffer at + * 'output', using up to 'avail' bytes of the output buffer. On + * success, return the number of bytes used. On failure, return -2 if + * the buffer was not long enough, and -1 if the input was invalid. + */ +ssize_t sendme_cell_encode(uint8_t *output, size_t avail, const sendme_cell_t *input); +/** Check whether the internal state of the sendme_cell in 'obj' is + * consistent. Return NULL if it is, and a short message if it is not. + */ +const char *sendme_cell_check(const sendme_cell_t *obj); +/** Clear any errors that were set on the object 'obj' by its setter + * functions. Return true iff errors were cleared. + */ +int sendme_cell_clear_errors(sendme_cell_t *obj); +/** Return the value of the version field of the sendme_cell_t in + * 'inp' + */ +uint8_t sendme_cell_get_version(const sendme_cell_t *inp); +/** Set the value of the version field of the sendme_cell_t in 'inp' + * to 'val'. Return 0 on success; return -1 and set the error code on + * 'inp' on failure. + */ +int sendme_cell_set_version(sendme_cell_t *inp, uint8_t val); +/** Return the value of the data_len field of the sendme_cell_t in + * 'inp' + */ +uint16_t sendme_cell_get_data_len(const sendme_cell_t *inp); +/** Set the value of the data_len field of the sendme_cell_t in 'inp' + * to 'val'. Return 0 on success; return -1 and set the error code on + * 'inp' on failure. + */ +int sendme_cell_set_data_len(sendme_cell_t *inp, uint16_t val); +/** Return the length of the dynamic array holding the data field of + * the sendme_cell_t in 'inp'. + */ +size_t sendme_cell_getlen_data(const sendme_cell_t *inp); +/** Return the element at position 'idx' of the dynamic array field + * data of the sendme_cell_t in 'inp'. + */ +uint8_t sendme_cell_get_data(sendme_cell_t *inp, size_t idx); +/** As sendme_cell_get_data, but take and return a const pointer + */ +uint8_t sendme_cell_getconst_data(const sendme_cell_t *inp, size_t idx); +/** Change the element at position 'idx' of the dynamic array field + * data of the sendme_cell_t in 'inp', so that it will hold the value + * 'elt'. + */ +int sendme_cell_set_data(sendme_cell_t *inp, size_t idx, uint8_t elt); +/** Append a new element 'elt' to the dynamic array field data of the + * sendme_cell_t in 'inp'. + */ +int sendme_cell_add_data(sendme_cell_t *inp, uint8_t elt); +/** Return a pointer to the variable-length array field data of 'inp'. + */ +uint8_t * sendme_cell_getarray_data(sendme_cell_t *inp); +/** As sendme_cell_get_data, but take and return a const pointer + */ +const uint8_t * sendme_cell_getconstarray_data(const sendme_cell_t *inp); +/** Change the length of the variable-length array field data of 'inp' + * to 'newlen'.Fill extra elements with 0. Return 0 on success; return + * -1 and set the error code on 'inp' on failure. + */ +int sendme_cell_setlen_data(sendme_cell_t *inp, size_t newlen); +/** Return a newly allocated sendme_data_v1 with all elements set to + * zero. + */ +sendme_data_v1_t *sendme_data_v1_new(void); +/** Release all storage held by the sendme_data_v1 in 'victim'. (Do + * nothing if 'victim' is NULL.) + */ +void sendme_data_v1_free(sendme_data_v1_t *victim); +/** Try to parse a sendme_data_v1 from the buffer in 'input', using up + * to 'len_in' bytes from the input buffer. On success, return the + * number of bytes consumed and set *output to the newly allocated + * sendme_data_v1_t. On failure, return -2 if the input appears + * truncated, and -1 if the input is otherwise invalid. + */ +ssize_t sendme_data_v1_parse(sendme_data_v1_t **output, const uint8_t *input, const size_t len_in); +/** Return the number of bytes we expect to need to encode the + * sendme_data_v1 in 'obj'. On failure, return a negative value. Note + * that this value may be an overestimate, and can even be an + * underestimate for certain unencodeable objects. + */ +ssize_t sendme_data_v1_encoded_len(const sendme_data_v1_t *obj); +/** Try to encode the sendme_data_v1 from 'input' into the buffer at + * 'output', using up to 'avail' bytes of the output buffer. On + * success, return the number of bytes used. On failure, return -2 if + * the buffer was not long enough, and -1 if the input was invalid. + */ +ssize_t sendme_data_v1_encode(uint8_t *output, size_t avail, const sendme_data_v1_t *input); +/** Check whether the internal state of the sendme_data_v1 in 'obj' is + * consistent. Return NULL if it is, and a short message if it is not. + */ +const char *sendme_data_v1_check(const sendme_data_v1_t *obj); +/** Clear any errors that were set on the object 'obj' by its setter + * functions. Return true iff errors were cleared. + */ +int sendme_data_v1_clear_errors(sendme_data_v1_t *obj); +/** Return the (constant) length of the array holding the digest field + * of the sendme_data_v1_t in 'inp'. + */ +size_t sendme_data_v1_getlen_digest(const sendme_data_v1_t *inp); +/** Return the element at position 'idx' of the fixed array field + * digest of the sendme_data_v1_t in 'inp'. + */ +uint8_t sendme_data_v1_get_digest(sendme_data_v1_t *inp, size_t idx); +/** As sendme_data_v1_get_digest, but take and return a const pointer + */ +uint8_t sendme_data_v1_getconst_digest(const sendme_data_v1_t *inp, size_t idx); +/** Change the element at position 'idx' of the fixed array field + * digest of the sendme_data_v1_t in 'inp', so that it will hold the + * value 'elt'. + */ +int sendme_data_v1_set_digest(sendme_data_v1_t *inp, size_t idx, uint8_t elt); +/** Return a pointer to the 4-element array field digest of 'inp'. + */ +uint8_t * sendme_data_v1_getarray_digest(sendme_data_v1_t *inp); +/** As sendme_data_v1_get_digest, but take and return a const pointer + */ +const uint8_t * sendme_data_v1_getconstarray_digest(const sendme_data_v1_t *inp); + + +#endif diff --git a/src/trunnel/sendme.trunnel b/src/trunnel/sendme.trunnel new file mode 100644 index 000000000..7294a09b4 --- /dev/null +++ b/src/trunnel/sendme.trunnel @@ -0,0 +1,18 @@ +/* This file contains the SENDME cell definition. */ + +struct sendme_cell { + /* Version field. */ + u8 version IN [0x00, 0x01]; + + /* The data content depends on the version. */ + u16 data_len; + u8 data[data_len]; +} + +/* SENDME version 0. No data. */ + +/* SENDME version 1. Authenticated with digest. */ +struct sendme_data_v1 { + /* A 4 bytes digest. */ + u8 digest[4]; +}

1 0

[tor/master] tests: Implement unit tests for SENDME v1
by asn＠torproject.org 02 May '19

02 May '19

commit cede93b2d83fb810ec8b2152882732ed0a7481dc Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Feb 19 14:49:38 2019 -0500 tests: Implement unit tests for SENDME v1 Part of #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/sendme.c | 12 +-- src/core/or/sendme.h | 23 +++++ src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_relaycell.c | 1 + src/test/test_sendme.c | 223 ++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 257 insertions(+), 5 deletions(-) diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index 76f551a92..980684c82 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -7,6 +7,8 @@ * creating/parsing cells and handling the content. */ +#define SENDME_PRIVATE + #include "core/or/or.h" #include "app/config/config.h" @@ -37,7 +39,7 @@ /* Return the minimum version given by the consensus (if any) that should be * used when emitting a SENDME cell. */ -static int +STATIC int get_emit_min_version(void) { return networkstatus_get_param(NULL, "sendme_emit_min_version", @@ -48,7 +50,7 @@ get_emit_min_version(void) /* Return the minimum version given by the consensus (if any) that should be * accepted when receiving a SENDME cell. */ -static int +STATIC int get_accept_min_version(void) { return networkstatus_get_param(NULL, "sendme_accept_min_version", @@ -112,7 +114,7 @@ cell_v1_is_valid(const sendme_cell_t *cell, const circuit_t *circ) /* Return true iff the given cell version can be handled or if the minimum * accepted version from the consensus is known to us. */ -static bool +STATIC bool cell_version_is_valid(uint8_t cell_version) { int accept_version = get_accept_min_version(); @@ -149,7 +151,7 @@ cell_version_is_valid(uint8_t cell_version) * This is the main critical function to make sure we can continue to * send/recv cells on a circuit. If the SENDME is invalid, the circuit should * be mark for close. */ -static bool +STATIC bool sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, size_t cell_payload_len) { @@ -206,7 +208,7 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, * * Return the size in bytes of the encoded cell in payload. A negative value * is returned on encoding failure. */ -static ssize_t +STATIC ssize_t build_cell_payload_v1(crypto_digest_t *cell_digest, uint8_t *payload) { ssize_t len = -1; diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index e7cf718bb..c2e2518da 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -36,4 +36,27 @@ int sendme_stream_data_packaged(edge_connection_t *conn); /* Track cell digest. */ void sendme_note_cell_digest(circuit_t *circ); +/* Private section starts. */ +#ifdef SENDME_PRIVATE + +/* + * Unit tests declaractions. + */ +#ifdef TOR_UNIT_TESTS + +STATIC int get_emit_min_version(void); +STATIC int get_accept_min_version(void); + +STATIC bool cell_version_is_valid(uint8_t cell_version); + +STATIC ssize_t build_cell_payload_v1(crypto_digest_t *cell_digest, + uint8_t *payload); +STATIC bool sendme_is_valid(const circuit_t *circ, + const uint8_t *cell_payload, + size_t cell_payload_len); + +#endif /* TOR_UNIT_TESTS */ + +#endif /* SENDME_PRIVATE */ + #endif /* !defined(TOR_SENDME_H) */ diff --git a/src/test/include.am b/src/test/include.am index 497aa320a..5f6b05fa9 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -179,6 +179,7 @@ src_test_test_SOURCES += \ src/test/test_routerlist.c \ src/test/test_routerset.c \ src/test/test_scheduler.c \ + src/test/test_sendme.c \ src/test/test_shared_random.c \ src/test/test_socks.c \ src/test/test_status.c \ diff --git a/src/test/test.c b/src/test/test.c index fbc30fb64..17159b71c 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -923,6 +923,7 @@ struct testgroup_t testgroups[] = { { "routerlist/", routerlist_tests }, { "routerset/" , routerset_tests }, { "scheduler/", scheduler_tests }, + { "sendme/", sendme_tests }, { "shared-random/", sr_tests }, { "socks/", socks_tests }, { "status/" , status_tests }, diff --git a/src/test/test.h b/src/test/test.h index 7d19af9b2..167fd090a 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -266,6 +266,7 @@ extern struct testcase_t routerkeys_tests[]; extern struct testcase_t routerlist_tests[]; extern struct testcase_t routerset_tests[]; extern struct testcase_t scheduler_tests[]; +extern struct testcase_t sendme_tests[]; extern struct testcase_t socks_tests[]; extern struct testcase_t sr_tests[]; extern struct testcase_t status_tests[]; diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c index 062358351..c4ed215c7 100644 --- a/src/test/test_relaycell.c +++ b/src/test/test_relaycell.c @@ -705,6 +705,7 @@ test_circbw_relay(void *arg) circ = helper_create_origin_circuit(CIRCUIT_PURPOSE_C_GENERAL, 0); circ->cpath->state = CPATH_STATE_AWAITING_KEYS; circ->cpath->deliver_window = CIRCWINDOW_START; + circ->cpath->crypto.sendme_digest = crypto_digest_new(); entryconn1 = fake_entry_conn(circ, 1); edgeconn = ENTRY_TO_EDGE_CONN(entryconn1); diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c new file mode 100644 index 000000000..ad6aac6c0 --- /dev/null +++ b/src/test/test_sendme.c @@ -0,0 +1,223 @@ +/* Copyright (c) 2014-2019, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/* Unit tests for handling different kinds of relay cell */ + +#define CIRCUITLIST_PRIVATE +#define NETWORKSTATUS_PRIVATE +#define SENDME_PRIVATE + +#include "core/or/circuit_st.h" +#include "core/or/or_circuit_st.h" +#include "core/or/origin_circuit_st.h" +#include "core/or/circuitlist.h" +#include "core/or/sendme.h" + +#include "feature/nodelist/networkstatus.h" +#include "feature/nodelist/networkstatus_st.h" + +#include "test/test.h" +#include "test/log_test_helpers.h" + +static void +setup_mock_consensus(void) +{ + current_md_consensus = current_ns_consensus = + tor_malloc_zero(sizeof(networkstatus_t)); + current_md_consensus->net_params = smartlist_new(); + current_md_consensus->routerstatus_list = smartlist_new(); +} + +static void +free_mock_consensus(void) +{ + SMARTLIST_FOREACH(current_md_consensus->routerstatus_list, void *, r, + tor_free(r)); + smartlist_free(current_md_consensus->routerstatus_list); + smartlist_free(current_ns_consensus->net_params); + tor_free(current_ns_consensus); +} + +static void +test_v1_note_digest(void *arg) +{ + or_circuit_t *or_circ = NULL; + origin_circuit_t *orig_circ = NULL; + circuit_t *circ = NULL; + + (void) arg; + + /* Create our dummy circuits. */ + orig_circ = origin_circuit_new(); + tt_assert(orig_circ); + or_circ = or_circuit_new(1, NULL); + + /* Start by pointing to the origin circuit. */ + circ = TO_CIRCUIT(orig_circ); + circ->purpose = CIRCUIT_PURPOSE_S_REND_JOINED; + + /* We should never note SENDME digest on origin circuit. */ + sendme_note_cell_digest(circ); + tt_assert(!circ->sendme_last_digests); + /* We do not need the origin circuit for now. */ + orig_circ = NULL; + circuit_free_(circ); + /* Points it to the OR circuit now. */ + circ = TO_CIRCUIT(or_circ); + or_circ->crypto.sendme_digest = crypto_digest_new(); + + /* The package window has to be a multiple of CIRCWINDOW_INCREMENT minus 1 + * in order to catched the CIRCWINDOW_INCREMENT-nth cell. Try something that + * shouldn't be noted. */ + circ->package_window = CIRCWINDOW_INCREMENT; + sendme_note_cell_digest(circ); + tt_assert(!circ->sendme_last_digests); + + /* This should work now. Package window at CIRCWINDOW_INCREMENT + 1. */ + circ->package_window++; + sendme_note_cell_digest(circ); + tt_assert(circ->sendme_last_digests); + tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1); + + /* Next cell in the package window shouldn't do anything. */ + circ->package_window++; + sendme_note_cell_digest(circ); + tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1); + + /* The next CIRCWINDOW_INCREMENT should add one more digest. */ + circ->package_window = (CIRCWINDOW_INCREMENT * 2) + 1; + sendme_note_cell_digest(circ); + tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 2); + + done: + circuit_free_(circ); +} + +static void +test_v1_consensus_params(void *arg) +{ + (void) arg; + + setup_mock_consensus(); + tt_assert(current_md_consensus); + + /* Both zeroes. */ + smartlist_add(current_md_consensus->net_params, + (void *) "sendme_emit_min_version=0"); + smartlist_add(current_md_consensus->net_params, + (void *) "sendme_accept_min_version=0"); + tt_int_op(get_emit_min_version(), OP_EQ, 0); + tt_int_op(get_accept_min_version(), OP_EQ, 0); + smartlist_clear(current_md_consensus->net_params); + + /* Both ones. */ + smartlist_add(current_md_consensus->net_params, + (void *) "sendme_emit_min_version=1"); + smartlist_add(current_md_consensus->net_params, + (void *) "sendme_accept_min_version=1"); + tt_int_op(get_emit_min_version(), OP_EQ, 1); + tt_int_op(get_accept_min_version(), OP_EQ, 1); + smartlist_clear(current_md_consensus->net_params); + + /* Different values from each other. */ + smartlist_add(current_md_consensus->net_params, + (void *) "sendme_emit_min_version=1"); + smartlist_add(current_md_consensus->net_params, + (void *) "sendme_accept_min_version=0"); + tt_int_op(get_emit_min_version(), OP_EQ, 1); + tt_int_op(get_accept_min_version(), OP_EQ, 0); + smartlist_clear(current_md_consensus->net_params); + + /* Validate is the cell version is coherent with our internal default value + * and the one in the consensus. */ + smartlist_add(current_md_consensus->net_params, + (void *) "sendme_accept_min_version=1"); + /* Minimum acceptable value is 1. */ + tt_int_op(cell_version_is_valid(1), OP_EQ, true); + /* Minimum acceptable value is 1 so a cell version of 0 is refused. */ + tt_int_op(cell_version_is_valid(0), OP_EQ, false); + + done: + free_mock_consensus(); +} + +static void +test_v1_build_cell(void *arg) +{ + uint8_t payload[RELAY_PAYLOAD_SIZE]; + ssize_t ret; + crypto_digest_t *cell_digest = NULL; + or_circuit_t *or_circ = NULL; + circuit_t *circ = NULL; + + (void) arg; + + or_circ = or_circuit_new(1, NULL); + circ = TO_CIRCUIT(or_circ); + + cell_digest = crypto_digest_new(); + crypto_digest_add_bytes(cell_digest, "AAAA", 4); + tt_assert(cell_digest); + + /* SENDME v1 payload is 7 bytes. See spec. */ + ret = build_cell_payload_v1(cell_digest, payload); + tt_int_op(ret, OP_EQ, 7); + + /* Validation. */ + + /* An empty payload means SENDME version 0 thus valid. */ + tt_int_op(sendme_is_valid(circ, payload, 0), OP_EQ, true); + + /* An unparseable cell means invalid. */ + setup_full_capture_of_logs(LOG_INFO); + tt_int_op(sendme_is_valid(circ, (const uint8_t *) "A", 1), OP_EQ, false); + expect_log_msg_containing("Unparseable SENDME cell received. " + "Closing circuit."); + teardown_capture_of_logs(); + + /* No cell digest recorded for this. */ + setup_full_capture_of_logs(LOG_INFO); + tt_int_op(sendme_is_valid(circ, payload, sizeof(payload)), OP_EQ, false); + expect_log_msg_containing("We received a SENDME but we have no cell digests " + "to match. Closing circuit."); + teardown_capture_of_logs(); + + /* Note the wrong digest in the circuit, cell should fail validation. */ + or_circ->crypto.sendme_digest = crypto_digest_new(); + circ->package_window = CIRCWINDOW_INCREMENT + 1; + sendme_note_cell_digest(circ); + tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1); + setup_full_capture_of_logs(LOG_INFO); + tt_int_op(sendme_is_valid(circ, payload, sizeof(payload)), OP_EQ, false); + /* After a validation, the last digests is always popped out. */ + tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 0); + expect_log_msg_containing("SENDME v1 cell digest do not match."); + teardown_capture_of_logs(); + + /* Cleanup */ + crypto_digest_free(or_circ->crypto.sendme_digest); + + /* Record the cell digest into the circuit, cell should validate. */ + or_circ->crypto.sendme_digest = crypto_digest_dup(cell_digest); + circ->package_window = CIRCWINDOW_INCREMENT + 1; + sendme_note_cell_digest(circ); + tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1); + tt_int_op(sendme_is_valid(circ, payload, sizeof(payload)), OP_EQ, true); + /* After a validation, the last digests is always popped out. */ + tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 0); + + done: + crypto_digest_free(cell_digest); + circuit_free_(circ); +} + +struct testcase_t sendme_tests[] = { + { "v1_note_digest", test_v1_note_digest, TT_FORK, + NULL, NULL }, + { "v1_consensus_params", test_v1_consensus_params, TT_FORK, + NULL, NULL }, + { "v1_build_cell", test_v1_build_cell, TT_FORK, + NULL, NULL }, + + END_OF_TESTCASES +};

1 0

[tor/master] prop289: Match the SENDME digest
by asn＠torproject.org 02 May '19

02 May '19

commit bb473a807ae94a1e6c45a069db6ddf213413940a Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Jan 9 15:27:51 2019 -0500 prop289: Match the SENDME digest Now that we keep the last seen cell digests on the Exit side on the circuit object, use that to match the SENDME v1 transforming this whole process into a real authenticated SENDME mechanism. Part of #26841 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/sendme.c | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index 69bcac468..afade43f7 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -17,6 +17,7 @@ #include "core/or/relay.h" #include "core/or/sendme.h" #include "feature/nodelist/networkstatus.h" +#include "lib/ctime/di_ops.h" #include "trunnel/sendme.h" /* The maximum supported version. Above that value, the cell can't be @@ -61,7 +62,7 @@ get_accept_min_version(void) * cell we saw which tells us that the other side has in fact seen that cell. * See proposal 289 for more details. */ static bool -cell_v1_is_valid(const sendme_cell_t *cell) +cell_v1_is_valid(const sendme_cell_t *cell, const circuit_t *circ) { sendme_data_v1_t *data = NULL; @@ -72,9 +73,33 @@ cell_v1_is_valid(const sendme_cell_t *cell) goto invalid; } - /* XXX: Match the digest in the cell to the previous cell. Needs to be - * implemented that is passed to this function and compared. For this, we - * need #26839 that is making tor remember the last digest(s). */ + /* We shouldn't have received this SENDME if we have no digests. Log at + * protocol warning because it can be tricked by sending many SENDMEs + * without prior data cell. */ + if (circ->sendme_last_digests == NULL || + smartlist_len(circ->sendme_last_digests) == 0) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "We received a SENDME but we have no cell digests to match. " + "Closing circuit."); + goto invalid; + } + + /* Pop the first element that was added (FIFO) and compare it. */ + { + uint8_t *digest = smartlist_get(circ->sendme_last_digests, 0); + smartlist_del_keeporder(circ->sendme_last_digests, 0); + + /* Compare the digest with the one in the SENDME. This cell is invalid + * without a perfect match. */ + if (tor_memcmp(digest, sendme_data_v1_getconstarray_digest(data), + sendme_data_v1_getlen_digest(data))) { + tor_free(digest); + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "SENDME v1 cell digest do not match."); + goto invalid; + } + tor_free(digest); + } /* Validated SENDME v1 cell. */ sendme_data_v1_free(data); @@ -124,11 +149,13 @@ cell_version_is_valid(uint8_t cell_version) * send/recv cells on a circuit. If the SENDME is invalid, the circuit should * be mark for close. */ static bool -sendme_is_valid(const uint8_t *cell_payload, size_t cell_payload_len) +sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, + size_t cell_payload_len) { uint8_t cell_version; sendme_cell_t *cell = NULL; + tor_assert(circ); tor_assert(cell_payload); /* An empty payload means version 0 so skip trunnel parsing. We won't be @@ -153,7 +180,7 @@ sendme_is_valid(const uint8_t *cell_payload, size_t cell_payload_len) /* Validate depending on the version now. */ switch (cell_version) { case 0x01: - if (!cell_v1_is_valid(cell)) { + if (!cell_v1_is_valid(cell, circ)) { goto invalid; } break; @@ -374,7 +401,7 @@ sendme_process_circuit_level(crypt_path_t *layer_hint, /* Validate the SENDME cell. Depending on the version, different * validation can be done. An invalid SENDME requires us to close the * circuit. It is only done if we are the Exit of the circuit. */ - if (!sendme_is_valid(cell_payload, cell_payload_len)) { + if (!sendme_is_valid(circ, cell_payload, cell_payload_len)) { return -END_CIRC_REASON_TORPROTOCOL; }

1 0

[tor/master] prop289: Remember the last cell digest for v1 SENDMEs
by asn＠torproject.org 02 May '19

02 May '19

commit 402f0a4f5d70bee128130f4dbd0ea18de1747410 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Jan 23 14:39:04 2019 -0500 prop289: Remember the last cell digest for v1 SENDMEs In order to do so, depending on where the cell is going, we'll keep the last cell digest that is either received inbound or sent outbound. Then it can be used for validation. Part of #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/crypto/relay_crypto.c | 13 +++++++++++++ src/core/or/relay.c | 2 +- src/core/or/relay_crypto_st.h | 2 ++ src/core/or/sendme.c | 28 +++++++++++++++++++--------- src/core/or/sendme.h | 3 +-- 5 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/core/crypto/relay_crypto.c b/src/core/crypto/relay_crypto.c index 0b83b2d0a..d4116d47a 100644 --- a/src/core/crypto/relay_crypto.c +++ b/src/core/crypto/relay_crypto.c @@ -142,6 +142,13 @@ relay_decrypt_cell(circuit_t *circ, cell_t *cell, if (relay_digest_matches(thishop->crypto.b_digest, cell)) { *recognized = 1; *layer_hint = thishop; + /* Keep current digest of this cell for the possible SENDME. */ + if (thishop->crypto.sendme_digest) { + crypto_digest_free(thishop->crypto.sendme_digest); + } + thishop->crypto.sendme_digest = + crypto_digest_dup(thishop->crypto.b_digest); + return 0; } } @@ -212,6 +219,11 @@ relay_encrypt_cell_inbound(cell_t *cell, or_circuit_t *or_circ) { relay_set_digest(or_circ->crypto.b_digest, cell); + /* Keep a record of this cell, we might use it for validating the SENDME. */ + if (or_circ->crypto.sendme_digest) { + crypto_digest_free(or_circ->crypto.sendme_digest); + } + or_circ->crypto.sendme_digest = crypto_digest_dup(or_circ->crypto.b_digest); /* encrypt one layer */ relay_crypt_one_payload(or_circ->crypto.b_crypto, cell->payload); } @@ -229,6 +241,7 @@ relay_crypto_clear(relay_crypto_t *crypto) crypto_cipher_free(crypto->b_crypto); crypto_digest_free(crypto->f_digest); crypto_digest_free(crypto->b_digest); + crypto_digest_free(crypto->sendme_digest); } /** Initialize crypto from the key material in key_data. diff --git a/src/core/or/relay.c b/src/core/or/relay.c index b26360b24..47275a811 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -1568,7 +1568,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, } /* Consider sending a circuit-level SENDME cell. */ - sendme_circuit_consider_sending(circ, layer_hint, NULL); + sendme_circuit_consider_sending(circ, layer_hint); if (rh.stream_id == 0) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Relay data cell with zero " diff --git a/src/core/or/relay_crypto_st.h b/src/core/or/relay_crypto_st.h index dafce257c..dbdf1599d 100644 --- a/src/core/or/relay_crypto_st.h +++ b/src/core/or/relay_crypto_st.h @@ -25,6 +25,8 @@ struct relay_crypto_t { /** Digest state for cells heading away from the OR at this step. */ struct crypto_digest_t *b_digest; + /** Digest used for the next SENDME cell if any. */ + struct crypto_digest_t *sendme_digest; }; #undef crypto_cipher_t diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index afade43f7..76f551a92 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -14,6 +14,7 @@ #include "core/or/cell_st.h" #include "core/or/circuitlist.h" #include "core/or/circuituse.h" +#include "core/or/or_circuit_st.h" #include "core/or/relay.h" #include "core/or/sendme.h" #include "feature/nodelist/networkstatus.h" @@ -342,18 +343,20 @@ sendme_connection_edge_consider_sending(edge_connection_t *conn) * more. */ void -sendme_circuit_consider_sending(circuit_t *circ, crypt_path_t *layer_hint, - crypto_digest_t *digest) +sendme_circuit_consider_sending(circuit_t *circ, crypt_path_t *layer_hint) { - tor_assert(digest); + crypto_digest_t *digest; while ((layer_hint ? layer_hint->deliver_window : circ->deliver_window) <= CIRCWINDOW_START - CIRCWINDOW_INCREMENT) { log_debug(LD_CIRC,"Queuing circuit sendme."); - if (layer_hint) + if (layer_hint) { layer_hint->deliver_window += CIRCWINDOW_INCREMENT; - else + digest = layer_hint->crypto.sendme_digest; + } else { circ->deliver_window += CIRCWINDOW_INCREMENT; + digest = TO_OR_CIRCUIT(circ)->crypto.sendme_digest; + } if (send_circuit_level_sendme(circ, layer_hint, digest) < 0) { return; /* The circuit's closed, don't continue */ } @@ -557,9 +560,16 @@ sendme_note_cell_digest(circuit_t *circ) return; } - digest = tor_malloc_zero(4); - if (circ->sendme_last_digests == NULL) { - circ->sendme_last_digests = smartlist_new(); + /* Only note the digest if we actually have the digest of the previous cell + * recorded. It should never happen in theory as we always record the last + * digest for the v1 SENDME. */ + if (TO_OR_CIRCUIT(circ)->crypto.sendme_digest) { + digest = tor_malloc_zero(4); + crypto_digest_get_digest(TO_OR_CIRCUIT(circ)->crypto.sendme_digest, + (char *) digest, 4); + if (circ->sendme_last_digests == NULL) { + circ->sendme_last_digests = smartlist_new(); + } + smartlist_add(circ->sendme_last_digests, digest); } - smartlist_add(circ->sendme_last_digests, digest); } diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index 300fb25d9..e7cf718bb 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -16,8 +16,7 @@ /* Sending SENDME cell. */ void sendme_connection_edge_consider_sending(edge_connection_t *edge_conn); void sendme_circuit_consider_sending(circuit_t *circ, - crypt_path_t *layer_hint, - crypto_digest_t *digest); + crypt_path_t *layer_hint); /* Processing SENDME cell. */ int sendme_process_circuit_level(crypt_path_t *layer_hint,

1 0

[tor/master] prop289: Support SENDME v1 cell parsing
by asn＠torproject.org 02 May '19

02 May '19

commit 81706d84279f0a2870f8b1789403188fd933b32a Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Jan 9 14:03:32 2019 -0500 prop289: Support SENDME v1 cell parsing This commit makes tor able to parse and handle a SENDME version 1. It will look at the consensus parameter "sendme_accept_min_version" to know what is the minimum version it should look at. IMPORTANT: At this commit, the validation of the cell is not fully implemented. For this, we need #26839 to be completed that is to match the SENDME digest with the last cell digest. Closes #26841 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/relay.c | 4 +- src/core/or/sendme.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++-- src/core/or/sendme.h | 3 +- 3 files changed, 140 insertions(+), 7 deletions(-) diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 6f69ed999..76f2203a9 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -1814,7 +1814,9 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, if (!rh.stream_id) { /* Circuit level SENDME cell. */ - ret = sendme_process_circuit_level(layer_hint, circ, rh.length); + ret = sendme_process_circuit_level(layer_hint, circ, + cell->payload + RELAY_HEADER_SIZE, + rh.length); if (ret < 0) { return ret; } diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index f22e7027d..64497055e 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -18,6 +18,10 @@ #include "feature/nodelist/networkstatus.h" #include "trunnel/sendme.h" +/* The maximum supported version. Above that value, the cell can't be + * recognized as a valid SENDME. */ +#define SENDME_MAX_SUPPORTED_VERSION 1 + /* The cell version constants for when emitting a cell. */ #define SENDME_EMIT_MIN_VERSION_DEFAULT 0 #define SENDME_EMIT_MIN_VERSION_MIN 0 @@ -39,7 +43,6 @@ get_emit_min_version(void) SENDME_EMIT_MIN_VERSION_MAX); } -#if 0 /* Return the minimum version given by the consensus (if any) that should be * accepted when receiving a SENDME cell. */ static int @@ -50,7 +53,124 @@ get_accept_min_version(void) SENDME_ACCEPT_MIN_VERSION_MIN, SENDME_ACCEPT_MIN_VERSION_MAX); } -#endif + +/* Return true iff the given decoded SENDME version 1 cell is valid. + * + * Validation is done by comparing the digest in the cell from the previous + * cell we saw which tells us that the other side has in fact seen that cell. + * See proposal 289 for more details. */ +static bool +cell_v1_is_valid(const sendme_cell_t *cell) +{ + sendme_data_v1_t *data = NULL; + + tor_assert(cell); + + if (sendme_data_v1_parse(&data, sendme_cell_getconstarray_data(cell), + sendme_cell_getlen_data(cell)) < 0) { + goto invalid; + } + + /* XXX: Match the digest in the cell to the previous cell. Needs to be + * implemented that is passed to this function and compared. For this, we + * need #26839 that is making tor remember the last digest(s). */ + + /* Validated SENDME v1 cell. */ + sendme_data_v1_free(data); + return 1; + invalid: + sendme_data_v1_free(data); + return 0; +} + +/* Return true iff the given cell version can be handled or if the minimum + * accepted version from the consensus is known to us. */ +static bool +cell_version_is_valid(uint8_t cell_version) +{ + int accept_version = get_accept_min_version(); + + /* Can we handle this version? */ + if (accept_version > SENDME_MAX_SUPPORTED_VERSION) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Unable to handle SENDME version %u. We only support <= %d " + "(from consensus). Probably your tor is too old?", + accept_version, cell_version); + goto invalid; + } + + /* We only accept a SENDME cell from what the consensus tells us. */ + if (cell_version < accept_version) { + log_info(LD_PROTOCOL, "Unacceptable SENDME version %d. Only " + "accepting %u (taken from the consensus). " + "Closing circuit.", + cell_version, accept_version); + goto invalid; + } + + return 1; + invalid: + return 0; +} + +/* Return true iff the encoded SENDME cell in cell_payload of length + * cell_payload_len is valid. For each version: + * + * 0: No validation + * 1: Authenticated with last cell digest. + * + * This is the main critical function to make sure we can continue to + * send/recv cells on a circuit. If the SENDME is invalid, the circuit should + * be mark for close. */ +static bool +sendme_is_valid(const uint8_t *cell_payload, size_t cell_payload_len) +{ + uint8_t cell_version; + sendme_cell_t *cell = NULL; + + tor_assert(cell_payload); + + /* An empty payload means version 0 so skip trunnel parsing. We won't be + * able to parse a 0 length buffer into a valid SENDME cell. */ + if (cell_payload_len == 0) { + cell_version = 0; + } else { + /* First we'll decode the cell so we can get the version. */ + if (sendme_cell_parse(&cell, cell_payload, cell_payload_len) < 0) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Unparseable SENDME cell received. Closing circuit."); + goto invalid; + } + cell_version = sendme_cell_get_version(cell); + } + + /* Validate that we can handle this cell version. */ + if (!cell_version_is_valid(cell_version)) { + goto invalid; + } + + /* Validate depending on the version now. */ + switch (cell_version) { + case 0x01: + if (!cell_v1_is_valid(cell)) { + goto invalid; + } + break; + case 0x00: + /* Fallthrough. Version 0, there is no work to be done on the payload so + * it is necessarily valid if we pass the version validation. */ + default: + /* Unknown version means we can't handle it so fallback to version 0. */ + break; + } + + /* Valid cell. */ + sendme_cell_free(cell); + return 1; + invalid: + sendme_cell_free(cell); + return 0; +} /* Build and encode a version 1 SENDME cell into payload, which must be at * least of RELAY_PAYLOAD_SIZE bytes, using the digest for the cell data. @@ -215,7 +335,8 @@ sendme_circuit_consider_sending(circuit_t *circ, crypt_path_t *layer_hint, /* Process a circuit-level SENDME cell that we just received. The layer_hint, * if not NULL, is the Exit hop of the connection which means that we are a * client. In that case, circ must be an origin circuit. The cell_body_len is - * the length of the SENDME cell payload (excluding the header). + * the length of the SENDME cell payload (excluding the header). The + * cell_payload is the payload. * * Return 0 on success that is the SENDME is valid and the package window has * been updated properly. @@ -224,9 +345,11 @@ sendme_circuit_consider_sending(circuit_t *circ, crypt_path_t *layer_hint, * be closed using the value as the reason for it. */ int sendme_process_circuit_level(crypt_path_t *layer_hint, - circuit_t *circ, uint16_t cell_body_len) + circuit_t *circ, const uint8_t *cell_payload, + uint16_t cell_payload_len) { tor_assert(circ); + tor_assert(cell_payload); /* If we are the origin of the circuit, we are the Client so we use the * layer hint (the Exit hop) for the package window tracking. */ @@ -245,8 +368,15 @@ sendme_process_circuit_level(crypt_path_t *layer_hint, /* We count circuit-level sendme's as valid delivered data because they * are rate limited. */ - circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), cell_body_len); + circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), cell_payload_len); } else { + /* Validate the SENDME cell. Depending on the version, different + * validation can be done. An invalid SENDME requires us to close the + * circuit. It is only done if we are the Exit of the circuit. */ + if (!sendme_is_valid(cell_payload, cell_payload_len)) { + return -END_CIRC_REASON_TORPROTOCOL; + } + /* We aren't the origin of this circuit so we are the Exit and thus we * track the package window with the circuit object. */ if ((circ->package_window + CIRCWINDOW_INCREMENT) > diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index ba01ecfaf..033bc6ff7 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -21,7 +21,8 @@ void sendme_circuit_consider_sending(circuit_t *circ, /* Processing SENDME cell. */ int sendme_process_circuit_level(crypt_path_t *layer_hint, - circuit_t *circ, uint16_t cell_body_len); + circuit_t *circ, const uint8_t *cell_payload, + uint16_t cell_payload_len); int sendme_process_stream_level(edge_connection_t *conn, circuit_t *circ, uint16_t cell_body_len);

1 0

[tor/master] prop289: Add random bytes to the unused portion of the cell
by asn＠torproject.org 02 May '19

02 May '19

commit a6e012508e5b0d676cdf204fcbd7942e3cc21419 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Feb 19 15:02:11 2019 -0500 prop289: Add random bytes to the unused portion of the cell Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/relay.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 47275a811..63c406d8a 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -572,6 +572,14 @@ relay_send_command_from_edge_,(streamid_t stream_id, circuit_t *circ, if (payload_len) memcpy(cell.payload+RELAY_HEADER_SIZE, payload, payload_len); + /* Add random bytes to the unused portion of the payload, to foil attacks + * where the other side can predict all of the bytes in the payload and thus + * compute authenticated sendme cells without seeing the traffic. See + * proposal 289. */ + crypto_fast_rng_getbytes(get_thread_fast_rng(), + cell.payload + RELAY_HEADER_SIZE + payload_len, + RELAY_PAYLOAD_SIZE - payload_len); + log_debug(LD_OR,"delivering %d cell %s.", relay_command, cell_direction == CELL_DIRECTION_OUT ? "forward" : "backward");

1 0

[tor/master] prop289: Move SENDME cell processing in a separate function
by asn＠torproject.org 02 May '19

02 May '19

commit 2ec25e847eb2d9af0a1a1c552ffa8dbd87cf6023 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Mar 7 11:35:52 2019 -0500 prop289: Move SENDME cell processing in a separate function No behavior change. Only moving code and fixing part of it in order to use the parameters passed as pointers. Part of #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/relay.c | 137 +++++++++++++++++++++++++++++----------------------- 1 file changed, 76 insertions(+), 61 deletions(-) diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 63c406d8a..6bf7ac1a7 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -1444,6 +1444,81 @@ connection_edge_process_relay_cell_not_open( // return -1; } +/** Process a SENDME cell that arrived on circ. If it is a stream level + * cell, it is destined for the given conn. If it is a circuit level + * cell, it is destined for the layer_hint. The domain is the + * logging domain that should be used. + * + * Return 0 if everything went well or a negative value representing a circuit + * end reason on error for which the caller is responsible for closing it. */ +static int +process_sendme_cell(const relay_header_t *rh, const cell_t *cell, + circuit_t *circ, edge_connection_t *conn, + crypt_path_t *layer_hint, int domain) +{ + int ret; + + tor_assert(rh); + + if (!rh->stream_id) { + /* Circuit level SENDME cell. */ + ret = sendme_process_circuit_level(layer_hint, circ, + cell->payload + RELAY_HEADER_SIZE, + rh->length); + if (ret < 0) { + return ret; + } + /* Resume reading on any streams now that we've processed a valid + * SENDME cell that updated our package window. */ + circuit_resume_edge_reading(circ, layer_hint); + /* We are done, the rest of the code is for the stream level. */ + return 0; + } + + /* No connection, might be half edge state. We are done if so. */ + if (!conn) { + if (CIRCUIT_IS_ORIGIN(circ)) { + origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); + if (connection_half_edge_is_valid_sendme(ocirc->half_streams, + rh->stream_id)) { + circuit_read_valid_data(ocirc, rh->length); + log_info(domain, "Sendme cell on circ %u valid on half-closed " + "stream id %d", + ocirc->global_identifier, rh->stream_id); + } + } + + log_info(domain, "SENDME cell dropped, unknown stream (streamid %d).", + rh->stream_id); + return 0; + } + + /* Stream level SENDME cell. */ + ret = sendme_process_stream_level(conn, circ, rh->length); + if (ret < 0) { + /* Means we need to close the circuit with reason ret. */ + return ret; + } + + /* We've now processed properly a SENDME cell, all windows have been + * properly updated, we'll read on the edge connection to see if we can + * get data out towards the end point (Exit or client) since we are now + * allowed to deliver more cells. */ + + if (circuit_queue_streams_are_blocked(circ)) { + /* Still waiting for queue to flush; don't touch conn */ + return 0; + } + connection_start_reading(TO_CONN(conn)); + /* handle whatever might still be on the inbuf */ + if (connection_edge_package_raw_inbuf(conn, 1, NULL) < 0) { + /* (We already sent an end cell if possible) */ + connection_mark_for_close(TO_CONN(conn)); + return 0; + } + return 0; +} + /** An incoming relay cell has arrived on circuit circ. If * conn is NULL this is a control cell, else cell is * destined for conn. @@ -1825,67 +1900,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, (unsigned)circ->n_circ_id, rh.stream_id); return 0; case RELAY_COMMAND_SENDME: - { - int ret; - - if (!rh.stream_id) { - /* Circuit level SENDME cell. */ - ret = sendme_process_circuit_level(layer_hint, circ, - cell->payload + RELAY_HEADER_SIZE, - rh.length); - if (ret < 0) { - return ret; - } - /* Resume reading on any streams now that we've processed a valid - * SENDME cell that updated our package window. */ - circuit_resume_edge_reading(circ, layer_hint); - /* We are done, the rest of the code is for the stream level. */ - return 0; - } - - /* No connection, might be half edge state. We are done if so. */ - if (!conn) { - if (CIRCUIT_IS_ORIGIN(circ)) { - origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); - if (connection_half_edge_is_valid_sendme(ocirc->half_streams, - rh.stream_id)) { - circuit_read_valid_data(ocirc, rh.length); - log_info(domain, "Sendme cell on circ %u valid on half-closed " - "stream id %d", - ocirc->global_identifier, rh.stream_id); - } - } - - log_info(domain, "SENDME cell dropped, unknown stream (streamid %d).", - rh.stream_id); - return 0; - } - - /* Stream level SENDME cell. */ - ret = sendme_process_stream_level(conn, circ, rh.length); - if (ret < 0) { - /* Means we need to close the circuit with reason ret. */ - return ret; - } - - /* We've now processed properly a SENDME cell, all windows have been - * properly updated, we'll read on the edge connection to see if we can - * get data out towards the end point (Exit or client) since we are now - * allowed to deliver more cells. */ - - if (circuit_queue_streams_are_blocked(circ)) { - /* Still waiting for queue to flush; don't touch conn */ - return 0; - } - connection_start_reading(TO_CONN(conn)); - /* handle whatever might still be on the inbuf */ - if (connection_edge_package_raw_inbuf(conn, 1, NULL) < 0) { - /* (We already sent an end cell if possible) */ - connection_mark_for_close(TO_CONN(conn)); - return 0; - } - return 0; - } + return process_sendme_cell(&rh, cell, circ, conn, layer_hint, domain); case RELAY_COMMAND_RESOLVE: if (layer_hint) { log_fn(LOG_PROTOCOL_WARN, LD_APP,

1 0

[tor/master] prop289: Rename packaged functions with better name
by asn＠torproject.org 02 May '19

02 May '19

commit 217b55319336227f9e397db526cea551dbd796e4 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Mar 7 11:45:38 2019 -0500 prop289: Rename packaged functions with better name The circuit and stream level functions that update the package window have been renamed to have a "_note_" in them to make their purpose more clear. Part of #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/relay.c | 4 ++-- src/core/or/sendme.c | 4 ++-- src/core/or/sendme.h | 5 +++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 6bf7ac1a7..fa008120b 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -2092,7 +2092,7 @@ connection_edge_package_raw_inbuf(edge_connection_t *conn, int package_partial, } /* Handle the circuit-level SENDME package window. */ - if (sendme_circuit_data_packaged(circ, cpath_layer) < 0) { + if (sendme_note_circuit_data_packaged(circ, cpath_layer) < 0) { /* Package window has gone under 0. Protocol issue. */ log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Circuit package window is below 0. Closing circuit."); @@ -2101,7 +2101,7 @@ connection_edge_package_raw_inbuf(edge_connection_t *conn, int package_partial, } /* Handle the stream-level SENDME package window. */ - if (sendme_stream_data_packaged(conn) < 0) { + if (sendme_note_stream_data_packaged(conn) < 0) { connection_stop_reading(TO_CONN(conn)); log_debug(domain,"conn->package_window reached 0."); circuit_consider_stop_edge_reading(circ, cpath_layer); diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index a333b02b6..16ff5bcb8 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -491,7 +491,7 @@ sendme_stream_data_received(edge_connection_t *conn) * layer_hint is NULL, this means we are the Exit end point else we are the * Client. Update the package window and return its new value. */ int -sendme_circuit_data_packaged(circuit_t *circ, crypt_path_t *layer_hint) +sendme_note_circuit_data_packaged(circuit_t *circ, crypt_path_t *layer_hint) { int package_window, domain; @@ -518,7 +518,7 @@ sendme_circuit_data_packaged(circuit_t *circ, crypt_path_t *layer_hint) /* Called when a relay DATA cell is packaged for the given edge connection * conn. Update the package window and return its new value. */ int -sendme_stream_data_packaged(edge_connection_t *conn) +sendme_note_stream_data_packaged(edge_connection_t *conn) { tor_assert(conn); return --conn->package_window; diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index c2e2518da..2154a29f4 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -30,8 +30,9 @@ int sendme_stream_data_received(edge_connection_t *conn); int sendme_circuit_data_received(circuit_t *circ, crypt_path_t *layer_hint); /* Update package window functions. */ -int sendme_circuit_data_packaged(circuit_t *circ, crypt_path_t *layer_hint); -int sendme_stream_data_packaged(edge_connection_t *conn); +int sendme_note_circuit_data_packaged(circuit_t *circ, + crypt_path_t *layer_hint); +int sendme_note_stream_data_packaged(edge_connection_t *conn); /* Track cell digest. */ void sendme_note_cell_digest(circuit_t *circ);

1 0

[tor/master] prop289: Use a 20 bytes digest instead of 4
by asn＠torproject.org 02 May '19

02 May '19

commit 504e05b02999afb6a58ebe4af5770ca8dc136233 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Mar 7 11:20:23 2019 -0500 prop289: Use a 20 bytes digest instead of 4 To achieve such, this commit also changes the trunnel declaration to use a union instead of a seperate object for the v1 data. A constant is added for the digest length so we can use it within the SENDME code giving us a single reference. Part of #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/sendme.c | 41 ++---- src/test/test_sendme.c | 6 +- src/trunnel/sendme.c | 349 +++++++++++++-------------------------------- src/trunnel/sendme.h | 107 +++----------- src/trunnel/sendme.trunnel | 19 +-- 5 files changed, 146 insertions(+), 376 deletions(-) diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index 980684c82..a333b02b6 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -59,7 +59,8 @@ get_accept_min_version(void) SENDME_ACCEPT_MIN_VERSION_MAX); } -/* Return true iff the given decoded SENDME version 1 cell is valid. +/* Return true iff the given decoded SENDME version 1 cell is valid and + * matches the expected digest on the circuit. * * Validation is done by comparing the digest in the cell from the previous * cell we saw which tells us that the other side has in fact seen that cell. @@ -67,14 +68,12 @@ get_accept_min_version(void) static bool cell_v1_is_valid(const sendme_cell_t *cell, const circuit_t *circ) { - sendme_data_v1_t *data = NULL; + const uint8_t *cell_digest = NULL; tor_assert(cell); + tor_assert(circ); - if (sendme_data_v1_parse(&data, sendme_cell_getconstarray_data(cell), - sendme_cell_getlen_data(cell)) < 0) { - goto invalid; - } + cell_digest = sendme_cell_getconstarray_data_v1_digest(cell); /* We shouldn't have received this SENDME if we have no digests. Log at * protocol warning because it can be tricked by sending many SENDMEs @@ -94,8 +93,7 @@ cell_v1_is_valid(const sendme_cell_t *cell, const circuit_t *circ) /* Compare the digest with the one in the SENDME. This cell is invalid * without a perfect match. */ - if (tor_memcmp(digest, sendme_data_v1_getconstarray_digest(data), - sendme_data_v1_getlen_digest(data))) { + if (tor_memcmp(digest, cell_digest, TRUNNEL_SENDME_V1_DIGEST_LEN)) { tor_free(digest); log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "SENDME v1 cell digest do not match."); @@ -105,10 +103,8 @@ cell_v1_is_valid(const sendme_cell_t *cell, const circuit_t *circ) } /* Validated SENDME v1 cell. */ - sendme_data_v1_free(data); return 1; invalid: - sendme_data_v1_free(data); return 0; } @@ -213,39 +209,26 @@ build_cell_payload_v1(crypto_digest_t *cell_digest, uint8_t *payload) { ssize_t len = -1; sendme_cell_t *cell = NULL; - sendme_data_v1_t *data = NULL; tor_assert(cell_digest); tor_assert(payload); cell = sendme_cell_new(); - data = sendme_data_v1_new(); /* Building a payload for version 1. */ sendme_cell_set_version(cell, 0x01); + /* Set the data length field for v1. */ + sendme_cell_set_data_len(cell, TRUNNEL_SENDME_V1_DIGEST_LEN); /* Copy the digest into the data payload. */ crypto_digest_get_digest(cell_digest, - (char *) sendme_data_v1_getarray_digest(data), - sendme_data_v1_getlen_digest(data)); - - /* Set the length of the data in the cell payload. It is the encoded length - * of the v1 data object. */ - sendme_cell_setlen_data(cell, sendme_data_v1_encoded_len(data)); - /* Encode into the cell's data field using its current length just set. */ - if (sendme_data_v1_encode(sendme_cell_getarray_data(cell), - sendme_cell_getlen_data(cell), data) < 0) { - goto end; - } - /* Set the DATA_LEN field to what we've just encoded. */ - sendme_cell_set_data_len(cell, sendme_cell_getlen_data(cell)); + (char *) sendme_cell_getarray_data_v1_digest(cell), + sendme_cell_get_data_len(cell)); /* Finally, encode the cell into the payload. */ len = sendme_cell_encode(payload, RELAY_PAYLOAD_SIZE, cell); - end: sendme_cell_free(cell); - sendme_data_v1_free(data); return len; } @@ -566,9 +549,9 @@ sendme_note_cell_digest(circuit_t *circ) * recorded. It should never happen in theory as we always record the last * digest for the v1 SENDME. */ if (TO_OR_CIRCUIT(circ)->crypto.sendme_digest) { - digest = tor_malloc_zero(4); + digest = tor_malloc_zero(TRUNNEL_SENDME_V1_DIGEST_LEN); crypto_digest_get_digest(TO_OR_CIRCUIT(circ)->crypto.sendme_digest, - (char *) digest, 4); + (char *) digest, TRUNNEL_SENDME_V1_DIGEST_LEN); if (circ->sendme_last_digests == NULL) { circ->sendme_last_digests = smartlist_new(); } diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c index ad6aac6c0..92f478df0 100644 --- a/src/test/test_sendme.c +++ b/src/test/test_sendme.c @@ -156,12 +156,12 @@ test_v1_build_cell(void *arg) circ = TO_CIRCUIT(or_circ); cell_digest = crypto_digest_new(); - crypto_digest_add_bytes(cell_digest, "AAAA", 4); + crypto_digest_add_bytes(cell_digest, "AAAAAAAAAAAAAAAAAAAA", 20); tt_assert(cell_digest); - /* SENDME v1 payload is 7 bytes. See spec. */ + /* SENDME v1 payload is 3 bytes + 20 bytes digest. See spec. */ ret = build_cell_payload_v1(cell_digest, payload); - tt_int_op(ret, OP_EQ, 7); + tt_int_op(ret, OP_EQ, 23); /* Validation. */ diff --git a/src/trunnel/sendme.c b/src/trunnel/sendme.c index 08f9ed5e9..262b91523 100644 --- a/src/trunnel/sendme.c +++ b/src/trunnel/sendme.c @@ -43,8 +43,6 @@ static void sendme_cell_clear(sendme_cell_t *obj) { (void) obj; - TRUNNEL_DYNARRAY_WIPE(&obj->data); - TRUNNEL_DYNARRAY_CLEAR(&obj->data); } void @@ -84,71 +82,40 @@ sendme_cell_set_data_len(sendme_cell_t *inp, uint16_t val) return 0; } size_t -sendme_cell_getlen_data(const sendme_cell_t *inp) +sendme_cell_getlen_data_v1_digest(const sendme_cell_t *inp) { - return TRUNNEL_DYNARRAY_LEN(&inp->data); + (void)inp; return TRUNNEL_SENDME_V1_DIGEST_LEN; } uint8_t -sendme_cell_get_data(sendme_cell_t *inp, size_t idx) +sendme_cell_get_data_v1_digest(sendme_cell_t *inp, size_t idx) { - return TRUNNEL_DYNARRAY_GET(&inp->data, idx); + trunnel_assert(idx < TRUNNEL_SENDME_V1_DIGEST_LEN); + return inp->data_v1_digest[idx]; } uint8_t -sendme_cell_getconst_data(const sendme_cell_t *inp, size_t idx) +sendme_cell_getconst_data_v1_digest(const sendme_cell_t *inp, size_t idx) { - return sendme_cell_get_data((sendme_cell_t*)inp, idx); + return sendme_cell_get_data_v1_digest((sendme_cell_t*)inp, idx); } int -sendme_cell_set_data(sendme_cell_t *inp, size_t idx, uint8_t elt) +sendme_cell_set_data_v1_digest(sendme_cell_t *inp, size_t idx, uint8_t elt) { - TRUNNEL_DYNARRAY_SET(&inp->data, idx, elt); + trunnel_assert(idx < TRUNNEL_SENDME_V1_DIGEST_LEN); + inp->data_v1_digest[idx] = elt; return 0; } -int -sendme_cell_add_data(sendme_cell_t *inp, uint8_t elt) -{ -#if SIZE_MAX >= UINT16_MAX - if (inp->data.n_ == UINT16_MAX) - goto trunnel_alloc_failed; -#endif - TRUNNEL_DYNARRAY_ADD(uint8_t, &inp->data, elt, {}); - return 0; - trunnel_alloc_failed: - TRUNNEL_SET_ERROR_CODE(inp); - return -1; -} uint8_t * -sendme_cell_getarray_data(sendme_cell_t *inp) +sendme_cell_getarray_data_v1_digest(sendme_cell_t *inp) { - return inp->data.elts_; + return inp->data_v1_digest; } const uint8_t * -sendme_cell_getconstarray_data(const sendme_cell_t *inp) +sendme_cell_getconstarray_data_v1_digest(const sendme_cell_t *inp) { - return (const uint8_t *)sendme_cell_getarray_data((sendme_cell_t*)inp); -} -int -sendme_cell_setlen_data(sendme_cell_t *inp, size_t newlen) -{ - uint8_t *newptr; -#if UINT16_MAX < SIZE_MAX - if (newlen > UINT16_MAX) - goto trunnel_alloc_failed; -#endif - newptr = trunnel_dynarray_setlen(&inp->data.allocated_, - &inp->data.n_, inp->data.elts_, newlen, - sizeof(inp->data.elts_[0]), (trunnel_free_fn_t) NULL, - &inp->trunnel_error_code_); - if (newlen != 0 && newptr == NULL) - goto trunnel_alloc_failed; - inp->data.elts_ = newptr; - return 0; - trunnel_alloc_failed: - TRUNNEL_SET_ERROR_CODE(inp); - return -1; + return (const uint8_t *)sendme_cell_getarray_data_v1_digest((sendme_cell_t*)inp); } const char * sendme_cell_check(const sendme_cell_t *obj) @@ -159,8 +126,18 @@ sendme_cell_check(const sendme_cell_t *obj) return "A set function failed on this object"; if (! (obj->version == 0 || obj->version == 1)) return "Integer out of bounds"; - if (TRUNNEL_DYNARRAY_LEN(&obj->data) != obj->data_len) - return "Length mismatch for data"; + switch (obj->version) { + + case 0: + break; + + case 1: + break; + + default: + return "Bad tag for union"; + break; + } return NULL; } @@ -178,9 +155,21 @@ sendme_cell_encoded_len(const sendme_cell_t *obj) /* Length of u16 data_len */ result += 2; + switch (obj->version) { - /* Length of u8 data[data_len] */ - result += TRUNNEL_DYNARRAY_LEN(&obj->data); + case 0: + break; + + case 1: + + /* Length of u8 data_v1_digest[TRUNNEL_SENDME_V1_DIGEST_LEN] */ + result += TRUNNEL_SENDME_V1_DIGEST_LEN; + break; + + default: + trunnel_assert(0); + break; + } return result; } int @@ -201,6 +190,8 @@ sendme_cell_encode(uint8_t *output, const size_t avail, const sendme_cell_t *obj const ssize_t encoded_len = sendme_cell_encoded_len(obj); #endif + uint8_t *backptr_data_len = NULL; + if (NULL != (msg = sendme_cell_check(obj))) goto check_failed; @@ -216,22 +207,43 @@ sendme_cell_encode(uint8_t *output, const size_t avail, const sendme_cell_t *obj written += 1; ptr += 1; /* Encode u16 data_len */ + backptr_data_len = ptr; trunnel_assert(written <= avail); if (avail - written < 2) goto truncated; trunnel_set_uint16(ptr, trunnel_htons(obj->data_len)); written += 2; ptr += 2; - - /* Encode u8 data[data_len] */ { - size_t elt_len = TRUNNEL_DYNARRAY_LEN(&obj->data); - trunnel_assert(obj->data_len == elt_len); + size_t written_before_union = written; + + /* Encode union data[version] */ trunnel_assert(written <= avail); - if (avail - written < elt_len) - goto truncated; - if (elt_len) - memcpy(ptr, obj->data.elts_, elt_len); - written += elt_len; ptr += elt_len; + switch (obj->version) { + + case 0: + break; + + case 1: + + /* Encode u8 data_v1_digest[TRUNNEL_SENDME_V1_DIGEST_LEN] */ + trunnel_assert(written <= avail); + if (avail - written < TRUNNEL_SENDME_V1_DIGEST_LEN) + goto truncated; + memcpy(ptr, obj->data_v1_digest, TRUNNEL_SENDME_V1_DIGEST_LEN); + written += TRUNNEL_SENDME_V1_DIGEST_LEN; ptr += TRUNNEL_SENDME_V1_DIGEST_LEN; + break; + + default: + trunnel_assert(0); + break; + } + /* Write the length field back to data_len */ + trunnel_assert(written >= written_before_union); +#if UINT16_MAX < SIZE_MAX + if (written - written_before_union > UINT16_MAX) + goto check_failed; +#endif + trunnel_set_uint16(backptr_data_len, trunnel_htons(written - written_before_union)); } @@ -279,21 +291,41 @@ sendme_cell_parse_into(sendme_cell_t *obj, const uint8_t *input, const size_t le CHECK_REMAINING(2, truncated); obj->data_len = trunnel_ntohs(trunnel_get_uint16(ptr)); remaining -= 2; ptr += 2; - - /* Parse u8 data[data_len] */ - CHECK_REMAINING(obj->data_len, truncated); - TRUNNEL_DYNARRAY_EXPAND(uint8_t, &obj->data, obj->data_len, {}); - obj->data.n_ = obj->data_len; - if (obj->data_len) - memcpy(obj->data.elts_, ptr, obj->data_len); - ptr += obj->data_len; remaining -= obj->data_len; + { + size_t remaining_after; + CHECK_REMAINING(obj->data_len, truncated); + remaining_after = remaining - obj->data_len; + remaining = obj->data_len; + + /* Parse union data[version] */ + switch (obj->version) { + + case 0: + /* Skip to end of union */ + ptr += remaining; remaining = 0; + break; + + case 1: + + /* Parse u8 data_v1_digest[TRUNNEL_SENDME_V1_DIGEST_LEN] */ + CHECK_REMAINING(TRUNNEL_SENDME_V1_DIGEST_LEN, fail); + memcpy(obj->data_v1_digest, ptr, TRUNNEL_SENDME_V1_DIGEST_LEN); + remaining -= TRUNNEL_SENDME_V1_DIGEST_LEN; ptr += TRUNNEL_SENDME_V1_DIGEST_LEN; + break; + + default: + goto fail; + break; + } + if (remaining != 0) + goto fail; + remaining = remaining_after; + } trunnel_assert(ptr + remaining == input + len_in); return len_in - remaining; truncated: return -2; - trunnel_alloc_failed: - return -1; fail: result = -1; return result; @@ -313,180 +345,3 @@ sendme_cell_parse(sendme_cell_t **output, const uint8_t *input, const size_t len } return result; } -sendme_data_v1_t * -sendme_data_v1_new(void) -{ - sendme_data_v1_t *val = trunnel_calloc(1, sizeof(sendme_data_v1_t)); - if (NULL == val) - return NULL; - return val; -} - -/** Release all storage held inside 'obj', but do not free 'obj'. - */ -static void -sendme_data_v1_clear(sendme_data_v1_t *obj) -{ - (void) obj; -} - -void -sendme_data_v1_free(sendme_data_v1_t *obj) -{ - if (obj == NULL) - return; - sendme_data_v1_clear(obj); - trunnel_memwipe(obj, sizeof(sendme_data_v1_t)); - trunnel_free_(obj); -} - -size_t -sendme_data_v1_getlen_digest(const sendme_data_v1_t *inp) -{ - (void)inp; return 4; -} - -uint8_t -sendme_data_v1_get_digest(sendme_data_v1_t *inp, size_t idx) -{ - trunnel_assert(idx < 4); - return inp->digest[idx]; -} - -uint8_t -sendme_data_v1_getconst_digest(const sendme_data_v1_t *inp, size_t idx) -{ - return sendme_data_v1_get_digest((sendme_data_v1_t*)inp, idx); -} -int -sendme_data_v1_set_digest(sendme_data_v1_t *inp, size_t idx, uint8_t elt) -{ - trunnel_assert(idx < 4); - inp->digest[idx] = elt; - return 0; -} - -uint8_t * -sendme_data_v1_getarray_digest(sendme_data_v1_t *inp) -{ - return inp->digest; -} -const uint8_t * -sendme_data_v1_getconstarray_digest(const sendme_data_v1_t *inp) -{ - return (const uint8_t *)sendme_data_v1_getarray_digest((sendme_data_v1_t*)inp); -} -const char * -sendme_data_v1_check(const sendme_data_v1_t *obj) -{ - if (obj == NULL) - return "Object was NULL"; - if (obj->trunnel_error_code_) - return "A set function failed on this object"; - return NULL; -} - -ssize_t -sendme_data_v1_encoded_len(const sendme_data_v1_t *obj) -{ - ssize_t result = 0; - - if (NULL != sendme_data_v1_check(obj)) - return -1; - - - /* Length of u8 digest[4] */ - result += 4; - return result; -} -int -sendme_data_v1_clear_errors(sendme_data_v1_t *obj) -{ - int r = obj->trunnel_error_code_; - obj->trunnel_error_code_ = 0; - return r; -} -ssize_t -sendme_data_v1_encode(uint8_t *output, const size_t avail, const sendme_data_v1_t *obj) -{ - ssize_t result = 0; - size_t written = 0; - uint8_t *ptr = output; - const char *msg; -#ifdef TRUNNEL_CHECK_ENCODED_LEN - const ssize_t encoded_len = sendme_data_v1_encoded_len(obj); -#endif - - if (NULL != (msg = sendme_data_v1_check(obj))) - goto check_failed; - -#ifdef TRUNNEL_CHECK_ENCODED_LEN - trunnel_assert(encoded_len >= 0); -#endif - - /* Encode u8 digest[4] */ - trunnel_assert(written <= avail); - if (avail - written < 4) - goto truncated; - memcpy(ptr, obj->digest, 4); - written += 4; ptr += 4; - - - trunnel_assert(ptr == output + written); -#ifdef TRUNNEL_CHECK_ENCODED_LEN - { - trunnel_assert(encoded_len >= 0); - trunnel_assert((size_t)encoded_len == written); - } - -#endif - - return written; - - truncated: - result = -2; - goto fail; - check_failed: - (void)msg; - result = -1; - goto fail; - fail: - trunnel_assert(result < 0); - return result; -} - -/** As sendme_data_v1_parse(), but do not allocate the output object. - */ -static ssize_t -sendme_data_v1_parse_into(sendme_data_v1_t *obj, const uint8_t *input, const size_t len_in) -{ - const uint8_t *ptr = input; - size_t remaining = len_in; - ssize_t result = 0; - (void)result; - - /* Parse u8 digest[4] */ - CHECK_REMAINING(4, truncated); - memcpy(obj->digest, ptr, 4); - remaining -= 4; ptr += 4; - trunnel_assert(ptr + remaining == input + len_in); - return len_in - remaining; - - truncated: - return -2; -} - -ssize_t -sendme_data_v1_parse(sendme_data_v1_t **output, const uint8_t *input, const size_t len_in) -{ - ssize_t result; - *output = sendme_data_v1_new(); - if (NULL == *output) - return -1; - result = sendme_data_v1_parse_into(*output, input, len_in); - if (result < 0) { - sendme_data_v1_free(*output); - *output = NULL; - } - return result; -} diff --git a/src/trunnel/sendme.h b/src/trunnel/sendme.h index 8207cb56f..f3c3dd78c 100644 --- a/src/trunnel/sendme.h +++ b/src/trunnel/sendme.h @@ -8,22 +8,16 @@ #include <stdint.h> #include "trunnel.h" +#define TRUNNEL_SENDME_V1_DIGEST_LEN 20 #if !defined(TRUNNEL_OPAQUE) && !defined(TRUNNEL_OPAQUE_SENDME_CELL) struct sendme_cell_st { uint8_t version; uint16_t data_len; - TRUNNEL_DYNARRAY_HEAD(, uint8_t) data; + uint8_t data_v1_digest[TRUNNEL_SENDME_V1_DIGEST_LEN]; uint8_t trunnel_error_code_; }; #endif typedef struct sendme_cell_st sendme_cell_t; -#if !defined(TRUNNEL_OPAQUE) && !defined(TRUNNEL_OPAQUE_SENDME_DATA_V1) -struct sendme_data_v1_st { - uint8_t digest[4]; - uint8_t trunnel_error_code_; -}; -#endif -typedef struct sendme_data_v1_st sendme_data_v1_t; /** Return a newly allocated sendme_cell with all elements set to * zero. */ @@ -77,94 +71,31 @@ uint16_t sendme_cell_get_data_len(const sendme_cell_t *inp); * 'inp' on failure. */ int sendme_cell_set_data_len(sendme_cell_t *inp, uint16_t val); -/** Return the length of the dynamic array holding the data field of - * the sendme_cell_t in 'inp'. - */ -size_t sendme_cell_getlen_data(const sendme_cell_t *inp); -/** Return the element at position 'idx' of the dynamic array field - * data of the sendme_cell_t in 'inp'. - */ -uint8_t sendme_cell_get_data(sendme_cell_t *inp, size_t idx); -/** As sendme_cell_get_data, but take and return a const pointer - */ -uint8_t sendme_cell_getconst_data(const sendme_cell_t *inp, size_t idx); -/** Change the element at position 'idx' of the dynamic array field - * data of the sendme_cell_t in 'inp', so that it will hold the value - * 'elt'. - */ -int sendme_cell_set_data(sendme_cell_t *inp, size_t idx, uint8_t elt); -/** Append a new element 'elt' to the dynamic array field data of the - * sendme_cell_t in 'inp'. - */ -int sendme_cell_add_data(sendme_cell_t *inp, uint8_t elt); -/** Return a pointer to the variable-length array field data of 'inp'. - */ -uint8_t * sendme_cell_getarray_data(sendme_cell_t *inp); -/** As sendme_cell_get_data, but take and return a const pointer - */ -const uint8_t * sendme_cell_getconstarray_data(const sendme_cell_t *inp); -/** Change the length of the variable-length array field data of 'inp' - * to 'newlen'.Fill extra elements with 0. Return 0 on success; return - * -1 and set the error code on 'inp' on failure. - */ -int sendme_cell_setlen_data(sendme_cell_t *inp, size_t newlen); -/** Return a newly allocated sendme_data_v1 with all elements set to - * zero. - */ -sendme_data_v1_t *sendme_data_v1_new(void); -/** Release all storage held by the sendme_data_v1 in 'victim'. (Do - * nothing if 'victim' is NULL.) - */ -void sendme_data_v1_free(sendme_data_v1_t *victim); -/** Try to parse a sendme_data_v1 from the buffer in 'input', using up - * to 'len_in' bytes from the input buffer. On success, return the - * number of bytes consumed and set *output to the newly allocated - * sendme_data_v1_t. On failure, return -2 if the input appears - * truncated, and -1 if the input is otherwise invalid. - */ -ssize_t sendme_data_v1_parse(sendme_data_v1_t **output, const uint8_t *input, const size_t len_in); -/** Return the number of bytes we expect to need to encode the - * sendme_data_v1 in 'obj'. On failure, return a negative value. Note - * that this value may be an overestimate, and can even be an - * underestimate for certain unencodeable objects. - */ -ssize_t sendme_data_v1_encoded_len(const sendme_data_v1_t *obj); -/** Try to encode the sendme_data_v1 from 'input' into the buffer at - * 'output', using up to 'avail' bytes of the output buffer. On - * success, return the number of bytes used. On failure, return -2 if - * the buffer was not long enough, and -1 if the input was invalid. - */ -ssize_t sendme_data_v1_encode(uint8_t *output, size_t avail, const sendme_data_v1_t *input); -/** Check whether the internal state of the sendme_data_v1 in 'obj' is - * consistent. Return NULL if it is, and a short message if it is not. - */ -const char *sendme_data_v1_check(const sendme_data_v1_t *obj); -/** Clear any errors that were set on the object 'obj' by its setter - * functions. Return true iff errors were cleared. - */ -int sendme_data_v1_clear_errors(sendme_data_v1_t *obj); -/** Return the (constant) length of the array holding the digest field - * of the sendme_data_v1_t in 'inp'. +/** Return the (constant) length of the array holding the + * data_v1_digest field of the sendme_cell_t in 'inp'. */ -size_t sendme_data_v1_getlen_digest(const sendme_data_v1_t *inp); +size_t sendme_cell_getlen_data_v1_digest(const sendme_cell_t *inp); /** Return the element at position 'idx' of the fixed array field - * digest of the sendme_data_v1_t in 'inp'. + * data_v1_digest of the sendme_cell_t in 'inp'. */ -uint8_t sendme_data_v1_get_digest(sendme_data_v1_t *inp, size_t idx); -/** As sendme_data_v1_get_digest, but take and return a const pointer +uint8_t sendme_cell_get_data_v1_digest(sendme_cell_t *inp, size_t idx); +/** As sendme_cell_get_data_v1_digest, but take and return a const + * pointer */ -uint8_t sendme_data_v1_getconst_digest(const sendme_data_v1_t *inp, size_t idx); +uint8_t sendme_cell_getconst_data_v1_digest(const sendme_cell_t *inp, size_t idx); /** Change the element at position 'idx' of the fixed array field - * digest of the sendme_data_v1_t in 'inp', so that it will hold the - * value 'elt'. + * data_v1_digest of the sendme_cell_t in 'inp', so that it will hold + * the value 'elt'. */ -int sendme_data_v1_set_digest(sendme_data_v1_t *inp, size_t idx, uint8_t elt); -/** Return a pointer to the 4-element array field digest of 'inp'. +int sendme_cell_set_data_v1_digest(sendme_cell_t *inp, size_t idx, uint8_t elt); +/** Return a pointer to the TRUNNEL_SENDME_V1_DIGEST_LEN-element array + * field data_v1_digest of 'inp'. */ -uint8_t * sendme_data_v1_getarray_digest(sendme_data_v1_t *inp); -/** As sendme_data_v1_get_digest, but take and return a const pointer +uint8_t * sendme_cell_getarray_data_v1_digest(sendme_cell_t *inp); +/** As sendme_cell_get_data_v1_digest, but take and return a const + * pointer */ -const uint8_t * sendme_data_v1_getconstarray_digest(const sendme_data_v1_t *inp); +const uint8_t * sendme_cell_getconstarray_data_v1_digest(const sendme_cell_t *inp); #endif diff --git a/src/trunnel/sendme.trunnel b/src/trunnel/sendme.trunnel index 7294a09b4..300963e67 100644 --- a/src/trunnel/sendme.trunnel +++ b/src/trunnel/sendme.trunnel @@ -1,18 +1,19 @@ /* This file contains the SENDME cell definition. */ +/* v1 digest length in bytes. */ +const TRUNNEL_SENDME_V1_DIGEST_LEN = 20; + +/* SENDME cell declaration. */ struct sendme_cell { /* Version field. */ u8 version IN [0x00, 0x01]; - /* The data content depends on the version. */ + /* Length of data contained in this cell. */ u16 data_len; - u8 data[data_len]; -} -/* SENDME version 0. No data. */ - -/* SENDME version 1. Authenticated with digest. */ -struct sendme_data_v1 { - /* A 4 bytes digest. */ - u8 digest[4]; + /* The data content depends on the version. */ + union data[version] with length data_len { + 0x00: ignore; + 0x01: u8 v1_digest[TRUNNEL_SENDME_V1_DIGEST_LEN]; + }; }

1 0

[tor/master] prop289: Keep the digest bytes, not the object
by asn＠torproject.org 02 May '19

02 May '19

commit 77d560af64226eaa0fde157d7a6607791975a7a9 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Mar 7 12:30:13 2019 -0500 prop289: Keep the digest bytes, not the object The digest object is as large as the entire internal digest object's state, which is often much larger than the actual set of bytes you're transmitting. This commit makes it that we keep the digest itself which is 20 bytes. Part of #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/crypto/relay_crypto.c | 17 ++++++----------- src/core/or/relay_crypto_st.h | 2 +- src/core/or/sendme.c | 29 +++++++++++------------------ src/core/or/sendme.h | 2 +- src/test/test_relaycell.c | 1 - src/test/test_sendme.c | 16 +++++++--------- 6 files changed, 26 insertions(+), 41 deletions(-) diff --git a/src/core/crypto/relay_crypto.c b/src/core/crypto/relay_crypto.c index d4116d47a..94e906065 100644 --- a/src/core/crypto/relay_crypto.c +++ b/src/core/crypto/relay_crypto.c @@ -143,12 +143,9 @@ relay_decrypt_cell(circuit_t *circ, cell_t *cell, *recognized = 1; *layer_hint = thishop; /* Keep current digest of this cell for the possible SENDME. */ - if (thishop->crypto.sendme_digest) { - crypto_digest_free(thishop->crypto.sendme_digest); - } - thishop->crypto.sendme_digest = - crypto_digest_dup(thishop->crypto.b_digest); - + crypto_digest_get_digest(thishop->crypto.b_digest, + (char *) thishop->crypto.sendme_digest, + sizeof(thishop->crypto.sendme_digest)); return 0; } } @@ -220,10 +217,9 @@ relay_encrypt_cell_inbound(cell_t *cell, { relay_set_digest(or_circ->crypto.b_digest, cell); /* Keep a record of this cell, we might use it for validating the SENDME. */ - if (or_circ->crypto.sendme_digest) { - crypto_digest_free(or_circ->crypto.sendme_digest); - } - or_circ->crypto.sendme_digest = crypto_digest_dup(or_circ->crypto.b_digest); + crypto_digest_get_digest(or_circ->crypto.b_digest, + (char *) or_circ->crypto.sendme_digest, + sizeof(or_circ->crypto.sendme_digest)); /* encrypt one layer */ relay_crypt_one_payload(or_circ->crypto.b_crypto, cell->payload); } @@ -241,7 +237,6 @@ relay_crypto_clear(relay_crypto_t *crypto) crypto_cipher_free(crypto->b_crypto); crypto_digest_free(crypto->f_digest); crypto_digest_free(crypto->b_digest); - crypto_digest_free(crypto->sendme_digest); } /** Initialize crypto from the key material in key_data. diff --git a/src/core/or/relay_crypto_st.h b/src/core/or/relay_crypto_st.h index dbdf1599d..1f243ccdc 100644 --- a/src/core/or/relay_crypto_st.h +++ b/src/core/or/relay_crypto_st.h @@ -26,7 +26,7 @@ struct relay_crypto_t { struct crypto_digest_t *b_digest; /** Digest used for the next SENDME cell if any. */ - struct crypto_digest_t *sendme_digest; + uint8_t sendme_digest[DIGEST_LEN]; }; #undef crypto_cipher_t diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index 0a7b1cbc0..3dcd9df08 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -215,7 +215,7 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, * Return the size in bytes of the encoded cell in payload. A negative value * is returned on encoding failure. */ STATIC ssize_t -build_cell_payload_v1(crypto_digest_t *cell_digest, uint8_t *payload) +build_cell_payload_v1(const uint8_t *cell_digest, uint8_t *payload) { ssize_t len = -1; sendme_cell_t *cell = NULL; @@ -231,9 +231,8 @@ build_cell_payload_v1(crypto_digest_t *cell_digest, uint8_t *payload) sendme_cell_set_data_len(cell, TRUNNEL_SENDME_V1_DIGEST_LEN); /* Copy the digest into the data payload. */ - crypto_digest_get_digest(cell_digest, - (char *) sendme_cell_getarray_data_v1_digest(cell), - sendme_cell_get_data_len(cell)); + memcpy(sendme_cell_getarray_data_v1_digest(cell), cell_digest, + sendme_cell_get_data_len(cell)); /* Finally, encode the cell into the payload. */ len = sendme_cell_encode(payload, RELAY_PAYLOAD_SIZE, cell); @@ -249,7 +248,7 @@ build_cell_payload_v1(crypto_digest_t *cell_digest, uint8_t *payload) * because we failed to send the cell on it. */ static int send_circuit_level_sendme(circuit_t *circ, crypt_path_t *layer_hint, - crypto_digest_t *cell_digest) + const uint8_t *cell_digest) { uint8_t emit_version; uint8_t payload[RELAY_PAYLOAD_SIZE]; @@ -340,7 +339,7 @@ sendme_connection_edge_consider_sending(edge_connection_t *conn) void sendme_circuit_consider_sending(circuit_t *circ, crypt_path_t *layer_hint) { - crypto_digest_t *digest; + const uint8_t *digest; while ((layer_hint ? layer_hint->deliver_window : circ->deliver_window) <= CIRCWINDOW_START - CIRCWINDOW_INCREMENT) { @@ -539,7 +538,7 @@ sendme_note_stream_data_packaged(edge_connection_t *conn) void sendme_note_cell_digest(circuit_t *circ) { - uint8_t *digest; + const uint8_t *digest; tor_assert(circ); @@ -555,16 +554,10 @@ sendme_note_cell_digest(circuit_t *circ) return; } - /* Only note the digest if we actually have the digest of the previous cell - * recorded. It should never happen in theory as we always record the last - * digest for the v1 SENDME. */ - if (TO_OR_CIRCUIT(circ)->crypto.sendme_digest) { - digest = tor_malloc_zero(TRUNNEL_SENDME_V1_DIGEST_LEN); - crypto_digest_get_digest(TO_OR_CIRCUIT(circ)->crypto.sendme_digest, - (char *) digest, TRUNNEL_SENDME_V1_DIGEST_LEN); - if (circ->sendme_last_digests == NULL) { - circ->sendme_last_digests = smartlist_new(); - } - smartlist_add(circ->sendme_last_digests, digest); + /* Add the digest to the last seen list in the circuit. */ + digest = TO_OR_CIRCUIT(circ)->crypto.sendme_digest; + if (circ->sendme_last_digests == NULL) { + circ->sendme_last_digests = smartlist_new(); } + smartlist_add(circ->sendme_last_digests, tor_memdup(digest, DIGEST_LEN)); } diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index 2154a29f4..71df9b6f0 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -50,7 +50,7 @@ STATIC int get_accept_min_version(void); STATIC bool cell_version_is_valid(uint8_t cell_version); -STATIC ssize_t build_cell_payload_v1(crypto_digest_t *cell_digest, +STATIC ssize_t build_cell_payload_v1(const uint8_t *cell_digest, uint8_t *payload); STATIC bool sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c index c4ed215c7..062358351 100644 --- a/src/test/test_relaycell.c +++ b/src/test/test_relaycell.c @@ -705,7 +705,6 @@ test_circbw_relay(void *arg) circ = helper_create_origin_circuit(CIRCUIT_PURPOSE_C_GENERAL, 0); circ->cpath->state = CPATH_STATE_AWAITING_KEYS; circ->cpath->deliver_window = CIRCWINDOW_START; - circ->cpath->crypto.sendme_digest = crypto_digest_new(); entryconn1 = fake_entry_conn(circ, 1); edgeconn = ENTRY_TO_EDGE_CONN(entryconn1); diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c index 92f478df0..d6410a748 100644 --- a/src/test/test_sendme.c +++ b/src/test/test_sendme.c @@ -16,6 +16,8 @@ #include "feature/nodelist/networkstatus.h" #include "feature/nodelist/networkstatus_st.h" +#include "lib/crypt_ops/crypto_digest.h" + #include "test/test.h" #include "test/log_test_helpers.h" @@ -64,7 +66,6 @@ test_v1_note_digest(void *arg) circuit_free_(circ); /* Points it to the OR circuit now. */ circ = TO_CIRCUIT(or_circ); - or_circ->crypto.sendme_digest = crypto_digest_new(); /* The package window has to be a multiple of CIRCWINDOW_INCREMENT minus 1 * in order to catched the CIRCWINDOW_INCREMENT-nth cell. Try something that @@ -144,7 +145,7 @@ test_v1_consensus_params(void *arg) static void test_v1_build_cell(void *arg) { - uint8_t payload[RELAY_PAYLOAD_SIZE]; + uint8_t payload[RELAY_PAYLOAD_SIZE], digest[DIGEST_LEN]; ssize_t ret; crypto_digest_t *cell_digest = NULL; or_circuit_t *or_circ = NULL; @@ -156,11 +157,12 @@ test_v1_build_cell(void *arg) circ = TO_CIRCUIT(or_circ); cell_digest = crypto_digest_new(); - crypto_digest_add_bytes(cell_digest, "AAAAAAAAAAAAAAAAAAAA", 20); tt_assert(cell_digest); + crypto_digest_add_bytes(cell_digest, "AAAAAAAAAAAAAAAAAAAA", 20); + crypto_digest_get_digest(cell_digest, (char *) digest, sizeof(digest)); /* SENDME v1 payload is 3 bytes + 20 bytes digest. See spec. */ - ret = build_cell_payload_v1(cell_digest, payload); + ret = build_cell_payload_v1(digest, payload); tt_int_op(ret, OP_EQ, 23); /* Validation. */ @@ -183,7 +185,6 @@ test_v1_build_cell(void *arg) teardown_capture_of_logs(); /* Note the wrong digest in the circuit, cell should fail validation. */ - or_circ->crypto.sendme_digest = crypto_digest_new(); circ->package_window = CIRCWINDOW_INCREMENT + 1; sendme_note_cell_digest(circ); tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1); @@ -194,11 +195,8 @@ test_v1_build_cell(void *arg) expect_log_msg_containing("SENDME v1 cell digest do not match."); teardown_capture_of_logs(); - /* Cleanup */ - crypto_digest_free(or_circ->crypto.sendme_digest); - /* Record the cell digest into the circuit, cell should validate. */ - or_circ->crypto.sendme_digest = crypto_digest_dup(cell_digest); + memcpy(or_circ->crypto.sendme_digest, digest, sizeof(digest)); circ->package_window = CIRCWINDOW_INCREMENT + 1; sendme_note_cell_digest(circ); tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1);

1 0