Add -msg and -servername support to openssl s_client

[geedo0]

Description of changes:

Add -msg flag with full implementation using SSL_set_msg_callback to
print TLS protocol messages in OpenSSL-compatible format.

Add -servername flag that maps to DoClient's existing -server-name
for SNI support.

Call-outs:

I did not end up printing the hex dumps of each message like OpenSSL does. Should be easy enough to add if we wanted to, but the compatibility gap being addressed by this does not require it.

The -msg argument only applies to s_client but it had to be implemented inside client.cc because it impacts the SSL_CTX. This may raise the eyebrows of some purists.

Testing:

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

# Example -msg output
➜  tool-openssl git:(msg-fix) ✗ ./openssl s_client -connect amazon.com:443 -msg -servername amazon.com | grep -E '>>>|<<<'
>>> TLS 1.2, RecordHeader [length 0005]
>>> TLS 1.2, Handshake [length 05c8], ClientHello
<<< TLS 1.2, RecordHeader [length 0005]
<<< TLS 1.2, Handshake [length 007a], ServerHello
>>> TLS 1.2, RecordHeader [length 0005]
>>> TLS 1.3, ChangeCipherSpec [length 0001]
<<< TLS 1.2, RecordHeader [length 0005]
<<< TLS 1.2, RecordHeader [length 0005]
<<< TLS 1.3, Handshake [length 000a], EncryptedExtensions
<<< TLS 1.2, RecordHeader [length 0005]
<<< TLS 1.3, Handshake [length 144c], Certificate
<<< TLS 1.2, RecordHeader [length 0005]
<<< TLS 1.3, Handshake [length 0108], CertificateVerify
<<< TLS 1.2, RecordHeader [length 0005]
<<< TLS 1.3, Handshake [length 0024], Finished
>>> TLS 1.3, Handshake [length 0024], Finished
>>> TLS 1.2, RecordHeader [length 0005]
<<< TLS 1.2, RecordHeader [length 0005]
<<< TLS 1.3, Handshake [length 0039], NewSessionTicket
<<< TLS 1.2, RecordHeader [length 0005]
<<< TLS 1.3, Handshake [length 0039], NewSessionTicket
<<< TLS 1.2, RecordHeader [length 0005]
<<< TLS 1.3, Alert [length 0002]

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 1.63934% with 60 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.33%. Comparing base (4738958) to head (4bbff4c).

Files with missing lines	Patch %	Lines
tool/client.cc	0.00%	58 Missing ⚠️
tool-openssl/s_client.cc	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3098      +/-   ##
==========================================
+ Coverage   78.19%   78.33%   +0.14%     
==========================================
  Files         689      689              
  Lines      122037   122099      +62     
  Branches    17026    17036      +10     
==========================================
+ Hits        95430    95652     +222     
+ Misses      25701    25540     -161     
- Partials      906      907       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[justsmth]

Could we add a few tests to cover the new options?

[justsmth]

• What about ServerKeyExchange, ServerHelloDone, ClientKeyExchange, others?
• Should the default be something like "Unknown" or "Other" instead of nullptr?

[geedo0]

nullptr ends up printing nothing after the length which I think is fine. Didn't feel like being exhaustive here unless we feel strongly about it. I implemented enough to avoid any unhandled cases for a typical endpoint.

>>> TLS 1.2, Handshake [length 05c8]

[geedo0]

Could we add a few tests to cover the new options?

I wanted to, but I couldn't find any patterns in the test framework for capturing stdout and asserting on that. Is there something I'm missing? Or should I cobble something together? I suppose we could assert that passing these args in does not give an error return code, but is that really helpful?

[WillChilds-Klein]

Could we add a few tests to cover the new options?

I wanted to, but I couldn't find any patterns in the test framework for capturing stdout and asserting on that.

you're printing to stderr, no?

I suppose we could assert that passing these args in does not give an error return code, but is that really helpful?

i don't think the juice is worth the squeeze here if we're just asserting exit codes.

[WillChilds-Klein]

np: others return "Unknown" here, should this do same?

[geedo0]

See thread with @justsmth below

[justsmth]

I wanted to, but I couldn't find any patterns in the test framework for capturing stdout and asserting on that. Is there something I'm missing? ...

We have a bunch of "comparison tests" that capture our command output and compare it to the output of openssl. See the usage of RunCommandsAndCompareOutput. Could that be used?