On Sat, Aug 22, 2015 at 11:24 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 07/08/2015 01:15 PM, Дмитрий Воронин wrote:
>>
>> 07.07.2015, 18:34, "Michael Paquier" <michael.paquier@gmail.com>:
>>
>>> Speaking of which, I have rewritten the patch as attached. This looks
>>> way cleaner than the previous version submitted. Dmitry, does that
>>> look fine for you?
>>> I am switching this patch as "Waiting on Author".
>>
>>
>> Michael, hello. I'm looking your variant of patch. You create
>> function ssl_extensions_info(), that gives information of SSL
>> extensions in more informative view. So, it's cool.
>
>
> Should check the return value of every OpenSSL call for errors. At least
> BIO_new() could return NULL, but check all the docs of the others too.
Right, agreed for BIO_new(). BIO_write and BIO_get_mem_data can return
negative error code, but that's not necessarily the indication of an
error by looking at the docs, so I'd rather let them as-is.
X509V3_EXT_print is not documented but it can return <= 0 state code
if the operation fails so I guess that it makes sense to elog an
ERROR. sk_X509_EXTENSION_value and X509_EXTENSION_get_object return
NULL in case of failure (looking at the code tree of openssl), and
OBJ_obj2nid will return NID_undef in this case, so I think the code
as-is is fine. Another interesting thing is that BIO_free can fail and
we don't track that.
By the way, perhaps it would be worth doing similar things for the
other calls of BIO_free and BIO_new, no? I have attached a second
patch.
> Are all the functions used in the patch available in all the versions of
> OpenSSL we support?
We support openssl down to 0.9.7, right? And those functions are
present there (I recall vaguely checking that when looking at this
patch, and I just rechecked in case I missed something).
> Other than those little things, looks good to me.
Thanks!
--
Michael
