only apply reduced hash+salt to internally generated tokens

don't trust any user-provided tokens to have decent entropy, regardless of size
This commit is contained in:
Min RK
2017-08-01 15:22:35 +02:00
parent c78e31b136
commit 4a5f914a62
2 changed files with 24 additions and 8 deletions

View File

@@ -236,6 +236,12 @@ class Hashed(object):
salt_bytes = 8 salt_bytes = 8
min_length = 8 min_length = 8
# values to use for internally generated tokens,
# which have good entropy as UUIDs
generated = False
generated_salt_bytes = b''
generated_rounds = 1
@property @property
def token(self): def token(self):
raise AttributeError("token is write-only") raise AttributeError("token is write-only")
@@ -244,16 +250,13 @@ class Hashed(object):
def token(self, token): def token(self, token):
"""Store the hashed value and prefix for a token""" """Store the hashed value and prefix for a token"""
self.prefix = token[:self.prefix_length] self.prefix = token[:self.prefix_length]
if len(token) >= 32: if self.generated:
# Tokens are generally UUIDs, which have sufficient entropy on their own # Generated tokens are UUIDs, which have sufficient entropy on their own
# and don't need salt & hash rounds. # and don't need salt & hash rounds.
# ref: https://security.stackexchange.com/a/151262/155114 # ref: https://security.stackexchange.com/a/151262/155114
rounds = 1 rounds = self.generated_rounds
salt_bytes = b'' salt_bytes = self.generated_salt_bytes
else: else:
# users can still specify API tokens in a few ways,
# so trigger salt & hash rounds if they provide a short token
app_log.warning("Applying salt & hash rounds to %sB token" % len(token))
rounds = self.rounds rounds = self.rounds
salt_bytes = self.salt_bytes salt_bytes = self.salt_bytes
self.hashed = hash_token(token, rounds=rounds, salt=salt_bytes, algorithm=self.algorithm) self.hashed = hash_token(token, rounds=rounds, salt=salt_bytes, algorithm=self.algorithm)
@@ -360,9 +363,14 @@ class APIToken(Hashed, Base):
db = inspect(user or service).session db = inspect(user or service).session
if token is None: if token is None:
token = new_token() token = new_token()
# Don't need hash + salt rounds on generated tokens,
# which already have good entropy
generated = True
else: else:
generated = False
cls.check_token(db, token) cls.check_token(db, token)
orm_token = cls(token=token) orm_token = cls(generated=generated)
orm_token.token = token
if user: if user:
assert user.id is not None assert user.id is not None
orm_token.user_id = user.id orm_token.user_id = user.id

View File

@@ -67,6 +67,8 @@ def test_tokens(db):
assert found.match(token) assert found.match(token)
assert found.user is user assert found.user is user
assert found.service is None assert found.service is None
assert found.hashed.startswith('%s:1::' % orm.APIToken.algorithm)
found = orm.APIToken.find(db, 'something else') found = orm.APIToken.find(db, 'something else')
assert found is None assert found is None
@@ -74,6 +76,12 @@ def test_tokens(db):
token = user.new_api_token(secret) token = user.new_api_token(secret)
assert token == secret assert token == secret
assert len(user.api_tokens) == 3 assert len(user.api_tokens) == 3
found = orm.APIToken.find(db, token=token)
assert found.match(secret)
algo, rounds, salt, _ = found.hashed.split(':')
assert algo == orm.APIToken.algorithm
assert rounds == str(orm.APIToken.rounds)
assert len(salt) == 2 * orm.APIToken.salt_bytes
# raise ValueError on collision # raise ValueError on collision
with pytest.raises(ValueError): with pytest.raises(ValueError):