Re: [External] : Re: BIO_read() crash
On Mon, 2022-12-05 at 16:14 -0800, Benjamin Kaduk via openssl-users
wrote:
> On Mon, Dec 05, 2022 at 11:31:18AM -0800, Thomas Dwyer III wrote:
> > Why does EVP_get_digestbyname("md4") return non-NULL if the legacy
> > provider
> > isn't loaded? Similarly, why does it return non-NULL for "md5"
> > after doing
> > EVP_set_default_properties(NULL, "fips=yes")? This seems
> > unintuitive. Legacy
> > code that does not know about EVP_MD_fetch() checks the return
> > value of
> > EVP_get_digestbyname(). Isn't that where the error should be
> > detected? Why
> > let it get all the way to BIO_set_md() (or EVP_DigestInit() or
> > whatever)
> > before the error is detected?
>
> To do so would introduce a time-of-check/time-of-use race, as the set
> of
> providers available may change in the intervening period.
Not just that. IMO the primary motivation was to keep the digests (and
similarly ciphers) returned by the EVP_md4() (and similar) or
EVP_get_digestbyname() calls still constant. I.e., they would not
change based on the providers involved, etc. This implied the implicit
fetching done inside the EVP_DigestInit() and similar init routines.
As any correct code required checking result of EVP_DigestInit() and
thus also the result of BIO_set_md(), it was the least of all evils in
how to implement these legacy EVP_MD functions returning constant
EVP_MDs.
--
Tomáš Mráz, OpenSSL
Re: [External] : Re: BIO_read() crash
On Mon, Dec 05, 2022 at 11:31:18AM -0800, Thomas Dwyer III wrote:
> Why does EVP_get_digestbyname("md4") return non-NULL if the legacy provider
> isn't loaded? Similarly, why does it return non-NULL for "md5" after doing
> EVP_set_default_properties(NULL, "fips=yes")? This seems unintuitive. Legacy
> code that does not know about EVP_MD_fetch() checks the return value of
> EVP_get_digestbyname(). Isn't that where the error should be detected? Why
> let it get all the way to BIO_set_md() (or EVP_DigestInit() or whatever)
> before the error is detected?
To do so would introduce a time-of-check/time-of-use race, as the set of
providers available may change in the intervening period.
-Ben
Re: [External] : Re: BIO_read() crash
Why does EVP_get_digestbyname("md4") return non-NULL if the legacy
provider isn't loaded? Similarly, why does it return non-NULL for "md5"
after doing EVP_set_default_properties(NULL, "fips=yes")? This seems
unintuitive. Legacy code that does not know about EVP_MD_fetch() checks
the return value of EVP_get_digestbyname(). Isn't that where the error
should be detected? Why let it get all the way to BIO_set_md() (or
EVP_DigestInit() or whatever) before the error is detected?
Tom.III
||
On 12/5/22 02:24, Tomas Mraz wrote:
Hi,
there is an error in your code - see my comment below.
On Mon, 2022-12-05 at 08:45 +, Zhongyan Wang wrote:
...
md = EVP_get_digestbyname(dgst);
if (!md) {
printf("Error EVP_get_digestbyname %s\n", dgst);
goto err_exit;
}
in = BIO_new_file(datain, "rb");
if (!in) {
printf("Error BIO_new_file %s\n", datain);
goto err_exit;
}
out = BIO_new(BIO_s_mem());
if (!out) {
printf("Error BIO_new out\n");
goto err_exit;
}
rbio = in;
bmd = BIO_new(BIO_f_md());
if (!bmd){
printf("Error BIO_new bmd\n");
goto err_exit;
}
BIO_set_md(bmd, md);
You do not check the return value here. This call will return <= 0
return value in case the legacy provider is not loaded.
