Skip to content
This repository has been archived by the owner on Mar 22, 2022. It is now read-only.

[Security] JsonWebToken Lifecycle Concerns; Set HttpOnly = true in JWT cookie #132

Closed
petermikitsh opened this issue Mar 26, 2016 · 51 comments
Labels
Milestone

Comments

@petermikitsh
Copy link
Contributor

Currently the JsonWebToken ("JWT") is returned without the HttpOnly flag. Consequently, client side script can access the JWT. This introduces a theoretical XSS vulnerability.

Consider the security implication of allowing client side access to the JWT in this scenario:


A bad actor somehow injects a script a client-side script into your app (how it got in could be a wide range of possibilities, given the flexible nature of feathers framework. e.g., somehow some bad client JS code ends up being inserted in a database, and is now served in all client requests). In the script, the hacker inspects localStorage, read the feathers-jwt cookie (if httpOnly is false), and send a POST request to www.badactor.com/xss, and now have a copy of the end user's JWT. The hacker can then issue requests to the application server, authenticated as the end user.


This example illustrates the broader concern of authentication architecture within Feathers, specifically the lifecycle of the JSON Web Token. It's critical to be highly specific in the implementation to mitigate certain attack vectors. JWT's are permitted to be transported and stored through a number of options in feathers, e.g., headers, cookies, query parameters, localStorage, etc. A unique set of security concerns are introduced to each application. The documentation does address the tradeoffs of the transports.

If you set HttpOnly to true, and enforce an architectural pattern of limiting the JWT to cookies, you can effectively mitigate XSS. If the JWT is placed in localStorage, the XSS vector isn't mitigated because localStorage can be read by client side script (which is kind of the point of localStorage, making it a less than ideal place for JWT's to live).

@ekryski ekryski added this to the 0.7 milestone Mar 26, 2016
@marshallswain
Copy link
Member

Doesn't putting the token in a cookie actually make JWT less secure? Cookies are automatically sent with every request, so now we're back to being vulnerable to the attacker not needing to even get access to the JWT token in order to make API calls.

@daffl
Copy link
Member

daffl commented Mar 26, 2016

The cookie value will only be set once when loading the app after a login and I'm not sure it can be HTTP only because the JS app needs to access it in order to make requests using the JWT. We decided that sending it in the cookie is still safer than the current general practice of putting it in the callback query string. I am also not sure if there is any place other than LocalStorage where the JWT could be stored.

An alternative would be as you suggested to create an HTTP only cookie value for the JWT with the same expiry as the JWT (sounds almost like a session to me ;). This should work in pretty much any browser environment and for non-browser environment we are not doing the redirect anymore so I think this is the best way to go now.

@marshallswain
Copy link
Member

(sounds almost like a session to me ;)

Exactly my thoughts.

@daffl
Copy link
Member

daffl commented Mar 26, 2016

@marshallswain Well basically we are trading sending the token in the Authorization header for putting it in the Cookie header. So there really isn't that much of a difference except for that the browser manages HTTP only cookies (and sending them in the header) more securely and you can't access them through JS (so you already prevent XSS attacks).

@marshallswain
Copy link
Member

It seems I remember reading that Auth0's solution, when this kind of problem is a concern, is to keep the token life/expiry short and use refresh tokens.

EDIT: If there's a rogue script on the page, short-lived tokens don't matter.

@marshallswain
Copy link
Member

A bad actor somehow injects a script a client-side script into your app

It seems like this would be a more important focus. If you get a script on your page, especially if it's targeted, you no longer own your JWT (unless you're not using it on the client and you're only using an HttpOnly cookie for server side rendering, which kinda defeats the purpose. Again we're back to sessions).

@marshallswain
Copy link
Member

I'm hoping I'm not coming across as mean, here. You can never review security policy enough, so this is super important discussion.

@petermikitsh
Copy link
Contributor Author

@marshallswain Not at all! I think everyone in here is valuing the necessity of the conversation.

CSRF tokens are the solution to the concern in your first comment in this thread, and you'd need to use them in conjunction with the HttpOnly cookie.

Regarding inject scripts: With feathers, the vector for injecting script is quite large since feathers isn't all that opinionated about your stack. Feathers supports many ORM's, and any front end framework. In a more concrete case, suppose I'm using MySQL with a Sequelize ORM and React for my presentation layer. I suppose we'd have to assume that my ORM is sanitizing input and removing scripts (I don't believe it does), and we're assuming React won't inject scripts into the DOM (it won't). I will admit, the possibility for injecting scripts is low. But you're essentially saying, we're leaving it to our ORM's and to our client side frameworks to secure our JWT. What I'm suggesting is, let's assume someone can inject malicious client side code and your JWT is still safe in that exceptional circumstance.

@ekryski ekryski modified the milestones: 0.8, 0.7 Mar 26, 2016
@ekryski
Copy link
Member

ekryski commented Mar 26, 2016

Hmm. Okay. I'm going to have to think about this a bit. As @daffl mentioned we do actually use a script to pick up the JWT from the cookie and set it in localStorage (especially in the case of OAuth). I only have one possible solution involving websockets, other than the query string method which is even less secure, that might work.

Short of supporting shorter lived tokens and/or the ability to revoke tokens this might be something that we just need to document as making sure that you are not allowing XSS.

@daffl I don't think I followed what you were proposing for a solution.

@petermikitsh
Copy link
Contributor Author

Upon doing some research, HttpOnly cookies should be fine for websockets.

From http://tools.ietf.org/html/draft-hixie-thewebsocketprotocol-74:

   9.   If the client has any cookies that would be relevant to a
        resource accessed over HTTP, if /secure/ is false, or HTTPS, if
        it is true, on host /host/, port /port/, with /resource name/ as
        the path (and possibly query parameters), then HTTP headers that
        would be appropriate for that information should be sent at this
        point. This includes "HttpOnly" cookies (cookies with the http-only-
        flag set to true); the Web Socket protocol is not considered a
        non-HTTP API.

If this is the case, is it necessary to place the JWT in localStorage?

@ekryski
Copy link
Member

ekryski commented Mar 26, 2016

@petermikitsh we store the JWT in localStorage so that you don't have to get the user to authenticate every time you load the page if you already have a valid auth token. Sort of like a "remember me" cookie. If you are concerned about that and are using sockets then you can just set a short TTL on your auth token. The socket will stay authenticated until it is disconnected or unauthenticated on either the server side or the client side.

The cookie really only gets used in the case of OAuth, because with OAuth you have a series of redirects before you know whether you are authenticated and your only option of getting the token back to the client is to send it in the query string to the final redirect, or send it in the cookie. This is where I have one other idea using sockets to send back the token securely. Using HTTPS, sending it in the cookie is much more secure, since URLs can be logged by any intermediate agent regardless of whether you are using HTTPS. The cookie does also get used with local auth if you are just doing form posts but really I recommend using Ajax or sockets to authenticate and then the cookie is never sent.

Frankly, now that I've had time to think about this I would argue that even if you don't use the cookie and are running JS in your page then you are susceptible to XSS attacks. If using sockets, even though the token is passed securely via messages using HTTPS you could have a malicious script listening for the authenticated method and then log the JWT somewhere for use later.

The only way I see this really being prevented it twofold:

  1. Make sure you can't have a malicious script on your page (that's sort of up to the developer imho and what you should get paid the big bucks for)
  2. Use shorter lived tokens with the ability to revoke them (https://github.com/feathersjs/feathers-authentication/issues/73), as well as when a user logs out their token is revoked (Calling logout should revoke/blacklist a JWT #133).

@ekryski
Copy link
Member

ekryski commented Mar 26, 2016

To follow this up, the fine people at Auth0 have addressed the XSS concern quite well imho and I entirely agree. If you are susceptible to a XSS attack then it's pretty much open season in any app. You could log passwords, credentials, etc. None of that is really all that specific to Feathers and is something that you need to be aware of and protect against in any app. See:

One thing we may want to do though, is have a built in hook that runs on every POST, PATCH, PUT request and escapes script code rather than relying on making sure the ORM or the developer handle it.

@marshallswain
Copy link
Member

@petermikitsh Regarding this comment:

What I'm suggesting is, let's assume someone can inject malicious client side code and your JWT is still safe in that exceptional circumstance.

Since the token needs to be passed with every request (with the exception of sockets, maybe), and since it's possible for a script to intercept and analyze the contents of every request, there's not really a way to prevent the JWT from getting stolen once there is a rogue script on your page, unless you're not using a client-side framework at all. It's always going to be a problem unless there's a way to prevent native objects from being overwritten.

@marshallswain
Copy link
Member

One thing we may want to do though, is have a built in hook that runs on every POST, PATCH, PUT request and escapes script code rather than relying on making sure the ORM or the developer handle it.

@ekryski Good idea. I made an issue in the main repo.

@petermikitsh
Copy link
Contributor Author

@ekryski Thanks for the links to auth0, good discussion there. Implementing short-lived tokens with token revocation functionality, while allowing the token to remain in localStorage, may be the best balance of concerns. Kept in localStorage, you also don't need implement CSRF protection either, which is a nice plus.

@marshallswain I believe the JWT cookie (once authenticated) would be passed with every HTTP request, and based off reading the spec, that would hold for websockets as well. Client side scripts (malicious or otherwise) wouldn't be able to intercept or analyze HTTP-Only cookies, as they can only be issued and read by the server. Here's a write-up on that. Since it sounds like we're sticking with localStorage, that's a moot point.

@ekryski
Copy link
Member

ekryski commented Mar 27, 2016

I believe the JWT cookie (once authenticated) would be passed with every HTTP request, and based off reading the spec, that would hold for websockets as well.

@petermikitsh we could set the cookie's TTL to instant expiration so then it won't get passed. For websockets it would only happen if the cookie is alive and whenever a new socket connection is made. I think we've come up with a good plan here. I'll see what I can do...

@daffl
Copy link
Member

daffl commented Mar 27, 2016

I'm still thinking that one actionable item here would be to change the JWT cookie value to HTTP only and the server also reading that value additionally to the Authorization header. Even though it sounds a little bit like a session we're just trading one header for another (that is securely managed by the browser instead of the application). This should work in any browser environment and also be backwards compatible.

@marshallswain
Copy link
Member

While I do agree that it's making the token more secure to use a cookie, it would make our apps less secure in general. I really don't like the cookie idea.

If you have a cookie set in your browser, now a malicious script on any other website can make calls without a token and the browser will send that cookie along and the request gets treated as though you put your stamp of approval on it since your token's in there. With cookies, the attacker doesn't even have to go through the effort of stealing your JWT token. We've made it even easier for them to do the thing we're trying to prevent. If we keep it out of a cookie, they have to explicitly steal your token.

@marshallswain
Copy link
Member

If we're using cookies with the new auth plugin, we probably need to figure out a way to guard against that.

@petermikitsh
Copy link
Contributor Author

Would it be possible to add CSRF protection in feathers? Doing so would allow you to detect requests for unauthorized locations. It would completely mitigate that concern of using a cookie, where a rogue site you visit creates requests on your behalf that you didn't intend to.

Edit: I think this would be the CSRF protection strategy you're looking going for.

  • Setting the CSRF cookie on login would be simple. Feathers already sets cookies on login.
  • Through the client library, you would need to ensure the CSRF token gets included in all requests.

Developers not using the feathers client side libraries would be on their own to include the CSRF header in service calls. You'd also need some persistent storage for the CSRF tokens. Storing them in memory wouldn't scale across instances, so you might have to ask developers to provide you with a persistent store of their choosing.

Edit 2: Since you're already setting the JWT token as a cookie in the OAuth strategy, you already have a CSRF vulnerability. The REST middleware will authenticate using the second option, the cookie. Requests from OAuth authenticated users from unauthorized locations (here's an example) would be undetected as such.

@marshallswain
Copy link
Member

In order to implement CSRF protection, we would need a way for the server to associate the client app with the JWT token. I can think of a couple of possible options for increasing security. One of them is just a potential improvement in general security, the other is viable as a CSRF-esque protection mechanism:

  • IP Address - When a token is created, we include the IP address in the payload. This will work great with IPV6, since every device gets its own IP address, but with IPV4 and NAT overloading, it's not going to protect from attacks in the same building or cell tower. Also, since your IP address can potentially change during handoff from one cell tower to another, you'd no longer be authenticated after switching towers. That's only an IPV4 problem, still, but most of the world is still using IPV4. Either way, won't protect against attacks from other websites on your same computer, so not really a CSRF solution, even with IPV6. It does address the scenario you mentioned where they try to send the token to a remote server that harvests tokens. And it's not really a solution for mobile devices, which nowadays means it's not really a solution unless your app has a very limited user base that isn't mobile.
  • Client-generated passphrase The client could generate a hash and pass it to the server upon authentication. That passphrase would need to be included in the token payload, as well. The client would have to hold on to it, at least in memory, and include it with every request. So now both the token and passphrase have to travel together. The server would decode the token, check its validity, then compare the two passphrases. This would effectively associate the token with the app itself, not just the device. At this point, the token can't be stolen, since it's in an HttpOnly cookie. The passphrase could still be stolen, but without the token it's useless. It's not quite a session, since the server doesn't have to store any user data, so we still have that benefit in place. For scaling, the servers would all have to be configured to pass this passphrase and token around together in order to authenticate with each other on behalf of the current user, which isn't much different than what we're doing already. This actually seems like a pretty cool solution, to me, but what am I not thinking of? The passphrase would only be valid while the token is valid. We can store the passphrase in localStorage for the remember-me feature, which still keeps it so that other sites on the same computer can't use the cookie/token. The only problem (that I can think of) left unsolved is the scenario where you get a rogue script on your page, but I think that's the equivalent of a hacker having physical access to a machine. If you can get physical access to a machine, you own the machine. If you can get a script onto the page, you own the page.

Anyway, everybody please find the loopholes. If this works out, I really like the cookie idea. :)

@BigAB
Copy link

BigAB commented Mar 28, 2016

HTTP only cookies removes two of the main benefits of JWTs:

  1. The ability to use the same token across several domains, including the idea that you can offer tokens from a centralized server or tenant and use them across a plethora of non-host services.
  2. You can decode a JWT client side and use it as an indicator for scopes, for example: You can read it using JavaScript web app, see the "permission roles" or whatever, and render the page accordingly, i.e. only shoing the admin options to those with admin role.

*Number 2 there isn't about security on the client side, they may be able to hack the client to "see" the admin interface, but the token would then be invalid when the server verified them for any admin action.

The trade off of losing these 2 extremely useful benefits (especially #1 in a micro-service world) is JWTs vulnerability to XSS (which really screws you over regardless if you are using sessions or not, ie key-loggers on password fields), but comes with the added benefit of being immune to CSFR, and therefore don't nee the awareness or machinery of CSFR-tokens.

This is all of course, inherent with JWT and is certainly not feathers specific.

In light of these trade-offs, I prefer the JWT tokens to be available to the client (specifically JavaScript) and not stored in a cookie with the HTTP-only option.

If any action is taken, it should be optional, or at least reversable.

@BigAB
Copy link

BigAB commented Mar 28, 2016

@marshallswain I think your IP based solution limits JWTs way more than they should, restricting valid login to a machine rather than just a domain, it seems like a mistake, unless I misunderstand

You Client-generated passphrase is interesting, but let me see if I understand it.

  • token is stored in http-only cookie
  • passphrase is stored in localstorage (for a web-based example)
  • every request sends both
  • you need you passphrase in the request and token on the cookie to validate

Therefore:

  • CSFR is an incomplete attack vector because you need a passphrase still
  • XSS is an incomplete attack vector because you need a token still (can't get from JS in a browser)
  • The passphrase needs to be stored somewhere (database?) and available to all services, requiring a DB read for validation (just like cookie based sessions)
  • You are still limiting the ability to use the token across different domains
  • You still limit the client from reading the token for rendering/client side user knowledge

Though this may be a strategy for removing the need for CSFR-tokens, it doesn't seem to have much benefit over traditional sessions other than that.

You may be able to actually remove my last point, client side token read, by storing the token client side, with a hashed version of the passphrase, and store the passphrase in the http-only cookie. Which may tip it closer to a good solution for me.

...but I still prefer the domain flexibility offered by JWTs, at the expense of being slightly more vulnerable to XSS (meaning the XSS could happen at any time to be effective, not just before login, which kills all the aforementioned strategies anyway).

@BigAB
Copy link

BigAB commented Mar 28, 2016

Here's a library that does client side decoding, it's very small and easy to read what it is doing.
https://github.com/auth0/jwt-decode

@marshallswain
Copy link
Member

Well, how did I miss that! That's awesome. I honestly had no idea all this time. Thank you!

@marshallswain
Copy link
Member

That's the whole reason I was sending both the payload and the token in the successful auth response. I didn't realize I could just send the token and read its payload.

@BigAB
Copy link

BigAB commented Mar 28, 2016

Not to overload this discussion with ideas, but maybe we should mention somewhere in the docs to not store sensitive user information in a JWT payload, just in case more developers don't know it's totally readable client side.

@marshallswain
Copy link
Member

Absolutely. Please create an issue in the feathers-docs repo, if you don't mind. That would have come back to bite me later.

@ekryski
Copy link
Member

ekryski commented Mar 28, 2016

Uhmm... You don't need the secret to decode a JWT on the client, it's a non-changing specific encoding. The signature is all that is created with the secret, it's only 1 of three parts of a JWT and all the user info is in the second part, the scope or payload as Auth0 calls it

@BigAB yes this has actually been on my mind and was why we added the ability in 0.6 to be able to specify payload fields from the user object. I think I need to go back an revisit that because a better solution is for you to set hook.params.payload in any hook, with whatever data you want and it will be added to the payload.

Now, there is risk of your token getting too big, which is why we only add the user id by default. That is really all you need and you get the user object as part of the auth response.

This is also why if you are storing sensitive info in a token it should be encrypted using a strong encryption scheme.

@marshallswain
Copy link
Member

So, I think that leaves the solution back to being very mindful of XSS attacks in your app, right? (which is no different than what you should always do) Don't load an ad network script in your banking app. :)

@petermikitsh
Copy link
Contributor Author

It sounds like, by limiting JWT's to Http-Only, which certainly has its security benefits, preventing XSS attacks is the developer's responsibility. I concede, that earlier in the thread, I made the point that some developers might unknowingly have XSS vulnerabilities, but we could still provide protection via the Http-Only option -- but that solution creates pain points in certain use cases.

Having throughly mulled this over, I now feel it's best to consider it the developer's responsibility to secure the JWT. To assist in achieving that end, feathers can provide JWT expiration functionality (#73, #133), and sanitization hooks (feathersjs-ecosystem/feathers-hooks#62). The solution to the CSRF concern still is open. It only applies if you're storing the JWT in a cookie, so ironically enough, might I suggest forbidding the JWT from ever being in a cookie in the first place? Only keep it in localStorage. Then, we don't have to spend time developing the client-generated passphrase solution. This would mean re-massaging the OAuth implementation to not use cookies. Maybe we could pull that off.

Edit: Spit balling here, but maybe you could keep the OAuth implementation the same, but once you redirect back to the app, and once you've read the JWT cookie, in client code you delete the JWT cookie and maintain it in localStorage, so it only hangs around in cookie storage during the login flow (somewhat related to #122).

@marshallswain
Copy link
Member

It seems like @ekryski mentioned needing a cookie for some OAuth providers?

For CSRF, If it's an option for your app, you can still whitelist domains in the CORS configuration.

@ekryski
Copy link
Member

ekryski commented Mar 28, 2016

@marshallswain Yes I think so. Possibly being able to optionally set the cookie as httpOnly or disable it altogether. If you're not using OAuth or form posts without Ajax or sockets you don't need it.

@petermikitsh yes you are right. I believe XSS is the developer's responsibility and we'll help however we can to make it easy to protect against.

his would mean re-massaging the OAuth implementation to not use cookies.

I'm not sure if there is a better solution for OAuth really. Totally open to it if someone can cook something up but I trust the Auth0 team has been pretty thorough. I do have an idea around sockets but then you would have to use sockets.

In the interim what we might want to do (which is what I did originally) is just set the cookie to immediately expire so if you make another request, browse to a new page or open a new window it's not available*. I think I want to remove this code as well until we have the actual need for it (if at all). In hindsight this is a premature optimization that is not required and could introduce a security vulnerability currently.

*Although after some quick reading it looks like Firefox doesn't actually delete expired cookies automatically?? (come on!) So in theory an attacker could still inspect the expired cookies manually to access the token.

@marshallswain
Copy link
Member

Maybe we create a middleware for deleting the cookie from the server?

function logout(req, res){
  res.clearCookie('feathers-cookie', { path: '/' });
  res.status(200).send();
}

@petermikitsh
Copy link
Contributor Author

I assume most developers are immediately calling feathers.configure(authentication(...)); in client side code on page load (let me know if this not a reasonable assumption). In this function call, could you do a hard delete of the expired cookie, to handle use cases such as Firefox?

@marshallswain
Copy link
Member

The only solution I've seen to delete a cookie directly on the client is to set its expiry. Do you know of another way?

@petermikitsh
Copy link
Contributor Author

@marshallswain Wow, cookies suck. My research is suggesting you can only 'expire' it, but not outright delete it's existence in client code. On login, you could make a call to clear the cookie in server code, as you have suggested.

@ekryski
Copy link
Member

ekryski commented Mar 28, 2016

Alright, so that this thread doesn't become too lengthy and because I think we've all discussed this pretty thoroughly so far, I'm going to start tackling this today by:

  • Making sure the cookie is SSL only in production and it errors if not using HTTPS
  • Making sure you can disable the cookie being sent altogether
  • Enabling the ability to set httpOnly if you need to
  • Setting the cookie to expire within 30 seconds
  • Removing the ability to use the cookie for authentication. If we need this in the future it should be a separate cookie.
  • Scope cookie path to only be valid for the successRedirect route
  • Be able to "remove" the cookie on logout. @petermikitsh you can overwrite the cookie body and expire it using JS.

It will go in the 0.7 release that is coming up.

If anyone has any gripes or concerns then please create a sample app and prove that you can successfully attack it and post a gist or a link to the code. With something tangible we can adequately mitigate vulnerabilities and write tests against the code. Thanks for all the input and feedback! This was really good. 😄

Edit: You can't set the cookie to expire immediately or it won't be picked up by JS in some browsers so I gave it a 30 second window to account for generous network latency.

@ekryski ekryski modified the milestones: 0.7, 0.8 Mar 28, 2016
@marshallswain
Copy link
Member

That all sounds good to me. thanks @ekryski

@petermikitsh
Copy link
Contributor Author

Solid plan @ekryski. I feel much better about adopting feathers now that we had this discussion. Thanks everyone!

Oh -- one more comment -- I am updating my SAML implementation to include the JWT token in the response body (as opposed to setting a cookie). In my implementation, the single-sign on flow is done in an iframe. The response HTML includes a script tag that calls a global function on the parent window, passing the JWT as a parameter. This allows me to completely avoid cookies. I'll update the gist later today. It might be possible to adapt this concept for the OAuth flow, allowing feathers to move to a fully cookie-less authentication architecture.

@ekryski
Copy link
Member

ekryski commented Mar 28, 2016

@petermikitsh I had a thought about offering a popup or iframe approach so I'm very keen to see that. Ping me when you have it up.

@petermikitsh
Copy link
Contributor Author

@ekryski Will do.

Also, probably valuable to know:

In private browsing mode, Safari, iOS Safari and the Android browsers do not support setting sessionStorage or localStorage.

(Known issue number 5: http://caniuse.com/#feat=namevalue-storage)

So totally removing cookies might not be a good idea either, which case, this express module for CSRF protection can be applied.

@marshallswain
Copy link
Member

Oh. That's good to know. At that point you could also fall back to storing it in a global variable, couldn't you? In memory, only.

@petermikitsh
Copy link
Contributor Author

Hmm... so this might surprise you:

OS X, Safari, Private Browsing

screen shot 2016-03-28 at 2 57 39 pm

checking for localStorage is not enough. You need to attempt calling setItem and catching the exception.

Recall that most people are init'ing their storage like so:

import authentication from 'feathers-authentication/client'
import feathers from 'feathers/client'

var client = feathers()
        .configure(authentication({storage: window.localStorage}));

I'll create a new issue for ensuring the shim gets returned in private browsers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants