This project is archived and is in readonly mode.

#2000 ✓duplicate
Don Parish

Patch for HTTP Digest Authentication URI comparison

Reported by Don Parish | February 17th, 2009 @ 07:09 PM | in 2.x

This patch better implements the fix I made in #1848.

RFC 2617, Section 3.2.2.5, says that the authenticating server must ensure the document designated by the uri parameter is the same as the document served. To do this, the uri in the credentials should be compared to the uri in the request line.

This patch does that. The prior implementation before my first patch, passed in the full URL instead of the URI, that's why the tests passed, but it failed with most browsers which send a uri without the full URL.

I've added two tests. One that makes sure the uris in the request and credentials match, whether using a url or relative uri. The other test checks to see that the authorization will fail if the header uri is changed.

Comments and changes to this ticket

  • Don Parish
  • Don Parish

    Don Parish February 18th, 2009 @ 12:56 PM

    Attached patch includes changes from patch above (http_digest_authentication_fix_uri.diff) and adds support for using a hashed digest of the credentials instead of storing a plain text password. The format is the ha1 hash which is the MD5(user:realm:password), the same way Apache stores digest credentials. Protects the user password from use in other applications, although having the ha1 hash is enough to allow someone to authenticate.

  • Gregg Kellogg

    Gregg Kellogg February 18th, 2009 @ 03:48 PM

    Definitely like the ability to use ha1 for verification vs. a clear-text password. However, I think we need to make a variant of the ha1 method public, so that users can use it when initially storing the password. The password_procedure block should also pass the realm for completeness; best not to assume it's an application global.

  • Don Parish

    Don Parish February 19th, 2009 @ 07:21 PM

    Gregg, not sure if it is necessary to make ha1 method public as the code is just a one-liner. I added it just to make my line of code shorter and easier to follow.

    I took a look at passing the realm into the block, but it's not necessary since the realm is already a parameter of the authenticate method:

    Please take a look at my code. I've had an interest in using Digest and want to help make it a good implementation for Rails. My last change kept a plain text option for storing the passwords, but I'm not sure that is good idea to even have the option. Maybe it needs to be an option that has to be explicitly turned on.

    My time is limited, but I'd like to look at:

    • adding support for auth-int
    • using a nonce and opaque that don't rely on having sessions turned on
  • Don Parish

    Don Parish February 19th, 2009 @ 07:24 PM

    Here is code and gist for last comment:

    http://gist.github.com/67053

      ADMIN_REALM = "Admin Realm"
      USERS_REALM = "Users Realm"
    
      REALM_USERS = {
        ADMIN_REALM => {
          "dhh" => "as",
          "don" => "ap"},
        USERS_REALM => {
          "dhh" => "us",
          "don" => "up"}
      }
    
      def authenticate
        authenticate_or_request_with_http_digest(USERS_REALM) do |username|
          REALM_USERS[USERS_REALM][username]
        end
      end
    
    
  • Gregg Kellogg

    Gregg Kellogg February 19th, 2009 @ 08:56 PM

    Without a session_id, something else must be used on which to base the creation of the nonce. The OAuth plugin creates a DB table to track nonces, which is too expensive, in my mind. Using the client IP address is a possibility, but it can change for mobile clients and can be a problem with proxies. This might best be left as a policy that can be implemented by the application, defaulting to the session_id-based nonce if not overridden. This would require overriding nonce and validate_nonce in the ApplicationController (and moving the methods into this scope), or specifying callback routines.

    I think that providing an encoding function (ha1) is preferable to specifying an algorithm for an application to implement. This can be as easy as creating an http_digest_encode_password method in ControllerMethods.

    I agree with eliminating the clear password use case.

    Thanks for the work on improving this implementation.

  • Don Parish

    Don Parish February 23rd, 2009 @ 06:53 PM

    Updated patch from February 18th, 2009 @ 12:56 PM with minor correction to documentation to require 'md5'.

  • Don Parish

    Don Parish February 23rd, 2009 @ 07:07 PM

    Gregg,

    Attached is a diff that can implement Digest auth without sessions having to be turned on. I've tested it with curl on Win32. It used the session secret created by the Rails generator, so this will work as long as it is not deleted. What do you think?

    Is it necessary for the opaque to change for each client? The RFC say the opaque can be used to pass session-type information, but the requirement is just that they match.

    I put the nonce timeout in as a parameter. I think it could be reduced to seconds, if the Stale directive of the RFC was implemented to allow the client to use the new given nonce without having to type in the username and password again.

    Thanks,

    Don

  • Gregg Kellogg

    Gregg Kellogg February 25th, 2009 @ 07:24 AM

    Looking within ActionController, it looks like the session secret is an attribute of the CookieSessionStore, and not included in other session stores (e.g., memcache). Also, the session secret is a global for the site, and subject to greater potential for attack than something that's generated on a per-connection basis.

    I like the direction you're going in, but it looks like dependencies are spreading a little wide. If we want to exclude needing a session, then we're really only left with information from the HTTP header, along with site-global information. Perhaps hashing the HOST_ADDR with something like the session secret would work. This would require promoting session secret from the cookie store to the abstract store, but shouldn't be an issue, as the option gets set in boilerplate environment code.

    I'd certainly like to hear the opinion of some core members.

  • Don Parish

    Don Parish February 25th, 2009 @ 01:26 PM

    As long as the session secret is available in the session_store.rb initializer, it is available as:

    ActionController::Base.session_options[:secret]
    
    

    even if the session store is set to ActiveRecord::SessionStore. You can try this on the Rails console:

    I'm using a secret key to use the suggestion in RFC3617

    A nonce might, for example, be constructed as the base 64 encoding of

    time-stamp H(time-stamp ":" ETag ":" private-key)

    where ... private-key is data known only to the server.

    I see your point about dependencies getting spread thin, but I Digest auth should work out of the box, and the session secret should be there for each newly generated Rails app. I need to make this clear in the docs.

    We could try adding in the ETAG to add additional security to the nonce?

    In any case, I hope we agree that we don't want the digest auth to be dependent on sessions being turned on. With current code, a sessionless client, such as Curl will fail on authorizing You could turn on cookie support, but you shouldn't have to, as HTTP should generally be stateless.

    Testing Session Secret with Different Session Stores:

        >> ActionController::Base.session_store
        => ActionController::Session::CookieStore
    
        >> ActionController::Base.session_options[:secret]
        => "bbbaf7c139ad50d759d...387a2306f2b500997e3"
    
        $ rake db:sessions:create
        $ rake db:migrate
    
        >> ActionController::Base.session_store
        => ActiveRecord::SessionStore
    
        >> ActionController::Base.session_options[:secret]
        => "bbbaf7c139ad50d759d...387a2306f2b500997e3"
    
  • Gregg Kellogg

    Gregg Kellogg February 25th, 2009 @ 04:02 PM

    I think using ETag is problematic: We'd have to avoid header calculation and authentication detection until after the ETag's been generated: i.e., in an after_filter. Also, we'd need to be sure that the ETag was generated identically on both the 401 response and the subsequent authenticated response. This might not happen, for instance, if a timestamp or other volatile information is included in the response body used to generate the ETag.

    I think all we're left with is using the session secret. This is another reason to promote nonce to ActionController::HttpAuthentication::Digest: it allows an application to make a more secure nonce using application-specific knowledge.

  • Don Parish

    Don Parish March 11th, 2009 @ 06:54 AM

    I've replaced with patch with #2209. It incorporates the fixes mentioned here. I believe the current implementation will not work in a controller action that does not reference sessions nor from a session less connection via curl.

  • Pratik

    Pratik March 12th, 2009 @ 01:22 PM

    • State changed from “new” to “duplicate”
  • Jeff Kreeftmeijer

    Jeff Kreeftmeijer November 8th, 2010 @ 08:25 AM

    • Tag cleared.

    Automatic cleanup of spam.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages