Implement support for Rustls - v2 by rharish101 · Pull Request #725 · redis-rs/redis-rs

rharish101 · 2022-11-24T19:30:08Z

This PR takes over #601.

I've rebased @LeoRowan's PR onto the latest main. Then, I took @djc's suggestions about the feature flags, and did the following:

Rustls features:
- Rustls support is added under the tls-rustls feature flag.
- Insecure TLS connections are permitted with Rustls using the tls-rustls-insecure feature flag.
- Rustls can be instructed to use Mozilla's root certificates using the tls-rustls-webpki-roots feature flag.
Rustls support for async I/O:
- Tokio Rustls support is added under the tokio-rustls-comp feature flag.
- async-std Rustls support is added under the async-std-rustls-comp feature flag.
Renames for native-tls (deprecation of older features):
- The native-tls feature flag has been renamed from tls to tls-native-tls (for lack of a better naming scheme that's consistent with Rustls).
- The async-std + native-tls feature flag has been renamed from async-std-tls-comp to async-std-native-tls-comp (for consistency).
- Both of the older flags are retained for backwards-compatibility, with deprecation notices in the README and in Cargo.toml.

I've also documented how to use the Rustls feature flags in the README.

Closes #601 and resolves #574.

djc · 2022-12-02T10:46:06Z

Thanks for picking this up! Hope to be able to review this soon.

rharish101 · 2022-12-02T19:55:11Z

I just fixed a bug (a misconfiguration in the x509v3 extensions file for generating TLS certs used in testing) causing the Redis cluster tests to hang forever. All tests now pass on my machine. Could you run the CI again?

0xAlcibiades · 2022-12-04T01:22:42Z

This is exciting. Would like to use here: valorem-labs-inc/quay#41 - just saw this PR, serendipitous, because I'm trying to converge everything on tokio/rustls for sqlx, redis, http, grpc, etc.

arnaudpoullet-dkt · 2022-12-09T14:51:46Z

Nice! I hope to see this merged soon as well to remove openssl completely from our work project 🚀

gd87429 · 2023-02-09T22:12:35Z

Is there an ETA on this merge?

djc · 2023-02-10T11:54:55Z

Nope, sorry. @rharish101 would you mind rebasing this once more?

rharish101 · 2023-02-12T10:51:59Z

@djc Done 👍🏽

stevethedev · 2023-03-02T05:44:47Z

I'm checking in to see if there's any movement on rustls support.

PeterGrace · 2023-03-14T13:27:46Z

Hi @djc , @rharish101 , what can I do to help get this PR merged? I'd like to use this crate in a project that cannot use openssl.

djc · 2023-03-14T14:24:02Z

I'll take a look at it tomorrow -- sorry for the slow response.

rharish101 · 2023-03-14T16:35:20Z

Hi @djc , @rharish101 , what can I do to help get this PR merged? I'd like to use this crate in a project that cannot use openssl.

From my side, I don't think there's anything that needs to be done before code review, apart from rebasing the PR when asked. Let me know if I'm wrong, though.

djc

This is looking pretty good to me -- apologies again for the slow review!

rharish101 · 2023-03-18T08:25:51Z

Just rebased it, fixed tests, and made Rustls use native certs by default. The tests pass on my machine, so if the CI says it's good, feel free to merge it.

rharish101 · 2023-03-19T09:13:53Z

The failing tests seem odd. The error seems like a correctness bug, not a TLS connection issue (as you might expect from this PR). It's also only for one specific configuration, which I cannot reproduce locally (on Arch Linux, with Rust 1.68.0, Redis 7.0.9 and Redis-JSON 2.2.0).

jaymell · 2023-03-19T14:43:18Z

Likely test flakiness, re-running

rharish101 · 2023-03-19T15:58:41Z

Great, they work now. Thanks!

henrysun007 · 2023-03-22T09:55:59Z

Will this PR be merged soon? Eager to use the rustls support.

jaymell · 2023-03-30T04:28:28Z

@rharish101 Do you mind resolving the conflict? 🙏

- Fixed syntax such that it's supported by the MSRV - Renamed "tls" to "tls-native-tls" - Renamed "rustls" to "tls-rustls" - Renamed "rustls-insecure" to "tls-rustls-insecure" - Renamed "async-std-tls-comp" to "async-std-native-tls-comp"

Rustls only accepts x509v3 certs, hence update the code to generate x509v3 certs for the redis server.

The bug also exists in `main`

Also add deprecation notice for older features

rharish101 · 2023-03-30T08:06:12Z

I've rebased the PR onto the latest main 👍🏽

djc · 2023-03-30T13:07:01Z

(Might be worth noting that rustls 0.21 has come out, but there are no matching tokio-rustls/webpki-roots versions quite yet.)

rharish101 · 2023-03-30T14:31:58Z

(Might be worth noting that rustls 0.21 has come out, but there are no matching tokio-rustls/webpki-roots versions quite yet.)

If there are libraries that haven't been updated with the new Rustls, then I think it would be better to merge this PR and update Rustls in a new PR, rather than wait for them and update this PR.

jaymell · 2023-03-30T15:50:15Z

I'm a bit worried that, as is, our test runs will not be hitting native-tls at all with the current configuration, correct? I hate to keep adding more jobs, but I think it's probably worth it in this case.

rharish101 · 2023-03-30T15:59:23Z

Oh yeah, good catch. I'll work on it.

jaymell · 2023-03-30T16:05:05Z

Thanks. And sorry "jobs" was not the right terminology. I was thinking in terms of Github CI for some reason, but obviously I think the solution is to add yet another run to the Makefile 😓

rharish101 · 2023-03-30T19:48:27Z

Test flakiness, perhaps, since the failing test is for RedisJSON, which wasn't changed at all.

jaymell · 2023-03-31T05:44:07Z

Merged. Thanks for bringing this to the finish line!

jonasms · 2023-10-20T21:46:02Z

Folks, thank you for this work AND please update the features section of the docs when you add features!

Add python/core support for configuring client name during connection…

rharish101 marked this pull request as draft November 25, 2022 00:58

rharish101 marked this pull request as ready for review November 25, 2022 01:34

This was referenced Feb 13, 2023

Enable tokio tls redis-rs/redis-cluster-async#14

Merged

Move Redis TLS to Rustls apollographql/router#2606

Closed

djc approved these changes Mar 17, 2023

View reviewed changes

Comment thread redis/Cargo.toml Outdated

jaymell reviewed Mar 25, 2023

View reviewed changes

Comment thread redis/Cargo.toml

jaymell mentioned this pull request Mar 30, 2023

[Feat]: Implement support for rustls #601

Closed

LiaRowan added 6 commits March 30, 2023 10:02

Implement basic rustls support

b0ab329

Implement rustls for tokio runtime

5b80fc3

Clean up duplicate code

d8ceebd

Support most redis protocols, cypher suites, and key exchanges

6cb4e3d

Make rustls default if both rustls and tls enabled

58347fc

Hide rustls insecure clients behind a feature

f9e1bd9

rharish101 added 12 commits March 30, 2023 10:03

Renamed TLS feature flags

03b1eb5

- Fixed syntax such that it's supported by the MSRV - Renamed "tls" to "tls-native-tls" - Renamed "rustls" to "tls-rustls" - Renamed "rustls-insecure" to "tls-rustls-insecure" - Renamed "async-std-tls-comp" to "async-std-native-tls-comp"

Added info about Rustls support in README

6ee057d

Implement rustls for async-std runtime

1ca18d9

Fixed TLS tests for Rustls

2605124

Rustls only accepts x509v3 certs, hence update the code to generate x509v3 certs for the redis server.

Added support for native certs with Rustls

78d2434

Added info about additional rustls flags to README

3b00115

Fixed tokio-*tls-comp feature flags

00690ab

The bug also exists in `main`

Renamed native-tls-specific connection structs

42cb7fb

Fix TLS feature check for cluster client

e0ade79

Use native certs with Rustls by default

b24e345

Fix async std test for rustls

72682a7

Add backward compat with older tls features

06bb1bf

Also add deprecation notice for older features

Restore native-tls tests

485fe72

jaymell merged commit 7eab4cf into redis-rs:main Mar 31, 2023

rharish101 deleted the rustls branch March 31, 2023 05:44

barshaul pushed a commit to barshaul/redis-rs that referenced this pull request Jul 11, 2024

Merge pull request redis-rs#725 from ikolomi/add_clientname_support

e5bd265

Add python/core support for configuring client name during connection…

Conversation

rharish101 commented Nov 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

djc commented Dec 2, 2022

Uh oh!

rharish101 commented Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xAlcibiades commented Dec 4, 2022

Uh oh!

arnaudpoullet-dkt commented Dec 9, 2022

Uh oh!

gd87429 commented Feb 9, 2023

Uh oh!

djc commented Feb 10, 2023

Uh oh!

rharish101 commented Feb 12, 2023

Uh oh!

stevethedev commented Mar 2, 2023

Uh oh!

PeterGrace commented Mar 14, 2023

Uh oh!

djc commented Mar 14, 2023

Uh oh!

rharish101 commented Mar 14, 2023

Uh oh!

djc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rharish101 commented Mar 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rharish101 commented Mar 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaymell commented Mar 19, 2023

Uh oh!

rharish101 commented Mar 19, 2023

Uh oh!

henrysun007 commented Mar 22, 2023

Uh oh!

Uh oh!

jaymell commented Mar 30, 2023

Uh oh!

rharish101 commented Mar 30, 2023

Uh oh!

djc commented Mar 30, 2023

Uh oh!

rharish101 commented Mar 30, 2023

Uh oh!

jaymell commented Mar 30, 2023

Uh oh!

rharish101 commented Mar 30, 2023

Uh oh!

jaymell commented Mar 30, 2023

Uh oh!

rharish101 commented Mar 30, 2023

Uh oh!

jaymell commented Mar 31, 2023

Uh oh!

jonasms commented Oct 20, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

rharish101 commented Nov 24, 2022 •

edited

Loading

rharish101 commented Dec 2, 2022 •

edited

Loading

rharish101 commented Mar 18, 2023 •

edited

Loading

rharish101 commented Mar 19, 2023 •

edited

Loading