commit 88c2a6b9361d7d624f9d34dc855b940554a05fb3 Author: Nick Mathewson <nickm@torproject.org> Date: Tue Aug 30 10:33:57 2016 -0400 Send and receive AUTHENTICATE cells correctly with ED keys. Includes updated test for authchallenge cells --- src/or/channeltls.c | 71 +++++++++++++++++++++++++++++++++--------- src/or/connection_or.c | 49 ++++++++++++++++++++++++----- src/or/connection_or.h | 11 ++++--- src/test/test_link_handshake.c | 14 ++++----- 4 files changed, 112 insertions(+), 33 deletions(-) diff --git a/src/or/channeltls.c b/src/or/channeltls.c index 6de3bd4..e5e82dd 100644 --- a/src/or/channeltls.c +++ b/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) - use_type = authtype; + if (authchallenge_type_is_supported(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); diff --git a/src/or/connection_or.c b/src/or/connection_or.c index b922e97..37af617 100644 --- a/src/or/connection_or.c +++ b/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, - int server) + const ed25519_keypair_t *ed_signing_key, + 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, - pk, - NULL, - 0 /* not server */); + authtype, + pk, + get_current_auth_keypair(), + 0 /* not server */); if (! cell) { log_warn(LD_BUG, "Unable to compute authenticate cell!"); return -1; diff --git a/src/or/connection_or.h b/src/or/connection_or.h index 7fdfbb0..da95718 100644 --- a/src/or/connection_or.h +++ b/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, - crypto_pk_t *signing_key, - ed25519_keypair_t *ed_signing_key, - int server); + const int authtype, + crypto_pk_t *signing_key, + const ed25519_keypair_t *ed_signing_key, + int server); MOCK_DECL(int,connection_or_send_authenticate_cell, (or_connection_t *conn, int type)); diff --git a/src/test/test_link_handshake.c b/src/test/test_link_handshake.c index 05c8400..7bf6e10 100644 --- a/src/test/test_link_handshake.c +++ b/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(36, ==, cell2->payload_len); + tt_int_op(40, ==, cell1->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\x01\x00\x01", ==, cell2->payload + 32, 4); + tt_mem_op("\x00\x03\x00\x01\x00\x02\x00\x03", ==, cell1->payload + 32, 8); + 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 " - "certificate"; + require_failure_message = "We never got an RSA " + "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) \