Commit Graph

3810 Commits

Author SHA1 Message Date
Dirk d6f1064b9c Fix STARTTLS IMAP
introduce @ b49399e7c7
2018-10-04 18:27:47 +02:00
Dirk Wetter f607702dad
Merge pull request #1138 from dcooper16/fix_typo
Remove typo
2018-10-04 18:22:34 +02:00
David Cooper d23eb11566
Remove typo
This PR removes a typo that was introduced in b49399e7c7.
2018-10-04 09:00:01 -04:00
Dirk b49399e7c7 Adding LMTP as a STARTTLS protocol
This commit adds LMTP to the STARTTLS protocols
supported. It requires an openssl version which
supports this which is either openssl 1.1.1
or a backported version 1.0.2 (binary is in
process).

A check is in place whetrher the binary supports
this.

Furthermore some framework additions were made for
further STARTTLS protocols like IRC and NNTP.
2018-10-04 11:24:52 +02:00
Dirk Wetter f40f53938b
Merge pull request #1137 from dcooper16/fix_1097
Name check for XMPP servers
2018-10-04 10:32:28 +02:00
David Cooper c4db88526f Handle critical subjectAltName extension
For XMPP servers, when extracting the SRV-ID and XmppAddr names from the subjectAltName extension, need to take into account that the subjectAltName extension may be marked as critical, in which case there will be the DER encoding of TRUE (0101FF) between the DER encoding of the subjectAltName extension's OID (0603551D11) and the tag for OCTET STRING (04).
2018-10-03 08:44:23 -04:00
David Cooper e0f5c7513a Name check for XMPP servers
This PR is an attempt to address #1097. I have only been able to test it against jabber.topf.org and against locally created test certificates, so it needs more testing.

The main issue that this addresses is that testssl.sh currently checks against the wrong name for XMPP servers. According to RFC 6120, Section 13.7.2.1:

   o  The initiating entity sets the source domain of its reference
      identifiers to the 'to' address it communicates in the initial
      stream header; i.e., this is the identity it expects the receiving
      entity to provide in a PKIX certificate.

So, if the --xmpphost option is provided, then the name provided in that option should be compared against the name in the certificate rather than the host name.

compare_server_name_to_cert() currently takes the server name to look for in the certificate as an parameter, but every call to compare_server_name_to_cert() uses $NODE as the argument. So, this PR removes the parameter and sets $servername to either $XMPP_HOST or $NODE as appropriate. This small change alone should fix the problem for most XMPP servers since the server's name SHOULD appear in the certificate encoded as a DNS name. That is the case for the one server I could test - jabber.topf.org.

The majority of the code in this PR is to address the other possibilities noted in RFC 6120, Section 13.7.1.2.1. This section notes that an XMPP server's identity name also appear in the subjectAltName extension as an otherName, either an SRV-ID or an XmppAddr identifier. Unfortunately, OpenSSL's certificate printer does not support otherName and just prints "othername:<unsupported>". So, this PR includes code to manually extract any SRV-ID or XmppAddr names from the certificate. This involves parsing the DER encoding of the certificate to look for the subjectAltName extension, looping through all of the names in the extension, and pulling out the names of these two name forms. This code is only run if the server is an XMPP server and the certificate does not have a matching DNS name. So, this code will rarely be executed.

This PR addresses one other issue. There is code in certificate_info() to set the variables $has_dns_sans and $has_dns_sans_nosni. These variables are needed to address the following requirement:

     # Find out if the subjectAltName extension is present and contains
     # a DNS name, since Section 6.3 of RFC 6125 says:
     #      Security Warning: A client MUST NOT seek a match for a reference
     #      identifier of CN-ID if the presented identifiers include a DNS-ID,
     #      SRV-ID, URI-ID, or any application-specific identifier types
     #      supported by the client.

While it is relatively easy to determine whether a certificate includes a DNS name in the subjectAltName extension, as noted above, it is not easy to determine whether it has an SRV-ID or an XmppAddr. So, this PR leverages the work compare_server_name_to_cert() does in parsing the subjectAltName extension by having compare_server_name_to_cert() set a global variable indicating whether the certificate has a subjectAltName extension with a relevant name form (DNS, SRV-ID, or XmppAddr for XMPP, or DNS for other servers). $has_dns_sans and $has_dns_sans_nosni are then just set to the value of this global variable.
2018-10-03 08:44:23 -04:00
Dirk 1b52834dfc Extend workaround for TCP fragmentation
Instead of checking via uname for Linux this commit does a check
whether the outcome for an external printf is what is expected. This
makes it more compatible e.g. with OpenBSD which surprisingly works
similar like the GNU counterpart.

Also it checks all external printfs installed wther it's the
"right one" to use. Previously it stopped just at the first one
and if this was "wrong", bash's printf was used.
2018-10-02 23:04:02 +02:00
Dirk Wetter b22e0d08fd
Merge pull request #1136 from dcooper16/undo_1113
Revert #1113
2018-09-26 13:52:05 +02:00
Dirk db948cd6b5 Make sure length bytes are two chars wide
Linux bash internal printf shortened the string
when using len2twobytes() with 3 chars, FreeBSD
e.g. did not.

It worked under both OS though when piping to
the socket with printf.

This commit makes sure that always 2+2 chars
are returned when a 3 char number is supplied.

Kudos @dcooper16
2018-09-26 13:41:35 +02:00
Dirk e1ee04fbd7 Put temporary workaounrd in place for Linux + TCP fragmentation
This commit basically reverses the previous commit 305eefc for
Linux only. Here the external printf is working fine, where as
the BSDish counterpart is not (see e.g. #1134).

For Linux is basically addresses #1130 / #1113.

A compatible solution for all OS needs to be found.
2018-09-26 09:21:28 +02:00
David Cooper 83a9302718
Revert #1113
This PR removes the changes that were added in #1113. As noted in the conversation for #1113, it was eventually determined that the actual bug was related to the first message fragment being too short rather than the overall length of the message. So, the warning message that is displayed by the code is misleading. In addition, if changes are made to avoid TCP fragmentation, then this code will no longer even test for intolerance to short message fragments.
2018-09-25 14:16:00 -04:00
Dirk 305eefc9e8 Reverse dd3657e temporarily to get a clear picture
See #1134, #1135, also #1132 is a suspect
2018-09-25 16:18:59 +02:00
Dirk d9f0258171 Fix of errorneous return value due to ALPN check
This commit fixes a problem when not every supplied $ALPN_PROTOs
was suported which is probably never the case ;-/

See #1133
2018-09-25 10:54:17 +02:00
Dirk Wetter 441f14ae90
Merge pull request #1124 from dcooper16/deprecated_curves
Deprecated elliptic curves
2018-09-24 10:30:30 +02:00
Dirk 39822d89e9 Merge branch '2.9dev' of github.com:drwetter/testssl.sh into 2.9dev 2018-09-23 14:16:42 +02:00
Dirk dd3657eb23 Fix TCP fragmentation
As @tomato42 pointed out in #1113 '\x0a' causes the printf buffer
to flush before all data was sent. As a result any '\x0a' in
a ClientHello causes a new TCP fragment.

This commit changes all TCP sockets write to use an external
printf if available which doesn't show this behaviour, see #1130.
It was checked against wireshark.

The external printf was available for Linux, FreeBSD 9 and OpenBSD,
so I do not expect any problems with MacOS X either.

There might be further solutions like 'stdbuf' or 'dd' which
are shown in #1130.
2018-09-23 13:30:46 +02:00
Dirk 11d74d2f6e Remove the cat in get_cipher() and get_protocol()
See #1129
2018-09-22 09:50:15 +02:00
Dirk Wetter 0727a30456
Merge pull request #1129 from dcooper16/fix_1118
Fix #1118
2018-09-22 09:23:15 +02:00
Dirk Wetter 7b9a673b6e
Merge pull request #1131 from dcooper16/avoid_oa
Avoid unnecessary '0a' characters in ClientHello
2018-09-22 09:21:48 +02:00
David Cooper 2b46664a83
Remove '0a' character from public keys
This commit removes the '0a' character from public keys used in the key_share extension. New key pairs were created by repeatedly generating new keys until one was found that had no '0a' characters in the public key.
2018-09-21 17:07:46 -04:00
David Cooper 41c7e74823
Avoid unnecessary '0a' characters in ClientHello
As noted in #1130, the current implementation of socksend_tls_clienthello() results in packets being fragmented wherever a '0a' character appears in the message. This cannot be avoided, but there are a few places where a '0a' character appears in which the character could easily be replaced:

* In the session_id for a TLSv1.3 ClientHello.
* In the 32-byte client random value
* In any public key sent in the key_share extension

This PR removes those uses of the '0a' character. While this does not do much to address the problem, it does result in a slight reduction in the amount of fragmentation of messages.
2018-09-21 17:05:08 -04:00
David Cooper 9b9f435059
Fix #1118
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.
2018-09-21 15:08:29 -04:00
David Cooper 4effe1dbf3 Deprecated elliptic curves
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.
2018-09-21 10:02:45 -04:00
Dirk 96a1002f84 Re-adding IP/FQDN + PORT to CSV output
This commit fixes a recently introduced bug, see #1128
2018-09-20 21:43:39 +02:00
Dirk Wetter ef442f1c1c
Merge pull request #1126 from dcooper16/fewer_key_shares
Send fewer key shares
2018-09-19 09:30:28 +02:00
Dirk Wetter 04dfb66a42
Merge pull request #1125 from dcooper16/fix_grease_findings
Fix run_grease() severity levels
2018-09-19 09:26:33 +02:00
David Cooper 59e2c686c5
Send fewer key shares
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.
2018-09-18 14:27:06 -04:00
David Cooper 1130e30120
Fix run_grease() severity levels
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.
2018-09-18 11:47:32 -04:00
Dirk Wetter 355ba91b65
Merge pull request #1122 from dcooper16/fix_size_calculation
Fix calculation of ClientHello size
2018-09-17 13:24:40 +02:00
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