-
Notifications
You must be signed in to change notification settings - Fork 37
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
Improve (na)ssl dependency situation #714
Comments
Plan: explore a number of libraries and see how they match up with our requirements, probably do some testing on good and bad setups to see how they handle. At least consider: sslscan, sslyze, testssl.sh, cryptonice, tls-scanner. |
Cryptography 39.0.0 is broken for us, possibly because they have dropped OpenSSL 1.1.0 support. The error is weird, but we've seen weird ways for errors to surface before:
38.0.4 definitely works, and we do use OpenSSL 1.1.0, so that is my best guess. #872 pins us to <39.0.0 for now. |
CryptoNicePython. Built on top of SSLyze and a few other tools. Maintained by F5, last commit August 2021. A bit fragile, some docs are outdated and the requirements are incorrect as it does not run with the latest version of sslyze. JSON output is a lot less than sslyze, and very likely insufficient. Not well suited to our needs. tls-scannerJava. Last commit July 2022. Could not get it running reliably. testssl.shBash. Last commit February 2023. Supports STARTTLS. May not support testing on IP while sending SNI hostname, but can connect on IP. Allows testing individual ciphers, giving us opportunities to shortcut. Can be slow, even when limiting tests to the important ones for us, when many TLS versions are supported. Does detect cipher order. Detects SSLv3 but does not seem to detect SSLv2. JSON is ok but not great. Some scan results. sslscanC. Last commit January 2023. Supports STARTTLS. Seems to support testing by IP and sending a hostname on SNI. Detects SSLv2/3, but not the ciphers on there. Does detect cipher order. XML seems a bit shallow. sslyzePython. Last commit January 18. Detects SSLv2/v3 and the ciphers. Supports testing on IP with SNI. Does not detect cipher order. Great JSON output. ConclusionThe best candidates for now are testssl and sslyze, and those should get a deeper dive to see how close they can get to what we need and whether adopting one of them is a good plan forwarc. Sslyze definitely misses cipher order, but perhaps we can get that added. It does have the advantage of being in a known language for us. Testssl may be more functionally complete, but there is less chance for us to patch it if needed. |
Did more testing specifically looking at our current tests: TLS
For OCSP stapling, check how deeply we verify currently. Certificate
Running the test
Tested as:
|
Our current nassl fork is also not compatible with Python 3.10 and newer:
|
Great analysis @mxsasha. I agree it's probably best to go for sslyze in the current Python setup. I'm a huge fan of testssl and bash, but the 1.14 MB testssl.sh file with almost 24k of code is insane, even for a bash fan like me. About the parallel option, that's only used in bulk testing (so this won't work). I'm pretty sure because of this code block testssl.sh#L23856-L23873, you can search for all You can trace time with: docker run --rm -e MEASURE_TIME=true -ti drwetter/testssl.sh internet.nl The 131 seconds are split like this:
If the code setup is moved more to docker and small tasks, some sub-test could be done with testssl in worker-nodes. Probably if it's properly split in sub-tasks, it can be fast when the sub-tasks run parallel. Since testssl is only a thin wrapper around openssl, it's probably first to have new features. |
@gthess do you have more thoughts on this that we should know? (We discussed this before, your earlier comments are in the initial description.) |
Hi @mxsasha, I was following this issue and although I don't have time to go into detail, based on your elaborate analysis I also believe that sslyze seems the way to go. It could also simplify the installation procedure. Being based on nassl, I assume its raw clients could be used to do internet.nl specific tests if they are not supported by sslyze; not preferred but good to know if that is a possibility. "Limit which ciphers we test for" and "Terminate test on first violation" sound like easy additions to sslyze. "Cipher order preference on server" not so but that could be a case where the raw nassl clients are used to do manual testing. My main concern is email testing where most restrictions are required like no parallel connections and fail early. |
I. It seems the command line option for testing a mail server is II. To prevent running into rate limits there seem to be two useful settings:
Source: nabla-c0d3/sslyze#385 and
Source: https://github.com/nabla-c0d3/sslyze/releases/tag/3.0.0 |
…in bad ssl While using a standard HTTP client is good, it does mean we can't connect to some very obscure setups with it anymore. In the case of cert detection, the HTTP client was exclusively used for guessing if there even is any SSL, while the rest of the code can still handle very bad configs. As we needed our legacy clients in a few places anyways until finishing #714, this commit reverts the cert check back to the legacy client, allowing cert checks in these very bad configs.
…in bad ssl While using a standard HTTP client is good, it does mean we can't connect to some very obscure setups with it anymore. In the case of cert detection, the HTTP client was exclusively used for guessing if there even is any SSL, while the rest of the code can still handle very bad configs. As we needed our legacy clients in a few places anyways until finishing #714, this commit reverts the cert check back to the legacy client, allowing cert checks in these very bad configs.
Obsoleted by next comment, keeping for reference:Based on our latest discussions, I've been testing testssl and sslyze tools more, and found the following:
This is all kind of anecdotal as server behaviour can also be a bit unpredictable. However, it seems sslyze is a great choice for web, testssl is best for mail, and both are missing an essential feature. Thoughts on paths forward:
Therefore, perhaps our best path forward is to utilize both tools depending on the type of test. This is not great - it adds complexity and it means some detection will use different code for web vs mail. But I think it is the least bad so far. This may affect our ability to test some mail servers that accept our current test but not testssl. Alternatives (which all also require implementing zero-rtt in testssl or server cipher preference in sslyze):
Update: I just now re-read baknu's comment on "2. An option to control the number of concurrent connections per single server (since version 3.0.0)" - there is no command line option so I missed it. Will try it. |
Update: with the (not exposed through command line) connection limit that I entirely overlooked, sslyze seems to perform for mail similarly to testssl based on my limited dataset. I will move forward with an experimental implementation from there so we can compare it in a larger dataset. Still, it seems there are at least some mail hosts that succeed in current internet.nl, but fail in both testssl and sslyze, and sslyze is still missing cipher preference detection. |
Thanks for the update.
Is this the setting that I referred to under 2 in the following comment: #714 (comment) ?
Nice. We maybe can use the gov domain set and the Tranco 5000 .NL domain set for comparison/reference. See public reports on https://dashboard.internet.nl/ I have asked EJ to enable the mail test for the Tranco list too.
So does Internet.nl make fewer connections in total to get a result than testssl and sslyze? Or is Internet.nl making fewer connections per time unit?
I believe the issue that is referred to in nabla-c0d3/sslyze#338 (comment) has been encountered and tackled in the Internet.nl project: #461 Maybe our code could be an inpiration to get this fixed in sslyze. Furthermore, I was thinking about our current "Terminate test on first violation" functionality that you mention in your comment on #714 (comment). I believe (but am not sure) that this only goes for the "Cipher order" subtest and not for the "Ciphers (Algorithm selections)" subtest. In the test explanation for "cipher order" the following is stated:
|
Yes.
Yes, that would be good.
Those ciphers are now included in at least testssl, but there are so few it is unlikely to make any significant difference. I think our own code makes fewer connections by trying a little harder to get maximum data from each connection. I can't say how big the difference is though and how much effect it has.
Ah right, may be. |
Our TLS tests currently depend on our own fork of nassl (the internetnl branch). This fork is based on a 4 year old version of nassl, built on a fork of an older openssl version. The build process is fragile, and this is not a sustainable setup. We have already talked/mailed about this - this issue is meant to summarize plans and thoughts about improving this.
The custom nassl fork adds support for TLS 1.3, ChaCha20 and Poly1305. These have since been added to nassl itself, so we likely do not need our own fork anymore. However, there are also API differences, which may be APIs we added to our own fork, and/or changes in nassl in the last years.
In addition to nassl dependencies, our TLS checks are a few thousand lines of our own code. From what I’ve seen so far, a lot of this code also has a high complexity. Together with the vague dependencies and their build process, this makes TLS one of the more complex and fragile parts of the project.
Built on top of nassl is sslyze, an SSL/TLS scanning tool by the same author. I played around with it a bit, and I am impressed by how thorough it is, and the interface is really neat. Potentially, we could replace a lot of our code with calls to sslyze instead. It is currently not feature complete enough for us, but perhaps we could build some features and contribute them back. It does make judgements about the results, but can report enough raw data that we can still apply our own rules and scoring. This is an example JSON output to show the kind of checks and detail.
@gthess contributed this list of probable requirements:
names?
"check_server" function that runs the whole thing? If the latter is
the case, can checks be conditionally turned on/off for a domain? For
example to limit connections/testing time when testing a mail server
for checks that are not relevant for mail servers e.g., OCSP-stapling.
(consecutive/parallel). Especially for email we want the connections
to be done in series.
we only care for non good ciphers we don't want the test to waste
connections (and possibly hitting connection limits) on testing
ciphers that we don't want to take any action about. Another example
is to stop the cipher test on the first offending cipher; no need to
waste connections if we know the verdict already.
compression/HSTS/HTTP header tests). For these we use the clients that
nassl provides for initiating the TLS connection. We would still need
to use these for connecting via HTTPS (we can't rely on python's
requests for example in case the TLS connection is considered
insecure/old for the system's cryptolibraries)
This bullet point could be up for discussion to fail those HTTP tests if a
TLS connection cannot be made by a modern system.
Personally I do not think the last bullet should be a hard requirement. If your TLS setup is so exceptionally broken that an ordinary Python client can't connect anymore, I feel fine with reporting that we skipped some tests - because whatever those tests would report, it isn't remotely the main issue that site is having.
Also important to consider some configuration cases like #753.
Currently sslyze does not support cipher order, but this is planned, so that is one clear missing feature.
I see a few potential paths:
Some mix may also be a good choice. It's probably worth exploring all options in some more detail.
(Note that our nassl fork does have a "how to keep the fork up to date" section in the readme, but this only updates the main branch, not our custom branch).
The text was updated successfully, but these errors were encountered: