Add support for signature_algorithms_cert extension

[Odinmylord]

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]

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]

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]

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]

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]

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]

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]

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.

[Odinmylord]

Hi @dcooper16,
it took me a while but I finally completed the new feature. I managed to handle the CertificateRequest message in the TLS sockets. In order to check whether the client certificate is required or optional the TLS socket now sends an empty Certificate message and checks the response, if it gets an Alert 116 certificate_required the client Certificate is required.
With these additions the code should be ready for the final review and the merge.

[drwetter]

Hi @dcooper16 ,

you mind reviewing this?

[dcooper16]

Hi @drwetter,

Reviewing this is on my "to do" list, but I don't have time at the moment.

[dcooper16]

I've commented on some issues in the code, but have not had a chance to review everything yet, particularly the new code in tls_sockets(). I think there are some issues with that code, but I would need more time to remember how that function works in order to better understand the new code.

As it is though, all of the new code in tls_sockets() and the code setting $CLIENT_AUTH in parse_tls_serverhello() only seem to be necessary in the case of a TLS 1.3-only server and an $OPENSSL that does not support TLS 1.3. If that is the case, then it seems the need for the code is rather limited, particularly now that testssl.sh automatically switches to using the system OpenSSL in that case if the provided $OPENSSL doesn't support TLS 1.3. In cases where it can't do that, the user is warned to expect incorrect results. Besides, even if it is useful, it could be considered a separate PR, since it is not about determining the set of supported signature algorithms for client authentication.

[dcooper16]

= should be replaced with ==.

[dcooper16]

out_row_aligned() here and on lines 10368 and 10372 is the wrong function (also out_row_aligned() requires two parameters). Use out_row_aligned_max_width():

out_row_aligned_max_width "$CLIENT_AUTH_SIGALGS_LIST_13" "                              " $TERM_WIDTH

[dcooper16]

The discussion in #2568 reminded me that the output from out_row_aligned_max_width() needs to be captured and then printed. For example,

outln "$(out_row_aligned_max_width "$CLIENT_AUTH_SIGALGS_LIST_13" "                              " $TERM_WIDTH)"

[dcooper16]

This is okay, but isn't really needed. Setting $jsonID and then using $jsonID in fileout() is really only useful if the same identifier is used in more than one call to fileout().

[dcooper16]

No need to check if [[ "$process_full" =~ all ]]. Given line 14224, $tls_certificate_request_ascii_len will be 0 if $process_full does not contain all.

[dcooper16]

This part of the comment needs to be updated, as the output may not be from OpenSSL.

[dcooper16]

Replace = with ==.

[dcooper16]

I don't understand what this comment line is supposed to mean.

[dcooper16]

This code seems to only work as expected if the server supports both TLS 1.2 and TLS 1.3. Need to take into account servers that only support one of these. Also need to consider servers that support client authentication, but only support TLS 1.1 and earlier.

[dcooper16]

I think the information only needs to be sent to fileout() if the corresponding protocol is supported. (It may be okay to omit this whenever the value is "empty".)

[dcooper16]

I don't understand this. If $OPENSSL and the server support TLS 1.2, then sclient_auth() will correctly determine whether the server sends a CertificateRequest message. It will not fill in CLIENT_AUTH_SIGALGS_LIST_13 and CLIENT_AUTH_SIGALGS_CERT_LIST_13. But then, if $OPENSSL and the server both support TLS 1.3, the code above will not fill in CLIENT_AUTH_SIGALGS_LIST_12, and the code below doesn't address that.

Also, this code does nothing in the case that $OPENSSL does not support TLS 1.3 and the server only supports TLS 1.3. Running it seems inappropriate when $CLIENT_AUTH is unknown.

It seems that this code isn't needed. In the case that the server supports TLS 1.2, but $OPENSSL does not, calls to tls_sockets() from within get_server_certificate() will already set the necessary variables. So, there is no need for a special call here.