Find supported_groups extension

[Odinmylord]

Describe your changes

One thing I noticed is that since testssl.sh does not check the extensions sent while discovering the supported curves, it may miss the "supported_groups" extension sent by the server. This happens because the server only sends the extension "If the server has a group it prefers to the ones in the "key_share" extension but is still willing to accept the ClientHello" [RFC8446]. According to my tests when testssl.sh sends the "normal" ClientHello to check for extensions the server does not send the "supported_groups" extension because it finds the curves it wants in the ClientHello. If the order of the curves is different from the one defined in the configuration the server sends the extension. The solution I used is to add the "-tlsextdebug" flag while finding the curves and the OpenSSL binary is used. For the TLS sockets I added two additional requests (with all+ as the process_full level) one with the curve secp384r1 as the first one and the secp256r1 as the last one (since it is the most commonly used), if after this request the extension is not found another request is performed after moving secp384r1 to the last place. If after both requests the server does not send the extension we can assume that it does not send it.

What is your pull request about?

• Bug fix
• Improvement
• New feature (adds functionality)
• Breaking change (bug fix, feature or improvement 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, indentation is five spaces and any line endings do not contain any blank chars
• I've read CONTRIBUTING.md and Coding_Convention.md
• I have tested this fix or improvement 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]

Hi @Odinmylord,

Thanks for submitting the PR. I had noticed that the set of TLS extensions listed by testssl.sh for a server isn't always the same. Using a different version of OpenSSL for the testing can affect the list. It may be that the only difference is whether or not the supported_groups extension is listed, but I would have to do some testing to see if I can find any other extensions that sometimes appear and sometimes don't.

I think there is a question about the purpose of the "TLS extension (standard)" output. For most extensions, the server will include the extension in the ServerHello, if it supports the extension. However, a server can support and process the supported_groups extension without including it in the ServerHello. So, what information are we providing by noting that a ServerHello included a supported_groups extension? I think that needs to be answered in order to determine whether it makes sense to go to extra effort to try to get the server to send a ServerHello with a supported_groups extension.

I haven't had a chance to look closely at your PR, but if it is agreed that "TLS extension (standard)" should always include "supported_groups/# 10" if the server can be made to send a ServerHello with that extension, then this PR as written will not work. The "TLS extension (standard)" information is output by run_server_defaults(), but your code changes were made in run_fs(). This will provide the expected behavior if a full run of testssl.sh is performed, or at least if both --forward-secrecy and --server_defaults are run. However, ideally server defaults will output the same information whether a full run of testssl.sh is performed or one just runs testssl --server_defaults <URI>.

[dcooper16]

Another issue that occurred to me is that there are a few functions that test for vulnerabilities that check whether $TLS_EXTENSIONS is empty, and if it is run determine_tls_extensions(). These functions need to know whether the server supports specific extensions in order to know whether the server might be vulnerable. If run_fs() adds extensions to $TLS_EXTENSIONS, but might potentially not add an extension that is relevant to one of these other functions, that could pose a problem.

[dcooper16]

Note that since #2689, $TLS_EXTENSIONS is now an array, so this check will not work (it will only look at the first element in the array). So, this line should be changed to use ${TLS_EXTENSIONS[*]}. For consistency with other checks for whether a string contains a certain substring, I would suggest changing this to:

[[ ! "${TLS_EXTENSIONS[*]}" =~ supported_groups ]]

[Odinmylord]

Hi @dcooper16,
I put the code in the run_fs function since some request, with different supported_groups values, are already performed there.
The supported_groups extension only appears in the TLS1.3 Encrypted Extensions messages and only if the server wants to share some preferences (not to be used in the current handshake) regarding the supported_groups values.
Knowing that the handshake does not depend in any way on this extension, what do you think would be the best course of action?