From 7360e5356fb7f4d61da123d9eba7a067bc4d15c9 Mon Sep 17 00:00:00 2001 From: Donald Parish Date: Tue, 17 Feb 2009 11:17:57 -0600 Subject: [PATCH 1/3] Digest authentication handles either absolute or relative uri. The request-uri must match the digest-uri. --- .../lib/action_controller/http_authentication.rb | 2 +- .../controller/http_digest_authentication_test.rb | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/actionpack/lib/action_controller/http_authentication.rb b/actionpack/lib/action_controller/http_authentication.rb index 2ccbc22..6e0a79a 100644 --- a/actionpack/lib/action_controller/http_authentication.rb +++ b/actionpack/lib/action_controller/http_authentication.rb @@ -183,7 +183,7 @@ module ActionController if valid_nonce && realm == credentials[:realm] && opaque(request.session.session_id) == credentials[:opaque] password = password_procedure.call(credentials[:username]) - expected = expected_response(request.env['REQUEST_METHOD'], credentials[:uri], credentials, password) + expected = expected_response(request.env['REQUEST_METHOD'], request.env['REQUEST_URI'], credentials, password) expected == credentials[:response] end end diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index 4913e76..91c7515 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -107,8 +107,19 @@ class HttpDigestAuthenticationTest < ActionController::TestCase assert_equal 'Definitely Maybe', @response.body end - test "authentication request with relative URI" do - @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:uri => "/", :username => 'pretty', :password => 'please') + test "authentication request with request-uri that doesn't match credentials digest-uri" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please') + @request.env['REQUEST_URI'] = "/http_digest_authentication_test/dummy_digest/altered/uri" + get :display + + assert_response :unauthorized + assert_equal "Authentication Failed", @response.body + end + + test "authentication request with absolute uri" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:uri => "http://test.host/http_digest_authentication_test/dummy_digest/display", + :username => 'pretty', :password => 'please') + @request.env['REQUEST_URI'] = "http://test.host/http_digest_authentication_test/dummy_digest/display" get :display assert_response :success @@ -122,14 +133,14 @@ class HttpDigestAuthenticationTest < ActionController::TestCase options.reverse_merge!(:nc => "00000001", :cnonce => "0a4f113b") password = options.delete(:password) - # Perform unautheticated get to retrieve digest parameters to use on subsequent request + # Perform unauthenticated GET to retrieve digest parameters to use on subsequent request get :index assert_response :unauthorized credentials = decode_credentials(@response.headers['WWW-Authenticate']) credentials.merge!(options) - credentials.reverse_merge!(:uri => "http://#{@request.host}#{@request.env['REQUEST_URI']}") + credentials.reverse_merge!(:uri => "#{@request.env['REQUEST_URI']}") ActionController::HttpAuthentication::Digest.encode_credentials("GET", credentials, password) end -- 1.6.1.3 From c2c8a421c4c1fe9b882c581e269e8126b755ba2d Mon Sep 17 00:00:00 2001 From: Donald Parish Date: Wed, 18 Feb 2009 06:43:13 -0600 Subject: [PATCH 2/3] Add support for using ha1 hash of MD5(user:realm:passwod) instead of plain text password Correct Digest example to include md5 module. Changed md5 require to be Ruby 1.9 compatible. --- .../lib/action_controller/http_authentication.rb | 42 ++++++++++++++------ .../controller/http_digest_authentication_test.rb | 20 +++++++-- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/actionpack/lib/action_controller/http_authentication.rb b/actionpack/lib/action_controller/http_authentication.rb index 6e0a79a..a66d4ee 100644 --- a/actionpack/lib/action_controller/http_authentication.rb +++ b/actionpack/lib/action_controller/http_authentication.rb @@ -67,9 +67,12 @@ module ActionController # end # # Simple Digest example: - # + # + # require 'digest/md5' # class PostsController < ApplicationController - # USERS = {"dhh" => "secret"} + # REALM = "SuperSecret" + # USERS = {"dhh" => "secret", #plain text password + # "dap" => Digest:MD5::hexdigest(["dap",REALM,"secret"].join(":")) #ha1 digest password # # before_filter :authenticate, :except => [:index] # @@ -83,14 +86,18 @@ module ActionController # # private # def authenticate - # authenticate_or_request_with_http_digest(realm) do |username| + # authenticate_or_request_with_http_digest(REALM) do |username| # USERS[username] # end # end # end # - # NOTE: The +authenticate_or_request_with_http_digest+ block must return the user's password so the framework can appropriately - # hash it to check the user's credentials. Returning +nil+ will cause authentication to fail. + # NOTE: The +authenticate_or_request_with_http_digest+ block must return the user's password or the ha1 digest hash so the framework can appropriately + # hash to check the user's credentials. Returning +nil+ will cause authentication to fail. + # Storing the ha1 hash: MD5(username:realm:password), is better than storing a plain password. If + # the password file or database is compromised, the attacker would be able to use the ha1 hash to + # authenticate as the user at this +realm+, but would not have the user's password to try using at + # other sites. # # On shared hosts, Apache sometimes doesn't pass authentication headers to # FCGI instances. If your environment matches this description and you cannot @@ -177,26 +184,37 @@ module ActionController end # Raises error unless the request credentials response value matches the expected value. + # First try the password as a ha1 digest password. If this fails, then try it as a plain + # text password. def validate_digest_response(request, realm, &password_procedure) credentials = decode_credentials_header(request) valid_nonce = validate_nonce(request, credentials[:nonce]) if valid_nonce && realm == credentials[:realm] && opaque(request.session.session_id) == credentials[:opaque] password = password_procedure.call(credentials[:username]) - expected = expected_response(request.env['REQUEST_METHOD'], request.env['REQUEST_URI'], credentials, password) - expected == credentials[:response] + + [true, false].any? do |password_is_ha1| + expected = expected_response(request.env['REQUEST_METHOD'], request.env['REQUEST_URI'], credentials, password, password_is_ha1) + expected == credentials[:response] + end end end # Returns the expected response for a request of +http_method+ to +uri+ with the decoded +credentials+ and the expected +password+ - def expected_response(http_method, uri, credentials, password) - ha1 = ::Digest::MD5.hexdigest([credentials[:username], credentials[:realm], password].join(':')) + # Optional parameter +password_is_ha1+ is set to +true+ by default, since best practice is to store ha1 digest instead + # of a plain-text password. + def expected_response(http_method, uri, credentials, password, password_is_ha1=true) + ha1 = password_is_ha1 ? password : ha1(credentials, password) ha2 = ::Digest::MD5.hexdigest([http_method.to_s.upcase, uri].join(':')) ::Digest::MD5.hexdigest([ha1, credentials[:nonce], credentials[:nc], credentials[:cnonce], credentials[:qop], ha2].join(':')) end - def encode_credentials(http_method, credentials, password) - credentials[:response] = expected_response(http_method, credentials[:uri], credentials, password) + def ha1(credentials, password) + ::Digest::MD5.hexdigest([credentials[:username], credentials[:realm], password].join(':')) + end + + def encode_credentials(http_method, credentials, password, password_is_ha1) + credentials[:response] = expected_response(http_method, credentials[:uri], credentials, password, password_is_ha1) "Digest " + credentials.sort_by {|x| x[0].to_s }.inject([]) {|a, v| a << "#{v[0]}='#{v[1]}'" }.join(', ') end @@ -267,7 +285,7 @@ module ActionController # Opaque based on digest of session_id def opaque(session_id) - Base64.encode64(::Digest::MD5::hexdigest(session_id)).gsub("\n", '') + Base64.encode64(::Digest::MD5::hexdigest(session_id.to_s)).gsub("\n", '') end end end diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index 91c7515..d776361 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -5,8 +5,9 @@ class HttpDigestAuthenticationTest < ActionController::TestCase before_filter :authenticate, :only => :index before_filter :authenticate_with_request, :only => :display - USERS = { 'lifo' => 'world', 'pretty' => 'please' } - + USERS = { 'lifo' => 'world', 'pretty' => 'please', + 'dhh' => ::Digest::MD5::hexdigest(["dhh","SuperSecret","secret"].join(":"))} + def index render :text => "Hello Secret" end @@ -126,11 +127,22 @@ class HttpDigestAuthenticationTest < ActionController::TestCase assert assigns(:logged_in) assert_equal 'Definitely Maybe', @response.body end + + test "authentication request with password stored as ha1 digest hash" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'dhh', + :password => ::Digest::MD5::hexdigest(["dhh","SuperSecret","secret"].join(":")), + :password_is_ha1 => true) + get :display + + assert_response :success + assert assigns(:logged_in) + assert_equal 'Definitely Maybe', @response.body + end private def encode_credentials(options) - options.reverse_merge!(:nc => "00000001", :cnonce => "0a4f113b") + options.reverse_merge!(:nc => "00000001", :cnonce => "0a4f113b", :password_is_ha1 => false) password = options.delete(:password) # Perform unauthenticated GET to retrieve digest parameters to use on subsequent request @@ -141,7 +153,7 @@ class HttpDigestAuthenticationTest < ActionController::TestCase credentials = decode_credentials(@response.headers['WWW-Authenticate']) credentials.merge!(options) credentials.reverse_merge!(:uri => "#{@request.env['REQUEST_URI']}") - ActionController::HttpAuthentication::Digest.encode_credentials("GET", credentials, password) + ActionController::HttpAuthentication::Digest.encode_credentials("GET", credentials, password, options[:password_is_ha1]) end def decode_credentials(header) -- 1.6.1.3 From c78f08f04be8778b1d645e2d0f941fa1bfe68246 Mon Sep 17 00:00:00 2001 From: Donald Parish Date: Mon, 23 Feb 2009 12:30:43 -0600 Subject: [PATCH 3/3] Use session_options[:secret] instead of session_id in nonce. If the session_id is used, and cookies are not supported by the HTTP client, like cURL without cookies, the digest auth will fail. This happens while testing with a browser or curl. The session-based nonce will fail as the session_id may be nil instead of "". Make tests pass Assumes that ActionController::Base.session_options[:secret] is available. I've found this to be true whether sessions are used or not. Added test for case where sessions are turned off for valid credentials --- .../lib/action_controller/http_authentication.rb | 36 +++++++++++++------ .../controller/http_digest_authentication_test.rb | 16 +++++++++ 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/actionpack/lib/action_controller/http_authentication.rb b/actionpack/lib/action_controller/http_authentication.rb index a66d4ee..6797397 100644 --- a/actionpack/lib/action_controller/http_authentication.rb +++ b/actionpack/lib/action_controller/http_authentication.rb @@ -190,7 +190,7 @@ module ActionController credentials = decode_credentials_header(request) valid_nonce = validate_nonce(request, credentials[:nonce]) - if valid_nonce && realm == credentials[:realm] && opaque(request.session.session_id) == credentials[:opaque] + if valid_nonce && realm == credentials[:realm] && opaque == credentials[:opaque] password = password_procedure.call(credentials[:username]) [true, false].any? do |password_is_ha1| @@ -231,8 +231,7 @@ module ActionController end def authentication_header(controller, realm) - session_id = controller.request.session.session_id - controller.headers["WWW-Authenticate"] = %(Digest realm="#{realm}", qop="auth", algorithm=MD5, nonce="#{nonce(session_id)}", opaque="#{opaque(session_id)}") + controller.headers["WWW-Authenticate"] = %(Digest realm="#{realm}", qop="auth", algorithm=MD5, nonce="#{nonce}", opaque="#{opaque}") end def authentication_request(controller, realm, message = nil) @@ -270,23 +269,36 @@ module ActionController # POST or PUT requests and a time-stamp for GET requests. For more details on the issues involved see Section 4 # of this document. # - # The nonce is opaque to the client. - def nonce(session_id, time = Time.now) + # The nonce is opaque to the client. Composed of Time, and hash of Time with secret + # key from the Rails session secret generated upon creation of project. Ensures + # the time cannot be modifed by client. + def nonce(time = Time.now) t = time.to_i - hashed = [t, session_id] + hashed = [t, secret_key] digest = ::Digest::MD5.hexdigest(hashed.join(":")) Base64.encode64("#{t}:#{digest}").gsub("\n", '') end - - def validate_nonce(request, value) + + # Might want a shorter timeout depending on whether the request + # is a PUT or POST, and if client is browser or web service. + # Can be much shorter if the Stale directive is implemented. This would + # allow a user to use new nonce without prompting user again for their + # username and password. + def validate_nonce(request, value, seconds_to_timeout=5*60) t = Base64.decode64(value).split(":").first.to_i - nonce(request.session.session_id, t) == value && (t - Time.now.to_i).abs <= 10 * 60 + nonce(t) == value && (t - Time.now.to_i).abs <= seconds_to_timeout end - # Opaque based on digest of session_id - def opaque(session_id) - Base64.encode64(::Digest::MD5::hexdigest(session_id.to_s)).gsub("\n", '') + # Opaque based on random generation - but changing each request? + def opaque() + ::Digest::MD5.hexdigest(secret_key) + end + + # Set in /initializers/session_store.rb, and loaded even if sessions are not in use. + def secret_key + ActionController::Base.session_options[:secret] end + end end end diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index d776361..e9e7cfb 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -107,6 +107,18 @@ class HttpDigestAuthenticationTest < ActionController::TestCase assert assigns(:logged_in) assert_equal 'Definitely Maybe', @response.body end + + test "authentication request with valid credential and nil session" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please') + + # session_id = "" in functional test, but is +nil+ in real life + @request.session.session_id = nil + get :display + + assert_response :success + assert assigns(:logged_in) + assert_equal 'Definitely Maybe', @response.body + end test "authentication request with request-uri that doesn't match credentials digest-uri" do @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please') @@ -145,6 +157,10 @@ class HttpDigestAuthenticationTest < ActionController::TestCase options.reverse_merge!(:nc => "00000001", :cnonce => "0a4f113b", :password_is_ha1 => false) password = options.delete(:password) + # Set in /initializers/session_store.rb. Used as secret in generating nonce + # to prevent tampering of timestamp + ActionController::Base.session_options[:secret] = "session_options_secret" + # Perform unauthenticated GET to retrieve digest parameters to use on subsequent request get :index -- 1.6.1.3