PR #346 added a test for version tolerance to `run_protocols()`, but I think it may now be more appropriate to remove that test. Draft -16 of TLS 1.3, which was posted on September 22, changed the way that version negotiation is handled for TLS 1.3 and above. The current version tolerance test sends a ClientHello with the version field set to "03, 05", to represent a TLS 1.4 ClientHello. While this was consistent with RFC 5246 and with drafts of TLS 1.3 up to -15, draft -16 changed the version field to `legacy_version` and declared that its value should be "03, 03" for TLS 1.2 and above. (For TLS 1.3 and above a Supported Versions extension is included to inform the server which versions of TLS the client supports.) The change in draft -16 was made as a result of the problems with servers not handling version negotiation correctly.
Since the current draft suggests that a server should never be presented with a ClientHello with a version higher than "03, 03" (even for clients that support TLS versions higher than 1.2), it seems there is no reason to include the version tolerance test anymore.
For servers that do not support TLS 1.2, the additional checks that were added by PR #346 will already detect if the server cannot perform version negotiation correctly.
s_client's manpage states for -nextprotoneg:
"Empty list of protocols is treated specially and will cause the client
to advertise support for the TLS extension but disconnect just after
reciving ServerHello with a list of server supported protocols."
Consequently, the previous workaround of just quoting an empty variable
is insufficient and the "-nextprotoneg" parameter has to be removed
entirely from the command-line in case of an empty argument.
In other locations where "-nextprotoneg" is used
- its argument cannot be empty ($NPN_PROTOs is initialized to a non-
empty value and set read-only) or
- its argument is intended to be empty (line 3724) or
- the command will not be invoked at all (for-loop parameter, line 3725)
This fixes#467 - again.
Additionally this patch prefers usage of -alpn over -nextprotoneg if the
openssl binary used supports it.
Refactor the while loop so it doesn't use a subshell anymore. Also use
"read -r" to prevent backslash escaping.
```
In testssl.sh line 1193:
app_banners="$app_bannersline"
^-- SC2030: Modification of app_banners is local (to subshell caused by pipeline).
In testssl.sh line 1195:
fileout "app_banner" "WARN" "Application Banners found: $app_banners"
^-- SC2031: app_banners was modified in a subshell. That change might be lost.
```
Found by ShellCheck.
This commit fixes the following two instances of referenced but not assigned
variables:
```
In testssl.sh line 1159:
rp_banners="$rp_bannersline"
^-- SC2154: rp_bannersline is referenced but not assigned.
In testssl.sh line 1193:
app_banners="$app_bannersline"
^-- SC2154: app_bannersline is referenced but not assigned.
```
Found by ShellCheck.
The argument to -nextprotoneg is provided in sometimes empty an unquoted
variables. Because of the missing quotes, the next word on the line "-status"
gets parsed as "-nextprotoneg"'s argument instead of enabling the OCSP status
check.
This fixes#467.
Fix referenced but not assigned variable 'sign_algo'.
In testssl.sh line 4309:
fileout "${json_prefix}algorithm" "DEBUG" "Signature Algorithm: $sign_algo"
^-- SC2154: sign_algo is referenced but not assigned.
Found by ShellCheck.
