commit 96a53221b08436d1fa97e3024f46039591f988c7 Author: David Goulet dgoulet@torproject.org Date: Tue May 28 13:00:57 2019 -0400
hs-v3: Keep descriptor in cache if client auth is missing or bad
We now keep the descriptor in the cache, obviously not decoded, if it can't be decrypted for which we believe client authorization is missing or unusable (bad).
This way, it can be used later once the client authorization are added or updated.
Part of #30382
Signed-off-by: David Goulet dgoulet@torproject.org --- src/feature/hs/hs_cache.c | 39 +++++++++++++++++++++++++++++++++------ src/feature/hs/hs_client.c | 13 ++++++++----- src/feature/hs/hs_client.h | 2 +- src/feature/hs/hs_descriptor.c | 6 +++++- src/feature/hs/hs_descriptor.h | 26 +++++++++++++++++++++----- src/test/test_hs_descriptor.c | 4 ++-- 6 files changed, 70 insertions(+), 20 deletions(-)
diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c index 395839fce..39c226746 100644 --- a/src/feature/hs/hs_cache.c +++ b/src/feature/hs/hs_cache.c @@ -397,6 +397,7 @@ static hs_cache_client_descriptor_t * cache_client_desc_new(const char *desc_str, const ed25519_public_key_t *service_identity_pk) { + hs_desc_decode_status_t ret; hs_descriptor_t *desc = NULL; hs_cache_client_descriptor_t *client_desc = NULL;
@@ -404,10 +405,24 @@ cache_client_desc_new(const char *desc_str, tor_assert(service_identity_pk);
/* Decode the descriptor we just fetched. */ - if (hs_client_decode_descriptor(desc_str, service_identity_pk, &desc) < 0) { + ret = hs_client_decode_descriptor(desc_str, service_identity_pk, &desc); + if (ret != HS_DESC_DECODE_OK && + ret != HS_DESC_DECODE_NEED_CLIENT_AUTH && + ret != HS_DESC_DECODE_BAD_CLIENT_AUTH) { + /* In the case of a missing or bad client authorization, we'll keep the + * descriptor in the cache because those credentials can arrive later. */ goto end; } - tor_assert(desc); + /* Make sure we do have a descriptor if decoding was successful. */ + if (ret == HS_DESC_DECODE_OK) { + tor_assert(desc); + } else { + if (BUG(desc != NULL)) { + /* We are not suppose to have a descriptor if the decoding code is not + * indicating success. Just in case, bail early to recover. */ + goto end; + } + }
/* All is good: make a cache object for this descriptor */ client_desc = tor_malloc_zero(sizeof(hs_cache_client_descriptor_t)); @@ -635,9 +650,19 @@ cache_store_as_client(hs_cache_client_descriptor_t *client_desc) tor_assert(client_desc);
/* Check if we already have a descriptor from this HS in cache. If we do, - * check if this descriptor is newer than the cached one */ + * check if this descriptor is newer than the cached one only if we have a + * decoded descriptor. We do keep non-decoded descriptor that requires + * client authorization. */ cache_entry = lookup_v3_desc_as_client(client_desc->key.pubkey); if (cache_entry != NULL) { + /* Signalling an undecrypted descriptor. We'll always replace the one we + * have with the new one just fetched. */ + if (cache_entry->desc == NULL) { + remove_v3_desc_as_client(cache_entry); + cache_client_desc_free(cache_entry); + goto store; + } + /* If we have an entry in our cache that has a revision counter greater * than the one we just fetched, discard the one we fetched. */ if (cache_entry->desc->plaintext_data.revision_counter > @@ -657,6 +682,7 @@ cache_store_as_client(hs_cache_client_descriptor_t *client_desc) cache_client_desc_free(cache_entry); }
+ store: /* Store descriptor in cache */ store_v3_desc_as_client(client_desc);
@@ -752,7 +778,9 @@ hs_cache_lookup_encoded_as_client(const ed25519_public_key_t *key) }
/** Public API: Given the HS ed25519 identity public key in <b>key</b>, return - * its HS descriptor if it's stored in our cache, or NULL if not. */ + * its HS descriptor if it's stored in our cache, or NULL if not or if the + * descriptor was never decrypted. The later can happen if we are waiting for + * client authorization to be added. */ const hs_descriptor_t * hs_cache_lookup_as_client(const ed25519_public_key_t *key) { @@ -761,8 +789,7 @@ hs_cache_lookup_as_client(const ed25519_public_key_t *key) tor_assert(key);
cached_desc = lookup_v3_desc_as_client(key->pubkey); - if (cached_desc) { - tor_assert(cached_desc->desc); + if (cached_desc && cached_desc->desc) { return cached_desc->desc; }
diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 4f6686143..491c52a04 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1286,13 +1286,15 @@ hs_client_note_connection_attempt_succeeded(const edge_connection_t *conn) * service_identity_pk, decode the descriptor and set the desc pointer with a * newly allocated descriptor object. * - * Return 0 on success else a negative value and desc is set to NULL. */ -int + * On success, HS_DESC_DECODE_OK is returned and desc is set to the decoded + * descriptor. On error, desc is set to NULL and a decoding error status is + * returned depending on what was the issue. */ +hs_desc_decode_status_t hs_client_decode_descriptor(const char *desc_str, const ed25519_public_key_t *service_identity_pk, hs_descriptor_t **desc) { - int ret; + hs_desc_decode_status_t ret; uint8_t subcredential[DIGEST256_LEN]; ed25519_public_key_t blinded_pubkey; hs_client_service_authorization_t *client_auth = NULL; @@ -1333,12 +1335,13 @@ hs_client_decode_descriptor(const char *desc_str, log_warn(LD_GENERAL, "Descriptor signing key certificate signature " "doesn't validate with computed blinded key: %s", tor_cert_describe_signature_status(cert)); + ret = HS_DESC_DECODE_GENERIC_ERROR; goto err; }
- return 0; + return HS_DESC_DECODE_OK; err: - return -1; + return ret; }
/** Return true iff there are at least one usable intro point in the service diff --git a/src/feature/hs/hs_client.h b/src/feature/hs/hs_client.h index 69e48ca31..0e22a2e04 100644 --- a/src/feature/hs/hs_client.h +++ b/src/feature/hs/hs_client.h @@ -48,7 +48,7 @@ void hs_client_launch_v3_desc_fetch( const ed25519_public_key_t *onion_identity_pk, const smartlist_t *hsdirs);
-int hs_client_decode_descriptor( +hs_desc_decode_status_t hs_client_decode_descriptor( const char *desc_str, const ed25519_public_key_t *service_identity_pk, hs_descriptor_t **desc); diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c index 056dc81a6..65a6f9480 100644 --- a/src/feature/hs/hs_descriptor.c +++ b/src/feature/hs/hs_descriptor.c @@ -2277,12 +2277,14 @@ desc_decode_encrypted_v3(const hs_descriptor_t *desc, * authorization is failing. */ log_warn(LD_REND, "Client authorization for requested onion address " "is invalid. Can't decrypt the descriptor."); + ret = HS_DESC_DECODE_BAD_CLIENT_AUTH; } else { /* Inform at notice level that the onion address requested can't be * reached without client authorization most likely. */ log_notice(LD_REND, "Fail to decrypt descriptor for requested onion " "address. It is likely requiring client " "authorization."); + ret = HS_DESC_DECODE_NEED_CLIENT_AUTH; } goto err; } @@ -2810,7 +2812,9 @@ hs_desc_encrypted_obj_size(const hs_desc_encrypted_data_t *data) size_t hs_desc_obj_size(const hs_descriptor_t *data) { - tor_assert(data); + if (data == NULL) { + return 0; + } return (hs_desc_plaintext_obj_size(&data->plaintext_data) + hs_desc_encrypted_obj_size(&data->encrypted_data) + sizeof(data->subcredential)); diff --git a/src/feature/hs/hs_descriptor.h b/src/feature/hs/hs_descriptor.h index 8d9dac174..4f726f8c9 100644 --- a/src/feature/hs/hs_descriptor.h +++ b/src/feature/hs/hs_descriptor.h @@ -71,11 +71,27 @@ typedef enum {
/** Error code when decoding a descriptor. */ typedef enum { - HS_DESC_DECODE_ENCRYPTED_ERROR = -4, - HS_DESC_DECODE_SUPERENC_ERROR = -3, - HS_DESC_DECODE_PLAINTEXT_ERROR = -2, - HS_DESC_DECODE_GENERIC_ERROR = -1, - HS_DESC_DECODE_OK = 0, + /* The configured client authorization for the requested .onion address + * failed to decode the descriptor. */ + HS_DESC_DECODE_BAD_CLIENT_AUTH = -6, + + /* The requested .onion address requires a client authorization. */ + HS_DESC_DECODE_NEED_CLIENT_AUTH = -5, + + /* Error during decryption of the encrypted layer. */ + HS_DESC_DECODE_ENCRYPTED_ERROR = -4, + + /* Error during decryption of the super encrypted layer. */ + HS_DESC_DECODE_SUPERENC_ERROR = -3, + + /* Error while decoding the plaintext section. */ + HS_DESC_DECODE_PLAINTEXT_ERROR = -2, + + /* Generic error. */ + HS_DESC_DECODE_GENERIC_ERROR = -1, + + /* Decoding a descriptor was successful. */ + HS_DESC_DECODE_OK = 0, } hs_desc_decode_status_t;
/** Introduction point information located in a descriptor. */ diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c index 3d6d8e67d..9587cae85 100644 --- a/src/test/test_hs_descriptor.c +++ b/src/test/test_hs_descriptor.c @@ -317,14 +317,14 @@ test_decode_descriptor(void *arg) hs_descriptor_free(decoded); ret = hs_desc_decode_descriptor(encoded, subcredential, NULL, &decoded); - tt_int_op(ret, OP_EQ, HS_DESC_DECODE_ENCRYPTED_ERROR); + tt_int_op(ret, OP_EQ, HS_DESC_DECODE_NEED_CLIENT_AUTH); tt_assert(!decoded);
/* If we have an invalid client secret key, the decoding must fail. */ hs_descriptor_free(decoded); ret = hs_desc_decode_descriptor(encoded, subcredential, &invalid_client_kp.seckey, &decoded); - tt_int_op(ret, OP_EQ, HS_DESC_DECODE_ENCRYPTED_ERROR); + tt_int_op(ret, OP_EQ, HS_DESC_DECODE_BAD_CLIENT_AUTH); tt_assert(!decoded);
/* If we have the client secret key, the decoding must succeed and the