Security: Store api token hashed

[p1gp1g]

Is your feature request related to a problem? Please describe.
Tokens shouldn't be stored in plain text. (There isn't security issue to fill so I'm publishing here)

Describe the solution you'd like
The easiest to do is to store them hashed, like passwords. But unlike passwords, they need to be unique. Also, they aren't user input, that means that we can have a really long token and we can use fast hashing algorithm (long to crack but fast with the token)
So the main solution is to use a non-salted hash (e.g sha256) with a longer API token (e.g. 128 chars instead of 14).

Describe alternatives you've considered
An other solution would be to have an unique token_id with a salted hash.

Additional context
nothing

(One thing apart, the default work factor for bcrypt should be 12, as OWASP recommend)

[jmattheis]

Yeah, this is kinda intentional. Otherwise, tokens can't be viewed at any time inside the webui.
I'll ask in our community chat if anyone uses this feature to decide if we can change this feature to be more secure. As this is a breaking change it will probably take some time before deciding.

The bcrypt default can be changed.

[jmattheis]

Nope, nothing happened yet.

[eternal-flame-AD]

Why should token be hashed? I am a little confused.

The reason passwords are derived is that they have patterns that reduce entropy and grant additional scope if the user do not use unique passwords:

• A brute force or heuristic brute force on the actual password that be used to compromise other services is difficult even if the hashed password is leaked. If you can get your hands on this hash it is likely the service is compromised already.
• It completely scrambles any visible pattern on the password.
• If two users use the same passwords they do not look the same in the database.

Tokens are randomized to begin with and grants no additional scope than the session originally has, if you can get your hand on the DB entry of that token chances are the whole service is already compromised (everything we do is in the database), session integrity is already not a concern now. The only additional thing an attacker can do is maybe delete all messages if somehow their original database access is read only?

The only thing that is useful I think is to avoid showing tokens back to the user once it is viewed to prevent session hijacking or impersonation that is caused by vulnerability on the client side. Hashing on the server side has nothing to do with this.

@jmattheis Do you think this is worth patching? If not I think we can close this.

[p1gp1g]

Passwords are hashed so users can't be impersonated if the db leaks (with a backup publicly accessible for instance). They are hashed with a salt to not make obvious when a password is reused (and avoid rainbow table) and hashed with a resource intensive algorithm because passwords are often weak (pattern etc.), to avoid brute force/dictionary.

With API token, you don't want users to be impersonate when the db leaks too. But the API tokens are unique and need to be unique: so you must not use a salt. And token are randomly generated, so you'll prefer a fast algorithm (like SHA256).

Most of the time, token are displayed only once to users, and that's (hopefully) the reason, or one of them. But tokens won't be able to be seen again

[eternal-flame-AD]

Thanks for the reply. If the DB is leaked, what is the additional thing an attacker can gain by obtaining an access token?

The only scenario this would be worth it is if you somehow get a read only copy of the DB, and are not satisfied with seeing essentially all data on Gotify but need to actively modify it. I think this precise level of initial foothold and desired outcome is not common in the context of Gotify.

Password is different because they may be reused hence grant additional scope in an attack.

[p1gp1g]

If the leak isn't notice, they can continue to get data about users, or modify some subscriptions.

• Imagine a small network using gotify to be notified when an alert is detected. Having a token allows to know when an alert is raised, or to delete the topic related to the alerts.
• A connected house sending a notification when all lights are off may be used to know if there is nobody at home.
• Etc.

[eternal-flame-AD]

@p1gp1g

Thanks for the follow up, understand your position better now.

If the leak isn't notice, they can continue to get data about users

This is true regardless whether the token is leaked or not.

to delete the topic related to the alerts.

These dependent on Gotify being used for integrity critical scenarios which we did not make such an claim, nor did we discourage such use. So this is not invalid scenario but is an attack precondition out of the attacker's control.

The other exploit cases can be done with DB access alone.

My assessment is CVSS:3.1/AV:N/AC:L/PR:H/UI:N/S:U/C:N/I:L/A:L - 3.8 Low. The score is low because by CVSS standards the effect of integrity compromise is limited (you are only allowed to do what the user can do legitimately, not override the application, and the effect of the integrity compromise does not by itself lead to changed scope such as the IDS scenario you mentioned).

You are welcome to open a security advisory on GitHub if you want to be credited. Let me know if you are working on it. I think this can be fixed if @jmattheis agrees.

[eternal-flame-AD]

If you want to report, I also encourage you to think creatively about ways to chain weaknesses like these together for a bigger impact :) (My previous job is threat hunting before I started doing academics instead).

But please don't publish them beforehand of course. Thanks!

[p1gp1g]

The other exploit cases can be done with DB access alone.

Sure, but the thing with readable tokens is you can read live content from a snapshot of the DB, without access to the actual DB.

So it would be C:L/I:N/A:L or even C:H/I:N/A:H depending on the content, may contain personal data etc. I:N because we can't modify content, only delete it. Nevertheless, I agree having a snapshot of the DB is a huge condition, and the risk is limited

[eternal-flame-AD]

CVSS is neutral in what kind of content is actually leaked/modified. Low simply means you are still bound by something (what the application usually allows the user to do, what the application would typically allow the user to see, etc), no matter if it is my secret recipe or my banking information.

I think this can be classified as C:L in certain scenarios (like a snapshot as opposed to direct access). It is definitely not high as you no longer have reliable access to all information, it is dependent on the token you use still being valid.
I give I:L because you get additional integrity compromise that is not present by precondition. I think this is the primary compromise here.
I give A:L because you can technically induce much more load on the server authenticated you are still bound by the regular rate limit and such on firewalls, it is also highly unlikely you can get complete unavailability just with legal requests alone.

Regardless this is an edge case scenario as you have mentioned. As an open source project it is usually assumed that no more security warranties once you tell me to connect to a compromised DB.

[jmattheis]

Currently the android app depends on the availability of the plaintext application tokens for sending messages with certain applications https://github.com/gotify/android/blob/master/app/src/main/kotlin/com/github/gotify/sharing/ShareActivity.kt#L135

We need a different way to achieve this functionality, if the tokens are hashed. Maybe the /message endpoint can be accessed with a client token and the application-id is passed via query param/header.

[eternal-flame-AD]

@jmattheis (I deleted my previous comment, I misunderstood) Yes this looks reasonable or /message/impersonate/:id maybe (this will start some facility for #337 as well)

A more "complete" fix is to use HMAC signed requests as opposed to just a token. However it will be not backwards compliant and pushing a message would become much harder to do by hand.

[najtin]

@eternal-flame-AD Could explain what the correct interpretation of jmattheis comment is? I think i have the same misunderstanding.

I thought this issue is about storing the secrets hashed on the server. I'm confused how this influences applications. Is jmattheis proposing JWTs or something like that?

I think a hmac signature would be reasonable. Couldn't it be an optional field and disabled or enabled via settings? Of course that would mean that there is an additional feature to maintain.

[eternal-flame-AD]

The android app need to impersonate an app to "share" the message so it needs to query for the token after it is created. Since clients can already create more applications we should just allow it to impersonate one without access to the actual token of that app.

[jmattheis]

I think a great feature of gotify is it's simplicity, so I want sending messages to be as simple as possible. So signing with hmac seems a little overkill.

Our message object already has the appid field

we could make this writable when submitting to /message when a client token is provided. I think I'd prefer this solution.

[p1gp1g]

There can be one app token per client token with an API endpoint to re-generate the application token for the device, for instance:

GET /application/token

return {"token":"Afoo"}

and the server stores the hash of the token. If the client loses the token, it can generate a new token that will override the previous