feat(s2n-quic-dc): Send detailed transport error information over the wire#3065
feat(s2n-quic-dc): Send detailed transport error information over the wire#3065ben-r-smith wants to merge 6 commits intoaws:mainfrom
Conversation
ben-r-smith
changed the title
| ClientProviders | ||
| ); | ||
|
|
||
| #[cfg(any(test, feature = "unstable-provider-connection-close-formatter"))] |
There was a problem hiding this comment.
The stable builder methods have some pretty lengthy comments on them, though it seems we've fallen out of that practice for the unstable methods. I think its probably worth documenting in a comment what this is for
There was a problem hiding this comment.
Added some info including an example here.
| //# Reason Phrase field and SHOULD use the APPLICATION_ERROR code when | ||
| //# converting to a CONNECTION_CLOSE of type 0x1c. | ||
|
|
||
| transport::Error::APPLICATION_ERROR.into() |
There was a problem hiding this comment.
are you losing anything interesting by doing this?
There was a problem hiding this comment.
My understanding is we need to clear the reason field regardless so we wouldn't be able to pass this over in a similar fashion. That brings into question if this should be customizable at all. Maybe I'm misunderstanding something?
There was a problem hiding this comment.
I think it all comes down to the threat model of how you are using it. For Development anything is fine, so the customization is allowed. I was just trying to understand if there is anything this would throw away that could be useful for s2n-quic-dc and still safe given the threat model. But its not a one-way door, so we can change it later anyway
| use s2n_quic::provider::connection_close_formatter::{ConnectionClose, Context, Formatter}; | ||
| use s2n_quic_core::{application, transport}; | ||
|
|
||
| /// A formatter that passes transport errors through for debuggability. |
There was a problem hiding this comment.
Could this comment be fleshed out a bit, just explaining what this does more specifically from the Production and Development formatters
There was a problem hiding this comment.
And perhaps a small discussion on when/why it is safe to use this formatter
There was a problem hiding this comment.
Yes, updated for both.
2 participants
Release Summary:
Resolved issues:
N/A
Description of changes:
Currently, s2n-quic does not support overriding the connection close formatter on its endpoints. The default formatter for production does not share specific transport error reasons for security purposes.
This PR introduces support for the connection close formatter on QUIC server/client endpoints. It then utilizes this feature in s2n-quic-dc to send transport error messages as-is to the remote.
Call-outs:
N/A
Testing:
A test was added to s2n-quic-dc.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.