[tor-commits] [tor/master] Send and receive AUTHENTICATE cells correctly with ED keys.
nickm at torproject.org
nickm at torproject.org
Thu Nov 3 13:18:59 UTC 2016
commit 88c2a6b9361d7d624f9d34dc855b940554a05fb3
Author: Nick Mathewson <nickm at 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) \
More information about the tor-commits
mailing list