socksend_tls_clienthello() does not calculate the length of the ClientHello message in the case of a TLS 1.3 ClientHello, since it does not take into account the inclusion of a 32-byte session id. The length value that is being calculated incorrectly is only used to determine whether to include a padding extension, and if so, how long that extension should be.
This fix was previously included as part of PR #1120, since a correct length calculation is needed to avoid a ClientHello length such that length mod 256 = 10, but I removed it from that PR and am making it a separate PR, since it is a bug that should be fixed even if #1120 isn't adopted.
This commit updates the size bug GREASE test in a few ways:
* It removes the changes to socksend_tls_clienthello() - these will be submitted as a separate PR.
* It adds a test for a ClientHello message length of 266 bytes, but only if the server can generally handle messages with lengths between 256 and 511 bytes.
* It corrects the calculation of the length of the padding extension in cases in which a TLS 1 or TLS 1.1 ClientHello is being sent.
Just as some servers will fail if the length of the ClientHello is between 256 and 511 bytes (see RFC 7685), it seems that some servers (or a middlebox sitting in front of the servers) will fail if the length of the ClientHello is 522, 778, 1034, ... bytes in length (i.e., if length mod 256 = 10). I have also encountered one server that will also fail if the length of the ClientHello is 526, 782, 1038, ... bytes in length (i.e., if length mod 256 = 14).
In the case of that one server, the first ClientHello sent by run_pfs() was exactly 1038 bytes, and so run_pfs() was reporting that the server didn't support any PFS ciphers even though it did..
This PR addresses the problem in two ways. First, it modifies socksend_tls_clienthello() so that if the length of the ClientHello would be more than 511 bytes and length mod 256 would be 10 or 14, it adds a 5-byte padding extension in order to ensure that the final length of the ClientHello will not be a length that could trigger the bug.
Second, this PR adds a test to run_grease() to send ClientHello messages of the exact lengths that do trigger the bug so that users can be made aware that their servers have the problem.
In cases where a finding was empty (error condition), the JSON output
wasn't valid because the finding wasn't printed to file.
This commit makes sure that always a finding is printed,
also if it is empty.
FIX#1112
As #1119 noted, there's a warning for users with an OpenSSL 1.1.1
config file because of #1117 / #1098 .
This commit suppresses the warning on the screen if a config file
from OpenSSL 1.1.1 was detected (kludge from
b524b808a1).
This addresses a bug where openssl s_client connects hiccuped
because of newer config files which our openssl 1.0.2 couldn't
swallow.
It appeared first on Debian.
FIX#1117FIX#1098
RFC 8446 specifies the following for the list of certificates provided by the server:
The sender's certificate MUST come in the first
CertificateEntry in the list. Each following certificate SHOULD
directly certify the one immediately preceding it.
In RFC 5246 the "SHOULD" was a "MUST". This commit adds a check of whether the certificates provided by the server are in the correct order and issues a low severity warning if they are not.
As mentioned in #1106 proxying ocsp protocol doesn't work (yet)
This commit notifies the user that it is not possible. One
can ignore that and try by supplying IGN_OCSP_PROXY=true.
It also fixes a typo I probably introduced (pVULN_THRESHLD).
The standard separator after $FNAME_PREFIX is now '-'.
You can as well supply a different <fname_prefix> ending in '.', '_' or ',' , then
no no additional '-' will be appended.
Also a small bash function get_last_char() has been introduced which returns
the last char from a supplied string.
... for curl, wget and sockets. Tested and worked.
Furthermore: fd_socket() now is a bit more injection safe as
an echo statement was exchange by printf. For possible future
changes fd_socket now also has and arg1 for the file descriptor.
... previously it depended on the order of DNS replies otherwise. This was
one outcome of discussion in #1026 where it seemed more logical
to pick an IPv6 address as opposed to an abitrary (v4/v6) address.
This PR fixes checks where those two cmdline options were supplied
but errorneously also the IPv4 address was tested.
It also lables supplied IPv6 addresses as AAAA records
instead of A records.
Still, determine_ip_addresses() has space for improvements.
Some comparisons fixed strings popped up during debugging were polished
to avoid internal quoting
[[ $VAR == "teststr" ]]
will be otherwise expanded to
[[ $VAR == \t\e\s\t\s\t\r ]]
This PR changes run_logjam() so that it does not warn about the use of 2048-bit DH primes, even if the selected prime is a common prime.
This PR leaves two issues unaddressed. First, it does not detect servers that are vulnerable to Attack IV in https://weakdh.org/logjam.html. These are servers that use DH primes that are of sufficient length, but that are poorly generated, and so are still vulnerable to attack.
Second, it does not address the potential problem that use of a common prime could leak information about what server product is being used, even if this information is not leaked through other means (e.g., HTTP headers). This should not be an issue with common primes from an RFC (2409, 3526, 5114, 7919), but would be an issue with product-specific common primes.
This commit fixes a bug mentioned in #1084 where a server
with multiple host certificates wa missing a certificate
number the the host certificate itself.
It also adds a JSON object for the number of host certificates.
According to Section 7.4.2 of RFC 5246, when a server sends its certificate it MUST send a list in which the first certificate is the sender's certificate and "Each following certificate MUST directly certify the one preceding it." testssl.sh currently assumes that the server has populated the list way and so places the second certificate in the list into $TEMPDIR/hostcert_issuer.pem.
However, not all servers have been following this requirement, and so draft-ietf-tls-tls13 (soon to be RFC 8446) only says that servers SHOULD list the certificates in this way and says that clients "SHOULD be prepared to handle potentially extraneous certificates and arbitrary orderings from any TLS version, with the exception of the end-entity certificate which MUST be first."
testssl.sh needs to place the correct certificate in $TEMPDIR/hostcert_issuer.pem, since otherwise any OCSP request it sends will be incorrect, and any attempt to verify and OCSP response will be incorrect as well.
This PR changes extract_certificates() and parse_tls_serverhello() to populate $TEMPDIR/hostcert_issuer.pem with the first certificate in certificate_list that has a subject DN that matches the issuer DN in the server's certificate, rather than simply populating $TEMPDIR/hostcert_issuer.pem with the second certificate in the list.
In testing a random sampling of U.S. government servers, of 57 servers tested 5 reported "unauthorized" for the OCSP URI using the current testssl.sh and all 5 of these reported "not revoked" with this PR. This PR also corrects the same issue in some servers on the Alexa Top 1000, but this was only a problem for 12 of those 1000 servers.
In cases in which the server offers a stapled OCSP response, this commit extracts the OCSP response and then checks the response for the status of the server's certificate. The check is performed in the same way as when the certificate includes an OCSP URI and the "--phone-out" option is set, except that the OCSP response is received from the TLS server rather than coming directly from the OCSP responder. Since this only involves additional processing of data that testssl.sh is already receiving, the check is performed whether or not the "--phone-out" flag is set.
Reduce the offensive tests to 4: the others are "just" / mostly cipher
based checks which should not cause an IDS to block. (This maybe
subject to reconsider at a later time.)
Added a switch --ids-friendly
Updated VULN_COUNT accordingly
Added this (including PHONE_OUT to env debugging output)
Added help()
Manual section added
David Cooper
Dirk