Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  Don Parish February 18th, 2009 @ 05:16 AM

  Updated patch to consolidate commits.

  • http_digest_authentication_fix_uri.diff 3.6 KB

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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

• You flagged this item as spam.
  

  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
  
  

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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'.

  • http_digest_auth_with_hashed_password_2.diff 12.4 KB

• You flagged this item as spam.
  

  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

  • sessionless.diff 3.5 KB

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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"
  

• You flagged this item as spam.
  

  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.

• You flagged this item as spam.
  

  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 March 12th, 2009 @ 01:22 PM

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

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

  • Tag cleared.

  Automatic cleanup of spam.