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

🔒 SASL DIGEST-MD5: realm, host, service_name, etc #284

Merged
merged 1 commit into from
Jun 30, 2024

Conversation

nevans
Copy link
Collaborator

@nevans nevans commented May 19, 2024

(This work in this commit was done about 18 months ago, as part of #78. In fact, the work done in this PR was used to guide the style of all of the new authenticators. I wasn't sure whether it was worth submitting as a PR, but for the sake of completeness: here it is.)


Yes, DIGEST-MD5 is deprecated! But that also means that it was lower risk for experimenting with other SASL changes. Its complexity vs most other mechanisms made it a good test-bed for the completeness of net-imap's SASL implementation. For example:

  • It demonstrated that we were missing features such as done?.
    Added in 🔒 Verify SASL authentication has completed #179.
  • It demonstrates the utility of using callbacks for attributes such as realm (the user might select from a server-provided list).
    Please note: the initial work I did to support attribute callbacks was reverted, to simplify the SASL re-write. It could still be a useful feature for this and other mechanisms.
  • It shows that service should not be hard-coded to imap, and should be provided by the client (or the protocol adapter).
    Please note: Although the current (experimental) client adapters do have a #service method, it is not used by the (experimental) AuthenticationExchange yet.
  • It 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:

  • ✨ Add realm, host, service_name, service attributes. This allows non-IMAP clients to construct the correct digest-uri.
  • 🔒 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! 🙃

@nevans nevans added the SASL 🔒 Authentication and authentication mechanisms label May 19, 2024
@nevans nevans added this to the v0.5 milestone Jun 13, 2024
@nevans nevans force-pushed the sasl/digest-md5-updates-lol branch 2 times, most recently from 5dc3f1d to f675169 Compare June 22, 2024 16:08
@nevans nevans force-pushed the sasl/digest-md5-updates-lol branch 4 times, most recently from f2b5ebb to 27e21e8 Compare June 30, 2024 17:30
Yes, DIGEST-MD5 is deprecated!  But that also means that it was lower
risk for experimenting with other SASL changes.  Its complexity vs most
other mechanisms made it a good test-bed for the completeness of
net-imap's SASL implementation.  For example:

* It demonstrated that we were missing features such as `done?`.
* It demonstrates the utility of using callbacks for attributes such as
  `realm` (the user might select from a server-provided list).
  _Please note: the initial work I did to support attribute callbacks
  was reverted, to simplify the big SASL re-write.  It could still be a
  useful feature for this and other mechanisms._
* It shows that `service` should not be hard-coded to `imap`, and should
  be provided by the client (or the protocol adapter).
  _Please note: Although the current (experimental) client adapters _do_
  have a `#service` method, it is not used by the (experimental)
  AuthenticationExchange yet._
* It 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:
* ✨ Add `realm`, `host`, `service_name`, `service` attributes.  This
  allows non-IMAP clients to construct the correct `digest-uri`.
* 🔒 Use SecureRandom for cnonce (not Time.now + insecure PRNG!)
* ✨ Default `qop=auth` (as in RFC)
* ✨ Enforce requirements for `sparam` keys (required and no-multiples).
* ♻️  Various other minor refactorings.

However... it's still deprecated, so don't use if you don't need to! 🙃
@nevans nevans force-pushed the sasl/digest-md5-updates-lol branch from 27e21e8 to bf34d6f Compare June 30, 2024 17:38
@nevans nevans merged commit 725a10e into ruby:master Jun 30, 2024
9 checks passed
@nevans nevans deleted the sasl/digest-md5-updates-lol branch June 30, 2024 17:39
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

Successfully merging this pull request may close these issues.

1 participant