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

tests/feedback: fix for TLS 1.3, run as test #501

Merged
merged 2 commits into from
Aug 19, 2024

Conversation

hannesm
Copy link
Member

@hannesm hannesm commented Aug 19, 2024

  • fix feedback to finish the handshake until server and client are ready Previously, it was only waited until client is ready. This in not sufficient for the TLS 1.3 state machine. In TLS < 1.3, the server sends a final "Finished" message, while in TLS 1.3, the client sends the final "Finished" message. In feedback operations, this was dropped, leading to MAC mismatch.
  • tests/server.{key,pem} are symbolic links to ../certificates/server.{key,pem}
  • tests/feedback.exe is run as test now
  • tests/feedback.exe uses cmdliner and logs now

//cc @dinosaure this fixes feedback for the current main branch. I'll soon rebase the #497 PR on top of this to figure out whether this is fixed then there as well.

- fix feedback to finish the handshake until server and client are ready
  Previously, it was only waited until client is ready. This in not sufficient
  for the TLS 1.3 state machine. In TLS < 1.3, the server sends a final
  "Finished" message, while in TLS 1.3, the client sends the final "Finished"
  message. In feedback operations, this was dropped, leading to MAC mismatch.
- tests/server.{key,pem} are symbolic links to ../certificates/server.{key,pem}
- tests/feedback.exe is run as test now
- tests/feedback.exe uses cmdliner and logs now
also clean up the dependencies, and assign async/examples to tls-async
@hannesm
Copy link
Member Author

hannesm commented Aug 19, 2024

it's green, merging

@hannesm hannesm merged commit 1269e9d into mirleft:main Aug 19, 2024
1 check passed
@hannesm hannesm deleted the fix-feedback branch August 19, 2024 15:22
@hannesm hannesm mentioned this pull request Aug 19, 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.

1 participant