[tor-commits] [tor/master] hs-v3: Implement hs_parse_address_no_log()

[nickm at torproject.org]

commit 16a201e7039577070201828750e0b092f0e8eb7f
Author: David Goulet <dgoulet at 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 at 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. */