From 4a5f914a624c96128d16c8fc708c7c87a2552ab5 Mon Sep 17 00:00:00 2001
From: Min RK <benjaminrk@gmail.com>
Date: Tue, 1 Aug 2017 15:22:35 +0200
Subject: [PATCH] only apply reduced hash+salt to internally generated tokens

don't trust any user-provided tokens to have decent entropy, regardless of size
---
 jupyterhub/orm.py            | 24 ++++++++++++++++--------
 jupyterhub/tests/test_orm.py |  8 ++++++++
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py
index 66c98330..753b59db 100644
--- a/jupyterhub/orm.py
+++ b/jupyterhub/orm.py
@@ -236,6 +236,12 @@ class Hashed(object):
     salt_bytes = 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
     def token(self):
         raise AttributeError("token is write-only")
@@ -244,16 +250,13 @@ class Hashed(object):
     def token(self, token):
         """Store the hashed value and prefix for a token"""
         self.prefix = token[:self.prefix_length]
-        if len(token) >= 32:
-            # Tokens are generally UUIDs, which have sufficient entropy on their own
+        if self.generated:
+            # Generated tokens are UUIDs, which have sufficient entropy on their own
             # and don't need salt & hash rounds.
             # ref: https://security.stackexchange.com/a/151262/155114
-            rounds = 1
-            salt_bytes = b''
+            rounds = self.generated_rounds
+            salt_bytes = self.generated_salt_bytes
         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
             salt_bytes = self.salt_bytes
         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
         if token is None:
             token = new_token()
+            # Don't need hash + salt rounds on generated tokens,
+            # which already have good entropy
+            generated = True
         else:
+            generated = False
             cls.check_token(db, token)
-        orm_token = cls(token=token)
+        orm_token = cls(generated=generated)
+        orm_token.token = token
         if user:
             assert user.id is not None
             orm_token.user_id = user.id
diff --git a/jupyterhub/tests/test_orm.py b/jupyterhub/tests/test_orm.py
index 041f5f83..ebbe46fc 100644
--- a/jupyterhub/tests/test_orm.py
+++ b/jupyterhub/tests/test_orm.py
@@ -67,6 +67,8 @@ def test_tokens(db):
     assert found.match(token)
     assert found.user is user
     assert found.service is None
+    assert found.hashed.startswith('%s:1::' % orm.APIToken.algorithm)
+
     found = orm.APIToken.find(db, 'something else')
     assert found is None
 
@@ -74,6 +76,12 @@ def test_tokens(db):
     token = user.new_api_token(secret)
     assert token == secret
     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
     with pytest.raises(ValueError):