Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support get_ssl_ctx callback for client #186

Merged

Conversation

sumasrao
Copy link
Contributor

Extending the get_sst_ctx callback to support setting SSL contexts on the client side similar to the server implementation.

@sumasrao
Copy link
Contributor Author

sumasrao commented Oct 29, 2020

@dtikhonov Can you please check the changes and see if using the existing API for setting the client SSL CTX would work? Thanks.

@dtikhonov
Copy link
Contributor

It looks good -- thank you! I made a modification to keep ea_get_ssl_ctx() optional for the client -- check out branch 202010291309-client-get-ssl-ctx.

One question that we need to answer is: what to do with the session tickets? Current code sets the callback to an internal function in order to encode transport parameters:

        if (enc_sess->esi_enpub->enp_stream_if->on_sess_resume_info)
            SSL_CTX_sess_set_new_cb(ssl_ctx, iquic_new_session_cb);

This does not happen for SSL_CTX that is returned by the new call in the client.

@dtikhonov
Copy link
Contributor

(Continuing conversation from here.)

This is a good point: now that SSL_CTX can be provided via a callback instead of the client making a new one, there is no reason to keep keylog facilities in the lsquic APIs!

The same goes for the certificate verification callback -- the user can set it in the SSL_CTX.

The new session ticket callback is not so simple:

  1. The session ticket and transport parameters (the transport parameters must be used for 0-RTT data) are serialized in iquic_new_session_cb(). That logic could be taken out of the library and then it would be up to the client to fetch transport parameters, store them and the new ticket, and then pass both of them to lsquic_engine_connect().

  2. The bigger problem with (1) is the change in the API. Now, IETF QUIC and gQUIC would have different ways to do session resumption -- both providing the session information to the user and passing it to the "connect" function.

  3. When new session ticket becomes available, AL_SESS_TICKET alarm is cancelled in iquic_new_session_cb(). The client waits two seconds for new session ticket before dropping SSL object to release memory. If lsquic does not know when new session information is available, it has to wait full two seconds before dropping SSL object. This is probably just a minor inefficiency.

We will accept this PR and add two TODO items:

  1. Short-term: remove keylog and cert verification callbacks from the lsquic API.

  2. Long-term: change the way session resumption is handled in lsquic. This will probably have to wait until gQUIC support is removed from the library. (ca. 2022?)

@litespeedtech litespeedtech merged commit 21bcad8 into litespeedtech:master Nov 2, 2020
@sumasrao sumasrao deleted the support_client_ssl_ctx branch November 2, 2020 22:08
litespeedtech pushed a commit that referenced this pull request Nov 4, 2020
- [API] Allow use of ea_get_ssl_ctx() on the client (optional).  PR #186.
- [BUGFIX] Expand datagram with ack-eliciting Initial to 1200 bytes
  after connection promotion.
- [BUGFIX] Discard CRYPTO frames from lower encryption levels after
  connection promotion.
- [BUGFIX] Cancel path response if path could not be initialized.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants