Send and receive AUTHENTICATE cells correctly with ED keys.

Includes updated test for authchallenge cells

src/or/channeltls.c

@@ -2071,8 +2071,12 @@ channel_tls_process_auth_challenge_cell(var_cell_t *cell, channel_tls_t *chan)
   /* Now see if there is an authentication type we can use */
   for (i = 0; i < n_types; ++i) {
     uint16_t authtype = auth_challenge_cell_get_methods(ac, i);
-    if (authtype == AUTHTYPE_RSA_SHA256_TLSSECRET)
+    if (authchallenge_type_is_supported(authtype)) {
-      use_type = authtype;
+      if (use_type == -1 ||
+          authchallenge_type_is_better(authtype, use_type)) {
+        use_type = authtype;
+      }
+    }
   }
 
   chan->conn->handshake_state->received_auth_challenge = 1;
@@ -2087,9 +2091,10 @@ channel_tls_process_auth_challenge_cell(var_cell_t *cell, channel_tls_t *chan)
   if (use_type >= 0) {
     log_info(LD_OR,
              "Got an AUTH_CHALLENGE cell from %s:%d: Sending "
-             "authentication",
+             "authentication type %d",
              safe_str(chan->conn->base_.address),
-             chan->conn->base_.port);
+             chan->conn->base_.port,
+             use_type);
 
     if (connection_or_send_authenticate_cell(chan->conn, use_type) < 0) {
       log_warn(LD_OR,
@@ -2133,7 +2138,7 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
   var_cell_t *expected_cell = NULL;
   const uint8_t *auth;
   int authlen;
-  const int authtype = 1; /* XXXX extend this */
+  int authtype;
   int bodylen;
 
   tor_assert(cell);
@@ -2165,8 +2170,6 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
   }
   if (!(chan->conn->handshake_state->received_certs_cell))
     ERR("We never got a certs cell");
-  if (chan->conn->handshake_state->certs->auth_cert == NULL)
-    ERR("We never got an authentication certificate");
   if (chan->conn->handshake_state->certs->id_cert == NULL)
     ERR("We never got an identity certificate");
   if (cell->payload_len < 4)
@@ -2179,8 +2182,9 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
     if (4 + len > cell->payload_len)
       ERR("Authenticator was truncated");
 
-    if (type != authtype)
+    if (! authchallenge_type_is_supported(type))
       ERR("Authenticator type was not recognized");
+    authtype = type;
 
     auth += 4;
     authlen = len;
@@ -2194,11 +2198,18 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
   if (! expected_cell)
     ERR("Couldn't compute expected AUTHENTICATE cell body");
 
+  int sig_is_rsa;
   if (authtype == AUTHTYPE_RSA_SHA256_TLSSECRET ||
       authtype == AUTHTYPE_RSA_SHA256_RFC5705) {
     bodylen = V3_AUTH_BODY_LEN;
+    sig_is_rsa = 1;
   } else {
-    bodylen = authlen - ED25519_SIG_LEN; /* XXXX  DOCDOC */
+    tor_assert(authtype == AUTHTYPE_ED25519_SHA256_RFC5705);
+    /* Our earlier check had better have made sure we had room
+     * for an ed25519 sig (inadvertently) */
+    tor_assert(V3_AUTH_BODY_LEN > ED25519_SIG_LEN);
+    bodylen = authlen - ED25519_SIG_LEN;
+    sig_is_rsa = 0;
   }
   if (expected_cell->payload_len != bodylen+4) {
     ERR("Expected AUTHENTICATE cell body len not as expected.");
@@ -2211,9 +2222,15 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
   if (tor_memneq(expected_cell->payload+4, auth, bodylen-24))
     ERR("Some field in the AUTHENTICATE cell body was not as expected");
 
-  {
+  if (sig_is_rsa) {
+    if (chan->conn->handshake_state->certs->ed_id_sign != NULL)
+      ERR("RSA-signed AUTHENTICATE response provided with an ED25519 cert");
+
+    if (chan->conn->handshake_state->certs->auth_cert == NULL)
+      ERR("We never got an RSA authentication certificate");
+
     crypto_pk_t *pk = tor_tls_cert_get_key(
-                                   chan->conn->handshake_state->certs->auth_cert);
+                             chan->conn->handshake_state->certs->auth_cert);
     char d[DIGEST256_LEN];
     char *signed_data;
     size_t keysize;
@@ -2231,7 +2248,7 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
     crypto_pk_free(pk);
     if (signed_len < 0) {
       tor_free(signed_data);
-      ERR("Signature wasn't valid");
+      ERR("RSA signature wasn't valid");
     }
     if (signed_len < DIGEST256_LEN) {
       tor_free(signed_data);
@@ -2244,6 +2261,20 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
       ERR("Signature did not match data to be signed.");
     }
     tor_free(signed_data);
+  } else {
+    if (chan->conn->handshake_state->certs->ed_id_sign == NULL)
+      ERR("We never got an Ed25519 identity certificate.");
+    if (chan->conn->handshake_state->certs->ed_sign_auth == NULL)
+      ERR("We never got an Ed25519 authentication certificate.");
+
+    const ed25519_public_key_t *authkey =
+      &chan->conn->handshake_state->certs->ed_sign_auth->signed_key;
+    ed25519_signature_t sig;
+    tor_assert(authlen > ED25519_SIG_LEN);
+    memcpy(&sig.sig, auth + authlen - ED25519_SIG_LEN, ED25519_SIG_LEN);
+    if (ed25519_checksig(&sig, auth, authlen - ED25519_SIG_LEN, authkey)<0) {
+      ERR("Ed25519 signature wasn't valid.");
+    }
   }
 
   /* Okay, we are authenticated. */
@@ -2256,6 +2287,15 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
       tor_tls_cert_get_key(chan->conn->handshake_state->certs->id_cert);
     const common_digests_t *id_digests =
       tor_x509_cert_get_id_digests(chan->conn->handshake_state->certs->id_cert);
+    const ed25519_public_key_t *ed_identity_received = NULL;
+
+    if (! sig_is_rsa) {
+      chan->conn->handshake_state->authenticated_ed25519 = 1;
+      ed_identity_received =
+        &chan->conn->handshake_state->certs->ed_id_sign->signing_key;
+      memcpy(&chan->conn->handshake_state->authenticated_ed25519_peer_id,
+             ed_identity_received, sizeof(ed25519_public_key_t));
+    }
 
     /* This must exist; we checked key type when reading the cert. */
     tor_assert(id_digests);
@@ -2272,13 +2312,14 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
                   chan->conn->base_.port,
                   (const char*)(chan->conn->handshake_state->
                     authenticated_rsa_peer_id),
-                  NULL, // XXXX Ed key
+                  ed_identity_received,
                   0);
 
     log_info(LD_OR,
-             "Got an AUTHENTICATE cell from %s:%d: Looks good.",
+             "Got an AUTHENTICATE cell from %s:%d, type %d: Looks good.",
              safe_str(chan->conn->base_.address),
-             chan->conn->base_.port);
+             chan->conn->base_.port,
+             authtype);
   }
 
   var_cell_free(expected_cell);

src/or/connection_or.c

@@ -2271,6 +2271,38 @@ connection_or_send_certs_cell(or_connection_t *conn)
   return 0;
 }
 
+/** Return true iff <b>challenge_type</b> is an AUTHCHALLENGE type that
+ * we can send and receive. */
+int
+authchallenge_type_is_supported(uint16_t challenge_type)
+{
+  switch (challenge_type) {
+     case AUTHTYPE_RSA_SHA256_TLSSECRET:
+     case AUTHTYPE_RSA_SHA256_RFC5705:
+     case AUTHTYPE_ED25519_SHA256_RFC5705:
+       return 1;
+     default:
+       return 0;
+  }
+}
+
+/** Return true iff <b>challenge_type_a</b> is one that we would rather
+ * use than <b>challenge_type_b</b>. */
+int
+authchallenge_type_is_better(uint16_t challenge_type_a,
+                             uint16_t challenge_type_b)
+{
+  /* Any supported type is better than an unsupported one;
+   * all unsupported types are equally bad. */
+  if (!authchallenge_type_is_supported(challenge_type_a))
+    return 0;
+  if (!authchallenge_type_is_supported(challenge_type_b))
+    return 1;
+  /* It happens that types are superior in numerically ascending order.
+   * If that ever changes, this must change too. */
+  return (challenge_type_a > challenge_type_b);
+}
+
 /** Send an AUTH_CHALLENGE cell on the connection <b>conn</b>. Return 0
  * on success, -1 on failure. */
 int
@@ -2285,9 +2317,12 @@ connection_or_send_auth_challenge_cell(or_connection_t *conn)
 
   auth_challenge_cell_t *ac = auth_challenge_cell_new();
 
+  tor_assert(sizeof(ac->challenge) == 32);
   crypto_rand((char*)ac->challenge, sizeof(ac->challenge));
 
   auth_challenge_cell_add_methods(ac, AUTHTYPE_RSA_SHA256_TLSSECRET);
+  auth_challenge_cell_add_methods(ac, AUTHTYPE_RSA_SHA256_RFC5705);
+  auth_challenge_cell_add_methods(ac, AUTHTYPE_ED25519_SHA256_RFC5705);
   auth_challenge_cell_set_n_methods(ac,
                                     auth_challenge_cell_getlen_methods(ac));
 
@@ -2330,8 +2365,8 @@ var_cell_t *
 connection_or_compute_authenticate_cell_body(or_connection_t *conn,
                                              const int authtype,
                                              crypto_pk_t *signing_key,
-                                             ed25519_keypair_t *ed_signing_key,
+                                      const ed25519_keypair_t *ed_signing_key,
-                                             int server)
+                                      int server)
 {
   auth1_t *auth = NULL;
   auth_ctx_t *ctx = auth_ctx_new();
@@ -2559,17 +2594,17 @@ connection_or_send_authenticate_cell,(or_connection_t *conn, int authtype))
     log_warn(LD_BUG, "Can't compute authenticate cell: no client auth key");
     return -1;
   }
-  if (authtype != AUTHTYPE_RSA_SHA256_TLSSECRET) {
+  if (! authchallenge_type_is_supported(authtype)) {
     log_warn(LD_BUG, "Tried to send authenticate cell with unknown "
              "authentication type %d", authtype);
     return -1;
   }
 
   cell = connection_or_compute_authenticate_cell_body(conn,
-                                             AUTHTYPE_RSA_SHA256_TLSSECRET,
+                                                 authtype,
-                                                         pk,
+                                                 pk,
-                                                         NULL,
+                                                 get_current_auth_keypair(),
-                                                         0 /* not server */);
+                                                 0 /* not server */);
   if (! cell) {
     log_warn(LD_BUG, "Unable to compute authenticate cell!");
     return -1;

src/or/connection_or.h

@@ -88,11 +88,14 @@ int connection_or_send_versions(or_connection_t *conn, int v3_plus);
 MOCK_DECL(int,connection_or_send_netinfo,(or_connection_t *conn));
 int connection_or_send_certs_cell(or_connection_t *conn);
 int connection_or_send_auth_challenge_cell(or_connection_t *conn);
+int authchallenge_type_is_supported(uint16_t challenge_type);
+int authchallenge_type_is_better(uint16_t challenge_type_a,
+                                 uint16_t challenge_type_b);
 var_cell_t *connection_or_compute_authenticate_cell_body(or_connection_t *conn,
-                                             const int authtype,
+                                       const int authtype,
-                                             crypto_pk_t *signing_key,
+                                       crypto_pk_t *signing_key,
-                                             ed25519_keypair_t *ed_signing_key,
+                                       const ed25519_keypair_t *ed_signing_key,
-                                             int server);
+                                       int server);
 MOCK_DECL(int,connection_or_send_authenticate_cell,
           (or_connection_t *conn, int type));
 

src/test/test_link_handshake.c

@@ -484,15 +484,15 @@ test_link_handshake_send_authchallenge(void *arg)
   cell1 = mock_got_var_cell;
   tt_int_op(0, ==, connection_or_send_auth_challenge_cell(c1));
   cell2 = mock_got_var_cell;
-  tt_int_op(36, ==, cell1->payload_len);
+  tt_int_op(40, ==, cell1->payload_len);
-  tt_int_op(36, ==, cell2->payload_len);
+  tt_int_op(40, ==, cell2->payload_len);
   tt_int_op(0, ==, cell1->circ_id);
   tt_int_op(0, ==, cell2->circ_id);
   tt_int_op(CELL_AUTH_CHALLENGE, ==, cell1->command);
   tt_int_op(CELL_AUTH_CHALLENGE, ==, cell2->command);
 
-  tt_mem_op("\x00\x01\x00\x01", ==, cell1->payload + 32, 4);
+  tt_mem_op("\x00\x03\x00\x01\x00\x02\x00\x03", ==, cell1->payload + 32, 8);
-  tt_mem_op("\x00\x01\x00\x01", ==, cell2->payload + 32, 4);
+  tt_mem_op("\x00\x03\x00\x01\x00\x02\x00\x03", ==, cell2->payload + 32, 8);
   tt_mem_op(cell1->payload, !=, cell2->payload, 32);
 
  done:
@@ -909,8 +909,8 @@ AUTHENTICATE_FAIL(noidcert,
                   tor_x509_cert_free(d->c2->handshake_state->certs->id_cert);
                   d->c2->handshake_state->certs->id_cert = NULL)
 AUTHENTICATE_FAIL(noauthcert,
-                  require_failure_message = "We never got an authentication "
+                  require_failure_message = "We never got an RSA "
-                    "certificate";
+                    "authentication certificate";
                   tor_x509_cert_free(d->c2->handshake_state->certs->auth_cert);
                   d->c2->handshake_state->certs->auth_cert = NULL)
 AUTHENTICATE_FAIL(tooshort,
@@ -936,7 +936,7 @@ AUTHENTICATE_FAIL(badcontent,
                     "cell body was not as expected";
                   d->cell->payload[10] ^= 0xff)
 AUTHENTICATE_FAIL(badsig_1,
-                  require_failure_message = "Signature wasn't valid";
+                  require_failure_message = "RSA signature wasn't valid";
                   d->cell->payload[d->cell->payload_len - 5] ^= 0xff)
 
 #define TEST(name, flags)                                       \