serf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Zhakov <i...@visualsvn.com>
Subject Re: svn commit: r1774473 - in /serf/branches/ocsp-verification: buckets/ssl_buckets.c serf_bucket_types.h test/test_ssl.c
Date Fri, 16 Dec 2016 15:45:23 GMT
On 16 December 2016 at 15:56, Branko Čibej <brane@apache.org> wrote:
> On 16.12.2016 13:35, Ivan Zhakov wrote:
>> On 16 December 2016 at 09:15, Branko Čibej <brane@apache.org> wrote:
>>> On 16.12.2016 06:45, Ivan Zhakov wrote:
>>>> On 16 December 2016 at 01:48, Branko Čibej <brane@apache.org> wrote:
>>>>> On 15.12.2016 18:16, Ivan Zhakov wrote:
>>>>>> On 15 December 2016 at 17:31,  <brane@apache.org> wrote:
>>>>>>> +/*
>>>>>>> + * OCSP bits are here because they depend on OpenSSL and private
types
>>>>>>> + * defined in this file.
>>>>>>> + */
>>>>>>> +
>>>>>>> +struct serf_ssl_ocsp_request_t {
>>>>>>> +#ifndef OPENSSL_NO_OCSP
>>>>>>> +    /* OpenSSL's internal representation of the OCSP request.
*/
>>>>>>> +    OCSP_REQUEST *request;
>>>>>>> +
>>>>>>> +    /* DER-encoded request and size. */
>>>>>>> +    const void *der_request;
>>>>>>> +    apr_size_t der_request_size;
>>>>>>> +
>>>>>>> +    /* Exported server and issuer certificates. */
>>>>>>> +    const char *encoded_server_cert;
>>>>>>> +    const char *encoded_issuer_cert;
>>>>>>> +#endif  /* OPENSSL_NO_OCSP */
>>>>>>> +};
>>>>>> As far I remember C requires that a struct or union has at least
one member.
>>>>> You're absolutely right. I've been meddling in C++ for too long.
>>>>>
>>>>> FWIW, that file does not compile, even on trunk, when OPENSSL_NO_OCSP
is
>>>>> defined ... I wonder if we should just remove those conditional blocks?
>>>>> After all, it's not as if we want to encourage people to use OpenSSL
0.9.7.
>>>>>
>>>> As far I remember OpenSSL is very configurable in build time, so
>>>> OPENSSL_NO_OSCSP can be set even for OpenSSL 1.0.2 using '--no-ocsp'
>>>> option [1]:
>>>>
>>>> [1] https://github.com/openssl/openssl/blob/master/INSTALL
>>> If that's the case, I'll have a go at making Serf actually build with
>>> OPENSSL_NO_OCSP and OPENSSL_NO_TLSEXT. The problems aren't confined to
>>> ssl_buckets.c; the mock HTTP server used for testing has a bunch of
>>> issues with these symbols defined, too.
>>>
>> I don't think that serf must have support to build with all possible
>> OpenSSL compile time options, since even OpenSSL itself doesn't
>> support many combination of them. But I think it would be nice to
>> support some of them if doesn't require significant effort.
>
> I have no interest in making Serf support /all/ possible OpenSSL
> options.
> On trunk, we already use OPENSSL_NO_TLSEXT and OPENSSL_NO_OCSP
> in the code, but it doesn't compile if either or both of these are
> actually defined.
>

> I have that fixed locally (see attached patch), although the fix
> unfortunately involves adding some conditional blocks to the code ...
> not nice, but I can't think of a better way to make things work, other
> than removing the dependency on those symbols altogether.
>
> I tested the patch by running tests with either or both (or no) symbols
> defined. If it doesn't look too horrible, I'll commit it this evening.
>

May be better to use '#if !defined(OPENSSL_NO_TLSEXT) &&
!defined(OPENSSL_NO_OCSP)' instead of nested #ifndef? See attached
patch.


-- 
Ivan Zhakov

Mime
View raw message