From 3abdf6c2f5e3a2a0c87b95daace8935ddd452d53 Mon Sep 17 00:00:00 2001
From: Thomas NOEL <tnoel@entrouvert.com>
Date: Thu, 2 Apr 2020 13:50:36 +0200
Subject: [PATCH] signature: do not require nonce if not verified, and other
 fixes (#)

---
 hobo/signature.py       | 25 +++++++++++++++----------
 tests/test_signature.py | 27 ++++++++++++++++++---------
 2 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/hobo/signature.py b/hobo/signature.py
index 2c58bef..8ac3fea 100644
--- a/hobo/signature.py
+++ b/hobo/signature.py
@@ -2,7 +2,6 @@ import datetime
 import base64
 import hmac
 import hashlib
-import urllib
 import random
 
 from django.utils import six
@@ -11,13 +10,15 @@ from django.utils.http import quote, urlencode
 from django.utils.six.moves.urllib import parse as urlparse
 
 
-# Simple signature scheme for query strings
+'''Simple signature scheme for query strings'''
+
 
 def sign_url(url, key, algo='sha256', timestamp=None, nonce=None):
     parsed = urlparse.urlparse(url)
     new_query = sign_query(parsed.query, key, algo, timestamp, nonce)
     return urlparse.urlunparse(parsed[:4] + (new_query,) + parsed[5:])
 
+
 def sign_query(query, key, algo='sha256', timestamp=None, nonce=None):
     if timestamp is None:
         timestamp = datetime.datetime.utcnow()
@@ -29,28 +30,35 @@ def sign_query(query, key, algo='sha256', timestamp=None, nonce=None):
         new_query += '&'
     new_query += urlencode((
         ('algo', algo),
-        ('timestamp', timestamp),
-        ('nonce', nonce)))
+        ('timestamp', timestamp)))
+    if nonce:  # we don't add nonce if it's an empty string
+        new_query += '&nonce=' + quote(nonce)
     signature = base64.b64encode(sign_string(new_query, key, algo=algo))
     new_query += '&signature=' + quote(signature)
     return new_query
 
-def sign_string(s, key, algo='sha256', timedelta=30):
+
+def sign_string(s, key, algo='sha256'):
     digestmod = getattr(hashlib, algo)
+    if isinstance(key, six.text_type):
+        key = key.encode('utf-8')
     hash = hmac.HMAC(smart_bytes(key), digestmod=digestmod, msg=smart_bytes(s))
     return hash.digest()
 
 
 def check_url(url, key, known_nonce=None, timedelta=30):
     parsed = urlparse.urlparse(url, 'https')
-    return check_query(parsed.query, key)
+    return check_query(parsed.query, key, known_nonce=known_nonce, timedelta=timedelta)
 
 
 def check_query(query, key, known_nonce=None, timedelta=30):
     parsed = urlparse.parse_qs(query)
     if not ('signature' in parsed and 'algo' in parsed and
-            'timestamp' in parsed and 'nonce' in parsed):
+            'timestamp' in parsed):
         return False
+    if known_nonce is not None:
+        if ('nonce' not in parsed) or known_nonce(parsed['nonce'][0]):
+            return False
     unsigned_query, signature_content = query.split('&signature=', 1)
     if '&' in signature_content:
         return False  # signature must be the last parameter
@@ -58,9 +66,6 @@ def check_query(query, key, known_nonce=None, timedelta=30):
     algo = parsed['algo'][0]
     timestamp = parsed['timestamp'][0]
     timestamp = datetime.datetime.strptime(timestamp, '%Y-%m-%dT%H:%M:%SZ')
-    nonce = parsed['nonce']
-    if known_nonce is not None and known_nonce(nonce):
-        return False
     if abs(datetime.datetime.utcnow() - timestamp) > datetime.timedelta(seconds=timedelta):
         return False
     return check_string(unsigned_query, signature, key, algo=algo)
diff --git a/tests/test_signature.py b/tests/test_signature.py
index 529b685..d9e9e7a 100644
--- a/tests/test_signature.py
+++ b/tests/test_signature.py
@@ -43,18 +43,27 @@ def test_signature():
 
     # Test nonce parameter
     assert '&nonce=uuu&' in signature.sign_url(URL, KEY, nonce='uuu')
+    assert '&nonce=' in signature.sign_url(URL, KEY)
+    assert '&nonce=' not in signature.sign_url(URL, KEY, nonce='')
 
     # Test known_nonce
-    assert signature.check_url(signature.sign_url(URL, KEY), KEY,
-                               known_nonce=lambda nonce: nonce == 'xxx')
-    assert signature.check_url(signature.sign_url(URL, KEY, nonce='xxx'), KEY,
-                               known_nonce=lambda nonce: nonce == 'xxx')
+    def known_nonce(nonce):
+        return nonce == 'xxx'
+    assert signature.check_url(signature.sign_url(URL, KEY), KEY, known_nonce=known_nonce)
+    assert signature.check_url(signature.sign_url(URL, KEY, nonce='zzz'), KEY, known_nonce=known_nonce)
+    assert not signature.check_url(signature.sign_url(URL, KEY, nonce='xxx'), KEY, known_nonce=known_nonce)
+    assert not signature.check_url(signature.sign_url(URL, KEY, nonce=''), KEY, known_nonce=known_nonce)
 
     # Test timedelta
-    now = (datetime.datetime.utcnow() - datetime.timedelta(seconds=29))
+    now = (datetime.datetime.utcnow() - datetime.timedelta(seconds=20))
     assert signature.check_url(signature.sign_url(URL, KEY, timestamp=now), KEY)
-    now = (datetime.datetime.utcnow() - datetime.timedelta(seconds=30))
-    assert not signature.check_url(signature.sign_url(URL, KEY, timestamp=now), KEY)
-    now = (datetime.datetime.utcnow() - datetime.timedelta(seconds=2))
+    now = (datetime.datetime.utcnow() + datetime.timedelta(seconds=20))
     assert signature.check_url(signature.sign_url(URL, KEY, timestamp=now), KEY)
-    assert signature.check_url(signature.sign_url(URL, KEY, timestamp=now), KEY, timedelta=2)
+    # too late
+    now = (datetime.datetime.utcnow() - datetime.timedelta(seconds=40))
+    assert not signature.check_url(signature.sign_url(URL, KEY, timestamp=now), KEY)
+    now = (datetime.datetime.utcnow() - datetime.timedelta(seconds=20))
+    assert not signature.check_url(signature.sign_url(URL, KEY, timestamp=now), KEY, timedelta=10)
+    # too early
+    now = (datetime.datetime.utcnow() + datetime.timedelta(seconds=40))
+    assert not signature.check_url(signature.sign_url(URL, KEY, timestamp=now), KEY)
-- 
2.25.1

