8000 Warn when using deprecated SASL mechanisms · ruby/net-imap@d466c2e · GitHub
[go: up one dir, main page]
Skip to content

Commit d466c2e

Browse files
nevanssingpolyma
nevans
and
singpolyma
committed
Warn when using deprecated SASL mechanisms
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 <singpolyma@singpolyma.net>
1 parent 3545549 commit d466c2e

File tree

7 files changed

+175
-45
lines changed

7 files changed

+175
-45
lines changed

‎lib/net/imap.rb

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -378,28 +378,37 @@ def starttls(options = {}, verify = true)
378378
# Sends an AUTHENTICATE command to authenticate the client.
379379
# The +auth_type+ parameter is a string that represents
380380
# the authentication mechanism to be used. Currently Net::IMAP
381-
# supports the authentication mechanisms:
382-
#
383-
# LOGIN:: login using cleartext user and password.
384-
# CRAM-MD5:: login with cleartext user and encrypted password
385-
# (see [RFC-2195] for a full description). This
386-
# mechanism requires that the server have the user's
387-
# password stored in clear-text password.
388-
#
389-
# For both of these mechanisms, there should be two +args+: username
390-
# and (cleartext) password. A server may not support one or the other
391-
# of these mechanisms; check #capability for a capability of
392-
# the form "AUTH=LOGIN" or "AUTH=CRAM-MD5".
393-
#
394-
# Authentication is done using the appropriate authenticator object:
395-
# see +add_authenticator+ for more information on plugging in your own
396-
# authenticator.
381+
# supports the following mechanisms:
382+
#
383+
# PLAIN:: Login using cleartext user and password. Secure with TLS.
384+
# See Net::IMAP::PlainAuthenticator.
385+
# CRAM-MD5:: DEPRECATED: Use PLAIN (or DIGEST-MD5) with TLS.
386+
# DIGEST-MD5:: DEPRECATED by RFC6331. Must be secured using TLS.
387+
# See Net::IMAP::DigestMD5Authenticator.
388+
# LOGIN:: DEPRECATED: Use PLAIN.
389+
#
390+
# Most mechanisms require two args: authentication identity (e.g. username)
391+
# and credentials (e.g. a password). But each mechanism requires and allows
392+
# different arguments; please consult the documentation for the specific
393+
# mechanisms you are using. <em>Several obsolete mechanisms are available
394+
# for backwards compatibility. Using deprecated mechanisms will issue
395+
# warnings.</em>
396+
#
397+
# Servers do not support all mechanisms and clients must not attempt to use
398+
# a mechanism unless "AUTH=#{mechanism}" is listed as a #capability.
399+
# Clients must not attempt to authenticate or #login when +LOGINDISABLED+ is
400+
# listed with the capabilities. Server capabilities, especially auth
401+
# mechanisms, do change after calling #starttls so they need to be checked
402+
# again.
397403
#
398404
# For example:
399405
#
400-
# imap.authenticate('LOGIN', user, password)
406+
# imap.authenticate('PLAIN', user, password)
401407
#
402408
# A Net::IMAP::NoResponseError is raised if authentication fails.
409+
#
410+
# See +Net::IMAP::Authenticators+ for more information on plugging in your
411+
# own authenticator.
403412
def authenticate(auth_type, *args)
404413
authenticator = self.class.authenticator(auth_type, *args)
405414
send_command("AUTHENTICATE", auth_type) do |resp|

‎lib/net/imap/authenticators.rb

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@ def add_authenticator(auth_type, authenticator)
1919

2020
# Builds an authenticator for Net::IMAP#authenticate. +args+ will be passed
2121
# directly to the chosen authenticator's +#initialize+.
22-
def authenticator(auth_type, *args)
23-
auth_type = auth_type.upcase
24-
unless authenticators.has_key?(auth_type)
25-
raise ArgumentError,
26-
format('unknown auth type - "%s"', auth_type)
22+
def authenticator(mechanism, *authargs, **properties, &callback)
23+
authenticator = authenticators.fetch(mechanism.upcase) do
24+
raise ArgumentError, 'unknown auth type - "%s"' % mechanism
25+
end
26+
if authenticator.respond_to?(:new)
27+
authenticator.new(*authargs, **properties, &callback)
28+
else
29+
authenticator.call(*authargs, **properties, &callback)
2730
end
28-
authenticators[auth_type].new(*args)
2931
end
3032

3133
private
@@ -38,7 +40,8 @@ def authenticators
3840

3941
Net::IMAP.extend Net::IMAP::Authenticators
4042

41-
require_relative "authenticators/login"
4243
require_relative "authenticators/plain"
44+
45+
require_relative "authenticators/login"
4346
require_relative "authenticators/cram_md5"
4447
require_relative "authenticators/digest_md5"

‎lib/net/imap/authenticators/cram_md5.rb

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
# frozen_string_literal: true
22

3-
require "digest/md5"
4-
53
# Authenticator for the "+CRAM-MD5+" SASL mechanism, specified in
64
# RFC2195[https://tools.ietf.org/html/rfc2195]. See Net::IMAP#authenticate.
75
#
@@ -23,7 +21,11 @@ def process(challenge)
2321

2422
private
2523

26-
def initialize(user, password)
24+
def initialize(user, password, warn_deprecation: true, **_ignored)
25+
if warn_deprecation
26+
warn "WARNING: CRAM-MD5 mechanism is deprecated." # TODO: recommend SCRAM
27+
end
28+
require "digest/md5"
2729
@user = user
2830
@password = password
2931
end

‎lib/net/imap/authenticators/digest_md5.rb

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
# frozen_string_literal: true
22

3-
require "digest/md5"
4-
require "strscan"
5-
63
# Net::IMAP authenticator for the "`DIGEST-MD5`" SASL mechanism type, specified
74
# in RFC2831(https://tools.ietf.org/html/rfc2831). See Net::IMAP#authenticate.
85
#
@@ -29,8 +26,8 @@ def process(challenge)
2926
sparams[k] = v
3027
end
3128

32-
raise DataFormatError, "Bad Challenge: '#{challenge}'" unless c.rest.size == 0
33-
raise Error, "Server does not support auth (qop = #{sparams['qop'].join(',')})" unless sparams['qop'].include?("auth")
29+
raise Net::IMAP::DataFormatError, "Bad Challenge: '#{challenge}'" unless c.eos?
30+
raise Net::IMAP::Error, "Server does not support auth (qop = #{sparams['qop'].join(',')})" unless sparams['qop'].include?("auth")
3431

3532
response = {
3633
:nonce => sparams['nonce'],
@@ -77,11 +74,18 @@ def process(challenge)
7774
end
7875
end
7976

80-
def initialize(user, password, authname = nil)
77+
def initialize(user, password, authname = nil, warn_deprecation: true)
78+
if warn_deprecation
79+
warn "WARNING: DIGEST-MD5 SASL mechanism was deprecated by RFC6331."
80+
# TODO: recommend SCRAM instead.
81+
end
82+
require "digest/md5"
83+
require "strscan"
8184
@user, @password, @authname = user, password, authname
8285
@nc, @stage = {}, STAGE_ONE
8386
end
8487

88+
8589
private
8690

8791
STAGE_ONE = :stage_one
@@ -100,7 +104,7 @@ def nc(nonce)
100104
def qdval(k, v)
101105
return if k.nil? or v.nil?
102106
if %w"username authzid realm nonce cnonce digest-uri qop".include? k
103-
v.gsub!(/([\\"])/, "\\\1")
107+
v = v.gsub(/([\\"])/, "\\\1")
104108
return '%s="%s"' % [k, v]
105109
else
106110
return '%s=%s' % [k, v]

‎lib/net/imap/authenticators/login.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@ def process(data)
3333
STATE_USER = :USER
3434
STATE_PASSWORD = :PASSWORD
3535

36-
def initialize(user, password)
36+
def initialize(user, password, warn_deprecation: true, **_ignored)
37+
if warn_deprecation
38+
warn "WARNING: LOGIN SASL mechanism is deprecated. Use PLAIN instead."
39+
end
3740
@user = user
3841
@password = password
3942
@state = STATE_USER

‎net-imap.gemspec

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,6 @@ Gem::Specification.new do |spec|
3232
spec.require_paths = ["lib"]
3333

3434
spec.add_dependency "net-protocol"
35-
spec.add_dependency "digest"
36-
spec.add_dependency "strscan"
35+
spec.add_development_dependency "digest"
36+
spec.add_development_dependency "strscan"
3737
end

‎test/net/imap/test_imap_authenticators.rb

Lines changed: 117 additions & 8 deletions
< 10000 tr class="diff-line-row">
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,128 @@
55

66
class IMAPAuthenticatorsTest < Test::Unit::TestCase
77

8-
PLAIN = Net::IMAP::PlainAuthenticator
8+
# ----------------------
9+
# PLAIN
10+
# ----------------------
911

10-
def test_plain
11-
assert_equal("\0authc\0passwd",
12-
PLAIN.new("authc", "passwd").process(nil))
12+
def plain(*args, **kwargs, &block)
13+
Net::IMAP.authenticator("PLAIN", *args, **kwargs, &block)
14+
end
15+
16+
def test_plain_authenticator_matches_mechanism
17+
assert_kind_of(Net::IMAP::PlainAuthenticator, plain("user", "pass"))
18+
end
19+
20+
def test_plain_response
21+
assert_equal("\0authc\0passwd", plain("authc", "passwd").process(nil))
1322
assert_equal("authz\0user\0pass",
14-
PLAIN.new("user", "pass", authzid: "authz").process(nil))
23+
plain("user", "pass", authzid: "authz").process(nil))
1524
end
1625

1726
def test_plain_no_null_chars
18-
assert_raise(ArgumentError) { PLAIN.new("bad\0user", "pass") }
19-
assert_raise(ArgumentError) { PLAIN.new("user", "bad\0pass") }
20-
assert_raise(ArgumentError) { PLAIN.new("u", "p", authzid: "bad\0authz") }
27+
assert_raise(ArgumentError) { plain("bad\0user", "pass") }
28+
assert_raise(ArgumentError) { plain("user", "bad\0pass") }
29+
assert_raise(ArgumentError) { plain("u", "p", authzid: "bad\0authz") }
30+
end
31+
32+
# ----------------------
33+
# LOGIN (obsolete)
34+
# ----------------------
35+
36+
def login(*args, warn_deprecation: false, **kwargs, &block)
37+
Net::IMAP.authenticator(
38+
"LOGIN", *args, warn_deprecation: warn_deprecation, **kwargs, &block
39+
)
40+
end
41+
42+
def test_login_authenticator_matches_mechanism
43+
assert_kind_of(Net::IMAP::LoginAuthenticator, login("n", "p"))
44+
end
45+
46+
def test_login_authenticator_deprecated
47+
assert_warn(/LOGIN.+deprecated.+PLAIN/) do
48+
Net::IMAP.authenticator("LOGIN", "user", "pass")
49+
end
50+
end
51+
52+
def test_login_responses
53+
auth_session = login("username", "password")
54+
assert_equal("username", auth_session.process("username?"))
55+
assert_equal("password", auth_session.process("password?"))
56+
end
57+
58+
# ----------------------
59+
# CRAM-MD5 (obsolete)
60+
# ----------------------
61+
62+
def cram_md5(*args, warn_deprecation: false, **kwargs, &block)
63+
Net::IMAP.authenticator(
64+
"CRAM-MD5", *args, warn_deprecation: warn_deprecation, **kwargs, &block
65+
)
66+
end
67+
68+
def test_cram_md5_authenticator_matches_mechanism
69+
assert_kind_of(Net::IMAP::CramMD5Authenticator, cram_md5("n", "p"))
70+
end
71+
72+
def test_cram_md5_authenticator_deprecated
73+
assert_warn(/CRAM-MD5.+deprecated./) do
74+
Net::IMAP.authenticator("CRAM-MD5", "user", "pass")
75+
end
76+
end
77+
78+
def test_cram_md5_authenticator
79+
auth = cram_md5("username", "password")
80+
assert_match("username e2ce8ff3d1b914ddf339aa9f55198f86",
81+
auth.process("fake-server-challence-string"))
82+
end
83+
84+
# ----------------------
85+
# DIGEST-MD5 (obsolete)
86+
# ----------------------
87+
88+
def digest_md5(*args, warn_deprecation: false, **kwargs, &block)
89+
Net::IMAP.authenticator(
90+
"DIGEST-MD5", *args, warn_deprecation: warn_deprecation, **kwargs, &block
91+
)
92+
end
93+
94+
def test_digest_md5_authenticator_matches_mechanism
95+
assert_kind_of(Net::IMAP::DigestMD5Authenticator, digest_md5("n", "p", "z"))
96+
end
97+
98+
def test_digest_md5_authenticator_deprecated
99+
assert_warn(/DIGEST-MD5.+deprecated.+RFC6331/) do
100+
Net::IMAP.authenticator("DIGEST-MD5", "user", "pass")
101+
end
102+
end
103+
104+
def test_digest_md5_authenticator
105+
auth = digest_md5("cid", "password", "zid")
106+
assert_match(
107+
%r{\A
108+
nonce="OA6MG9tEQGm2hh",
109+
username="cid",
110+
realm="somerealm",
111+
cnonce="[a-zA-Z0-9+/]{12,}={0,3}", # RFC2831: >= 64 bits of entropy
112+
digest-uri="imap/somerealm",
113+
qop="auth",
114+
maxbuf=65535,
115+
nc=00000001,
116+
charset=utf-8,
117+
authzid="zid",
118+
response=[a-f0-9]+
119+
\Z}x,
120+
auth.process(
121+
%w[
122+
realm="somerealm"
123+
nonce="OA6MG9tEQGm2hh"
124+
qop="auth"
125+
charset=utf-8
126+
algorithm=md5-sess
127+
].join(",")
128+
)
129+
)
21130
end
22131

23132
end

0 commit comments

Comments
 (0)
0