Hey k00b and ek,
I took a closer look at the implementation of API Keys in #915 and I wanted to share some thoughts/concerns. I am opening a security advisory to follow the responsible disclosure policy, though I have not actually identified an exploit. I thought it would be better to err on the side of caution. If we decide it's appropriate, we can move the discussion to a public GitHub issue or the like.
If I read the PR correctly, when a user generates an API key, the database generates a random string of 32 bytes and stores it in plain text in the database. The API key is then returned to the user. It is also subsequently returned to the user upon request at any point in the future.
In general, I am concerned about storing API Keys in plaintext in the database. As I understand it, this is the first time that any credentials are stored in the stacker.news database. All of the current authentication methods either defer authentication to other identity providers (GitHub, Twitter) via OAuth, or uses asymmetric encryption (a broad generalization) to assert an identity like nostr or LNAuth.
With this approach for API Keys, the stacker.news database now has the potential to become a target for attacks with plaintext API keys. Prior to this point, while there is plenty of interesting info in the database, including funds, gaining access would not grant an attacker the ability to impersonate another user accessing the app externally, since no login credentials are stored in the DB.
I am curious if you considered taking other approaches, or at least approaches that build upon what's already implemented? Based on some quick research, the following seems like a decent approach (sourced from https://security.stackexchange.com/a/180364):
- Generate the random bytes as you're doing today
- Hash the random bytes and store that as the hashed API key in the DB
- Show the API key to the user on screen, but make it clear that it will not ever be retrievable again
- When validating a request with an API key, you'd need to hash it with the same hashing algorithm first in order to do the lookup in the DB.
You could also build upon that and sign the generated keys before hashing them, in order to prevent forgery (inspired from https://stackoverflow.com/a/24367962).
I hope I am not overstepping, but I wanted to share my thoughts here and see what you all think.
Looking forward to hearing back from you.
Best,
WeAreAllSatoshi
SatsAllDay Reporter
Hey k00b and ek,
I took a closer look at the implementation of API Keys in #915 and I wanted to share some thoughts/concerns. I am opening a security advisory to follow the responsible disclosure policy, though I have not actually identified an exploit. I thought it would be better to err on the side of caution. If we decide it's appropriate, we can move the discussion to a public GitHub issue or the like.
If I read the PR correctly, when a user generates an API key, the database generates a random string of 32 bytes and stores it in plain text in the database. The API key is then returned to the user. It is also subsequently returned to the user upon request at any point in the future.
In general, I am concerned about storing API Keys in plaintext in the database. As I understand it, this is the first time that any credentials are stored in the stacker.news database. All of the current authentication methods either defer authentication to other identity providers (GitHub, Twitter) via OAuth, or uses asymmetric encryption (a broad generalization) to assert an identity like nostr or LNAuth.
With this approach for API Keys, the stacker.news database now has the potential to become a target for attacks with plaintext API keys. Prior to this point, while there is plenty of interesting info in the database, including funds, gaining access would not grant an attacker the ability to impersonate another user accessing the app externally, since no login credentials are stored in the DB.
I am curious if you considered taking other approaches, or at least approaches that build upon what's already implemented? Based on some quick research, the following seems like a decent approach (sourced from https://security.stackexchange.com/a/180364):
You could also build upon that and sign the generated keys before hashing them, in order to prevent forgery (inspired from https://stackoverflow.com/a/24367962).
I hope I am not overstepping, but I wanted to share my thoughts here and see what you all think.
Looking forward to hearing back from you.
Best,
WeAreAllSatoshi