From 0fbf55aaf13ca998928fadc8b5cf824d6a3b27a6 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 4 Nov 2022 22:09:46 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20SASL=20DIGEST-MD5:=20realm,=20ho?= =?UTF-8?q?st,=20service=5Fname,=20etc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Yes, DIGEST-MD5 is deprecated! But that also means that it was lower risk as a test-bed for refactoring a more complicated challenge/response SASL mechanism. I improved the existing authenticator it in several ways: * ♻️ Refactor to the style used in the new ScramAuthenticator. * 🔒 Use SecureRandom for cnonce (not Time.now + insecure PRNG!) * ✨ Default qop=auth (as in RFC) * ✨ User can configure realm, host, service_name, service. * This allows a correct "digest-uri" for non-IMAP clients. * ✨ Enforce requirements for sparam keys (required and no-multiples). However... it's still deprecated, so don't use it! --- lib/net/imap/sasl/digest_md5_authenticator.rb | 328 ++++++++++++++---- test/net/imap/test_imap_authenticators.rb | 68 +++- 2 files changed, 320 insertions(+), 76 deletions(-) diff --git a/lib/net/imap/sasl/digest_md5_authenticator.rb b/lib/net/imap/sasl/digest_md5_authenticator.rb index d1576ba5..866f1be8 100644 --- a/lib/net/imap/sasl/digest_md5_authenticator.rb +++ b/lib/net/imap/sasl/digest_md5_authenticator.rb @@ -9,6 +9,9 @@ # RFC-6331[https://tools.ietf.org/html/rfc6331] and should not be relied on for # security. It is included for compatibility with existing servers. class Net::IMAP::SASL::DigestMD5Authenticator + DataFormatError = Net::IMAP::DataFormatError + ResponseParseError = Net::IMAP::ResponseParseError + STAGE_ONE = :stage_one STAGE_TWO = :stage_two private_constant :STAGE_ONE, :STAGE_TWO @@ -21,6 +24,7 @@ class Net::IMAP::SASL::DigestMD5Authenticator # RFC-4616[https://tools.ietf.org/html/rfc4616] and many later RFCs abbreviate # that to +authcid+. So +authcid+ is available as an alias for #username. attr_reader :username + alias authcid username # A password or passphrase that matches the #username. # @@ -40,6 +44,60 @@ class Net::IMAP::SASL::DigestMD5Authenticator # attr_reader :authzid + # A namespace or collection of identities which contains +username+. + # + # Used by DIGEST-MD5, GSS-API, and NTLM. This is often a domain name that + # contains the name of the host performing the authentication. + # + # Defaults to the last realm in the server-provided list of + # realms. + attr_reader :realm + + # Fully qualified canonical DNS host name for the requested service. + # + # Defaults to #realm. + attr_reader :host + + # The service protocol, a + # {registered GSSAPI service name}[https://www.iana.org/assignments/gssapi-service-names/gssapi-service-names.xhtml], + # e.g. "imap", "ldap", or "xmpp". + # + # For Net::IMAP, the default is "imap" and should not be overridden. This + # must be set appropriately to use authenticators in other protocols. + # + # If an IANA-registered name isn't available, GSS-API + # (RFC-2743[https://tools.ietf.org/html/rfc2743]) allows the generic name + # "host". + attr_reader :service + + # The generic server name when the server is replicated. + # + # Not used by other \SASL mechanisms. +service_name+ will be ignored when it + # is +nil+ or identical to +host+. + # + # From RFC-2831[https://tools.ietf.org/html/rfc2831]: + # >>> + # The service is considered to be replicated if the client's + # service-location process involves resolution using standard DNS lookup + # operations, and if these operations involve DNS records (such as SRV, or + # MX) which resolve one DNS name into a set of other DNS names. In this + # case, the initial name used by the client is the "serv-name", and the + # final name is the "host" component. + attr_reader :service_name + + # Parameters sent by the server are stored in this hash. + attr_reader :sparams + + # The charset sent by the server. "UTF-8" (case insensitive) is the only + # allowed value. +nil+ should be interpreted as ISO 8859-1. + attr_reader :charset + + # nonce sent by the server + attr_reader :nonce + + # qop-options sent by the server + attr_reader :qop + # :call-seq: # new(username, password, authzid = nil, **options) -> authenticator # new(username:, password:, authzid: nil, **options) -> authenticator @@ -53,86 +111,75 @@ class Net::IMAP::SASL::DigestMD5Authenticator # * #username — Identity whose #password is used. # * #password — A password or passphrase associated with this #username. # * #authzid ― Alternate identity to act as or on behalf of. Optional. + # * #realm — A namespace for the #username, e.g. a domain. Defaults to the + # last realm in the server-provided . + # * #host — FQDN for requested service. Defaults to #realm. + # * #service_name — The generic host name when the server is replicated. + # * #service — the registered service protocol. e.g. "imap", "smtp", "ldap", + # "xmpp". For Net::IMAP, this defaults to "imap". # * +warn_deprecation+ — Set to +false+ to silence the warning. # # See the documentation for each attribute for more details. - def initialize(user = nil, pass = nil, authz = nil, + def initialize(username_arg = nil, password_arg = nil, authzid_arg = nil, username: nil, password: nil, authzid: nil, - warn_deprecation: true, **) - username ||= user or raise ArgumentError, "missing username" - password ||= pass or raise ArgumentError, "missing password" - authzid ||= authz + authcid: nil, # alias for username + realm: nil, service: "imap", host: nil, service_name: nil, + warn_deprecation: true, + **) if warn_deprecation - warn "WARNING: DIGEST-MD5 SASL mechanism was deprecated by RFC6331." - # TODO: recommend SCRAM instead. + warn "WARNING: DIGEST-MD5 SASL mechanism was deprecated by RFC-6331." end + require "digest/md5" + require "securerandom" require "strscan" - @username, @password, @authzid = username, password, authzid + + @username = username || username_arg || authcid + @password = password || password_arg + @authzid = authzid || authzid_arg + @realm = realm + @host = host + @service = service + @service_name = service_name + + @username or raise ArgumentError, "missing username" + @password or raise ArgumentError, "missing password" + [username, username_arg, authcid].compact.count == 1 or + raise ArgumentError, "conflicting values for username" + [password, password_arg].compact.count == 1 or + raise ArgumentError, "conflicting values for password" + [authzid, authzid_arg].compact.count <= 1 or + raise ArgumentError, "conflicting values for authzid" + @nc, @stage = {}, STAGE_ONE end + # From RFC-2831[https://tools.ietf.org/html/rfc2831]: + # >>> + # Indicates the principal name of the service with which the client wishes + # to connect, formed from the serv-type, host, and serv-name. For + # example, the FTP service on "ftp.example.com" would have a "digest-uri" + # value of "ftp/ftp.example.com"; the SMTP server from the example above + # would have a "digest-uri" value of "smtp/mail3.example.com/example.com". + def digest_uri + if service_name && service_name != host + "#{service}/#{host}/#{service_name}" + else + "#{service}/#{host}" + end + end + # Responds to server challenge in two stages. def process(challenge) case @stage when STAGE_ONE @stage = STAGE_TWO - sparams = {} - c = StringScanner.new(challenge) - while c.scan(/(?:\s*,)?\s*(\w+)=("(?:[^\\"]|\\.)*"|[^,]+)\s*/) - k, v = c[1], c[2] - if v =~ /^"(.*)"$/ - v = $1 - if v =~ /,/ - v = v.split(',') - end - end - sparams[k] = v - end - - raise Net::IMAP::DataFormatError, "Bad Challenge: '#{challenge}'" unless c.eos? and sparams['qop'] - raise Net::IMAP::Error, "Server does not support auth (qop = #{sparams['qop'].join(',')})" unless sparams['qop'].include?("auth") - - response = { - :nonce => sparams['nonce'], - :username => @username, - :realm => sparams['realm'], - :cnonce => Digest::MD5.hexdigest("%.15f:%.15f:%d" % [Time.now.to_f, rand, Process.pid.to_s]), - :'digest-uri' => 'imap/' + sparams['realm'], - :qop => 'auth', - :maxbuf => 65535, - :nc => "%08d" % nc(sparams['nonce']), - :charset => sparams['charset'], - } - - response[:authzid] = @authzid unless @authzid.nil? - - # now, the real thing - a0 = Digest::MD5.digest( [ response.values_at(:username, :realm), @password ].join(':') ) - - a1 = [ a0, response.values_at(:nonce,:cnonce) ].join(':') - a1 << ':' + response[:authzid] unless response[:authzid].nil? - - a2 = "AUTHENTICATE:" + response[:'digest-uri'] - a2 << ":00000000000000000000000000000000" if response[:qop] and response[:qop] =~ /^auth-(?:conf|int)$/ - - response[:response] = Digest::MD5.hexdigest( - [ - Digest::MD5.hexdigest(a1), - response.values_at(:nonce, :nc, :cnonce, :qop), - Digest::MD5.hexdigest(a2) - ].join(':') - ) - - return response.keys.map {|key| qdval(key.to_s, response[key]) }.join(',') + process_stage_one(challenge) + stage_one_response when STAGE_TWO @stage = nil - # if at the second stage, return an empty string - if challenge =~ /rspauth=/ - return '' - else - raise ResponseParseError, challenge - end + process_stage_two(challenge) + "" # if at the second stage, return an empty string else raise ResponseParseError, challenge end @@ -140,23 +187,158 @@ def process(challenge) private + 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 %p (%p)" % [emptykey, challenge] + elsif (multikey = NO_MULTIPLES.find { sparams[_1].length > 1 }) + raise DataFormatError, "Server sent multiple %p (%p)" % [multikey, challenge] + end + end + + 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 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 compute_digest(response) + a1 = compute_a1(response) + a2 = compute_a2(response) + Digest::MD5.hexdigest( + [ + Digest::MD5.hexdigest(a1), + response.values_at(:nonce, :nc, :cnonce, :qop), + Digest::MD5.hexdigest(a2) + ].join(":") + ) + end + + def compute_a0(response) + Digest::MD5.digest( + [ response.values_at(:username, :realm), password ].join(":") + ) + end + + def compute_a1(response) + a0 = compute_a0(response) + a1 = [ a0, response.values_at(:nonce, :cnonce) ].join(":") + a1 << ":#{response[:authzid]}" unless response[:authzid].nil? + a1 + end + + def compute_a2(response) + a2 = "AUTHENTICATE:#{response[:"digest-uri"]}" + if response[:qop] and response[:qop] =~ /^auth-(?:conf|int)$/ + a2 << ":00000000000000000000000000000000" + end + a2 + end + + # Directives which must not have multiples. The RFC states: + # >>> + # This directive may appear at most once; if multiple instances are present, + # the client should abort the authentication exchange. + NO_MULTIPLES = %w[nonce stale maxbuf charset algorithm].freeze + + # Required directives which must occur exactly once. The RFC states: >>> + # This directive is required and MUST appear exactly once; if not present, + # or if multiple instances are present, the client should abort the + # authentication exchange. + REQUIRED = %w[nonce algorithm].freeze + + # Directives which are composed of one or more comma delimited tokens + QUOTED_LISTABLE = %w[qop cipher].freeze + + private_constant :NO_MULTIPLES, :REQUIRED, :QUOTED_LISTABLE + + LWS = /[\r\n \t]*/n # less strict than RFC, more strict than '\s' + TOKEN = /[^\x00-\x20\x7f()<>@,;:\\"\/\[\]?={}]+/n + QUOTED_STR = /"(?: [\t\x20-\x7e&&[^"]] | \\[\x00-\x7f] )*"/nx + LIST_DELIM = /(?:#{LWS} , )+ #{LWS}/nx + AUTH_PARAM = / + (#{TOKEN}) #{LWS} = #{LWS} (#{QUOTED_STR} | #{TOKEN}) #{LIST_DELIM}? + /nx + + private_constant :LWS, :TOKEN, :QUOTED_STR, :LIST_DELIM, :AUTH_PARAM + + def parse_challenge(challenge) + sparams = Hash.new {|h, k| h[k] = [] } + c = StringScanner.new(challenge) + c.skip LIST_DELIM + while c.scan AUTH_PARAM + k, v = c[1], c[2] + k = k.downcase + if v =~ /\A"(.*)"\z/mn + v = $1.gsub(/\\(.)/mn, '\1') + v = split_quoted_list(v, challenge) if QUOTED_LISTABLE.include? k + end + sparams[k] << v + end + c.eos? or raise DataFormatError, "Bad Challenge: %p" % [challenge] + sparams.any? or raise DataFormatError, "Bad Challenge: %p" % [challenge] + sparams + end + + def split_quoted_list(value, challenge) + value.split(LIST_DELIM).reject(&:empty?).tap do + _1.any? or raise DataFormatError, "Bad Challenge: %p" % [challenge] end - return @nc[nonce] + end + + def format_response(response) + response + .keys + .map {|key| qdval(key.to_s, response[key]) } + .join(",") end # some responses need quoting - def qdval(k, v) - return if k.nil? or v.nil? - if %w"username authzid realm nonce cnonce digest-uri qop".include? k - v = v.gsub(/([\\"])/, "\\\1") - return '%s="%s"' % [k, v] + def qdval(key, val) + return if key.nil? or val.nil? + if %w[username authzid realm nonce cnonce digest-uri qop].include? key + val = val.gsub(/([\\"])/n, "\\\1") + '%s="%s"' % [key, val] else - return '%s=%s' % [k, v] + "%s=%s" % [key, val] end end diff --git a/test/net/imap/test_imap_authenticators.rb b/test/net/imap/test_imap_authenticators.rb index c0c1e299..3bc6d874 100644 --- a/test/net/imap/test_imap_authenticators.rb +++ b/test/net/imap/test_imap_authenticators.rb @@ -337,8 +337,8 @@ def test_digest_md5_authenticator_matches_mechanism end def test_digest_md5_authenticator_deprecated - assert_warn(/DIGEST-MD5.+deprecated.+RFC6331/) do - Net::IMAP::SASL.authenticator("DIGEST-MD5", "user", "pass") + assert_warn(/DIGEST-MD5.+deprecated.+RFC-?6331/) do + Net::IMAP.authenticator("DIGEST-MD5", "user", "pass") end end @@ -374,6 +374,68 @@ def test_digest_md5_authenticator ) end + def test_digest_md5_authenticator_realm_and_digest_uri + auth = digest_md5(authcid: "authc", + authzid: "authz", + password: "pass", + realm: "myrealm", + service: "smtp", + host: "mail.example.com", + service_name: "example.com") + assert_match( + %r{\A + nonce="OA6MG9tEQGm2hh", + username="authc", + realm="myrealm", + cnonce="[a-zA-Z0-9+/]{12,}={0,3}", + digest-uri="smtp/mail\.example\.com/example\.com", + qop="auth", + maxbuf=65535, + nc=00000001, + charset=utf-8, + authzid="authz", + response=[a-f0-9]+ + \Z}x, + auth.process( + %w[ + realm="somerealm" + nonce="OA6MG9tEQGm2hh" + qop="auth" + charset=utf-8 + algorithm=md5-sess + ].join(",") + ) + ) + end + + def test_digest_md5_authenticator_empty_challenge + auth = digest_md5("user", "pass") + assert_raise(Net::IMAP::DataFormatError) do + auth.process(" ") + end + end + + def test_digest_md5_authenticator_empty_challenge_commas + auth = digest_md5("user", "pass") + assert_raise(Net::IMAP::DataFormatError) do + auth.process(" , , ") + end + end + + def test_digest_md5_authenticator_garbage_no_equal_sign + auth = digest_md5("user", "pass") + assert_raise(Net::IMAP::DataFormatError) do + auth.process('nonce=required,algorithm=md5-sess,foo') + end + end + + def test_digest_md5_authenticator_qdstr_with_comma + auth = digest_md5("user", "pass") + assert_raise(Net::IMAP::DataFormatError) do + auth.process('nonce=required,algorithm=md5-sess,.') + end + end + def test_digest_md5_authenticator_garbage auth = digest_md5("user", "pass") assert_raise(Net::IMAP::DataFormatError) do @@ -381,7 +443,7 @@ def test_digest_md5_authenticator_garbage end end - def test_digest_md5_authenticator_no_qop + def test_digest_md5_authenticator_empty_qop auth = digest_md5("user", "pass") assert_raise(Net::IMAP::DataFormatError) do auth.process('Qop=""')