From 212ef7a4ea01309e66cf0019cb657eb0864e0c71 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 4 Nov 2022 22:09:46 -0400 Subject: [PATCH 1/2] =?UTF-8?q?=E2=99=BB=EF=B8=8F=F0=9F=92=A9=20SASL=20DIG?= =?UTF-8?q?EST-MD5=20detritus?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These are further changes I made while working on the SASL authenticators that never got merged... However... it's still deprecated, so don't use it! 🙃 --- lib/net/imap/sasl/digest_md5_authenticator.rb | 112 +++++++++++------- 1 file changed, 68 insertions(+), 44 deletions(-) diff --git a/lib/net/imap/sasl/digest_md5_authenticator.rb b/lib/net/imap/sasl/digest_md5_authenticator.rb index 13a2fd35..1686ed0c 100644 --- a/lib/net/imap/sasl/digest_md5_authenticator.rb +++ b/lib/net/imap/sasl/digest_md5_authenticator.rb @@ -156,10 +156,6 @@ def initialize(user = nil, pass = nil, authz = nil, authcid: nil, secret: nil, realm: nil, service: "imap", host: nil, service_name: nil, warn_deprecation: true, **) - username = authcid || username || user or - raise ArgumentError, "missing username (authcid)" - password ||= secret || pass or raise ArgumentError, "missing password" - authzid ||= authz if warn_deprecation warn("WARNING: DIGEST-MD5 SASL mechanism was deprecated by RFC6331.", category: :deprecated) @@ -168,11 +164,25 @@ def initialize(user = nil, pass = nil, authz = nil, require "digest/md5" require "securerandom" require "strscan" - @username, @password, @authzid = username, password, authzid + + [authcid, username, user].compact.count == 1 or + raise ArgumentError, "conflicting values for username" + [authzid, authz].compact.count <= 1 or + raise ArgumentError, "conflicting values for authzid" + [password, secret, pass].compact.count == 1 or + raise ArgumentError, "conflicting values for password" + + @username = authcid || username || user + @password = password || secret || pass + @authzid = authzid || authz @realm = realm @host = host @service = service @service_name = service_name + + @username or raise ArgumentError, "missing username (authcid)" + @password or raise ArgumentError, "missing password" + @nc, @stage = {}, STAGE_ONE end @@ -198,42 +208,11 @@ def process(challenge) case @stage when STAGE_ONE @stage = STAGE_TWO - @sparams = parse_challenge(challenge) - @qop = sparams.key?("qop") ? ["auth"] : sparams["qop"].flatten - @nonce = sparams["nonce"] &.first - @charset = sparams["charset"]&.first - @realm ||= sparams["realm"] &.last - @host ||= realm - - if !qop.include?("auth") - raise DataFormatError, "Server does not support auth (qop = %p)" % [ - sparams["qop"] - ] - elsif (emptykey = REQUIRED.find { sparams[_1].empty? }) - raise DataFormatError, "Server didn't send %s (%p)" % [emptykey, challenge] - elsif (multikey = NO_MULTIPLES.find { sparams[_1].length > 1 }) - raise DataFormatError, "Server sent multiple %s (%p)" % [multikey, challenge] - end - - response = { - nonce: nonce, - username: username, - realm: realm, - cnonce: SecureRandom.base64(32), - "digest-uri": digest_uri, - qop: "auth", - maxbuf: 65535, - nc: "%08d" % nc(nonce), - charset: charset, - } - - response[:authzid] = @authzid unless @authzid.nil? - - response[:response] = response_value(response) - format_response(response) + process_stage_one(challenge) + stage_one_response when STAGE_TWO @stage = STAGE_DONE - raise ResponseParseError, challenge unless challenge =~ /rspauth=/ + process_stage_two(challenge) "" # if at the second stage, return an empty string else raise ResponseParseError, challenge @@ -284,14 +263,59 @@ def split_quoted_list(value, challenge) end def nc(nonce) - if @nc.has_key? nonce - @nc[nonce] = @nc[nonce] + 1 - else - @nc[nonce] = 1 + @nc[nonce] = @nc.key?(nonce) ? @nc[nonce] + 1 : 1 + @nc[nonce] + end + + def process_stage_one(challenge) + @sparams = parse_challenge(challenge) + @qop = sparams.key?("qop") ? ["auth"] : sparams["qop"].flatten + + guard_stage_one(challenge) + + @nonce = sparams["nonce"] .first + @charset = sparams["charset"].first + + @realm ||= sparams["realm"].last + @host ||= realm + end + + def guard_stage_one(challenge) + if !qop.include?("auth") + raise DataFormatError, "Server does not support auth (qop = %p)" % [ + sparams["qop"] + ] + elsif (emptykey = REQUIRED.find { sparams[_1].empty? }) + raise DataFormatError, "Server didn't send %s (%p)" % [emptykey, challenge] + elsif (multikey = NO_MULTIPLES.find { sparams[_1].length > 1 }) + raise DataFormatError, "Server sent multiple %s (%p)" % [multikey, challenge] end end - def response_value(response) + def stage_one_response + response = { + nonce: nonce, + username: username, + realm: realm, + cnonce: SecureRandom.base64(32), + "digest-uri": digest_uri, + qop: "auth", + maxbuf: 65535, + nc: "%08d" % nc(nonce), + charset: charset, + } + + response[:authzid] = authzid unless authzid.nil? + response[:response] = compute_digest(response) + + format_response(response) + end + + def process_stage_two(challenge) + raise ResponseParseError, challenge unless challenge =~ /rspauth=/ + end + + def compute_digest(response) a1 = compute_a1(response) a2 = compute_a2(response) Digest::MD5.hexdigest( From a1089f690aff300d231157294ea3cb3c14b56a12 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 13 Jun 2023 18:25:52 -0400 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=9A=A8=20SASL:=20add=5Fauthenticator?= =?UTF-8?q?=20warns=20on=20reassignment=20(=F0=9F=9A=A7=20=3F=3F=3F)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The question is: do we even want to this? --- lib/net/imap/sasl/authenticators.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/net/imap/sasl/authenticators.rb b/lib/net/imap/sasl/authenticators.rb index 4c26e452..5d28cc8e 100644 --- a/lib/net/imap/sasl/authenticators.rb +++ b/lib/net/imap/sasl/authenticators.rb @@ -68,7 +68,7 @@ def names; @authenticators.keys end # When only a single argument is given, the authenticator class will be # lazily loaded from Net::IMAP::SASL::#{name}Authenticator (case is # preserved and non-alphanumeric characters are removed.. - def add_authenticator(name, authenticator = nil) + def add_authenticator(name, authenticator = nil, warn_overwrite: true) authenticator ||= begin class_name = "#{name.gsub(/[^a-zA-Z0-9]/, "")}Authenticator".to_sym auth_class = nil @@ -78,6 +78,11 @@ def add_authenticator(name, authenticator = nil) } end key = Authenticators.normalize_name(name) + if warn_overwrite && (original = @authenticators[key]) + warn("%p: replacing existing %p authenticator: %p" % [ + self, key, original + ], uplevel: 1) + end @authenticators[key] = authenticator end