From 9c8fbdb44e06e949add19bbb47f67a18eba29b00 Mon Sep 17 00:00:00 2001 From: nick evans Date: Wed, 27 Sep 2023 16:24:55 -0400 Subject: [PATCH 1/6] =?UTF-8?q?=F0=9F=94=A5=20Remove=20`SASL.initial=5Fres?= =?UTF-8?q?ponse=3F`,=20`SASL.done=3F`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since they haven't been in any release yet, `SASL.initial_response?` and `SASL.done?` have been removed without deprecation. The logic has been moved directly into `Net::IMAP#authenticate` (for now). Implementing `#initial_response?` is still optional. But, to simplify the tests, `#initial_response? => false` was added to all of the deprecated SASL mechanisms. --- lib/net/imap.rb | 5 +++-- lib/net/imap/sasl.rb | 18 ------------------ lib/net/imap/sasl/cram_md5_authenticator.rb | 2 ++ lib/net/imap/sasl/digest_md5_authenticator.rb | 2 ++ lib/net/imap/sasl/login_authenticator.rb | 2 ++ test/net/imap/test_imap_authenticators.rb | 8 +++----- 6 files changed, 12 insertions(+), 25 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 1539dcbc..c5e053f0 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -1251,7 +1251,8 @@ def authenticate(mechanism, *creds, sasl_ir: true, **props, &callback) authenticator = SASL.authenticator(mechanism, *creds, **props, &callback) cmdargs = ["AUTHENTICATE", mechanism] if sasl_ir && capable?("SASL-IR") && auth_capable?(mechanism) && - SASL.initial_response?(authenticator) + authenticator.respond_to?(:initial_response?) && + authenticator.initial_response? response = authenticator.process(nil) cmdargs << (response.empty? ? "=" : [response].pack("m0")) end @@ -1263,7 +1264,7 @@ def authenticate(mechanism, *creds, sasl_ir: true, **props, &callback) put_string(response + CRLF) end end - unless SASL.done?(authenticator) + if authenticator.respond_to?(:done?) && !authenticator.done? logout! raise SASL::AuthenticationFailed, "authentication ended prematurely" end diff --git a/lib/net/imap/sasl.rb b/lib/net/imap/sasl.rb index 5f01371b..4af1c4be 100644 --- a/lib/net/imap/sasl.rb +++ b/lib/net/imap/sasl.rb @@ -158,24 +158,6 @@ def saslprep(string, **opts) Net::IMAP::StringPrep::SASLprep.saslprep(string, **opts) end - # Returns whether +authenticator+ is client-first and supports sending an - # "initial response". - def initial_response?(authenticator) - authenticator.respond_to?(:initial_response?) && - authenticator.initial_response? - end - - # Returns whether +authenticator+ considers the authentication exchange to - # be complete. - # - # The authentication should not succeed if this returns false, but - # returning true does *not* indicate success. Authentication succeeds - # when this method returns true and the server responds with a - # protocol-specific success. - def done?(authenticator) - !authenticator.respond_to?(:done?) || authenticator.done? - end - end end end diff --git a/lib/net/imap/sasl/cram_md5_authenticator.rb b/lib/net/imap/sasl/cram_md5_authenticator.rb index 6a6242eb..3aac7b35 100644 --- a/lib/net/imap/sasl/cram_md5_authenticator.rb +++ b/lib/net/imap/sasl/cram_md5_authenticator.rb @@ -24,6 +24,8 @@ def initialize(user, password, warn_deprecation: true, **_ignored) @done = false end + def initial_response?; false end + def process(challenge) digest = hmac_md5(challenge, @password) return @user + " " + digest diff --git a/lib/net/imap/sasl/digest_md5_authenticator.rb b/lib/net/imap/sasl/digest_md5_authenticator.rb index d77afdad..dcc6fc59 100644 --- a/lib/net/imap/sasl/digest_md5_authenticator.rb +++ b/lib/net/imap/sasl/digest_md5_authenticator.rb @@ -73,6 +73,8 @@ def initialize(user = nil, pass = nil, authz = nil, @nc, @stage = {}, STAGE_ONE end + def initial_response?; false end + # Responds to server challenge in two stages. def process(challenge) case @stage diff --git a/lib/net/imap/sasl/login_authenticator.rb b/lib/net/imap/sasl/login_authenticator.rb index f13cac6a..5132a09e 100644 --- a/lib/net/imap/sasl/login_authenticator.rb +++ b/lib/net/imap/sasl/login_authenticator.rb @@ -32,6 +32,8 @@ def initialize(user, password, warn_deprecation: true, **_ignored) @state = STATE_USER end + def initial_response?; false end + def process(data) case @state when STATE_USER diff --git a/test/net/imap/test_imap_authenticators.rb b/test/net/imap/test_imap_authenticators.rb index c0c1e299..38773a29 100644 --- a/test/net/imap/test_imap_authenticators.rb +++ b/test/net/imap/test_imap_authenticators.rb @@ -41,7 +41,6 @@ def test_plain_authenticator_matches_mechanism def test_plain_supports_initial_response assert plain("foo", "bar").initial_response? - assert Net::IMAP::SASL.initial_response?(plain("foo", "bar")) end def test_plain_response @@ -194,7 +193,6 @@ def test_xoauth2_kwargs def test_xoauth2_supports_initial_response assert xoauth2("foo", "bar").initial_response? - assert Net::IMAP::SASL.initial_response?(xoauth2("foo", "bar")) end # ---------------------- @@ -276,7 +274,7 @@ def test_login_authenticator_matches_mechanism end def test_login_does_not_support_initial_response - refute Net::IMAP::SASL.initial_response?(login("foo", "bar")) + refute login("foo", "bar").initial_response? end def test_login_authenticator_deprecated @@ -306,7 +304,7 @@ def test_cram_md5_authenticator_matches_mechanism end def test_cram_md5_does_not_support_initial_response - refute Net::IMAP::SASL.initial_response?(cram_md5("foo", "bar")) + refute cram_md5("foo", "bar").initial_response? end def test_cram_md5_authenticator_deprecated @@ -343,7 +341,7 @@ def test_digest_md5_authenticator_deprecated end def test_digest_md5_does_not_support_initial_response - refute Net::IMAP::SASL.initial_response?(digest_md5("foo", "bar")) + refute digest_md5("foo", "bar").initial_response? end def test_digest_md5_authenticator From 74e468737698d4b0c35476fbb772e83672de84dd Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 29 Sep 2023 09:32:41 -0400 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=92=A5=20Add=20`SASL::Authenticators`?= =?UTF-8?q?=20defaults,=20by=20default?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reversed the `use_defaults` default, and added another option for `use_deprecated`. --- lib/net/imap/sasl.rb | 4 +--- lib/net/imap/sasl/authenticators.rb | 33 ++++++++++++++--------------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/lib/net/imap/sasl.rb b/lib/net/imap/sasl.rb index 4af1c4be..353bef62 100644 --- a/lib/net/imap/sasl.rb +++ b/lib/net/imap/sasl.rb @@ -141,9 +141,7 @@ module SASL autoload :LoginAuthenticator, "#{sasl_dir}/login_authenticator" # Returns the default global SASL::Authenticators instance. - def self.authenticators - @authenticators ||= Authenticators.new(use_defaults: true) - end + def self.authenticators; @authenticators ||= Authenticators.new end # Delegates to ::authenticators. See Authenticators#authenticator. def self.authenticator(...) authenticators.authenticator(...) end diff --git a/lib/net/imap/sasl/authenticators.rb b/lib/net/imap/sasl/authenticators.rb index 88d5feb5..7536630b 100644 --- a/lib/net/imap/sasl/authenticators.rb +++ b/lib/net/imap/sasl/authenticators.rb @@ -26,24 +26,23 @@ class Authenticators # This class is usually not instantiated directly. Use SASL.authenticators # to reuse the default global registry. # - # By default, the registry will be empty--without any registrations. When - # +add_defaults+ is +true+, authenticators for all standard mechanisms will - # be registered. - # - def initialize(use_defaults: false) + # When +use_defaults+ is +false+, the registry will start empty. When + # +use_deprecated+ is +false+, deprecated authenticators will not be + # included with the defaults. + def initialize(use_defaults: true, use_deprecated: true) @authenticators = {} - if use_defaults - add_authenticator "Anonymous" - add_authenticator "External" - add_authenticator "OAuthBearer" - add_authenticator "Plain" - add_authenticator "Scram-SHA-1" - add_authenticator "Scram-SHA-256" - add_authenticator "XOAuth2" - add_authenticator "Login" # deprecated - add_authenticator "Cram-MD5" # deprecated - add_authenticator "Digest-MD5" # deprecated - end + return unless use_defaults + add_authenticator "Anonymous" + add_authenticator "External" + add_authenticator "OAuthBearer" + add_authenticator "Plain" + add_authenticator "Scram-SHA-1" + add_authenticator "Scram-SHA-256" + add_authenticator "XOAuth2" + return unless use_deprecated + add_authenticator "Login" # deprecated + add_authenticator "Cram-MD5" # deprecated + add_authenticator "Digest-MD5" # deprecated end # Returns the names of all registered SASL mechanisms. From 80c6e961211367b60f160997b9c165b1a12772b5 Mon Sep 17 00:00:00 2001 From: nick evans Date: Thu, 28 Sep 2023 12:37:41 -0400 Subject: [PATCH 3/6] =?UTF-8?q?=F0=9F=A5=85=20Add=20`SASL::AuthenticationI?= =?UTF-8?q?ncomplete`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap.rb | 2 +- lib/net/imap/sasl.rb | 12 ++++++++++++ test/net/imap/test_imap.rb | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index c5e053f0..e3d6b0dd 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -1266,7 +1266,7 @@ def authenticate(mechanism, *creds, sasl_ir: true, **props, &callback) end if authenticator.respond_to?(:done?) && !authenticator.done? logout! - raise SASL::AuthenticationFailed, "authentication ended prematurely" + raise SASL::AuthenticationIncomplete, result end @capabilities = capabilities_from_resp_code result result diff --git a/lib/net/imap/sasl.rb b/lib/net/imap/sasl.rb index 353bef62..62185ba8 100644 --- a/lib/net/imap/sasl.rb +++ b/lib/net/imap/sasl.rb @@ -114,6 +114,18 @@ module SASL # messages has not passed integrity checks. AuthenticationFailed = Class.new(Error) + # Indicates that authentication cannot proceed because one of the server's + # ended authentication prematurely. + class AuthenticationIncomplete < AuthenticationFailed + # The success response from the server + attr_reader :response + + def initialize(response, message = "authentication ended prematurely") + super(message) + @response = response + end + end + # autoloading to avoid loading all of the regexps when they aren't used. sasl_stringprep_rb = File.expand_path("sasl/stringprep", __dir__) autoload :StringPrep, sasl_stringprep_rb diff --git a/test/net/imap/test_imap.rb b/test/net/imap/test_imap.rb index 81ebc269..03700447 100644 --- a/test/net/imap/test_imap.rb +++ b/test/net/imap/test_imap.rb @@ -966,7 +966,7 @@ def test_id server.state.authenticate(server.config.user) cmd.done_ok end - assert_raise(Net::IMAP::SASL::AuthenticationFailed) do + assert_raise(Net::IMAP::SASL::AuthenticationIncomplete) do imap.authenticate("DIGEST-MD5", "test_user", "test-password", warn_deprecation: false) end From 3bb21086c35979ebe75f61a46bea7a0ce594b8b1 Mon Sep 17 00:00:00 2001 From: nick evans Date: Thu, 28 Sep 2023 13:10:18 -0400 Subject: [PATCH 4/6] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Extract=20`send=5Fcomm?= =?UTF-8?q?and=5Fwith=5Fcontinuations`=20method?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Abstracts running a command with a continuation handler. In the base specifications (both RFC-3501 and RFC-9051), the only command to need this is `AUTHENTICATE`. The only other continuations are for sending literals, which are simple command arguments. Are there any IMAP extensions that use continuations in this way? Even with only one command, this still makes that code a little more readable. More importantly, it also simplifies the creation of a generic SASL protocol adapter, for use by `net-sasl`, `net-pop`, and others. --- lib/net/imap.rb | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index e3d6b0dd..69fdc3f1 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -1256,14 +1256,11 @@ def authenticate(mechanism, *creds, sasl_ir: true, **props, &callback) response = authenticator.process(nil) cmdargs << (response.empty? ? "=" : [response].pack("m0")) end - result = send_command(*cmdargs) do |resp| - if resp.instance_of?(ContinuationRequest) - challenge = resp.data.text.unpack1("m") - response = authenticator.process(challenge) - response = [response].pack("m0") - put_string(response + CRLF) - end - end + result = send_command_with_continuations(*cmdargs) {|data| + challenge = data.unpack1("m") + response = authenticator.process challenge + [response].pack("m0") + } if authenticator.respond_to?(:done?) && !authenticator.done? logout! raise SASL::AuthenticationIncomplete, result @@ -2571,6 +2568,18 @@ def capabilities_from_resp_code(resp) ############################# + # Calls send_command, yielding the text of each ContinuationRequest and + # responding with each block result. Returns TaggedResponse. Raises + # NoResponseError or BadResponseError. + def send_command_with_continuations(cmd, *args) + send_command(cmd, *args) do |server_response| + if server_response.instance_of?(ContinuationRequest) + client_response = yield server_response.data.text + put_string(client_response + CRLF) + end + end + end + def send_command(cmd, *args, &block) synchronize do args.each do |i| From 7c6938be04a108d3a2132c9b07ef26077000ef25 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 29 Sep 2023 09:24:47 -0400 Subject: [PATCH 5/6] =?UTF-8?q?=E2=9C=A8=20Add=20`SASL::Authenticators#new?= =?UTF-8?q?`=20alias?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap/sasl/authenticators.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/net/imap/sasl/authenticators.rb b/lib/net/imap/sasl/authenticators.rb index 7536630b..02f322f8 100644 --- a/lib/net/imap/sasl/authenticators.rb +++ b/lib/net/imap/sasl/authenticators.rb @@ -99,6 +99,7 @@ def authenticator(mechanism, ...) end auth.respond_to?(:new) ? auth.new(...) : auth.call(...) end + alias new authenticator end From 8a1ac02166a8ee4137171e8f612b2c7a0c1c95ce Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 29 Sep 2023 09:25:23 -0400 Subject: [PATCH 6/6] =?UTF-8?q?=E2=9C=A8=20Add=20`SASL::Authenticators#rem?= =?UTF-8?q?ove=5Fauthenticator`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap/sasl/authenticators.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/net/imap/sasl/authenticators.rb b/lib/net/imap/sasl/authenticators.rb index 02f322f8..e4c46250 100644 --- a/lib/net/imap/sasl/authenticators.rb +++ b/lib/net/imap/sasl/authenticators.rb @@ -77,6 +77,12 @@ def add_authenticator(name, authenticator = nil) @authenticators[key] = authenticator end + # Removes the authenticator registered for +name+ + def remove_authenticator(name) + key = name.upcase.to_sym + @authenticators.delete(key) + end + # :call-seq: # authenticator(mechanism, ...) -> auth_session #