[BUG] Secure Client-Initiated Renegotiation Testing Yields False Vulnerability

[prabhakaran-motorq]

Secure Client-Initiated Renegotiation Testing Yields False Vulnerability

While testing client-initiated renegotiation, piping the character 'R' into OpenSSL's s_client command can lead to a false vulnerability detection. The issue arises because the client can close the connection before the server has a chance to respond with "closed." As a result, the "closed" status may not be logged when the status code is 0, causing testssl.sh to incorrectly classify the connection as vulnerable.

Details:

We operate our own Kafka servers and ran tests using testssl.sh. During some of these tests, client-initiated renegotiation was incorrectly flagged as vulnerable. Upon further investigation, we discovered that sending the 'R' character through stdin even with a delay was inconsistent and unreliable.

To address this, we devised a more reliable testing method using the expect command. This approach ensures the connection is fully established before sending the renegotiation request ('R').

Commands to Test:

1. Original Flaky Command
   The following command sometimes fails to give reliable results:

   echo 'R' | openssl s_client -tls1_2 -connect ashu-kafka-broker-0.motorq.com:9094

   To observe the flakiness, the command can be run multiple times:

   # Run original flaky command 50 times
   for i in {1..50}; do echo 'R' | openssl s_client -tls1_2 -connect ashu-kafka-broker-0.motorq.com:9094 |& tail -1 & done 2>/dev/null | sort | uniq -c

   

2. Reliable Testing Using expect
   The expect command waits for a "CONNECTED" message before sending the 'R' character, ensuring the connection is stable:

   expect -c 'spawn openssl s_client -tls1_2 -connect ashu-kafka-broker-0.motorq.com:9094; expect "CONNECTED"; send "R\r"; interact'

   Running this command multiple times produces consistent results:

   # Run expect command 50 times
   for i in {1..50}; do expect -c 'spawn openssl s_client -tls1_2 -connect ashu-kafka-broker-0.motorq.com:9094; expect "CONNECTED"; send "R\r"; interact' | tail -1; done | sort | uniq -c

   

---

Issue in testssl.sh

Here is a screenshot of the issue within testssl.sh:

I'm using version 3.2, commit ID: 6452ec9. Similar issues have been reported in these discussions:

• Issue #2038
• Issue #2469
• Issue #2444
• Issue #2389

Expected Behavior:

As testssl.sh is widely used in the industry, it should yield reliable results. In cases where results are inconclusive, the script should indicate this rather than erroneously reporting a vulnerability. The following expect command can be used to test multiple renegotiations within a single session:

expect -c 'spawn openssl s_client -tls1_2 -connect ashu-kafka-broker-0.motorq.com:9094; expect "CONNECTED"; send "R\rR\rR\rR\rQ\r"; interact' | grep 'RENEGOTIATING' -A2

To further validate, a dummy server can be spun up without disabling renegotiation:

openssl req -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -days 365 -nodes
openssl s_server -key key.pem -cert cert.pem -accept 44330 -www -tls1_2
expect -c 'spawn openssl s_client -connect localhost:44330; expect "CONNECTED"; send "R\rR\rR\rR\rQ\r"; interact' | grep 'RENEGOTIATING' -A2

---

System Information:

• OS: Ubuntu 20.04.6 LTS
• Platform: Linux 5.15.0-1053-azure x86_64
• OpenSSL & Bash: OpenSSL 1.0.2-bad | Bash 5.0.17

---

PS:

This is my first time raising an issue here. If there's any additional information that would be helpful, please let me know, and I'd be happy to provide it. Our partner company relies heavily on testssl.sh and is currently blocked due to this issue, so resolving it is critical. I am more than willing to assist in testing, debugging, or providing any further details to expedite a solution. Additionally, feel free to use ashu-kafka-broker-0.motorq.com:9094 for your testing and confirmation. Your support is greatly appreciated.

[drwetter]

Hi,

thanks for reporting!

Unfortunately I can't reproduce it. I tried 50x from North America and 50x from Europe to generate a response VULNERABLE but I wasn't able to.

Independent on that: While suggestions are always appreciated but with expect this is something which won't happen. We'd like to keep dependencies low, like really low.

Actually I don´t know where CONNECTED should come from. Is that a kafka thing?

[Tazmaniac]

The testing logic was completely rewritten to take into account what you called "flaky command". The test no longer use such construct.
see #2470 #2472 #2475 and #2487

[Tazmaniac]

You are in the specific case $SERVICE != HTTP.
Which mean the single command "echo R | $OPENSSL s_client $(s_client_options "$proto $legacycmd $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI")" returned 0 two times, without server connection close (see L17184 to L17200).
The complex retry logic is even not attempted.
So the observed behavior could only be an other deviation from the known single renegotiation s_client invocation (like L17196).
If you could reproduce the unexpected behavior in debug mode, the content of the TMPFILE and ERRFILE should give us insights.

[Tazmaniac]

Other though:
Your command
for i in {1..50}; do echo 'R' | openssl s_client -tls1_2 -connect ashu-kafka-broker-0.motorq.com:9094 |& tail -1 & done 2>/dev/null | sort | uniq -c
is launching openssl s_client multiple times in a raw (and not issuing multiple 'R' in the std input of the running command). As the first command to openssl s_client is buffered, it should not be lost.
On the contrary, issuing multiple commands in a raw without waiting for each intermediate results is known to be flawed, either in pure bash or with expect (send "R\rR\rR\rR\rQ\r").
I hope this is not a openssl s_client regression.
I will prepare a paranoid patch to wait for the establishment of the TLS session before issuing the R command even in the single 'R' case for you to test. It could be the exhibit of a local openssl initialization corner case.

[drwetter]

@Tazmaniac : Thanks a lot for taking care! Love that 👍

I would suggest though that we need to reproduce the problem before patching. If we try to handle all not-really-possible or not-possible cases it's not good for the code base. There's enough legacy stuff already

[Tazmaniac]

@prabhakaran-motorq : you really have a problem on your server/LB side
I put in place a quick/dirty paranoid version of testssl.sh and while testing it targeting your server, I reproduce multiple times a successful renego from openssl point of view, without server side disconnect:

/tmp/testssl.pWO5WK/52.252.235.54.run_renego.errorlog
depth=2 C = US, ST = New Jersey, L = Jersey City, O = The USERTRUST Network, CN = USERTrust RSA Certification Authority
verify return:1
depth=1 C = GB, ST = Greater Manchester, L = Salford, O = Sectigo Limited, CN = Sectigo RSA Domain Validation Secure Server CA
verify return:1
depth=0 CN = *.motorq.com
verify return:1
RENEGOTIATING
DONE

tail -n 10 /tmp/testssl.pWO5WK/52.252.235.54.run_renego.txt
    12c0 - dd 2c bb c6 ac 3d 70 1b-71 5e f1 c2 b5 75 13 27   .,...=p.q^...u.'
    12d0 - 90 ed 2d 2a 8f c2 5b 42-57 d6 8b 6c 38 09 3f ed   ..-*..[BW..l8.?.
    12e0 - f0 1d 53 d1 d9 02 8c fb-cb e8 93 34 26 4a f3 da   ..S........4&J..
    12f0 - a3 72                                             .r

    Start Time: 1730299478
    Timeout   : 7200 (sec)
    Verify return code: 0 (ok)
    Extended master secret: yes
---

Correct behavior:

cat /tmp/testssl.xk8bEJ/52.252.235.54.run_renego.errorlog
depth=2 C = US, ST = New Jersey, L = Jersey City, O = The USERTRUST Network, CN = USERTrust RSA Certification Authority
verify return:1
depth=1 C = GB, ST = Greater Manchester, L = Salford, O = Sectigo Limited, CN = Sectigo RSA Domain Validation Secure Server CA
verify return:1
depth=0 CN = *.motorq.com
verify return:1
RENEGOTIATING

tail -n 10 /tmp/testssl.xk8bEJ/52.252.235.54.run_renego.txt
    12d0 - 1e d5 15 d5 44 f2 e3 56-60 26 de b5 80 43 c5 bb   ....D..V`&...C..
    12e0 - d6 d7 58 55 82 f7 f0 4f-33 dd 2b 0c 79 66 66 69   ..XU...O3.+.yffi
    12f0 - e9                                                .

    Start Time: 1730300037
    Timeout   : 7200 (sec)
    Verify return code: 0 (ok)
    Extended master secret: yes
---
closed

[prabhakaran-motorq]

Hi @Tazmaniac, thanks for taking the time to go over this.

Could you share the paranoid version of testssl.sh to me, please?

I reproduce multiple times a successful renego

Also, during your testing were you able to do multiple renegotiations initiated by the client in a single connection?

Also, are you able to reproduce this renegotiation when you manually run the openssl (vulnerable version) s_client and typing out 'R' via stdin?

[Tazmaniac]

Also, during your testing were you able to do multiple renegotiations initiated by the client in a single connection?
No, because it is only an acceptable mitigation on https for old client certificate authentication scheme specific to http protocol context. For the others protocols, renegociation should be disabled.

Also, are you able to reproduce this renegotiation when you manually run the openssl (vulnerable version) s_client and typing out 'R' via stdin?

No, but it give me insight, we have another not handled openssl s_client corner case:
The equivalent testssl openssl command is : echo R |openssl s_client -no_tls1_3 -connect 52.252.235.54:9094 -servername ashu-kafka-broker-0.motorq.com -no_comp -cipher @SECLEVEL=0:ALL:COMPLEMENTOFALL
With THIS command I could easily reproduce the

    12f0 - 82 51                                             .Q

    Start Time: 1730711040
    Timeout   : 7200 (sec)
    Verify return code: 0 (ok)
    Extended master secret: yes
---
RENEGOTIATING
DONE

pattern.

But with (echo R; sleep 1) |openssl s_client -no_tls1_3 -connect 52.252.235.54:9094 -servername ashu-kafka-broker-0.motorq.com -no_comp -cipher @SECLEVEL=0:ALL:COMPLEMENTOFALL never.

In the general http case, we stay in interactive mode, STDIN being closed at the end of the test or by the server. So we could not hit this race/bug. Adding SSL_RENEG_WAIT (0.25s by default) seems to be sufficient to close the race window in your case, but as a quick safe fix I will put a 1s delay if @drwetter is ok.

The proper fix is to do a big cleanup, merging the second test with the http one (with ssl_reneg_attempts=2) as the big http loop is properly taking care of all possible case, waiting for the server result without closing prematurely STDIN.
As a bonus, we will get complete tracking of the session state ("paranoid" behavior) issuing the renegotiation only when the SSL session is completely "stable", without additional code and faster http test (one less openssl renego connection).

But for this fix, I will need time to do it carefully with proper testing.

[Tazmaniac]

@prabhakaran-motorq you can test https://github.com/Tazmaniac/testssl.sh/tree/client-renego-refactoring which contain the proper fix / cleanup and fix a bad interaction between --openssl-timeout and the renego loop.

[prabhakaran-motorq]

@Tazmaniac thanks for the working on this.

When I initially found this issue, I tried sleep 1, but even in that case, I still got "DONE" very rarely. Especially in my MacBook M1 Pro (15.0.1 (24A348)) but that worked fine in my Linux machine, that is why I resorted to expect.

I tried this fix (https://github.com/Tazmaniac/testssl.sh/tree/client-renego-refactoring) in my MacBook

in my Linux VM

Our customer partners generally test using testssl.sh on their local system before sending it to their security team. So even with this fix, sometimes it may classify it as vulnerable.

[Tazmaniac]

You did not use the correct branch.
Switch to the client-renego-refactoring before use.

You shoud see

  testssl.sh version 3.2rc3 from https://testssl.sh/dev/
  (7625422 2024-11-04 21:02:03)

as the version (latest commit).

[prabhakaran-motorq]

Switch to the client-renego-refactoring before use.

Thanks for pointing it out, client-renego-refactoring branch works. Thanks for the quick fix.

[Tazmaniac]

Please do a git pull to test the latest version (git tag 7625422 2024-11-04 21:02:03).

Today I will do more testing and submit a PR if all is OK.

[drwetter]

@Tazmaniac : thanks for the PR!

Your suggestion to do a git pull and to test was for @prabhakaran-motorq or for me?

[Tazmaniac]

It was for @prabhakaran-motorq.
I will do a new PR today for my client-renego-refactoring branch.
Mass testing was done on it and the result are very good.

[prabhakaran-motorq]

@drwetter @Tazmaniac what the timeline that we are looking to merge this?

Thanks for all the support on this. Appreciate this.

[drwetter]

@prabhakaran-motorq Which of the two PRs do you mean?

[prabhakaran-motorq]

@prabhakaran-motorq Which of the two PRs do you mean?

@drwetter #2598

[drwetter]

next week

[drwetter]

I merged the quick fix first from @Tazmaniac (thanks!), hoping it'll hel

For #2598 I need more time