Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADDED] Cookie JWT auth for WebSocket #1477

Merged
merged 1 commit into from
Jun 18, 2020
Merged

Conversation

pas2k
Copy link
Contributor

@pas2k pas2k commented Jun 14, 2020

When authenticating via browser, the best security practice is to use HttpOnly cookies, to avoid scripts stealing client identity via XSS.

HttpOnly cookies can't be accessed by JavaScript, making those perfect for server-certified authentication.

PR implements optional JWT auth using cookies by specifying this option in a config file:
ws: { jwt_cookie: "cookie_name" }

This will make NATS treat a cookie named "cookie_name" as JWT for authentication. If this cookie exists in the HTTP request and is valid, the server will act as if authentication is not required. If this token is missing or invalid, it's possible to authenticate using other configured methods.

Notice that since signing is not possible when authorizing this way, BearerToken should be set in the JWT (e.g. User should be created with nsc add user --bearer).

Tests for regular JWT auth and cookie auth are included.

/cc @nats-io/core

@kozlovic
Copy link
Member

@pas2k First I need to congratulate you on the quality of this PR. Your included test touches areas that are not so easy to grasp and you have made use of all the existing JWT test helper functions.

I need to look more into this approach, but I have reservations on how you use isClientAuthorized() in createClient since this is something normally invoked when receiving a CONNECT, etc.. I wonder if it would not be cleaner and safer to extract (make it a function) the jwt specific authentication part so it could be called here. Again, will look more into it.

@pas2k
Copy link
Contributor Author

pas2k commented Jun 15, 2020

@kozlovic Thank you for the quick review and kind words.

A couple of thoughts regarding isClientAuthorized() use, since it is certainly the most contentius change in the PR.

Checking JWT changes both server and client state. The server might pull new accounts from account server, for example. So extracting JWT checking logic won't decrease the mutex use. It can eliminate some checks for empty fields, certainly, but that's barely noticeable.

Separating JWT checking from isClientAuthorized() can certainly be unsafe, though. Since isClientAuthorized() is used on config reload, there's space for subtle auth bugs if there's a mismatch between JWT-only auth and the rest of the system. Keeping the two synchronized will become a maintenance burden.

Seeing how it's used in reload logic, isClientAuthorized() seems to be the only function idempotent against the client auth fields and configuration, and that's why I decided to use it.

@kozlovic
Copy link
Member

@pas2k With @aricart we tried few things and here are our observations:

We used https://github.com/aricart/natsws-sandbox to test and made small modifications to bin/server.js:

  • Replace the listen's address from 127.0.0.1 to localhost
  • Add res.setHeader('Set-Cookie','jwt=<some jwt>; HttpOnly') before res.writeHead(200)

Run the NATS Server with your PR and a proper config and started a webpage on https://localhost:1443 and that worked, it could connect using the new logic. However, when looking at the cookie for this page, we can see the jwt cookie, so that does seem to defeat the purpose.

If we remove setting the cookie in bin/server.js but instead do it in the client, that is in content/index.js, in run() with something like:

  ...
  const id = nats.nuid.next()
  
  document.cookie = "jwt=<some jwt>; HttpOnly"

Then the NATS Server does not see this cookie, and so does not work. If we remove "HttpOnly" from the command above, then it works, NATS Server see cookies and authenticate based on that.

The point is that in no case we see "HttpOnly" either masking the jwt in the client or its use in the client prevents the NATS Server from seeing it.

Are we missing something?

@pas2k
Copy link
Contributor Author

pas2k commented Jun 16, 2020

Hi @kozlovic and @aricart,

Just tried it. It's working as intended. Here are my results:

  • document.cookie = "jwt=<some jwt>; HttpOnly" doesn't work; you can't set HttpOnly cookies from JavaScript code.
  • document.cookie = "jwt=<some jwt>" defeats the purpose, since the script knows what <some_jwt> is.
  • With res.setHeader('Set-Cookie','jwt=<some jwt>; HttpOnly'), if I open Chrome's developer console, and enter document.cookie, I see "" as a result. This means a rogue script cannot steal this cookie and send it to the malicious actors.
  • With res.setHeader('Set-Cookie','jwt=<some jwt>'), if I open Chrome's developer console, and enter document.cookie, I see "jwt=<some jwt>" as a result. This means that it can be stolen.

When you say "looking at the cookie for this page", you probably mean looking at the cookie list in the developer tools, right? HttpOnly cookies are displayed there, but are inaccessible to any client JavaScript.

Here's an example attack that this prevents:

  • A company hosts a multi-user spreadsheet application using NATS for communication
  • There are ads on the website, provided by AdAgency, by including a script inside the page
  • Page's Content-Security-Policy allows for this script's execution.
  • AdAgency becomes compromised, the ad script is changed to do whatever spreadsheet script does to get the JWT, then does an AJAX request to the remote attacker's collection server.
  • The attacker logs in into NATS and steals all important spreadsheets :-)

Same can be done by a malicious user.

  • User logs in into the spreadsheet app
  • Inserts a <script> tag into the cell
  • If the website improperly escapes the input, JWT can be stolen from users who are currently logged in.

XSS is still the most exploited website vulnerability, so having this feature closes a huge attack surface.

@aricart
Copy link
Member

aricart commented Jun 16, 2020

Yes absolutely this is awesome. I am going to augment the authentication example that I have that uses WSS to include this.

@kozlovic
Copy link
Member

When you say "looking at the cookie for this page", you probably mean looking at the cookie list in the developer tools, right? HttpOnly cookies are displayed there, but are inaccessible to any client JavaScript.

Yes, and on the lock icon in address bar, then Cookies, etc..

Thanks for all the details. Now that we have established the validity and usefulness of that, I will comment on the code specifically. Stay tuned.

Copy link
Member

@aricart aricart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this LGTM. Soon we'll have the ability for the client to verify identity by signing. When we have this on the client, access to the JWT won't jeopardize it, as we'll rely on the nkey signature. Awesome on the tests.

@pas2k
Copy link
Contributor Author

pas2k commented Jun 16, 2020

Soon we'll have the ability for the client to verify identity by signing. When we have this on the client, access to the JWT won't jeopardize it, as we'll rely on the nkey signature.

@aricart If the website's script can sign the auth request, that means it has access to the private key. That also means that any malicious script can acquire the private key the way genuine script did.

Any protection against XSS always has to rely on something that JS can't access. Having an ability to sign something with privkey obviously exposes it to XSS, but it is a much less of a problem, since you can't derive a JWT from privkey because JWT is signed with Account key, and without the ability to log in, the privkey is kinda useless.

@aricart
Copy link
Member

aricart commented Jun 17, 2020

Soon we'll have the ability for the client to verify identity by signing. When we have this on the client, access to the JWT won't jeopardize it, as we'll rely on the nkey signature.

@aricart If the website's script can sign the auth request, that means it has access to the private key. That also means that any malicious script can acquire the private key the way genuine script did.

Any protection against XSS always has to rely on something that JS can't access. Having an ability to sign something with privkey obviously exposes it to XSS, but it is a much less of a problem, since you can't derive a JWT from privkey because JWT is signed with Account key, and without the ability to log in, the privkey is kinda useless.

Dialing it back a bit:

  • Browsers are not really secure platforms. Especially since the web application cannot control the types of helpful plugins that the user may have installed.

  • If a site requires very strong privacy (banking app), then more than likely they are not embedding ads or other things, but still, the user could have installed some helpful addon.

  • Regardless of the authentication mechanism (here we are really talking about playback), it is in the realm of the possibility that a plugin/or script could be targetted to take advantage of some specific issue with a particular web app. If that is the case, they already can probably hijack some other kind of resource, so yes, the HttpOnly will prevent playback of the authentication.

If we had access to private keys, we can still be fairly creative, even if HttpOnly is not supported. The server could create a one-shot user in the context of an account with a specific limit of a single connection, the authentication process would require the private key for verifying it. The client would by default be isolated, and services would be specifically scoped to the client. On session completion, the account invalidates the user jwt, making any kind of replay impossible.

@aricart aricart requested a review from philpennock June 17, 2020 14:51
@aricart
Copy link
Member

aricart commented Jun 17, 2020

@philpennock would probably have additional comments on the strategies, as he's our security expert.

@pas2k
Copy link
Contributor Author

pas2k commented Jun 17, 2020

Creative security strategy

@aricart This is getting a bit off-topic, which is mostly my fault. It would be nice to have a repo with best practice configuration you described and see how it compares. If you ever do, feel free to ping me, would be happy to discuss it there.

server/server.go Outdated Show resolved Hide resolved
@pas2k pas2k force-pushed the ws_cookie_auth branch 3 times, most recently from 158b971 to 43b51b7 Compare June 18, 2020 14:23
@pas2k
Copy link
Contributor Author

pas2k commented Jun 18, 2020

Implemented @kozlovic's suggestions, rebased vs master, covered options and protocol JWT override with tests.

Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very close! Just one obsolete comment and I think that we could alter a bit the way we reference the ws.cookieJwt (see comment)

server/opts.go Outdated Show resolved Hide resolved
server/client.go Outdated Show resolved Hide resolved
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for this contribution and your patience.

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

Successfully merging this pull request may close these issues.

3 participants