Skip to content

Commit bff7bc7

Browse files
committed
🥅 Cancel AUTHENTICATE if process raises an error [🚧 server error]
The exception will be re-raised after the protocol cancel response has been sent.
1 parent 4603eee commit bff7bc7

File tree

4 files changed

+50
-9
lines changed

4 files changed

+50
-9
lines changed

lib/net/imap.rb

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,11 +1351,19 @@ def authenticate(mechanism, *creds,
13511351
response = authenticator.process(nil)
13521352
cmdargs << (response.empty? ? "=" : [response].pack("m0"))
13531353
end
1354+
process_error = nil
13541355
result = send_command_with_continuations(*cmdargs) {|data|
1355-
challenge = data.unpack1("m")
1356-
response = authenticator.process challenge
1357-
[response].pack("m0")
1356+
unless process_error
1357+
challenge = data.unpack1("m")
1358+
response = begin
1359+
authenticator.process challenge
1360+
rescue => ex
1361+
process_error = ex
1362+
end
1363+
end
1364+
process_error ? "*" : [response].pack("m0")
13581365
}
1366+
raise process_error if process_error
13591367
if authenticator.respond_to?(:done?) && !authenticator.done?
13601368
logout!
13611369
raise SASL::AuthenticationIncomplete, result

lib/net/imap/sasl/authentication_exchange.rb

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ module SASL
66

77
# This API is *experimental*, and may change.
88
#
9-
# TODO: catch exceptions in #process and send #cancel_response.
10-
# TODO: raise an error if the command succeeds after being canceled.
119
# TODO: use with more clients, to verify the API can accommodate them.
1210
# TODO: pass ClientAdapter#service to SASL.authenticator
1311
#
@@ -80,6 +78,9 @@ def self.build(client, mechanism, *args, sasl_ir: true, **kwargs, &block)
8078

8179
attr_reader :mechanism, :authenticator
8280

81+
# An exception that has been raised by <tt>authenticator.process</tt>.
82+
attr_reader :process_error
83+
8384
def initialize(client, mechanism, authenticator, sasl_ir: true)
8485
client => SASL::ClientAdapter
8586
@client = client
@@ -93,8 +94,17 @@ def initialize(client, mechanism, authenticator, sasl_ir: true)
9394
# using #authenticator. Authentication failures will raise an
9495
# exception. Any exceptions other than AuthenticationCanceled or those
9596
# in <tt>client.response_errors</tt> will drop the connection.
97+
#
98+
# When <tt>authenticator.process</tt> raises any StandardError
99+
# (including AuthenticationCanceled), the authentication exchange will
100+
# be canceled before re-raising the exception. The server will usually
101+
# respond with an error response, and the client will most likely raise
102+
# that error. This client error will supercede the original error.
103+
# Unfortunately, the original error will not be the +#cause+ for the
104+
# client error. But it will be available on #process_error.
96105
def authenticate
97106
client.run_command(mechanism, initial_response) { process _1 }
107+
.tap { raise process_error if process_error }
98108
.tap { raise AuthenticationIncomplete, _1 unless done? }
99109
rescue AuthenticationCanceled, *client.response_errors
100110
raise # but don't drop the connection
@@ -128,9 +138,12 @@ def initial_response
128138
end
129139

130140
def process(challenge)
131-
client.encode authenticator.process client.decode challenge
132-
ensure
133141
@processed = true
142+
return client.cancel_response if process_error
143+
client.encode authenticator.process client.decode challenge
144+
rescue => process_error
145+
@process_error = process_error
146+
client.cancel_response
134147
end
135148

136149
end

test/net/imap/fake_server/command_router.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,9 @@ def handler_for(command)
9797
response_b64 = resp.request_continuation("") || ""
9898
state.commands << {continuation: response_b64}
9999
end
100-
response = Base64.decode64(response_b64)
101-
response.empty? and return resp.fail_bad "canceled"
100+
response_b64.strip == ?* and return resp.fail_bad "canceled"
101+
response = Base64.decode64(response_b64) rescue :decode64_failed
102+
response == :decode64_failed and return resp.fail_bad "invalid b64"
102103
# TODO: support mechanisms other than PLAIN.
103104
parts = response.split("\0")
104105
parts.length == 3 or return resp.fail_bad "invalid"

test/net/imap/test_imap.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,6 +1011,25 @@ def test_id
10111011
end
10121012
end
10131013

1014+
test "#authenticate can be canceled with SASL::AuthenticationCanceled" do
1015+
with_fake_server(
1016+
preauth: false, cleartext_auth: true,
1017+
sasl_ir: false, sasl_mechanisms: %i[PLAIN]
1018+
) do |server, imap|
1019+
registry = Net::IMAP::SASL::Authenticators.new(use_defaults: false)
1020+
registry.add_authenticator :plain, ->(*a, **kw, &b) {
1021+
->(challenge) {
1022+
raise(Net::IMAP::SASL::AuthenticationCanceled,
1023+
"a: %p, kw: %p, b: %p" % [a, kw, b])
1024+
}
1025+
}
1026+
assert_raise_with_message(Net::IMAP::BadResponseError, "canceled") do
1027+
imap.authenticate(:plain, hello: :world, registry: registry)
1028+
end
1029+
refute imap.disconnected?
1030+
end
1031+
end
1032+
10141033
def test_uidplus_uid_expunge
10151034
with_fake_server(select: "INBOX",
10161035
extensions: %i[UIDPLUS]) do |server, imap|

0 commit comments

Comments
 (0)