Comments (8)
It seems like
ClientConnection
andServerConnection
should have theirConnectionCommon<Data>
member replaced with an adapter layer that holds anUnbufferedConnectionCommon<Data>
and additional buffers (ChunkVecBuffer
instances?) that can augment theUnbufferedConnectionCommon
state for use when handling eachUnbufferedStatus
we encounter while processing.
I think my high-level goal would be that ConnectionCommon<Data>
will contain an UnbufferedConnectionCommon<Data>
plus fields that contain the buffers. I think ServerConnection
and ClientConnection
types should still hold ConnectionCommon<Data>
because buffered/unbuffered is orthogonal to client/server AIUI.
Other than this detail, that all sounds good!
from rustls.
I have some initial thoughts but I'll hold off until the general idea of trying to handle one
UnbufferedStatus
per call toprocess_new_packets
is confirmed as sensible - it might be that I'm jammed up here because it's not the right approach.
I think one call of process_new_packets
should handle potentially many UnbufferedStatus
es, until it gets ConnectionState::BlockedHandshake
or has a completely empty received data buffer. The motivation for that is to maintain the existing API behaviour: where one read_tls
can read many many individual TLS messages in one go, and then process_new_packets
has to work through every one to make further progress.
Remainder sounds good!
from rustls.
I think ServerConnection and ClientConnection types should still hold ConnectionCommon because buffered/unbuffered is orthogonal to client/server AIUI.
I think we're on the same page here. Agreed for sure that the buffered/unbuffered part is orthogonal to whether it's a client or server conn.
I think one call of process_new_packets should handle potentially many UnbufferedStatuses, until it gets ConnectionState::BlockedHandshake or has a completely empty received data buffer. The motivation for that is to maintain the existing API behaviour: where one read_tls can read many many individual TLS messages in one go, and then process_new_packets has to work through every one to make further progress.
Ah ok yes, that makes sense and I think resolves some of the uncertainty I had with regards to how to map some of the UnbufferedStatus
variants to operations within process_new_packets
Thank you both for the input 🙇
from rustls.
How about doing process_new_packets
inside ClientConnection::new
? That would maintain the invariant that a client starts out life with something to send.
from rustls.
I've spent some time thinking about this. Here's my thought process so far. @ctz, @djc does this match your expectations?
It seems like ClientConnection
and ServerConnection
should have their ConnectionCommon<Data>
member replaced with an adapter layer that holds an UnbufferedConnectionCommon<Data>
and additional buffers (ChunkVecBuffer
instances?) that can augment the UnbufferedConnectionCommon
state for use when handling each UnbufferedStatus
we encounter while processing.
I'm imagining that the loop over UnbufferedConnectionCommon<Data>
's process_tls_records_common()
results (e.g. this code in the client example) needs to be done in the adapter layer's process_new_packets()
fn, but using a single call to process_tls_records_common()
instead of in a loop while open_connection { ... }
. Each call would handle one UnbufferedStatus
state, discarding from the adapter's internal buffer at the end.
I think the reader
, writer
, read_tls
and write_tls
, and set_buffer_limit
fns would be implemented based on the adapter's internal buffers. I think since complete_io
is already implemented in terms of the other fns, it would be unchanged. Similarly I think export_keying_material
and dangerous_extract_secrets
fns are unchanged.
I think the majority of work would be in adapting process_new_packets
to handle each of the UnbufferedStatus
states. I need some assistance figuring out how to map the states to actions on the connection/buffers. I have some initial thoughts but I'll hold off until the general idea of trying to handle one UnbufferedStatus
per call to process_new_packets
is confirmed as sensible - it might be that I'm jammed up here because it's not the right approach.
WDYT?
from rustls.
I'm still struggling to find a way to make the strategy I described above work with the existing buffered and unbuffered API designs. The biggest issue I'm wrangling right now is how to rectify the design of a client like tlsclient-mio
with the way I was trying to structure the adapter between the buffered API and the unbuffered API.
Currently for both APIs the guts of the library put writable tls data into the core.common_state.sendable_tls
buffer. In the existing buffered API the write_tls
function on ConnectionCommon
drains directly out of that common state buffer to the writer. In the unbuffered API that core.common_state
buffer is pop
'd from in UnbufferedConnectionCommon
's process_tls_records_common
fn, yielding EncodeTlsData
state UnbufferedStatus
events that the caller has to handle, producing bytes that get written to the caller's own outbound TLS buffer that it must write to a socket once the TransmitTlsData
or WriteTraffic
events are encountered.
My idea for how to replace the buffered API with an adapter to the unbuffered API was to have the adapter layer maintain that second outbound TLS buffer, writing to it as dictacted by the UnbufferedStatus
's it processes, and draining from it in write_tls
. The process_new_packets
fn would be responsible for handling EncodeTlsData
state's by encoding the data that was popped from the common state buffer, writing the contents into the adapter's outbound TLS buffer. After processing all of the unbuffered events until a connection close or a blocked handshake state the adapter's process_new_packets
fn would yield an IoState
with a tls_bytes_to_write
field set to the length of the adapter's sendable TLS buffer.
The issue I'm seeing with tlsclient-mio is that it doesn't call process_new_packets
until it finds the socket it's polling is readable. It calls ConnectionCommon::write_tls()
up-front, which with the existing bifurcated API is enough to push the client hello to the socket (since its draining directly from the common state buffer) and then once the server writes its response the socket polls as readable and only then does it begin processing TLS packets. With the adapter based API, calling write_tls
before process_new_packets
is fruitless: the adapter's sendable TLS buffer is always empty at this stage. The client hello bytes won't be present in that buffer until we process the unbuffered EncodeTlsData
event from a call to process_new_packets
. Since there are no bytes to write from write_tls
before processing we stall here - the socket is never readable because we never sent a hello.
Is there something obvious I'm missing or is this approach a non-starter?
from rustls.
Is there something obvious I'm missing or is this approach a non-starter?
Maybe the updated connection common write_tls
should call process_new_packets
itself before trying to write from the adapter's sendable TLS buffer? I'm not sure if that will break any other assumptions 🤔
from rustls.
Continuing my practice of rubber ducking long comments into this issue.
How about doing process_new_packets inside ClientConnection::new? That would maintain the invariant that a client starts out life with something to send.
That was a helpful suggestion, but the same general problem crops up elsewhere.
APIs that queue data for the next write_tls
The existing API offers methods that are described as queuing tls data to be sent on the next write (e.g. send_close_notify). With the buffered->unbuffered bridge approach calling one will queue deframer buffer data but as before, without a call to the bridge processing logic to handle the pending EncodeTlsData
state unbuffered statuses there's nothing for write_tls
to write immediately after the call. This breaks the described API and was stalling the tlsserver-mio example. I experimented with my idea to have write_tls
call the bridge processing logic before trying to write and that seems to work, but its a blunt solution.
I have a local branch that works for all of the examples/
programs but I had to do some unpleasant refactoring to get there and I don't think it's the right solution. The challenge I keep bumping into is that the unbuffered API processing logic isn't as generic over a connection that is either a client connection or a server connection as the classic API is.
Loss of "side" generality in conn types
In the existing API most of the interesting processing logic fns (e.g. process_new_packets
and complete_io
) are defined on ConnectionCommon<Data>
or ConnectionCore<Data>
, generic over both client/server data. In turn the Stream
and OwnedStream
types can be generic over anything that derefs to ConnectionCommon<S>
. In the new buffered API there isn't an equivalent to the Connection
enum and the processing logic entry-point (process_tls_records
) is implemented per-side (on UnbufferedConnectionCommon<ClientConnectionData>
and UnbufferedConnectionCommon<ServerConnectionData>
respectively). Unbuffered client and server connections process unbuffered status events differently to account for early data: clients can send early data, servers can read early data.
I tried to centralize as much of the processing logic as I could into generic BufferedConnectionCommon<Side>
impls but ultimately the process_new_packets
equivalent needed to be implemented on BufferedConnectionCommon<ClientConnectionData>
and BufferedConnectionCommon<ServerConnectionData>
instead of the generic type. This in turn spreads like an infection backwards through the API. We need to be able to process packets from write_tls
to address the issues highlighted at the start of my comment, and so that gets lifted to the per-side impl. We need to be able to process packets from complete_io
, so that gets lifted to the per-side impl. Now the Stream
can't be generic over any ConnectionComm<S>
because it too relies on process_new_packets
and complete_io
. To avoid making ClientStream
, ClientOwnedStream
, ServerStream
, ServerOwnedStream
-like-types I re-implemented Stream
in terms of the Connection
enum, which can then have general fns that match
over itself and dispatch to the correct per-side version but it's all very awkward....
I'm going to try and think about this more but I thought if I shared the issues I've encountered someone might have some input.
from rustls.
Related Issues (20)
- 0.23 docs build is broken HOT 2
- optimize receiving data with TLS 1.2 and aes-128-gcm HOT 1
- optimize receiving data with TLS 1.3 and aes-256-gcm
- optimize server-side full handshakes for TLS 1.2 and 1.3 HOT 1
- Connection::dangerous_extract_secrets returns ConnectionTrafficSecrets::Aes128Gcm even when AES-256-GCM is negotiated
- Error: badRecordMac HOT 4
- Cipher suites configured through WebPkiServerVerifier::builder_with_provider is not working. Client hello contains more cipher suites then it configured. HOT 6
- rand_core::RngCore & CryptoRng support for CryptoProvider HOT 7
- expose more information in ClientHello HOT 4
- No common ciphersuit when FFDHE and ECDHE ciphersuites are available on server and client using TLS 1.2 HOT 4
- US Export control information HOT 4
- doc: AcceptedAlert::write doesn't necessarily write all bytes HOT 7
- Support using rustls without using specific ring or aws-lc-rs apis HOT 2
- Feature request: a way to set/get default ticketer dynamically HOT 6
- Feature Request: Avoid panicking when ring and aws_lc_rs are both specified HOT 19
- Option to Relax SNI Host Name Validation for IP Addresses HOT 7
- unbuffered: B: `CapacityBuffer` for `output_tls.capacity()` HOT 9
- The support for "mipsel-unknown-linux-musl" has failed. HOT 2
- Io(Custom { kind: InvalidData, error: AlertReceived(HandshakeFailure) }) HOT 6
- Linux compilation is slow and seems unable to store compilation results HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from rustls.