Add QUIC stateless reset test #23384
Conversation
QUIC supports the concept of stateless reset, in which a specially crafted frame is sent to a client informing it that the QUIC state information is no longer available, and the connection should be closed immediately. Test for proper client support here
In writing the quic stateless reset test we found that the quic rx code wasn't checking for stateless reest conditions, as the SRT frames were getting discarded due to failed lcdim lookups. Move the SRT check above the lcdim lookup in the rx path to ensure we handle SRT properly in the client.
nhorman
force-pushed
the
stateless_reset_test
branch
from
ef3f67d
to
8649834
Compare
nhorman
added
branch: master
|
hmm, it would seem the macos tests are timing out waiting for the connection close condition to be detected. Unfortunately I cant OP_[S|C]WRITE anything to force a socket read, as that fails prior to the expecation of the close detection. OP{C|S]_READ_FAIL looks like it might be promising, but those also timeout detecting a close condition. Thoughts? |
t8m
added
triaged: bug
|
@hlandau, scratch that, found a way around it. The close condition on the client is consistent and suffices to confirm.thw reset. Side note: there is a thread race that occurs when the inject function is changed during a test, but I chose to avoid it by setting the function at the start of the script, rather than introduce a bunch of new locking |
|
ACQUIRE_S() doesn't work in injector functions as helper_local isn't passed. Will need to find a way around that |
hlandau
added
approval: review pending
t8m
added
approval: done
|
Note: tested locally against 3.2 here and the test fails. It appears to be occuring because, during stateless reset injection on 3.2 demux_process_pending_urxe issues a callback to reset_token_cb to determine if the frame is an SR, and, if so, returns -1, whcih goes back up the call stack, causing an error in SSL_inject_net_dgram, resulting in a failed test. The master branch silently discards SR frames in port_default_packet_handler after closing the connection as a side effect. Point being, backporting to 3.2 will take some additional work in the quic code to operate properly |
|
24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually. |
nhorman
added
approval: ready to merge
|
setting ready to merge as my last comment blocked openssl-machine from automating it |
|
Although this is "ready-to-merge" - it seems its not really. At least not to 3.2 since tests would fail if it is merged there. I guess we're blocked on having fixes for the 3.2 issues. |
|
You can merge it to master. 3.2 is a separate issue and PR. |
|
I've removed the 3.2 label for now to prevent an accidental merge there (although I believe the approvals above indicate it can be merged there once 3.2 is fixed) |
|
merged |
QUIC supports the concept of stateless reset, in which a specially crafted frame is sent to a client informing it that the QUIC state information is no longer available, and the connection should be closed immediately. Test for proper client support here Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #23384)
In writing the quic stateless reset test we found that the quic rx code wasn't checking for stateless reest conditions, as the SRT frames were getting discarded due to failed lcdim lookups. Move the SRT check above the lcdim lookup in the rx path to ensure we handle SRT properly in the client. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #23384)
|
@nhorman IMO create a separate issue for the 3.2 investigation and close this. |
|
Ack will do |
QUIC supports the concept of stateless reset, in which a specially crafted frame is sent to a client informing it that the QUIC state information is no longer available, and the connection should be closed immediately. Test for proper client support here Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#23384)
In writing the quic stateless reset test we found that the quic rx code wasn't checking for stateless reest conditions, as the SRT frames were getting discarded due to failed lcdim lookups. Move the SRT check above the lcdim lookup in the rx path to ensure we handle SRT properly in the client. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#23384)
QUIC supports the concept of stateless reset, in which a specially crafted frame is sent to a client informing it that the QUIC state information is no longer available, and the connection should be closed immediately. Test for proper client support here Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#23384)
QUIC supports the concept of stateless reset, in which a specially
crafted frame is sent to a client informing it that the QUIC state
information is no longer available, and the connection should be closed
immediately. Test for proper client support here
Checklist