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

Add support for SCRAM-SHA-* #5

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
14 changes: 10 additions & 4 deletions lib/net/sasl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require_relative "sasl/digest_md5_authenticator"
require_relative "sasl/login_authenticator"
require_relative "sasl/plain_authenticator"
require_relative "sasl/scram_authenticator"

module Net

Expand Down Expand Up @@ -57,10 +58,15 @@ def self.authenticator(mechanism, *args, **kwargs)
# The default global registry used by Net::SASL.authenticator
DEFAULT_REGISTRY = Registry.new

add_authenticator "PLAIN", PlainAuthenticator
add_authenticator "LOGIN", LoginAuthenticator
add_authenticator "DIGEST-MD5", DigestMD5Authenticator
add_authenticator "CRAM-MD5", CramMD5Authenticator
add_authenticator "PLAIN", PlainAuthenticator
add_authenticator "LOGIN", LoginAuthenticator
add_authenticator "DIGEST-MD5", DigestMD5Authenticator
add_authenticator "CRAM-MD5", CramMD5Authenticator
add_authenticator "SCRAM-SHA-1", ScramAuthenticator.for("SHA1")
add_authenticator "SCRAM-SHA-224", ScramAuthenticator.for("SHA224")
add_authenticator "SCRAM-SHA-256", ScramAuthenticator.for("SHA256")
add_authenticator "SCRAM-SHA-384", ScramAuthenticator.for("SHA384")
add_authenticator "SCRAM-SHA-512", ScramAuthenticator.for("SHA512")
Copy link
Owner

@nevans nevans Jul 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that it only takes one line of code. But, even though plugging in alternate hashes is fairly straightforward and predictable, I'd prefer to only include official SASL mechanisms in the default set. I think that the IETF drafts for 512 (or at least, the ones that I knew about) recently expired, although I haven't researched why.

For any that are left out, we can include documentation so that users can add it themselves, if they really need it sooner.


end

Expand Down
91 changes: 91 additions & 0 deletions lib/net/sasl/scram_authenticator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# frozen_string_literal: true

require "idn"
require "openssl"
require "securerandom"

module Net

module SASL

# Authenticator for the "`SCRAM`" family of SASL mechanism types specified
# in RFC5802(https://tools.ietf.org/html/rfc5802).
class ScramAuthenticator < Authenticator

def self.for(hash)
Class.new(ScramAuthenticator) do
define_method :initialize do |*args, **options|
super(*args, hash: hash, **options)
end
end
end

# Provide the +username+ and +password+ credentials. An optional
# +authzid+ is defined as: "The "authorization ID" as per
# RFC2222[https://tools.ietf.org/html/rfc2222],
# encoded in UTF-8. optional. If present, and the
# authenticating user has sufficient privilege, and the server supports
# it, then after authentication the server will use this identity for
# making all accesses and access checks. If the client specifies it, and
# the server does not support it, then the response-value will be
# incorrect, and authentication will fail."
#
# This should generally be instantiated via Net::SASL.authenticator.
def initialize(username, password, authzid = nil, hash:, **options)
super
@hash = OpenSSL::Digest.new(hash)
Copy link
Owner

@nevans nevans Jul 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer to define hash on the subclasses. It loses the nice one liner for new SCRAM-* mechanisms, but it'll be simpler and only like... three lines.

class SCRAMAuthenticator
  def hash; @hash ||= OpenSSL::Digest.new(self.class::HASH) end
end

# Defined in RFC5802, etc etc etc
class SCRAM256Authenticator < SCRAMAuthenticator
  HASH = "SHA256"
end

The one-liner could be recreated with only a little bit of extra work. But I'm not sure it's worth it, especially because I prefer an explicitly named class constant to which we can add rdoc for each specific mechanism.

@cnonce = options[:cnonce] || SecureRandom.hex(32)
@done = false
end

def supports_initial_response?
true
end

def done?
@done
end

# responds to the server's challenges
def process(challenge)
return "n,#{'a=' + @authzid if @authzid},#{initial_message}" if challenge.nil?
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't rely on nil, because not every protocol supports initial response. E.g. IMAP4rev1 without the SASL-IR extension. Per the SCRAM RFC, it should just be an empty string in that case. But it's still better to just explicitly track the state, IMO. The first time the method is called, challenge will be ignored and the initial response will be sent.


sparams = challenge.split(/,/).each_with_object({}) do |pair, h|
k, v = pair.split(/=/)
h[k] = v
end

if @server_signature
@done = sparams["v"].unpack("m").first == @server_signature
return if @done

raise ChallengeParseError, "Bad server signature"
end

bare = "c=biws,r=#{sparams['r']}"
salted_password = OpenSSL::KDF.pbkdf2_hmac(
IDN::Stringprep.with_profile(@password.encode("utf-8"), "SASLprep"),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a pure ruby stdlib implementation of SASLprep in mind for this but it isn't ready yet. nearly done, but not quite. The RFC does give us an official way to cheat: only allow ASCII strings. ;) So this could just guard raise unless password.ascii_only? and I'd replace it with my Net::SASL::SASLprep.saslprep when it's done.

salt: sparams["s"].unpack("m").first,
iterations: sparams["i"].to_i,
length: @hash.digest_length,
hash: @hash
)
client_key = OpenSSL::HMAC.digest(@hash, salted_password, "Client Key")
stored_key = @hash.digest(client_key)
auth_message = "#{initial_message},#{challenge},#{bare}"
client_signature = OpenSSL::HMAC.digest(@hash, stored_key, auth_message)
client_proof = client_key.bytes.zip(client_signature.bytes).map { |x,y| (x ^ y).chr }.join
server_key = OpenSSL::HMAC.digest(@hash, salted_password, "Server Key")
@server_signature = OpenSSL::HMAC.digest(@hash, server_key, auth_message)
"#{bare},p=#{[client_proof].pack('m').chomp}"
end

protected

def initial_message
"n=#{@username},r=#{@cnonce}"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should do some error checking or formatting on these values, here or in #initialize? eg "\0" or "," chars.

end

end
end
end
1 change: 1 addition & 0 deletions net-sasl.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ Gem::Specification.new do |spec|
spec.require_paths = ["lib"]

spec.add_dependency "digest"
spec.add_dependency "idn-ruby"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer a pure-ruby saslprep, to avoid dependencies. I'm hoping to replace the SASL implementations in net/imap and net/pop and net/smtp, which are all bundled gems. Adding another dependency might prevent that from happening; especially a C dependency, which isn't ideal for for JRuby and TruffleRuby.

My plan is to allow a swap-able saslprep library, for performance. But that will need benchmarks.

spec.add_dependency "strscan"
end
13 changes: 13 additions & 0 deletions test/net/sasl_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,17 @@ def plain(*args, **kwargs)
assert_raise(ArgumentError) { plain("u", "p", "bad\0authz") }
end

test "SCRAM-SHA-1 authenticator" do
authenticator = Net::SASL.authenticator("SCRAM-SHA-1", "user", "pencil", "zid", cnonce: "fyko+d2lbbFgONRv9qkxdawL")
assert_equal "n,a=zid,n=user,r=fyko+d2lbbFgONRv9qkxdawL", authenticator.process(nil)
refute authenticator.done?
assert_equal(
"c=biws,r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,p=v0X8v3Bz2T0CJGbJQyF0X+HI4Ts=",
authenticator.process("r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,s=QSXCR+Q6sek8bf92,i=4096")
)
refute authenticator.done?
assert authenticator.process("v=rmF9pqV8S7suAoZWja4dJRkFsKQ=").nil?
assert authenticator.done?
end

end