commit 16a201e7039577070201828750e0b092f0e8eb7f Author: David Goulet dgoulet@torproject.org Date: Mon Jan 13 12:15:14 2020 -0500

hs-v3: Implement hs_parse_address_no_log()

The hs_parse_address() can not be used without an options_t object existing since on error it uses the escaped_safe_str() that looks at the options.

This new function won't log and returns an error message in case of failure that can then be used to log.

Signed-off-by: David Goulet dgoulet@torproject.org --- src/feature/hs/hs_common.c | 41 +++++++++++++++++++++++++++++++---------- src/feature/hs/hs_common.h | 4 ++++ src/test/test_hs_common.c | 6 +++--- 3 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index f8b031cc2..47594b582 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -909,30 +909,35 @@ hs_set_conn_addr_port(const smartlist_t *ports, edge_connection_t *conn) * case the caller would want only one field. checksum_out MUST at least be 2 * bytes long. * - * Return 0 if parsing went well; return -1 in case of error. */ + * Return 0 if parsing went well; return -1 in case of error and if errmsg is + * non NULL, a human readable string message is set. */ int -hs_parse_address(const char *address, ed25519_public_key_t *key_out, - uint8_t *checksum_out, uint8_t *version_out) +hs_parse_address_no_log(const char *address, ed25519_public_key_t *key_out, + uint8_t *checksum_out, uint8_t *version_out, + const char **errmsg) { char decoded[HS_SERVICE_ADDR_LEN];

tor_assert(address);

+ if (errmsg) { + *errmsg = NULL; + } + /* Obvious length check. */ if (strlen(address) != HS_SERVICE_ADDR_LEN_BASE32) { - log_warn(LD_REND, "Service address %s has an invalid length. " - "Expected %lu but got %lu.", - escaped_safe_str(address), - (unsigned long) HS_SERVICE_ADDR_LEN_BASE32, - (unsigned long) strlen(address)); + if (errmsg) { + *errmsg = "Invalid length"; + } goto invalid; }

/* Decode address so we can extract needed fields. */ if (base32_decode(decoded, sizeof(decoded), address, strlen(address)) != sizeof(decoded)) { - log_warn(LD_REND, "Service address %s can't be decoded.", - escaped_safe_str(address)); + if (errmsg) { + *errmsg = "Unable to base32 decode"; + } goto invalid; }

@@ -944,6 +949,22 @@ hs_parse_address(const char *address, ed25519_public_key_t *key_out, return -1; }

+/** Same has hs_parse_address_no_log() but emits a log warning on parsing + * failure. */ +int +hs_parse_address(const char *address, ed25519_public_key_t *key_out, + uint8_t *checksum_out, uint8_t *version_out) +{ + const char *errmsg = NULL; + int ret = hs_parse_address_no_log(address, key_out, checksum_out, + version_out, &errmsg); + if (ret < 0) { + log_warn(LD_REND, "Service address %s failed to be parsed: %s", + escaped_safe_str(address), errmsg); + } + return ret; +} + /** Validate a given onion address. The length, the base32 decoding, and * checksum are validated. Return 1 if valid else 0. */ int diff --git a/src/feature/hs/hs_common.h b/src/feature/hs/hs_common.h index 8f743d4d3..2bcf0f67c 100644 --- a/src/feature/hs/hs_common.h +++ b/src/feature/hs/hs_common.h @@ -179,6 +179,10 @@ void hs_build_address(const struct ed25519_public_key_t *key, uint8_t version, int hs_address_is_valid(const char *address); int hs_parse_address(const char *address, struct ed25519_public_key_t *key_out, uint8_t *checksum_out, uint8_t *version_out); +int hs_parse_address_no_log(const char *address, + struct ed25519_public_key_t *key_out, + uint8_t *checksum_out, uint8_t *version_out, + const char **errmsg);

void hs_build_blinded_pubkey(const struct ed25519_public_key_t *pubkey, const uint8_t *secret, size_t secret_len, diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c index 61306778d..20f88b637 100644 --- a/src/test/test_hs_common.c +++ b/src/test/test_hs_common.c @@ -53,14 +53,14 @@ test_validate_address(void *arg) setup_full_capture_of_logs(LOG_WARN); ret = hs_address_is_valid("blah"); tt_int_op(ret, OP_EQ, 0); - expect_log_msg_containing("has an invalid length"); + expect_log_msg_containing("Invalid length"); teardown_capture_of_logs();

setup_full_capture_of_logs(LOG_WARN); ret = hs_address_is_valid( "p3xnclpu4mu22dwaurjtsybyqk4xfjmcfz6z62yl24uwmhjatiwnlnadb"); tt_int_op(ret, OP_EQ, 0); - expect_log_msg_containing("has an invalid length"); + expect_log_msg_containing("Invalid length"); teardown_capture_of_logs();

/* Invalid checksum (taken from prop224) */ @@ -83,7 +83,7 @@ test_validate_address(void *arg) ret = hs_address_is_valid( "????????????????????????????????????????????????????????"); tt_int_op(ret, OP_EQ, 0); - expect_log_msg_containing("can't be decoded"); + expect_log_msg_containing("Unable to base32 decode"); teardown_capture_of_logs();

/* Valid address. */