Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add regression test for recently added OpenSSL 1.1 support #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

welwood08
Copy link
Contributor

@welwood08 welwood08 commented Nov 15, 2016

This took a bit longer than I would've liked but as requested in #64 here is a regression test for all the methods I touched while adding OpenSSL 1.1 support.

Requires openssl and sed binaries to be on the path, which should be fine considering an SSL library is needed for the test and sed is already used in the makefile.

The openssl binary should support the same protocol versions as the SSL library used to build Csocket and this test, otherwise the protocol version checks will fail.

The bulk of the makefile changes were so that libraries are only linked when needed. This would've taken longer if I had to worry about compiling curl against every OpenSSL when this test doesn't use curl, or if I had to find out how to get cares on the ancient system I'm using.

SSLv2 is omitted from the test - it was consistently failing to connect whenever OpenSSL was compiled with it enabled, and since it's cryptographically broken anyway I didn't think it was worth spending the time trying to make it work. It's easily added back to the test if wanted, making it pass is another matter.

I've run this test on Debian against 6b28102 (just before the OpenSSL 1.1 support merged in) and 94e21a8 (current master). OpenSSL 1.0.0t, 1.0.1u, 1.0.2j and 1.1.0c were freshly compiled for each of the following configure options: shared enable-ssl3 enable-ssl3-method, shared no-ssl3, and shared no-ssl2 no-ssl3 no-md4 no-comp. All these combinations pass this regression test (1.1 obviously only with the latter commit).

OpenSSL 0.9.8 fails most of this test but I haven't spent much time trying to fix it. In any case, 1 minor build error in Csocket itself was introduced after my code was refactored. Is support for this ancient version still desired?

Requires `openssl` and `sed` binaries to be on the path, which should
be fine considering an SSL library is needed for the test and sed is
already used in the makefile.

The openssl binary should support the same protocol versions as the SSL
library used to build Csocket and this test, otherwise the protocol
version checks will fail.
@lll2086
Copy link
Collaborator

lll2086 commented Nov 23, 2016

I've only had the chance to run it on 1.1.0c, but so far it looks good. Will try with more version and configuration when I have some time.

As for OpenSSL 0.9.8, I'm not quite sure if support is desire, but I'll see if I can find out.

@jimloco
Copy link
Owner

jimloco commented Nov 23, 2016

Thats a great question. I would say we only need to support openssl >= 1.0.2, but I still see plenty of shell servers using 0.9.8. So I guess we better continue to support 0.9.8 for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants