From d497c3b7a7504bb578874cc706dc75849ad84c2d Mon Sep 17 00:00:00 2001 From: Yuriy Andamasov Date: Sat, 26 Sep 2026 16:58:39 +0000 Subject: python: T8859: fail loudly on an invalid Diffie-Hellman keysize argument The Diffie-Hellman length check stringified the requested minimum keysize into a variable it never used, then converted the original argument again for the comparison. A non-numeric value therefore escaped as an uncaught ValueError from the comparison rather than being reported where it was passed in. The argument is now typed and validated up front: a non-integer raises TypeError, booleans included, and a non-positive integer raises ValueError. Both indicate a logic error in the calling script and should fail loudly instead of silently reporting an undersized key. The only callers today are the unit tests, so the contract is free to change. A file that exists but holds no readable parameters is user input rather than a logic error, and now fails the check instead of propagating the error from openssl. The bit-count pattern becomes a raw string, and the tests pass integer keysizes and cover the new paths. --- python/vyos/configverify.py | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) (limited to 'python') diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index c1c038a7a..17476f02d 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -424,24 +424,38 @@ def verify_vlan_config(config): verify_mtu_ipv6(c_vlan) -def verify_diffie_hellman_length(file, min_keysize): - """ Verify Diffie-Hellamn keypair length given via file. It must be greater - then or equal to min_keysize """ +def verify_diffie_hellman_length(file: str, min_keysize: int) -> bool: + """ Verify Diffie-Hellman keypair length given via file. It must be greater + than or equal to min_keysize. + + Raises TypeError if min_keysize is not an integer and ValueError if it is + not a positive integer - both indicate a logic error in the caller. An + unreadable or malformed file is user input, not a logic error, and simply + fails the check. """ import os import re from vyos.utils.process import cmdl - try: - keysize = str(min_keysize) - except: - return False + if isinstance(min_keysize, bool) or not isinstance(min_keysize, int): + raise TypeError( + f'min_keysize must be an integer, got {type(min_keysize).__name__}') + if min_keysize < 1: + raise ValueError( + f'min_keysize must be a positive integer, got {min_keysize}') if os.path.exists(file): - out = cmdl(['openssl', 'dhparam', '-inform', 'PEM', '-in', file, '-text']) - prog = re.compile('\d+\s+bit') - if prog.search(out): - bits = prog.search(out)[0].split()[0] - if int(bits) >= int(min_keysize): + try: + out = cmdl( + ['openssl', 'dhparam', '-inform', 'PEM', '-in', file, '-text']) + except OSError: + # Not a readable PEM encoded Diffie-Hellman parameter file + return False + + prog = re.compile(r'\d+\s+bit') + match = prog.search(out) + if match: + bits = match[0].split()[0] + if int(bits) >= min_keysize: return True return False -- cgit v1.2.3 From 514dfbc9ba33dd1a18a3a891d1a7d23ce9e959ee Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 26 Sep 2026 21:05:40 +0200 Subject: configverify: T8859: remove dead code for verify_diffie_hellman_length() Commit a9e9c4acfa ("pki: openvpn: T3642: Migrate OpenVPN to PKI and refactor") was the last caller of verify_diffie_hellman_length() despite the build time testcases. Unused code is dead code, drop it. --- python/vyos/configverify.py | 37 ------------------------------ src/tests/test_configverify.py | 52 ------------------------------------------ 2 files changed, 89 deletions(-) delete mode 100644 src/tests/test_configverify.py (limited to 'python') diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index 17476f02d..89f1b7ad7 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -423,43 +423,6 @@ def verify_vlan_config(config): verify_mtu_parent(c_vlan, s_vlan) verify_mtu_ipv6(c_vlan) - -def verify_diffie_hellman_length(file: str, min_keysize: int) -> bool: - """ Verify Diffie-Hellman keypair length given via file. It must be greater - than or equal to min_keysize. - - Raises TypeError if min_keysize is not an integer and ValueError if it is - not a positive integer - both indicate a logic error in the caller. An - unreadable or malformed file is user input, not a logic error, and simply - fails the check. """ - import os - import re - from vyos.utils.process import cmdl - - if isinstance(min_keysize, bool) or not isinstance(min_keysize, int): - raise TypeError( - f'min_keysize must be an integer, got {type(min_keysize).__name__}') - if min_keysize < 1: - raise ValueError( - f'min_keysize must be a positive integer, got {min_keysize}') - - if os.path.exists(file): - try: - out = cmdl( - ['openssl', 'dhparam', '-inform', 'PEM', '-in', file, '-text']) - except OSError: - # Not a readable PEM encoded Diffie-Hellman parameter file - return False - - prog = re.compile(r'\d+\s+bit') - match = prog.search(out) - if match: - bits = match[0].split()[0] - if int(bits) >= min_keysize: - return True - - return False - def verify_common_route_maps(config): """ Common helper function used by routing protocol implementations to perform diff --git a/src/tests/test_configverify.py b/src/tests/test_configverify.py deleted file mode 100644 index a8476d849..000000000 --- a/src/tests/test_configverify.py +++ /dev/null @@ -1,52 +0,0 @@ -# Copyright VyOS maintainers and contributors -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License version 2 or later as -# published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -import os - -from unittest import TestCase -from vyos.configverify import verify_diffie_hellman_length -from vyos.utils.process import cmdl - -dh_file = '/tmp/dh.pem' -dh_file_invalid = '/tmp/dh_invalid.pem' - -class TestVerifyDiffieHellmanLength(TestCase): - def setUp(self): - pass - - def test_dh_key_none(self): - self.assertFalse(verify_diffie_hellman_length('/tmp/non_existing_file', 1024)) - - def test_dh_key_512(self): - key_len = 512 - cmdl(['openssl', 'dhparam', '-out', dh_file, str(key_len)]) - self.assertTrue(verify_diffie_hellman_length(dh_file, key_len)) - - def test_dh_key_malformed(self): - with open(dh_file_invalid, 'w') as f: - f.write('this is not a PEM encoded file') - self.assertFalse(verify_diffie_hellman_length(dh_file_invalid, 512)) - os.unlink(dh_file_invalid) - - def test_dh_keysize_not_integer(self): - with self.assertRaises(TypeError): - verify_diffie_hellman_length(dh_file, '1024') - with self.assertRaises(TypeError): - verify_diffie_hellman_length(dh_file, True) - - def test_dh_keysize_not_positive(self): - with self.assertRaises(ValueError): - verify_diffie_hellman_length(dh_file, -512) - with self.assertRaises(ValueError): - verify_diffie_hellman_length(dh_file, 0) -- cgit v1.2.3