Bug #12677
closed
OpenVPN form validation issues
100%
Description
There are a few issues with how we currently handle the data cipher list in OpenVPN client and server pages, including:
- When an OpenVPN server or client instance is set to shared key mode, the data cipher list is not validated even though it is included in the OpenVPN configuration
- The data cipher list should not be included in the configuration for shared key mode, it should be using
cipheralone similar to when NCP is disabled (though OpenVPN automatically disables cipher negotiation since it's not in client/server mode) - The data cipher list should always be validated and stored in the configuration since someone may temporarily decide to change modes or toggle NCP and it shouldn't lose their cipher list/ordering preferences
- The GUI does not note on the data cipher and fallback fields that they are treated differently in shared key mode
- The client and server list views do not accurately reflect the ciphers being used in shared key mode, it should not include the contents of the data ciphers list in shared key mode or when NCP is diabled
Most of these are cosmetic and harmless, but the validation issue could potentially be used to include unintended directives in the configuration by someone with access to the client and server pages.
Updated by Jim Pingle almost 3 years ago
Updated by Jim Pingle almost 3 years ago
- Status changed from New to Pull Request Review
Updated by Jim Pingle almost 3 years ago
- Status changed from Pull Request Review to Feedback
- % Done changed from 0 to 100
Applied in changeset 78ce96a9af3b2ab5159ef6623078bfc4b15f8a89.
Updated by Jim Pingle almost 3 years ago
- Subject changed from OpenVPN Data Cipher list validation and usage issues to OpenVPN form validation issues
- Status changed from Feedback to In Progress
- % Done changed from 100 to 50
This affects a few more fields: allow_compression, protocol, dev_mode, digest, verbosity_level
But validating those will be much simpler than the changes needed for data_ciphers. While we're here, add validation for any other similar selection lists that don't already have validation.
Updated by Jim Pingle almost 3 years ago
- Status changed from In Progress to Pull Request Review
MR for the remaining validation:
https://gitlab.netgate.com/pfSense/pfSense/-/merge_requests/564
Added validation for the following fields:
- OpenVPN Server:
- mode, dev_mode, protocol, interface, ecdh_curve, digest, engine, cert_depth, caref, crlref, certref, allow_compression, compression, tlsauth_keydir, netbios_ntype, serverbridge_interface, verbosity_level
- OpenVPN Client:
- mode, dev_mode, protocol, interface, digest, engine, caref, crlref, certref, allow_compression, compression, proxy_authtype, tlsauth_keydir, verbosity_level
- OpenVPN Client-Specific Overrides:
- server_list, netbios_ntype
Focus testing on allow_compression, protocol, dev_mode, digest, verbosity_level primarily, since those were the mentioned vectors, but the other field validation needs testing as well. I tested it locally on a couple VMs with a wide variety of server and client types without issue. Valid entries were allowed, invalid entries were blocked.
Updated by Christian McDonald almost 3 years ago
- Status changed from Pull Request Review to Feedback
Tested and Merged.
Updated by Jim Pingle almost 3 years ago
- % Done changed from 50 to 100
Applied in changeset ba815f3d219e5bdf404be859e723db2ff0c9258c.
Updated by Jim Pingle almost 3 years ago
This has now been picked back to the RC branches for Plus and CE, and will be in the next RC build.