commit 5b33d95a3dfe943625d78983bb53be2901a51150 Author: David Goulet dgoulet@torproject.org Date: Tue May 30 10:27:42 2017 -0400

hs: Correctly validate v3 descriptor encrypted length

The encrypted_data_length_is_valid() function wasn't validating correctly the length of the encrypted data of a v3 descriptor. The side effect of this is that an HSDir was rejecting the descriptor and ultimately not storing it.

Fixes #22447

Signed-off-by: David Goulet dgoulet@torproject.org --- changes/bug22447 | 3 +++ src/or/hs_descriptor.c | 23 ++++------------------- src/test/test_hs_descriptor.c | 5 ++--- 3 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/changes/bug22447 b/changes/bug22447 new file mode 100644 index 0000000..f5649d6 --- /dev/null +++ b/changes/bug22447 @@ -0,0 +1,3 @@ + o Major bugfixes (hidden service v3): + - HSDir failed to validate the encrypted size of a v3 descriptor and thus + rejecting it. Fixes bug 22447; bugfix on tor-0.3.0.1-alpha. diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index f16a2fd..938b7a7 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -1023,30 +1023,15 @@ cert_parse_and_validate(tor_cert_t **cert_out, const char *data, STATIC int encrypted_data_length_is_valid(size_t len) { - /* Check for the minimum length possible. */ - if (len < HS_DESC_ENCRYPTED_MIN_LEN) { + /* Make sure there is enough data for the salt and the mac. The equality is + * there to ensure that there is at least one byte of encrypted data. */ + if (len <= HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN) { log_warn(LD_REND, "Length of descriptor's encrypted data is too small. " "Got %lu but minimum value is %d", - (unsigned long)len, HS_DESC_ENCRYPTED_MIN_LEN); + (unsigned long)len, HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN); goto err; }

- /* Encrypted data has the salt and MAC concatenated to it so remove those - * from the validation calculation. */ - len -= HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN; - - /* Check that it's aligned on the block size of the crypto algorithm. */ - if (len % HS_DESC_PLAINTEXT_PADDING_MULTIPLE) { - log_warn(LD_REND, "Length of descriptor's encrypted data is invalid. " - "Got %lu which is not a multiple of %d.", - (unsigned long) len, HS_DESC_PLAINTEXT_PADDING_MULTIPLE); - goto err; - } - - /* XXX: Check maximum size. Will strongly depends on the maximum intro point - * allowed we decide on and probably if they will all have to use the legacy - * key which is bigger than the ed25519 key. */ - return 1; err: return 0; diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c index 02a71aa..97fe191 100644 --- a/src/test/test_hs_descriptor.c +++ b/src/test/test_hs_descriptor.c @@ -587,9 +587,8 @@ test_encrypted_data_len(void *arg) /* No length, error. */ ret = encrypted_data_length_is_valid(0); tt_int_op(ret, OP_EQ, 0); - /* Not a multiple of our encryption algorithm (thus no padding). It's - * suppose to be aligned on HS_DESC_PLAINTEXT_PADDING_MULTIPLE. */ - value = HS_DESC_PLAINTEXT_PADDING_MULTIPLE * 10 - 1; + /* This value is missing data. */ + value = HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN; ret = encrypted_data_length_is_valid(value); tt_int_op(ret, OP_EQ, 0); /* Valid value. */