From 41f7da6633d3270468027ab387e3bfbcab4157c6 Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Tue, 15 Sep 2026 11:15:48 +0200 Subject: pki: T9323: make the elliptic curve fallback reachable The "default to SECP256R1" branch stored an instance where the other branch stored the class, so the shared call at the end invoked the instance and raised "'SECP256R1' object is not callable". Any curve size without a matching SECPR1 - the very case the default exists for - aborted instead of falling back. Take the default through getattr() so both paths carry the class. --- python/vyos/pki.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'python') diff --git a/python/vyos/pki.py b/python/vyos/pki.py index e9f961911..b014c7234 100644 --- a/python/vyos/pki.py +++ b/python/vyos/pki.py @@ -112,12 +112,10 @@ def encode_dh_parameters(dh_parameters): # EC Helper def get_elliptic_curve(size): - curve_func = None name = f'SECP{size}R1' - if hasattr(ec, name): - curve_func = getattr(ec, name) - else: - curve_func = ec.SECP256R1() # Default to SECP256R1 + # getattr() must yield the class, not an instance - the caller below is the + # one that instantiates it + curve_func = getattr(ec, name, ec.SECP256R1) # Default to SECP256R1 return curve_func() # Creation functions -- cgit v1.2.3 From 609b8d4d8008729af2478029b8e25ae4359ad56b Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Tue, 15 Sep 2026 11:15:59 +0200 Subject: 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. --- python/vyos/pki.py | 30 +++++++++++++++++------- smoketest/scripts/cli/test_interfaces_openvpn.py | 4 ++-- src/op_mode/pki.py | 7 ++++++ 3 files changed, 31 insertions(+), 10 deletions(-) (limited to 'python') 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([ diff --git a/smoketest/scripts/cli/test_interfaces_openvpn.py b/smoketest/scripts/cli/test_interfaces_openvpn.py index 4f1021a36..191fde970 100755 --- a/smoketest/scripts/cli/test_interfaces_openvpn.py +++ b/smoketest/scripts/cli/test_interfaces_openvpn.py @@ -33,7 +33,7 @@ from vyos.utils.process import is_systemd_service_running from vyos.utils.file import read_file from vyos.pki import create_certificate from vyos.pki import create_certificate_request -from vyos.pki import create_private_key +from vyos.pki import create_ec_private_key from vyos.pki import encode_certificate from vyos.pki import encode_private_key from vyos.utils.network import get_interface_config @@ -99,7 +99,7 @@ def generate_pki(): 'organization': 'VyOS', 'common_name': 'VyOS Test', } - key = create_private_key('ec', 256) + key = create_ec_private_key(256) request = create_certificate_request(subject, key) # is_ca gives it both CLIENT_AUTH and SERVER_AUTH, so one certificate # serves either end of a tunnel diff --git a/src/op_mode/pki.py b/src/op_mode/pki.py index 575aa89e0..d53be1f92 100755 --- a/src/op_mode/pki.py +++ b/src/op_mode/pki.py @@ -41,6 +41,7 @@ from vyos.pki import create_certificate from vyos.pki import create_certificate_request from vyos.pki import create_certificate_revocation_list from vyos.pki import create_private_key +from vyos.pki import create_ec_private_key from vyos.pki import create_dh_parameters from vyos.pki import load_certificate from vyos.pki import load_certificate_request @@ -466,6 +467,12 @@ def generate_private_key(): valid_responses=size_valid, ) + # An elliptic curve key is picked by curve, not by modulus size, so it gets + # its own constructor - that keeps the number below out of the function that + # also builds RSA and DSA keys + if key_type == 'ec': + return create_ec_private_key(size), key_type + return create_private_key(key_type, size), key_type -- cgit v1.2.3