RoboSats / robosats

A simple and private bitcoin exchange
https://learn.robosats.com
GNU Affero General Public License v3.0
707 stars 141 forks source link

Authentication that does not require CSRF tokens #249

Open kn0wmad opened 2 years ago

kn0wmad commented 2 years ago

This is unfortunately required for LAN access on Robosats for Embassy

Reckless-Satoshi commented 2 years ago

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.

redphix commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 https://github.com/Reckless-Satoshi/robosats/issues/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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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? :crying_cat_face: . 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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 }
  4. You add to the payload the public key:
    { test: 1, public_key: ABC123 }
  5. Encrypt (sign) the hash to a JWT with your secret key and use it as bearer token.
  6. 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.
  7. 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 commented 2 years ago

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 commented 2 years ago

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 https://github.com/Reckless-Satoshi/robosats/issues/39

Reckless-Satoshi commented 2 years ago

even a db of users is no longer required.

I do not see anyway around needing a User model :thinking:

redphix commented 2 years ago

@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. :sweat_smile: The current backend heavily relies on the user table, so that example wasn't meant for RoboSats. Wasn't clear about it :sweat_smile: 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 commented 2 years ago

More context https://github.com/Reckless-Satoshi/robosats/issues/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 commented 2 years ago

@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 commented 2 years ago

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