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

Using PLAIN is broken in ruby 3.1.0 #52

Closed
chdiza opened this issue Dec 28, 2021 · 5 comments
Closed

Using PLAIN is broken in ruby 3.1.0 #52

chdiza opened this issue Dec 28, 2021 · 5 comments
Labels
SASL 🔒 Authentication and authentication mechanisms

Comments

@chdiza
Copy link

chdiza commented Dec 28, 2021

For years, up to and including ruby 2.7.5, I've been able to authenticate into some IMAP servers (even gmail) like so:

server = Net::IMAP.new(HOST, :port=>993, :ssl=>true)
server.authenticate('PLAIN', USERNAME, PASSWORD)

I've just upgraded to ruby 3.1.0 (from 2.7.5), and now this doesn't work on any of those servers. Instead I get:

lib/ruby/gems/3.1.0/gems/net-imap-0.2.2/lib/net/imap.rb:1252:in `get_tagged_response': Authentication failed. (Net::IMAP::NoResponseError)

If I switch back to ruby 2.7.5 it works again.

I'm not knowledgeable enough to understand why this stopped working.

@chdiza
Copy link
Author

chdiza commented Jan 3, 2022

The problem also occurs in ruby 3.0.3, so I did some more digging. Bisection reveals that this is the commit that broke authenticating with PLAIN:

23f241b081d94b6385e24f550a24e65a66a0d46d is the first bad commit
commit 23f241b081d94b6385e24f550a24e65a66a0d46d
Author: nicholas a. evans <[email protected]>
Date:   Tue Apr 27 16:33:27 2021 -0400

    Move each authenticator to its own file
    
    Also updates rdoc with SASL specifications and deprecations.  Of these
    four, only `PLAIN` isn't deprecated!
    
    +@@authenticators+ was changed to a class instance var
    +@authenticators+.  No one should have been using the class variable
    directly, so that should be fine.

@chdiza
Copy link
Author

chdiza commented Jan 3, 2022

Version 0.1.1 of the net-imap gem is fine. But when I install a new ruby, gem update brings the buggy version of the gem into play.

@shugo shugo closed this as completed in ed4c3b2 Jan 6, 2022
@shugo
Copy link
Member

shugo commented Jan 6, 2022

@chdiza Thanks for your report. I've just released net-imap-0.2.3. Please try it.

@chdiza
Copy link
Author

chdiza commented Jan 6, 2022

Seems to work, thanks!!

@nevans
Copy link
Collaborator

nevans commented Jan 6, 2022

Yikes. Sorry about that. 😳 Thanks, @shugo!

nevans added a commit to nevans/net-imap that referenced this issue Jul 16, 2022
Mark obolete SASL mechanisms as deprecated (fixes rubyGH-55):
* Warn every time a deprecated mechanism is used.
* Warnings can be disabled with `warn_deprecation: false`
* delay loading stdgem dependencies until `#initialize`.  Fixes rubyGH-56.
* This is a backwards-compatible alternative to the approach in rubyGH-58
  (don't require and add the deprecated authenticators automatically).
  We can use that incompatible approach in a later version.

Additionally:
* Adds basic tests for every authenticator (to avoid another rubyGH-52!)
* Fixes a frozen string bug in DigestMD5Authenticator.
* By making these optional, there's no reason to require the `digest` or
  `strscan` gems anymore; fixes rubyGH-56.

The DIGEST-MD5 bug was originally reported, tested, and fixed by
@singpolyma here: nevans/net-sasl#3.

Co-authored-by: Stephen Paul Weber <[email protected]>
nevans added a commit that referenced this issue Jul 16, 2022
Mark obolete SASL mechanisms as deprecated (fixes GH-55):
* This is a backwards-compatible alternative to the approach in GH-58
  (don't require and add the deprecated authenticators automatically).
  We can use that incompatible approach in a later version.
* Warn every time a deprecated mechanism is used.
* Warnings can be disabled with `warn_deprecation: false`
* delay loading stdgem dependencies until `#initialize`.  Fixes GH-56.

Additionally:
* Adds basic tests for every authenticator (to avoid another GH-52!)
* Fixes a frozen string bug in DigestMD5Authenticator.
* By making these optional, there's no reason to require the `digest` or
  `strscan` gems anymore; fixes GH-56.

The DIGEST-MD5 bug was originally reported, tested, and fixed by
@singpolyma here: nevans/net-sasl#3.

Co-authored-by: Stephen Paul Weber <[email protected]>
nevans added a commit that referenced this issue Jul 16, 2022
Mark obolete SASL mechanisms as deprecated (fixes GH-55):
* This is a backwards-compatible alternative to the approach in GH-58
  (don't require and add the deprecated authenticators automatically).
  We can use that incompatible approach in a later version.
* Warn every time a deprecated mechanism is used.
* Warnings can be disabled with `warn_deprecation: false`
* Fixes GH-56: delay loading standard gem dependencies until
  `#initialize`, and convert the gems to development dependencies.

Additionally:
* Adds basic tests for every authenticator (to avoid another GH-52!)
* Fixes a frozen string bug in DigestMD5Authenticator.
* Fixes constant resolution for exceptions in DigestMD5Authenticator.
* Can register an authenticator type that responds to #call (instead of
  #new).  I was originally going to register deprecated authenticators
  with a Proc that required the file and issued a warning, but I decided
  to put everything into the initializer instead.  `#authenticator`
  needed to be updated to safely delegate all args, and I left this in.

The DIGEST-MD5 bug was originally reported, tested, and fixed by
@singpolyma here: nevans/net-sasl#3.

Co-authored-by: Stephen Paul Weber <[email protected]>
@nevans nevans added the SASL 🔒 Authentication and authentication mechanisms label Feb 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SASL 🔒 Authentication and authentication mechanisms
Development

No branches or pull requests

3 participants