Should this be back-ported to 1.1.1? Considering that both EVP_DigestSignInit() and EVP_DigestVerifyInit() use the same underlying function, they may both potentially return a negative value.

This PR fixes this nasty segfault in the #10797 builds:

https://travis-ci.org/openssl/openssl/jobs/634890004#L12080-L12081

Approved if no problems with Travis.

If our tests were using a "broken check for errors", how much existing code in applications using openssl also has such a "broken check for errors".

I share the concern about fixing the documentation over fixing the behavior: when we promise that we won't break API compatibility (or as little as possible) with 3.0, do we refer to the API as documented or to the API as it should have been documented to match the behavior of a specific revision of the code? (I don't know if this is the case, but what if 1.0.2 and 1.1.0 adhered to the boolean return value documentation and 1.1.1 introduced the return of negative integers for special errors? Should that be seen as a bug to be fixed in 1.1.1 rather than a documentation out of sync with 1.1.1 behavior?)

Apart from the hypothetical case above, I'd like for this kind of issues to include in the PR discussion a survey of how things worked in previous (relevant) releases to understand when documentation and implementation diverged and have clearly in mind if it is more appropriate to fix documentation or behavior!

This is also about patterns. Most of the API surrounding EVP_PKEY has the same kind of return values (including the magic -2), so the documentation on these sets of functions broke away from that pattern in the documentation, but not in actual code as far as I can tell (it's not immediately visible in pre-3.0 code, you have to dig around a bit). I found it even more exasperating that, although related, the documentation for DigestSign and DigestVerify was quite different regarding return values.

To exasperated things further, we do use negative values much more visibly in 3.0, including -2 to indicate unsupported operations, as return values.

Regarding the stark difference in documentation of return values between DigestSign and DigestVerify, I can't help but wonder if someone just documented a snapshot of what the code looked like at that time.

It should be obvious by now that I'm looking for some sort of consistency across the board, rather than having each small set of functions do each their own thing.

@levitte I am not against fixing the documentation, at all, and I am with you on the efforts to uniform behavior whenever possible across the board.

My point is just that we should include in the PR this kind of survey!

I would say that I am fine with fixing the documentation in this case, for all the reasons you stated, but to be fair to our external users we should include a CHANGES entry to be fair to those diligent users that conformed to our misleading documentation!

we should include a CHANGES...

Ah, yes! Thanks for the reminder

I took a moment to look more thoroughly at the 1.1.1 crypto/evp/m_sigver.c, and it turns out that it looks like I'm wrong regarding the history of these functions.

Except for EVP_DigestVerifyFinal, it does appear that all these functions did return only 0 or 1 (including the EVP_DigestSign functions, in spite of what the manual says).

So I'm changing my mind. You were right about the compatibility remarks, and even though I don't quite like the way our return values vary so much between sets of functions, you're right that we have promised to leave the API unchanged as far as we can help, and for this set of functions, I'm not at all sure that the returned -2 is justifiable in this case.

Closing this and starting over.