-
Notifications
You must be signed in to change notification settings - Fork 413
Bring sslscan into a OpenSSL 3.x world #326
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
Conversation
|
I attempted to compile the changes introduced in this PR, but failed: Once this is fixed, bonus points if you can run |
Remove ifdefs for old versions of OpenSSL. Remove ifdefs for SSL_MODE_SEND_FALLBACK_SCSV. It's always there. Remove ifdefs for OPENSSL_NO_TLSEXT. It's always there. Cleanup a bit of whitespace and cruft in adjacent blocks.
This is a runtime detection logic for old versions of OpenSSL.
Per the comment, this is a belt and suspenders methodology that is unlikely to work in practice. It's now deprecated anyway, so remove it entirely.
Remove library init functions. They were deprecated in 1.1.0. Several functions have been const returns. Prefer those.
1811e5b to
7d64305
Compare
|
Sorry, that was a c23-ism that snuck in. I've fixed the commit for that. I don't have a docker setup; I'm a FreeBSD/MacOS guy myself. Can I trouble you to run the docker test for me? I will see what it would take to get a functional setup, but that's going to be a bit. |
|
Yep, the docker tests all pass. Nice!
|
|
Thanks for submitting this (and @jtesta for testing). The main hesitation that I have with requiring OpenSSL 3.x is that it's only been out for three years, which means that there are still several widely used and supported distros (such as Ubuntu 20.04 LTS and RHEL 8, although that Ubuntu release seems to have dropped the package) that are shipping OpenSSL 1.x with backported security patches - so requiring 3.x means breaking their builds, as they generally don't seem keen on statically compiling packages. But the distro packaged versions of sslscan are often pretty outdated and built against versions of OpenSSL that don't support everything we'd want them to - and with the ease of both static builds and Docker builds I'm not sure that's a significant issue. And most of the people using sslscan won't be doing so from those older distros. So I guess the question would be whether we gain much from dropping support for OpenSSL 1.x? If it's the first step of a wider cleanup and refactoring process, or that legacy support is causing issues elsewhere then that makes perfect sense - but I'm always a little hesitant to drop backwards compatibility without a good reason. @jtesta what're your thoughts? |
|
Sorry for the very late response! I think our focus should be on enumerating TLS properties as completely as possible. Which means using the version of OpenSSL that best aligns with that goal. Then it'll be up to the distros to decide if they'd rather keep an older version of sslscan with less functionality but with dynamic linking, or statically link a modern OpenSSL and get very accurate TLS testing. My recent PR (#331) requires OpenSSL v3.5, which is an LTS release that just came out last month. This version is necessary to find post-quantum and hybrid key exchanges. I strongly suspect TLS defaults across multiple implementations are going to quickly shift to these types given how quickly they've taken over in the SSH protocol. In fact, OpenSSL v3.5 uses X25519MLKEM768 as the default exchange, and I can see some large sites already using it too (google.com, instagram.com). |
|
With the need for the 3.5 LTS release for that new functionality that sounds like reasonable grounds to drop support for the older ones. And probably a version bump up to 2.2.x, as this will be a big breaking change. I'll do some final testing and look to get this merged. Although thinking about it, since 3.0 only has another year or so, it might be sensible to just jump straight to 3.5 LTS (which not only gives us support until 2030, but also paves the way for #331 Thanks |
|
Perhaps now is a good time to take an official position on what the minimum version of OpenSSL is, and how manage it in the future. Like I said before, I think the top goal should be getting the best results from a TLS endpoint. So the minimum version should get bumped whenever it needs to in order to get those best results. Distros can either ship an older version of sslscan in order to link dynamically, or ship a static version that they need to update more frequently (up to them). Users can use their platform's version for convenience, or compile from git/use the Docker version if they want more features. If we officially settle on a policy (either the above, or something else), then it makes decisions like what code cleanups to make, what build systems to support, etc. much more straightforward. |
|
I've merged this, and also updated the makefile to build against 3.5.x - as it's LTS and is required for #331. Before I make the next tag, I'll add some notes to the readme/install guides making it clear that static building is strongly recommended, and if you need to build against a legacy version of OpenSSL then you'll be missing out on any new functionality. Thanks both. |
This set of code changes drags sslscan kicking and screaming into the modern era with OpenSSL. There isn't any functional code changes, it's mostly purging old ifdef's and dropping old code that has runtime detection for ancient versions of OpenSSL.
This also moves over several calls that have drop-in modern methods.
Still outstanding deprecated feature usage will require deeper code changes to deal with. Specifically protocol versioned SSL_METHOD (which is sprinkled absolutely everywhere in the codebase) and moving away from key type specific object (EC_KEY, DH, RSA, etc) to the generic EVP_PKEY framework.