OpenVPN: Fix handling of CNs in Client Certificates #33
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This addresses Bugzilla issue #10552
OpenVPN client certificates were saved by regexing the 'CN=' string in the output
of '/usr/bin/openssl x509 -text -in <certificate>'. However, the output of this
command may not print the correct CN. For example, if the certificate's subject
contains an emailAddress attribute, it will be appended to the CN separated with
a slash ('/') character. There might also be some other edge cases with unusual
certificate names that cause the regex not to work correctly.
This change makes OpenSSL output the subject with each attribute on one line which
makes parsing the CN much easier and safer. The CN is now guaranteed to be on
exactly one line.
…tly saved CNs This addresses Bugzilla issue #10552 The regex that was used to extract the CN from the certificate subject extracted everything after the 'CN=' part which included possible other attributes in the subject, e.g. an emailAddress. This commit does mainly fix the regex. But since the same issue occurred when client certificates were saved, some compatibility code had to be added to still support previously and incorrectly saved certificates.
This addresses Bugzilla issue #10552 In case of incorrectly saved CNs (i.e. CNs containing additional attributs from the certificate subject), the connection status of the client's was always 'DISCONNECTED'. This commit fixes this issue by matching the client's CN with the entry in ovpnconfig.
This addresses Bugzilla issue #10552 The OpenSSL documentation [1] is pretty clear about how the subject is formatted in a multiline output. Thus, the regex to grep the CN can be made much stricter. [1] https://www.openssl.org/docs/apps/x509.html#NAME_OPTIONS
This addresses Bugzilla issue #10552 This commit contains some improvement in matching CNs while remaining compatible with possible incorrectly saved CNs in ovpnconfig.
This addresses Bugzilla issue #10552 The OpenSSL specification says 4 spaces at the beginning. So this should be part of the regular expression.
This addresses Bugzilla issue #10552 The previous solution to verify a client's CN was to construct a regular expression to match it against the entries in ovpnconfig. This must be avoided in order the client's CN is not guaranteed to be spoiled.
mtremer
pushed a commit
that referenced
this pull request
Oct 22, 2021
- Update from 2.35 (2006) to 2.73 (2020)
- Update of rootfile
- Updated version of perl-GD required ExtUtils-PkgConfig for build. Seperate patch
to build that is part of this series
- Changelog
2.73 * allow --options override the libgd options. Not recommended.
See GH #33 and RT #130045
2.72 * fix CVE 2019-6977 colorMatch for older unpatched libgd versions.
This is a severe security problem, an exploitable heap-overflow.
See https://nvd.nist.gov/vuln/detail/CVE-2019-6977
2.71 * skip Test::Fork on freebsd (GH #25)
2.70 * fixes for hardened CCFLAGS with -Werror (RT #128167)
2.69 * little spelling error, GH #29 Xavier Guimard
2.68 * fix GD::Polygon->clear, RT #124463 Michael Cain
2.67 * fix thread-safety for GD::Simple %COLORS (#26 melak)
* fix arc start-angle docs, RT #123277 Andrew G Gray
* improve setBrush docs, RT #123194 Andrew G Gray
* improve StringFT docs, RT #123193
* replace MacOSX by darwin, and not by Mac OS X/macOS as suggested
in PR #24
* add GD::Image->_file method as suggested in RT #60488 by Kevin Ryde,
also the helper GD::supportsFileType
2.66 * throw proper error on newFrom* with not-existing file
* add t/transp.t from RT #40525
* Improve RT #54366 multiple gd.h warning
* better doc for GD::Simple->arc
* fix ANIMGIF with libgd 2.3.0-dev
2.65 * fix --gdlib_config_path to accept an argument (fperrad)
2.64 * Update doc for LIBGD_VERSION()
* Fix 5.6.2, which does not have float in its typemap
2.63 * renamed VERSION() to LIBGD_VERSION(), RT #121307.
It was treated magically by "use GD 2.18"
2.62 * fixed wrong <5.14 code generated with ExtUtils::Constants
RT #121297. Don't generate const-xs.inc, only when missing.
* add -liconv on hpux also (our pkgconfig parser cannot handle it)
2.61 * add CONFIGURE_REQUIRES META
* add --gdlib_config_path
* add Image Filters: scatter, pixelate, negate, grayscale, brightness,
contrast, color, selectiveBlur, edgeDetectQuick, gaussianBlur, emboss,
meanRemoval, smooth, copyGaussianBlurred
* add palette methods: createPaletteFromTrueColor,
neuQuant (but discouraged), colorMatch.
* add interpolation methods: copyScale, copyRotateInterpolated,
interpolationMethod.
* add double GD::VERSION
* add all gd.h constants
2.60 * add missing methods newFromWBMP, newFromXbm,
(RT #68784) and some missing docs
* Add --lib_fontconfig_path, --fcgi options
* rewrote most of the XS code
* cleanup Makefile.PL #20
2.59 * error on failing libgd calls
* fix colorClosestAlpha, colorAllocateAlpha
* add missing documentation
2.58 * fix VERSION_STRING for 2.0.x
* honor --lib_gd_path specific gdlib-config
* Loosen the comparison tests with GDIMAGETYPE ne gd2
* Improve gdlib-config parsing (PR #17), esp. with 2.0.34
2.57 * fix Jpeg magic number detection RT #26146
* fix RGB - HSV roundtrips: RT #120572 by J2N-FORGET
* fix -print-search-dirs errors RT #106265
* co-maint to rurban
* add hv_fetchs, CI smokers
* add GD::VERSION_STRING api
2.56_03 * add alpha method
* improve option handling
* fix meta data
2.56_02 * fix feature extraction >= 2.2 [RT #119459]
2.56_01 * rm Build.PL, fix permissions, fix for missing gdlib-config
2.56 * Fix Makefile.PL so that it works again.
2.55 * Great simplification of regression framework ought to fix make test problems.
* Replace ExtUtils::MakeMaker script with Module::Build system
(just in time for Module::Build to be deprecated).
* Remove archaic qd.pl (for creating QuickDraw picts) from distribution.
2.54 Patch from yurly@unet.net to fix image corruption in rotate180 when image height is odd.
2.53 Points to Gabor Szabo's GD::Simple tutorial, and fix link to repository.
2.52 Fix regression tests to run on Ubuntu 12.04 64bit.
2.51 Fix misleading warning message about location of gd.h file.
2.50 Fix gdUseFontConfig so that it can be called as a class method.
2.49 Add GitHub information to README.
2.48 Fix compile crash on windows and strawberry (https://rt.cpan.org/Public/Bug/Display.html?id=67990).
2.47 Fix compilation on older perl's without the Newxz macros.
2.46 Added a basic "use" test for GD::Simple
2.45 Clarified the GD license. There is now a formal LICENSE file in the package.
2.44 GD::Group now installed properly.
Quenched compiler warning caused by Newxs() calls.
2.43 Added "transparent" color to GD::Simple.
Fixed Makefile so that GD/Image.pm depends both on GD/Image.pm.PLS and .config.cache
2.42 Fixed magic number detection to autodetect certain missed jpeg files (thanks to Mike Walker)
2.41 Added backend support for grouping features in GD::SVG module.
2.40 ** Do not use - contains a bug **
2.39 Makefile.PL will refuse to run if the proper version of libgd is unavailable.
2.38 Fixed bizarre warning about /usr/include/gd.h != /usr/include/gd.h.
2.37 GD/Image.pm did not bring in croak() properly, meaning that incorrect error messages are printed out when any of the newFromXXX() calls are made.
2.36 Instructions on using gdAntiAliased with palette images.
Tested-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Arne Fitzenreiter <arne_f@ipfire.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: This PR solves Bugzilla issue #10552
This PR fixes a few issues in the handling of OpenVPN client CNs. The main problem is that the CN is currently expected to be the last attribute in a certificate's subject string. In case the subject contains further attributes after the CN such as an
emailAddress, the CN is incorrectly parsed, which leads to misbehaviour at a few places:Upload of client certificates
When a client certificate with further attributes after the CN is uploaded, the resulting entry in
/var/ipfire/ovpn/ovpnconfigcontains the CN and all other attributes separated by a slash (/) character. The reason is that/srv/web/ipfire/cgi-bin/ovpnmain.cgiexecutes/usr/bin/openssl x509 -text ...and tries to grep the CN from the output of the command. However, the-textoption causes attributes after the CN to be appended to the CN with a slash character as separator.Example
The certificate with the subject
C=XX, L=Xxxxxx, O=xxx, OU=XX, CN=ovpnClient, emailAddress=ovpnClient@example.comwill generate this name in ovpnconfig when uploaded:ovpnClient/emailAddress=ovpnClient@example.comtls-verify is not working
The script
/usr/lib/openvpn/verifywhich is executed by OpenVPN during the TLS handshake parses the CN incorrecly as well. It will contain all attributes of the subject after the CN separated by a comma. Using the example from the last chapter, the parsed CN will beovpnClient, emailAddress=ovpnClient@example.com. This CN is then compared with the entries in/var/ipfire/ovpn/ovpnconfigwhich will never match. As a consequence, the TLS handshake will fail with this error:Connection status in the Web UI
To display the connection status of the OpenVPN clients the script
/srv/web/ipfire/cgi-bin/ovpnmain.cgireads the correct CNs of the connected clients from/var/log/ovpnserver.logand tries to compare them with the (possibly wrong) CNs in/var/ipfire/ovpn/ovpnconfig. As a consequence, the clients will always be displayed asDISCONNECTEDeven if they are connected.This PR fixes all the issues described above and stays compatible with previously created wrong entries in
/var/ipfire/ovpn/ovpnconfig:/usr/lib/openvpn/verifyscript will parse the CN correctly from the subject. If it cannot be matched exactly with an entry in/var/ipfire/ovpn/ovpnconfig, it uses a regex to match it with an eventually incorrectly saved CN./srv/web/ipfire/cgi-bin/ovpnmain.cgihas been fixed to use the correct CN when uploading or creating a client certificate./srv/web/ipfire/cgi-bin/ovpnmain.cgiwill use a regex to match the CNs of the connected clients to the possibly wrong CNs in/var/ipfire/ovpn/ovpnconfig.