Merge pull request #2976 from testssl/mitigate_2083_3.2

Mitigate inconsistent test results for ROBOT (3.2)

Author: drwetter

doc/testssl.1

@@ -227,7 +227,7 @@ Security headers (X\-Frame\-Options, X\-XSS\-Protection, Expect\-CT,\|\.\|\.\|\.
 .P
 \fB\-T, \-\-ticketbleed\fR Checks for Ticketbleed memory leakage in BigIP loadbalancers\.
 .P
-\fB\-\-BB, \-\-robot\fR Checks for vulnerability to ROBOT / (\fIReturn Of Bleichenbacher's Oracle Threat\fR) attack\.
+\fB\-\-BB, \-\-robot\fR Checks for vulnerability to ROBOT / (\fIReturn Of Bleichenbacher's Oracle Threat\fR) attack\. The predefined timeout of 10 seconds can be changed with the environment variable \fBROBOT_TIMEOUT\fR\.
 .P
 \fB\-\-SI, \-\-starttls\-injection\fR Checks for STARTTLS injection vulnerabilities (SMTP, IMAP, POP3 only)\. \fBsocat\fR and OpenSSL >=1\.1\.0 is needed\.
 .P
@@ -393,6 +393,8 @@ CCS_MAX_WAITSOCK Is the similar to above but applies only to the CCS handshakes,
 .IP "\[ci]" 4
 HEARTBLEED_MAX_WAITSOCK Is the similar to MAX_WAITSOCK but applies only to the ServerHello after sending the Heartbleed payload\. Don't change this unless you're absolutely sure what you're doing\. Value is in seconds\.
 .IP "\[ci]" 4
+ROBOT_TIMEOUT is similar to above and applies to the ROBOT check\.
+.IP "\[ci]" 4
 MEASURE_TIME_FILE For seldom cases when you don't want the scan time to be included in the output you can set this to false\.
 .IP "\[ci]" 4
 STARTTLS_SLEEP is per default set to 10 (seconds)\. That's the value testssl\.sh waits for a string in the STARTTLS handshake before giving up\.

doc/testssl.1.html

@@ -236,7 +236,7 @@ Also for multiple server certificates are being checked for as well as for the c
 
 `-T, --ticketbleed`             Checks for Ticketbleed memory leakage in BigIP loadbalancers.
 
-`--BB, --robot`          Checks for vulnerability to ROBOT / (*Return Of Bleichenbacher's Oracle Threat*) attack.
+`--BB, --robot`          	Checks for vulnerability to ROBOT / (*Return Of Bleichenbacher's Oracle Threat*) attack. The predefined timeout of 10 seconds can be changed with the environment variable `ROBOT_TIMEOUT`.
 
 `--SI, --starttls-injection`          Checks for STARTTLS injection vulnerabilities (SMTP, IMAP, POP3 only). `socat` and OpenSSL >=1.1.0 is needed.
 
@@ -386,6 +386,7 @@ Except the environment variables mentioned above which can replace command line
 * MAX_WAITSOCK: It instructs testssl.sh to wait until the specified time before declaring a socket connection dead. Don't change this unless you're absolutely sure what you're doing. Value is in seconds.
 * CCS_MAX_WAITSOCK Is the similar to above but applies only to the CCS handshakes, for both of the two the two CCS payload. Don't change this unless you're absolutely sure what you're doing. Value is in seconds.
 * HEARTBLEED_MAX_WAITSOCK  Is the similar to MAX_WAITSOCK but applies only to the ServerHello after sending the Heartbleed payload. Don't change this unless you're absolutely sure what you're doing. Value is in seconds.
+* ROBOT_TIMEOUT  is similar to above and applies to the ROBOT check.
 * MEASURE_TIME_FILE For seldom cases when you don't want the scan time to be included in the output you can set this to false.
 * STARTTLS_SLEEP is per default set to 10 (seconds). That's the value testssl.sh waits for a string in the STARTTLS handshake before giving up.
 * MAX_PARALLEL is the maximum number of tests to run in parallel in parallel mass testing mode. The default value of 20 may be made larger on systems with faster processors.

doc/testssl.1.md

@@ -207,6 +207,7 @@ MAX_HEADER_FAIL=${MAX_HEADER_FAIL:-2}   # If this many failures for HTTP GET are
 MAX_WAITSOCK=${MAX_WAITSOCK:-10}        # waiting at max 10 seconds for socket reply. There shouldn't be any reason to change this.
 CCS_MAX_WAITSOCK=${CCS_MAX_WAITSOCK:-5} # for the two CCS payload (each). There shouldn't be any reason to change this.
 HEARTBLEED_MAX_WAITSOCK=${HEARTBLEED_MAX_WAITSOCK:-8}      # for the heartbleed payload. There shouldn't be any reason to change this.
+ROBOT_TIMEOUT=${ROBOT_TIMEOUT:10}       # Initial timeout for ROBOT check
 STARTTLS_SLEEP=${STARTTLS_SLEEP:-10}    # max time wait on a socket for STARTTLS. MySQL has a fixed value of 1 which can't be overwritten (#914)
 FAST_STARTTLS=${FAST_STARTTLS:-true}    # at the cost of reliability decrease the handshakes for STARTTLS
 USLEEP_SND=${USLEEP_SND:-0.1}           # sleep time for general socket send
@@ -20400,7 +20401,7 @@ run_robot() {
      local -i i subret len iteration testnum pubkeybytes
      local pubkeybits
      local vulnerable=false send_ccs_finished=true
-     local -i start_time end_time robottimeout=$MAX_WAITSOCK
+     local -i start_time end_time robottimeout=$ROBOT_TIMEOUT
      local cve="CVE-2017-17382 CVE-2017-17427 CVE-2017-17428 CVE-2017-13098 CVE-2017-1000385 CVE-2017-13099 CVE-2016-6883 CVE-2012-5081 CVE-2017-6168"
      local cwe="CWE-203"
      local jsonID="ROBOT"
@@ -20571,6 +20572,11 @@ run_robot() {
                     end_time=$(LC_ALL=C date "+%s")
                     resp=$(hexdump -v -e '16/1 "%02x"' "$SOCK_REPLY_FILE")
                     response[testnum]="${resp%%[!0-9A-F]*}"
+                    # TLS alert length seems to vary sometimes within this loop which leads to
+                    # wrong test results, see #2083. Thus we cut this here to length 14, if
+                    # it's a TLS alert with the length of 2
+                    [[ ${response[testnum]::2} == 15 ]] && [[ ${response[testnum]:10:2} == 02 ]] &&
+                          response[testnum]=${response[testnum]::14}
                     # The first time a response is received to a client key
                     # exchange message, measure the amount of time it took to
                     # receive a response and set the timeout value for future