Message ID | 20211111130051.3674331-1-arne@rfc2549.org |
---|---|
State | Superseded |
Headers | show |
Series | [Openvpn-devel,v4,OSSL,3.0] Allow loading of non default providers | expand |
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 One tiny typo: Sent with ProtonMail Secure Email. ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Thursday, November 11th, 2021 at 13:00, Arne Schwabe <arne@rfc2549.org> wrote: > This allows OpenVPN to load non-default providers. This is mainly > > useful for loading the legacy provider with --provider legacy default > > Patch v4: use spaces to seperate providers, unload providers. > > Signed-off-by: Arne Schwabe arne@rfc2549.org > > doc/man-sections/generic-options.rst | 10 ++++++++++ > > src/openvpn/crypto_backend.h | 14 +++++++++++++ > > 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 | 7 +++++++ > > src/openvpn/options.h | 9 +++++++++ > > 9 files changed, 108 insertions(+) > > diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst > > index e6c1fe455..f5b8a9135 100644 > > --- a/doc/man-sections/generic-options.rst > > +++ b/doc/man-sections/generic-options.rst > > @@ -280,6 +280,16 @@ 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. > > +--provider providers > > - Load the : separated 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 > > - :: > > - --provider "legacy:default" > > > > --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..40984c559 100644 > > --- a/src/openvpn/crypto_backend.h > > +++ b/src/openvpn/crypto_backend.h > > @@ -78,6 +78,20 @@ 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 (OpneSSL) provider > - - @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..ab38d6e5c 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 hardware crypto engine 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 undload provider '%s'", provname); undload -> unload > > > - } > > +#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..095d448b0 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 OpenVPN 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..87062d58d 100644 > > --- a/src/openvpn/options.c > > +++ b/src/openvpn/options.c > > @@ -8157,6 +8157,13 @@ add_option(struct options *options, > > options->engine = "auto"; > > } > } > > > - else if (streq(p[0], "provider") && 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 > > > Openvpn-devel mailing list > > Openvpn-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/openvpn-devel -----BEGIN PGP SIGNATURE----- Version: ProtonMail wsBzBAEBCAAGBQJhjR3tACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec 9muQuJ0z7gf7BDEY0eogQt1IyxRsviF0xmpbN/vmAZo0Fbkid6DSxQF9IrOv XiIp3OZl48IlqR1PGcl2uuI71bwcmFfpU2cXjPPEDklqRxBDEr5RVT3a5Zjf Jd4Kb3n52hzfDpHqak1CNvQpc6aTSyK8fKQzk0cYJRr7JcyU9c+JVVHELU98 eBsEyXCz9IcD/7sX3NpNJS+pKds35SV/TWmOyzzBrPZFOijkE3g03R/cLhxD YVzIj+3EeewlYLufTNH9yVofUT8B8elKMeCBqij3FWTIh5eYofboG6JuXXQu KgbOeaM6/djMDWnXcdjg92gkLcRhetOb0vhfck+uSrbrplA6UE1hdg== =KyKQ -----END PGP SIGNATURE-----
Hi, Looks good except for some documentation errors and some nits On Thu, Nov 11, 2021 at 8:01 AM Arne Schwabe <arne@rfc2549.org> wrote: > This allows OpenVPN to load non-default providers. This is mainly > useful for loading the legacy provider with --provider legacy default > > Patch v4: use spaces to seperate providers, unload providers. > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > doc/man-sections/generic-options.rst | 10 ++++++++++ > src/openvpn/crypto_backend.h | 14 +++++++++++++ > 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 | 7 +++++++ > src/openvpn/options.h | 9 +++++++++ > 9 files changed, 108 insertions(+) > > diff --git a/doc/man-sections/generic-options.rst > b/doc/man-sections/generic-options.rst > index e6c1fe455..f5b8a9135 100644 > --- a/doc/man-sections/generic-options.rst > +++ b/doc/man-sections/generic-options.rst > @@ -280,6 +280,16 @@ 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. > > +--provider providers > For the option name please use "--providers" in plural. Here and in options.c. For the missing one-line description in usage_message, "--providers provider-list" would be apt. + Load the : separated 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 > + > + :: > + > + --provider "legacy:default" > This is now changed to space-separated, right? So: --providers legacy default It may be worth pointing out that it's best to not change this option between SIGHUPs. Unlike most other options, this would behave somewhat unintuitively on SIGHUP. If started with "--providers foo" and then if this option is removed and a SIGHUP done, no providers will be loaded and connection will fail. On the other hand a fresh start with no "--providers" option, the fallback provider (default) will get auto-loaded. > + > + > --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..40984c559 100644 > --- a/src/openvpn/crypto_backend.h > +++ b/src/openvpn/crypto_backend.h > @@ -78,6 +78,20 @@ 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 (OpneSSL) provider To nit-pick: @param provname > + * @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) > +{ Personally I do not care, but in case we have a policy on no-unused vars, something like "(void) provname;" etc would be needed. > +} > + > /* > * > * 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..ab38d6e5c 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 hardware crypto engine functionality is > not available"); > "hardware crypto engine" --> "provider" > + 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 undload provider '%s'", provname); > "undload" --> "unload" > + } > +#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..095d448b0 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 OpenVPN provider, this needs to be initialised this > "OpenVPN provider" --> "OpenSSL provider" > + * 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..87062d58d 100644 > --- a/src/openvpn/options.c > +++ b/src/openvpn/options.c > @@ -8157,6 +8157,13 @@ add_option(struct options *options, > options->engine = "auto"; > } > } > + else if (streq(p[0], "provider") && p[1]) > "providers" please > + { > + 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; > The new option is not added to usage_message for --help and usage(). Selva <div dir="ltr"><div>Hi,</div><div><br></div><div>Looks good except for some documentation errors and some nits</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 11, 2021 at 8:01 AM Arne Schwabe <<a href="mailto:arne@rfc2549.org" target="_blank">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 --provider legacy default<br> <br> Patch v4: use spaces to seperate providers, unload providers.<br> <br> Signed-off-by: Arne Schwabe <<a href="mailto:arne@rfc2549.org" target="_blank">arne@rfc2549.org</a>><br> ---<br> doc/man-sections/generic-options.rst | 10 ++++++++++<br> src/openvpn/crypto_backend.h | 14 +++++++++++++<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 | 7 +++++++<br> src/openvpn/options.h | 9 +++++++++<br> 9 files changed, 108 insertions(+)<br> <br> diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst<br> index e6c1fe455..f5b8a9135 100644<br> --- a/doc/man-sections/generic-options.rst<br> +++ b/doc/man-sections/generic-options.rst<br> @@ -280,6 +280,16 @@ 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> +--provider providers<br></blockquote><div><br></div><div>For the option name please use "--providers" in plural. Here and in options.c. For the missing one-line description in usage_message, "--providers provider-list" would be apt.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + Load the : separated list of (OpenSSL) providers. This is mainly useful for<br> + using an external provider for key management like tpm2-openssl or to load<br> + the legacy provider with<br> +<br> + ::<br> +<br> + --provider "legacy:default"<br></blockquote><div><br></div><div>This is now changed to space-separated, right? So:</div><div><br></div><div>--providers legacy default</div><div><br></div><div>It may be worth pointing out that it's best to not change this option between SIGHUPs.<br></div><div><br></div><div>Unlike most other options, this would behave somewhat unintuitively on SIGHUP. If started with "--providers foo" and then if this option is removed and a SIGHUP done, no providers will be loaded and connection will fail. On the other hand a fresh start with no "--providers" option, the fallback provider (default) will get auto-loaded.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +<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..40984c559 100644<br> --- a/src/openvpn/crypto_backend.h<br> +++ b/src/openvpn/crypto_backend.h<br> @@ -78,6 +78,20 @@ 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 (OpneSSL) provider </blockquote><div><br></div><div>To nit-pick: @param provname </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+ * @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> +{ </blockquote><div><br></div><div>Personally I do not care, but in case we have a policy on no-unused vars, something like "(void) provname;" etc would be needed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +}<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..ab38d6e5c 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 hardware crypto engine functionality is not available");<br></blockquote><div><br></div><div>"hardware crypto engine" --> "provider"</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + 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 undload provider '%s'", provname);<br></blockquote><div><br></div><div>"undload" --> "unload"</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + }<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..095d448b0 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 OpenVPN provider, this needs to be initialised this<br></blockquote><div><br></div><div>"OpenVPN provider" --> "OpenSSL provider"</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + * 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..87062d58d 100644<br> --- a/src/openvpn/options.c<br> +++ b/src/openvpn/options.c<br> @@ -8157,6 +8157,13 @@ add_option(struct options *options,<br> options->engine = "auto";<br> }<br> }<br> + else if (streq(p[0], "provider") && p[1])<br></blockquote><div><br></div><div> "providers" please<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + {<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></blockquote><div><br></div><div>The new option is not added to usage_message for --help and usage().</div><div><br></div><div>Selva</div></div></div>
diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst index e6c1fe455..f5b8a9135 100644 --- a/doc/man-sections/generic-options.rst +++ b/doc/man-sections/generic-options.rst @@ -280,6 +280,16 @@ 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. +--provider providers + Load the : separated 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 + + :: + + --provider "legacy:default" + + --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..40984c559 100644 --- a/src/openvpn/crypto_backend.h +++ b/src/openvpn/crypto_backend.h @@ -78,6 +78,20 @@ 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 (OpneSSL) provider + * @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..ab38d6e5c 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 hardware crypto engine 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 undload 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..095d448b0 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 OpenVPN 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..87062d58d 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -8157,6 +8157,13 @@ add_option(struct options *options, options->engine = "auto"; } } + else if (streq(p[0], "provider") && 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 --provider legacy default Patch v4: use spaces to seperate providers, unload providers. Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- doc/man-sections/generic-options.rst | 10 ++++++++++ src/openvpn/crypto_backend.h | 14 +++++++++++++ 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 | 7 +++++++ src/openvpn/options.h | 9 +++++++++ 9 files changed, 108 insertions(+)