Bug #12677
closed
OpenVPN form validation issues
Added by Jim Pingle over 2 years ago.
Updated over 2 years ago.
Plus Target Version:
22.01
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
cipher
alone 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.
- Status changed from New to Pull Request Review
- Status changed from Pull Request Review to Feedback
- % Done changed from 0 to 100
- 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.
- 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.
- Status changed from Pull Request Review to Feedback
- % Done changed from 50 to 100
This has now been picked back to the RC branches for Plus and CE, and will be in the next RC build.
- Status changed from Feedback to Closed
- Private changed from Yes to No
Also available in: Atom
PDF