Raw handshake I/O for QUIC

[Ralith]

This works towards supporting the new QUIC TLS interface, wherein QUIC subsumes the TLS record layer, taking over responsibility for fragmentation, encryption/decryption, and communication of alerts.

While read_hs as written should handle incoming data correctly, I'm unsure how to proceed with handling outgoing handshake data and alerts. The current implementation performs encoding of alerts, encryption, and fragmentation eagerly, all of which are undesired.

Ideally I'd like to defer these operations to being performed on demand, so that the QUIC interface can easily pass the data out as-is, while the regular interface performs the transformations necessary for conventional use. This seems likely to be a more invasive change, so before I proceed I'd like feedback on the idea, and input on how to approach implementing it.

This interface will likely need to be further extended with helpers to perform the necessary key derivation, which uses different labels than that of TLS.

[coveralls]

Coverage decreased (-1.8%) to 95.041% when pulling be6348e on Ralith:quic-13 into eb2556d on ctz:master.

[Ralith]

Took a swing at an API for key derivations. I had to touch a whole bunch of stuff to make that happen, so I'd really like to hear opinions on the approaches taken before I fill it out the rest of the way.

[Ralith]

Thanks for the feedback! I'll dig into the proposed changes later today.

[Ralith]

Applied the requested changes. I'll now start filling out the remaining key derivation functionality.

Still hoping to hear some thoughts on the refactoring needed for getting outgoing messages and alerts prior to encryption and fragmentation.

[Ralith]

Tests currently failing on QUICTransportParams-{Client,Server}-Rejected-TLS12. I'm not sure where those are coming from; I thought I removed them. Those tests make no sense to me, because QUIC does not use TLS1.2 at all: a QUIC implementation is required to reject attempts to negotiate a TLS 1.2 connection regardless of the presence of transport parameters.

edit: Found the proper way to disable these.

[ctz]

Still hoping to hear some thoughts on the refactoring needed for getting outgoing messages and alerts prior to encryption and fragmentation.

If I were doing this I'd extract a trait from the necessary functions on SessionCommon (like send_msg, send_msg_encrypt, etc.) and move data they require (the sequence numbers, message fragmenter, encryption/decryption things, etc.) into a struct that implements that trait.

[Ralith]

I'd extract a trait from the necessary functions

And then have a no-op implementation for QUIC? Hmm, that could work. I wonder if it would be cleaner to instead defer all encryption and fragmentation as late as possible (i.e. perform it on-demand inside SessionCommon::write_tls), and thereby keep a common code path and dodge any virtual dispatch.

[djc]

I wonder if it would be cleaner to instead defer all encryption and fragmentation as late as possible (i.e. perform it on-demand inside SessionCommon::write_tls), and thereby keep a common code path and dodge any virtual dispatch.

This sounds like it would map better to what QUIC is doing these days.

[Ralith]

After some discussion with @djc we realized there's a much less invasive approach to handling outgoing messages. I think the necessary API is now fully prototyped; next steps should be to try to use it in practice, and gather any further feedback.

[Ralith]

It would be nice if TLSError had pub fn alert(&self) -> Option<AlertDescription> or similar; in that case, QuicExt::take_alert could probably be removed in favor of inspecting the return value of QuicExt::read_hs.

[Ralith]

Travis seems to be failing because QUICTransportParams-Client-TLS13 constructs configurations that accept TLS versions other than 1.3. Is that intended?

[Ralith]

Quinn is now employing the full breadth of this API to successfully communication with other implementations speaking the current draft, which is fortunately less invasive than draft-13 was. I think we're ready for another round of review.

[ctz]

This looks all reasonable to me, code-wise. The things I'd like to see are some basic tests that exercise the new public API (in tests/api.rs or perhaps tests/quic.rs). Mainly so I can be sure I don't break this code in the future without resorting to building and testing dependent projects.

[Ralith]

Good thought, I'll bang that out.

Reviewing this code, I've also been thinking about making SessionCommon::quic an Option<quic::State> that only exists if the feature is enabled, rather than the current approach of having a separate Protocol enum. Protocol made more sense when KeySchedule had to be aware of the state, but that's no longer necessary.

[Ralith]

This test covers everything except get_early_secret, which I don't see a good way to test until rustls itself supports enabling 0-RTT on the server

[Ralith]

I managed to fill in the necessary parts for QUIC 0-RTT support without too much disruption. I also added the transport parameters to client session storage under QUIC, because this is always required and is very simple for rustls to provide.

[ctz]

This'll get merged tomorrow.

[Ralith]

Rebased, and added some last-minute internal fixes isolated into the final 3 4 commits for easy review.

Note that most of the server-sized 0-RTT changes here are generally applicable to TLS1.3, and should be pulled out of the feature guards if/when conventional TLS1.3 0-RTT is implemented.

Still dunno what's going on with the bogo tests trying to combine TLS1.2 with QUIC, but presumably it's a trivial fix.