commit af2459f09e86aece0fb0679f7e8abd8f2439ed80 Author: Nick Mathewson <nickm@torproject.org> Date: Sat Sep 10 11:30:33 2016 -0400 Unit tests for cert-chain-processing, including failed cases Check out the coverage! --- src/or/channeltls.c | 5 +- src/or/routerkeys.c | 10 ++ src/or/routerkeys.h | 1 + src/or/torcert.c | 27 +++-- src/test/test_link_handshake.c | 265 ++++++++++++++++++++++++++++++++++++++++- 5 files changed, 292 insertions(+), 16 deletions(-) diff --git a/src/or/channeltls.c b/src/or/channeltls.c index e5e82dd..7a6f0b3 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -1954,8 +1954,11 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan) ERR("Couldn't compute digests for key in ID cert"); identity_rcvd = tor_tls_cert_get_key(id_cert); - if (!identity_rcvd) + if (!identity_rcvd) { + //LCOV_EXCL_START ERR("Internal error: Couldn't get RSA key from ID cert."); + //LCOV_EXCL_STOP + } memcpy(chan->conn->handshake_state->authenticated_rsa_peer_id, id_digests->d[DIGEST_SHA1], DIGEST_LEN); channel_set_circid_type(TLS_CHAN_TO_BASE(chan), identity_rcvd, diff --git a/src/or/routerkeys.c b/src/or/routerkeys.c index 88d091a..f0f6252 100644 --- a/src/or/routerkeys.c +++ b/src/or/routerkeys.c @@ -1059,6 +1059,16 @@ get_master_identity_key(void) return &master_identity_key->pubkey; } +#ifdef TOR_UNIT_TESTS +/* only exists for the unit tests, since otherwise the identity key + * should be used to sign nothing but the signing key. */ +const ed25519_keypair_t * +get_master_identity_keypair(void) +{ + return master_identity_key; +} +#endif + const ed25519_keypair_t * get_master_signing_keypair(void) { diff --git a/src/or/routerkeys.h b/src/or/routerkeys.h index c2b20b3..307a1cd 100644 --- a/src/or/routerkeys.h +++ b/src/or/routerkeys.h @@ -74,6 +74,7 @@ int write_encrypted_secret_key(const ed25519_secret_key_t *out, void routerkeys_free_all(void); #ifdef TOR_UNIT_TESTS +const ed25519_keypair_t *get_master_identity_keypair(void); void init_mock_ed_keys(const crypto_pk_t *rsa_identity_key); #endif diff --git a/src/or/torcert.c b/src/or/torcert.c index cff1ed1..cfd2210 100644 --- a/src/or/torcert.c +++ b/src/or/torcert.c @@ -461,7 +461,7 @@ or_handshake_certs_rsa_ok(int severity, if (certs->started_here) { if (! (id_cert && link_cert)) - ERR("The certs we wanted were missing"); + ERR("The certs we wanted (ID, Link) 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, now, 0)) @@ -470,7 +470,7 @@ or_handshake_certs_rsa_ok(int severity, ERR("The ID certificate was not valid"); } else { if (! (id_cert && auth_cert)) - ERR("The certs we wanted were missing"); + ERR("The certs we wanted (ID, Auth) were missing"); /* Remember these certificates so we can check an AUTHENTICATE cell * XXXX make sure we do that */ @@ -505,20 +505,20 @@ or_handshake_certs_ed25519_ok(int severity, ERR("Could not get checkable cert."); \ } while (0) - if (! certs->ed_id_sign || !certs->ed_id_sign->signing_key_included) - ERR("No signing key"); + if (! certs->ed_id_sign || !certs->ed_id_sign->signing_key_included) { + ERR("No Ed25519 signing key"); + } ADDCERT(certs->ed_id_sign, NULL); if (certs->started_here) { if (! certs->ed_sign_link) - ERR("No link key"); + ERR("No Ed25519 link key"); { /* check for a match with the TLS cert. */ tor_x509_cert_t *peer_cert = tor_tls_get_peer_cert(tls); - /* XXXX Does 'cert' match spec in this case? I hope so; if not, fix - * spec */ - if (!peer_cert) - ERR("No x509 peer cert"); + if (BUG(!peer_cert)) { + ERR("No x509 peer cert"); // LCOV_EXCL_LINE + } const common_digests_t *peer_cert_digests = tor_x509_cert_get_cert_digests(peer_cert); int okay = tor_memeq(peer_cert_digests->d[DIGEST_SHA256], @@ -526,14 +526,14 @@ or_handshake_certs_ed25519_ok(int severity, DIGEST256_LEN); tor_x509_cert_free(peer_cert); if (!okay) - ERR("link certificate does not match TLS certificate"); + ERR("Link certificate does not match TLS certificate"); } ADDCERT(certs->ed_sign_link, &certs->ed_id_sign->signed_key); } else { if (! certs->ed_sign_auth) - ERR("No link authentiction key"); + ERR("No Ed25519 link authentication key"); ADDCERT(certs->ed_sign_auth, &certs->ed_id_sign->signed_key); } @@ -555,6 +555,9 @@ or_handshake_certs_ed25519_ok(int severity, if (! tor_tls_cert_is_valid(severity, rsa_id_cert, rsa_id_cert, now, 1)) { ERR("The legacy RSA ID certificate was not valid"); } + if (! certs->ed_rsa_crosscert) { + ERR("Missing RSA->Ed25519 crosscert"); + } crypto_pk_t *rsa_id_key = tor_tls_cert_get_key(rsa_id_cert); if (rsa_ed25519_crosscert_check(certs->ed_rsa_crosscert, @@ -563,7 +566,7 @@ or_handshake_certs_ed25519_ok(int severity, &certs->ed_id_sign->signing_key, now) < 0) { crypto_pk_free(rsa_id_key); - ERR("Invalid/missing RSA crosscert"); + ERR("Invalid RSA->Ed25519 crosscert"); } crypto_pk_free(rsa_id_key); rsa_id_key = NULL; diff --git a/src/test/test_link_handshake.c b/src/test/test_link_handshake.c index d427973..04bfb5a 100644 --- a/src/test/test_link_handshake.c +++ b/src/test/test_link_handshake.c @@ -6,12 +6,16 @@ #define CHANNELTLS_PRIVATE #define CONNECTION_PRIVATE #define TOR_CHANNEL_INTERNAL_ +#define TORTLS_PRIVATE +#include <openssl/x509.h> +#include <openssl/ssl.h> #include "or.h" #include "config.h" #include "connection.h" #include "connection_or.h" #include "channeltls.h" #include "link_handshake.h" +#include "router.h" #include "routerkeys.h" #include "scheduler.h" #include "torcert.h" @@ -337,7 +341,6 @@ recv_certs_setup(const struct testcase_t *test) int is_auth = !strcmpend(test->setup_data, "-Auth"); tor_assert(is_ed != is_rsa); tor_assert(is_link != is_auth); - printf("rsa == %d; is_link == %d\n", is_rsa, is_link); d->c = or_connection_new(CONN_TYPE_OR, AF_INET); d->chan = tor_malloc_zero(sizeof(*d->chan)); @@ -490,6 +493,7 @@ test_link_handshake_recv_certs_ok_server(void *arg) tt_int_op(d->c->handshake_state->authenticated, ==, 0); tt_int_op(d->c->handshake_state->received_certs_cell, ==, 1); tt_assert(d->c->handshake_state->certs->id_cert != NULL); + tt_assert(d->c->handshake_state->certs->link_cert == NULL); if (d->is_ed) { tt_assert(d->c->handshake_state->certs->ed_sign_auth != NULL); tt_assert(d->c->handshake_state->certs->auth_cert == NULL); @@ -514,6 +518,8 @@ test_link_handshake_recv_certs_ok_server(void *arg) tt_int_op(1, ==, mock_close_called); \ tt_int_op(0, ==, mock_send_authenticate_called); \ tt_int_op(0, ==, mock_send_netinfo_called); \ + tt_int_op(0, ==, d->c->handshake_state->authenticated_rsa); \ + tt_int_op(0, ==, d->c->handshake_state->authenticated_ed25519); \ if (require_failure_message) { \ expect_log_msg_containing(require_failure_message); \ } \ @@ -554,12 +560,41 @@ CERTS_FAIL(truncated_3, d->cell->payload_len = 7; memcpy(d->cell->payload, "\x01\x01\x00\x05""abc", 7); }) +CERTS_FAIL(truncated_4, /* ed25519 */ + { + require_failure_message = "It couldn't be parsed"; + d->cell->payload_len -= 10; + }) +CERTS_FAIL(truncated_5, /* ed25519 */ + { + require_failure_message = "It couldn't be parsed"; + d->cell->payload_len -= 100; + }) + #define REENCODE() do { \ + const char *msg = certs_cell_check(d->ccell); \ + if (msg) puts(msg); \ ssize_t n = certs_cell_encode(d->cell->payload, 4096, d->ccell); \ tt_int_op(n, >, 0); \ d->cell->payload_len = n; \ } while (0) +CERTS_FAIL(truncated_6, /* ed25519 */ + { + /* truncate the link certificate */ + require_failure_message = "undecodable Ed certificate"; + certs_cell_cert_setlen_body(certs_cell_get_certs(d->ccell, 3), 7); + certs_cell_get_certs(d->ccell, 3)->cert_len = 7; + REENCODE(); + }) +CERTS_FAIL(truncated_7, /* ed25519 */ + { + /* truncate the crosscert */ + require_failure_message = "Unparseable or overlong crosscert"; + certs_cell_cert_setlen_body(certs_cell_get_certs(d->ccell, 4), 7); + certs_cell_get_certs(d->ccell, 4)->cert_len = 7; + REENCODE(); + }) CERTS_FAIL(not_x509, { require_failure_message = "Received undecodable certificate"; @@ -588,6 +623,206 @@ CERTS_FAIL(both_auth, certs_cell_get_certs(d->ccell, 1)->cert_type = 3; REENCODE(); }) +CERTS_FAIL(duplicate_id, /* ed25519 */ + { + require_failure_message = "Duplicate Ed25519 certificate"; + certs_cell_get_certs(d->ccell, 2)->cert_type = 4; + certs_cell_get_certs(d->ccell, 3)->cert_type = 4; + REENCODE(); + }) +CERTS_FAIL(duplicate_link, /* ed25519 */ + { + require_failure_message = "Duplicate Ed25519 certificate"; + certs_cell_get_certs(d->ccell, 2)->cert_type = 5; + certs_cell_get_certs(d->ccell, 3)->cert_type = 5; + REENCODE(); + }) +CERTS_FAIL(duplicate_crosscert, /* ed25519 */ + { + require_failure_message = "Duplicate RSA->Ed25519 crosscert"; + certs_cell_get_certs(d->ccell, 2)->cert_type = 7; + certs_cell_get_certs(d->ccell, 3)->cert_type = 7; + REENCODE(); + }) +static void +test_link_handshake_recv_certs_missing_id(void *arg) /* ed25519 */ +{ + certs_data_t *d = arg; + tt_int_op(certs_cell_getlen_certs(d->ccell), OP_EQ, 5); + certs_cell_set_certs(d->ccell, 2, certs_cell_get_certs(d->ccell, 4)); + certs_cell_set0_certs(d->ccell, 4, NULL); /* prevent free */ + certs_cell_setlen_certs(d->ccell, 4); + d->ccell->n_certs = 4; + REENCODE(); + + /* This handshake succeeds, but since we have no ID cert, we will + * just do the RSA handshake. */ + channel_tls_process_certs_cell(d->cell, d->chan); + tt_int_op(0, ==, mock_close_called); + tt_int_op(0, ==, d->c->handshake_state->authenticated_ed25519); + tt_int_op(1, ==, d->c->handshake_state->authenticated_rsa); + done: + ; +} +CERTS_FAIL(missing_signing_key, /* ed25519 */ + { + require_failure_message = "No Ed25519 signing key"; + tt_int_op(certs_cell_getlen_certs(d->ccell), OP_EQ, 5); + certs_cell_cert_t *cert = certs_cell_get_certs(d->ccell, 2); + tt_int_op(cert->cert_type, ==, CERTTYPE_ED_ID_SIGN); + /* replace this with a valid master->signing cert, but with no + * signing key. */ + const ed25519_keypair_t *mk = get_master_identity_keypair(); + const ed25519_keypair_t *sk = get_master_signing_keypair(); + tor_cert_t *bad_cert = tor_cert_create(mk, CERT_TYPE_ID_SIGNING, + &sk->pubkey, time(NULL), 86400, + 0 /* don't include signer */); + certs_cell_cert_setlen_body(cert, bad_cert->encoded_len); + memcpy(certs_cell_cert_getarray_body(cert), + bad_cert->encoded, bad_cert->encoded_len); + cert->cert_len = bad_cert->encoded_len; + tor_cert_free(bad_cert); + REENCODE(); + }) +CERTS_FAIL(missing_link, /* ed25519 */ + { + require_failure_message = "No Ed25519 link key"; + tt_int_op(certs_cell_getlen_certs(d->ccell), OP_EQ, 5); + certs_cell_set_certs(d->ccell, 3, certs_cell_get_certs(d->ccell, 4)); + certs_cell_set0_certs(d->ccell, 4, NULL); /* prevent free */ + certs_cell_setlen_certs(d->ccell, 4); + d->ccell->n_certs = 4; + REENCODE(); + }) +CERTS_FAIL(missing_auth, /* ed25519 */ + { + d->c->handshake_state->started_here = 0; + d->c->handshake_state->certs->started_here = 0; + require_failure_message = "No Ed25519 link authentication key"; + tt_int_op(certs_cell_getlen_certs(d->ccell), OP_EQ, 5); + certs_cell_set_certs(d->ccell, 3, certs_cell_get_certs(d->ccell, 4)); + certs_cell_set0_certs(d->ccell, 4, NULL); /* prevent free */ + certs_cell_setlen_certs(d->ccell, 4); + d->ccell->n_certs = 4; + REENCODE(); + }) +CERTS_FAIL(missing_crosscert, /* ed25519 */ + { + require_failure_message = "Missing RSA->Ed25519 crosscert"; + tt_int_op(certs_cell_getlen_certs(d->ccell), OP_EQ, 5); + certs_cell_setlen_certs(d->ccell, 4); + d->ccell->n_certs = 4; + REENCODE(); + }) +CERTS_FAIL(missing_rsa_id, /* ed25519 */ + { + require_failure_message = "Missing legacy RSA ID cert"; + tt_int_op(certs_cell_getlen_certs(d->ccell), OP_EQ, 5); + certs_cell_set_certs(d->ccell, 1, certs_cell_get_certs(d->ccell, 4)); + certs_cell_set0_certs(d->ccell, 4, NULL); /* prevent free */ + certs_cell_setlen_certs(d->ccell, 4); + d->ccell->n_certs = 4; + REENCODE(); + }) +CERTS_FAIL(link_mismatch, /* ed25519 */ + { + require_failure_message = "Link certificate does not match " + "TLS certificate"; + const tor_x509_cert_t *idc; + tor_tls_get_my_certs(1, NULL, &idc); + tor_x509_cert_free(mock_peer_cert); + /* Pretend that the peer cert was something else. */ + mock_peer_cert = tor_x509_cert_dup(idc); + /* No reencode needed. */ + }) +CERTS_FAIL(bad_ed_sig, /* ed25519 */ + { + require_failure_message = "At least one Ed25519 certificate was " + "badly signed"; + certs_cell_cert_t *cert = certs_cell_get_certs(d->ccell, 3); + uint8_t *body = certs_cell_cert_getarray_body(cert); + ssize_t body_len = certs_cell_cert_getlen_body(cert); + /* Frob a byte in the signature */ + body[body_len - 13] ^= 7; + REENCODE(); + }) +CERTS_FAIL(bad_crosscert, /*ed25519*/ + { + require_failure_message = "Invalid RSA->Ed25519 crosscert"; + certs_cell_cert_t *cert = certs_cell_get_certs(d->ccell, 4); + uint8_t *body = certs_cell_cert_getarray_body(cert); + ssize_t body_len = certs_cell_cert_getlen_body(cert); + /* Frob a byte in the signature */ + body[body_len - 13] ^= 7; + REENCODE(); + }) +CERTS_FAIL(bad_rsa_id_cert, /*ed25519*/ + { + require_failure_message = "legacy RSA ID certificate was not valid"; + certs_cell_cert_t *cert = certs_cell_get_certs(d->ccell, 1); + uint8_t *body = certs_cell_cert_getarray_body(cert); + ssize_t body_len = certs_cell_cert_getlen_body(cert); + /* Frob a byte in the signature */ + body[body_len - 13] ^= 7; + REENCODE(); + }) +CERTS_FAIL(expired_rsa_id, /* both */ + { + require_failure_message = "Certificate already expired"; + /* we're going to replace the identity cert with an expired one. */ + certs_cell_cert_t *cert = certs_cell_get_certs(d->ccell, 1); + const tor_x509_cert_t *idc; + tor_tls_get_my_certs(1, NULL, &idc); + X509 *newc = X509_dup(idc->cert); + time_t new_end = time(NULL) - 86400 * 10; + X509_time_adj(X509_get_notAfter(newc), 0, &new_end); + EVP_PKEY *pk = crypto_pk_get_evp_pkey_(d->key2, 1); + tt_assert(X509_sign(newc, pk, EVP_sha1())); + int len = i2d_X509(newc, NULL); + certs_cell_cert_setlen_body(cert, len); + uint8_t *body = certs_cell_cert_getarray_body(cert); + int len2 = i2d_X509(newc, &body); + tt_int_op(len, ==, len2); + REENCODE(); + X509_free(newc); + EVP_PKEY_free(pk); + }) +CERTS_FAIL(expired_ed_id, /* ed25519 */ + { + /* we're going to replace the Ed Id->sign cert with an expired one. */ + require_failure_message = "At least one certificate expired"; + /* We don't need to re-sign, since we check for expiration first. */ + certs_cell_cert_t *cert = certs_cell_get_certs(d->ccell, 2); + uint8_t *body = certs_cell_cert_getarray_body(cert); + /* The expiration field is bytes [2..5]. It is in HOURS since the + * epoch. */ + set_uint32(body+2, htonl(24)); /* Back to jan 2, 1970. */ + REENCODE(); + }) +CERTS_FAIL(expired_ed_link, /* ed25519 */ + { + /* we're going to replace the Ed Sign->link cert with an expired one. */ + require_failure_message = "At least one certificate expired"; + /* We don't need to re-sign, since we check for expiration first. */ + certs_cell_cert_t *cert = certs_cell_get_certs(d->ccell, 3); + uint8_t *body = certs_cell_cert_getarray_body(cert); + /* The expiration field is bytes [2..5]. It is in HOURS since the + * epoch. */ + set_uint32(body+2, htonl(24)); /* Back to jan 2, 1970. */ + REENCODE(); + }) +CERTS_FAIL(expired_crosscert, /* ed25519 */ + { + /* we're going to replace the Ed Sign->link cert with an expired one. */ + require_failure_message = "Crosscert is expired"; + /* We don't need to re-sign, since we check for expiration first. */ + certs_cell_cert_t *cert = certs_cell_get_certs(d->ccell, 4); + uint8_t *body = certs_cell_cert_getarray_body(cert); + /* The expiration field is bytes [32..35]. once again, HOURS. */ + set_uint32(body+32, htonl(24)); /* Back to jan 2, 1970. */ + REENCODE(); + }) + CERTS_FAIL(wrong_labels_1, { require_failure_message = "The link certificate was not valid"; @@ -612,14 +847,16 @@ CERTS_FAIL(wrong_labels_2, }) CERTS_FAIL(wrong_labels_3, { - require_failure_message = "The certs we wanted were missing"; + require_failure_message = + "The certs we wanted (ID, Link) were missing"; certs_cell_get_certs(d->ccell, 0)->cert_type = 2; certs_cell_get_certs(d->ccell, 1)->cert_type = 3; REENCODE(); }) CERTS_FAIL(server_missing_certs, { - require_failure_message = "The certs we wanted were missing"; + require_failure_message = + "The certs we wanted (ID, Auth) were missing"; d->c->handshake_state->started_here = 0; d->c->handshake_state->certs->started_here = 0; @@ -1214,10 +1451,32 @@ struct testcase_t link_handshake_tests[] = { TEST_RCV_CERTS(truncated_1), TEST_RCV_CERTS(truncated_2), TEST_RCV_CERTS(truncated_3), + TEST_RCV_CERTS_ED(truncated_4, "Ed25519-Link"), + TEST_RCV_CERTS_ED(truncated_5, "Ed25519-Link"), + TEST_RCV_CERTS_ED(truncated_6, "Ed25519-Link"), + TEST_RCV_CERTS_ED(truncated_7, "Ed25519-Link"), TEST_RCV_CERTS(not_x509), TEST_RCV_CERTS(both_link), TEST_RCV_CERTS(both_id_rsa), TEST_RCV_CERTS(both_auth), + TEST_RCV_CERTS_ED(duplicate_id, "Ed25519-Link"), + TEST_RCV_CERTS_ED(duplicate_link, "Ed25519-Link"), + TEST_RCV_CERTS_ED(duplicate_crosscert, "Ed25519-Link"), + TEST_RCV_CERTS_ED(missing_crosscert, "Ed25519-Link"), + TEST_RCV_CERTS_ED(missing_id, "Ed25519-Link"), + TEST_RCV_CERTS_ED(missing_signing_key, "Ed25519-Link"), + TEST_RCV_CERTS_ED(missing_link, "Ed25519-Link"), + TEST_RCV_CERTS_ED(missing_auth, "Ed25519-Auth"), + TEST_RCV_CERTS_ED(missing_rsa_id, "Ed25519-Link"), + TEST_RCV_CERTS_ED(link_mismatch, "Ed25519-Link"), + TEST_RCV_CERTS_ED(bad_ed_sig, "Ed25519-Link"), + TEST_RCV_CERTS_ED(bad_rsa_id_cert, "Ed25519-Link"), + TEST_RCV_CERTS_ED(bad_crosscert, "Ed25519-Link"), + TEST_RCV_CERTS_RSA(expired_rsa_id, "RSA-Link"), + TEST_RCV_CERTS_ED(expired_rsa_id, "Ed25519-Link"), + TEST_RCV_CERTS_ED(expired_ed_id, "Ed25519-Link"), + TEST_RCV_CERTS_ED(expired_ed_link, "Ed25519-Link"), + TEST_RCV_CERTS_ED(expired_crosscert, "Ed25519-Link"), TEST_RCV_CERTS(wrong_labels_1), TEST_RCV_CERTS(wrong_labels_2), TEST_RCV_CERTS(wrong_labels_3),