Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔒 Handle cancel for SASL AUTHENTICATE #324

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion lib/net/imap/sasl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,15 @@ module SASL

# Indicates an authentication exchange that will be or has been canceled
# by the client, not due to any error or failure during processing.
AuthenticationCanceled = Class.new(Error)
class AuthenticationCanceled < Error
# The error response from the server
attr_reader :response

def initialize(message = "authentication canceled", response: nil)
super(message)
@response = response
end
end

# Indicates an error when processing a server challenge, e.g: an invalid
# or unparsable challenge. An underlying exception may be available as
Expand Down
78 changes: 69 additions & 9 deletions lib/net/imap/sasl/authentication_exchange.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ module SASL
# AuthenticationExchange is used internally by Net::IMAP#authenticate.
# But the API is still *experimental*, and may change.
#
# TODO: catch exceptions in #process and send #cancel_response.
# TODO: raise an error if the command succeeds after being canceled.
# TODO: use with more clients, to verify the API can accommodate them.
# TODO: pass ClientAdapter#service to SASL.authenticator
#
Expand Down Expand Up @@ -81,6 +79,12 @@ def self.build(client, mechanism, *args, sasl_ir: true, **kwargs, &block)

attr_reader :mechanism, :authenticator

# An exception that has been raised by <tt>authenticator.process</tt>.
attr_reader :process_error

# An exception that represents an error response from the server.
attr_reader :response_error

def initialize(client, mechanism, authenticator, sasl_ir: true)
@client = client
@mechanism = Authenticators.normalize_name(mechanism)
Expand All @@ -91,12 +95,23 @@ def initialize(client, mechanism, authenticator, sasl_ir: true)

# Call #authenticate to execute an authentication exchange for #client
# using #authenticator. Authentication failures will raise an
# exception. Any exceptions other than those in RESPONSE_ERRORS will
# drop the connection.
# exception. Any exceptions other than AuthenticationCanceled or those
# in <tt>client.response_errors</tt> will drop the connection.
#
# When <tt>authenticator.process</tt> raises any StandardError
# (including AuthenticationCanceled), the authentication exchange will
# be canceled before re-raising the exception. The server will usually
# respond with an error response, and the client will most likely raise
# that error. This client error will supercede the original error.
# Unfortunately, the original error will not be the +#cause+ for the
# client error. But it will be available on #process_error.
def authenticate
client.run_command(mechanism, initial_response) { process _1 }
.tap { raise AuthenticationIncomplete, _1 unless done? }
rescue *client.response_errors
handle_cancellation do
client.run_command(mechanism, initial_response) { process _1 }
.tap { raise process_error if process_error }
.tap { raise AuthenticationIncomplete, _1 unless done? }
end
rescue AuthenticationCanceled, *client.response_errors
raise # but don't drop the connection
rescue
client.drop_connection
Expand Down Expand Up @@ -128,9 +143,54 @@ def initial_response
end

def process(challenge)
client.encode authenticator.process client.decode challenge
ensure
@processed = true
return client.cancel_response if process_error
client.encode authenticator.process client.decode challenge
rescue AuthenticationCanceled => error
@process_error = error
client.cancel_response
rescue => error
@process_error = begin
raise AuthenticationError, "error while processing server challenge"
rescue
$!
end
client.cancel_response
end

# | process | response | => result |
# |---------|----------|------------------------------------------|
# | success | success | success |
# | success | error | reraise response error |
# | error | success | raise incomplete error (cause = process) |
# | error | error | raise canceled error (cause = process) |
def handle_cancellation
result = begin
yield
rescue *client.response_errors => error
@response_error = error
raise unless process_error
end
raise_mutual_cancellation! if process_error && response_error
raise_incomplete_cancel!(result) if process_error && !response_error
result
end

def raise_mutual_cancellation!
raise process_error # sets the cause
rescue
raise AuthenticationCanceled.new(
"authentication canceled (see error #cause and #response)",
response: response_error
)
end

def raise_incomplete_cancellation!
raise process_error # sets the cause
rescue
raise AuthenticationIncomplete.new(
response_error, "server ignored canceled authentication"
)
end

end
Expand Down
5 changes: 3 additions & 2 deletions test/net/imap/fake_server/command_router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@ def handler_for(command)
response_b64 = resp.request_continuation("") || ""
state.commands << {continuation: response_b64}
end
response = Base64.decode64(response_b64)
response.empty? and return resp.fail_bad "canceled"
response_b64.strip == ?* and return resp.fail_bad "canceled"
response = Base64.decode64(response_b64) rescue :decode64_failed
response == :decode64_failed and return resp.fail_bad "invalid b64"
# TODO: support mechanisms other than PLAIN.
parts = response.split("\0")
parts.length == 3 or return resp.fail_bad "invalid"
Expand Down
29 changes: 29 additions & 0 deletions test/net/imap/test_imap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1066,6 +1066,35 @@
end
end

test "#authenticate can be canceled with SASL::AuthenticationCanceled" do
with_fake_server(
preauth: false, cleartext_auth: true,
sasl_ir: false, sasl_mechanisms: %i[PLAIN]
) do |server, imap|
registry = Net::IMAP::SASL::Authenticators.new(use_defaults: false)
registry.add_authenticator :plain, ->(*a, **kw, &b) {
obj = Object.new
obj.define_singleton_method(:process) do |challenge|
raise(Net::IMAP::SASL::AuthenticationCanceled,
"a: %p, kw: %p, b: %p, c: %p" % [a, kw, b, challenge])
end
obj
}
error = nil
assert_raise_with_message(Net::IMAP::SASL::AuthenticationCanceled,
/authentication canceled/i) do
imap.authenticate(:plain, foo: :bar, registry: registry)
rescue => error
raise # for assert_raise
end
assert_kind_of Net::IMAP::SASL::AuthenticationCanceled, error.cause
assert_equal 'a: [], kw: {:foo=>:bar}, b: nil, c: ""', error.cause.to_s

Check failure on line 1091 in test/net/imap/test_imap.rb

View workflow job for this annotation

GitHub Actions / build (head / ubuntu-latest)

Failure

<"a: [], kw: {:foo=>:bar}, b: nil, c: \"\""> expected but was <"a: [], kw: {foo: :bar}, b: nil, c: \"\"">. diff: - a: [], kw: {:foo=>:bar}, b: nil, c: "" ? - ^^ + a: [], kw: {foo: :bar}, b: nil, c: "" ? ^^

Check failure on line 1091 in test/net/imap/test_imap.rb

View workflow job for this annotation

GitHub Actions / build (head / macos-latest)

Failure

<"a: [], kw: {:foo=>:bar}, b: nil, c: \"\""> expected but was <"a: [], kw: {foo: :bar}, b: nil, c: \"\"">. diff: - a: [], kw: {:foo=>:bar}, b: nil, c: "" ? - ^^ + a: [], kw: {foo: :bar}, b: nil, c: "" ? ^^

Check failure on line 1091 in test/net/imap/test_imap.rb

View workflow job for this annotation

GitHub Actions / build (head / ubuntu-latest)

Failure

<"a: [], kw: {:foo=>:bar}, b: nil, c: \"\""> expected but was <"a: [], kw: {foo: :bar}, b: nil, c: \"\"">. diff: - a: [], kw: {:foo=>:bar}, b: nil, c: "" ? - ^^ + a: [], kw: {foo: :bar}, b: nil, c: "" ? ^^

Check failure on line 1091 in test/net/imap/test_imap.rb

View workflow job for this annotation

GitHub Actions / build (head / macos-latest)

Failure

<"a: [], kw: {:foo=>:bar}, b: nil, c: \"\""> expected but was <"a: [], kw: {foo: :bar}, b: nil, c: \"\"">. diff: - a: [], kw: {:foo=>:bar}, b: nil, c: "" ? - ^^ + a: [], kw: {foo: :bar}, b: nil, c: "" ? ^^
assert_kind_of Net::IMAP::BadResponseError, error.response
assert_equal "canceled", error.response.to_s
refute imap.disconnected?
end
end

test "#uid_expunge with EXPUNGE responses" do
with_fake_server(select: "INBOX",
extensions: %i[UIDPLUS]) do |server, imap|
Expand Down
Loading