On 16 December 2016 at 15:56, Branko Čibej <[email protected]> wrote:
> On 16.12.2016 13:35, Ivan Zhakov wrote:
>> On 16 December 2016 at 09:15, Branko Čibej <[email protected]> wrote:
>>> On 16.12.2016 06:45, Ivan Zhakov wrote:
>>>> On 16 December 2016 at 01:48, Branko Čibej <[email protected]> wrote:
>>>>> On 15.12.2016 18:16, Ivan Zhakov wrote:
>>>>>> On 15 December 2016 at 17:31, <[email protected]> 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
Index: buckets/ssl_buckets.c
===================================================================
--- buckets/ssl_buckets.c (revision 1761526)
+++ buckets/ssl_buckets.c (working copy)
@@ -587,7 +587,7 @@
#endif
}
-#ifndef OPENSSL_NO_TLSEXT
+#if !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_OCSP)
/* Callback called when the server response has some OCSP info.
Returns 1 if the application accepts the OCSP response as successful,
0 in case of error.
@@ -2030,8 +2030,7 @@
apr_status_t
serf_ssl_check_cert_status_request(serf_ssl_context_t *ssl_ctx, int enabled)
{
-
-#ifndef OPENSSL_NO_TLSEXT
+#if !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_OCSP)
SSL_CTX_set_tlsext_status_cb(ssl_ctx->ctx, ocsp_callback);
SSL_CTX_set_tlsext_status_arg(ssl_ctx->ctx, ssl_ctx);
SSL_set_tlsext_status_type(ssl_ctx->ssl, TLSEXT_STATUSTYPE_ocsp);
Index: test/MockHTTPinC/MockHTTP_server.c
===================================================================
--- test/MockHTTPinC/MockHTTP_server.c (revision 1761526)
+++ test/MockHTTPinC/MockHTTP_server.c (working copy)
@@ -2448,6 +2448,7 @@
#endif
}
+#if !defined( OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_OCSP)
static int ocspCreateResponse(OCSP_RESPONSE **resp, mhOCSPRespnseStatus_t
status)
{
int ret = 1;
@@ -2526,6 +2527,7 @@
/* Couldn't find match */
return SSL_TLSEXT_ERR_ALERT_FATAL;
}
+#endif /* !defined( OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_OCSP) */
/* Convert an ssl error into an apr status code for a specific context */
static apr_status_t status_from_ssl(sslCtx_t *ssl_ctx, int ret_code)
@@ -2625,6 +2627,7 @@
return APR_SUCCESS;
}
+#ifndef OPENSSL_NO_TLSEXT
static int alpn_select_callback(SSL *ssl,
const unsigned char **out,
unsigned char *outlen,
@@ -2653,6 +2656,7 @@
return SSL_TLSEXT_ERR_ALERT_FATAL;
}
+#endif /* OPENSSL_NO_TLSEXT */
/**
* Inits the OpenSSL context.
@@ -2702,7 +2706,7 @@
SSL_CTX_set_options(ssl_ctx->ctx, SSL_OP_NO_TLSv1_2);
#endif
-#if OPENSSL_VERSION_NUMBER >= 0x10002000L /* >= 1.0.2 */
+#if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(OPENSSL_NO_TLSEXT)/* >=
1.0.2 */
if (cctx->serv_ctx->alpn) {
SSL_CTX_set_alpn_select_cb(ssl_ctx->ctx,
alpn_select_callback,
@@ -2773,7 +2777,7 @@
break;
}
-#ifndef OPENSSL_NO_TLSEXT
+#if !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_OCSP)
if (cctx->ocspEnabled) {
SSL_CTX_set_tlsext_status_cb(ssl_ctx->ctx, ocspStatusCallback);
SSL_CTX_set_tlsext_status_arg(ssl_ctx->ctx, cctx);
Index: test/test_ssl.c
===================================================================
--- test/test_ssl.c (revision 1761526)
+++ test/test_ssl.c (working copy)
@@ -1996,7 +1996,9 @@
handler_ctx, tb->pool);
CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
+#if !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_OCSP)
CuAssertTrue(tc, tb->result_flags & TEST_RESULT_OCSP_CHECK_SUCCESSFUL);
+#endif
}
/* Validate that the subject's CN containing a '\0' byte is reported as failure
@@ -2164,6 +2166,7 @@
CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
}
+#ifndef OPENSSL_NO_TLSEXT
static apr_status_t http11_select_protocol(void *baton,
const char *protocol)
{
@@ -2203,10 +2206,12 @@
return APR_SUCCESS;
}
+#endif /* OPENSSL_NO_TLSEXT */
static void test_ssl_alpn_negotiate(CuTest *tc)
{
+#ifndef OPENSSL_NO_TLSEXT
test_baton_t *tb = tc->testBaton;
handler_baton_t handler_ctx[1];
const int num_requests = sizeof(handler_ctx)/sizeof(handler_ctx[0]);
@@ -2251,6 +2256,7 @@
run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
handler_ctx, tb->pool);
+#endif /* OPENSSL_NO_TLSEXT */
}
CuSuite *test_ssl(void)
