Browse Source

Refactor RSA certificate checking into its own function.

Nick Mathewson 9 years ago
parent
commit
348b90a915
6 changed files with 89 additions and 39 deletions
  1. 24 37
      src/or/channeltls.c
  2. 1 0
      src/or/connection_or.c
  3. 4 0
      src/or/or.h
  4. 47 0
      src/or/torcert.c
  5. 4 0
      src/or/torcert.h
  6. 9 2
      src/test/test_link_handshake.c

+ 24 - 37
src/or/channeltls.c

@@ -1899,27 +1899,30 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan)
   tor_x509_cert_t *auth_cert = x509_certs[OR_CERT_TYPE_AUTH_1024];
   tor_x509_cert_t *link_cert = x509_certs[OR_CERT_TYPE_TLS_LINK];
 
+  chan->conn->handshake_state->certs->auth_cert = auth_cert;
+  chan->conn->handshake_state->certs->link_cert = link_cert;
+  chan->conn->handshake_state->certs->id_cert = id_cert;
+  x509_certs[OR_CERT_TYPE_ID_1024] =
+    x509_certs[OR_CERT_TYPE_AUTH_1024] =
+    x509_certs[OR_CERT_TYPE_TLS_LINK] = NULL;
+
+  int severity;
+  /* Note that this warns more loudly about time and validity if we were
+   * _trying_ to connect to an authority, not necessarily if we _did_ connect
+   * to one. */
+  if (chan->conn->handshake_state->started_here &&
+      router_digest_is_trusted_dir(TLS_CHAN_TO_BASE(chan)->identity_digest))
+    severity = LOG_WARN;
+  else
+    severity = LOG_PROTOCOL_WARN;
+
+  if (! or_handshake_certs_rsa_ok(severity,
+                                  chan->conn->handshake_state->certs,
+                                  chan->conn->tls))
+    ERR("Invalid RSA certificates!");
+
   if (chan->conn->handshake_state->started_here) {
-    int severity;
-    if (! (id_cert && link_cert))
-      ERR("The certs we wanted were missing");
-    /* Okay. We should be able to check the certificates now. */
-    if (! tor_tls_cert_matches_key(chan->conn->tls, link_cert)) {
-      ERR("The link certificate didn't match the TLS public key");
-    }
-    /* Note that this warns more loudly about time and validity if we were
-    * _trying_ to connect to an authority, not necessarily if we _did_ connect
-    * to one. */
-    if (router_digest_is_trusted_dir(
-          TLS_CHAN_TO_BASE(chan)->identity_digest))
-      severity = LOG_WARN;
-    else
-      severity = LOG_PROTOCOL_WARN;
-
-    if (! tor_tls_cert_is_valid(severity, link_cert, id_cert, 0))
-      ERR("The link certificate was not valid");
-    if (! tor_tls_cert_is_valid(severity, id_cert, id_cert, 1))
-      ERR("The ID certificate was not valid");
+    /* No more information is needed. */
 
     chan->conn->handshake_state->authenticated = 1;
     {
@@ -1947,9 +1950,6 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan)
              "Got some good certificates from %s:%d: Authenticated it.",
              safe_str(chan->conn->base_.address), chan->conn->base_.port);
 
-    chan->conn->handshake_state->certs->id_cert = id_cert;
-    x509_certs[OR_CERT_TYPE_ID_1024] = NULL;
-
     if (!public_server_mode(get_options())) {
       /* If we initiated the connection and we are not a public server, we
        * aren't planning to authenticate at all.  At this point we know who we
@@ -1957,26 +1957,13 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan)
       send_netinfo = 1;
     }
   } else {
-    if (! (id_cert && auth_cert))
-      ERR("The certs we wanted were missing");
-
-    /* Remember these certificates so we can check an AUTHENTICATE cell */
-    if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, auth_cert, id_cert, 1))
-      ERR("The authentication certificate was not valid");
-    if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, id_cert, id_cert, 1))
-      ERR("The ID certificate was not valid");
-
+    /* We can't call it authenticated till we see an AUTHENTICATE cell. */
     log_info(LD_OR,
              "Got some good certificates from %s:%d: "
              "Waiting for AUTHENTICATE.",
              safe_str(chan->conn->base_.address),
              chan->conn->base_.port);
     /* XXXX check more stuff? */
-
-    chan->conn->handshake_state->certs->id_cert = id_cert;
-    chan->conn->handshake_state->certs->auth_cert = auth_cert;
-    x509_certs[OR_CERT_TYPE_ID_1024] = x509_certs[OR_CERT_TYPE_AUTH_1024]
-      = NULL;
   }
 
   chan->conn->handshake_state->received_certs_cell = 1;

+ 1 - 0
src/or/connection_or.c

@@ -1765,6 +1765,7 @@ connection_init_or_handshake_state(or_connection_t *conn, int started_here)
   s->digest_sent_data = 1;
   s->digest_received_data = 1;
   s->certs = or_handshake_certs_new();
+  s->certs->started_here = s->started_here;
   return 0;
 }
 

+ 4 - 0
src/or/or.h

@@ -1387,8 +1387,12 @@ typedef struct listener_connection_t {
 #define V3_AUTH_BODY_LEN (V3_AUTH_FIXED_PART_LEN + 8 + 16)
 
 typedef struct or_handshake_certs_t {
+  /** DOCDOC */
+  int started_here;
   /** The cert for the key that's supposed to sign the AUTHENTICATE cell */
   tor_x509_cert_t *auth_cert;
+  /** DOCDOC */
+  tor_x509_cert_t *link_cert;
   /** A self-signed identity certificate */
   tor_x509_cert_t *id_cert;
   /** DOCDOC */

+ 47 - 0
src/or/torcert.c

@@ -9,6 +9,7 @@
  */
 
 #include "or.h"
+#include "config.h"
 #include "crypto.h"
 #include "torcert.h"
 #include "ed25519_cert.h"
@@ -315,3 +316,49 @@ or_handshake_certs_free(or_handshake_certs_t *certs)
   memwipe(certs, 0xBD, sizeof(*certs));
   tor_free(certs);
 }
+
+#define ERR(s)                                                  \
+  do {                                                          \
+    log_fn(severity, LD_PROTOCOL,                               \
+           "Received a bad CERTS cell: %s",                     \
+           (s));                                                \
+    return 0;                                                   \
+  } while (0)
+
+int
+or_handshake_certs_rsa_ok(int severity,
+                          or_handshake_certs_t *certs,
+                          tor_tls_t *tls)
+{
+  tor_x509_cert_t *link_cert = certs->link_cert;
+  tor_x509_cert_t *auth_cert = certs->auth_cert;
+  tor_x509_cert_t *id_cert = certs->id_cert;
+
+  if (certs->started_here) {
+    if (! (id_cert && link_cert))
+      ERR("The certs we wanted were missing");
+    if (! tor_tls_cert_matches_key(tls, link_cert))
+      ERR("The link certificate didn't match the TLS public key");
+    if (! tor_tls_cert_is_valid(severity, link_cert, id_cert, 0))
+      ERR("The link certificate was not valid");
+    if (! tor_tls_cert_is_valid(severity, id_cert, id_cert, 1))
+      ERR("The ID certificate was not valid");
+  } else {
+    if (! (id_cert && auth_cert))
+      ERR("The certs we wanted were missing");
+    /* Remember these certificates so we can check an AUTHENTICATE cell */
+    if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, auth_cert, id_cert, 1))
+      ERR("The authentication certificate was not valid");
+    if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, id_cert, id_cert, 1))
+      ERR("The ID certificate was not valid");
+  }
+
+  return 1;
+}
+
+int
+or_handshake_certs_ed25519_ok(or_handshake_certs_t *certs)
+{
+  (void) certs;
+  return 0;
+}

+ 4 - 0
src/or/torcert.h

@@ -74,6 +74,10 @@ ssize_t tor_make_rsa_ed25519_crosscert(const ed25519_public_key_t *ed_key,
 
 or_handshake_certs_t *or_handshake_certs_new(void);
 void or_handshake_certs_free(or_handshake_certs_t *certs);
+int or_handshake_certs_rsa_ok(int severity,
+                              or_handshake_certs_t *certs,
+                              tor_tls_t *tls);
+int or_handshake_certs_ed25519_ok(or_handshake_certs_t *certs);
 
 #endif
 

+ 9 - 2
src/test/test_link_handshake.c

@@ -315,6 +315,7 @@ test_link_handshake_recv_certs_ok_server(void *arg)
 {
   certs_data_t *d = arg;
   d->c->handshake_state->started_here = 0;
+  d->c->handshake_state->certs->started_here = 0;
   certs_cell_get_certs(d->ccell, 0)->cert_type = 3;
   certs_cell_get_certs(d->ccell, 1)->cert_type = 2;
   ssize_t n = certs_cell_encode(d->cell->payload, 2048, d->ccell);
@@ -450,17 +451,21 @@ CERTS_FAIL(server_missing_certs,
            {
              require_failure_message = "The certs we wanted were missing";
              d->c->handshake_state->started_here = 0;
+             d->c->handshake_state->certs->started_here = 0;
+
            })
 CERTS_FAIL(server_wrong_labels_1,
            {
              require_failure_message =
                "The authentication certificate was not valid";
              d->c->handshake_state->started_here = 0;
+             d->c->handshake_state->certs->started_here = 0;
              certs_cell_get_certs(d->ccell, 0)->cert_type = 2;
              certs_cell_get_certs(d->ccell, 1)->cert_type = 3;
              REENCODE();
            })
 
+
 static void
 test_link_handshake_send_authchallenge(void *arg)
 {
@@ -637,7 +642,8 @@ AUTHCHALLENGE_FAIL(badproto,
 AUTHCHALLENGE_FAIL(as_server,
                    require_failure_message = "We didn't originate this "
                      "connection";
-                   d->c->handshake_state->started_here = 0;)
+                   d->c->handshake_state->started_here = 0;
+                   d->c->handshake_state->certs->started_here = 0;)
 AUTHCHALLENGE_FAIL(duplicate,
                    require_failure_message = "We already received one";
                    d->c->handshake_state->received_auth_challenge = 1)
@@ -874,7 +880,8 @@ AUTHENTICATE_FAIL(badproto,
                   d->c2->link_proto = 2)
 AUTHENTICATE_FAIL(atclient,
                   require_failure_message = "We originated this connection";
-                  d->c2->handshake_state->started_here = 1)
+                  d->c2->handshake_state->started_here = 1;
+                  d->c2->handshake_state->certs->started_here = 1;)
 AUTHENTICATE_FAIL(duplicate,
                   require_failure_message = "We already got one";
                   d->c2->handshake_state->received_authenticate = 1)