pki: T9323: give elliptic curve keys their own constructor

create_private_key() took one "size" for every key type, but the number
means entirely different things per type: for RSA and DSA it is a modulus
size, for EC it selects a curve. CodeQL is not path sensitive across the
key_type comparison, so it follows the 256 an EC caller passes into the
RSA and DSA branches that caller can never reach, and reports pki.py as
generating 256 bit RSA and DSA keys.

Build elliptic curve keys in create_ec_private_key() instead and route the
op mode generator and the OpenVPN smoketest there, so no curve size ever
enters the function that also builds RSA and DSA keys.
create_private_key() now refuses a key type it does not build rather than
returning None, so a caller cannot drift back into the old shape
unnoticed.

1 files changed, 22 insertions, 8 deletions

diff --git a/python/vyos/pki.py b/python/vyos/pki.py
index b014c7234..556c041dd 100644
--- a/python/vyos/pki.py
+++ b/python/vyos/pki.py
@@ -118,18 +118,32 @@ def get_elliptic_curve(size):
     curve_func = getattr(ec, name, ec.SECP256R1)  # Default to SECP256R1
     return curve_func()
 
+
 # Creation functions
 
+
+def create_ec_private_key(curve_size=256):
+    """An elliptic curve key is chosen by curve, not by modulus size: 256 here
+    selects SECP256R1 and says nothing about the strength a 256 bit RSA modulus
+    would have. Elliptic curve keys are built only here, so that number can
+    never reach the RSA and DSA generators - neither by accident nor in the
+    eyes of a static analyser."""
+    return ec.generate_private_key(get_elliptic_curve(curve_size))
+
+
 def create_private_key(key_type, key_size=None):
-    private_key = None
+    """Build an RSA or DSA key of key_size bits. Elliptic curve keys are
+    deliberately not built here - see create_ec_private_key()."""
     if key_type == 'rsa':
-        private_key = rsa.generate_private_key(public_exponent=65537, key_size=key_size)
-    elif key_type == 'dsa':
-        private_key = dsa.generate_private_key(key_size=key_size)
-    elif key_type == 'ec':
-        curve = get_elliptic_curve(key_size)
-        private_key = ec.generate_private_key(curve)
-    return private_key
+        return rsa.generate_private_key(public_exponent=65537, key_size=key_size)
+    if key_type == 'dsa':
+        return dsa.generate_private_key(key_size=key_size)
+    if key_type == 'ec':
+        raise ValueError(
+            'Elliptic curve keys are built by create_ec_private_key() - its '
+            'argument selects a curve, not a modulus size'
+        )
+    raise ValueError(f'Unsupported key type "{key_type}"')
 
 def create_certificate_request(subject, private_key, subject_alt_names=[]):
     subject_obj = x509.Name([