August 2018 - tor-commits - lists.torproject.org

[tor/maint-0.3.3] Use our x509 wrapper code in tor_tls_cert_matches_key()
by nickm＠torproject.org 22 Aug '18

22 Aug '18

commit c1f476a3d59a66e39302b51f6f937aabf0b3a0d5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 20 17:42:38 2018 -0400 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 a4e188603..4cbe8b10e 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)

1 0

[tor/maint-0.3.3] Merge branch 'maint-0.3.2' into maint-0.3.3
by nickm＠torproject.org 22 Aug '18

22 Aug '18

commit 245025a3dff5ffb893c0cc079a8e5d59df553529 Merge: 4c0c4d4ad d52f40600 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 21 19:16:40 2018 -0400 Merge branch 'maint-0.3.2' into maint-0.3.3 changes/bug27226 | 5 +++ src/common/tortls.c | 10 +++-- src/test/test_tortls.c | 119 +++++++++++++++++++++++++------------------------ 3 files changed, 71 insertions(+), 63 deletions(-)

1 0

[tor/maint-0.3.4] Use our x509 wrapper code in tor_tls_cert_matches_key()
by nickm＠torproject.org 22 Aug '18

22 Aug '18

commit c1f476a3d59a66e39302b51f6f937aabf0b3a0d5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 20 17:42:38 2018 -0400 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 a4e188603..4cbe8b10e 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)

1 0

[tor/maint-0.3.4] Merge branch 'maint-0.2.9' into maint-0.3.2
by nickm＠torproject.org 22 Aug '18

22 Aug '18

commit d52f406001082474c3decf04747bf6f742b82c96 Merge: c798957b5 85a879234 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 21 19:16:40 2018 -0400 Merge branch 'maint-0.2.9' into maint-0.3.2 changes/bug27226 | 5 +++ src/common/tortls.c | 10 +++-- src/test/test_tortls.c | 119 +++++++++++++++++++++++++------------------------ 3 files changed, 71 insertions(+), 63 deletions(-)

1 0

[tor/maint-0.3.4] Merge branch 'maint-0.3.3' into maint-0.3.4
by nickm＠torproject.org 22 Aug '18

22 Aug '18

commit f68aab83ba6ab61b010d4445b6c68a6cf780471a Merge: df874ed0f 245025a3d Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 21 19:20:31 2018 -0400 Merge branch 'maint-0.3.3' into maint-0.3.4 changes/bug27226 | 5 ++ src/common/tortls.c | 10 ++-- src/test/test_tortls.c | 122 ++++++++++++++++++++++++------------------------- 3 files changed, 72 insertions(+), 65 deletions(-) diff --cc src/test/test_tortls.c index 388f6df32,bb899a957..5b5e69b00 --- a/src/test/test_tortls.c +++ b/src/test/test_tortls.c @@@ -553,22 -547,6 +553,15 @@@ test_tortls_x509_cert_get_id_digests(vo } #ifndef OPENSSL_OPAQUE - static int - fixed_pub_cmp(const EVP_PKEY *a, const EVP_PKEY *b) - { - (void) a; (void) b; - return 1; - } - +/* + * Use only for the matching fake_x509_free() call + */ +static X509 * +fake_x509_malloc(void) +{ + return tor_malloc_zero(sizeof(X509)); +} + static void fake_x509_free(X509 *cert) { @@@ -2795,7 -2784,7 +2796,7 @@@ struct testcase_t tortls_tests[] = LOCAL_TEST_CASE(always_accept_verify_cb, 0), INTRUSIVE_TEST_CASE(x509_cert_free, 0), LOCAL_TEST_CASE(x509_cert_get_id_digests, 0), -- INTRUSIVE_TEST_CASE(cert_matches_key, 0), ++ LOCAL_TEST_CASE(cert_matches_key, 0), INTRUSIVE_TEST_CASE(cert_get_key, 0), LOCAL_TEST_CASE(get_my_client_auth_key, TT_FORK), LOCAL_TEST_CASE(get_my_certs, TT_FORK), @@@ -2839,4 -2828,4 +2840,3 @@@ LOCAL_TEST_CASE(context_init_one, 0), END_OF_TESTCASES }; --

1 0

[tor/maint-0.3.4] Rewrite test_tortls_cert_matches_key()
by nickm＠torproject.org 22 Aug '18

22 Aug '18

commit 85a87923440361bc5a3b82c6e874192ebe08b8d4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 20 17:43:41 2018 -0400 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 | 119 +++++++++++++++++++++++++------------------------ 2 files changed, 65 insertions(+), 59 deletions(-) diff --git a/changes/bug27226 b/changes/bug27226 new file mode 100644 index 000000000..9030773cd --- /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 47455cff8..b0aec88ec 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); - - cert->cert = two; - res = tor_tls_cert_matches_key(tls, cert); - tt_int_op(res, OP_EQ, 0); - - 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; - - 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; - - res = tor_tls_cert_matches_key(tls, cert); - tt_int_op(res, OP_EQ, 0); - - 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); + 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); + + tt_assert(cert1 && cert2 && cert3 && cert4); + + 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; + + tt_assert(c1 && c2 && c3 && c4); + + MOCK(tor_tls_get_peer_cert, get_peer_cert_mock_return_fixed); + + 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) {

1 0

[tor/master] Use our x509 wrapper code in tor_tls_cert_matches_key()
by nickm＠torproject.org 22 Aug '18

22 Aug '18

commit c1f476a3d59a66e39302b51f6f937aabf0b3a0d5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 20 17:42:38 2018 -0400 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 a4e188603..4cbe8b10e 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)

1 0

[tor/maint-0.3.4] Merge branch 'maint-0.3.2' into maint-0.3.3
by nickm＠torproject.org 22 Aug '18

22 Aug '18

commit 245025a3dff5ffb893c0cc079a8e5d59df553529 Merge: 4c0c4d4ad d52f40600 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 21 19:16:40 2018 -0400 Merge branch 'maint-0.3.2' into maint-0.3.3 changes/bug27226 | 5 +++ src/common/tortls.c | 10 +++-- src/test/test_tortls.c | 119 +++++++++++++++++++++++++------------------------ 3 files changed, 71 insertions(+), 63 deletions(-)

1 0

[tor/master] Merge branch 'maint-0.3.3' into maint-0.3.4
by nickm＠torproject.org 22 Aug '18

22 Aug '18

commit f68aab83ba6ab61b010d4445b6c68a6cf780471a Merge: df874ed0f 245025a3d Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 21 19:20:31 2018 -0400 Merge branch 'maint-0.3.3' into maint-0.3.4 changes/bug27226 | 5 ++ src/common/tortls.c | 10 ++-- src/test/test_tortls.c | 122 ++++++++++++++++++++++++------------------------- 3 files changed, 72 insertions(+), 65 deletions(-) diff --cc src/test/test_tortls.c index 388f6df32,bb899a957..5b5e69b00 --- a/src/test/test_tortls.c +++ b/src/test/test_tortls.c @@@ -553,22 -547,6 +553,15 @@@ test_tortls_x509_cert_get_id_digests(vo } #ifndef OPENSSL_OPAQUE - static int - fixed_pub_cmp(const EVP_PKEY *a, const EVP_PKEY *b) - { - (void) a; (void) b; - return 1; - } - +/* + * Use only for the matching fake_x509_free() call + */ +static X509 * +fake_x509_malloc(void) +{ + return tor_malloc_zero(sizeof(X509)); +} + static void fake_x509_free(X509 *cert) { @@@ -2795,7 -2784,7 +2796,7 @@@ struct testcase_t tortls_tests[] = LOCAL_TEST_CASE(always_accept_verify_cb, 0), INTRUSIVE_TEST_CASE(x509_cert_free, 0), LOCAL_TEST_CASE(x509_cert_get_id_digests, 0), -- INTRUSIVE_TEST_CASE(cert_matches_key, 0), ++ LOCAL_TEST_CASE(cert_matches_key, 0), INTRUSIVE_TEST_CASE(cert_get_key, 0), LOCAL_TEST_CASE(get_my_client_auth_key, TT_FORK), LOCAL_TEST_CASE(get_my_certs, TT_FORK), @@@ -2839,4 -2828,4 +2840,3 @@@ LOCAL_TEST_CASE(context_init_one, 0), END_OF_TESTCASES }; --

1 0

[tor/master] Merge branch 'maint-0.2.9' into maint-0.3.2
by nickm＠torproject.org 22 Aug '18

22 Aug '18

commit d52f406001082474c3decf04747bf6f742b82c96 Merge: c798957b5 85a879234 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 21 19:16:40 2018 -0400 Merge branch 'maint-0.2.9' into maint-0.3.2 changes/bug27226 | 5 +++ src/common/tortls.c | 10 +++-- src/test/test_tortls.c | 119 +++++++++++++++++++++++++------------------------ 3 files changed, 71 insertions(+), 63 deletions(-)

1 0