From 6be7d14d243eeeaaf6b4b98c3359c3e1695f2046 Mon Sep 17 00:00:00 2001
From: Carl Dong <accounts@carldong.me>
Date: Fri, 16 Nov 2018 23:24:57 -0800
Subject: Properly generate salt in rpcauth.py, update tests

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.
---
 share/rpcauth/rpcauth.py  | 15 ++++++---------
 test/util/rpcauth-test.py |  6 +++---
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/share/rpcauth/rpcauth.py b/share/rpcauth/rpcauth.py
index 13bef3d37a..cecc6c30a4 100755
--- a/share/rpcauth/rpcauth.py
+++ b/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:
diff --git a/test/util/rpcauth-test.py b/test/util/rpcauth-test.py
index 46e9fbc739..53058dc394 100755
--- a/test/util/rpcauth-test.py
+++ b/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)
 
-- 
cgit v1.2.3