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 support for signature_algorithms_cert extension #2481

Draft
wants to merge 5 commits into
base: 3.2
Choose a base branch
from

Conversation

Odinmylord
Copy link
Contributor

@Odinmylord Odinmylord commented Mar 22, 2024

Describe your changes

I noticed that at the moment testssl.sh does not provide the list of signature algorithms that the server offers only for client certificate verification. It is important to have both the original list and the shared list so that it is possible to know which algorithms the server offered and which the client approved.
Don't know if it is enough to get added to CREDITS.md so I did not do that.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Typo fix
  • Documentation update
  • Update of other files

If it's a code change please check the boxes which are applicable:

  • For the main program: My edits contain no tabs and the indentation is five spaces
  • I've read CONTRIBUTING.md and Coding_Convention.md
  • I have tested this fix against >=2 hosts and I couldn't spot a problem
  • I have tested this new feature against >=2 hosts which show this feature and >=2 host which does not (in order to avoid side effects) . I couldn't spot a problem
  • For the new feature I have made corresponding changes to the documentation and / or to help()
  • If it's a bigger change: I added myself to CREDITS.md (alphabetical order) and the change to CHANGELOG.md

@dcooper16
Copy link
Contributor

Hello @Odinmylord,

Thanks for submitting this PR. I had started work on something similar a long time ago, but never had the time to finish it.

I haven't tested you PR, but I looked at the code and also tried a few calls to openssl s_client against a server configured for client authentication. Based on that, I have a few comments.

It seems that you did not follow the coding style for this project. So, please read Coding_Convention.md.

Have you tried this with different versions of OpenSSL? I tried using OpenSSL 3.0.2, OpenSSL 1.0.2u, and LibreSSL 3.8.1, and got different results for each one.

There were two reasons for the differences between OpenSSL 3.0.2 and OpenSSL 1.0.2u. First, if OpenSSL doesn't recognize one of the listed algorithms, it just provides its two octet code (e.g., "0x07+0x08") rather than a meaningful string name. Since OpenSSL 3.0.2 recognizes more algorithms, fewer are listed by their octet codes. The second issue is that the list of requested algorithms differs depending on whether the negotiated TLS version is 1.2 or 1.3.

LibreSSL, for some reason, responded "No client certificate CA names sent," even though a list was sent and extract_calist() was able to extract them from the LibreSSL's output (when the -msg option is used).

Given the different behaviors of the different versions of OpenSSL/LibreSSL, in the PR that I started to write I extracted the list of signature algorithms from the raw encoded response, just as extract_calist() currently extracts the list of CA names.

One of the reasons that I never completed the PR is that it is not as simple as just using sclient_auth() to extract the list of signature algorithms. As I noted above, the list will differ depending on whether TLS 1.3 or TLS 1.2 (or an earlier version) is negotiated. So, if the server supports TLS 1.3, the list that sclient_auth() sees will depend on whether the version of OpenSSL that testssl.sh uses supports TLS 1.3. Also, if the server supports both TLS 1.3 and TLS 1.2, then testssl.sh should really show the list of signature algorithms offered with each version, but sclient_auth() can only collect the information about one version.

Finally, I do not understand why you say:

It is important to have both the original list and the shared list so that it is possible to know which algorithms the server offered and which the client approved.

The shared list that you are printing seems to be the list of algorithms that the server and the OpenSSL that testssl.sh is using have in common. It's not clear how this is useful, as people using testssl.sh are interested in discovering information about the server they are testing, not about the version of OpenSSL they are using to perform the test.

If you'd like, I could post the PR that I started, so that you could work from it. However, it is very much a work in progress. It doesn't yet do everything needed to get the information from the response being parsed by sclient_auth() and it doesn't at all deal with the issue of servers that support both TLS 1.3 and TLS 1.2.

@Odinmylord
Copy link
Contributor Author

Hello @dcooper16,
First of all thank you for the explanation, I did not consider the issue related to the different openssl/libressl versions, and will surely do in the future. Regarding the shared list, I agree with you that it is useless, it was a mistake on my end. Even if you say that it is still a work in progress, for me it would be a pleasure to contribute to the work that you have already started.

You may also have noticed that this is not the first PR that I opened on this repository, this and the other issues were discovered while working on TLSAssistant, a tool developed by the Security & Trust unit of the Center for Cybersecurity at Fondazione Bruno Kessler. By integrating other open source tools (including testssl itself), our framework analyzes TLS configuration and provides actionable hints that can assist the user in correctly and easily fixing them. The latest release also added a compliance module that aims to provide a report on the compliance status of the configuration against one or multiple guidelines (AgID, ANSSI, BSI, Mozilla, NIST).
Here is the link to the dataset resulting from the gathering, translation, standardization and structuring of the technical requirements extracted from the five cybersecurity agencies' guidelines. Given your involvement in the NIST SP 800 52 Rev. 2 document, it would be great if you can validate our gathering and share with us your thoughts and criticisms to improve it. I am really looking forward to hearing from you.

@dcooper16
Copy link
Contributor

Hello @Odinmylord,

I placed my initial work on a PR at dcooper16@2ff7891. It has been quite a while since I looked at this, but I tried my best in the commit text to list the problems with the current code. One problem I should have listed is that the code needs more explanatory comments. :-)

Looking my code and at RFC 8446 further, it seems that I made some mistakes in my comments. First, it seems that this does not apply to TLS 1.1 or earlier, only TLS 1.2 and TLS 1.3. This isn't particularly significant, but it means that there are lists of algorithms that apply to TLS 1.3, lists that apply to TLS 1.2, and that is it.

Second, it seems that the situation is more complicated than I thought. In TLS 1.3, both the signature_algorithms and signature_algorithms_cert extensions are in the extensions field of the CertificateRequest message. So, changing my code to extract the signature_algorithms_cert extension from the TLS 1.3 response should be easy. In the case of TLS 1.2, however, the main list of supported signature algorithms is in the CertificateRequest message (and is already parsed by the code in my commit). But, if a signature_algorithms_cert extension was also included, it would seemingly be in the extensions field of the ServerHello message. While it would be possible for extract_calist() to get this information, it wouldn't be nearly as easy, since extract_calist() currently does not parse the ServerHello message at all.

Despite its limitations, feel free to take anything from the commit that I posted, if you think any of it is useful.

I don't have any spare time right now, but I will definitely try to take a look at your work at some point.

@Odinmylord Odinmylord marked this pull request as draft April 2, 2024 09:56
@Odinmylord
Copy link
Contributor Author

Hi @dcooper16,
I started to check what would be needed in order to actually implement the new features we discussed and I discovered the following considerations.
Regarding the Signature Algorithms:

  • neither LibreSSL nor OpenSSL send the signature_algorithms_cert extension source . This means that the value should be retrieved from the signature_algorithms extension for TLS1.3 and from the CertificateRequest message (as testssl.sh already does) for TLS1.2. In order to make the code future proof, the signature_algorithms_cert extension should also be read if present;
  • retrieving the signature algorithms from the CertificateRequest message would be easy using your code. Additionally, since the values are extracted from the transmitted message, we won't need the data parsed by the library;
  • I think that the best course of action to retrieve the signature algorithms would be to always read them from the "determine_optimal_proto" temp file. Regarding the issue of both TLS1.2 and TLS1.3 being present, I think that the best course of action would be to keep the TLS1.2 handshake from the "determine_optimal_proto" so that it can be used later to retrieve the required information.

Regarding the other aspects:

  • I noticed that testssl.sh does not list the extensions found in the CertificateRequest message (in the case of TLS1.3);
  • the issue you found with LibreSSL could be related to how it handles the CertificateRequest extensions field. Since testssl.sh reads the "raw" data from the message it is not affected by how the library handles the extension's data.

@dcooper16
Copy link
Contributor

neither LibreSSL nor OpenSSL send the signature_algorithms_cert extension source . This means that the value should be retrieved from the signature_algorithms extension for TLS1.3 and from the CertificateRequest message (as testssl.sh already does) for TLS1.2. In order to make the code future proof, the signature_algorithms_cert extension should also be read if present;

Yes, it seems that OpenSSL can process the signature_algorithms_cert extension, but it cannot generate it. openssl/openssl@e639c37 explains why OpenSSL does not need to generate this extension.

Ideally testssl.sh should present the information in both extensions. However, at the moment testssl.sh doesn't present the list of algorithms that the server includes in the signature_algorithms extension (the ones it will accept for client authentication), so having testssl.sh do that would be progress. (It just wouldn't match the title of this PR).

It would be a nice enhancement to also parse the signature_algorithms_cert extension, for cases in which the server sends both the signature_algorithms and signature_algorithms_cert extension. Unfortunately, if OpenSSL cannot generate this extension that will make testing more difficult.

I think that the best course of action to retrieve the signature algorithms would be to always read them from the "determine_optimal_proto" temp file. Regarding the issue of both TLS1.2 and TLS1.3 being present, I think that the best course of action would be to keep the TLS1.2 handshake from the "determine_optimal_proto" so that it can be used later to retrieve the required information.

Yes, I was thinking something like that as well. If the server supports both TLS 1.2 and TLS 1.3, then get the information for one of them from determine_optimal_proto() and then get the other one at some other point. However, as I noted, whether determine_optimal_proto() gets the TLS 1.2 or TLS 1.3 handshake will depend on which version of OpenSSL/LibreSSL is being used.

I noticed that testssl.sh does not list the extensions found in the CertificateRequest message (in the case of TLS1.3);

True. I guess that would be another potential enhancement -- to list all of the extensions in the TLS 1.3 CertificateRequest message, not just the contents of one or two of the extensions. Again, any such enhancement would have to work even if the OpenSSL being used does not support TLS 1.3.

@Odinmylord
Copy link
Contributor Author

Odinmylord commented Apr 11, 2024

Unfortunately, if OpenSSL cannot generate this extension that will make testing more difficult.

Regarding this issue the solution could be to use https://github.com/tlsfuzzer/tlslite-ng, I opened a PR to make it possible for a server started with that lib to send the signature_algorithms_cert extension.

Again, any such enhancement would have to work even if the OpenSSL being used does not support TLS 1.3.

I don't think that this would be possible because if the client does not support TLS 1.3 the handshake does not get to the step of the CertificateRequest. The CertificateRequest extensions are sent only with TLS 1.3 so a TLS 1.2 CertificateRequest does not contain them.

Assuming this I'll update this PR, using your code as a base, to output the list of signature_algorithms of the CertificateRequest. After this one I'll do another one for the CertificateRequest extensions.

EDIT: In order to maintain the code attribution, would you prefer to fork this repository, add your code, and then let me add the new feature, or (to avoid you additional work) let me put a set of comments in the source giving you attribution?

@dcooper16
Copy link
Contributor

Hi @Odinmylord,

Sorry for being so slow to respond, but I don't have much time to look at testssl.sh at the moment.

Again, any such enhancement would have to work even if the OpenSSL being used does not support TLS 1.3.

I don't think that this would be possible because if the client does not support TLS 1.3 the handshake does not get to the step of the CertificateRequest. The CertificateRequest extensions are sent only with TLS 1.3 so a TLS 1.2 CertificateRequest does not contain them.

It would be possible, but it would involve some work. testssl.sh does a lot of testing using tls_sockets(), which is testssl.sh's partial implementation of the TLS handshake. It can send a TLS 1.3 ClientHello and parse the response from the server. It can even decrypt the portions of the TLS 1.3 handshake that are encrypted. At the moment, however, it does not parse the CertificateRequest message, so code would have to be written to do that. tls_sockets() is a lot slower than $OPENSSL s_client, which is one of the reasons that testssl.sh sometimes uses $OPENSSL s_client, but tls_sockets() allows testssl.sh to test for things that are not supported by $OPENSSL.

EDIT: In order to maintain the code attribution, would you prefer to fork this repository, add your code, and then let me add the new feature, or (to avoid you additional work) let me put a set of comments in the source giving you attribution?

Thanks for considering this, however, I'm not concerned about whether I get credit for what's in dcooper16@2ff7891. Feel free to copy from the changes I made in that commit, to the degree they are useful, but there's no need for there to be comments or a commit history indicating that any of it came from me.

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.

None yet

2 participants