Fix test failures with TLS 1.3-capable OpenSSL #138
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* topic/test-memory-leak: Enable OSSL_MDEBUG on CI builds Add OpenSSL.print_mem_leaks test: prepare test PKey instances on demand test: let OpenSSL::TestCase include OpenSSL::TestUtils Don't define main() when built with --enable-debug (cherry picked from commit 5c586ac) Note that fix for new test cases that use the old constants removed by this is squashed in.
rhenium
force-pushed
the
ky/maint-fix-test-with-tls13
branch
2 times, most recently
from
fbdcde4 to
58b5c76
Compare
Fix wrong nesting in test/utils.rb. Remove unnecessary requires. Wrap the code with 'if defined?(OpenSSL::TestUtils) ~ end' and avoid class definition with modifier if.
Use EnvUtil.suppress_warning instead. We have started to use it already, and the name 'suppress_warning' expresses what it does more clearly.
Add a method that returns whether the OpenSSL supports TLS 1.2 or not. This will be useful for test cases that are specific to TLS ~1.2.
The block passed to start_server is invoked with two arguments, the running thread object for the server and the automatically-selected port number. The first argument is completely useless and actually is not used anywhere.
An assumption in OpenSSL::TestSSL#test_get_ephemeral_key is that the ephemeral key type is always EVP_PKEY_EC when negotiated with an ECDHE cipher suite. This is not true if X25519 is chosen. The test is passing because we happen to fix the group to P-256 in start_server, but let's make it explicit.
Close the socket by server_loop rather than by server_proc. This reduces code duplication.
start_server can hang if the given block exits before closing sockets that the block opens. While this is a carelessness of the caller, we can do a better job.
Add methods that check whether the running OpenSSL is an OpenSSL or a LibreSSL, and optionally check whether the version is newer or equal to the given version number.
LibreSSL 2.6.1 removed DSA support from its SSL code. Also, TLS 1.3 will not support DSA certificates. Use an RSA certificate as the client certificate in the tests, too.
The very patch that added this test case made the dfree function not send close_notify alert when an SSLSocket is being GCed. Anyway, the new OSSL_GC_STRESS option added by 6ee4b28 ("test: run test cases under GC.stress if OSSL_GC_STRESS is specified", 2016-12-04) will cover this kind of issues.
Use TLS 1.2 explicitly where needed, since TLS 1.3 will remove session ID based session resumption.
Fix test cases failing with TLS 1.3-enabled OpenSSL master.
rhenium
force-pushed
the
ky/maint-fix-test-with-tls13
branch
from
58b5c76 to
e3a3050
Compare
hsbt
approved these changes
Aug 25, 2017
Member
hsbt
left a comment
hsbt
left a comment
There was a problem hiding this comment.
Looks good. Fixture feature and integrate some methods to TestUtils module are so cool.
| @@ -1,9 +1,9 @@ | |||
| # frozen_string_literal: false | |||
| require_relative 'utils' | |||
| require 'stringio' | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.