This PR is an attempt to fix the problem identified in #1118.
Currently, get_cipher() and get_protocol() attempt the extract the cipher and protocol from the SSL-Session information printed by OpenSSL s_client. This does not always work for TLSv1.3, however, since OpenSSL 1.1.1 will only print SSL-Session information for a TLSv1.3 connection if the server sends New Session Ticket. If the server doesn't, then get_cipher() and get_protocol() return empty strings.
For TLSv1.3 connections in which the server does not send a New Session Ticket, there seems to be only one other source for this information. A line of the form:
New, TLSv1.3, Cipher is TLS_AES_256_GCM_SHA384
[Note that "New" would be "Reused" if the connection were created via session resumption.]
The use of this line seems to be okay for extracting the negotiated cipher, but it cannot be used in general to extract the negotiated protocol. The reason is that this line is created as follows:
c = SSL_get_current_cipher(s);
BIO_printf(bio, "%s, Cipher is %s\n",
SSL_CIPHER_get_version(c), SSL_CIPHER_get_name(c));
While the cipher that is printed seems to be the negotiated cipher, the protocol that is printed is "the SSL/TLS protocol version that first defined the cipher." Since TLS 1.3 ciphers may only be used with TLS 1.3, protocol version printed on this line may be accepted as the negotiated protocol if and only if it is "TLSv1.3."
This PR addresses the problem by modifying get_cipher() and get_protocol() to check the "New, ..., Cipher is ..." line if lines from SSL-Session ("Cipher : ...", "Protocol : ...") cannot be found. In the case of get_protocol() the protocol on the "New, ..., Cipher is ..." will be accepted only if the protocol is "TLSv1.3" and the cipher is a TLSv1.3 cipher.
This PR also adds a check for the "New, ..., Cipher is ..." to sclient_connect_successful(). If this line is present, and the protocol and cipher are not "(NONE)", then this is accepted as an indication that the connection was successful even if the "Master-Key" line does not appear. It is not clear whether this extra test is needed, however, as sclient_connect_successful() will not even look at the text in the output of OpenSSL s_client if function's return value is 0, and OpenSSL s_client should return 0 if the connection was successful.
Most of the curves that were defined for the supported_groups extension in RFC 4492 have been deprecated in RFC 8422 and RFC 8446. Appendix B.3.1.4 of RFC 8446 says that these deprecated values "are used in previous versions of TLS and MUST NOT be offered or negotiated by TLS 1.3 implementations."
According to a recent discussion on the TLS mail list (see, for example, https://www.ietf.org/mail-archive/web/tls/current/msg26974.html and https://www.ietf.org/mail-archive/web/tls/current/msg26980.html) a TLS 1.3 server implementation may choose to reject a TLS 1.3 ClientHello simply because the ClientHello offers one or more of the deprecated curves.
This PR address this issue by no longer offering the deprecated curves in TLS 1.3 ClientHello messages. This only affects run_pfs(), since socksend_tls_clienthello() already does not offer the deprecated curves in TLS 1.3 ClientHello messages.
The change in this PR has no affect on the testing of servers that do not support TLS 1.3. For those that do support TLS 1.3, only the 5 non-deprecated curves are tested with TLS 1.3, but all 30 curves are tested with TLS 1.2.
This PR reduces the number of public keys that are included in the key_share extension for a TLS 1.3 ClientHello.
When creating the key_share extension for a TLS 1.3 ClientHello, generate_key_share_extension() generally omits the public keys for larger finite-field groups (ffdhe3072, ffdhe4096, ffdhe6144, and ffdhe8192) so that the extension will not be overly large. However, the extension that it creates is still much larger than what is created by other software.
For a generic TLS 1.3 ClientHello, socksend_tls_clienthello() offers 7 groups in the supported_groups extension (P-256, P-384, P-521, X25519, X448, ffdhe2048, ffdhe3072) and 6 public keys in the key_share extension (P-256, P-384, P-521, X25519, X448, ffdhe2048). While the largest public key is omitted, this still creates a 665 byte key_share extension.
By contrast, Firefox offers 6 groups in the supported_groups extension (X25519, P-256, P-384, P-521, ffdhe2028, ffdhe3072), but only includes two public keys in the key_share extension (X25519, P-256). OpenSSL 1.1.1 offers 5 groups in the supported_groups extension (X25519, P-256, P-384, P-521, X448) and only includes one key in the key_share extension (X25519). Chrome offers 3 groups in the supported_groups extension (X25519, P-256, P-384) and only includes one key in the key_share extension (X25519).
Following the examples of OpenSSL, Firefox, and Chrome, this PR changes generate_key_share_extension() to include at most two public keys in the key_share extension. In general it will offer the public keys for the first two groups that appear in the supported_groups extension. However, it will still exclude the public key for any ffdhe group larger than ffdhe2048 unless that group appears first in the supported_groups extension.
In most cases this change will simply result in the ClientHello message being smaller. In some unusual cases, this change will force a second round-trip, with the server sending a HelloRetryRequest in order to ask for the key_share that it needs, but this will not affect the results of the testing.
In run_grease() there is a mismatch between the severity level of finds as printed and as sent to fileout(). Problems are labeled as medium when printing, but as CRITICAL in the call to fileout(). This PR fixes the problem by changing CRITICAL to MEDIUM.
This commit fixes#1123 where a security header containing an asterix lead
to a local filename expansion which was included in the CSV file output.
A new function fileout_csv_finding() addresses this.
Also if "$GIVE_HINTS" isn't true the headline and each line in the CSV file doesn't include
anymore the word hint -- which is more consistent with the JSON output.
As described in #1113, some servers will fail if the length of the ClientHello message is 522, 778, 1034, ... bytes (i.e., if length mod 256 = 10) or 526, 782, 1038, ... bytes (i.e., if length mod 256 = 14). This commit avoid this issue for normal testing by adding a 5-byte padding extension to the message if the length would otherwise be one of these lengths.
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
This PR fixes#615 for the case in which tls_sockets() is used by splitting the list of CBC ciphers into two lists, each with fewer than 128 ciphers and then testing each list separately.
For the --ssl-native case, no changes were needed. Even though $cbc_ciphers contains 154 ciphers, no version of OpenSSL supports all of these ciphers, and so the actual ClientHello sent by every version of OpenSSL contains fewer than 128 ciphers.
I did, however, add the -no_ssl2 flag to the "$OPENSSL s_client" command to prevent OpenSSL from sending an SSLv2-compatible ClientHello. As is noted in a comment in run_server_preference(), "the supplied openssl will send an SSLv2 ClientHello if $SNI is empty and the -no_ssl2 isn't provided."
This commit is a FIX for #1069, thus when running in
wide mode it corrects an additional line feed which
happened sometimes.
As @dcooper16 pointed out it also cleans up the needless
if-statements in run_rc4(), run_lucky13() and run_beast().
It also inserts for wide mode lines a blank so the alignment
is not at the left border anymore (check for leftovers
needed).
This PR fixes problems with check_revocation_crl() sometimes reporting that a certificate is revoked even when it isn't, and with check_revocation_ocsp() sometimes reporting "error querying OCSP responder" even if the OCSP responder provided a good response. The most common reason for this to happen is that OpenSSL cannot validate the server's certificate (even without status checking). PR #1051 attempted to get status checking to work even in cases in which the server's certificate could not be validated. This PR instead addresses the problem by not checking status if determine_trust() was unable to validate the server's certificate.
In some cases the server's certificate can be validated using some, but not all of the bundles of trusted certificates. For example, I have encountered some sites that can be validated using the Microsoft and Apple bundles, but not the Linux or Mozilla bundles.
This PR introduces GOOD_CA_BUNDLE to store a bundle that could be used to successfully validate the server's certificate. If there is no such bundle, then neither check_revocation_crl() nor check_revocation_ocsp() is run. When check_revocation_crl() and check_revocation_ocsp() are called, the status checks within them closely match the validation check in determine_trust(), which helps to ensure that if the check fails it is because of the status information.
As noted in #1057, at least one CA provides incorrect information when the CRL is downloaded, so validation could fail for a reason other than the certificate being revoked. So, this PR adds a check of the reason that validation failed and only reports "revoked" if the validation failed for that reason.
As noted in #1056, it is not possible to perform an OCSP query without access to the certificate issuer's public key. So, with this PR check_revocation_ocsp() is only called if the server's provided at least one intermediate certificate (i.e., the issuer's certificate, which contains the issuer's public key).
This PR improves the handling of error responses when checking status using OCSP. It can handle a few types of errors:
* When the responder just returns an error (e.g., "Responder error: unauthorized").
* When the response cannot be verified (e.g., invalid signature, expired certificate).
* When the response is valid ("Response verify OK"), but there is a problem with the response for the individual certificate (e.g., information is too old, or status is "unknown").
