Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tazmaniac client renego refactoring #2632

Merged
merged 13 commits into from
Jan 24, 2025
Merged

Conversation

drwetter
Copy link
Collaborator

Describe your changes

merge and massage #2598, see there

emmanuelfuste and others added 12 commits November 4, 2024 17:27
All cases could be handled by the single openssl s_client invocation
loop:
- dispatch and adjust comments to not loose them
- remove the first s_client invocation: stuck connections are allready
  handled by the main loop
- remove the second s_client invocation: normal case and server closed
  connections are allready handled by the main loop. The loop take care
  of the race between server connection close and s_client terminating
  too by doing another loop run, not closing STDIN.
- special non HTTP case equivalent to ssl_reneg_attempts=2
- specialcase only the HTTP result printing to not change the output

- openssl-timeout option clashe badly with the main loop logic:
  Introduce $OPENSSL_NOTIMEOUT
In the wait loop, I was relying on a 1s sleep to eliminate a possible
late zero return value server close on the last attempt.
- do globaly one more harmless "for" iteration
  and remove the sleep 1 for faster and more robust result
- correct the non HTTP case iteration value
- adjust the timeout to the conservative 6s in the non HTTP case,
  for HTTP case it become 33s
- improve comments
- Recover the "not vulnerable" case (no mitigation) printing, cosmetic
  fix.
- With the removing of all s_client invocation other than the main loop
  one, fix the init of the ERRFILE and TMPFILE: no need to append, no
  need to remove, inconditionally zap the content before the loop.
testssl.sh worked as expected.
Under the hood, broken pipes are expected as part of the fast loop exit
strategy that relies as little as possible on timeout detection.
But under the CI, testssl.sh output is garbled by the subshells stderr
outputs, catched for some reason by 'prove -v'.
Simply redirecting the stderr output of the offending command to
/dev/null fixes the problem.
@drwetter drwetter mentioned this pull request Jan 24, 2025
12 tasks
@Tazmaniac
Copy link

Perfect ! Thank you !

@drwetter drwetter merged commit 5eeab64 into 3.2 Jan 24, 2025
3 checks passed
@drwetter drwetter deleted the Tazmaniac-client-renego-refactoring branch January 24, 2025 13:24
@drwetter
Copy link
Collaborator Author

No problem. And au contraire: Thank YOU! ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants