Add Initial Early Data support

[ad-chaos]

We had proposed a design in #14013, we've implemented it for OpenSSL initially for discussion, it can be implemented similarly for other SSL backends.

Since early data is sent during the connection itself we defer the connection until ossl_send in order to send early data.
CURLOPT_SSL_EARLYDATA can be set to 1L in order to enable sending early data.

Currently if early data was rejected, we do nothing. How would you suggest we handle this? Fallback to a normal send again or error out? Resending the request would require some co-operation from multi_runsingle, erroring out is easy.

All tests pass, the ones that fail are for missing documentation for CURLOPT_SSL_EARLY_DATA.

[alexsn]

RFC 8446 mentions: "If the server rejects early data, it will respond with a ServerHello or HelloRetryRequest that does not contain the "early_data" extension. In this case, the client MUST discard all early data sent and continue the handshake as if the early data had never been sent."

To answer your question: on error you MUST resend the data as if "early_data" was never used.

[icing]

First off, this looks good, going in the right direction!

To answer your question: on error you MUST resend the data as if "early_data" was never used.

Yes, indeed. How this works in this design would be as follows:

1. first ossl_send() writes via SSL_write_early_data(), then continues the handshake. This may not complete on the first attempt and need to return.
2. Returning before handshake is complete must return CURLE_AGAIN and remember how much data was already written.
3. next ossl_send() calls continue the handshake, once that succeeds, check the EarlyData indicator from the server.
   a. Accepted: Return CURLE_OK with the amount of data send in the initial SSL_write_early_data().
   b. Rejected: forget the earlier amount and SSL_write_ex() the complete mem content. Return what that resulted in

You may want to check in 3 that this is being called with at least the len that has been written into early data.

Another suggestion: if you defined a flag like SSLSUPP_EARLYDATA, you can set that in the Curl_ssl_openssl and avoid the additional early_data callback member. That way, you would not need to change the other TLS backends.

[ad-chaos]

Thanks for the comments!

This may not complete on the first attempt and need to return

Openssl says that unless SSL_MODE_ENABLE_PARTIAL_WRITE is enabled, it will not return success if the complete chunk was not written. We're assuming that it instead raises SSL_WANT_READ and in that case currently curl sets written to -1 and tries the whole chunk again.

SSL_MODE_ENABLE_PARTIAL_WRITE
Allow SSL_write_ex(..., n, &r) to return with 0 < r < n (i.e. report success when just a single record has been written). This works in a similar way for SSL_write(). When not set (the default), SSL_write_ex() or SSL_write() will only report success once the complete chunk was written. Once SSL_write_ex() or SSL_write() returns successful, r bytes have been written and the next call to SSL_write_ex() or SSL_write() must only send the n-r bytes left, imitating the behaviour of write().

Given this information, maybe we don't have to keep track of written bytes?

Thus, if we succeed in writing early data, we complete the connection then check early data status, if early data was rejected we resend the message on this newly formed connection.

[icing]

Thanks for the comments!

This may not complete on the first attempt and need to return

Openssl says that unless SSL_MODE_ENABLE_PARTIAL_WRITE is enabled, it will not return success if the complete chunk was not written. We're assuming that it instead raises SSL_WANT_READ and in that case currently curl sets written to -1 and tries the whole chunk again.

You may early_write the whole buf, yes, but the handshake is not complete. So you do not know if you can skip the buf content until the server completes the handshake and acknowledges or rejects the early data.

Thus, if we succeed in writing early data, we complete the connection then check early data status, if early data was rejected we resend the message on this newly formed connection.

In order to resend, you would need to have the buf with the same bytes as before. To achieve that, the method should return CURLE_AGAIN until the handshake is done, right?

[ad-chaos]

In order to resend, you would need to have the buf with the same bytes as before. To achieve that, the method should return CURLE_AGAIN until the handshake is done, right?

yes, so we'll set written to -1 and return CURLE_AGAIN
d122018

[bagder]

Can we add this feature as a new flag to CURLOPT_SSL_OPTIONS instead of creating a new option?

[ad-chaos]

Can we add this feature as a new flag to CURLOPT_SSL_OPTIONS instead of creating a new option?

Done

Also removed the unnecessary checking before setting earlydata since only the backend implementation file will ever read it

[ad-chaos]

What we are doing now is deferring connect until ossl_send and on the first successful send we complete the handshake. This doesn't maximally utilise sending early data since the application level request might be chunked and sent over multiple calls to ossl_send, protocols like http/2 can call ossl_send after connecting on the tls socket in MSTATE_CONNECTING to send the preface and settings and in MSTATE_DO send the actual request.

If we were to send the entire request as early data, we'll need to defer the completion of the connection until we've sent the entire request, maybe in something like Curl_req_done. That would also require an addition of a new method like transfer_done to cfilter and vtls. If the server rejects early data in this case, we'll have to keep track of all the request 'chunks' and retry them afterwards. Should we go down this route? Or keep the early data limited to whatever is in the first ossl_send?

As an admittedly incredibly terrible hack, the latest commit 13a8a7c skips cf_h2_connect if early data is enabled and thus anything that would be sent during the MSTATE_CONNECTING phase is sent during the MSTATE_DO phase, thus maximally utilising sending of early data.

[icing]

What we are doing now is deferring connect until ossl_send and on the first successful send we complete the handshake. This doesn't maximally utilise sending early data since the application level request might be chunked and sent over multiple calls to ossl_send, protocols like http/2 can call ossl_send after connecting on the tls socket in MSTATE_CONNECTING to send the preface and settings and in MSTATE_DO send the actual request.

If we were to send the entire request as early data, we'll need to defer the completion of the connection until we've sent the entire request, maybe in something like Curl_req_done. That would also require an addition of a new method like transfer_done to cfilter and vtls. If the server rejects early data in this case, we'll have to keep track of all the request 'chunks' and retry them afterwards. Should we go down this route? Or keep the early data limited to whatever is in the first ossl_send?

As an admittedly incredibly terrible hack, the latest commit skips cf_h2_connect if early data is enabled and thus anything that would be sent during the MSTATE_CONNECTING phase is sent during the MSTATE_DO phase, thus maximally utilising sending of early data.

Yeah, this is tricky. I think you should scope the work to remain in the openssl filter only. As you noticed, the optimal use of early data will require all filters "above" to send as much as they can in the first call. Meaning HTTP/2 would need to send the initial frames together with the first stream frames.

tl;dr

To make this work in the best way, the complete MSTATE_CONNECTING would need to be eliminated and all the connect work must happen lazily in the MSTATE_DO phase. That is a lot of work to get it right.

Personally, I would restrict this PR to do the right thing for HTTP/1.1 request without any proxy involved.

[icing]

My summary on reviewing this PR and how I think we should go forward:

1. I think we should implement Early Data on one TLS backend first, before changing all the others. This would make it easier to get it "air tight" and allow us to focus the technical discussions on one source. What you did on the other TLS backends remains good work. We just save effort by doing one first.
2. You correctly foresaw the problem of getting a filter recv() call before the handshake is done. Looking at OpenSSL, which seems the most advanced here, you simulate a 0-length early data send and return, but do not progress the handshake. I am not sure if a subsequent ossl_send() does the right thing.
3. In openssl.c:5085, on having successfully written the early data bytes (meaning OpenSSL has sent them off) you proceed the ossl_connect() as should be, but the DEBUGASSERT() on line 5089 does not need to hold. The ossl_connect() can have retries. If you remove the ASSERT, the logic does not hold, as the ossl_send() would return success before the server answer is back.

The handling of all possible states here is quite complex. There is SSL_write_early_data() that may fail to send everything and need retrying. And even if it succeeds, it means only that the ClientHello plus data has been flushed down the filter chain. It might be on its way to the server or it might be in the early data buffer of a TLS filter "below" this one. The response from the sever, which may be negative, may take several calls to arrive and be processed.

I think it works better when we enhance ossl_connect() and have that as a "guard" at the beginnings of ossl_send()/ossl_recv(). Only when that succeeds can these functions progress as before. Something like:

CURLcode ossl_connect_send(struct Curl_cfilter *cf,
                         struct Curl_easy *data,
                         const void *buf, size_t blen,
                         size_t *pconsumed);

which SSL_write_early_data() the buf until that succeeds, then progresses the connect, returning CURLE_AGAIN until finished and on CURLE_OK returns in pconsumed how many of the early data bytes have been accepted by the server.

Analog a

CURLcode ossl_connect_recv(struct Curl_cfilter *cf,
                         struct Curl_easy *data);

for guarding ossl_recv(). This requires the early data to be buffered internally, so the send/recv calls can be interleaved.

Does that make sense?

I do not know how much time you can spend on this. If you want me to, I can make a proposal in a separate PR.

[icing]

I think the TLS filters should be unaware of what bytes are actually send, e.g. what filter sits "above".

I think we should defer the improvement of overall request sending outside of this PR, so that HTTP/2 can benefit as much as HTTP/1.1 does.

[icing]

I would prefer us to make a solid implementation on one TLS backend first, before changing others. Since curl is now on debian with GnuTLS, maybe we should focus on that.

Once we know how we want this feature to be implemented, it should be easy to adapt other backens. You have laid the groundwork here quite nicely.

[icing]

I think this flag and all things we need for handling of early data should reside in struct ssl_connect_data.

[icing]

Would it not be easier to store the max amount of early data in struct ssl_connect_data on init and one?

[icing]

I'll make a general comment on the handling of connect/send/recv at the end.

[ad-chaos]

Superseded by #15211