Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#20460 closed defect (fixed)

tortls test failures with recent LibreSSL (OpenBSD -current)

Reported by: rubiate Owned by:
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version: Tor: 0.3.0.0-alpha-dev
Severity: Normal Keywords: libressl openbsd
Cc: yawning Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Some tortls tests will segfault with recent LibreSSL

    tortls/classify_client_ciphers: [forking] [Lost connection!] 
      [classify_client_ciphers FAILED]
    tortls/client_is_using_v2_ciphers: [forking] [Lost connection!] 
      [client_is_using_v2_ciphers FAILED]
    [...]
    tortls/session_secret_cb: [forking] [Lost connection!] 
      [session_secret_cb FAILED]

The tests all do something like this:

one = get_cipher_by_name("ECDH-RSA-AES256-GCM-SHA384");
one->id = 0x00ff;

The problem is LibreSSL has removed support for ECDH ciphers (https://marc.info/?l=openbsd-cvs&m=147689515531541&w=2), so get_cipher_by_name() returns NULL.

This isn't in any released LibreSSL version yet but is in OpenBSD -current.

Child Tickets

Change History (7)

comment:1 Changed 3 years ago by yawning

Cc: yawning added

comment:2 Changed 3 years ago by yawning

Milestone: Tor: 0.3.0.x-final

This is probably a backport candidate to all supported stable releases.

comment:3 Changed 3 years ago by nickm

I think the right fix is to have the tests say "ECDHE" instead; they were probably supposed to in the first place.

comment:4 in reply to:  3 Changed 3 years ago by rubiate

Replying to nickm:

I think the right fix is to have the tests say "ECDHE" instead; they were probably supposed to in the first place.

Sure, that makes them pass. I wasn't sure what the ->id refers to or if they need to be changed, guess not?

diff --git a/src/test/test_tortls.c b/src/test/test_tortls.c
index 44961c8..e2fee81 100644
--- a/src/test/test_tortls.c
+++ b/src/test/test_tortls.c
@@ -834,9 +834,9 @@ test_tortls_classify_client_ciphers(void *ignored)
 
   sk_SSL_CIPHER_zero(ciphers);
 
-  one = get_cipher_by_name("ECDH-RSA-AES256-GCM-SHA384");
+  one = get_cipher_by_name("ECDHE-RSA-AES256-GCM-SHA384");
   one->id = 0x00ff;
-  two = get_cipher_by_name("ECDH-RSA-AES128-GCM-SHA256");
+  two = get_cipher_by_name("ECDHE-RSA-AES128-GCM-SHA256");
   two->id = 0x0000;
   sk_SSL_CIPHER_push(ciphers, one);
   tls->client_cipher_list_type = 0;
@@ -906,7 +906,7 @@ test_tortls_client_is_using_v2_ciphers(void *ignored)
   tt_int_op(ret, OP_EQ, 0);
 
   ciphers = sk_SSL_CIPHER_new_null();
-  SSL_CIPHER *one = get_cipher_by_name("ECDH-RSA-AES256-GCM-SHA384");
+  SSL_CIPHER *one = get_cipher_by_name("ECDHE-RSA-AES256-GCM-SHA384");
   one->id = 0x00ff;
   sk_SSL_CIPHER_push(ciphers, one);
   sess->ciphers = ciphers;
@@ -1551,7 +1551,7 @@ test_tortls_session_secret_cb(void *ignored)
   tor_tls_session_secret_cb(tls->ssl, NULL, NULL, NULL, NULL, NULL);
   tt_assert(!tls->ssl->tls_session_secret_cb);
 
-  one = get_cipher_by_name("ECDH-RSA-AES256-GCM-SHA384");
+  one = get_cipher_by_name("ECDHE-RSA-AES256-GCM-SHA384");
   one->id = 0x00ff;
   ciphers = sk_SSL_CIPHER_new_null();
   sk_SSL_CIPHER_push(ciphers, one);

Does this need a changes file? If so...

diff --git a/changes/20460 b/changes/20460
new file mode 100644
index 0000000..d51ec3a
--- /dev/null
+++ b/changes/20460
@@ -0,0 +1,4 @@
+  o Minor bugfixes (testing)
+    - Use ECDHE ciphers instead of ECDH in tortls tests. LibreSSL has
+      removed the ECDH ciphers which caused the tests to fail on
+      platforms which use it. Closes ticket 20460.

comment:5 Changed 3 years ago by nickm

Milestone: Tor: 0.3.0.x-finalTor: 0.2.9.x-final
Status: newneeds_review

comment:6 Changed 3 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

I've applied this to 0.2.9, on the theory that openbsd-current people will want to use it and it shouldn't break anything. Thanks!

comment:7 Changed 3 years ago by nickm

[applied as def41e93bdcce741c7eb87a06690fb36a133b8bb]

Note: See TracTickets for help on using tickets.