Fix khmac: allowing separators in userids and adding some test cases (#…

…161)

* fixing khmac

* allowing separators in hmac token userid and adding some test cases

* fix unit test import reference

* making tests work again

.github/workflows/python.yml

@@ -33,7 +33,7 @@ jobs:
     - uses: actions/checkout@v2
     - name: Install node
       run: |
-        export TZ=Europe/Madrid
+        export TZ=UTC
         ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone
         apt update
         apt install -y tzdata

authapi/api/tests.py

@@ -30,7 +30,7 @@
 from .models import ACL, AuthEvent, Action, BallotBox, TallySheet, SuccessfulLogin
 from authmethods.models import Code, MsgLog
 from authmethods import m_sms_otp
-from utils import verifyhmac, reproducible_json_dumps
+from utils import HMACToken, verifyhmac, reproducible_json_dumps
 from authmethods.utils import get_cannonical_tlf, get_user_code
 
 def flush_db_load_fixture(ffile="initial.json"):
@@ -135,6 +135,49 @@ def delete(self, url, data):
         return super(JClient, self).delete(url, jdata,
             content_type="application/json", HTTP_AUTH=self.auth_token)
 
+class TestHmacToken(TestCase):
+    def test_verify_simple_token(self):
+        cases = [
+            dict(
+                token="khmac:///sha-256;48a51120ffd034872c4f1fcd3e61f23bade1181309a66c79bcb33e7838423540/example@nvotes.com:AuthEvent:150017:vote:1620927640",
+                digest='sha-256',
+                hash='48a51120ffd034872c4f1fcd3e61f23bade1181309a66c79bcb33e7838423540',
+                msg='example@nvotes.com:AuthEvent:150017:vote:1620927640',
+                timestamp='1620927640',
+                userid='example@nvotes.com',
+                other_values=['AuthEvent', '150017', 'vote']
+            )
+        ]
+        self._verify_cases(cases)
+
+    def test_verify_tricky_token(self):
+        '''
+        This is a tricky token because the message contains the '/', ':' and ';'
+        separators, meaning that the implementation might get confused if not
+        implemented properly.
+        '''
+        cases = [
+            dict(
+                token="khmac:///sha-256;48a51120ffd034872c4f1fcd3e61f23bade1181309a66c79bcb33e7838423540/ex:amp./le@nvot;es.com:AuthEvent:150017:vote:1620927640",
+                digest='sha-256',
+                hash='48a51120ffd034872c4f1fcd3e61f23bade1181309a66c79bcb33e7838423540',
+                msg='ex:amp./le@nvot;es.com:AuthEvent:150017:vote:1620927640',
+                timestamp='1620927640',
+                userid='ex:amp./le@nvot;es.com',
+                other_values=['AuthEvent', '150017', 'vote']
+            )
+        ]
+        self._verify_cases(cases)
+
+    def _verify_cases(self, cases):
+        for case in cases:
+            token = HMACToken(case['token'])
+            self.assertEqual(token.digest, case['digest'])
+            self.assertEqual(token.hash, case['hash'])
+            self.assertEqual(token.msg, case['msg'])
+            self.assertEqual(token.timestamp, case['timestamp'])
+            self.assertEqual(token.userid, case['userid'])
+            self.assertEqual(token.other_values, case['other_values'])
 
 class ApiTestCreateNotReal(TestCase):
     def setUpTestData():
@@ -2601,7 +2644,6 @@ def setUp(self):
 
     def genhmac(self, key, msg):
         import hmac
-        import datetime
 
         if not key or not msg:
            return

authapi/utils.py

@@ -188,17 +188,22 @@ def verifyhmac(key, msg, seconds=300, at=None):
     valid = valid and at.check_expiration(seconds)
     return valid
 
-
 class HMACToken:
     def __init__(self, token):
         self.token = token
         l = len('khmac:///')
         self.head = token[0:l]
-        msg = token[l:]
-        self.digest, msg = msg.split(';')
-        self.hash, msg = msg.split('/')
-        self.msg = msg
-        self.timestamp = self.msg.split(':')[-1]
+        tails = token[l:]
+        self.digest, data = tails.split(';', 1)
+        self.hash, self.msg = data.split('/', 1)
+        msg_split = self.msg.split(':')
+        self.timestamp = msg_split[-1]
+        if len(msg_split) >= 5:
+            self.userid = ':'.join(msg_split[0:-4])
+            self.other_values = msg_split[-4:-1]
+        else:
+            self.userid = msg_split[0]
+            self.other_values = msg_split[1:-1]
 
     def check_expiration(self, seconds=300):
         t = self.timestamp
@@ -214,15 +219,14 @@ def get_userid(self):
         '''
         Note! Can only be used if it's an auth token, with userid
         '''
-        userid = self.msg.split(':')[0]
-        return userid
+        return self.userid
 
     def get_other_values(self):
         '''
         Removed the userid and the timestamp, returns the list of string objects
         in the message, that are separated by ':'
         '''
-        return self.msg.split(':')[1:-1]
+        return self.other_values
 
 
 def constant_time_compare(val1, val2):