[gnutls-devel] gnutls 3.5.5

[Thomas Klute]

Am 22.10.2016 um 10:22 schrieb Stefan Bühler:
> On 10/22/2016 09:57 AM, Nikos Mavrogiannopoulos wrote:
>> My suggestion as you will see above in the merge request is for
>> applications to use if (retval < 0) to check for errors rather than
>> check for a specific success code. That cannot of course change past
>> applications.
>>
>> My expectation was that few software will check for equality with zero,
>> mainly due to the examples which use the <0 pattern, and that even if
>> they did, the fact that the first certificate index would be zero, will
>> mitigate any issue (most applications load a single certificate).
> 
> I don't think I've ever read the generic Error handling section; I only
> read the sections for single functions.  I think those should be adapted
> as well, they simply don't read like you have to expect positive values
> as success too:
> 
>     On success, GNUTLS_E_SUCCESS (0) is returned, otherwise an error
>     code is returned.
> 
> Maybe something like this instead:
> 
>     On success, a value equal to or greater than GNUTLS_E_SUCCESS (0)
>     is returned, otherwise a negative error code is returned.
> 
> Gonna be a large patch though :)
> 
>> Do you think this is going to cause issues? Most likely we can still
>> revert the change by introducing a flag in
>> gnutls_certificate_set_flags() which can enable the behavior of
>> returning indexes, instead of returning them by default.
> 
> It did cause issues with lighttpd2; as there is no release and I fixed
> it in git HEAD I don't see any remaining issues in this case.
> 
> I can't speak for other applications :)

It is definitely a problem for mod_gnutls, which has a lot of "(==|!=)
GNUTLS_E_SUCCESS" status checks. The one function affected by this API
change should be easy enough to change, but I don't want to do a pattern
change over the whole code base. The risk of unintended side effects
seems low, but I wouldn't want to take the chance.

Generally I think this change violates the principle of least surprise:
If the API documentation says "returns GNUTLS_E_SUCCESS or an error
code", checking for equality is the obviously safe thing to do. Of
course if GnuTLS explicitly guarantees that error codes will always be
negative that's no less safe, but not something I would've expected
based on the old API doc.

If there are similar changes in the future, all functions should be
listed under "API and ABI modifications" to make it as easy as possible
to find affected code. Without a complete list I'm still worried I might
have missed something broken by the change in 3.5.5.

Best regards,
Thomas