When run_rc4() is run with the "--show-each" option, but without the "--wide" option, a list of all RC4 ciphers is printed, without any distinction between those that are supported by the server and those that are not. This is the same issue I noted in #332 for run_pfs().
In run_pfs(), the displayed output was corrected, but all ciphers were still being added to $pfs_ciphers, so the list of supported PFS ciphers sent to fileout() was incorrect.
This PR fixes both issues.
When certificate_info() is trying to determine what type of public key the server has so that it can determine whether the key size is acceptable, it sometimes looks at $cert_sig_algo rather than $cert_key_algo. This PR fixes that and also adds support for DSA public keys.
The dec2hex() was actually converting from hex to decimal. Since it was only being used in one place, and wasn't really needed there, I just deleted it.
Revised parse_tls_serverhello() to more carefully check the response for errors, and to provide for more flexibility (e.g., if handshake messages are split across multiple fragments).
The new test in PR #346 sends a TLSv1.4 ClientHello, so socksend_tls_clienthello() needs to include the signature algorithms extension if $tls_low_byte >= 3 rather than only if it is equal to 3.
Changed to only include the signature algorithms extension for TLSv1.2, since RFC 5246 says:
Note: this extension is not meaningful for TLS versions prior to 1.2.
Clients MUST NOT offer it if they are offering prior versions.
However, even if clients do offer it, the rules specified in [TLSEXT]
require servers to ignore extensions they do not understand.
Inclusion of the extension for TLS 1.1 didn't seem to cause any harm, but it seems better to follow the RFC and not include it for TLSv1.0 or TLSv1.1.
RFC 7685 notes that there is at least one TLS implementation that hangs if the client sends a ClientHello with a TLSCiphertext.length between 256 and 511 bytes, and so the padding extension was defined in order to get around this bug. (OpenSSL s_client includes this extension when the -bugs option is used.) So, I changed socksend_tls_clienthello() to include the padding extension if the CLientHello would have a length between 256 and 511 bytes, making the padding extension just large enough to make the ClientHello 512 bytes.
I also fixed a typo (a missing "0x") in the check for whether any ECC ciphers are included in the Client Hello.
In doing some work on cipher_pref_check() I noticed that it was failing on SSLv2 since the call to "$OPENSSL s_client" includes SNI. I've also noticed in my testing that "$OPENSSL s_client" will not connect to an SSLv2-only server unless the "-ssl2" flag is included. So, I carefully checked each call to "$OPENSSL s_client" in the program (other than in run_allciphers and run_cipher_per_proto, since those functions are already addresses in PR #341) to see whether they would inappropriate fail with an SSLv2-only (or SSLv3-only) server.
As a general rule, if the call doesn't currently include the protocol, then I added "-ssl2" if $OPTIMAL_PROTO is "-ssl2", indicating that the server only supports SSLv2, and I removed any $SNI if a protocol is specified if a protocol is specified and it is either SSLv2 or SSLv3.
I tested it on an SSLv2-only server, and the results are much better. I also tested it on a collection of other servers, none of which support SSLv2, and the results are the same as with the current code.
The only thing I haven't been able to test is how the revised code works when the "--starttls" option is used. I don't believe the changes I made would cause anything to break in that case, but I also don't think code will work any better in that case, if the server only supports SSLv2. Of course, since no server should support SSLv2 (let alone only SSLv2), it shouldn't really be an issue.
One thing that I did not change, but that I do not understand; why does determine_optimal_proto() try the protocols in the order "-tls1_2 -tls1 -ssl3 -tls1_1 -ssl2" rather than "-tls1_2 -tls1_1 -tls1 -ssl3 -ssl2"? Doesn't the current ordering imply that TLS v1.0 and SSLv3 are better than TLS v1.1?
This PR adds the signature algorithms, heartbeat, session ticket, and next protocol extensions to the client hello message created by socksend_tls_clienthello() for TLS 1.0 and above. It also adds the supported elliptic curves and ec points format extensions if the client hello message includes any ECC cipher suites.
I tested this version against several servers with $EXPERIMENTAL set to true and get the same results as with the current code with $EXPERIMENTAL set to false.
The last line of neat_list currently uses $HEXC as the parameter to show_rfc_style(), but it should use $hexcode. At the moment using $HEXC instead of $hexcode makes no difference, since hexcode="$1" and in all calls to neat_list() the first parameter is $HEXC. However, this bug could create problems in the future since neat_list() will misbehave if the value of the first parameter (hexcode) isn't the same as $HEXC.
This PR makes basically the same changes to run_cipher_per_proto() as I previously made to run_allciphers(). The main difference is that in this function, round 0 consists of a single call to "$OPENSSL s_client" with "-cipher" including all of the locally supported ciphers. The reason for the difference is that in run_allciphers() its saves time to assume the server supports at least one cipher suite. In the case of run_cipher_per_proto(), however, it is likely that the server will not support some protocols at all, so its usually faster to start with a single call to "$OPENSSL s_client" that tests whether the server supports the protocol at all.
There is a tab on the line for SSL_CK_RC2_128_CBC_WITH_MD5. When testssl.sh is called with "-E" and "--show-each," this causes the string "not a/v" to be printed two characters to the right of the same string on every other line (at least on Linux systems). This PR just deletes the tab character.
The run_allciphers() function currently works by calling "$OPENSSL s_client" once for each cipher suite supported by $OPENSSL. In the case of "OpenSSL 1.0.2-chacha (1.0.2e-dev)" that means 195 calls to "$OPENSSL s_client" even though servers tend to only support a small fraction of these cipher suites.
This PR produces the same output as the current run_allciphers() with fewer calls to "$OPENSSL s_client", which results in the function running faster (usually much faster). The basic idea behind the revised function is to test cipher suites in blocks. If $OPENSSL supports 195 cipher suites, then it group these cipher suites into 4 blocks of 64 (with the final block being smaller). It makes one call to "$OPENSSL s_client" with cipher suites 1-64, and if it fails, then it knows that none of these 64 cipher suites are supported by the server and it doesn't need to perform any more tests on these 64 cipher suites. If it succeeds, then it breaks the 64 cipher suites into 4 blocks of 16 and calls "$OPENSSL s_client" with each of those blocks. The blocks of 16 that are successful are broken into blocks of 4, and for each of the successful blocks of 4 the individual cipher suites are tested.
For testssl.sh and www.google.com the number of calls to "$OPENSSL s_client" is reduced from 195 to 88. For github.com the number of calls is reduced to 56!
I haven't made any changes to run_cipher_per_proto yet, but if this PR is accepted I can make the same changes in that function.
Thanks,
David
This corrects the indentation within determine_trust() when there are multiple certificates and the output for "Chain of trust (experim.)" takes up more than one lines.
In addition, it fixes the ID field of the JSON output for entries related to the certificate. At the moment, each ID string begins with a blank space. This changes it to remove the space if there is one certificate and to add "Server Certificate #X" at the beginning of each ID if there is more than one certificate.
Perhaps there's a better way than just using, for example, "Server Certificate #1 key_size" as a way to distinguish multiple "key_size" entries in the JSON file. This is just one idea, and it can certainly be changed if those who intend to use the JSON output prefer something else.
The number of .pem files in $INSTALL_DIR/etc is currently hard-coded into determine_trust. This modifies the code so that the number of files can be changed without having to change the code.
This should fix issue #278. I'm not sure whether openssl verify will ever print out more than one error, so to be safe, I wrote the code to handle the possibility that it might; if there is more than one error, it just takes the first and ignores the rest.
David Cooper