Browse Source

Move rend auth cookie en-/decoding to a function

Tor stores client authorization cookies in two slightly different forms.
The service's client_keys file has the standard base64-encoded cookie,
including two chars of padding. The hostname file and the client remove
the two padding chars, and store an auth type flag in the unused bits.

The distinction makes no sense. Refactor all decoding to use the same
function, which will accept either form, and use a helper function for
encoding the truncated format.
John Brooks 9 years ago
parent
commit
d5a23ce115
6 changed files with 208 additions and 66 deletions
  1. 6 28
      src/or/rendclient.c
  2. 110 0
      src/or/rendcommon.c
  3. 8 0
      src/or/rendcommon.h
  4. 15 22
      src/or/rendservice.c
  5. 6 16
      src/or/routerparse.c
  6. 63 0
      src/test/test_hs.c

+ 6 - 28
src/or/rendclient.c

@@ -1465,12 +1465,10 @@ rend_parse_service_authorization(const or_options_t *options,
   strmap_t *parsed = strmap_new();
   smartlist_t *sl = smartlist_new();
   rend_service_authorization_t *auth = NULL;
-  char descriptor_cookie_tmp[REND_DESC_COOKIE_LEN+2];
-  char descriptor_cookie_base64ext[REND_DESC_COOKIE_LEN_BASE64+2+1];
+  char *err_msg = NULL;
 
   for (line = options->HidServAuth; line; line = line->next) {
     char *onion_address, *descriptor_cookie;
-    int auth_type_val = 0;
     auth = NULL;
     SMARTLIST_FOREACH(sl, char *, c, tor_free(c););
     smartlist_clear(sl);
@@ -1499,31 +1497,13 @@ rend_parse_service_authorization(const or_options_t *options,
     }
     /* Parse descriptor cookie. */
     descriptor_cookie = smartlist_get(sl, 1);
-    if (strlen(descriptor_cookie) != REND_DESC_COOKIE_LEN_BASE64) {
-      log_warn(LD_CONFIG, "Authorization cookie has wrong length: '%s'",
-               descriptor_cookie);
+    if (rend_auth_decode_cookie(descriptor_cookie, auth->descriptor_cookie,
+                                &auth->auth_type, &err_msg) < 0) {
+      tor_assert(err_msg);
+      log_warn(LD_CONFIG, "%s", err_msg);
+      tor_free(err_msg);
       goto err;
     }
-    /* Add trailing zero bytes (AA) to make base64-decoding happy. */
-    tor_snprintf(descriptor_cookie_base64ext,
-                 REND_DESC_COOKIE_LEN_BASE64+2+1,
-                 "%sAA", descriptor_cookie);
-    if (base64_decode(descriptor_cookie_tmp, sizeof(descriptor_cookie_tmp),
-                      descriptor_cookie_base64ext,
-                      strlen(descriptor_cookie_base64ext)) < 0) {
-      log_warn(LD_CONFIG, "Decoding authorization cookie failed: '%s'",
-               descriptor_cookie);
-      goto err;
-    }
-    auth_type_val = (((uint8_t)descriptor_cookie_tmp[16]) >> 4) + 1;
-    if (auth_type_val < 1 || auth_type_val > 2) {
-      log_warn(LD_CONFIG, "Authorization cookie has unknown authorization "
-                          "type encoded.");
-      goto err;
-    }
-    auth->auth_type = auth_type_val == 1 ? REND_BASIC_AUTH : REND_STEALTH_AUTH;
-    memcpy(auth->descriptor_cookie, descriptor_cookie_tmp,
-           REND_DESC_COOKIE_LEN);
     if (strmap_get(parsed, auth->onion_address)) {
       log_warn(LD_CONFIG, "Duplicate authorization for the same hidden "
                           "service.");
@@ -1546,8 +1526,6 @@ rend_parse_service_authorization(const or_options_t *options,
   } else {
     strmap_free(parsed, rend_service_authorization_strmap_item_free);
   }
-  memwipe(descriptor_cookie_tmp, 0, sizeof(descriptor_cookie_tmp));
-  memwipe(descriptor_cookie_base64ext, 0, sizeof(descriptor_cookie_base64ext));
   return res;
 }
 

+ 110 - 0
src/or/rendcommon.c

@@ -936,3 +936,113 @@ rend_data_client_create(const char *onion_address, const char *desc_id,
   return NULL;
 }
 
+/* Length of the 'extended' auth cookie used to encode auth type before
+ * base64 encoding. */
+#define REND_DESC_COOKIE_LEN_EXT (REND_DESC_COOKIE_LEN + 1)
+/* Length of the zero-padded auth cookie when base64 encoded. These two
+ * padding bytes always (A=) are stripped off of the returned cookie. */
+#define REND_DESC_COOKIE_LEN_EXT_BASE64 (REND_DESC_COOKIE_LEN_BASE64 + 2)
+
+/** Encode a client authorization descriptor cookie.
+ * The result of this function is suitable for use in the HidServAuth
+ * option.  The trailing padding characters are removed, and the
+ * auth type is encoded into the cookie.
+ *
+ * Returns a new base64-encoded cookie. This function cannot fail.
+ * The caller is responsible for freeing the returned value.
+ */
+char *
+rend_auth_encode_cookie(const uint8_t *cookie_in, rend_auth_type_t auth_type)
+{
+  uint8_t extended_cookie[REND_DESC_COOKIE_LEN_EXT];
+  char *cookie_out = tor_malloc_zero(REND_DESC_COOKIE_LEN_EXT_BASE64 + 1);
+  int re;
+
+  tor_assert(cookie_in);
+
+  memcpy(extended_cookie, cookie_in, REND_DESC_COOKIE_LEN);
+  extended_cookie[REND_DESC_COOKIE_LEN] = ((int)auth_type - 1) << 4;
+  re = base64_encode(cookie_out, REND_DESC_COOKIE_LEN_EXT_BASE64 + 1,
+                     (const char *) extended_cookie, REND_DESC_COOKIE_LEN_EXT,
+                     0);
+  tor_assert(re == REND_DESC_COOKIE_LEN_EXT_BASE64);
+
+  /* Remove the trailing 'A='.  Auth type is encoded in the high bits
+   * of the last byte, so the last base64 character will always be zero
+   * (A).  This is subtly different behavior from base64_encode_nopad. */
+  cookie_out[REND_DESC_COOKIE_LEN_BASE64] = '\0';
+  memwipe(extended_cookie, 0, sizeof(extended_cookie));
+  return cookie_out;
+}
+
+/** Decode a base64-encoded client authorization descriptor cookie.
+ * The descriptor_cookie can be truncated to REND_DESC_COOKIE_LEN_BASE64
+ * characters (as given to clients), or may include the two padding
+ * characters (as stored by the service).
+ *
+ * The result is stored in REND_DESC_COOKIE_LEN bytes of cookie_out.
+ * The rend_auth_type_t decoded from the cookie is stored in the
+ * optional auth_type_out parameter.
+ *
+ * Return 0 on success, or -1 on error.  The caller is responsible for
+ * freeing the returned err_msg.
+ */
+int
+rend_auth_decode_cookie(const char *cookie_in, uint8_t *cookie_out,
+                        rend_auth_type_t *auth_type_out, char **err_msg_out)
+{
+  uint8_t descriptor_cookie_decoded[REND_DESC_COOKIE_LEN_EXT + 1] = { 0 };
+  char descriptor_cookie_base64ext[REND_DESC_COOKIE_LEN_EXT_BASE64 + 1];
+  const char *descriptor_cookie = cookie_in;
+  char *err_msg = NULL;
+  int auth_type_val = 0;
+  int res = -1;
+  int decoded_len;
+
+  size_t len = strlen(descriptor_cookie);
+  if (len == REND_DESC_COOKIE_LEN_BASE64) {
+    /* Add a trailing zero byte to make base64-decoding happy. */
+    tor_snprintf(descriptor_cookie_base64ext,
+                 sizeof(descriptor_cookie_base64ext),
+                 "%sA=", descriptor_cookie);
+    descriptor_cookie = descriptor_cookie_base64ext;
+  } else if (len != REND_DESC_COOKIE_LEN_EXT_BASE64) {
+    tor_asprintf(&err_msg, "Authorization cookie has wrong length: %s",
+                 escaped(cookie_in));
+    goto err;
+  }
+
+  decoded_len = base64_decode((char *) descriptor_cookie_decoded,
+                              sizeof(descriptor_cookie_decoded),
+                              descriptor_cookie,
+                              REND_DESC_COOKIE_LEN_EXT_BASE64);
+  if (decoded_len != REND_DESC_COOKIE_LEN &&
+      decoded_len != REND_DESC_COOKIE_LEN_EXT) {
+    tor_asprintf(&err_msg, "Authorization cookie has invalid characters: %s",
+                 escaped(cookie_in));
+    goto err;
+  }
+
+  if (auth_type_out) {
+    auth_type_val = (descriptor_cookie_decoded[REND_DESC_COOKIE_LEN] >> 4) + 1;
+    if (auth_type_val < 1 || auth_type_val > 2) {
+      tor_asprintf(&err_msg, "Authorization cookie type is unknown: %s",
+                   escaped(cookie_in));
+      goto err;
+    }
+    *auth_type_out = auth_type_val == 1 ? REND_BASIC_AUTH : REND_STEALTH_AUTH;
+  }
+
+  memcpy(cookie_out, descriptor_cookie_decoded, REND_DESC_COOKIE_LEN);
+  res = 0;
+ err:
+  if (err_msg_out) {
+    *err_msg_out = err_msg;
+  } else {
+    tor_free(err_msg);
+  }
+  memwipe(descriptor_cookie_decoded, 0, sizeof(descriptor_cookie_decoded));
+  memwipe(descriptor_cookie_base64ext, 0, sizeof(descriptor_cookie_base64ext));
+  return res;
+}
+

+ 8 - 0
src/or/rendcommon.h

@@ -67,5 +67,13 @@ rend_data_t *rend_data_service_create(const char *onion_address,
                                       const char *pk_digest,
                                       const uint8_t *cookie,
                                       rend_auth_type_t auth_type);
+
+char *rend_auth_encode_cookie(const uint8_t *cookie_in,
+                              rend_auth_type_t auth_type);
+int rend_auth_decode_cookie(const char *cookie_in,
+                            uint8_t *cookie_out,
+                            rend_auth_type_t *auth_type_out,
+                            char **err_msg_out);
+
 #endif
 

+ 15 - 22
src/or/rendservice.c

@@ -1157,7 +1157,6 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname)
   strmap_t *parsed_clients = strmap_new();
   FILE *cfile, *hfile;
   open_file_t *open_cfile = NULL, *open_hfile = NULL;
-  char extended_desc_cookie[REND_DESC_COOKIE_LEN+1];
   char desc_cook_out[3*REND_DESC_COOKIE_LEN_BASE64+1];
   char service_id[16+1];
   char buf[1500];
@@ -1211,6 +1210,8 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname)
     } else {
       crypto_rand((char *) client->descriptor_cookie, REND_DESC_COOKIE_LEN);
     }
+    /* For compatibility with older tor clients, this does not
+     * truncate the padding characters, unlike rend_auth_encode_cookie.  */
     if (base64_encode(desc_cook_out, 3*REND_DESC_COOKIE_LEN_BASE64+1,
                       (char *) client->descriptor_cookie,
                       REND_DESC_COOKIE_LEN, 0) < 0) {
@@ -1273,6 +1274,8 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname)
         log_warn(LD_BUG, "Could not write client entry.");
         goto err;
       }
+    } else {
+      strlcpy(service_id, s->service_id, sizeof(service_id));
     }
 
     if (fputs(buf, cfile) < 0) {
@@ -1281,27 +1284,18 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname)
       goto err;
     }
 
-    /* Add line to hostname file. */
-    if (s->auth_type == REND_BASIC_AUTH) {
-      /* Remove == signs (newline has been removed above). */
-      desc_cook_out[strlen(desc_cook_out)-2] = '\0';
-      tor_snprintf(buf, sizeof(buf),"%s.onion %s # client: %s\n",
-                   s->service_id, desc_cook_out, client->client_name);
-    } else {
-      memcpy(extended_desc_cookie, client->descriptor_cookie,
-             REND_DESC_COOKIE_LEN);
-      extended_desc_cookie[REND_DESC_COOKIE_LEN] =
-        ((int)s->auth_type - 1) << 4;
-      if (base64_encode(desc_cook_out, 3*REND_DESC_COOKIE_LEN_BASE64+1,
-                        extended_desc_cookie,
-                        REND_DESC_COOKIE_LEN+1, 0) < 0) {
-        log_warn(LD_BUG, "Could not base64-encode descriptor cookie.");
-        goto err;
-      }
-      desc_cook_out[strlen(desc_cook_out)-2] = '\0'; /* Remove A=. */
-      tor_snprintf(buf, sizeof(buf),"%s.onion %s # client: %s\n",
-                   service_id, desc_cook_out, client->client_name);
+    /* Add line to hostname file. This is not the same encoding as in
+     * client_keys. */
+    char *encoded_cookie = rend_auth_encode_cookie(client->descriptor_cookie,
+                                                   s->auth_type);
+    if (!encoded_cookie) {
+      log_warn(LD_BUG, "Could not base64-encode descriptor cookie.");
+      goto err;
     }
+    tor_snprintf(buf, sizeof(buf), "%s.onion %s # client: %s\n",
+                 service_id, encoded_cookie, client->client_name);
+    memwipe(encoded_cookie, 0, strlen(encoded_cookie));
+    tor_free(encoded_cookie);
 
     if (fputs(buf, hfile)<0) {
       log_warn(LD_FS, "Could not append host entry to file: %s",
@@ -1333,7 +1327,6 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname)
   memwipe(buf, 0, sizeof(buf));
   memwipe(desc_cook_out, 0, sizeof(desc_cook_out));
   memwipe(service_id, 0, sizeof(service_id));
-  memwipe(extended_desc_cookie, 0, sizeof(extended_desc_cookie));
 
   return r;
 }

+ 6 - 16
src/or/routerparse.c

@@ -5290,6 +5290,7 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr)
   directory_token_t *tok;
   const char *current_entry = NULL;
   memarea_t *area = NULL;
+  char *err_msg = NULL;
   if (!ckstr || strlen(ckstr) == 0)
     return -1;
   tokens = smartlist_new();
@@ -5300,7 +5301,6 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr)
   while (!strcmpstart(current_entry, "client-name ")) {
     rend_authorized_client_t *parsed_entry;
     size_t len;
-    char descriptor_cookie_tmp[REND_DESC_COOKIE_LEN+2];
     /* Determine end of string. */
     const char *eos = strstr(current_entry, "\nclient-name ");
     if (!eos)
@@ -5356,23 +5356,13 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr)
     /* Parse descriptor cookie. */
     tok = find_by_keyword(tokens, C_DESCRIPTOR_COOKIE);
     tor_assert(tok->n_args == 1);
-    if (strlen(tok->args[0]) != REND_DESC_COOKIE_LEN_BASE64 + 2) {
-      log_warn(LD_REND, "Descriptor cookie has illegal length: %s",
-               escaped(tok->args[0]));
-      goto err;
-    }
-    /* The size of descriptor_cookie_tmp needs to be REND_DESC_COOKIE_LEN+2,
-     * because a base64 encoding of length 24 does not fit into 16 bytes in all
-     * cases. */
-    if (base64_decode(descriptor_cookie_tmp, sizeof(descriptor_cookie_tmp),
-                      tok->args[0], strlen(tok->args[0]))
-        != REND_DESC_COOKIE_LEN) {
-      log_warn(LD_REND, "Descriptor cookie contains illegal characters: "
-               "%s", escaped(tok->args[0]));
+    if (rend_auth_decode_cookie(tok->args[0], parsed_entry->descriptor_cookie,
+                                NULL, &err_msg) < 0) {
+      tor_assert(err_msg);
+      log_warn(LD_REND, "%s", err_msg);
+      tor_free(err_msg);
       goto err;
     }
-    memcpy(parsed_entry->descriptor_cookie, descriptor_cookie_tmp,
-           REND_DESC_COOKIE_LEN);
   }
   result = strmap_size(parsed_clients);
   goto done;

+ 63 - 0
src/test/test_hs.c

@@ -435,6 +435,67 @@ test_hs_rend_data(void *arg)
   rend_data_free(client_dup);
 }
 
+/* Test encoding and decoding service authorization cookies */
+static void
+test_hs_auth_cookies(void *arg)
+{
+#define TEST_COOKIE_RAW ((const uint8_t *) "abcdefghijklmnop")
+#define TEST_COOKIE_ENCODED "YWJjZGVmZ2hpamtsbW5vcA"
+#define TEST_COOKIE_ENCODED_STEALTH "YWJjZGVmZ2hpamtsbW5vcB"
+#define TEST_COOKIE_ENCODED_INVALID "YWJjZGVmZ2hpamtsbW5vcD"
+
+  char *encoded_cookie;
+  uint8_t raw_cookie[REND_DESC_COOKIE_LEN];
+  rend_auth_type_t auth_type;
+  char *err_msg;
+  int re;
+
+  (void)arg;
+
+  /* Test that encoding gives the expected result */
+  encoded_cookie = rend_auth_encode_cookie(TEST_COOKIE_RAW, REND_BASIC_AUTH);
+  tt_str_op(encoded_cookie, OP_EQ, TEST_COOKIE_ENCODED);
+  tor_free(encoded_cookie);
+
+  encoded_cookie = rend_auth_encode_cookie(TEST_COOKIE_RAW, REND_STEALTH_AUTH);
+  tt_str_op(encoded_cookie, OP_EQ, TEST_COOKIE_ENCODED_STEALTH);
+  tor_free(encoded_cookie);
+
+  /* Decoding should give the original value */
+  re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED, raw_cookie, &auth_type,
+                               &err_msg);
+  tt_assert(!re);
+  tt_assert(!err_msg);
+  tt_mem_op(raw_cookie, OP_EQ, TEST_COOKIE_RAW, REND_DESC_COOKIE_LEN);
+  tt_int_op(auth_type, OP_EQ, REND_BASIC_AUTH);
+  memset(raw_cookie, 0, sizeof(raw_cookie));
+
+  re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED_STEALTH, raw_cookie,
+                               &auth_type, &err_msg);
+  tt_assert(!re);
+  tt_assert(!err_msg);
+  tt_mem_op(raw_cookie, OP_EQ, TEST_COOKIE_RAW, REND_DESC_COOKIE_LEN);
+  tt_int_op(auth_type, OP_EQ, REND_STEALTH_AUTH);
+  memset(raw_cookie, 0, sizeof(raw_cookie));
+
+  /* Decoding with padding characters should also work */
+  re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED "==", raw_cookie, NULL,
+                               &err_msg);
+  tt_assert(!re);
+  tt_assert(!err_msg);
+  tt_mem_op(raw_cookie, OP_EQ, TEST_COOKIE_RAW, REND_DESC_COOKIE_LEN);
+
+  /* Decoding with an unknown type should fail */
+  re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED_INVALID, raw_cookie,
+                               &auth_type, &err_msg);
+  tt_int_op(re, OP_LT, 0);
+  tt_assert(err_msg);
+  tor_free(err_msg);
+
+ done:
+  return;
+}
+
 struct testcase_t hs_tests[] = {
   { "hs_rend_data", test_hs_rend_data, TT_FORK,
     NULL, NULL },
@@ -445,6 +506,8 @@ struct testcase_t hs_tests[] = {
   { "pick_bad_tor2web_rendezvous_node",
     test_pick_bad_tor2web_rendezvous_node, TT_FORK,
     NULL, NULL },
+  { "hs_auth_cookies", test_hs_auth_cookies, TT_FORK,
+    NULL, NULL },
   END_OF_TESTCASES
 };