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

Authentication that does not require CSRF tokens #249

Open
kn0wmad opened this issue Sep 19, 2022 · 17 comments · Fixed by #243
Open

Authentication that does not require CSRF tokens #249

kn0wmad opened this issue Sep 19, 2022 · 17 comments · Fixed by #243
Labels
enhancement 🆙 New feature or request needs research More knowledge is needed in this issue

Comments

@kn0wmad
Copy link

kn0wmad commented Sep 19, 2022

This is unfortunately required for LAN access on Robosats for Embassy

@Reckless-Satoshi
Copy link
Collaborator

Thanks for reporting and the work to put RoboSats App together for EmbassyOS.

HTTPS access from not whitelisted domains are blocked by the backend, unfortunately white-listening all origins is not a good idea.

Given that we are storing the Robot Token in cookies, we might want to rethink authentification all together. I am not sure what would it take and whether this is safe, but maybe sending the token hash with very request (as a sort of token authentication) might work. In addition, this would make authentication for API users much easier. Needs more research.

@Reckless-Satoshi Reckless-Satoshi added enhancement 🆙 New feature or request needs research More knowledge is needed in this issue labels Sep 19, 2022
@redphix
Copy link
Contributor

redphix commented Sep 27, 2022

This is perfectly timed. I was actually looking into authentication for API users. This is certainly needed for API users as well and to also mention in the openapi docs on how to authenticate requests for users and let them use routes that need authentication (/make, /order, etc.) (PR #243). Will look into it a bit more and update if I have some useful insights.

@redphix
Copy link
Contributor

redphix commented Sep 30, 2022

I think we can either:

  1. Keep the cookie based auth, but also enable a token based authentication using either DRF's builtin Token Authentication or Django REST knox (which drf themselves recommend as well for more capabilities)
  2. Completely move away from cookie based auth (which then means we no longer require CSRF protection - see here) and only use token based auth.

We can return a token when the POST /user route is called. This token can effectively then be used to in the Authorization Header as Authorization: Token <token>. If we go with 1 then we should make sure the token overrides (or given first priority over the default session based auth)

Some more points about 2 - No more cookie to be used means that we don't have to deal with weird cookie behavior. Not entirely sure of the details but maybe there was an issue with the android app (?). On the browser, we store the token in local storage and the JS client is responsible to populate the Authorization header properly, which is a common practice. Now if you ask the internet, there's a lot of debate on whether to use cookies or store token in localstorage which is nicely summarzied by this post and this SO thread as well. Both of advantages and disadvantages, but browsers these already come with many mitigation which prevent attacks.

Option 1 might be the way to go as well, but I am not entirely sure that it's a widely used method. But in this option we protect the frontend users but also give flexibility to API users and third party tools. But nonetheless we need to have auth method that doesn't involve setting cookies.

@Reckless-Satoshi
Copy link
Collaborator

Reckless-Satoshi commented Sep 30, 2022

2. Completely move away from cookie based auth (which then means we no longer require CSRF protection - see here) and only use token based auth.

I would favor a unique general solution for both: the project frontends, and custom tools developed by users using the API. The token authentication looks best.

Any idea on whether we could override the tokens of DRF / or Django REST Knox? Given how RoboSats robot identities were designed, users are already doing token authentication at least once by submitting the hash of their token to generate or recover their robot. It would further simplify and streamline the API usage if the same hash(token) can be used for authenticating every request. I do not know, however, if something is escaping my understanding and this is considered a bad security practice.

robots

If one can send authenticated requests only knowing the hash(token) (without need to first send an authentication request to get an arbitrary auth token from the API), the RoboSats PRO frontend #177 will always be able send POST / PUT requests without "logining" first. This might be a great advantage as we are designing workspaces were a user would be managing many robots at once. If loading a workspace from savefile implies login into 20 robots at once, the coordinator could determine these 20 robots belong to the same user by correlation. However, if no login is needed to retrieve an auth token, the whole set up would be way lighter, faster and private. Loading the workspace savefile would already load every hash(token) (ofc, also pub and priv PGP keys), and would be ready to send authenticated request for any of the robot identities.

@redphix
Copy link
Contributor

redphix commented Sep 30, 2022

API usage if the same hash can then be used for authenticating every request

I don't see why not. At the end of the day a token is just a random string and the hash of the token is perfectly fine for this if we trust our over-the-wire protocol, but if we don't, see below.

I do not know, however, if something is escaping my understanding and this is considered a bad security practice.

And that simply is that - Sending the hash(token) for every request has drastically increased your attack surface (i.e the more the requests go over-the-wire, the more chances of MITM, Request Forgories, etc might take place. By just having to send hash(token) once to the /login route and getting a bearer-token (or simply "token" in general sense), which is ephemeral, reduces the same attack surface, since bearer tokens can be expired.

This is how I see it. Essentially hash(token) is the thing that you know - if you know it you have access to the account, it's the equivalent of password in our system and bearer-token is the thing that you have and is short lived. Otherwise, if you trust your over-the-wire protocol (which is SSL usually - so you are good) hash(token) and bearer-token would be the same for you.

If we assume to not trust tor or i2p, then it's better to not expose our hash(token), and use a separate token that can expire. But we can choose to also just use the hash(token) and let users take the risk. My vote would go to having tokens that are generated after each login. But, as you see these are really subtle points, and choosing hash(token) over bearer-token won't really matter that much. Eg. if we choose the expiry to be 24h, then, in that 24h window the exposure of the bearer-token is the same as hash(token). It's exactly like the problem of choosing an expiry for your pgp keys. If you loose them and they had an expiry, then the damage would atleast be limited to until the key expires - but is that useful? That question can only be answered when we know what that key can do.

However, if no login is needed to retrieve an auth token, the whole set up would be way lighter, faster and private.

Certainly, I personally like that as well, hence I also suggested moving away from any sort of cookie/session altogether - making it completely stateless. But having the cookie auth around will do no harm, so better to keep it (legacy) as long as it is viable.

@Reckless-Satoshi
Copy link
Collaborator

Reckless-Satoshi commented Sep 30, 2022

Understood! Thanks for the explanation.

I believe TOR network communication to Onion services should be at least as secure as SSL. In addition, our robots are in any case ephemeral (you should not reuse them and they last about as much as an order 24-48h must). We will eventually enforce robots to only be usable for 1 completed order (once the UX is improved for easier and intuitive robot generation). I think probably using only hash(token) is as secure as using hash(token) + bearer-token`, plus it is more private and will be faster/lighter (less requests needed). Do you see it the same way?

@redphix
Copy link
Contributor

redphix commented Sep 30, 2022

Do you see it the same way?

Certainly. Stateless and Private.

We will eventually enforce robots to only be usable for 1 completed order

Which means throwing out the idea of user rating and reputation forever? I have a robot that trades with me frequently. Will I not see him ever again? 😿 . On a serious note, seems like people like re-using tokens. And maybe the reputation thing is actually worth it(?) when number users increase. What do you think?

@KoalaSat
Copy link
Member

KoalaSat commented Sep 30, 2022

Hello guys I'm just passing through this interesting conversation.

I'm not sure if maybe I'm saying the same thing as @Reckless-Satoshi. But if the idea is to totally relay on who the client says to be, what about using as bearer token the signed JWT of the payload + public key? Or maybe that's what you call password? Sorry for messing around but I'm the newbie here 😄

@redphix
Copy link
Contributor

redphix commented Oct 1, 2022

@KoalaSat JWT is not a bad idea either. It makes a system completely stateless i.e even a db of users is no longer required. The server can issue tokens and later trust the singed jwt token fully. But I think it would be a little overkill as we already have a database of users and we manage their state ourselves on a single backend. JWT is mostly used when the authentication/identity server is decoupled from the server providing some service and instead of sharing a state between the two (ie. user db) the api server simply trusts the jwt token issued by the identity server by verifying it with the identity server's pub key.

@KoalaSat
Copy link
Member

KoalaSat commented Oct 1, 2022

@redphix but you don't need the server for validate it's the same person, basically you, as a server should be able to identify all requests made by the same signer:

  1. You have a token
  2. With the token you generate a secret - public pair
  3. And now let's say you wanna make a request with a payload:
{ test: 1 }
  1. You add to the payload the public key:
{ test: 1, public_key: ABC123 }
  1. Encrypt (sign) the hash to a JWT with your secret key and use it as bearer token.
  2. When the request is done to the server, the server decrypt the JWT, that can be done by anyone, and use public_key to validate the encryption.
  3. From now on the server can use public_key to identify the same user, as far every request is signed the same way.

Additionally you can even add some kind of time related identifier to the payload to make every request usable just once.

@KoalaSat
Copy link
Member

KoalaSat commented Oct 1, 2022

Thinking again, instead of the payload you can just include the sha256 of that payload ( including the public key), so you can easily validate it on the server without affecting the size of the request

{
  public_key: ABD124,
  payload_sha: DEF567
}

@Reckless-Satoshi
Copy link
Collaborator

Reckless-Satoshi commented Oct 1, 2022

Which means throwing out the idea of user rating and reputation forever?

Yes and it will be for the better. Not the right topic to develop on this issue, but I will elaborate for future reference.

Reusing identities is a bad idea no matter how one looks at it. Bad privacy for the user. Unnecessary data to hold for the coordinator: can be used to learn about real user identity of robots and therefore makes the coordinator a target for hacks or attacks.

The only "upsides" (note the quotes, I do not believe these are upsides) of reusing robots: 1) just faster/easier in the current UI and 2) allows us to have a reputation system.

For 1) we can obviously improve the UX experience of throwaway robots. As for 2), this is simple, why are reputation systems needed? They are needed so you know your counterpart is trustworthy. Well, RoboSats solves the trust issue already with privacy friendly fidelity bonds. A reputation system will only introduce overhead, complexity and a barrier for new users who have no trades on their history.

Reusing identity is a big nono for me, and it is the main differentiator between RoboSats and any other P2P exchange. In fact, reusing identity is the single biggest privacy issue most P2P exchanges have. E.g., in Hodlhodl, if you go into a dispute and need to unveil some info, all your past history with your user account can be linked to your real persona. It's a privacy-rugpull, many prefer to simply lose the funds of the dispute. Still, you cannot avoid this, since anyone who wants to know who you are simply needs to take a trade with you (depends on the payment method used); then he can know right away you have traded X BTC in N contracts over Y years....

More context #39

@Reckless-Satoshi
Copy link
Collaborator

even a db of users is no longer required.

I do not see anyway around needing a User model 🤔

@redphix
Copy link
Contributor

redphix commented Oct 1, 2022

@Reckless-Satoshi

I do not see anyway around needing a User model

Yes, not for our architecture, I meant that for architectures that have decoupled identity systems and the user db is another server somewhere else and doesn't know about other services. 😅 The current backend heavily relies on the user table, so that example wasn't meant for RoboSats. Wasn't clear about it 😅 But If we were super ambitious, we could actually do away with User table. We would just store the user id in the JWT token itself during creation/login. I see it actually being doable, but it seems very hacky in nature/impractical and we would loose a lot of features - one of the bigger downsides i can think at the top of my head being needing to store a long jwt string like eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c

instead of a short token that we currently have.

@redphix
Copy link
Contributor

redphix commented Oct 1, 2022

More context #39

That makes sense! I was vouching for the reputation system, but it's not worth the privacy trade-offs. I like your answer at the very end of that thread, changed my mind about reputation quite much actually. It's good that Robosats' isn't following other p2p trade platform's UX patterns and is definitely a breath of fresh air. It's worth doing it differently!

@redphix
Copy link
Contributor

redphix commented Oct 1, 2022

@KoalaSat I might have not understood you completely, but I believe you showed an example of login flow? Yes, that's how It would probably be done (there are myriad of ways to do it) but I don't see the upside of adding an overhead of wrapping the token in a jwt and and checking it's signature for each request. At the end of the day, you are just wrapping a token inside of a token. If either (ie. jwt token or normal token) get's lost or stolen, the attacker has access anyways. JWT's are particularly useful when you have more meta info that you don't want to keep track of server side (which is usually refereed as "keeping state") (or as I previously explained - in a decoupled system, that info is stored elsewhere). You might have seen "claims", "permissions", "scopes" and other things that are usually included in a jwt token, which are the meta info about a user/service making a request to a server.

To summarize, I don't see the upside to using JWT. If there is, would love to know.

@Reckless-Satoshi
Copy link
Collaborator

This issue was closed as collateral damage of merging PR #243, but it is yet to solve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🆙 New feature or request needs research More knowledge is needed in this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants