Skip to content

feat: optional SSLKEYLOGFILE support #1539

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 1, 2025
Merged

feat: optional SSLKEYLOGFILE support #1539

merged 1 commit into from
Mar 1, 2025

Conversation

crepererum
Copy link
Contributor

Motivation

Using SSLKEYLOGFILE is helpful when you want to intercept TLS traffic for debugging and is generally supported by many libraries and browsers. Also see: https://wiki.wireshark.org/TLS#using-the-pre-master-secret

Solution

Add a use_key_log option to server and client TLS configs that -- when set -- will enable rustls's SSLKEYLOGFILE handling.

@crepererum
Copy link
Contributor Author

Since this is mostly config wiring and relies on the already existing rustls features, does this need a test? If so, where should this test be placed?

@crepererum crepererum mentioned this pull request Sep 28, 2023
Closed
@tottoto
Copy link
Collaborator

tottoto commented Mar 2, 2024

Seems to be related to #893.

@crepererum
Copy link
Contributor Author

The solution there seems to be: just bypass tonic::transport entirely and write 100+ lines of glue-code to wire up all the components (tower, hyper, rustls) yourself. That seems doable, but somewhat defeats the purpose of tonic, i.e. having an easy-to-use client&server toolkit.

tottoto
tottoto approved these changes Jan 23, 2025
View reviewed changes
Copy link
Collaborator

@tottoto tottoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option is useful in terms of providing debuggability, and I think the benefits are worth the additional cost of adding this option. However, @LucioFranco has already rejected adding this feature in #893 and #1102, so we need his approval.

@LucioFranco As it's been a while since your previous review, I ask you if you've changed your mind and made this acceptable. If you accept it, I think it would be good to include this in the next v0.13 release.

@tottoto tottoto requested a review from LucioFranco January 23, 2025 23:12
@tottoto tottoto added this to the 0.13 milestone Jan 23, 2025
@crepererum
Copy link
Contributor Author

will rebase...

@crepererum crepererum force-pushed the crepererum/keylog branch 2 times, most recently from 961f8ac to 7bb05e8 Compare February 26, 2025 10:17
@crepererum
feat: optional SSLKEYLOGFILE support
0c576da 
Add a `use_key_log` option to server and client TLS configs that -- when
set -- will enable rustls's `SSLKEYLOGFILE` handling.

This is helpful when you want to intercept TLS traffic for debugging and
is generally supported by many libraries and browsers. Also see:
https://wiki.wireshark.org/TLS#using-the-pre-master-secret
@LucioFranco LucioFranco added this pull request to the merge queue Mar 1, 2025
Merged via the queue into hyperium:master with commit 77cee9c Mar 1, 2025
18 checks passed
github-merge-queue bot pushed a commit that referenced this pull request May 6, 2025
@LucioFranco @amrhassan
@tottoto @maximevtush @crepererum @dakaizou @jimmycathy @adamcavendish @Suficio @dfawley @bmwill @dbolduc @tamasfe @vigneshs-12 @vigneshwar-sm-11323 @rafaeling @LeonHartley
Merge master -> next (#2267)
2a6f0ab 
* Add From<T> for Response<T> (#1064)

Co-authored-by: tottoto <tottotodev@gmail.com>

* chore: Add getrandom and wasi crate to cargo-deny skip config (#2169)

* chore(examples): Update to rand 0.9 (#2168)

* chore(interop): Replace repeat and take with repeat_n (#2170)

* Update LICENSE (#2167)

* chore(transport): Update url to http crate document (#2173)

* chore: Refactor redundant pattern match (#2174)

* chore(transport): Remove redundant type reconstruct (#2176)

* chore: Update to strum 0.27 (#2180)

* feat: optional `SSLKEYLOGFILE` support (#1539)

Add a `use_key_log` option to server and client TLS configs that -- when
set -- will enable rustls's `SSLKEYLOGFILE` handling.

This is helpful when you want to intercept TLS traffic for debugging and
is generally supported by many libraries and browsers. Also see:
https://wiki.wireshark.org/TLS#using-the-pre-master-secret

* chore: Remove html_root_url (#2184)

* chore: Remove unused mutability (#2183)

* chore: Update generated code (#2222)

* chore: Update cargo-deny config (#2210)

* chore: Add rustix and linux-raw-sys crate to cargo-deny skip config

* chore: Ignore RUSTSEC-2024-0436

* Remove unnecessary mut (#2219)

* remove unnecessary mut

* remove unnecessary mut for health_reporter

---------

Co-authored-by: tottoto <tottotodev@gmail.com>

* chore: fix some comments (#2224)

Signed-off-by: jimmycathy <clonecode@outlook.com>
Co-authored-by: tottoto <tottotodev@gmail.com>

* feat: Allow convert i32 to Code in const context (#2195)

* chore: Disable unused tower feature (#2196)

* chore(router): Remove unnecessary body type converting (#2214)

* chore(server): Use standard library pin macro (#2212)

* chore(build): Use idiomatic api (#2211)

* feat(tonic): Exclude benches-disabled to remove Apache-2.0 resource (#2204)

* chore(ci): Add concurrency group to cancel old ci (#2202)

* chore(test): Use library crate in test (#2201)

* chore: Remove unused rand crate from dev-dependencies (#2198)

* chore: Remove documentation config in manifest (#2193)

* chore(test): Remove unnecessary macro_use (#2200)

* feat: Add proto header to generated code (#2205)

* chore(router): Use upstream poll_ready to implement service (#2215)

* feat(router): Use infallible as error type (#2232)

* chore: Remove unnecessary license file from private crate (#2203)

* chore: update changelog to point to releases (#2235)

* chore: fix changelog header

* chore(server): Remove import sleep and pending function (#2234)

* chore(server): Refactor default http2 keepalive timeout config (#2213)

* chore: Remove unnecessary docs.rs config (#2223)

* feat(transport): add support for uds, unix domain socket (#2218)

* feat(transport): add support for uds, unix domain socket (#2218)

Previously the uds support lives as an example in the `example/src/uds` folder.
Endpoint is refactored to support multiple endpoint types, including Uri and
Uds.

The supported unix domain socket URI follows RFC-3986 which is aligned with
the gRPC naming convention.
- unix:relative_path
- unix:///absolute_path

References:
- https://datatracker.ietf.org/doc/html/rfc3986
- https://github.com/grpc/grpc/blob/master/doc/naming.md

* fix feature flag error

* fix windows build

* fix windows build 2

* fix windows build 3

* fix windows build 4

* fix windows build 5

---------

Co-authored-by: Lucio Franco <luciofranco14@gmail.com>

* Handle stream error correctly (#2199)

Co-authored-by: Lucio Franco <luciofranco14@gmail.com>

* chore: Remove resolved cargo-deny config (#2230)

* Create place for grpc crate and initial contents (#2192)

* Create place for grpc crate and initial contents

* Cargo.toml fixes

* clippy

* clippy 2

* 3

* grpc-web: relax bounds for inner service's response body (#2245)

* grpc-web: relax bounds for inner service's response body

* address feedback

* chore(test): Allow clippy::doc_overindented_list_items lint in generated code (#2246)

* chore(test): Update to rand 0.9 (#2236)

* chore(router): Remove unnecessary type converting (#2237)

* chore(ci): Update to nightly-2025-03-27 on udeps ci (#2242)

* chore(codegen): Update to protox 0.8 (#2254)

* chore(ci): Remove deny job (#2255)

Removing the deny ci job it has become more of a pain to manage than
actually helpful.

* feat: preserve request user-agent (#2250)

Co-authored-by: Lucio Franco <luciofranco14@gmail.com>

* feat(server): Add method to get local addr to TcpIncoming (#2233)

* feat: expose Status as a Response extension (#2145)

Co-authored-by: Lucio Franco <luciofranco14@gmail.com>

* chore(server): Remove unnecessary await service ready (#2258)

* chore: Use symbolic link for license file (#2241)

* chore: Use inline format argument (#2260)

* chore: Add `flake.nix` (#2261)

* chore: Fix interop test certs (#2262)

* chore: Fix interop test certs

* fix bash script:

* fix: tls config overwrite in endpoint (#2252)

* fix: tls config overwrite in endpoint

PR #1866 fixed the breaking change introduced in #1731, but
resets the TLS config without checking if `tls` is set. This
patch resolves the regression and restores expected behaviour.

* fix: cargo fmt whitespace check

---------

Co-authored-by: vigneshwar.sm <vigneshwar.sm@zohocorp.com>
Co-authored-by: Lucio Franco <luciofranco14@gmail.com>

* chore(tonic-bench): Fix failing bench (#2207)

Co-authored-by: Lucio Franco <luciofranco14@gmail.com>

* feat: expose creation of HealthService and HealthReporter (#2251)

* Expose creation of HealthService and HealthReporter

* add default impl for HealthReporter

* [spr] initial version (#2264)

Created using spr 1.3.6-beta.1

* Revert "[spr] initial version (#2264)" (#2265)

* chore: Prepare `v0.13.1` release



Reviewers: 

Pull Request: #2266

---------

Signed-off-by: jimmycathy <clonecode@outlook.com>
Co-authored-by: Amr Hassan <amr.hassan@gmail.com>
Co-authored-by: tottoto <tottotodev@gmail.com>
Co-authored-by: Maxim Evtush <154841002+maximevtush@users.noreply.github.com>
Co-authored-by: Marco Neumann <marco@crepererum.net>
Co-authored-by: DAKAI, TZOU <dakaizou@gmail.com>
Co-authored-by: jimmycathy <clonecode@outlook.com>
Co-authored-by: Adam Basfop Cavendish <adamcavendish@users.noreply.github.com>
Co-authored-by: Jakub Łabor <jacob.labor@gmail.com>
Co-authored-by: Doug Fawley <dfawley@google.com>
Co-authored-by: Brandon Williams <brandon@mystenlabs.com>
Co-authored-by: Darren Bolduc <dbolduc@google.com>
Co-authored-by: Ferenc Tamás <tamasf97@outlook.com>
Co-authored-by: Vigneshwar S <80331703+vigneshs-12@users.noreply.github.com>
Co-authored-by: vigneshwar.sm <vigneshwar.sm@zohocorp.com>
Co-authored-by: Rafael RL <Rafael.Ruiz-Lucena@etas.com>
Co-authored-by: Leon Hartley <lhartley97@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LucioFranco LucioFranco LucioFranco approved these changes

@tottoto tottoto tottoto approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.13
Development

Successfully merging this pull request may close these issues.
3 participants
@crepererum @tottoto @LucioFranco