| Message ID | 20210129104708.7484-1-arne@rfc2549.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
Hi, On 29/01/2021 11:47, Arne Schwabe wrote: > Without this OpenVPN will later segfault on a FIPS enabled system due > to the algorithm available but not allowed. > > Patch V2: Use (!func) instead (func != 1) > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > src/openvpn/crypto_openssl.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c > index c60d4a54..e124a7b6 100644 > --- a/src/openvpn/crypto_openssl.c > +++ b/src/openvpn/crypto_openssl.c > @@ -954,7 +954,10 @@ md_ctx_init(EVP_MD_CTX *ctx, const EVP_MD *kt) > ASSERT(NULL != ctx && NULL != kt); > > EVP_MD_CTX_init(ctx); > - EVP_DigestInit(ctx, kt); > + if (!EVP_DigestInit(ctx, kt)) > + { > + crypto_msg(M_FATAL, "EVP_DigestInit failed"); > + } > } > > void > @@ -1011,7 +1014,10 @@ hmac_ctx_init(HMAC_CTX *ctx, const uint8_t *key, int key_len, > ASSERT(NULL != kt && NULL != ctx); > > HMAC_CTX_reset(ctx); > - HMAC_Init_ex(ctx, key, key_len, kt, NULL); > + if (!HMAC_Init_ex(ctx, key, key_len, kt, NULL)) > + { > + crypto_msg(M_FATAL, "HMAC_Init_ex failed"); > + } > > /* make sure we used a big enough key */ > ASSERT(HMAC_size(ctx) <= key_len); > @@ -1032,7 +1038,10 @@ hmac_ctx_size(const HMAC_CTX *ctx) > void > hmac_ctx_reset(HMAC_CTX *ctx) > { > - HMAC_Init_ex(ctx, NULL, 0, NULL, NULL); > + if(!HMAC_Init_ex(ctx, NULL, 0, NULL, NULL)) > + { > + crypto_msg(M_FATAL, "HMAC_Init_ex failed"); > + } > } I saw that you missed this case earlier, but I thought that this call cannot really fail. Assuming it can fail under certain conditions, wouldn't the M_FATAL somewhat become a DoS on the server side? Regards,
> > I saw that you missed this case earlier, but I thought that this call > cannot really fail. > > Assuming it can fail under certain conditions, wouldn't the M_FATAL > somewhat become a DoS on the server side? The condition it can fail is basically that the crypto library is unable or unwilling to create a context for that hash algorithm. If that happens we later segfault. This basically only happend on the OpenSSL in FIPS mode, which claim to have MD5 but will then not accept to create MD5. So a fatal fail here is better than a segfault. Arne
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index c60d4a54..e124a7b6 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -954,7 +954,10 @@ md_ctx_init(EVP_MD_CTX *ctx, const EVP_MD *kt) ASSERT(NULL != ctx && NULL != kt); EVP_MD_CTX_init(ctx); - EVP_DigestInit(ctx, kt); + if (!EVP_DigestInit(ctx, kt)) + { + crypto_msg(M_FATAL, "EVP_DigestInit failed"); + } } void @@ -1011,7 +1014,10 @@ hmac_ctx_init(HMAC_CTX *ctx, const uint8_t *key, int key_len, ASSERT(NULL != kt && NULL != ctx); HMAC_CTX_reset(ctx); - HMAC_Init_ex(ctx, key, key_len, kt, NULL); + if (!HMAC_Init_ex(ctx, key, key_len, kt, NULL)) + { + crypto_msg(M_FATAL, "HMAC_Init_ex failed"); + } /* make sure we used a big enough key */ ASSERT(HMAC_size(ctx) <= key_len); @@ -1032,7 +1038,10 @@ hmac_ctx_size(const HMAC_CTX *ctx) void hmac_ctx_reset(HMAC_CTX *ctx) { - HMAC_Init_ex(ctx, NULL, 0, NULL, NULL); + if(!HMAC_Init_ex(ctx, NULL, 0, NULL, NULL)) + { + crypto_msg(M_FATAL, "HMAC_Init_ex failed"); + } } void
Without this OpenVPN will later segfault on a FIPS enabled system due to the algorithm available but not allowed. Patch V2: Use (!func) instead (func != 1) Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/crypto_openssl.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)