addons icon indicating copy to clipboard operation
addons copied to clipboard

Find a simpler replacement for our current encrypted api-key fields

Open EnTeQuAk opened this issue 7 years ago • 3 comments

Porting https://github.com/EnTeQuAk/m2secret (originally forked from Andy and originally written back in 2009) to https://github.com/pyca/cryptography/ kinda revealed that our currently used system for api-keys is a little bit over-engineered for it's use-case and also revealed how old and not up-to-date cryptographicaly m2secret is (sha1+hmac, no constant time compares, it's generally a bit more complex at some places than it should be). All that is fixable but that takes a lot of work and needs careful reviews and I honestly don't like us to maintain cryptographic code if we don't have to.

As far as I understood we are encrypting the API keys so that there's another layer of security in case a security-hole like a SQL injection happens and the attacker gets all or some of our users api-keys which wouldn't be immediately be useful to him and we have another layer to invalid them all by changing our server-side secret key.

Potential proposals:

  • Use a system like https://docs.djangoproject.com/en/1.10/topics/signing/ or http://pythonhosted.org/itsdangerous/ (which would need to be updated to not use sha1 but that's another topic) to have hashed and signed api-tokens
  • Use scrypt or bcrypt to hash the tokens together with our secret-key to add some pepper to the mix

Whatever we use will need to have better or similar security features (easy to expire / block keys and accounts, safe against sql injection and other attacks, etc) and at best should be good maintained and a generally accepted solution to avoid brewing our own stuff when there's no need for it.

cc kumar303 (initial api-key implementation) and jvehent (security) to the discussion and for potential input

As far as I can see it this isn't really a ticking timebomb since the algorithms we use aren't up-to-date but are not yet known as insecure. Also, we need a system that takes old api-tokens into account and allows silent upgrades to a potential new system.

┆Issue is synchronized with this Jira Task

EnTeQuAk avatar Mar 22 '17 22:03 EnTeQuAk

Before picking an algorithm, we need to answer the question "Does AMO ever want to show users their current API tokens, or simply allow them to change them?".

Sites differ in this approach. Twitter will show API tokens, AWS will not. Obviously, the latter is more secure because a DB leak or fraudulent access to an account wouldn't leak tokens, but it's also less usable.

I have a strong preference for storing tokens in an irreversible manner, and only allowing users to change them. If we can implement that, I'd recommend to use bcrypt with a random salt for each key. Don't use the single secret key of AMO across all hashes, it's not resistant to rainbow tables attacks.

jvehent avatar Mar 23 '17 13:03 jvehent

I'm glad we're finally moving this off of M2Crypto! Here are some responses just for historical context:

Porting m2secret ... to pyca/cryptography kinda revealed that our currently used system for api-keys is a little bit over-engineered for it's use-case

The use case of the secret API key is this: a developer uses it to generate a one-time token (a JWT) to upload their add-on package to Mozilla. Mozilla then signs it for installation in Firefox. If this secret key were ever leaked out, an attacker could masquerade as another developer and inject malware into Firefox.

For example, if a popular extension like uBlock Origin were self-hosted and an attacker stole this developer's secret key, they could very quickly propagate an add-on containing malware to all uBlock Origin users.

This was the original decision for why we wanted to encrypt the secret key in the database. We were worried that a SQL injection or some other database-only exploit might leak it.

Does AMO ever want to show users their current API tokens, or simply allow them to change them?

Either way would work. Not showing tokens might be inconvenient for developers but that's not a deal breaker. They can just regenerate the token if they forget what it is.

I have a strong preference for storing tokens in an irreversible manner, and only allowing users to change them.

The reason we store the secret key is because we are currently using JWTs in a symmetric way. The developer signs a JWT on every API request with the secret key and the server uses the same secret key to verify the signature. This architecture could be changed if needed but we do have a lot of tools currently invested in the JWT approach.

But, yeah, it would be helpful to store keys in an irreversible manner because of the threat I mentioned above.

kumar303 avatar Mar 24 '17 10:03 kumar303

Related to https://github.com/mozilla/addons/issues/4567

EnTeQuAk avatar Jul 27 '17 15:07 EnTeQuAk