Commit Graph

3980 Commits

Author SHA1 Message Date
Dirk Wetter d7e9794062
Merge pull request #1120 from dcooper16/avoid_clienthello_size_bug
Avoid ClientHello size bug
2018-09-17 09:59:23 +02:00
Dirk 9c075469c2 Code readability change to c9b725e 2018-09-16 18:18:43 +02:00
Dirk c9b725e6ff Fix filename expansion in CSV output
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.
2018-09-16 18:08:05 +02:00
David Cooper bc3a812de4 Avoid ClientHello size bug
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.
2018-09-14 16:24:05 -04:00
David Cooper 83bd48df0d Fix calculation of ClientHello size
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.
2018-09-14 16:22:19 -04:00
Dirk 15261b2cf4 Merge branch '2.9dev' of github.com:drwetter/testssl.sh into 2.9dev 2018-09-14 16:34:09 +02:00
Dirk 4722033f40 Accept square brackets in supplied IPv6 address 2018-09-14 16:33:09 +02:00
Dirk Wetter 23df63ed74
Merge pull request #1113 from dcooper16/clienthello_length_bug
A ClientHello length intolerance bug
2018-09-12 17:30:46 +02:00
David Cooper 767ee94cb3
Some updates to size bug GREASE test
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.
2018-09-12 11:17:27 -04:00
David Cooper 25fef82977 A ClientHello length intolerance bug
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.
2018-09-11 14:58:04 -04:00
Dirk f2303637b9 Minor polishing 2018-09-10 20:09:39 +02:00
Dirk a24a502716 Make JSON more robust
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
2018-09-10 18:51:43 +02:00
Dirk 1bfc9ca5b8 Avoid warning for OpenSSL 1.1.1 config file
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).
2018-09-10 17:52:56 +02:00
Dirk 44570541c0 Tell which OpenSSL versions support IPv6 out of the box 2018-09-10 09:52:59 +02:00
Dirk 5de89aedc2 Avoid conflicts of OpenSSL 1.1.1 config file
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 #1117

FIX #1098
2018-09-05 16:48:28 +02:00
Dirk a66f5cfdbc curl added for --phone-out checks 2018-09-04 20:20:09 +02:00
Dirk 9a3b6e334b BigIP F5: routed domains
Set routed domains to 3 digits, see also
https://github.com/drwetter/F5-BIGIP-Decoder/issues/1
2018-09-03 13:25:19 +02:00
Dirk 61508b1443 add missing line feed in run_preferences() 2018-09-03 11:10:24 +02:00
Dirk 563dbebffb Added rDNS to file output (CSV+JSON) 2018-09-03 10:45:28 +02:00
Dirk Wetter a3d8143043
Merge pull request #1109 from dcooper16/shellcheck
Fix some shellcheck issues
2018-08-30 23:07:31 +02:00
Dirk Wetter c83612d8ec
Merge pull request #1110 from dcooper16/certlist_ordering_problem
Fix #1080
2018-08-30 22:03:29 +02:00
David Cooper 37e9065d36
Check for certificate_list ordering problems
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.
2018-08-28 15:33:31 -04:00
David Cooper e84470b939
Fix some shellcheck issues
This commit fixes some issues identified by shellcheck.
2018-08-28 09:23:06 -04:00
Dirk 8d7dd663f9 Finalizing proxy support for OCSP checks
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).
2018-08-24 15:43:25 +02:00
Dirk 3fdcd034f3 Fine tuning of --outprefix
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.
2018-08-23 11:40:50 +02:00
Dirk 5da7454e7a Merge branch 'ocsp_crl_final' into 2.9dev
and bump version to 3.0rc1
2018-08-17 12:32:35 +02:00
Dirk ed17797b13 Finalize proxy support for http_get()
... 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.
2018-08-17 12:23:16 +02:00
Dirk 5837e82c85 Supplying of both -6 and --ip=one results in picking an IPv6 address
... 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.
2018-08-16 12:03:56 +02:00
Dirk efa56a34f2 Fix error introduced from previous commit 2018-08-15 02:15:19 +02:00
Dirk 89f7814f81 Fix #1100: scenarios with -6 and --ip=<ipv6address>
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 ]]
2018-08-15 01:50:10 +02:00
Dirk 7cc584027c Save work for later -- proxy not working 2018-07-26 22:49:12 +02:00
Dirk Wetter 94828aed03
Merge pull request #1091 from dcooper16/common_primes_update
Update etc/common-primes.txt
2018-07-26 22:45:36 +02:00
Dirk Wetter 811c66bb80
Merge pull request #1092 from dcooper16/2048_bit_common_primes
Consider 2048-bit DH primes as acceptable
2018-07-26 21:28:43 +02:00
David Cooper ddfc6d5506
Consider 2048-bit DH primes as acceptable
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.
2018-07-23 15:06:53 -04:00
David Cooper 0f9e6b9883
Remove duplicate common primes
Remove three additional common primes that appeared in both https://svn.nmap.org/nmap/scripts/ssl-dh-params.nse and https://github.com/cryptosense/diffie-hellman-groups/blob/master/gen/common.json. Note that run_logjam() will not work properly if the server's prime appears twice in etc/common-primes.txt.
2018-07-23 13:48:18 -04:00
David Cooper 81981b7c27
Update etc/common-primes.txt
The primes in etc/common-primes.txt that were taken from https://github.com/cryptosense/diffie-hellman-groups/blob/master/gen/common.json were encoded in decimal rather than hexadecimal, preventing them from being matched against the primes offered by servers. This PR converts the primes from https://github.com/cryptosense/diffie-hellman-groups/blob/master/gen/common.json to hexadecimal, removing those that were duplicates from https://svn.nmap.org/nmap/scripts/ssl-dh-params.nse.
2018-07-23 13:30:04 -04:00
Dirk Wetter 093108d297
Merge pull request #1090 from dcooper16/rfc7919_primes
Add RFC 7919 primes to etc/common-primes.txt
2018-07-20 15:29:39 +02:00
David Cooper 99c5f42b3f
Add RFC 7919 primes to etc/common-primes.txt
This PR adds the 6 primes from RFC 7919 to etc/common-primes.txt so that they can be detected by run_logjam().
2018-07-20 09:20:44 -04:00
Dirk d83aed83fd server banner message polishing if empty 2018-07-19 14:09:19 +02:00
Dirk ee8c70bce3 Minor polish
Typos, cleanup ec_nistp_64_gcc_128 (for 64 bit at least), add -DOPENSSL_TLS_SECURITY_LEVEL=0
2018-07-18 00:57:32 +02:00
Dirk 5f7f392e83 Merge branch '2.9dev' of github.com:drwetter/testssl.sh into 2.9dev 2018-07-17 01:14:35 +02:00
Dirk b6dca77cd2 Merge branch '2.9dev' of github.com:drwetter/testssl.sh into 2.9dev 2018-07-17 00:44:47 +02:00
Dirk 5d5d21af04 Make script for OpenSSL 1.1.1 tree 2018-07-17 00:41:21 +02:00
Dirk 6e5f7c15af Make Travis CI shut up.
A soon-to-be-expired cert can be also HIGH, thus a test
for critical is appropriate.
2018-07-11 17:14:29 +02:00
Dirk c0921c8877 Merge branch '2.9dev' of github.com:drwetter/testssl.sh into 2.9dev 2018-07-11 11:03:52 +02:00
Dirk 61c5e8b96d (Slightly) improved JSON output for certificates
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.
2018-07-11 10:59:05 +02:00
Dirk Wetter 09182cb196
Merge pull request #1079 from dcooper16/bad_certificate_list
Handle incorrectly populated certificate_list
2018-07-05 09:54:58 +02:00
David Cooper 72ef69aeae
Handle incorrectly populated certificate_list
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.
2018-06-28 16:17:04 -04:00
Dirk Wetter 488cb03c60
Merge pull request #1078 from dcooper16/stapled_ocsp_revocation_check
Check stapled OCSP response for revocation status
2018-06-28 20:25:46 +02:00
David Cooper b5595a9205
Check stapled OCSP response for revocation status
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.
2018-06-28 14:15:55 -04:00