| Message ID | 20211111192051.3706965-1-arne@rfc2549.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel,v5,OSSL,3.0] Allow loading of non default providers | expand |
Hi, On Thu, Nov 11, 2021 at 2:21 PM Arne Schwabe <arne@rfc2549.org> wrote: > This allows OpenVPN to load non-default providers. This is mainly > useful for loading the legacy provider with --providers legacy default > > Patch v4: use spaces to seperate providers, unload providers. > Patch v5: General cleanup, rename option to --providers, add > option to usage() and add an entry to Changes.rst > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > Changes.rst | 7 +++++++ > doc/man-sections/generic-options.rst | 12 +++++++++++ > src/openvpn/crypto_backend.h | 15 ++++++++++++++ > src/openvpn/crypto_mbedtls.c | 13 ++++++++++++ > src/openvpn/crypto_mbedtls.h | 3 +++ > src/openvpn/crypto_openssl.c | 30 ++++++++++++++++++++++++++++ > src/openvpn/crypto_openssl.h | 9 +++++++++ > src/openvpn/openvpn.c | 13 ++++++++++++ > src/openvpn/options.c | 8 ++++++++ > src/openvpn/options.h | 9 +++++++++ > 10 files changed, 119 insertions(+) > > diff --git a/Changes.rst b/Changes.rst > index 6f04e59ac..0ce39a839 100644 > --- a/Changes.rst > +++ b/Changes.rst > @@ -50,6 +50,13 @@ Compatibility mode (``--compat-mode``) > with older peers. The options ``--compat-mode`` allows UIs to provide > users > with an easy way to still connect to older servers. > > +OpenSSL 3.0 support > + OpenSSL 3.0 has been added. Most of OpenSSL 3.0 changes are not user > visible but > + improve general compatibility with OpenSSL 3.0. ``--tls-cert-profile > insecure`` > + has been added to allow selecting the lowest OpenSSL security level > (not > + recommended, use only if you must). OpenSSL 3.0 no longer supports > the Blowfish > + (and other deprecated) algorithm by default and the new option > ``--provider`` > + allows loading the legacy provider to renable these algorithms. > > Deprecated features > ------------------- > diff --git a/doc/man-sections/generic-options.rst > b/doc/man-sections/generic-options.rst > index e6c1fe455..a8f049f20 100644 > --- a/doc/man-sections/generic-options.rst > +++ b/doc/man-sections/generic-options.rst > @@ -280,6 +280,18 @@ which mode OpenVPN is configured as. > This option solves the problem by persisting keys across :code:`SIGUSR1` > resets, so they don't need to be re-read. > > +--providers providers > + Load the list of (OpenSSL) providers. This is mainly useful for using an > + external provider for key management like tpm2-openssl or to load the > + legacy provider with > + > + :: > + > + --providers legacy default > + > + Behaviour of changing this option between SIGHUP might not be well > behaving. > + If you need to change/add/remove this option, fully restart OpenVPN. > + > --remap-usr1 signal > Control whether internally or externally generated :code:`SIGUSR1` > signals > are remapped to :code:`SIGHUP` (restart without persisting state) or > diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h > index 5aab3e1b7..c16837619 100644 > --- a/src/openvpn/crypto_backend.h > +++ b/src/openvpn/crypto_backend.h > @@ -78,6 +78,21 @@ void crypto_clear_error(void); > */ > void crypto_init_lib_engine(const char *engine_name); > > + > +/** > + * Load the given (OpenSSL) providers > + * @param provider name of providers to load > + * @return reference to the loaded provider > + */ > +provider_t *crypto_load_provider(const char *provider); > + > +/** > + * Unloads the given (OpenSSL) provider > + * @param provname name of the provider to unload > + * @param provider pointer to the provider to unload > + */ > +void crypto_unload_provider(const char* provname, provider_t *provider); > + > #ifdef DMALLOC > /* > * OpenSSL memory debugging. If dmalloc debugging is enabled, tell > diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c > index 08b9e004f..39dbf38a5 100644 > --- a/src/openvpn/crypto_mbedtls.c > +++ b/src/openvpn/crypto_mbedtls.c > @@ -69,6 +69,19 @@ crypto_init_lib_engine(const char *engine_name) > "available"); > } > > +provider_t *crypto_load_provider(const char *provider) > +{ > + if (provider) > + { > + msg(M_WARN, "Note: mbed TLS provider functionality is not > available"); > + } > + return NULL; > +} > + > +void crypto_unload_provider(const char* provname, provider_t *provider) > +{ > +} > + > /* > * > * Functions related to the core crypto library > diff --git a/src/openvpn/crypto_mbedtls.h b/src/openvpn/crypto_mbedtls.h > index 019de01d1..758ab1b40 100644 > --- a/src/openvpn/crypto_mbedtls.h > +++ b/src/openvpn/crypto_mbedtls.h > @@ -48,6 +48,9 @@ typedef mbedtls_md_context_t md_ctx_t; > /** Generic HMAC %context. */ > typedef mbedtls_md_context_t hmac_ctx_t; > > +/* Use a dummy type for the provider */ > +typedef void provider_t; > + > /** Maximum length of an IV */ > #define OPENVPN_MAX_IV_LENGTH MBEDTLS_MAX_IV_LENGTH > > diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c > index cc1d62210..c9731adb3 100644 > --- a/src/openvpn/crypto_openssl.c > +++ b/src/openvpn/crypto_openssl.c > @@ -54,6 +54,9 @@ > #if (OPENSSL_VERSION_NUMBER >= 0x10100000L) && > !defined(LIBRESSL_VERSION_NUMBER) > #include <openssl/kdf.h> > #endif > +#if OPENSSL_VERSION_NUMBER >= 0x30000000L > +#include <openssl/provider.h> > +#endif > > #if defined(_WIN32) && defined(OPENSSL_NO_EC) > #error Windows build with OPENSSL_NO_EC: disabling EC key is not > supported. > @@ -149,6 +152,33 @@ crypto_init_lib_engine(const char *engine_name) > #endif > } > > +provider_t * > +crypto_load_provider(const char *provider) > +{ > +#if OPENSSL_VERSION_NUMBER >= 0x30000000L > + /* Load providers into the default (NULL) library context */ > + OSSL_PROVIDER* prov = OSSL_PROVIDER_load(NULL, provider); > + if (!prov) > + { > + crypto_msg(M_FATAL, "failed to load provider '%s'", provider); > + } > + return prov; > +#else /* OPENSSL_VERSION_NUMBER >= 0x30000000L */ > + msg(M_WARN, "Note: OpenSSL provider functionality is not available"); > + return NULL; > +#endif > +} > + > +void crypto_unload_provider(const char* provname, provider_t *provider) > +{ > +#if OPENSSL_VERSION_NUMBER >= 0x30000000L > + if (!OSSL_PROVIDER_unload(provider)) > + { > + crypto_msg(M_FATAL, "failed to unload provider '%s'", provname); > + } > +#endif > +} > + > /* > * > * Functions related to the core crypto library > diff --git a/src/openvpn/crypto_openssl.h b/src/openvpn/crypto_openssl.h > index e540a76b9..446f08508 100644 > --- a/src/openvpn/crypto_openssl.h > +++ b/src/openvpn/crypto_openssl.h > @@ -33,6 +33,10 @@ > #include <openssl/hmac.h> > #include <openssl/md5.h> > #include <openssl/sha.h> > +#if OPENSSL_VERSION_NUMBER >= 0x30000000L > +#include <openssl/provider.h> > +#endif > + > > /** Generic cipher key type %context. */ > typedef EVP_CIPHER cipher_kt_t; > @@ -49,12 +53,17 @@ typedef EVP_MD_CTX md_ctx_t; > /** Generic HMAC %context. */ > #if OPENSSL_VERSION_NUMBER < 0x30000000L > typedef HMAC_CTX hmac_ctx_t; > + > +/* Use a dummy type for the provider */ > +typedef void provider_t; > #else > typedef struct { > OSSL_PARAM params[3]; > uint8_t key[EVP_MAX_KEY_LENGTH]; > EVP_MAC_CTX *ctx; > } hmac_ctx_t; > + > +typedef OSSL_PROVIDER provider_t; > #endif > > /** Maximum length of an IV */ > diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c > index da06f59c2..a18a4dd43 100644 > --- a/src/openvpn/openvpn.c > +++ b/src/openvpn/openvpn.c > @@ -112,10 +112,23 @@ void init_early(struct context *c) > /* init verbosity and mute levels */ > init_verb_mute(c, IVM_LEVEL_1); > > + /* Initialise OpenSSL provider, this needs to be initialised this > + * early since option post-processing and also openssl info > + * printing depends on it */ > + for (int j=1; j < MAX_PARMS && c->options.providers.names[j]; j++) > + { > + c->options.providers.providers[j] = > + crypto_load_provider(c->options.providers.names[j]); > + } > } > > static void uninit_early(struct context *c) > { > + for (int j=1; j < MAX_PARMS && c->options.providers.providers[j]; j++) > + { > + crypto_unload_provider(c->options.providers.names[j], > + c->options.providers.providers[j]); > + } > net_ctx_free(&c->net_ctx); > } > > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > index b5d65d293..b1f9473dc 100644 > --- a/src/openvpn/options.c > +++ b/src/openvpn/options.c > @@ -581,6 +581,7 @@ static const char usage_message[] = > " : Use --show-tls to see a list of supported TLS > ciphers (suites).\n" > "--tls-cert-profile p : Set the allowed certificate crypto algorithm > profile\n" > " (default=legacy).\n" > + "--providers l : A list l of OpenSSL providers to load.\n" > "--tls-timeout n : Packet retransmit timeout on TLS control channel\n" > " if no ACK from remote within n seconds > (default=%d).\n" > "--reneg-bytes n : Renegotiate data chan. key after n bytes sent and > recvd.\n" > @@ -8157,6 +8158,13 @@ add_option(struct options *options, > options->engine = "auto"; > } > } > + else if (streq(p[0], "providers") && p[1]) > + { > + for (size_t j = 1; j < MAX_PARMS && p[j] != NULL;j++) > + { > + options->providers.names[j] = p[j]; > + } > + } > #endif /* ENABLE_CRYPTO_MBEDTLS */ > #ifdef ENABLE_PREDICTION_RESISTANCE > else if (streq(p[0], "use-prediction-resistance") && !p[1]) > diff --git a/src/openvpn/options.h b/src/openvpn/options.h > index 20b34ed4e..d4f41cd71 100644 > --- a/src/openvpn/options.h > +++ b/src/openvpn/options.h > @@ -179,6 +179,14 @@ struct remote_list > struct remote_entry *array[CONNECTION_LIST_SIZE]; > }; > > +struct provider_list > +{ > + /* Names of the providers */ > + const char *names[MAX_PARMS]; > + /* Pointers to the loaded providers to unload them */ > + provider_t *providers[MAX_PARMS]; > +}; > + > enum vlan_acceptable_frames > { > VLAN_ONLY_TAGGED, > @@ -519,6 +527,7 @@ struct options > const char *ncp_ciphers; > const char *authname; > const char *engine; > + struct provider_list providers; > bool replay; > bool mute_replay_warnings; > int replay_window; > -- > 2.33.0 > Acked-by: Selva Nair <selva.nair@gmail.com> Request for nit-fix during commit: In the newly added paragraph in Changes.rst please change ``--provider`` to ``--providers`` Selva <div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 11, 2021 at 2:21 PM Arne Schwabe <<a href="mailto:arne@rfc2549.org">arne@rfc2549.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This allows OpenVPN to load non-default providers. This is mainly<br> useful for loading the legacy provider with --providers legacy default<br> <br> Patch v4: use spaces to seperate providers, unload providers.<br> Patch v5: General cleanup, rename option to --providers, add<br> option to usage() and add an entry to Changes.rst<br> <br> Signed-off-by: Arne Schwabe <<a href="mailto:arne@rfc2549.org" target="_blank">arne@rfc2549.org</a>><br> ---<br> Changes.rst | 7 +++++++<br> doc/man-sections/generic-options.rst | 12 +++++++++++<br> src/openvpn/crypto_backend.h | 15 ++++++++++++++<br> src/openvpn/crypto_mbedtls.c | 13 ++++++++++++<br> src/openvpn/crypto_mbedtls.h | 3 +++<br> src/openvpn/crypto_openssl.c | 30 ++++++++++++++++++++++++++++<br> src/openvpn/crypto_openssl.h | 9 +++++++++<br> src/openvpn/openvpn.c | 13 ++++++++++++<br> src/openvpn/options.c | 8 ++++++++<br> src/openvpn/options.h | 9 +++++++++<br> 10 files changed, 119 insertions(+)<br> <br> diff --git a/Changes.rst b/Changes.rst<br> index 6f04e59ac..0ce39a839 100644<br> --- a/Changes.rst<br> +++ b/Changes.rst<br> @@ -50,6 +50,13 @@ Compatibility mode (``--compat-mode``)<br> with older peers. The options ``--compat-mode`` allows UIs to provide users<br> with an easy way to still connect to older servers.<br> <br> +OpenSSL 3.0 support<br> + OpenSSL 3.0 has been added. Most of OpenSSL 3.0 changes are not user visible but<br> + improve general compatibility with OpenSSL 3.0. ``--tls-cert-profile insecure``<br> + has been added to allow selecting the lowest OpenSSL security level (not<br> + recommended, use only if you must). OpenSSL 3.0 no longer supports the Blowfish<br> + (and other deprecated) algorithm by default and the new option ``--provider``<br> + allows loading the legacy provider to renable these algorithms.<br> <br> Deprecated features<br> -------------------<br> diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst<br> index e6c1fe455..a8f049f20 100644<br> --- a/doc/man-sections/generic-options.rst<br> +++ b/doc/man-sections/generic-options.rst<br> @@ -280,6 +280,18 @@ which mode OpenVPN is configured as.<br> This option solves the problem by persisting keys across :code:`SIGUSR1`<br> resets, so they don't need to be re-read.<br> <br> +--providers providers<br> + Load the list of (OpenSSL) providers. This is mainly useful for using an<br> + external provider for key management like tpm2-openssl or to load the<br> + legacy provider with<br> +<br> + ::<br> +<br> + --providers legacy default<br> +<br> + Behaviour of changing this option between SIGHUP might not be well behaving.<br> + If you need to change/add/remove this option, fully restart OpenVPN.<br> +<br> --remap-usr1 signal<br> Control whether internally or externally generated :code:`SIGUSR1` signals<br> are remapped to :code:`SIGHUP` (restart without persisting state) or<br> diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h<br> index 5aab3e1b7..c16837619 100644<br> --- a/src/openvpn/crypto_backend.h<br> +++ b/src/openvpn/crypto_backend.h<br> @@ -78,6 +78,21 @@ void crypto_clear_error(void);<br> */<br> void crypto_init_lib_engine(const char *engine_name);<br> <br> +<br> +/**<br> + * Load the given (OpenSSL) providers<br> + * @param provider name of providers to load<br> + * @return reference to the loaded provider<br> + */<br> +provider_t *crypto_load_provider(const char *provider);<br> +<br> +/**<br> + * Unloads the given (OpenSSL) provider<br> + * @param provname name of the provider to unload<br> + * @param provider pointer to the provider to unload<br> + */<br> +void crypto_unload_provider(const char* provname, provider_t *provider);<br> +<br> #ifdef DMALLOC<br> /*<br> * OpenSSL memory debugging. If dmalloc debugging is enabled, tell<br> diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c<br> index 08b9e004f..39dbf38a5 100644<br> --- a/src/openvpn/crypto_mbedtls.c<br> +++ b/src/openvpn/crypto_mbedtls.c<br> @@ -69,6 +69,19 @@ crypto_init_lib_engine(const char *engine_name)<br> "available");<br> }<br> <br> +provider_t *crypto_load_provider(const char *provider)<br> +{<br> + if (provider)<br> + {<br> + msg(M_WARN, "Note: mbed TLS provider functionality is not available");<br> + }<br> + return NULL;<br> +}<br> +<br> +void crypto_unload_provider(const char* provname, provider_t *provider)<br> +{<br> +}<br> +<br> /*<br> *<br> * Functions related to the core crypto library<br> diff --git a/src/openvpn/crypto_mbedtls.h b/src/openvpn/crypto_mbedtls.h<br> index 019de01d1..758ab1b40 100644<br> --- a/src/openvpn/crypto_mbedtls.h<br> +++ b/src/openvpn/crypto_mbedtls.h<br> @@ -48,6 +48,9 @@ typedef mbedtls_md_context_t md_ctx_t;<br> /** Generic HMAC %context. */<br> typedef mbedtls_md_context_t hmac_ctx_t;<br> <br> +/* Use a dummy type for the provider */<br> +typedef void provider_t;<br> +<br> /** Maximum length of an IV */<br> #define OPENVPN_MAX_IV_LENGTH MBEDTLS_MAX_IV_LENGTH<br> <br> diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c<br> index cc1d62210..c9731adb3 100644<br> --- a/src/openvpn/crypto_openssl.c<br> +++ b/src/openvpn/crypto_openssl.c<br> @@ -54,6 +54,9 @@<br> #if (OPENSSL_VERSION_NUMBER >= 0x10100000L) && !defined(LIBRESSL_VERSION_NUMBER)<br> #include <openssl/kdf.h><br> #endif<br> +#if OPENSSL_VERSION_NUMBER >= 0x30000000L<br> +#include <openssl/provider.h><br> +#endif<br> <br> #if defined(_WIN32) && defined(OPENSSL_NO_EC)<br> #error Windows build with OPENSSL_NO_EC: disabling EC key is not supported.<br> @@ -149,6 +152,33 @@ crypto_init_lib_engine(const char *engine_name)<br> #endif<br> }<br> <br> +provider_t *<br> +crypto_load_provider(const char *provider)<br> +{<br> +#if OPENSSL_VERSION_NUMBER >= 0x30000000L<br> + /* Load providers into the default (NULL) library context */<br> + OSSL_PROVIDER* prov = OSSL_PROVIDER_load(NULL, provider);<br> + if (!prov)<br> + {<br> + crypto_msg(M_FATAL, "failed to load provider '%s'", provider);<br> + }<br> + return prov;<br> +#else /* OPENSSL_VERSION_NUMBER >= 0x30000000L */<br> + msg(M_WARN, "Note: OpenSSL provider functionality is not available");<br> + return NULL;<br> +#endif<br> +}<br> +<br> +void crypto_unload_provider(const char* provname, provider_t *provider)<br> +{<br> +#if OPENSSL_VERSION_NUMBER >= 0x30000000L<br> + if (!OSSL_PROVIDER_unload(provider))<br> + {<br> + crypto_msg(M_FATAL, "failed to unload provider '%s'", provname);<br> + }<br> +#endif<br> +}<br> +<br> /*<br> *<br> * Functions related to the core crypto library<br> diff --git a/src/openvpn/crypto_openssl.h b/src/openvpn/crypto_openssl.h<br> index e540a76b9..446f08508 100644<br> --- a/src/openvpn/crypto_openssl.h<br> +++ b/src/openvpn/crypto_openssl.h<br> @@ -33,6 +33,10 @@<br> #include <openssl/hmac.h><br> #include <openssl/md5.h><br> #include <openssl/sha.h><br> +#if OPENSSL_VERSION_NUMBER >= 0x30000000L<br> +#include <openssl/provider.h><br> +#endif<br> +<br> <br> /** Generic cipher key type %context. */<br> typedef EVP_CIPHER cipher_kt_t;<br> @@ -49,12 +53,17 @@ typedef EVP_MD_CTX md_ctx_t;<br> /** Generic HMAC %context. */<br> #if OPENSSL_VERSION_NUMBER < 0x30000000L<br> typedef HMAC_CTX hmac_ctx_t;<br> +<br> +/* Use a dummy type for the provider */<br> +typedef void provider_t;<br> #else<br> typedef struct {<br> OSSL_PARAM params[3];<br> uint8_t key[EVP_MAX_KEY_LENGTH];<br> EVP_MAC_CTX *ctx;<br> } hmac_ctx_t;<br> +<br> +typedef OSSL_PROVIDER provider_t;<br> #endif<br> <br> /** Maximum length of an IV */<br> diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c<br> index da06f59c2..a18a4dd43 100644<br> --- a/src/openvpn/openvpn.c<br> +++ b/src/openvpn/openvpn.c<br> @@ -112,10 +112,23 @@ void init_early(struct context *c)<br> /* init verbosity and mute levels */<br> init_verb_mute(c, IVM_LEVEL_1);<br> <br> + /* Initialise OpenSSL provider, this needs to be initialised this<br> + * early since option post-processing and also openssl info<br> + * printing depends on it */<br> + for (int j=1; j < MAX_PARMS && c->options.providers.names[j]; j++)<br> + {<br> + c->options.providers.providers[j] =<br> + crypto_load_provider(c->options.providers.names[j]);<br> + }<br> }<br> <br> static void uninit_early(struct context *c)<br> {<br> + for (int j=1; j < MAX_PARMS && c->options.providers.providers[j]; j++)<br> + {<br> + crypto_unload_provider(c->options.providers.names[j],<br> + c->options.providers.providers[j]);<br> + }<br> net_ctx_free(&c->net_ctx);<br> }<br> <br> diff --git a/src/openvpn/options.c b/src/openvpn/options.c<br> index b5d65d293..b1f9473dc 100644<br> --- a/src/openvpn/options.c<br> +++ b/src/openvpn/options.c<br> @@ -581,6 +581,7 @@ static const char usage_message[] =<br> " : Use --show-tls to see a list of supported TLS ciphers (suites).\n"<br> "--tls-cert-profile p : Set the allowed certificate crypto algorithm profile\n"<br> " (default=legacy).\n"<br> + "--providers l : A list l of OpenSSL providers to load.\n"<br> "--tls-timeout n : Packet retransmit timeout on TLS control channel\n"<br> " if no ACK from remote within n seconds (default=%d).\n"<br> "--reneg-bytes n : Renegotiate data chan. key after n bytes sent and recvd.\n"<br> @@ -8157,6 +8158,13 @@ add_option(struct options *options,<br> options->engine = "auto";<br> }<br> }<br> + else if (streq(p[0], "providers") && p[1])<br> + {<br> + for (size_t j = 1; j < MAX_PARMS && p[j] != NULL;j++)<br> + {<br> + options->providers.names[j] = p[j];<br> + }<br> + }<br> #endif /* ENABLE_CRYPTO_MBEDTLS */<br> #ifdef ENABLE_PREDICTION_RESISTANCE<br> else if (streq(p[0], "use-prediction-resistance") && !p[1])<br> diff --git a/src/openvpn/options.h b/src/openvpn/options.h<br> index 20b34ed4e..d4f41cd71 100644<br> --- a/src/openvpn/options.h<br> +++ b/src/openvpn/options.h<br> @@ -179,6 +179,14 @@ struct remote_list<br> struct remote_entry *array[CONNECTION_LIST_SIZE];<br> };<br> <br> +struct provider_list<br> +{<br> + /* Names of the providers */<br> + const char *names[MAX_PARMS];<br> + /* Pointers to the loaded providers to unload them */<br> + provider_t *providers[MAX_PARMS];<br> +};<br> +<br> enum vlan_acceptable_frames<br> {<br> VLAN_ONLY_TAGGED,<br> @@ -519,6 +527,7 @@ struct options<br> const char *ncp_ciphers;<br> const char *authname;<br> const char *engine;<br> + struct provider_list providers;<br> bool replay;<br> bool mute_replay_warnings;<br> int replay_window;<br> -- <br> 2.33.0<br></blockquote><div><br></div><div>Acked-by: Selva Nair <<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>></div><div><br></div><div>Request for nit-fix during commit: </div><div>In the newly added paragraph in Changes.rst please change</div><div>``--provider`` to ``--providers``</div><div><br></div><div>Selva</div></div></div>
Hi, On Thu, Nov 11, 2021 at 08:20:51PM +0100, Arne Schwabe wrote: > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > index b5d65d293..b1f9473dc 100644 > --- a/src/openvpn/options.c > +++ b/src/openvpn/options.c > @@ -8157,6 +8158,13 @@ add_option(struct options *options, > options->engine = "auto"; > } > } > + else if (streq(p[0], "providers") && p[1]) > + { > + for (size_t j = 1; j < MAX_PARMS && p[j] != NULL;j++) > + { > + options->providers.names[j] = p[j]; > + } > + } > #endif /* ENABLE_CRYPTO_MBEDTLS */ This seems to be in an #ifndef ENABLE_CRYPTO_MBEDTLS block, which means an mbedTLS build won't understand the option "--providers" (but --help shows it, and there's a "mbed TLS provider functionality is not available" patch in crypto_mbedtls.c...) Sounds somewhat unintentional? (I'm primarily complaining because my server side testbed strongly dislikes options that are necessary for particular builds, but break on other builds... :-) ) gert
On Thu, Nov 11, 2021 at 4:09 PM Gert Doering <gert@greenie.muc.de> wrote: > > Hi, > > On Thu, Nov 11, 2021 at 08:20:51PM +0100, Arne Schwabe wrote: > > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > > index b5d65d293..b1f9473dc 100644 > > --- a/src/openvpn/options.c > > +++ b/src/openvpn/options.c > > @@ -8157,6 +8158,13 @@ add_option(struct options *options, > > options->engine = "auto"; > > } > > } > > + else if (streq(p[0], "providers") && p[1]) > > + { > > + for (size_t j = 1; j < MAX_PARMS && p[j] != NULL;j++) > > + { > > + options->providers.names[j] = p[j]; > > + } > > + } > > #endif /* ENABLE_CRYPTO_MBEDTLS */ > > This seems to be in an #ifndef ENABLE_CRYPTO_MBEDTLS block, which > means an mbedTLS build won't understand the option "--providers" > (but --help shows it, and there's a "mbed TLS provider functionality > is not available" patch in crypto_mbedtls.c...) hmm.. obviously I did not build with mbed TLS nor think about it. Some empty functions in crypto_mbedtls.c are still required as the load and unload are unconditionally called. Moving this out of the #ifndef will make --help consistent with the option, but at the same time it's misleading to include this in --help for mbedTLS builds: the user will get a warning if the option is used. I think we should add this option to --help only for OpenSSL. And, while parsing, add provider names to the list only for OpenSSL, show a warning for mbedTLS. That way the list will remain empty for mbedTLS. I'm supposing that we do not want --provider to become a M_FATAL error in mbedTLS builds. Whether the msg(WARN,..) in crypto_mbedtls.c are removed or not is a matter of taste -- they will never get executed if not parsed here. Selva
Hi, On Thu, Nov 11, 2021 at 04:26:17PM -0500, Selva Nair wrote: > I'm supposing that we do not want --provider to become a M_FATAL error > in mbedTLS builds. For "generic OpenVPN" I'm not sure this makes a real difference - you know what your build uses, and if you need --provider, you know that you do (it's not something you add for fun). For my server testing environment it does, as it builds with different crypto libraries for different days (mbedTLS, OpenSSL 1.1, OpenSSL 3.0), so having an option that is needed for 3.0 builds but breaks mbedTLS builds would complicate things... That said, the code looks like it assumed "the option is always there, so on mbedTLS, ignore and warn" :-) gert
diff --git a/Changes.rst b/Changes.rst index 6f04e59ac..0ce39a839 100644 --- a/Changes.rst +++ b/Changes.rst @@ -50,6 +50,13 @@ Compatibility mode (``--compat-mode``) with older peers. The options ``--compat-mode`` allows UIs to provide users with an easy way to still connect to older servers. +OpenSSL 3.0 support + OpenSSL 3.0 has been added. Most of OpenSSL 3.0 changes are not user visible but + improve general compatibility with OpenSSL 3.0. ``--tls-cert-profile insecure`` + has been added to allow selecting the lowest OpenSSL security level (not + recommended, use only if you must). OpenSSL 3.0 no longer supports the Blowfish + (and other deprecated) algorithm by default and the new option ``--provider`` + allows loading the legacy provider to renable these algorithms. Deprecated features ------------------- diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst index e6c1fe455..a8f049f20 100644 --- a/doc/man-sections/generic-options.rst +++ b/doc/man-sections/generic-options.rst @@ -280,6 +280,18 @@ which mode OpenVPN is configured as. This option solves the problem by persisting keys across :code:`SIGUSR1` resets, so they don't need to be re-read. +--providers providers + Load the list of (OpenSSL) providers. This is mainly useful for using an + external provider for key management like tpm2-openssl or to load the + legacy provider with + + :: + + --providers legacy default + + Behaviour of changing this option between SIGHUP might not be well behaving. + If you need to change/add/remove this option, fully restart OpenVPN. + --remap-usr1 signal Control whether internally or externally generated :code:`SIGUSR1` signals are remapped to :code:`SIGHUP` (restart without persisting state) or diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h index 5aab3e1b7..c16837619 100644 --- a/src/openvpn/crypto_backend.h +++ b/src/openvpn/crypto_backend.h @@ -78,6 +78,21 @@ void crypto_clear_error(void); */ void crypto_init_lib_engine(const char *engine_name); + +/** + * Load the given (OpenSSL) providers + * @param provider name of providers to load + * @return reference to the loaded provider + */ +provider_t *crypto_load_provider(const char *provider); + +/** + * Unloads the given (OpenSSL) provider + * @param provname name of the provider to unload + * @param provider pointer to the provider to unload + */ +void crypto_unload_provider(const char* provname, provider_t *provider); + #ifdef DMALLOC /* * OpenSSL memory debugging. If dmalloc debugging is enabled, tell diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 08b9e004f..39dbf38a5 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -69,6 +69,19 @@ crypto_init_lib_engine(const char *engine_name) "available"); } +provider_t *crypto_load_provider(const char *provider) +{ + if (provider) + { + msg(M_WARN, "Note: mbed TLS provider functionality is not available"); + } + return NULL; +} + +void crypto_unload_provider(const char* provname, provider_t *provider) +{ +} + /* * * Functions related to the core crypto library diff --git a/src/openvpn/crypto_mbedtls.h b/src/openvpn/crypto_mbedtls.h index 019de01d1..758ab1b40 100644 --- a/src/openvpn/crypto_mbedtls.h +++ b/src/openvpn/crypto_mbedtls.h @@ -48,6 +48,9 @@ typedef mbedtls_md_context_t md_ctx_t; /** Generic HMAC %context. */ typedef mbedtls_md_context_t hmac_ctx_t; +/* Use a dummy type for the provider */ +typedef void provider_t; + /** Maximum length of an IV */ #define OPENVPN_MAX_IV_LENGTH MBEDTLS_MAX_IV_LENGTH diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index cc1d62210..c9731adb3 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -54,6 +54,9 @@ #if (OPENSSL_VERSION_NUMBER >= 0x10100000L) && !defined(LIBRESSL_VERSION_NUMBER) #include <openssl/kdf.h> #endif +#if OPENSSL_VERSION_NUMBER >= 0x30000000L +#include <openssl/provider.h> +#endif #if defined(_WIN32) && defined(OPENSSL_NO_EC) #error Windows build with OPENSSL_NO_EC: disabling EC key is not supported. @@ -149,6 +152,33 @@ crypto_init_lib_engine(const char *engine_name) #endif } +provider_t * +crypto_load_provider(const char *provider) +{ +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + /* Load providers into the default (NULL) library context */ + OSSL_PROVIDER* prov = OSSL_PROVIDER_load(NULL, provider); + if (!prov) + { + crypto_msg(M_FATAL, "failed to load provider '%s'", provider); + } + return prov; +#else /* OPENSSL_VERSION_NUMBER >= 0x30000000L */ + msg(M_WARN, "Note: OpenSSL provider functionality is not available"); + return NULL; +#endif +} + +void crypto_unload_provider(const char* provname, provider_t *provider) +{ +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + if (!OSSL_PROVIDER_unload(provider)) + { + crypto_msg(M_FATAL, "failed to unload provider '%s'", provname); + } +#endif +} + /* * * Functions related to the core crypto library diff --git a/src/openvpn/crypto_openssl.h b/src/openvpn/crypto_openssl.h index e540a76b9..446f08508 100644 --- a/src/openvpn/crypto_openssl.h +++ b/src/openvpn/crypto_openssl.h @@ -33,6 +33,10 @@ #include <openssl/hmac.h> #include <openssl/md5.h> #include <openssl/sha.h> +#if OPENSSL_VERSION_NUMBER >= 0x30000000L +#include <openssl/provider.h> +#endif + /** Generic cipher key type %context. */ typedef EVP_CIPHER cipher_kt_t; @@ -49,12 +53,17 @@ typedef EVP_MD_CTX md_ctx_t; /** Generic HMAC %context. */ #if OPENSSL_VERSION_NUMBER < 0x30000000L typedef HMAC_CTX hmac_ctx_t; + +/* Use a dummy type for the provider */ +typedef void provider_t; #else typedef struct { OSSL_PARAM params[3]; uint8_t key[EVP_MAX_KEY_LENGTH]; EVP_MAC_CTX *ctx; } hmac_ctx_t; + +typedef OSSL_PROVIDER provider_t; #endif /** Maximum length of an IV */ diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c index da06f59c2..a18a4dd43 100644 --- a/src/openvpn/openvpn.c +++ b/src/openvpn/openvpn.c @@ -112,10 +112,23 @@ void init_early(struct context *c) /* init verbosity and mute levels */ init_verb_mute(c, IVM_LEVEL_1); + /* Initialise OpenSSL provider, this needs to be initialised this + * early since option post-processing and also openssl info + * printing depends on it */ + for (int j=1; j < MAX_PARMS && c->options.providers.names[j]; j++) + { + c->options.providers.providers[j] = + crypto_load_provider(c->options.providers.names[j]); + } } static void uninit_early(struct context *c) { + for (int j=1; j < MAX_PARMS && c->options.providers.providers[j]; j++) + { + crypto_unload_provider(c->options.providers.names[j], + c->options.providers.providers[j]); + } net_ctx_free(&c->net_ctx); } diff --git a/src/openvpn/options.c b/src/openvpn/options.c index b5d65d293..b1f9473dc 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -581,6 +581,7 @@ static const char usage_message[] = " : Use --show-tls to see a list of supported TLS ciphers (suites).\n" "--tls-cert-profile p : Set the allowed certificate crypto algorithm profile\n" " (default=legacy).\n" + "--providers l : A list l of OpenSSL providers to load.\n" "--tls-timeout n : Packet retransmit timeout on TLS control channel\n" " if no ACK from remote within n seconds (default=%d).\n" "--reneg-bytes n : Renegotiate data chan. key after n bytes sent and recvd.\n" @@ -8157,6 +8158,13 @@ add_option(struct options *options, options->engine = "auto"; } } + else if (streq(p[0], "providers") && p[1]) + { + for (size_t j = 1; j < MAX_PARMS && p[j] != NULL;j++) + { + options->providers.names[j] = p[j]; + } + } #endif /* ENABLE_CRYPTO_MBEDTLS */ #ifdef ENABLE_PREDICTION_RESISTANCE else if (streq(p[0], "use-prediction-resistance") && !p[1]) diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 20b34ed4e..d4f41cd71 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -179,6 +179,14 @@ struct remote_list struct remote_entry *array[CONNECTION_LIST_SIZE]; }; +struct provider_list +{ + /* Names of the providers */ + const char *names[MAX_PARMS]; + /* Pointers to the loaded providers to unload them */ + provider_t *providers[MAX_PARMS]; +}; + enum vlan_acceptable_frames { VLAN_ONLY_TAGGED, @@ -519,6 +527,7 @@ struct options const char *ncp_ciphers; const char *authname; const char *engine; + struct provider_list providers; bool replay; bool mute_replay_warnings; int replay_window;
This allows OpenVPN to load non-default providers. This is mainly useful for loading the legacy provider with --providers legacy default Patch v4: use spaces to seperate providers, unload providers. Patch v5: General cleanup, rename option to --providers, add option to usage() and add an entry to Changes.rst Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- Changes.rst | 7 +++++++ doc/man-sections/generic-options.rst | 12 +++++++++++ src/openvpn/crypto_backend.h | 15 ++++++++++++++ src/openvpn/crypto_mbedtls.c | 13 ++++++++++++ src/openvpn/crypto_mbedtls.h | 3 +++ src/openvpn/crypto_openssl.c | 30 ++++++++++++++++++++++++++++ src/openvpn/crypto_openssl.h | 9 +++++++++ src/openvpn/openvpn.c | 13 ++++++++++++ src/openvpn/options.c | 8 ++++++++ src/openvpn/options.h | 9 +++++++++ 10 files changed, 119 insertions(+)