-
Notifications
You must be signed in to change notification settings - Fork 29
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
[🚧 WIP] SASL refactoring and new mechanisms #78
base: master
Are you sure you want to change the base?
Conversation
n.b this PR is currently based on #70, #71, #72, #73, #74, #75, and #76. I can rebase part of all of this on master, but I think those will be merged before this is. Also, it's currently failing 2.6, because I used some numbered parameters in a few places. As discussed on #68, we can remove support for 2.6. |
966416d
to
fa98ee7
Compare
73e5c8b
to
69fdec7
Compare
@nevans: Good job! |
87a0365
to
26b4d74
Compare
26b4d74
to
8818e79
Compare
The documentation on these methods is meant to complement a new SASL::Authenticator base class and new documentation on each of the individual authenticator classes. See #78 and #82. That base class is added in another PR (#78), but the documentation for these methods can be updated without it. Also, somehow I misremembered `LOGINDISABLED`: it only applies to `LOGIN`, not `AUTHENTICATE`! This fixes that error.
@nevans: Happy New Year! Little question, have you a timeline? |
be02ca0
to
8ee0bfa
Compare
078ea33
to
fd56ca3
Compare
43180a5
to
1a8deb3
Compare
@nevans: I think that you can remove CRAM-MD5, DIGEST-MD5, LOGIN from all. |
3076a03
to
0af0c6d
Compare
52c19d6
to
de7fea5
Compare
efcfcf4
to
9222f15
Compare
9a837d7
to
b398b46
Compare
Yes, DIGEST-MD5 is deprecated! But that also means that it was lower risk for experimenting with other SASL changes. It's complexity vs most other mechanisms makes it a good test-bed for the completeness of net-imap's SASL implementation: e.g: it demonstrated that we were missing features such as `done?`, demonstrates the utility of using callbacks for attributes such as `realm` (the user might select from a server-provided list), it shows that `service` cannot be hard-coded to `imap` and must be provided by the client, and requires other attributes that should be provided by the client such as `host`, `port` (also used by `OAUTHBEARER`). I improved the existing authenticator in several ways: * ✨ User can configure `realm`, `host`, `service_name`, `service`. This allows a correct "digest-uri" for non-IMAP clients. * 🔒 Use SecureRandom for cnonce (not Time.now + insecure PRNG!) * ✨ Default `qop=auth` (as in RFC) * ✨ Enforce requirements for `sparam` keys (required and no-multiples). * ♻️ Refactor toward the style used in the new ScramAuthenticator. However... it's still deprecated, so don't use it! 🙃
The question is: do we even want to this?
NOTE: This PR started as a big rough-draft for many of the other PRs listed below. Rather than close the PR and create a new tracking issue, I've been keeping the branch around as a set of experimental implementations for some of the TODO list items, while cherry-picking parts of it into their own PRs when they are ready.
EXTERNAL
ANONYMOUS
OAUTHBEARER
SCRAM-SHA-1
,SCRAM-SHA-256
SASL-IR
(included inIMAP4rev2
) #34SCRAM-*
and to supportnet-smtp
, which already followed the RFC on this.secret
alias (forpassword
,oauth2_token
, etc) to relevant SASL mechanisms #195AuthenticationSuccess (subclass of TaggedResponse)(subclass of NoResponseError)(subclass of BadResponseError)This PR originally had an implementation of this, but it was over-complicated and removed to simplify the API for the v0.4.0 release.
process
state machine, e.g:mongo
does.net-smtp
API and could greatly simplify the mechanism implementation. It is a little bit trickier to adapt this style to multiple threads and Net::IMAP's receiver loop.net-imap
SASL implementation with other gemsShould we create a shared net-sasl gem? #23.
net-smtp
,net-pop
,net-ldap
,mongo
, etc.net-smtp
: ruby/net-smtp@master...nevans:net-smtp:net-imap-sasl.net-pop
net-ldap
,mongo
, and possibly others...