Message ID | 20210405080007.1665-3-a@unstable.cc |
---|---|
State | Changes Requested |
Headers | show |
Series | [Openvpn-devel,1/3] openssl: fix EVP_PKEY_CTX memory leak | expand |
Hi, On Mon, Apr 05, 2021 at 10:00:07AM +0200, Antonio Quartulli wrote: > /* compute PRF */ > - if (!ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, output_len)) > - { > - ret = false; > - } > + ret = ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, > + output_len); If touching this, I would move the "bool ret" declaration to here as well. Doesn't make sense to have a "bool ret = true" declaration 15 lines further up if the first use is "assign it a new value", then. Thus, + bool ret = ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, + output, output_len); gert
Hi, On 05/04/2021 12:38, Gert Doering wrote: > Hi, > > On Mon, Apr 05, 2021 at 10:00:07AM +0200, Antonio Quartulli wrote: >> /* compute PRF */ >> - if (!ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, output_len)) >> - { >> - ret = false; >> - } >> + ret = ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, >> + output_len); > > If touching this, I would move the "bool ret" declaration to here as > well. Doesn't make sense to have a "bool ret = true" declaration 15 > lines further up if the first use is "assign it a new value", then. > > Thus, > > + bool ret = ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, > + output, output_len); > Agreed. v2 will land soon. Regards,
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 9d18c6e5..cb2a3e82 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1614,10 +1614,8 @@ openvpn_PRF(const uint8_t *secret, } /* compute PRF */ - if (!ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, output_len)) - { - ret = false; - } + ret = ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, + output_len); buf_clear(&seed); free_buf(&seed);