sminnee / silverstripe-apikey

API Key management for SilverStripe
7 stars 16 forks source link

Store encrypted keys #11

Open sminnee opened 5 years ago

sminnee commented 5 years ago
blueo commented 5 years ago

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

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:

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

blueo commented 5 years ago

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

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:

Thoughts?

sminnee commented 5 years ago

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

madmatt commented 5 years ago

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:

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:

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

sminnee commented 5 years ago

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

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).