Merge 4886b85e10 into 5581499da5

Merge pull request #2487 from Tazmaniac/renego-fix3
[Client Renego] Small important fixes
2024-04-02 21:34:29 +02:00 · 2024-04-02 11:15:12 +02:00 · 2024-03-26 12:23:38 +01:00 · 2023-10-31 10:59:12 +01:00 · 2023-10-31 10:48:02 +01:00
2 changed files with 42 additions and 14 deletions
--- a/.github/workflows/shellcheck.yml
+++ b/.github/workflows/shellcheck.yml
@ -0,0 +1,20 @@
+---
+name: Shellcheck
+on: [push, pull_request]
+permissions:
+  contents: read
+
+jobs:
+  shellcheck:
+    name: Shellcheck
+    runs-on: ubuntu-latest
+    env:
+      LANG: C.UTF-8
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install shellcheck
+        run: sudo apt update && sudo apt install -y shellcheck && shellcheck --version
+
+      - name: Check shell scripts
+        run: shellcheck -x -P "SCRIPTDIR"/utils --severity=error testssl.sh
--- a/testssl.sh
+++ b/testssl.sh
@ -17060,6 +17060,8 @@ run_renego() {
          # We will need $ERRFILE for mitigation detection
          if [[ $ERRFILE =~ dev.null ]]; then
               ERRFILE=$TEMPDIR/errorfile.txt || exit $ERR_FCREATE
+               # cleanup previous run if any (multiple IP)
+               rm -f $ERRFILE
               restore_errfile=1
          else
               restore_errfile=0
@ -17105,14 +17107,16 @@ run_renego() {
                              (j=0; while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]] && [[ $j -lt 30 ]]; do sleep $ssl_reneg_wait; ((j++)); done; \
                                   for ((i=0; i < ssl_reneg_attempts; i++ )); do sleep $ssl_reneg_wait; echo R; k=0; \
                                       while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $((i+3)) ]] && [[ -f $TEMPDIR/allowed_to_loop ]] \
-					       && [[ $(tail -n1 $ERRFILE |grep -acE '^(RENEGOTIATING|depth|verify)') -eq 1 ]] && [[ $k -lt 120 ]]; \
-                                       do sleep $ssl_reneg_wait; ((k++)); done; \
+                                             && [[ $(tail -n1 $ERRFILE |grep -acE '^(RENEGOTIATING|depth|verify|notAfter)') -eq 1 ]] \
+                                             && [[ $k -lt 120 ]]; \
+                                           do sleep $ssl_reneg_wait; ((k++)); if (tail -5 $TMPFILE| grep -qa '^closed'); then sleep 1; break; fi; done; \
                                   done) | \
                                   $OPENSSL s_client $(s_client_options "$proto $legacycmd $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>>$ERRFILE &
                              pid=$!
                              ( sleep $((ssl_reneg_attempts*3)) && kill $pid && touch $TEMPDIR/was_killed ) >&2 2>/dev/null &
                              watcher=$!
-                              # Trick to get the return value of the openssl command, output redirection and a timeout. Yes, some target hang/block after some tries.  
+                              # Trick to get the return value of the openssl command, output redirection and a timeout.
+                              # Yes, some target hang/block after some tries.
                              wait $pid
                              tmp_result=$?
                              pkill -HUP -P $watcher
@ -17120,6 +17124,10 @@ run_renego() {
                              rm -f $TEMPDIR/allowed_to_loop
                              # If we are here, we have done two successful renegotiation (-2) and do the loop
                              loop_reneg=$(($(grep -ac '^RENEGOTIATING' $ERRFILE)-2))
+                              # As above, some servers close the connection and return value is zero
+                              if (tail -5 $TMPFILE| grep -qa '^closed'); then
+                                   tmp_result=1
+                              fi
                              if [[ -f $TEMPDIR/was_killed ]]; then
                                   tmp_result=2
                                   rm -f $TEMPDIR/was_killed
@ -17395,19 +17403,19 @@ run_breach() {

               # Final verdict (if not happened preemptively before). We reuse $detected_compression here
               detected_compression=""
-               if [[ ${has_compression[@]} =~ warn ]]; then
+               if [[ ${has_compression[*]} =~ warn ]]; then
                    # warn_empty / warn_stalled
-                    if [[ ${has_compression[@]} =~ warn_empty ]]; then
-                         pr_warning "At least 1/4 checks failed (HTTP header request was empty, debug: ${has_compression[@]}"
-                         out ", debug: ${has_compression[@]})"
-                         fileout "$jsonID" "WARN" "Test failed as HTTP response was empty, debug: ${has_compression[@]}" "$cve" "$cwe"
+                    if [[ ${has_compression[*]} =~ warn_empty ]]; then
+                         pr_warning "At least 1/4 checks failed (HTTP header request was empty, debug: ${has_compression[*]}"
+                         out ", debug: ${has_compression[*]})"
+                         fileout "$jsonID" "WARN" "Test failed as HTTP response was empty, debug: ${has_compression[*]}" "$cve" "$cwe"
                    else # warn_stalled
                         pr_warning "At least 1/4 checks failed (HTTP header request stalled and was terminated"
-                         out ", debug: ${has_compression[@]})"
+                         out ", debug: ${has_compression[*]})"
                         fileout "$jsonID" "WARN" "Test failed as HTTP request stalled and was terminated" "$cve" "$cwe"
                    fi
               else
-                    for c in ${has_compression[@]}; do
+                    for c in "${has_compression[@]}"; do
                         if [[ $c =~ yes ]]; then
                              detected_compression+="${c%:*} "
                         fi
@ -17418,7 +17426,7 @@ run_breach() {
                    outln "${spaces}${when_makesense}"
                    fileout "$jsonID" "MEDIUM" "potentially VULNERABLE, $detected_compression HTTP compression detected $disclaimer" "$cve" "$cwe" "$hint"
               fi
-               debugme outln "${spaces}has_compression: ${has_compression[@]}"
+               debugme outln "${spaces}has_compression: ${has_compression[*]}"
               ;;
     esac

@ -18813,7 +18821,7 @@ run_winshock() {
          # Check whether there are any TLS extension which should not be available under <= Windows 2012 R2
          for tls_ext in $TLS_EXTENSIONS; do
               # We use the whole array, got to be careful when the array becomes bigger (unintended match)
-               if [[ ${forbidden_tls_ext[@]} =~ $tls_ext ]]; then
+               if [[ ${forbidden_tls_ext[*]} =~ $tls_ext ]]; then
                    pr_svrty_best "not vulnerable (OK)"; outln " - TLS extension $tls_ext detected"
                    fileout "$jsonID" "OK" "not vulnerable  - TLS extension $tls_ext detected" "$cve" "$cwe"
                    return 0
@ -23229,7 +23237,7 @@ debug_globals() {
 set_skip_tests() {
     local t

-     for t in ${SKIP_TESTS[@]} ; do
+     for t in "${SKIP_TESTS[@]}" ; do
          t="do_${t}"
          # declare won't do it here --> local scope
          eval "$t"=false
@ -23413,7 +23421,7 @@ parse_cmd_line() {
                    # then we need to make sure we catch --ids-friendly. Normally we do not,
                    # see #1717.  The following statement makes sure. In the do-while + case-esac
                    # loop it will be execute again, but it does not hurt
-                    if [[ "${CMDLINE_ARRAY[@]}" =~ --ids-friendly ]]; then
+                    if [[ "${CMDLINE_ARRAY[*]}" =~ --ids-friendly ]]; then
                         OFFENSIVE=false
                    fi
                    do_vulnerabilities=true
Author	SHA1	Message	Date
Meisam	e832a62e06	Merge `4886b85e10` into `5581499da5`	2024-04-02 21:34:29 +02:00
Dirk Wetter	5581499da5	Merge pull request #2487 from Tazmaniac/renego-fix3 [Client Renego] Small important fixes	2024-04-02 11:15:12 +02:00
Emmanuel Fusté	6d80ec189f	Small important fixes: - In case of multiple IP testing, clear ERRFILE bettween runs - Zero return value with server connection close should be taken into account in the looping logic case too. Add it. - Break the wait loop in case of connection close for faster result. - Ignore "notAfter" in the wait loop for expired certificates. - Indentation and tab cleanup. Tested on 57 url with previously suspicious results with success. No regressions on 50 others.	2024-03-26 12:23:38 +01:00
meisam	4886b85e10	fix SC2068, SC2145, SC2199	2023-10-31 10:59:12 +01:00
meisam	502c08ce54	add shellcheck to ci	2023-10-31 10:48:02 +01:00