pkey: fix error check of i2d_DHparams()

[ndossche]

The first call correctly checks for a value <=0, the second call does not.
According to LibreSSL docs [1] the error is <=0.
For OpenSSL, we refer to [2] which refers to [3] only states that a value <0 returns an error, but this appears to be a docs inconsistency that does not match reality.
Fix this inconsistency for LibreSSL by using the same check.

[1] https://man.openbsd.org/d2i_DHparams.3
[2] https://manpages.debian.org/stretch/libssl-doc/i2d_DHparams.3ssl.en.html
[3] https://manpages.debian.org/stretch/libssl-doc/d2i_X509.3ssl.en.html

This was found by a hybrid static-dynamic analyser that looks for inconsistent handling of error checks in bindings.

[rhenium]

Related: #1012

I think we should merge this if the 0-return path is actually reachable from ruby/openssl. If it's currently not, I'd prefer to wait until the conflicting docs are clarified at upstream and then fix the inconsistent checks accordingly.

Have you raised this with OpenSSL/LibreSSL people? Their internal usage seems inconsistent as well, for example:

• https://github.com/openssl/openssl/blob/f023662d1bde1fcb7fecf976b25a45afd55734b8/crypto/x509/x509_cmp.c#L269-L271
• https://github.com/openssl/openssl/blob/f023662d1bde1fcb7fecf976b25a45afd55734b8/crypto/ct/ct_log.c#L78-L85

I wasn't aware the man page for i2d_DHparams() in OpenSSL 1.1.1 stated that it may return 0 on error: https://docs.openssl.org/1.1.1/man3/d2i_DHparams/
However, master no longer mentions the 0 return: https://docs.openssl.org/master/man3/d2i_RSAPrivateKey/

It seems this wording was introduced in openssl/openssl#4976 for 1.1.1 and later removed in openssl/openssl#13138 for 3.0.0. Neither PR discussed this detail, so it's unclear whether either change was intentional.

From an API design perspective, it would be unusual for a function that encodes opaque data and returns its length to use 0 to indicate an error. A return value of 0 would normally imply a successful call that encoded 0 bytes... though, in this case, a successful output might not be expected to be 0 bytes long.