| Message ID | 20210312150629.57302-1-juliusz@wolfssl.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] EVP_DigestSignFinal siglen parameter correction | expand |
Am 12.03.21 um 16:06 schrieb Juliusz Sosinowicz:
> In the EVP_DigestSignFinal API, "before the call the siglen parameter should contain the length of the sig buffer".
This is probably correct but I need to take a closer look as this
function is directly taken from OpenSSL's internal function and only has
minimal adaptations. Note also that we plan to remove this compatibility
path as soon as we drop OpenSSL 1.0.2 support.
Arne
Am 12.03.21 um 16:06 schrieb Juliusz Sosinowicz: > In the EVP_DigestSignFinal API, "before the call the siglen parameter should contain the length of the sig buffer". > > Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com> > --- > src/openvpn/crypto_openssl.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c > index 49698e4b3..4486d246d 100644 > --- a/src/openvpn/crypto_openssl.c > +++ b/src/openvpn/crypto_openssl.c > @@ -1195,7 +1195,7 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec, > EVP_MD_CTX ctx, ctx_tmp, ctx_init; > EVP_PKEY *mac_key; > unsigned char A1[EVP_MAX_MD_SIZE]; > - size_t A1_len; > + size_t A1_len = EVP_MAX_MD_SIZE; > int ret = false; > > chunk = EVP_MD_size(md); > @@ -1249,6 +1249,7 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec, > > if (olen > chunk) > { > + j = olen; > if (!EVP_DigestSignFinal(&ctx, out, &j)) > { > goto err; > @@ -1263,6 +1264,7 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec, > } > else > { > + A1_len = EVP_MAX_MD_SIZE; > /* last one */ > if (!EVP_DigestSignFinal(&ctx, A1, &A1_len)) > { > Acked-By: Arne Schwabe <arne@rfc2549.org> The uninitialised parameter should not work and the OpenSSL documentation says that it should contain the size of the buffer but the internal code of OpenVPN seems to ignore that and somehow it still works. But I cannot blame wolfSSL being lax about the OpenSSL API :)
Your patch has been applied to the master branch.
Mildly tested on an OpenSSL 1.0.2u box, client side only. Works :-)
commit 476990d41ad78ac4419a3743cdab55c85c41b041
Author: Juliusz Sosinowicz
Date: Fri Mar 12 16:06:29 2021 +0100
EVP_DigestSignFinal siglen parameter correction
Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210312150629.57302-1-juliusz@wolfssl.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21663.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index 49698e4b3..4486d246d 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -1195,7 +1195,7 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec, EVP_MD_CTX ctx, ctx_tmp, ctx_init; EVP_PKEY *mac_key; unsigned char A1[EVP_MAX_MD_SIZE]; - size_t A1_len; + size_t A1_len = EVP_MAX_MD_SIZE; int ret = false; chunk = EVP_MD_size(md); @@ -1249,6 +1249,7 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec, if (olen > chunk) { + j = olen; if (!EVP_DigestSignFinal(&ctx, out, &j)) { goto err; @@ -1263,6 +1264,7 @@ tls1_P_hash(const EVP_MD *md, const unsigned char *sec, } else { + A1_len = EVP_MAX_MD_SIZE; /* last one */ if (!EVP_DigestSignFinal(&ctx, A1, &A1_len)) {
In the EVP_DigestSignFinal API, "before the call the siglen parameter should contain the length of the sig buffer". Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com> --- src/openvpn/crypto_openssl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)