[Openvpn-devel] [PATCH v3] Support x509 field list to be username
OpenVPN has the ability to choose different x509 field in case "CN" can't be use used to be unique connected username since commit 935c62be9c0c8a256112df818bfb8470586a23b6. Unfortunately it's not enough in case client has multiple and valid certificates from PKI for different devices (ex. laptop, mobile, etc) with the same CN/UID. Having --duplicate-cn as a workaround helps only partially: clients can be connected, but it breaks coexistance with --ifconfig-pool-persist, --client-config-dir and opens doors to DoS possibility since same client device (with the same cert) being reconnected no more replaces previously connected session, so it can exhaust server resources (ex. address pool) and can prevent other clients to be connected. With this patch, multiple x509 fields incl. "serialNumber" can be chosen to be username as --x509-username-field space-separated parameters. Multiple fields will be joined into one username using '/' delimeter for consistency with CN/addr logging and preserving ability for hierarchical ccd. As long as resulting username is unique, --duplicate-cn will not be required. Default value is preserved as "CN" only. Openssl backend is the only supported at the moment, since so far MbedTLS has no alt user name support at all. v2: conform C99, man update, fix typos v3: reuse buffer methods, drop delimiter define, use memcpy Signed-off-by: Vladislav Grishenko --- doc/man-sections/tls-options.rst | 9 -- src/openvpn/init.c | 6 ++-- src/openvpn/options.c| 49 +--- src/openvpn/options.h| 4 +-- src/openvpn/ssl_common.h | 8 +- src/openvpn/ssl_verify.c | 49 +--- src/openvpn/ssl_verify_openssl.c | 14 + 7 files changed, 96 insertions(+), 43 deletions(-) diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst index 8c2db7cd..8449f5dc 100644 --- a/doc/man-sections/tls-options.rst +++ b/doc/man-sections/tls-options.rst @@ -632,13 +632,13 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa options can be defined to track multiple attributes. --x509-username-field args - Field in the X.509 certificate subject to be used as the username + Field list in the X.509 certificate subject to be used as the username (default :code:`CN`). Valid syntax: :: - x509-username-field [ext:]fieldname + x509-username-field [ext:]fieldname [[ext:]fieldname...] Typically, this option is specified with **fieldname** as either of the following: @@ -646,6 +646,7 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa x509-username-field emailAddress x509-username-field ext:subjectAltName + x509-username-field CN serialNumber The first example uses the value of the :code:`emailAddress` attribute in the certificate's Subject field as the username. The second example @@ -653,7 +654,9 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa ``fieldname`` :code:`subjectAltName` be searched for an rfc822Name (email) field to be used as the username. In cases where there are multiple email addresses in :code:`ext:fieldname`, the last occurrence - is chosen. + is chosen. The last example uses the value of :code:`CN` attribute in + the Subject field and the certificate's :code:`serialNumber` field in + hex delimited by slash symbol as the resulting username. When this option is used, the ``--verify-x509-name`` option will match against the chosen ``fieldname`` instead of the Common Name. diff --git a/src/openvpn/init.c b/src/openvpn/init.c index dfa045b0..b748d985 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2905,14 +2905,14 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.crl_file_inline = options->crl_file_inline; to.ssl_flags = options->ssl_flags; to.ns_cert_type = options->ns_cert_type; -memmove(to.remote_cert_ku, options->remote_cert_ku, sizeof(to.remote_cert_ku)); +memcpy(to.remote_cert_ku, options->remote_cert_ku, sizeof(to.remote_cert_ku)); to.remote_cert_eku = options->remote_cert_eku; to.verify_hash = options->verify_hash; to.verify_hash_algo = options->verify_hash_algo; #ifdef ENABLE_X509ALTUSERNAME -to.x509_username_field = (char *) options->x509_username_field; +memcpy(to.x509_username_field, options->x509_username_field, sizeof(to.x509_username_field)); #else -to.x509_username_field = X509_USERNAME_FIELD_DEFAULT; +to.x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT; #endif to.es = c->c2.es; to.net_ctx = >net_ctx; diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 8bf82c57..f7e76d3e 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -876,7 +876,7 @@ init_options(struct options *o, const bool init_gc) o->tls_cert_profile = NULL; o->ecdh_curve = NULL; #ifdef ENABLE_X509ALTUSERNAME -
Re: [Openvpn-devel] [PATCH 1/2] Support multiple x509 field list to be username
Hi,
>> #ifdef ENABLE_X509ALTUSERNAME
>> -to.x509_username_field = (char *) options->x509_username_field;
>> +memmove(to.x509_username_field, options->x509_username_field,
>> sizeof(to.x509_username_field));
>> #else
>> -to.x509_username_field = X509_USERNAME_FIELD_DEFAULT;
>> +to.x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT;
>> #endif
>
> Can't we use the same code in both cases? And why memmove instead memcpy?
Can't, options struct contains x509_username_field only with
ENABLE_X509ALTUSERNAME.
Memmove is used to be consistent with remote_cert_ku copying several lines
above, although I think memcpy is enough for here and above - tls and options
mem regions can't be crossed.
>> /* Default field in X509 to be username */
>> #define X509_USERNAME_FIELD_DEFAULT "CN"
>> +#define X509_USERNAME_FIELD_DELIMITER '/'
>
> This seems to be used nowhere.
It's used down below as a field delimiter for possible easy change:
> +{
> +*buf = X509_USERNAME_FIELD_DELIMITER;
> }
I'll drop it with just "/%s" in v3.
>> +#define TLS_USERNAME_LEN 128
>
> NAK on this one. The X509 CN name length is max 64 characeters. So you
> either have stick to that limit in your patch or have to untagle the CN
> name length vs the field you are using, which might end up in a seperate
> patch.
> ...
> Not really wrong but we normally do stuff like appending etc to a string
> with our buf method/ classes. I would prefer we do it the same here
> instead of doing it completely different (espeically the !! is nowhere
> else found in the code base).
Good point, 64 limit can be kept for each backend_x509_get_username() call,
subsequent appending will be done via buffer methods - this way buffer size
will be untied from TLS_USERNAME_LEN.
> C89 style instead C99. The !!i feels weird. It is the same as max(i, 1)
> but less readable.
Yes, sure.
--
Best Regards, Vladislav Grishenko
-Original Message-
From: Arne Schwabe
Sent: Friday, August 14, 2020 6:56 PM
To: Vladislav Grishenko ;
openvpn-devel@lists.sourceforge.net
Subject: Re: [Openvpn-devel] [PATCH 1/2] Support multiple x509 field list to be
username
Am 28.07.20 um 00:13 schrieb Vladislav Grishenko:
> OpenVPN has the ability to choose different x509 field in case "CN"
> can't be use used to be unique connected username since commit
> 935c62be9c0c8a256112df818bfb8470586a23b6.
> Unfortunately it's not enough in case client has multiple and valid
> certificates from PKI for different devices (ex. laptop, mobile, etc)
> with the same CN/UID.
>
> Having --duplicate-cn as a workaround helps only partially - clients
> can be connected, but it breaks coexistance with --ifconfig-pool-persist,
> --client-config-dir and opens doors to DoS possibility since same client
> device (with the same cert) being reconnected doesn't replace previously
> connected session no more, so can exhaust server ressources (ex. address
> pool) and can prevent other clients to be connected.
>
> With this patch, multiple x509 fields incl. "serialNumber" can be chosen
> to be username as --x509-username-files space-separated parameters.
> Multiple fields will be joined into one username using '/' delimeter for
> consistency with CN/addr logging and preserving ability for hierarchical
> ccd. As long as resulting username is unique, --duplicate-cn will not
> be required. Default value is preserved as "CN" only.
>
> Openssl backend is the only supported at the moment, since so far MbedTLS
> has no alt user name support at all.
> ---
> doc/man-sections/tls-options.rst | 9 ---
> src/openvpn/init.c | 4 +--
> src/openvpn/options.c| 46 ++--
> src/openvpn/options.h| 4 +--
> src/openvpn/ssl.h| 1 +
> src/openvpn/ssl_common.h | 8 +-
> src/openvpn/ssl_verify.c | 35
> src/openvpn/ssl_verify_openssl.c | 15 +++
> 8 files changed, 83 insertions(+), 39 deletions(-)
>
> diff --git a/doc/man-sections/tls-options.rst
> b/doc/man-sections/tls-options.rst
> index 8c2db7cd..301f8be4 100644
> --- a/doc/man-sections/tls-options.rst
> +++ b/doc/man-sections/tls-options.rst
> @@ -632,13 +632,13 @@ certificates and keys:
> https://github.com/OpenVPN/easy-rsa
>options can be defined to track multiple attributes.
>
> --x509-username-field args
> - Field in the X.509 certificate subject to be used as the username
> + Field list in the X.509 certificate subject to be used as the username
>(default :code:`CN`).
>
>Valid syntax:
>::
>
> - x509-username-field [ext:]fieldname
> + x509-username-field [ext:]fieldname [[ext:]fieldname...]
>
>Typically, this option is specified with **fieldname** as
>either of the following:
> @@ -646,6 +646,7 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
>
> x509-username-field emailAddress
> x509-username-field
Re: [Openvpn-devel] [PATCH 0/2] Fix mistyped option names
Hi, On Sat, Aug 15, 2020 at 02:05:20PM +0200, Magnus Kroken wrote: > While preparing a 2.5_beta1 package, I had to look > back and forth betwenn various sources to get option > names right. It seems that in some places names are > altered to make grammatical sense, which I find > confusing. Thanks for pointing that out, and of course you are right that this needs fixing. Thanks for your patches, will apply ASAP (tomorrow-ish). gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] 2.5-beta-1 Windows installer problems
Comment inline: On 15/08/2020 19:29, tincanteksup wrote: Hi, I would like to document the very strange issues I had testing the 2.5 MSI installers. The first test was on a Win7-Ent/32bit VM with 32bit installer. The second test was a real PC with Win7-HomePro/64bit (yeah, user did not want w10) 64bit installer. The third test was Win10/64bit VM 64bit installer. The results were almost identical: 1. Logged in as admin user. 2. Installed over previous installation. Completed apparently OK ? 3. On testing the VPN I found that: IPv6 worked (could ping both ways) IPv4 did not (could not ping either way) Firewall disabled. Dual stack VPN. 4. Logged out/Logged in as normal user. 5. OpenVPN Installer forced a re-install of the TAP/wintun drivers. The user could select "do not install" but the installer was auto-run. I did "Install" the drivers as non-admin user and was *not* prompted for admin password. The user was *not* prompted for admin password. 6. Using the same VPN config as above, the VPN now functions correctly. The only difference was that on Win10 I could not test logging back in as a standard user because my VM could not be activated. I do not know where would be the bast place to raise these issues and so sent my feedback to the list for further consideration. Regards Richard ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] 2.5-beta-1 Windows installer problems
Hi, I would like to document the very strange issues I had testing the 2.5 MSI installers. The first test was on a Win7-Ent/32bit VM with 32bit installer. The second test was a real PC with Win7-HomePro/64bit (yeah, user did not want w10) 64bit installer. The third test was Win10/64bit VM 64bit installer. The results were almost identical: 1. Logged in as admin user. 2. Installed over previous installation. Completed apparently OK ? 3. On testing the VPN I found that: IPv6 worked (could ping both ways) IPv4 did not (could not ping either way) Firewall disabled. Dual stack VPN. 4. Logged out/Logged in as normal user. 5. OpenVPN Installer forced a re-install of the TAP/wintun drivers. The user could select "do not install" but the installer was auto-run. The user was *not* prompted for admin password. 6. Using the same VPN config as above, the VPN now functions correctly. The only difference was that on Win10 I could not test logging back in as a standard user because my VM could not be activated. I do not know where would be the bast place to raise these issues and so sent my feedback to the list for further consideration. Regards Richard ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 0/2] Fix mistyped option names
While preparing a 2.5_beta1 package, I had to look back and forth betwenn various sources to get option names right. It seems that in some places names are altered to make grammatical sense, which I find confusing. "Data cipher" and "compression" are simply words, and it makes sense to use the grammatically correct form whenever they are used. However I interpret --data-cipher, ``--data-cipher`` and --compression as literal, as the dashes and code highlighting hints that this is exactly how these are used in configuration and commands. There is no option --data-cipher in OpenVPN, so I would not expect the literal --data-cipher to appear anywhere, even when a sentence refers to one singular cipher in the list of --data-ciphers. Magnus Kroken (2): Changes.rst: fix mistyped option names doc: fix typos in cipher-negotiation.rst Changes.rst | 10 +- doc/man-sections/cipher-negotiation.rst | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) -- 2.20.1 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 2/2] doc: fix typos in cipher-negotiation.rst
Signed-off-by: Magnus Kroken --- doc/man-sections/cipher-negotiation.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/man-sections/cipher-negotiation.rst b/doc/man-sections/cipher-negotiation.rst index f1433052..a2feb5f9 100644 --- a/doc/man-sections/cipher-negotiation.rst +++ b/doc/man-sections/cipher-negotiation.rst @@ -38,7 +38,7 @@ options to avoid this behaviour. OpenVPN 3 clients - Clients based on the OpenVPN 3.x library (https://github.com/openvpn/openvpn3/) -do not have a configurable ``--ncp-ciphers`` or ``--data-cipher`` option. Instead +do not have a configurable ``--ncp-ciphers`` or ``--data-ciphers`` option. Instead these clients will announce support for all their supported AEAD ciphers (`AES-256-GCM`, `AES-128-GCM` and in newer versions also `Chacha20-Poly1305`). @@ -90,7 +90,7 @@ version. The default was never changed to ensure backwards compatibility. In OpenVPN 2.5 this behaviour has now been changed so that if the ``--cipher`` is not explicitly set it does not allow the weak ``BF-CBC`` cipher any more and needs to explicitly added as ``--cipher BFC-CBC`` or added to -``-data-ciphers``. +``--data-ciphers``. We strongly recommend to switching away from BF-CBC to a more secure cipher as soon as possible instead. -- 2.20.1 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 1/2] Changes.rst: fix mistyped option names
Signed-off-by: Magnus Kroken --- Changes.rst | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Changes.rst b/Changes.rst index 0aee3603..f67e1d76 100644 --- a/Changes.rst +++ b/Changes.rst @@ -34,7 +34,7 @@ Improved Data channel cipher negotiation Removal of BF-CBC support in default configuration: By default OpenVPN 2.5 will only accept AES-256-GCM and AES-128-GCM as data ciphers. OpenVPN 2.4 allows AES-256-GCM,AES-128-GCM and BF-CBC when -no --cipher and --ncp-cipher options are present. Accepting BF-CBC can be +no --cipher and --ncp-ciphers options are present. Accepting BF-CBC can be enabled by adding data-ciphers AES-256-GCM:AES-128-GCM:BF-CBC @@ -101,7 +101,7 @@ Linux VRF support TLS 1.3 support TLS 1.3 support has been added to OpenVPN. Currently, this requires OpenSSL 1.1.1+. -The options ``--tls-cipher-suites`` and ``--tls-groups`` have been +The options ``--tls-ciphersuites`` and ``--tls-groups`` have been added to fine tune TLS protocol options. Most of the improvements were also backported to OpenVPN 2.4 as part of the maintainance releases. @@ -112,7 +112,7 @@ Support setting DHCP search domain wintun support yet). Other platforms need to support this via ``--up`` script (Linux) or GUI (OSX/Tunnelblick). -per-client changing of ``--data-cipher`` or ``data-ciphers-fallback`` +per-client changing of ``--data-ciphers`` or ``data-ciphers-fallback`` from client-connect script/dir (NOTE: this only changes preference of ciphers for NCP, but can not override what the client announces as "willing to accept") @@ -213,9 +213,9 @@ User-visible Changes the client configuration almost immediately as result of the faster connection setup feature. -- ``--compression`` is nowadays considered risky, because attacks exist +- ``--compress`` is nowadays considered risky, because attacks exist leveraging compression-inside-crypto to reveal plaintext (VORACLE). So - by default, ``--compression xxx`` will now accept incoming compressed + by default, ``--compress xxx`` will now accept incoming compressed packets (for compatibility with peers that have not been upgraded yet), but will not use compression outgoing packets. This can be controlled with the new option ``--allow-compression yes|no|asym``. -- 2.20.1 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
