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

Use a fast key hasher instead of password hashers #244

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ API key permissions for the [Django REST Framework](https://www.django-rest-fram
### Features

- ✌️ **Simple to use**: create, view and revoke API keys via the admin site, or use built-in helpers to create API keys programmatically.
- 🔒 **As secure as possible**: API keys are treated with the same level of care as user passwords. They are hashed using the default password hasher before being stored in the database, and only visible at creation.
- 🔒 **As secure as possible**: API keys are treated with the same level of care as user passwords. They are only visible at creation and hashed before storing in the database.
- 🎨 **Customizable**: satisfy specific business requirements by building your own customized API key models, permission classes and admin panels.

### Should I use API keys?
Expand Down
2 changes: 1 addition & 1 deletion docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
### Features

- ✌️ **Simple to use**: create, view and revoke API keys via the admin site, or use built-in helpers to create API keys programmatically.
- 🔒 **As secure as possible**: API keys are treated with the same level of care than user passwords. They are hashed using the default password hasher before being stored in the database, and only visible at creation.
- 🔒 **As secure as possible**: API keys are treated with the same level of care as user passwords. They are only visible at creation and hashed before storing in the database.
- 🎨 **Customizable**: satisfy specific business requirements by building your own customized API key models, permission classes and admin panels.

### Should I use API keys?
Expand Down
4 changes: 2 additions & 2 deletions docs/security.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ An API key is composed of two items:

The generated key that clients use to [make authorized requests](#making-authorized-requests) is `GK = P.SK`. It is treated with the same level of care as passwords:

- Only a hashed version is stored in the database. The hash is computed using the default password hasher. [^1]
- Only a hashed version is stored in the database. The hashing algorithm is sha512. [^1]
- The generated key is shown only once to the client upon API key creation.

[^1]: All hashers provided by Django should be supported. This package is tested against the [default list of `PASSWORD_HASHERS`](https://docs.djangoproject.com/en/2.2/ref/settings/#std:setting-PASSWORD_HASHERS). See also [How Django stores passwords](https://docs.djangoproject.com/en/2.2/topics/auth/passwords/#how-django-stores-passwords) for more information.
[^1]: Older versions of this module used the same hashers as Django's [`PASSWORD_HASHERS`](https://docs.djangoproject.com/en/2.2/ref/settings/#std:setting-PASSWORD_HASHERS). These hashers come with a large performance penalty and while critical for passwords, they aren't needed for high-entropy, randomly generated keys like the ones created by this module. Keys stored using these slower hashers will be upgraded when used.

### Grant scheme

Expand Down
53 changes: 49 additions & 4 deletions src/rest_framework_api_key/crypto.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import hashlib
import typing

from django.contrib.auth.hashers import check_password, make_password
from django.utils.crypto import get_random_string
from django.contrib.auth.hashers import (
BasePasswordHasher,
check_password,
make_password,
)
from django.utils.crypto import constant_time_compare, get_random_string


def concatenate(left: str, right: str) -> str:
Expand All @@ -13,7 +18,36 @@ def split(concatenated: str) -> typing.Tuple[str, str]:
return left, right


class Sha512ApiKeyHasher(BasePasswordHasher):
"""
An API key hasher using the sha512 algorithm.

This hasher should *NEVER* be used in Django's `PASSWORD_HASHERS` setting.
It is insecure for use in hashing passwords, but is safe for hashing
high entropy, randomly generated API keys.
"""

algorithm = "sha512"

def salt(self) -> str:
"""No need for a salt on a high entropy key."""
return ""

def encode(self, password: str, salt: str) -> str:
if salt != "":
raise ValueError("salt is unnecessary for high entropy API tokens.")
hash = hashlib.sha512(password.encode()).hexdigest()
return "%s$$%s" % (self.algorithm, hash)

def verify(self, password: str, encoded: str) -> bool:
encoded_2 = self.encode(password, "")
return constant_time_compare(encoded, encoded_2)


class KeyGenerator:

preferred_hasher = Sha512ApiKeyHasher()

def __init__(self, prefix_length: int = 8, secret_key_length: int = 32):
self.prefix_length = prefix_length
self.secret_key_length = secret_key_length
Expand All @@ -25,7 +59,7 @@ def get_secret_key(self) -> str:
return get_random_string(self.secret_key_length)

def hash(self, value: str) -> str:
return make_password(value)
return make_password(value, hasher=self.preferred_hasher)

def generate(self) -> typing.Tuple[str, str, str]:
prefix = self.get_prefix()
Expand All @@ -35,4 +69,15 @@ def generate(self) -> typing.Tuple[str, str, str]:
return key, prefix, hashed_key

def verify(self, key: str, hashed_key: str) -> bool:
return check_password(key, hashed_key)
if self.using_preferred_hasher(hashed_key):
# New simpler hasher
result = self.preferred_hasher.verify(key, hashed_key)
else:
# Slower password hashers from Django
# If verified, these will be transparently updated to the preferred hasher
result = check_password(key, hashed_key)

return result

def using_preferred_hasher(self, hashed_key: str) -> bool:
return hashed_key.startswith(f"{self.preferred_hasher.algorithm}$$")
14 changes: 13 additions & 1 deletion src/rest_framework_api_key/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,19 @@ def _has_expired(self) -> bool:
has_expired = property(_has_expired)

def is_valid(self, key: str) -> bool:
return type(self).objects.key_generator.verify(key, self.hashed_key)
key_generator = type(self).objects.key_generator
valid = key_generator.verify(key, self.hashed_key)

# Transparently update the key to use the preferred hasher
# if it is using an outdated hasher.
if valid and not key_generator.using_preferred_hasher(self.hashed_key):
new_hashed_key = key_generator.hash(key)
type(self).objects.filter(prefix=self.prefix).update(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I acknowledge that this code feels very awkward due to the full hash and algorithm being part of the primary key (id). That doesn't seem avoidable if this is the direction we want to go.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, probably not avoidable. The shape of the ID field is what I'd consider a historical design flaw, tracked in e.g. #128. Let's do along with it for now.

id=concatenate(self.prefix, new_hashed_key),
hashed_key=new_hashed_key,
)

return valid

def clean(self) -> None:
self._validate_revoked()
Expand Down
18 changes: 18 additions & 0 deletions tests/test_hashers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import pytest

from rest_framework_api_key.crypto import Sha512ApiKeyHasher


def test_sha512hasher_encode() -> None:
hasher = Sha512ApiKeyHasher()

key = "test"
hashed_key = hasher.encode(key, "")
assert hasher.verify(key, hashed_key)
assert not hasher.verify("not-test", hashed_key)


def test_sha512hasher_invalid_salt() -> None:
hasher = Sha512ApiKeyHasher()
with pytest.raises(ValueError):
hasher.encode("test", "salt")