Add new explicit authentication-tokens that can be revoked

realulim commented 7 years ago

All token-based authentication schemes are vulnerable to CSRF (Cross Site Request Forgery) attacks. In the case of cookies there is an additional risk involved, because the browser sends the cookie automatically with every request. Thus if the user simply clicks on a malicious link during a CouchDB session, he will send the cookie to the attacker, who will then be able to take over the session.

This is a built-in problem with Cookie-based authentication schemes, but CouchDB makes it worse by not providing a way to delete the stateless token upon logout of the user. That means that the attacker can indefinitely use the captured cookie just by sending a ping every 10 minutes - the token will never expire.

I think this is a security issue and should be fixed. Anyone in possession of the stateless token should be able to delete it.

Expected Behavior

After calling the DELETE /_session endpoint the session should be invalidated on the server.

Current Behavior

The stateless token is not deleted (as per section 10.2.15 of the documentation).

Possible Solution

I have no suggestions, because I'm not sure what the problem is with deleting the stateless tokens. In case it has something to do with distribution, I would suggest that eventual consistency is good enough here. As well if it were possible to set the expiry date - a slightly more complicated way to get rid of the cookie, but it would suffice.

janl commented 7 years ago

I have no suggestions, because I'm not sure what the problem is with deleting the stateless tokens.

The problem is that the tokens are stateless. That means they are not stored on the server. That means they can not be deleted, because they don’t exist.

realulim commented 7 years ago

Ok, then I have misunderstood how the procedure works. I thought stateless meant that they are not related to a user session, i. e. the server does not keep state. So if the server does not store the tokens, an attacker can use them indefinitely? In terms of security they are equivalent to the username/password combination then?

realulim commented 7 years ago

Let me rephrase my last comment, it was written in too much of a hurry, for which I apologize.

What I didn't understand is that "stateless tokens" in CouchDB actually means more than simply not storing tokens. It means "no sessions". You could have sessions with stateless tokens (see my example below).

The point of token authentication in a general sense is that the user does not have to send his credentials with every request, so if a token is intercepted (CSRF, MITM, ...) the attacker has less than the credentials. "Less" means, for example, that once the user "logs out", all tokens become worthless. The point of this ticket was to suggest this functionality - a way to invalidate all tokens still out there.

However, the server would have to store at least the timestamp of the last "log out" operation, then it could refuse all tokens coming in after that timestamp. The next "log in" event (session creation) would then delete this timestamp. And session creation is only possible with the credentials, which the attacker doesn't have (I assume the credentials are hashed/salted).

A completely stateless solution is not possible, because all the token contains is the user's credentials and we cannot invalidate the credentials. But storing one additional timestamp per user would still be stateless in the sense that it doesn't change CouchDB's scaling properties. It is not a per-session value (which indeed would affect scalability), but a per-user value that could even be eventually consistent.

realulim commented 7 years ago

s/delete the timestamp/update the timestamp (so old tokens don't regain validity ;-)

realulim commented 7 years ago

I would assume that the user changing his password would invalidate all tokens still out there. But this is hardly practical and it doesn't help in case the user isn't aware that one of his session tokens was stolen. The security goal has to be that a user-facing application can be designed such that all tokens are invalidated as soon as the user explicitly logs out. Then the attack window is limited to session duration.

Feel free to shoot this down, if it has flaws:

The user authenticates with his credentials and we hand him a stateless token that includes a "now" timestamp. We store the "now" timestamp on the server.
Someone wants to authorise an operation with a token. If its timestamp is older than our now timestamp, we reject it.
The user logs out and our application sends an "invalidate" request to the server. The server updates the now timestamp and adds an "invalidated" flag to it.
All tokens coming in are now rejected because of the flag. We have succeeded in invalidating any tokens still out there.
The rightful user logs in again with his credentials. Upon verification we update the now timestamp and remove the "invalidated" flag. The user gets a fresh stateless token.
All other tokens are still invalid because they have an older timestamp.

Even though we store data on the server, this is still stateless in terms of scalability, because timestamp and invalidated flag are per-user (e. g. like the password) and not per-session. Plus, eventual consistency is good enough here.

realulim commented 6 years ago

Here's a good explanation of the various options for keeping session state on the client and the security hole introduced by stateless session tokens like the ones CouchDB uses: http://appsandsecurity.blogspot.de/2011/04/rest-and-stateless-session-ids.html

wohali commented 6 years ago

I haven't read or thought about your proposal, but anything that is backwards compatibility breaking couldn't possibly land until 3.0 at the earliest.

If you are concerned about this problem in your own deployments, you can use CouchDB's proxy authentication with a proxy server like Apache HTTP Server handling the auth (via something like OAuth 2.0 or SAML) and bypass the entire problem.

I know that some people were looking at alternatives to cookies for 3.0, i.e. something JWT-based perhaps. However, there are issues with them as well (1, 2).

realulim commented 6 years ago

I absolutely agree that JWT is not the way to go. Cookies are great, because they are immune to XSS attacks. We need to keep them around for XSS-immunity and in a web application can today already add a custom token (like JWT) for CSRF protection. Only that combination is secure.

Making CouchDB's cookies revokable does not have to backwards incompatible. You'll only add a new API endpoint for revocation and one internal timestamp per user. This makes CouchDB slightly more stateful in theory, but I don't think the effect on scalability is measurable.

janl commented 6 years ago

Renaming this as we are not likely to change the behaviour of this API. Instead, lets use this as a ticket to track a potential new feature: explicit tokens that can be generated. Think GitHub deploy tokens etc.

tabeth commented 5 years ago

Great discussion above, but I'd like to add one thing.

It isn't possible to manually revoke a stateless token, but it may be desirable to maintain a bit of state in an internal CouchDB document that will allow for @realulim 's request to be possible.

Imagine a _BLACKLIST document, which would be initially empty. Deleting a a _session would add the associated token in _BLACKLIST. With this, a request that requires authentication would simply check _BLACKLIST before proceeding (and of course return a 401 with a message to re-authenticate if blacklisted). What I'm proposing isn't really new and is effectively re-creating a session token, but in the context of CouchDB, this may be acceptable since CouchDB itself is stateful, and so adding a bit more state may be permissible.

Since CouchDB already maintains jobs for replication, a similar set of jobs could be maintained to delete expired tokens from the _BLACKLIST.

So all that would need to be done on the CouchDB side is just to look at this _BLACKLIST document before assuming a user is authenticated. This functionality could be turned off by default to preserve backwards compatibility.

I'd love to hear your thoughts @wohali @janl .

TLDR: A backwards compatible approach could be to add an internal document that's referenced when authenticated requests are received, before a response is sent. The nonexistence of a database (for older versions) would result in the same behavior as today. Deleting a session would simply add document to this database.

rnewson commented 5 years ago

The simplest way to implement a /_logout_all_sessions endpoint is as follows;

1) require the username and password in the request body (the request must prove knowledge of these values to proceed) 2) if user:pass is correct, generate a new random salt value and save updated password_sha, salt (or derived_key, salt if using pbkdf2)

All already issued session cookies are invalid, as the user salt is incorporated into them all. This is done so that a password change invalidates all session cookies, but it would be simple to reuse this mechanism for a forced logout feature.

@wohali @janl

rnewson commented 5 years ago

(we need user password in the request body in order to generate the new password_sha or derived_key, in case that wasn't obvious).

wohali commented 5 years ago

@rnewson that's a more intelligent way of doing it, yes. And it doesn't require adding new server-side storage to achieve it.

PR welcome :heart:

tabeth commented 5 years ago

@rnewson @wohali

I think the issue with that approach is that wouldn't be useful for the main use-case when there's been a breach, e.g. a stolen cookie. The administrator, you, potentially wouldn't know (or shouldn't know) the user's password so you would have no way of invalidating a user's token on behalf of the user.

After looking at couch_httpd_auth.erl I do see that the approach I suggested earlier would be quite a refactor, since you'd basically need to add that check I referred to earlier everywhere a session token could be input for authentication.

rnewson commented 5 years ago

In the event of a breach the remedy is to change the password, which also invalidates all previous session cookies. An administrator can change a password without knowing its current value, using their own credentials.

tabeth commented 5 years ago

That would work if you happened to know the user associated with the leaked token(s). My understanding of the HMAC used is that it can't be decrypted. @wohali , is it possible to derive the user from their session token and the couch_httpd_auth secret?

If that's the case then I think your solution (@rnewson) would be the easiest to implement. I'd also be willing to attempt adding said endpoint (I've been lurking for a while, meaning to contribute more).

rnewson commented 5 years ago

the username and timestamp are in plaintext (base64 encoded) in the session cookie (as couchdb needs to retrieve both items), the HMAC is irreversible but you don't need to.

tabeth commented 5 years ago

OK, just to put everything together @rnewson

You're saying:

If we POST for a new session token:

POST /_session HTTP/1.1
Accept: application/json
Content-Length: 24
Content-Type: application/x-www-form-urlencoded
Host: localhost:5984

name=root&password=relax

We'll get a response, containing the session token:

HTTP/1.1 200 OK
Cache-Control: must-revalidate
Content-Length: 43
Content-Type: application/json
Date: Mon, 03 Dec 2012 01:23:14 GMT
Server: CouchDB (Erlang/OTP)
Set-Cookie: AuthSession=cm9vdDo1MEJCRkYwMjq0LO0ylOIwShrgt8y-UkhI-c6BGw; Version=1; Path=/; HttpOnly

{"ok":true,"name":"root","roles":["_admin"]}

And so, if this AuthSession token, cm9vdDo1MEJCRkYwMjq0LO0ylOIwShrgt8y-UkhI-c6BGw, were to ever be compromised (let's just say it's accidentally sent in an email). Your solution would be to reset the user's password manually.

We don't know who the user is, but you've stated that we can get the username from the AuthSession token by base64 decoding it. When we do this, we decode the previous AuthSession token to:

root:520550C1:柪귒ㄥ【茗#

And there it is, root is colon separated from some random gibberish, likely related to the secret. Now that we know who the user is, we then would look-up their user in _users and reset their password using the method you described above.

Is that right?

rnewson commented 5 years ago

that's right. Note also that the cookie will expire if not used, so compromise via email is not super plausible. You don't need to decode the cookie to figure this out, do a GET /_session with the cookie and couchdb will tell you the username (and roles) if the cookie is still valid.

Other things to consider;

include the client ip in the hmac (but not in the cookie). When verifying, we know the client ip of that request and can include it in the hmac (much like we don't expose the salt value in the cookie).
revisit the auto-extension of session cookies. This is a very old decision and there's concern about what might break, but I think it's worth considering. It doesn't feel right for a session cookie to extend indefinitely, even if that is convenient. Cloudant has recently integrated with a different auth system and those tokens do not auto-extend (and they last only one hour).

the cookie is base64(username:timestamp:hmac)

tabeth commented 5 years ago

@rnewson A few things on this, after doing some experimentation:

Regarding recalculating the salt and expiring all previous sessions, the way to currently do this without having to modify any code would be to call _users and modify the password. By modifying the password, including modifying it with the same password, salt will be recalculated and therefore all previous sessions will become invalid. This has been tested using CouchDB 2.3

This obviously leads to a bigger question of how a logout would be realistically implemented. Based on what was just stated, you would logout by having the user confirm their password. This could be done by making a GET request _session and making sure you get {"ok": true} (to prevent users from logging out without the correct password).

The only question that remains is whether we'd want to support a logout scheme without the user putting in their password. I'm not a fan of this solution, but we could support logout by looking for a log_out: true key value pair in the user document, which could then be picked up in couch_users_db.erl to re-calculate the salt, and derived key.

save_doc(#doc{body={Body}} = Doc) ->
    %% Support both schemes to smooth migration from legacy scheme
    Scheme = config:get("couch_httpd_auth", "password_scheme", "pbkdf2"),
    case {couch_util:get_value(?PASSWORD, Body), Scheme} of
    {null, _} -> % server admins don't have a user-db password entry
        Doc;
    {undefined, _} ->
        case {couch_util:get_value(?LOGOUT, Body)} of
          {true} -> % Logout
              Iterations = list_to_integer(config:get("couch_httpd_auth", "iterations", "1000")),
              Salt = couch_uuids:random()
              DerivedKey = couch_passwords:pbkdf2("", Salt, Iterations),
              Body0 = ?replace(Body, ?SALT, Salt),
              Body1 = ?replace(Body, ?DERIVED_KEY, DerivedKey)
              Doc#doc{body={Body1}};
         {_} ->
             Doc;
         end.

Thanks for the tip on the cookie format! I think revisiting the auto-extension of cookies is good, but would be a pretty major change, no? I imagine it would involve effectively saying that a valid AuthSession and a username and password pair are no longer equivalent like they are now.

wohali commented 5 years ago

We'd still need the old password to logout, and we don't store that plaintext, so I don't see how a logout scheme without the user entering it can work.

Yes, we can't easily revisit how the cookie problem will be improved without making backward-incompatible changes. There was some traffic about this on the dev@couchdb.apache.org mailing list and it was decided not to make any changes until at least 3.0, if not later.

tabeth commented 5 years ago

@wohali

Yeah, right now you would need a password to logout. However if something similar to what I showed earlier were implemented a user with a valid session cookie would be able to modify their own user document with a logout or equivalent flag which could then change the derived key and salt, invalidating all sessions.

Then, upon logging in via _session they would receive a new, valid session token. I've tried the first part in Futon, but I'll need to make sure both that and the latter part I'm describing here works via just the API.

Depending on how far 3.0 is in terms of time line, it could be worth incorporating in the interim.

rnewson commented 5 years ago

@tabeth Wohali has already explained that we cannot change the derived key without the password, and we don't store the password, so the user would have to supply it to log out. Your scheme above will not work.

We document this at https://docs.couchdb.org/en/2.2.0/intro/security.html but I'd like to describe it here for clarity.

It's essential that this point is understood by everyone in the thread as it defines what can and cannot be done as a solution.

When a user is created;

the plaintext password is sent to couchdb.
couchdb generates a large random value which we call salt.
we calculate sha1(password + salt) or pbkdf2(password, salt, iterations)
this value is stored to the users doc, the plaintext password value is discarded.

Given the same inputs (password + salt) we can calculate the same output as step 3 and then compare it against what is stored. We cannot feasibly retrieve the password from what we store in the users doc.

You could, in an emergency, change the salt value of the _users doc to some new random value. This will invalidate all previous session cookies but obviously the user could then not acquire a new one as they wouldn't know the new password (and that value could not feasibly be calculated).

realulim commented 5 years ago

I've detailed a possible approach in my posting on October 10th, 2017. There may be holes in my scheme, so I'd welcome any comments. My idea involves keeping state on the server, but only per user, not per session. Also, eventual consistency suffices, so I see no issues with scalability and distribution and no changes to existing APIs are needed. Also I believe that the "invalidate" request does not need the user's password for authentication, it would be enough to use a valid stateless token. Sure, a hacker could "invalidate" with a stolen token, but he would actually be doing the user a favor, because he locks himself out that way.

rnewson commented 5 years ago

Ah, thank you, I was referring to more recent messages in this thread. The scheme near the top could work though I don't think we want to use wall clock time. Instead, we could store an epoch value in the _users doc, initialised to 0. When the logout endpoint is called, we'd increment that epoch. The epoch value would be included in the session cookie value (and covered by the HMAC). CouchDB would check that the epoch value in the cookie matches the current value in the _users doc.

Even more succinctly, we would separate the dual functionality of the 'salt' value into two distinct fields. We'd include the epoch instead of the salt value, and a password change would also increment the epoch value.

It doesn't have to be an incrementing integer, it could be a random string, it would depend on whether we wanted to record 'current epoch' and 'last logged out epoch' as you suggest earlier, and allow all cookies with an epoch greater than 'last logged out epoch' to work. I don't immediately see a reason to allow that.

One thing to note is that is still a per-user log out, and not a per-session log out. But the change I describe in this ticket is equivalent to the current semantics.

rnewson commented 5 years ago

by " is still a per-user log out, and not a per-session log out" I mean that we could not bump the epoch for a call to DELETE /_session as this would invalidate other sessions. We'd need a new endpoint that made the scope of the action clear, /_end_all_sessions or some such.

rnewson commented 5 years ago

thinking again, we wouldn't need to include the epoch in the cookie value, just in the HMAC, as long as we only cared for it not matching the stored epoch value in the _users doc. If we want to do the range thing (again, not sure we do), we'd need it in the cookie.

realulim commented 5 years ago

I was assuming that the epoch value (what I called a timestamp) would be updated every time the stateless token was refreshed. That could lead to a situation where newer tokens were already given out, but one of the nodes in the distributed system would still have the old epoch value stored for the user. But you would want requests against that node to succeed with a newer stateless token.

That way the per-user epoch value could be eventually consistent in order to not affect scalability.

tabeth commented 5 years ago

@rnewson

Wohali has already explained that we cannot change the derived key without the password

Really? A couple days ago I experimented with this by creating a session and was able to modify a user's derived_key, or anything in their document really, with just the session token (assuming they had access, of course).

Given the same inputs (password + salt) we can calculate the same output as step 3 and then compare it against what is stored. We cannot feasibly retrieve the password from what we store in the users doc.

You could, in an emergency, change the salt value of the _users doc to some new random value. This will invalidate all previous session cookies but obviously the user could then not acquire a new one as they wouldn't know the new password (and that value could not feasibly be calculated).

Yeah this meshes pretty much with what I discovered. I'd have to amend my earlier idea -- that would be more like a "deactivate and logout" function than simply "logout". A user could, today, "logout" and invalidate all of their sessions by attaining a valid session token and just modifying their salt and/or derived_key. This would also have the effect of preventing them from logging in again, which would make it more suitable for being a sort of "deactivate" function.

As far as the conversation above goes, why not do it on a per session basis? Isn't it pretty unusual in general for a client to be logged out on all clients when they log out on one? How would one implement a "remember me" or equivalent?

So to kind of summarize, right now we can:

Log out of all sessions (a user can re-save their password by entering it, invalidating all sessions)
"Deactivate" their account (a user can modify their salt and/or derived key, invalidating all sessions and preventing them from creating new ones)

We're still missing:

Log out of current session (e.g. invalidate specific sessions).
Log out of all sessions w/o password (AFAIK there's no way to do this without some changes).

At the moment I can't think of a way to do a per-session invalidation per the original scope of this issue without ultimately doing something that amounts to a blacklist.

wohali commented 5 years ago

Sure, you can modify the derived key, but without knowing the original (plaintext) password, you can't modify it to have the same password with a different salt. That's the point of password hash functions, they're one way :)

rnewson commented 5 years ago

Agree that 'log out all sessions' is unusual, Wohali and I are simply conveying the consequences of changing the salt value in the users doc.

The only way to invalidate individual sessions would be store some state on the server for each session, which is a big change and probably unwelcome to many (though it could be optional). It raises questions for clustering and we'd obviously need something to clean up expired session state completely (so it can't be a regular document unless we're going to use _purge for something this routine). Whether we record all sessions or just the ones that have been invalidated specifically doesn't alter the scope of the change much.

Perhaps a better path forward is to remove the auto-extension of session cookies. Given that session cookies already expire, through lack of use (or a password change), this is expected behaviour and is handled by the UI (by making you log in again) and the replicator (which can acquire a new cookie).

To compare, IBM Cloudant (disclaimer: I work for IBM), uses an alternate auth system that issues fixed-duration tokens that last one hour, so there's already a precedent.

It would also be important to return a 401 error when couchdb is passed an expired token rather than treating the request as one past without authentication, as anonymous writes might be allowed on the target (albeit perhaps restricted by a VDU) and this would lead to surprising behaviour for users and the replicator. That said, it would be a big improvement for the auth workflow, a 401 would ensure all tools re-acquire a cookie (by re-presenting credentials) exactly when needed.

rnewson commented 5 years ago

and, just to be clear, the 'log out all sessions' is scoped to the user, in case you thought I literally meant all sessions.

rnewson commented 5 years ago

I wrote some of these ideas up as code a while back so I might as well link to them here: https://github.com/apache/couchdb/compare/enhance-session-cookies?expand=1

tabeth commented 5 years ago

@wohali

Sure, you can modify the derived key, but without knowing the original (plaintext) password, you can't modify it to have the same password with a different salt. That's the point of password hash functions, they're one way :)

Right! I think I just misunderstood what you meant earlier. Sounds like we're in agreement now, haha.

@rnewson

Yeah I'm pretty new to CouchDB (about a couple weeks now) and wondered about the session token stuff. It seems like it would've been more ideal for session tokens to be such that:

_session allows you to specify the expiry of a given session (and consequently token) instead of that being done via configuration.
Usage of a token doesn't automatically extend itself by returning another.

Of course CouchDB is a huge project so I'm sure there are reasons it's the way it is now.

The code you linked seems to more or less implement what you described yesterday. I'm curious what's missing? Albeit I'm no erlang expert by any means and I'm not familiar with the CouchDB codebase.

Regarding what you just said too, do you see any disadvantages of just having an internal DB that has two keys, id, being the AuthSession token and expiry being the AuthSession expiration date? This would in let you just use regular CouchDB semantics, including using _purge to clean-up manually and a design doc to let you see specifically blacklisted tokens that haven't been expired, that way it would be easy for a user to clean-up expired docs.

Just sorta thinking out loud here. If the above is possible, then from what I can tell (still casually trying to see how CouchDB is written) all we'd have to do is just add something in the cookie_authentication_handler (https://github.com/apache/couchdb/blob/1347806d2feebce53325070b475f9e211d240ddf/src/couch/src/couch_httpd_auth.erl) to check to see if the token exists in this hypothetical blacklist database before finishing.

You could even allow users to opt-in by manually creating the database themselves, or even specify a database name and set the name of this database in the config. I'm kind of hung up on this approach manually because I still have a basic understanding of the code-base and am thinking in terms of what I could implement myself, if necessary.

Disadvantages of this approach would be:

Eventual consistency could be a problem. For example you could log-out and/or have a malicious session out there, put it on the blacklist, but it'll take too long for it to become consistent and the attacker already did whatever they needed to do.
Session information being aggregated in the same place where an attacker may have access may be counterproductive.
Making this code change would now make couch_httpd_auth necessarily aware of this database used for blacklisting. This may not be desirable.

wohali commented 5 years ago

@rnewson There was vocal objection to removing the auto-extension of cookies on the ML, by Jan if I recall correctly.

Reminder: As a major API change, this discussion should be happening on dev@, not here.

rnewson commented 5 years ago

@tabeth my concern for server-side session persistence is the high volume of operations, as every user would be doing this. The current design has very nice properties.

@wohali agreed this should be on dev@ and I'll maybe start a thread there. I did see @janl's objection but I don't believe we've gone deep enough to say that constitutes a veto. The current session scheme is a balancing act between Web expectations (I stayed logged into a website until I log out) and database expectations (a session ends irrevocably when I end it), like many other choices in couchdb.

realulim commented 5 years ago

As a reminder, I have opened this issue as a security problem. I did not and do not see this as balancing user expectations against each other, but as a security hole that needs to be fixed.

I do realise that wohali removed the security tagline, but if I'm not mistaken then no reason has been given for that and the exploitation scenarios I have linked to have not been discussed.

wohali commented 5 years ago

@realulim We reserve the security tag for things that are exploits, not "works as designed." :)

rnewson commented 5 years ago

@wohali where is that policy written down? I think it's right to call this a security related issue (it clearly is security related), even if it works as designed (we can disagree on whether the design is good or secure) and even if we (PMC) are deciding not to address it.

rnewson commented 5 years ago

@tabeth If you wish to continue this, I suggest you start a new thread on the couchdb-dev mailing list, which I think has a larger reach anyway.

janl commented 5 years ago

FWIW, I think we can design a new feature here. if we change any existing API’s, we need to run an email + lazy consensus by dev@, but that’s it in terms of process.

What would work best is:

agree on semantics for a new session endpoint
have a PR that adds that feature
ship a 2.x release where people can opt into that new behaviour
when 3.x comes out, make the new behaviour the new default and optionally, let folks opt into 2.x and earlier behaviour.
in 4.x then, we can remove the old behaviour, if we want

realulim commented 5 years ago

Sorry if everyone has understood and disregarded the security implications and I am beating a dead horse here. But I'm sure we all agree that in security matters it is better to make sure twice and thrice, so let me re-iterate the problem:

The exploit is that a stateless token, once stolen, can be used indefinitely by the attacker.

The point of using tokens (whether stateless or not) is that the sensitive password does not have to be transmitted with every request. If, however, tokens have the same longevity as passwords, then this purpose is defeated and you might as well make it easy on yourself and re-transmit the password every time.

Please note that I am talking about interactive applications here. The user logs out of his application and expects that everything is nice and secure. He most certainly does not expect that stateless tokens flying around can still be used and even refreshed indefinitely.

For non-interactive applications (such as SSO), it is understood and expected that a combination of short-lived access tokens and long-lived refresh tokens are employed.

Again, there is no need to change existing APIs. As Jan said, there should be a way to invalidate tokens without changing the password, so that developers can employ this method in their application, when their users are "logging out".

janl commented 5 years ago

@realulim I consider indefinite auto-renewal an API semantic, that if changed is a breaking change, so we can’t just add this.

There is no disagreement that this is a security concern, but there are worst-case-scenario failsafes available already, and I think that’s why we’re not treating this as an urgent issue (i.e change the http session secret).

You call this exploit and I’m sure you are using this terminology to install a sense of severity and urgency. I call this works as designed and fully acknowledge the design limitation.

The way forward I outlined in my previous message, so let’s focus on agreeing what the precise semantics for a new _session endpoint should be, or decide that @rnewson’s patch does the trick.

rnewson commented 5 years ago

Let's be clear;

"The exploit is that a stateless token, once stolen, can be used indefinitely by the attacker."

This is not the complete story, here's my attempt;

"The exploit is that a stateless token, once stolen, can be used by the attacker until the password of that user is changed and as long as the attacker continues to make frequent use of the token, replacing it when sent a new one."

any specific cookie can only be used until it expires, it requires further contact with couchdb to get a new cookie to replace the expired one.

As for the solution, I agree we should design it carefully.

To the idea that removing auto-extension is an API incompatible change, I'm not yet convinced. Session cookies certainly can expire today and everything that acquires them is expected to handle that situation. Specifically, clients are required to still retain, or be able to obtain, the username/password in case they need to request a new session cookie. Any tool that is solely depending on a "keep alive" approach to the session cookie is broken and we should not feel bound to support them.

To current remedies, there are two we can recommend;

1) change the salt value of the affected user. Invalidates all session cookies for that specific user. 2) change the couch_httpd secret value used to sign all session cookies. Invalidates all session cookies issued by that node. (nb: change this on all nodes of your cluster and to the same value).

rnewson commented 5 years ago

https://github.com/apache/couchdb/compare/hard-fail-cookie-mode

This was my first attempt at this a while ago and could be made simpler. In this version, it's optional and the replicator opts into it, specifically to get a 401 to force it to refresh the cookie rather than fallback to anonymous writes (if allowed).

tabeth commented 5 years ago

@rnewson

I will. I'll also try to summarize briefly our findings here when I start that thread.

@realulim

Again, there is no need to change existing APIs. As Jan said, there should be a way to invalidate tokens without changing the password, so that developers can employ this method in their application, when their users are "logging out".

FYI it's possible to do this now by having the user provide their password during a "logout" step. By resaving their password, the salt and/or derived key will be regenerated and their sessions will be invalidated. This isn't ideal in all situations (especially non-critical ones, where it's as hassle for the user).

The exploit is that a stateless token, once stolen, can be used indefinitely by the attacker.

I don't disagree, but it's worth elaborating here. Assuming tokens are not set to be persistent (in which case doing what I described above is the only option anyway), there are effectively three paths:

Path 1

Imagine a user's usage history looks something like this:

GET _session (Username, Password) -> Cookie1 (Expiry at +10 minutes)
GET _db (Cookie1) -> Cookie2 (Expiry at +11 minutes)
GET _db/1 (Cookie 1) -> Cookie3 (Expiry at +12 minutes)
GET _db/2 (Cookie 1) -> Cooke4 (Expiry at +13 minutes)

As you can see, using our original cookie, Cookie1, through completely normal usage we've incidentally created 3 other cookies, Cookie2, Cookie3, and Cookie4. In a normal application, despite the generation of the new cookies, Cookie1 would be the one used until near expiration, in which case a new cookie would be generated. A "soft" logout, that only invalidates a specific cookie would just be to blacklist Cookie1 until expiry.

Path 2

Using the same usage history as above, you could imagine that a malicious hacker somehow gets access to Cookie2, Cookie3, or Cookie4 and uses that to generate new cookies indefinitely (perhaps by just requesting access to _db in this case.

Assuming the hacker did this without a trace (meaning we don't know how they got the cookies), their behavior would be indistinguishable from our user's. If all we had was a hunch all we could do is just invalidate all of the sessions for the user. After this we'd have to send them a temporary password to use to access their account.

Path 3

This would be the exact scenario as Path 2, however instead of invalidating all of the sessions for the user, we would just ask them to "login", and on the client side we would actually log them out and immediately log them in, locking out the hacker and giving them access again. The main reason to do this is so we don't have to send them a new password.

In other words, if we suspect Path 2 is occurring, we set a flag and the next time they log in, on the client side we implement it as a "log out" (e.g. update their user document with the same password to regenerate salt) followed immediately by "log in".

In my opinion the security concern you point to can already be addressed. The goal, however would be to implement a "soft logout" via a blacklist, so that when users logout their tokens are explicitly logged out. This, combined with a sufficiently short expiry time, e.g. 10 minutes, would address 99.9% of the security concerns.

If you consider the situation proposed in Path 1, if there was a blacklist, and a user "logged out" after the last item in their history, a hacker would only have a 10 minute window (best case) to grab either Cookie2, Cookie3, or Cookie4, the only 4 cookies that haven't explicitly been blacklisted. After this window, Cookie1 would not work due to the blacklist and due to expiring, and the rest of them would have expired. A hacker would have no cookie to used and all is well.

The main wild card is that in a high volume situation replication could take too long. But as long as replication time is less than the time it takes for the hacker to find token it's fine.

rnewson commented 5 years ago

Any cookie-aware application (e.g, all web browsers) will indeed replace their cookie whenever couchdb issues a new one (via a Set-Cookie response header).

You elide how a cookie might leak, and it's important to note that browsers take care not to allow cookies to be exposed in general. CouchDB also sends the http only flag which tells the browsers not to allow any interrogation of the cookie via javascript. I'd be grateful for at least one plausible way for the cookie to leak.

The wildcard thing seems a non-starter given the volume of data required to hold, unless we do the epoch / timestamp idea instead, which has the side-effect of invalidating all sessions for the user in exchange for only requiring us to store a single value per user.

Nothing presented here so far appears better than simply removing the auto-extension piece of code (in 3.0 for semver reasons).

tabeth commented 5 years ago

My main concern with the timestamp idea is that it implicitly rejects the idea that a user would have multiple sessions. I think there are many reasonable situations where a user would want to have multiple sessions (ideally with multiple expiry times). In lieu of this, the only solution is to explicitly invalidate specific sessions. It may be possible to do this now with through the usage of _session and a _user document, but I haven't given it much thought yet.

Of course, the CouchDB team could just say that's not something they'll support. If that's the case, the timestamp idea is the best one.

I'm not sure relying on httponly isn't really a good idea, since not all browsers restrict it equally, anyway. From a security standpoint, I'd say it's best to assume if there's one browser that could read an HttpOnly cookie, a hacker would be using that browser to get the cookie's contents. That being said, I'm pretty sure all major browser's most latest versions restrict reading and writing with HttpOnly, and when combined with the Secure flag there's no way to get the cookie's contents.

rnewson commented 5 years ago

I've taken it as read that the couchdb server is protected by TLS, otherwise the actual user/pass can be trivially captured anyway, making anything we do with sessions moot. if couchdb itself is doing the TLS bit (rather than, say, haproxy), it should add the Secure flag also.

realulim commented 5 years ago

Logging out with password is a non-starter for user facing applications. This will lead to applications keeping the user's password in memory (or worse, local storage) after login and then re-using it for logout.

@janl I haven't advocated to change any existing API or semantics. I have actually said that the current scheme is necessary and expected behavior for non-interactive applications. My argument is that interactive applications (where users log in and out manually) need a different approach to security and thus a new API. This new API could be designed in a way that logout means logged out until reauthenticated.

For a reality check (which we all need at times), are there any well-known interactive applications out that use CouchDB and expose its authentication scheme to the user? If yes, then it would be interesting to get their take on this issue. Maybe I'm the only developer using CouchDB in that way and/or overly paranoid.

apache / couchdb