serf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Branko Čibej <br...@apache.org>
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 12:56:34 GMT
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.

-- Brane


Mime
View raw message