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

openssl: do not log excess "TLS app data" lines for TLS 1.3 #3281

Closed
wants to merge 1 commit into from

Conversation

Lekensteyn
Copy link
Contributor

The SSL_CTX_set_msg_callback callback is not just called for the
Handshake or Alert protocols, but also for the raw record header
(SSL3_RT_HEADER) and the decrypted inner record type
(SSL3_RT_INNER_CONTENT_TYPE). Be sure to ignore the latter to avoid
excess debug spam when using curl -v against a TLSv1.3-enabled server:

* TLSv1.3 (IN), TLS app data, [no content] (0):

(Following this message, another callback for the decrypted
handshake/alert messages will be be present anyway.)


Note that SSL3_RT_HEADER is only defined since 2012 via openssl/openssl@36b5bb6, otherwise the ssl_ver check could have been replaced by content_type != SSL3_RT_HEADER.

In #2403 (comment), @jay already questioned why this message needs to be logged. For symmetry with ignoring raw record headers, I think dropping the message is a good idea.

The SSL_CTX_set_msg_callback callback is not just called for the
Handshake or Alert protocols, but also for the raw record header
(SSL3_RT_HEADER) and the decrypted inner record type
(SSL3_RT_INNER_CONTENT_TYPE). Be sure to ignore the latter to avoid
excess debug spam when using `curl -v` against a TLSv1.3-enabled server:

    * TLSv1.3 (IN), TLS app data, [no content] (0):

(Following this message, another callback for the decrypted
handshake/alert messages will be be present anyway.)
@jay
Copy link
Member

jay commented Nov 16, 2018

I think dropping the message is a good idea.

I agree, 'No content' messages did not prove to be useful. We are still showing the follow up messages so it should be fine. Thanks

@jay jay closed this in 27e4ac2 Nov 16, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Feb 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants