refactor: compare tokens using compare_digest

ietf/submit/tests.py

@@ -51,8 +51,9 @@
                                process_submission_xml, process_uploaded_submission, 
                                process_and_validate_submission, apply_yang_checker_to_draft, 
                                run_all_yang_model_checks)
+from ietf.submit.views import access_token_is_valid, auth_token_is_valid
 from ietf.utils import tool_version
-from ietf.utils.accesstoken import generate_access_token
+from ietf.utils.accesstoken import generate_access_token, generate_random_key
 from ietf.utils.mail import outbox, get_payload_text
 from ietf.utils.test_runner import TestBlobstoreManager
 from ietf.utils.test_utils import login_testing_unauthorized, TestCase
@@ -3500,3 +3501,31 @@ def test_submissionerror(self, mock_sanitize_message):
             mock_sanitize_message.call_args_list,
             [mock.call("hi"), mock.call("there")],
         )
+
+
+class HelperTests(TestCase):
+    def test_access_token_is_valid(self):
+        submission: Submission = SubmissionFactory()  # type: ignore
+        valid_token = submission.access_token()
+        access_key = submission.access_key  # accept this for backwards compat
+        invalid_token = "not the valid token"
+        self.assertTrue(access_token_is_valid(submission, valid_token))
+        self.assertTrue(access_token_is_valid(submission, access_key))
+        self.assertFalse(access_token_is_valid(submission, invalid_token))
+
+    def test_auth_token_is_valid(self):
+        auth_key = generate_random_key()
+        submission: Submission = SubmissionFactory(auth_key = auth_key)  # type: ignore
+        valid_token = generate_access_token(submission.auth_key)
+        auth_key = submission.auth_key  # accept this for backwards compat
+        invalid_token = "not the valid token"
+        self.assertTrue(auth_token_is_valid(submission, valid_token))
+        self.assertTrue(auth_token_is_valid(submission, auth_key))
+        self.assertFalse(auth_token_is_valid(submission, invalid_token))
+
+        submission.auth_key = ""
+        submission.save()
+        self.assertFalse(auth_token_is_valid(submission, valid_token))
+        self.assertFalse(auth_token_is_valid(submission, auth_key))
+        self.assertFalse(auth_token_is_valid(submission, invalid_token))
+        self.assertFalse(auth_token_is_valid(submission, ""))

ietf/submit/views.py

@@ -2,6 +2,7 @@
 # -*- coding: utf-8 -*-
 import re
 import datetime
+from secrets import compare_digest
 
 from typing import Optional, cast         # pyflakes:ignore
 from urllib.parse import urljoin
@@ -255,19 +256,48 @@ def search_submission(request):
     )
 
 
-def can_edit_submission(user, submission, access_token):
-    key_matched = access_token and submission.access_token() == access_token
-    if not key_matched: key_matched = submission.access_key == access_token # backwards-compat
-    return key_matched or has_role(user, "Secretariat")
+def access_token_is_valid(submission: Submission, access_token: str):
+    """Check whether access_token is valid for submission, in constant time"""
+    token_matched = compare_digest(submission.access_token(), access_token)
+    # also compare key directly for backwards compatibility
+    key_matched = compare_digest(submission.access_key, access_token)
+    return token_matched or key_matched
+
+
+def auth_token_is_valid(submission: Submission, auth_token: str):
+    """Check whether auth_token is valid for submission, in constant time"""
+    auth_key = submission.auth_key
+    if not auth_key:
+        # Make the same calls as the other branch to keep constant time, then
+        # return False because there is no auth key
+        compare_digest(generate_access_token("fake"), auth_token)
+        compare_digest("fake", auth_token)
+        return False
+    else:
+        token_matched = compare_digest(generate_access_token(auth_key), auth_token)
+        # also compare key directly for backwards compatibility
+        key_matched = compare_digest(auth_key, auth_token)
+        return token_matched or key_matched
+
+
+def can_edit_submission(user, submission: Submission, access_token: str | None):
+    if  has_role(user, "Secretariat"):
+        return True
+    elif not access_token:
+        return False
+    return access_token_is_valid(submission, access_token)
+
 
 def submission_status(request, submission_id, access_token=None):
     # type: (HttpRequest, str, Optional[str]) -> HttpResponse
     submission = get_object_or_404(Submission, pk=submission_id)
 
-    key_matched = access_token and submission.access_token() == access_token
-    if not key_matched: key_matched = submission.access_key == access_token # backwards-compat
-    if access_token and not key_matched:
-        raise Http404
+    if access_token:
+        key_matched = access_token_is_valid(submission, access_token)
+        if not key_matched:
+            raise Http404
+    else:
+        key_matched = False
 
     if submission.state.slug == "cancel":
         errors = {}
@@ -621,8 +651,7 @@ def edit_submission(request, submission_id, access_token=None):
 def confirm_submission(request, submission_id, auth_token):
     submission = get_object_or_404(Submission, pk=submission_id)
 
-    key_matched = submission.auth_key and auth_token == generate_access_token(submission.auth_key)
-    if not key_matched: key_matched = auth_token == submission.auth_key # backwards-compat
+    key_matched = submission.auth_key and auth_token_is_valid(submission, auth_token)
 
     if request.method == 'POST' and submission.state_id in ("auth", "aut-appr") and key_matched:
         # Set a temporary state 'confirmed' to avoid entering this code