[tor-commits] [tor/maint-0.4.3] hs-v3: Don't allow registration of an all-zeroes client auth key.

[nickm at torproject.org]

commit 37bcc9f3d2f2df0335a42c9692e7d5deafc03514
Author: George Kadianakis <desnacked at riseup.net>
Date:   Mon Mar 30 16:09:52 2020 +0300

    hs-v3: Don't allow registration of an all-zeroes client auth key.
    
    The client auth protocol allows attacker-controlled x25519 private keys being
    passed around, which allows an attacker to potentially trigger the all-zeroes
    assert for client_auth_sk in hs_descriptor.c:decrypt_descriptor_cookie().
    
    We fixed that by making sure that an all-zeroes client auth key will not be
    used.
    
    There are no guidelines for validating x25519 private keys, and the assert was
    there as a sanity check for code flow issues (we don't want to enter that
    function with an unitialized key if client auth is being used). To avoid such
    crashes in the future, we also changed the assert to a BUG-and-err.
---
 changes/bug33545                 |  4 ++++
 src/feature/control/control_hs.c |  9 ++++++++-
 src/feature/hs/hs_client.h       |  2 +-
 src/test/test_hs_control.c       | 14 ++++++++++++++
 4 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/changes/bug33545 b/changes/bug33545
new file mode 100644
index 000000000..c051b0160
--- /dev/null
+++ b/changes/bug33545
@@ -0,0 +1,4 @@
+  o Minor bugfixes (hidden services):
+    - Block a client-side assert by disallowing the registration of an x25519
+      client auth key that's all zeroes. Fixes bug 33545; bugfix on
+      0.4.3.1-alpha. Patch based on patch from "cypherpunks".
\ No newline at end of file
diff --git a/src/feature/control/control_hs.c b/src/feature/control/control_hs.c
index d3cd363f6..f5b331de9 100644
--- a/src/feature/control/control_hs.c
+++ b/src/feature/control/control_hs.c
@@ -50,11 +50,18 @@ parse_private_key_from_control_port(const char *client_privkey_str,
 
   if (base64_decode((char*)privkey->secret_key, sizeof(privkey->secret_key),
                     key_blob,
-                   strlen(key_blob)) != sizeof(privkey->secret_key)) {
+                    strlen(key_blob)) != sizeof(privkey->secret_key)) {
     control_printf_endreply(conn, 512, "Failed to decode x25519 private key");
     goto err;
   }
 
+  if (fast_mem_is_zero((const char*)privkey->secret_key,
+                       sizeof(privkey->secret_key))) {
+    control_printf_endreply(conn, 553,
+                            "Invalid private key \"%s\"", key_blob);
+    goto err;
+  }
+
   retval = 0;
 
  err:
diff --git a/src/feature/hs/hs_client.h b/src/feature/hs/hs_client.h
index 3660bfa96..d0a3a7015 100644
--- a/src/feature/hs/hs_client.h
+++ b/src/feature/hs/hs_client.h
@@ -45,7 +45,7 @@ typedef enum {
   REGISTER_SUCCESS_AND_DECRYPTED,
   /* We failed to register these credentials, because of a bad HS address. */
   REGISTER_FAIL_BAD_ADDRESS,
-  /* We failed to register these credentials, because of a bad HS address. */
+  /* We failed to store these credentials in a persistent file on disk. */
   REGISTER_FAIL_PERMANENT_STORAGE,
 } hs_client_register_auth_status_t;
 
diff --git a/src/test/test_hs_control.c b/src/test/test_hs_control.c
index 9277711d2..8ba9f1173 100644
--- a/src/test/test_hs_control.c
+++ b/src/test/test_hs_control.c
@@ -467,6 +467,20 @@ test_hs_control_bad_onion_client_auth_add(void *arg)
   cp1 = buf_get_contents(TO_CONN(&conn)->outbuf, &sz);
   tt_str_op(cp1, OP_EQ, "512 Failed to decode x25519 private key\r\n");
 
+  tor_free(cp1);
+  tor_free(args);
+
+  /* Register with an all zero client key */
+  args = tor_strdup("jt4grrjwzyz3pjkylwfau5xnjaj23vxmhskqaeyfhrfylelw4hvxcuyd "
+                    "x25519:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=");
+  retval = handle_control_command(&conn, (uint32_t) strlen(args), args);
+  tt_int_op(retval, OP_EQ, 0);
+
+  /* Check contents */
+  cp1 = buf_get_contents(TO_CONN(&conn)->outbuf, &sz);
+  tt_str_op(cp1, OP_EQ, "553 Invalid private key \"AAAAAAAAAAAAAAAAAAAA"
+                        "AAAAAAAAAAAAAAAAAAAAAAA=\"\r\n");
+
   client_auths = get_hs_client_auths_map();
   tt_assert(!client_auths);