Wrap EOF/connection reset errors with TLS context after handshake#2031
Wrap EOF/connection reset errors with TLS context after handshake#2031
Conversation
When a TLS proxy (e.g. nginx with mTLS) completes the TLS handshake but then closes the connection due to an untrusted client certificate, the client would receive a bare "EOF" or "connection reset by peer" with no indication that TLS was involved. This wraps such errors with context: "nats: connection closed by remote after TLS handshake: EOF" The wrapped error preserves the original via %w for backwards compatibility (errors.Is(err, io.EOF) still returns true). Signed-off-by: Waldemar Quevedo <wally@nats.io>
wallyqs
force-pushed
the
wq/lb-tls-eof
branch
from
c2b39c7 to
71ea3db
Compare
philpennock
left a comment
philpennock
left a comment
There was a problem hiding this comment.
LGTM from a crypto/security perspective.
| } | ||
| // Should contain a TLS-related error message. | ||
| errStr := err.Error() | ||
| if !strings.Contains(errStr, "tls:") && !strings.Contains(errStr, "certificate") { |
There was a problem hiding this comment.
Are there any TLS errors which contain certificate but not tls:?
I know that x509: is another prefix in certain circumstances, depending upon the nature of the failure, but don't know which errors might trigger the second check to match but not the first.
There was a problem hiding this comment.
good point, think we can just check tls: here
| return err | ||
| } | ||
| if errors.Is(err, io.EOF) || isConnClosedError(err) { | ||
| return fmt.Errorf("nats: connection closed by remote after TLS handshake: %w", err) |
There was a problem hiding this comment.
Only saying this in case you haven't considered it: perhaps nats: tls: ... as a prefix so that tls: checks will match?
There was a problem hiding this comment.
haven't considered it, is that to help doing sttring.Contains on the tls: prefix? wdyt should we add it?
There was a problem hiding this comment.
@piotrpio wdyt about a good error string here?
3 participants
When a TLS proxy (e.g. nginx with mTLS) completes the TLS handshake but then closes the connection due to an untrusted client certificate, the client would receive a bare "EOF" most of the time, making it difficult to to troubleshoot (since not sending tls alerts):
Now we wrap these
io.EOFerrors with more context that this happened after the TLS handshake:The wrapped error preserves the original via %w for backwards compatibility (
errors.Is(err, io.EOF)still returns true).