Skip to content

allow option for enabling the SSLKEYLOGFILE environment variable#4254

Open
2ndDerivative wants to merge 3 commits intolaunchbadge:mainfrom
2ndDerivative:rustls-tls-keylog
Open

allow option for enabling the SSLKEYLOGFILE environment variable#4254
2ndDerivative wants to merge 3 commits intolaunchbadge:mainfrom
2ndDerivative:rustls-tls-keylog

Conversation

@2ndDerivative
Copy link
Copy Markdown
Contributor

Is this a breaking change?

I think only for the core crate

I wanted to observe some database traffic which contained encrypted sqlx traffic and since there was no dedicated way to include a rustls ClientConfig in the connection, this would greatly help my use case.

@abonander
Copy link
Copy Markdown
Collaborator

I agree with the Postgres maintainers that this is a high-risk option to add because anything could theoretically inject this environment variable and start sniffing TLS traffic: https://www.postgresql.org/message-id/1774813.1736385450%40sss.pgh.pa.us

Admittedly, you'd already have to have a compromised environment, but this could easily be chained into a much bigger attack. The Postgres maintainers decided to make it a connection parameter instead: https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-SSLKEYLOGFILE

It's worth noting there is an open PR from Luca Palmieri to allow a prebuilt rustls::ClientConfig (though I just realized he's been waiting for feedback this whole time): #4051

@abonander
Copy link
Copy Markdown
Collaborator

We could also add an option to log TLS keys, e.g. at DEBUG level (I think we need to phase out dynamic log/span levels as that's just not idiomatic), which could be more or less secure depending on your setup.

@2ndDerivative
Copy link
Copy Markdown
Contributor Author

For that we might as well just expose the trait that Rustls uses (Keylog) but I wanted to avoid having any outside API referencing rustls types for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants