From 97a1375a82393f01620e411639893e519cef3fc3 Mon Sep 17 00:00:00 2001
From: Brian Tiemann <btman@mac.com>
Date: Wed, 10 Jun 2026 12:34:43 -0400
Subject: [PATCH] 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>
---
 netbox/users/models/tokens.py     |  4 ++--
 netbox/users/tests/test_models.py | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/netbox/users/models/tokens.py b/netbox/users/models/tokens.py
index b47064c84..3c5afa94e 100644
--- a/netbox/users/models/tokens.py
+++ b/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):
         """
diff --git a/netbox/users/tests/test_models.py b/netbox/users/tests/test_models.py
index 7c1f5556e..fc59fe156 100644
--- a/netbox/users/tests/test_models.py
+++ b/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):