Closed Bug 666237 Opened 13 years ago Closed 12 years ago

cache authentication for n seconds

Categories

(Cloud Services :: Server: Core, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: tarek, Assigned: ygjb)

References

Details

(Whiteboard: [needs loadtest][qa+])

Attachments

(1 file)

In order to reduce the ldap load, we're adding a cache for the authentication that will last for 60 seconds. This is dramatically reduce the number of calls made on the ldap server, in particular on the initial sync.

Also, a password change API will purge it.

=> needs a security audit
Adding yvan for the sec review
Whiteboard: [pending secreview]
So the plan is to store a hash of (user name + password) into memcache as a key, and its id as the value.

the authentication function in our ldap backend becomes roughly:

def authenticate(user, pass):
    id = memcache,get(hash(user+pass))
    if id is found:
        return id
    id = get_id(user+pass)   # that authenticates in ldap. it's the original code
    memcache.set(hash(user+pass), id, time=60 seconds)
    return id

Specifics:

- the cache is shared by all webheads (because of the round robin load balancing) so probably membased
- the cache is purged when the password is changed via an API
- the hash is a sha1 (seems to be a good compromise speed/security)

Assumptions: The benefit expected is to switch to memcached for most authentication calls and thus avoiding ldap round trips and overload as it's easier for our infra to scale memcached (and memcached roundtrip are faster and won't require an auth) -- but we need to load test this !
Assignee: nobody → yboily
The biggest concern I would have here is ensuring that there is no simple mechanism to determine the usernames and passwords.  Using a basic hashing algorithm is a good start, but I would suggest using an HMAC using a key that is shared across all of the webheads. 

def authenticate(user, pass):
    hash = hmac.new(secretserverkey, user+pass, hashlib.sha1).hexdigest()
    id = memcache.get(hash)
    if id is found:
        return id
    id = get_id(user+pass)   # that authenticates in ldap. it's the original code
    memcache.set(hash, id, time=60 seconds)
    return id
If we're going to 60-second timeouts, purging the memcache on a password change is probably not worth the effort. As long as we hash the originating IP, they'll simply have a false positive from their IP for 60 seconds. 

This would actually be kind of compelling with a 25-hour timeout, but then we would need to work out a purging process.
Has this been implemented?  Is it still being planned?
This is an experimental implementation to try it out on our loadtest environment.

It will cache the auth using a local memcached at 127.0.0.1:11211

We should see a better qps with this patch, since it'll skip some ldap hits as long as our load tests are reusing the same user ids of course. 

If it's the case, I'll take the patch to phase 2 (tests + configurable + skippable)
Attachment #548720 - Flags: feedback?(petef)
Blocks: 676423
Pete, can we work on this this week ? the patch is getting old and deprecated a bit
Whiteboard: [pending secreview] → [pending secreview][needs loadtest]
Since we now have support for repoze.who in server-core from Bug 692709, this looks like an ideal pieces of functionality to factor out into a stand-alone plugin.  I've taken the ideas from Tarek's code and made it into a "memcached" plugin here:

    https://github.com/mozilla-services/repoze.who.plugins.memcached

If this seems like a good way to go, I can work up a configuration file that will load this for testing purposes.
Whiteboard: [pending secreview][needs loadtest] → [pending secreview][needs loadtest][qa+]
we probably don't need this, since browserid support is coming, right?
Agree with pete. Good idea at the time, supplanted by other forces.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
Whiteboard: [pending secreview][needs loadtest][qa+] → [needs loadtest][qa+]
Attachment #548720 - Flags: feedback?(petef)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: