Fix HRR with early_data #3933
Fix HRR with early_data #3933
Conversation
early_data is not allowed after an HRR. We failed to handle that correctly.
| @@ -157,13 +157,8 @@ int ossl_statem_skip_early_data(SSL *s) | |||
| if (s->ext.early_data != SSL_EARLY_DATA_REJECTED) | |||
| return 0; | |||
|
|
|||
| if (s->hello_retry_request) { | |||
| if (s->statem.hand_state != TLS_ST_SW_HELLO_RETRY_REQUEST) | |||
There was a problem hiding this comment.
Consider the case
ClientHello---------------->
stream EarlyData-------->
<-------------------HelloRetryRequest
still have some EarlyData in flight------->
ClientHello2--------------->
<-------------------ServerHello
Since the network is not instantaneous, we still need to be able to skip over early data records until we receive another handshake record. (See the list of three behaviors in section 4.2.9 of draft-21.)
There was a problem hiding this comment.
Well, in fact, the server sends the HRR immediately after processing the ClientHello. So from a server's perspective all early data records arrive afterwards (as if they had still be in flight). This PR is all about skipping early data records after HRR. We now handle it in a manner consistent with how we handle early data if there is no HRR, i.e. we enter the special TLS_ST_EARLY_DATA state. This means that the ossl_statem_skip_early_data() function no longer needs to special case an HRR.
| */ | ||
| if (!TEST_true(SSL_SESSION_set_time(sess, time(NULL) - 20))) | ||
| goto end; | ||
| } | ||
|
|
||
| /* Write some early data */ |
There was a problem hiding this comment.
We should probably add a test case that explicitly sends another early data record after receiving a HRR but before sending CH2, to simulate the "packets already in flight" behavior.
There was a problem hiding this comment.
As noted above all early data packets are processed after HRR anyway, so I'm not sure if this would gain us much.
|
Ping @kaduk |
| @@ -397,7 +396,8 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s) | |||
| return WRITE_TRAN_CONTINUE; | |||
|
|
|||
| case TLS_ST_SW_HELLO_RETRY_REQUEST: | |||
| return WRITE_TRAN_FINISHED; | |||
| st->hand_state = TLS_ST_EARLY_DATA; | |||
| return WRITE_TRAN_CONTINUE; | |||
There was a problem hiding this comment.
This becomes CONTINUE instead of FINISHED so that we hit the post-work that flushes the state machine? (Our write transition from TLS_ST_EARLY_DATA just returns WRITE_TRAN_FINISHED having done no work.)
There was a problem hiding this comment.
There is some pre-work for the early-data state which gets invoked.
|
Pushed. Thanks. |
early_data is not allowed after an HRR. We failed to handle that correctly. Reviewed-by: Ben Kaduk <kaduk@mit.edu> (Merged from #3933)
Reviewed-by: Ben Kaduk <kaduk@mit.edu> (Merged from #3933)
Early Data after an HRR is not allowed. We failed to handle that scenario correctly (s_client would hang). This fixes it and adds a test.
I swear this used to work!!
Checklist