[tor-commits] [tor/master] Rewrite test_tortls_cert_matches_key()

[nickm at torproject.org]

commit 85a87923440361bc5a3b82c6e874192ebe08b8d4
Author: Nick Mathewson <nickm at 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)
 {