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

Switch from rustls to native-tls #3

Merged
merged 1 commit into from
Dec 13, 2019
Merged

Switch from rustls to native-tls #3

merged 1 commit into from
Dec 13, 2019

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Dec 13, 2019

No description provided.

@Darkspirit
Copy link

Darkspirit commented Dec 13, 2019

Removing Rustls or unexpectedly changing the default would cause breakage. Please consider not damaging an ecosystem just because your chat system is fragile to a few misconfigured email servers using 1024 bit RSA. :-/ Please make this optional and at least inform your users that their mail password is transmitted over a connection that doesn't meet security standards so that they can inform their providers on their own.

@link2xt
Copy link
Collaborator Author

link2xt commented Dec 13, 2019

@Darkspirit We are happy to disable insecure ciphersuites by default, but simply telling users to switch to plaintext because the only provider available to them is using 1024-bit RSA is not an option. If rustls provides 1024-bit RSA support as an option that could be enabled via .danger(), we would consider using it, but right now we want to make a release ASAP and can't wait for the whole chain of changes in ring, webpki and rusttls to happen.

@Darkspirit
Copy link

Oh, that's reasonable! I did not comprehend that you've already shipped this regression. But could you merge this as optional feature, please?

@link2xt
Copy link
Collaborator Author

link2xt commented Dec 13, 2019

@Darkspirit It's getting a bit off-topic, but to clarify the situation: we have a stable version released on Google Play that works for users with 1024-bit RSA server, a beta version with native_tls that also works, and a nightly based on rusttls that doesn't allow them to connect. We can't even ship rusttls-based version to the beta channel as it will make the client unusable for a lot of users at once automatically, forcing them to either stop using the client or switch to plaintext configuration.

@link2xt
Copy link
Collaborator Author

link2xt commented Dec 13, 2019

But could you merge this as optional feature, please?

async-tls and async-native-tls have slightly different interfaces (for example, see the return value of connect). For async-imap I used conditional compilation, but it breaks doctests and has other problems like duplicate code because cfg! doesn't work with different return values in different branches of if. So I decided not to go this way for async-smtp. I would like to unify async-tls and async-native-tls interfaces first, hopefully merging them into a single repo, with trait-based interfaces. But that also requires coordinating things with async-tls developers, so it is postponed until we have a release.

@dignifiedquire dignifiedquire merged commit 8e61f43 into async-email:master Dec 13, 2019
@dignifiedquire
Copy link
Member

tracking #4 to bring back rustls as an option

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.

3 participants