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

Add the Miou implementation of TLS #494

Closed
wants to merge 3 commits into from
Closed

Conversation

dinosaure
Copy link
Contributor

This PR provides an implementation of tls with miou. It provides also a test which checks randomly different actions between a client and a server. The package is actually tested with httpcats (for http/1.1 and h2) and, if we want to take the opportunity to do some HTTP requests in parallel, this patch about mirage-crypto is needed - but it is not necessary if we only use Miou.call_cc which spawns a task concurrently.

The fuzzer

The fuzzer tests the behavior of Tls_miou when we use these actions:

type operation =
  | Send of string
  | Recv of int
  | Shutdown of [ `read | `write ]
  | Close
  | Noop

It does not test the TLS implementation (if we are able to correctly encrypt/decrypt the session). These actions are randomly chosen by the fuzzer. The idea is to produce a suit of actions for a client and a server. Note that we don't generate a situation where both read or both write "at the same time". The generation has 4 points where we are able to close, shutdown(recv) or shutdown(write). An example is:

client: [send(64); noop; recv(64); shutdown(send); recv(64); noop; noop; close; noop; close]
server: [recv(64); noop; send(64); shutdown(send); send(64); close; noop; close; noop; close]

Then, we launch a TLS server and start a new session with a client. Both do actions and fill a buffer. We catch all possible errors and terminate properly the server and the client. Finally, we compare what the server received and what the client received with an oracle. The oracle is the compile function. The rules are simple:

  • we record when the client and the server shutdown/close the connection
  • if the client/server want to send/recv something, we check the status of the connection. We continue if everything is normal (none of them shutdown/close their sides) or we directly return the final result (in our implementation, we should raise an exception End_of_file or Closed_by_peer).

The error cases

Actually, from an UNIX socket, read can raises ECONNRESET and write can raise EPIPE and ECONNRESET. For our perspective, these cases tell us that the connection is closed (on a specific side or totally). We handle these cases and transfer the information to the internal TLS state.

Another issues encountered is about read and close. Our fuzzer uses a UNIX socket - we can not rely on the throughput of the network. It can arrives that the server get the whole data and a close-notify before to do anything. In this situation, we must returns what we got and consider, afterwards, the connection closed even if the TLS state is actually `Closed.

Finally, the previous point enforces the fact that we must returns what the client/server received even if the connection was closed (or shutdown(recv)). However, the user can also decide to close or shutdown(recv) the connection before to read anything. We must drop pending application-data in such situation.

Parallelism

Currently, as said in the introduction, we must use a patch for mirage-crypto to be able to launch several parallel TLS sessions. The mirage-crypto-rng-miou is also needed if we don't want any data-race conditions when we generate a random value. The test actually uses only one domain and mirage-crypto-rng.unix according to the current situation of mirage-crypto.

@dinosaure
Copy link
Contributor Author

A new branch dinosaure/ocaml-tls#miou-string exists which is this PR on top of our changes about Cstruct.t -> string but I will propose it when the Cstruct.t -> string PR will be merge. The code does not change to much. Let me know when everything will be good about tls to propose then a rebased version of this PR.

@hannesm
Copy link
Member

hannesm commented Jun 6, 2024

I thought we'll do the cstruct -> string jump in mirage-crypto with a release soon, and then adapt x509 etc. as well, and get this PR merged when we also have tls using string/bytes. Or should this be merged earlier and released (I thought it'd depend on mirage-crypto-rng-miou).

@dinosaure
Copy link
Contributor Author

We should merge this after the Cstruct.t - > string PR. It does not require as is mirage-crypto-rng-miou-unix but it requires fixes about parallelism to work nicely then (so it requires a release of mirage-crypto which includes the Cstruct.t -> string move).

@hannesm
Copy link
Member

hannesm commented Aug 20, 2024

This can now that #497 is merged be rebased on the current main branch.

@hannesm
Copy link
Member

hannesm commented Aug 21, 2024

superseeded by #503

@hannesm hannesm closed this Aug 21, 2024
hannesm added a commit to hannesm/opam-repository that referenced this pull request Aug 21, 2024
CHANGES:

* API breaking change: remove usage of Cstruct.t inside of TLS, use bytes
  and string instead (mirleft/ocaml-tls#497 by @art-w, @hannesm, @dinosaure, @reynir)
  Performance is up to 3x improved (bandwidth), 2x improvement for handshake/s
  on an Intel Core(TM) i7-5600U CPU @ 2.60GHz
* FEATURE: add tls-miou-unix package, which adds miou support for TLS
  (mirleft/ocaml-tls#494 mirleft/ocaml-tls#503 @dinosaure)
* FEATURE: tls-lwt and tls-async: allow TLS over an existing connection
  `Tls_lwt.client_of_channels : Tls.Config.client -> ?host:[`host] Domain_name.t -> Lwt_io.input_channel * Lwt_io.output_channel -> t Lwt.t`
  and
  `Tls_lwt.server_of_channels : Tls.Config.server -> Lwt_io.input_channel * Lwt_io.output_channel -> t Lwt.t`
  (mirleft/ocaml-tls#499 @art-w @MisterDA)
* API breaking changes: revise errors - reduce the polymorphic variant
  in size, align it with RFC specified errors, be in parts more precise
  about errors, in other parts skip data (mirleft/ocaml-tls#505, @hannesm - fixes mirleft/ocaml-tls#491)
  NB: if you relied on a specific error constructor, please open an issue
* Remove unused constructors from Packet.{alert_type, compression_methods,
  client_certificate_type, extension_type} (mirleft/ocaml-tls#505, @hannesm)
  NB: if you relied on specific constructors, please open an issue
* API breaking change: Tls.Config.{server,client} now return a result
  type instead of raising an exception (mirleft/ocaml-tls#502, @hannesm, fixes mirleft/ocaml-tls#411)
* FEATURE: add bench/speed.exe, a benchmark for bandwidth (for different
  ciphersuites) and handshakes (different key exchanges and private keys)
  (mirleft/ocaml-tls#500 @hannesm @dinosaure @reynir)
* BUGFIX: tests/feedback.exe update with TLS 1.3 semantics, run as test
  (mirleft/ocaml-tls#501, @hannesm - reported by @dinosaure)
hannesm added a commit to hannesm/opam-repository that referenced this pull request Aug 22, 2024
CHANGES:

* API breaking change: remove usage of Cstruct.t inside of TLS, use bytes
  and string instead (mirleft/ocaml-tls#497 by @art-w, @hannesm, @dinosaure, @reynir)
  Performance is up to 3x improved (bandwidth), 2x improvement for handshake/s
  on an Intel Core(TM) i7-5600U CPU @ 2.60GHz
* FEATURE: add tls-miou-unix package, which adds miou support for TLS
  (mirleft/ocaml-tls#494 mirleft/ocaml-tls#503 @dinosaure)
* FEATURE: tls-lwt and tls-async: allow TLS over an existing connection
  `Tls_lwt.client_of_channels : Tls.Config.client -> ?host:[`host] Domain_name.t -> Lwt_io.input_channel * Lwt_io.output_channel -> t Lwt.t`
  and
  `Tls_lwt.server_of_channels : Tls.Config.server -> Lwt_io.input_channel * Lwt_io.output_channel -> t Lwt.t`
  (mirleft/ocaml-tls#499 @art-w @MisterDA)
* API breaking changes: revise errors - reduce the polymorphic variant
  in size, align it with RFC specified errors, be in parts more precise
  about errors, in other parts skip data (mirleft/ocaml-tls#505, @hannesm - fixes mirleft/ocaml-tls#491)
  NB: if you relied on a specific error constructor, please open an issue
* Remove unused constructors from Packet.{alert_type, compression_methods,
  client_certificate_type, extension_type} (mirleft/ocaml-tls#505, @hannesm)
  NB: if you relied on specific constructors, please open an issue
* API breaking change: Tls.Config.{server,client} now return a result
  type instead of raising an exception (mirleft/ocaml-tls#502, @hannesm, fixes mirleft/ocaml-tls#411)
* FEATURE: add bench/speed.exe, a benchmark for bandwidth (for different
  ciphersuites) and handshakes (different key exchanges and private keys)
  (mirleft/ocaml-tls#500 @hannesm @dinosaure @reynir)
* BUGFIX: tests/feedback.exe update with TLS 1.3 semantics, run as test
  (mirleft/ocaml-tls#501, @hannesm - reported by @dinosaure)
avsm pushed a commit to avsm/opam-repository that referenced this pull request Sep 5, 2024
CHANGES:

* API breaking change: remove usage of Cstruct.t inside of TLS, use bytes
  and string instead (mirleft/ocaml-tls#497 by @art-w, @hannesm, @dinosaure, @reynir)
  Performance is up to 3x improved (bandwidth), 2x improvement for handshake/s
  on an Intel Core(TM) i7-5600U CPU @ 2.60GHz
* FEATURE: add tls-miou-unix package, which adds miou support for TLS
  (mirleft/ocaml-tls#494 mirleft/ocaml-tls#503 @dinosaure)
* FEATURE: tls-lwt and tls-async: allow TLS over an existing connection
  `Tls_lwt.client_of_channels : Tls.Config.client -> ?host:[`host] Domain_name.t -> Lwt_io.input_channel * Lwt_io.output_channel -> t Lwt.t`
  and
  `Tls_lwt.server_of_channels : Tls.Config.server -> Lwt_io.input_channel * Lwt_io.output_channel -> t Lwt.t`
  (mirleft/ocaml-tls#499 @art-w @MisterDA)
* API breaking changes: revise errors - reduce the polymorphic variant
  in size, align it with RFC specified errors, be in parts more precise
  about errors, in other parts skip data (mirleft/ocaml-tls#505, @hannesm - fixes mirleft/ocaml-tls#491)
  NB: if you relied on a specific error constructor, please open an issue
* Remove unused constructors from Packet.{alert_type, compression_methods,
  client_certificate_type, extension_type} (mirleft/ocaml-tls#505, @hannesm)
  NB: if you relied on specific constructors, please open an issue
* API breaking change: Tls.Config.{server,client} now return a result
  type instead of raising an exception (mirleft/ocaml-tls#502, @hannesm, fixes mirleft/ocaml-tls#411)
* FEATURE: add bench/speed.exe, a benchmark for bandwidth (for different
  ciphersuites) and handshakes (different key exchanges and private keys)
  (mirleft/ocaml-tls#500 @hannesm @dinosaure @reynir)
* BUGFIX: tests/feedback.exe update with TLS 1.3 semantics, run as test
  (mirleft/ocaml-tls#501, @hannesm - reported by @dinosaure)
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants