瀏覽代碼

Security: replace random.choice with secrets.choice in Token.generate()

Token.generate() used Python's random module (Mersenne Twister PRNG).
Mersenne Twister is not a CSPRNG: observing ~624 outputs from the same
worker process allows full state recovery and prediction of subsequent
outputs. Any token minted in the same worker within that window becomes
predictable, including tokens for privileged accounts.

Fix: replace random.choice with secrets.choice. secrets is backed by
os.urandom() / getrandom() which provides OS-level CSPRNG entropy and
is immune to state-recovery attacks.

The import of the now-unused random module is removed.

Regression tests:
- test_generate_uses_csprng: patches secrets.choice with wraps= to
  confirm it is called exactly TOKEN_DEFAULT_LENGTH times per generate().
- test_generate_length_parameter: verifies length= is respected and
  output is drawn only from TOKEN_CHARSET.

Ref: SR-001 / VM-317 (internal security review, R1-F07 / R3-F1)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Brian Tiemann 3 周之前
父節點
當前提交
97a1375a82
共有 2 個文件被更改,包括 28 次插入2 次删除
  1. 2 2
      netbox/users/models/tokens.py
  2. 26 0
      netbox/users/tests/test_models.py

+ 2 - 2
netbox/users/models/tokens.py

@@ -1,6 +1,6 @@
 import hashlib
 import hmac
-import random
+import secrets
 import zoneinfo
 
 from django.conf import settings
@@ -260,7 +260,7 @@ class Token(models.Model):
         """
         Generate and return a random token value of the given length.
         """
-        return ''.join(random.choice(TOKEN_CHARSET) for _ in range(length))
+        return ''.join(secrets.choice(TOKEN_CHARSET) for _ in range(length))
 
     def update_digest(self):
         """

+ 26 - 0
netbox/users/tests/test_models.py

@@ -1,10 +1,13 @@
+import secrets
 from datetime import timedelta
+from unittest.mock import patch
 
 from django.core.exceptions import ValidationError
 from django.test import TestCase, override_settings
 from django.utils import timezone
 
 from users.choices import TokenVersionChoices
+from users.constants import TOKEN_CHARSET, TOKEN_DEFAULT_LENGTH
 from users.models import Token, User
 from utilities.testing import create_test_user
 
@@ -104,6 +107,29 @@ class TokenTestCase(TestCase):
         with self.assertRaises(ValidationError):
             token.clean()
 
+    def test_generate_uses_csprng(self):
+        """
+        Regression: Token.generate() must use secrets.choice (CSPRNG), not random.choice
+        (Mersenne Twister). Verify that the call is routed through the secrets module.
+        """
+        with patch('users.models.tokens.secrets.choice', wraps=secrets.choice) as mock_choice:
+            value = Token.generate()
+
+        self.assertEqual(mock_choice.call_count, TOKEN_DEFAULT_LENGTH,
+                         "secrets.choice must be called once per token character")
+        self.assertEqual(len(value), TOKEN_DEFAULT_LENGTH)
+        self.assertTrue(all(c in TOKEN_CHARSET for c in value),
+                        "Generated token must only contain characters from TOKEN_CHARSET")
+
+    def test_generate_length_parameter(self):
+        """
+        Token.generate(length=N) returns a string of exactly N characters from TOKEN_CHARSET.
+        """
+        for length in (8, 20, TOKEN_DEFAULT_LENGTH, 64):
+            value = Token.generate(length=length)
+            self.assertEqual(len(value), length, f"Expected length {length}, got {len(value)}")
+            self.assertTrue(all(c in TOKEN_CHARSET for c in value))
+
 
 class UserConfigTestCase(TestCase):