Bug33545 043

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".

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:

src/feature/hs/hs_client.c

@@ -2132,6 +2132,13 @@ parse_auth_file_content(const char *client_key_str)
                       "can't be decoded: %s", seckey_b32);
     goto err;
   }
+
+  if (fast_mem_is_zero((const char*)auth->enc_seckey.secret_key,
+                       sizeof(auth->enc_seckey.secret_key))) {
+    log_warn(LD_REND, "Client authorization private key can't be all-zeroes");
+    goto err;
+  }
+
   strncpy(auth->onion_address, onion_address, HS_SERVICE_ADDR_LEN_BASE32);
 
   /* We are reading this from the disk, so set the permanent flag anyway. */

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;
 

src/feature/hs/hs_descriptor.c

@@ -1424,10 +1424,14 @@ decrypt_descriptor_cookie(const hs_descriptor_t *desc,
   tor_assert(!fast_mem_is_zero(
         (char *) &desc->superencrypted_data.auth_ephemeral_pubkey,
         sizeof(desc->superencrypted_data.auth_ephemeral_pubkey)));
-  tor_assert(!fast_mem_is_zero((char *) client_auth_sk,
-                              sizeof(*client_auth_sk)));
   tor_assert(!fast_mem_is_zero((char *) desc->subcredential, DIGEST256_LEN));
 
+  /* Catch potential code-flow cases of an unitialized private key sneaking
+   * into this function. */
+  if (BUG(fast_mem_is_zero((char *)client_auth_sk, sizeof(*client_auth_sk)))) {
+    goto done;
+  }
+
   /* Get the KEYS component to derive the CLIENT-ID and COOKIE-KEY. */
   keystream_length =
     build_descriptor_cookie_keys(desc->subcredential, DIGEST256_LEN,

src/test/test_hs_client.c

@@ -732,6 +732,10 @@ test_parse_auth_file_content(void *arg)
   /* Bigger key than it should be */
   tt_assert(!parse_auth_file_content("xx:descriptor:x25519:"
                      "vjqea4jbhwwc4hto7ekyvqfbeodghbaq6nxi45hz4wr3qvhqv3yqa"));
+  /* All-zeroes key */
+  tt_assert(!parse_auth_file_content("xx:descriptor:x25519:"
+            "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"));
+
  done:
   tor_free(auth);
 }

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);