From c1f476a3d59a66e39302b51f6f937aabf0b3a0d5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 20 Aug 2018 17:42:38 -0400 Subject: [PATCH 1/2] Use our x509 wrapper code in tor_tls_cert_matches_key() This allows us to mock our own tor_tls_get_peer_certificate() function in order to test ..cert_matches_key(), which will in turn allow us to simplify test_tortls_cert_matches_key() considerably. Prep work for the fix for 27226. --- src/common/tortls.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index a4e188603c..4cbe8b10e5 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -857,18 +857,20 @@ tor_tls_cert_get_key(tor_x509_cert_t *cert) MOCK_IMPL(int, tor_tls_cert_matches_key,(const tor_tls_t *tls, const tor_x509_cert_t *cert)) { - X509 *peercert = SSL_get_peer_certificate(tls->ssl); + tor_x509_cert_t *peer = tor_tls_get_peer_cert((tor_tls_t *)tls); + if (!peer) + return 0; + + X509 *peercert = peer->cert; EVP_PKEY *link_key = NULL, *cert_key = NULL; int result; - if (!peercert) - return 0; link_key = X509_get_pubkey(peercert); cert_key = X509_get_pubkey(cert->cert); result = link_key && cert_key && EVP_PKEY_cmp(cert_key, link_key) == 1; - X509_free(peercert); + tor_x509_cert_free(peer); if (link_key) EVP_PKEY_free(link_key); if (cert_key) From 85a87923440361bc5a3b82c6e874192ebe08b8d4 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 20 Aug 2018 17:43:41 -0400 Subject: [PATCH 2/2] Rewrite test_tortls_cert_matches_key() Unlike the old test, this test no will no longer mess around with the forbidden internals of any openssl data structures. Additionally, it verifies several other behaviors of tor_tls_cert_matches_key() that we had wanted to verify, such as the possibility of the certificate's key not matching. Fixes bug 27226; bugfix on 0.2.5.1-alpha. --- changes/bug27226 | 5 ++ src/test/test_tortls.c | 109 +++++++++++++++++++++-------------------- 2 files changed, 60 insertions(+), 54 deletions(-) create mode 100644 changes/bug27226 diff --git a/changes/bug27226 b/changes/bug27226 new file mode 100644 index 0000000000..9030773cd5 --- /dev/null +++ b/changes/bug27226 @@ -0,0 +1,5 @@ + o Minor bugfixes (testing, openssl compatibility): + - Our "tortls/cert_matches_key" unit test no longer relies on OpenSSL + internals. Previously, it relied on unsupported OpenSSL behavior in + a way that caused it to crash with OpenSSL 1.0.2p. Fixes bug 27226; + bugfix on 0.2.5.1-alpha. diff --git a/src/test/test_tortls.c b/src/test/test_tortls.c index 47455cff83..b0aec88ec3 100644 --- a/src/test/test_tortls.c +++ b/src/test/test_tortls.c @@ -546,13 +546,6 @@ test_tortls_x509_cert_get_id_digests(void *ignored) } #ifndef OPENSSL_OPAQUE -static int -fixed_pub_cmp(const EVP_PKEY *a, const EVP_PKEY *b) -{ - (void) a; (void) b; - return 1; -} - static void fake_x509_free(X509 *cert) { @@ -569,70 +562,78 @@ fake_x509_free(X509 *cert) tor_free(cert); } } +#endif + +static tor_x509_cert_t *fixed_x509_cert = NULL; +static tor_x509_cert_t * +get_peer_cert_mock_return_fixed(tor_tls_t *tls) +{ + (void)tls; + if (fixed_x509_cert) + return tor_x509_cert_dup(fixed_x509_cert); + else + return NULL; +} static void test_tortls_cert_matches_key(void *ignored) { (void)ignored; - int res; - tor_tls_t *tls; - tor_x509_cert_t *cert; - X509 *one = NULL, *two = NULL; - EVP_PKEY_ASN1_METHOD *meth = EVP_PKEY_asn1_new(999, 0, NULL, NULL); - EVP_PKEY_asn1_set_public(meth, NULL, NULL, fixed_pub_cmp, NULL, NULL, NULL); - tls = tor_malloc_zero(sizeof(tor_tls_t)); - cert = tor_malloc_zero(sizeof(tor_x509_cert_t)); - one = tor_malloc_zero(sizeof(X509)); - one->references = 1; - two = tor_malloc_zero(sizeof(X509)); - two->references = 1; + X509 *cert1 = NULL, *cert2 = NULL, *cert3 = NULL, *cert4 = NULL; + tor_x509_cert_t *c1 = NULL, *c2 = NULL, *c3 = NULL, *c4 = NULL; + crypto_pk_t *k1 = NULL, *k2 = NULL, *k3 = NULL; - res = tor_tls_cert_matches_key(tls, cert); - tt_int_op(res, OP_EQ, 0); + k1 = pk_generate(1); + k2 = pk_generate(2); + k3 = pk_generate(3); - tls->ssl = tor_malloc_zero(sizeof(SSL)); - tls->ssl->session = tor_malloc_zero(sizeof(SSL_SESSION)); - tls->ssl->session->peer = one; - res = tor_tls_cert_matches_key(tls, cert); - tt_int_op(res, OP_EQ, 0); + cert1 = tor_tls_create_certificate(k1, k2, "A", "B", 1000); + cert2 = tor_tls_create_certificate(k1, k3, "C", "D", 1000); + cert3 = tor_tls_create_certificate(k2, k3, "C", "D", 1000); + cert4 = tor_tls_create_certificate(k3, k2, "E", "F", 1000); - cert->cert = two; - res = tor_tls_cert_matches_key(tls, cert); - tt_int_op(res, OP_EQ, 0); + tt_assert(cert1 && cert2 && cert3 && cert4); - one->cert_info = tor_malloc_zero(sizeof(X509_CINF)); - one->cert_info->key = tor_malloc_zero(sizeof(X509_PUBKEY)); - one->cert_info->key->pkey = tor_malloc_zero(sizeof(EVP_PKEY)); - one->cert_info->key->pkey->references = 1; - one->cert_info->key->pkey->ameth = meth; - one->cert_info->key->pkey->type = 1; + c1 = tor_x509_cert_new(cert1); cert1 = NULL; + c2 = tor_x509_cert_new(cert2); cert2 = NULL; + c3 = tor_x509_cert_new(cert3); cert3 = NULL; + c4 = tor_x509_cert_new(cert4); cert4 = NULL; - two->cert_info = tor_malloc_zero(sizeof(X509_CINF)); - two->cert_info->key = tor_malloc_zero(sizeof(X509_PUBKEY)); - two->cert_info->key->pkey = tor_malloc_zero(sizeof(EVP_PKEY)); - two->cert_info->key->pkey->references = 1; - two->cert_info->key->pkey->ameth = meth; - two->cert_info->key->pkey->type = 2; + tt_assert(c1 && c2 && c3 && c4); - res = tor_tls_cert_matches_key(tls, cert); - tt_int_op(res, OP_EQ, 0); + MOCK(tor_tls_get_peer_cert, get_peer_cert_mock_return_fixed); - one->cert_info->key->pkey->type = 1; - two->cert_info->key->pkey->type = 1; - res = tor_tls_cert_matches_key(tls, cert); - tt_int_op(res, OP_EQ, 1); + fixed_x509_cert = NULL; + /* If the peer has no certificate, it shouldn't match anything. */ + tt_assert(! tor_tls_cert_matches_key(NULL, c1)); + tt_assert(! tor_tls_cert_matches_key(NULL, c2)); + tt_assert(! tor_tls_cert_matches_key(NULL, c3)); + tt_assert(! tor_tls_cert_matches_key(NULL, c4)); + fixed_x509_cert = c1; + /* If the peer has a certificate, it should match every cert with the same + * subject key. */ + tt_assert(tor_tls_cert_matches_key(NULL, c1)); + tt_assert(tor_tls_cert_matches_key(NULL, c2)); + tt_assert(! tor_tls_cert_matches_key(NULL, c3)); + tt_assert(! tor_tls_cert_matches_key(NULL, c4)); done: - EVP_PKEY_asn1_free(meth); - tor_free(tls->ssl->session); - tor_free(tls->ssl); - tor_free(tls); - tor_free(cert); - fake_x509_free(one); - fake_x509_free(two); + tor_x509_cert_free(c1); + tor_x509_cert_free(c2); + tor_x509_cert_free(c3); + tor_x509_cert_free(c4); + if (cert1) X509_free(cert1); + if (cert2) X509_free(cert2); + if (cert3) X509_free(cert3); + if (cert4) X509_free(cert4); + crypto_pk_free(k1); + crypto_pk_free(k2); + crypto_pk_free(k3); + UNMOCK(tor_tls_get_peer_cert); } +#ifndef OPENSSL_OPAQUE static void test_tortls_cert_get_key(void *ignored) {