Merge #14742: Properly generate salt in rpcauth.py

6be7d14d24 Properly generate salt in rpcauth.py, update tests (Carl Dong)

Pull request description:

  Previously, when iterating over bytes of the generated salt to construct
  a hex string, only one character would be outputted when the byte is
  less than 0x10. Meaning that for a 16 byte salt, the hex string might be
  less than 32 characters and collisions would occur.

Tree-SHA512: 7038ecbbac846cd1851112396acd8a04475685f5b6f786e4e7316acba4a56cc711c275b7f52f0f2b6bc6cfdc0c0d9d39c3afeb2c0aff3a30fde516bf642fdf9f

share/rpcauth/rpcauth.py

@@ -5,17 +5,13 @@
 
 import sys
 import os
-from random import SystemRandom
 import base64
+from binascii import hexlify
 import hmac
 
-def generate_salt():
-    # This uses os.urandom() underneath
-    cryptogen = SystemRandom()
-
-    # Create 16 byte hex salt
-    salt_sequence = [cryptogen.randrange(256) for _ in range(16)]
-    return ''.join([format(r, 'x') for r in salt_sequence])
+def generate_salt(size):
+    """Create size byte hex salt"""
+    return hexlify(os.urandom(size)).decode()
 
 def generate_password():
     """Create 32 byte b64 password"""
@@ -32,7 +28,8 @@ def main():
 
     username = sys.argv[1]
 
-    salt = generate_salt()
+    # Create 16 byte hex salt
+    salt = generate_salt(16)
     if len(sys.argv) > 2:
         password = sys.argv[2]
     else:

test/util/rpcauth-test.py

@@ -24,8 +24,8 @@ class TestRPCAuth(unittest.TestCase):
         self.rpcauth = importlib.import_module('rpcauth')
 
     def test_generate_salt(self):
-        self.assertLessEqual(len(self.rpcauth.generate_salt()), 32)
-        self.assertGreaterEqual(len(self.rpcauth.generate_salt()), 16)
+        for i in range(16, 32 + 1):
+            self.assertEqual(len(self.rpcauth.generate_salt(i)), i * 2)
 
     def test_generate_password(self):
         password = self.rpcauth.generate_password()
@@ -34,7 +34,7 @@ class TestRPCAuth(unittest.TestCase):
         self.assertEqual(expected_password, password)
 
     def test_check_password_hmac(self):
-        salt = self.rpcauth.generate_salt()
+        salt = self.rpcauth.generate_salt(16)
         password = self.rpcauth.generate_password()
         password_hmac = self.rpcauth.password_to_hmac(salt, password)