commit 348b90a915a5867bc0d8888e0fd12e8ec2319628 Author: Nick Mathewson nickm@torproject.org Date: Thu May 21 14:28:12 2015 -0400

Refactor RSA certificate checking into its own function. --- src/or/channeltls.c | 61 +++++++++++++++++------------------------- src/or/connection_or.c | 1 + src/or/or.h | 4 +++ src/or/torcert.c | 47 ++++++++++++++++++++++++++++++++ src/or/torcert.h | 4 +++ src/test/test_link_handshake.c | 11 ++++++-- 6 files changed, 89 insertions(+), 39 deletions(-)

diff --git a/src/or/channeltls.c b/src/or/channeltls.c index fbe784c..cf57c29 100644 --- a/src/or/channeltls.c +++ b/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; diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 5a9c597..2591b40 100644 --- a/src/or/connection_or.c +++ b/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; }

diff --git a/src/or/or.h b/src/or/or.h index cdde448..5feba28 100644 --- a/src/or/or.h +++ b/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 */ diff --git a/src/or/torcert.c b/src/or/torcert.c index e8bee54..2f0cc53 100644 --- a/src/or/torcert.c +++ b/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; +} diff --git a/src/or/torcert.h b/src/or/torcert.h index 3f81fcd..0420b41 100644 --- a/src/or/torcert.h +++ b/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

diff --git a/src/test/test_link_handshake.c b/src/test/test_link_handshake.c index 3314880..91c6299 100644 --- a/src/test/test_link_handshake.c +++ b/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)