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

Store encrypted keys #11

Open
sminnee opened this issue Jul 24, 2018 · 7 comments
Open

Store encrypted keys #11

sminnee opened this issue Jul 24, 2018 · 7 comments

Comments

@sminnee
Copy link
Owner

sminnee commented Jul 24, 2018

  • keys should be stored as hashes, rather than raw values
  • the UI for creating a key should show the key when it is first loaded, and disappear on refresh
  • we should review the metadata made available for keys (such as date created and a free text description) to ensure that the system is usable after this change
@blueo
Copy link
Collaborator

blueo commented Aug 19, 2018

The trick with this is going to be maintaining some kind of user identifier as we can't match on encrypted values. Looking around it seems like some kind of HMAC solution is best practice but it would require a second property (like a secret key/nonce) and more setup client-side eg https://stackoverflow.com/questions/37951220/storing-api-keys-on-server

@sminnee
Copy link
Owner Author

sminnee commented Aug 19, 2018

We could potentially combine the two components into a single value like "[UT]-[SK]" so that calling the API was still a matter of sending a single header. On the server-side, it would be a process of:

  • Unpack the auth header into UT and SK
  • Look up the API token by UT
  • Compared hashed SK to the stored hash-SK value

I'm assuming that UT is unique per-key and not unique per-user.

@blueo
Copy link
Collaborator

blueo commented Aug 21, 2018

So I did a bit of looking into API key recommendations and the general points are:

  • don't use them to authenticate users (eg google's recommendations or owasp)
  • use HMAC and another shared secret (eg app key) to secure the key in transit (eg amazon.
  • apply rate limiting + replay (eg hmac) to the key.

So with this in mind I'm hesitating to just encrypt the key and add a user identifier. This essentially just becomes a username/password but one where the password is stored on some client and send in every request. So I'm thinking the following might be best:

  • Implement HMAC and add to the docs how to achieve this on the client - something akin to https://github.com/philipbrown/signature-php but with some extensibility added
  • create some kind of lower-permission entity in place of member and allow defining permissions there. Perhaps it would need to subclass member and add extra restrictions?
  • add some guidelines as to when to use an API key - much like google's docs

Thoughts?

@sminnee
Copy link
Owner Author

sminnee commented Aug 21, 2018

@madmatt can you help @blueo design a solution for this?

@madmatt
Copy link

madmatt commented Aug 21, 2018

Bear in mind that hashing and encryption are two different things, we shouldn't conflate them here. We want hashing for this use case (treating an API key like a user's password), as far as I can tell.

Generally, I agree with Google's guidance that API keys shouldn't be used to identify the user making the API request, however from a quick review of the code I think that's the stated goal of this module, so let's ignore that for now and focus on what AWS does, as that's more directly applicable.

My suggestion would be basically what AWS expects for S3, which is a single value that combines both user_identifier and hashed_key in one string. This is generally easier for users of the module to store, and is a bit more standard (as a client, you can consider it just a longer API key).

You will need to decide whether the user identifier should be per-user or per-api-key, I would suggest this is based on the API key (then users of the module can always say 'max of one API key per user' if they want to rate limit per user). In that case, it's not so much of a user identifier, it's more an API key identifier. I would strongly recommend that this is not just the API Key's ID value, but instead another unique value (UUID might be taking it a bit far, but that was my first thought).

So the process for users of the API is:

  • Request an API key, module creates a new record, hashes the API key and stores only the hashed value, provides the unhashed original to the user on screen along with the unique id, e.g. Your API key is "[id]:[api key]", you won't see this again so please take note of it.
  • Use the API key by providing it unaltered in any request.
    • Server takes the unhashed API key from the HTTPS request, looks up the hashed value in the db by the [id] value, calculates the HMAC, and if they match the user is authenticated.

It's worth pointing out that this only prevents immediate API key disclosure if the database is compromised. It won't help in these use cases:

  • Attacker gets access via website (e.g. remote code execution - they can just view your HMAC secret or intercept legitimate API traffic to get the original key)
  • Attacker gets access to your server directly (as above)
  • Attacker gets access to the client (outside of our control, but we can't verify the client with this)

Does this... help? I think I'm just re-hashing what's already been stated above to be honest :)

@sminnee
Copy link
Owner Author

sminnee commented Aug 23, 2018

For both identifiers, starting with http://php.net/manual/en/function.random-bytes.php and making readable with http://php.net/manual/en/function.base64-encode.php or http://php.net/manual/en/function.bin2hex.php seems like a good move.

@blueo
Copy link
Collaborator

blueo commented Aug 23, 2018

This all seems to make sense. Looks like the AWS style HMAC setup is a good place to start and can look into google's guidance at at later point (or possibly not given the modules' purpose).

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

No branches or pull requests

3 participants