Hi Collin,
Collin Funk wrote:
> I think that it would require changing all of the function return types
> to "int" or "bool" instead of "void".
>
> > In the past discussion [1][2], we found how to invoke malloc() for
> > the context, while still being future-proof.
>
> Since EVP_DigestUpdate, EVP_DigestFinal_ex, etc. all could theoretically
> throw errors, I think it is fine to just use EVP_MD_CTX_create. That
> avoids the possibility, however unlikely, that the size we allocate on
> the stack is too small.
I disagree with this plan.
I _don't_ think it is "just fine" to use EVP_MD_CTX_create, because that
makes a heap memory allocation, which can fail, and that forces to change
the return type from 'void' to something else, and forces the callers to
check the return value of the *_init_ctx functions.
1) It is a general principle of software design that a code unit should
try to hide complexity. A code unit has
- interface complexity and
- implementation complexity.
Each time an implementation complexity becomes visible as part of the
interface complexity, you should ask yourself whether it is justified.
This is the *only* way to build large software without drowning in
complexity.
Here, the SHA* and other algorithms are described by mathematical
formulas. There is no possibility of failure of the initialization
(*_init_ctx) nor of the "update" (*_process_bytes) functions.
Anyone who creates an API where *_init_ctx and *_process_bytes don't
return 'void' has not done their homework (regarding reducing the
interface complexity).
I understand from [1] that the OpenSSL people offer an API that works
for digests as well as for XOFs (whatever that may be). And
- The state for a XOF has a non-constant result of
EVP_MD_CTX_get_size_ex().
- They want a single heap allocation function EVP_MD_CTX_create().
- Thus it can fail.
But just because the OpenSSL library wants an API that handles XOFs
like digests, is *not* a sufficient reason to add interface complexity
to the Gnulib modules for digests.
2) Then, there's also the consideration of backward-compatibility for the
existing Gnulib modules crypto/sha1-buffer, crypto/sha256-buffer,
crypto/sha512-buffer, etc.
We can handle these, through different function names and through the
NEWS file, if the added complexity has an actual justification. But
as explained under 1), there is no such actual justification.
So, my take on this is:
* You can use EVP functions (and need to, since the functions in [2][3]
are deprecated), except EVP_MD_CTX_create().
* You should not change the return type of a 'void' function to something
else.
* Instead, use the trick discussed in September with an a-priori bound
on the context size, that is validated in a unit test that uses either
LD_PRELOAD or dlopen() or EVP_MD_CTX_get_size_ex().
Bruno
[1] https://docs.openssl.org/master/man3/EVP_DigestInit/
[2] https://docs.openssl.org/master/man3/SHA256_Init/
[3] https://docs.openssl.org/master/man3/MD5/
