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.
-- Brane
Index: buckets/ssl_buckets.c
===================================================================
--- buckets/ssl_buckets.c (revision 1774544)
+++ buckets/ssl_buckets.c (working copy)
@@ -588,6 +588,7 @@
}
#ifndef OPENSSL_NO_TLSEXT
+# ifndef 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.
@@ -670,6 +671,7 @@
return cert_valid;
}
+# endif
#endif
typedef enum san_copy_t {
@@ -2040,10 +2042,12 @@
{
#ifndef OPENSSL_NO_TLSEXT
+# ifndef 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);
return APR_SUCCESS;
+# endif
#endif
return APR_ENOTIMPL;
}
Index: test/MockHTTPinC/MockHTTP_server.c
===================================================================
--- test/MockHTTPinC/MockHTTP_server.c (revision 1774544)
+++ test/MockHTTPinC/MockHTTP_server.c (working copy)
@@ -2448,6 +2448,8 @@
#endif
}
+#ifndef OPENSSL_NO_TLSEXT
+# ifndef OPENSSL_NO_OCSP
static int ocspCreateResponse(OCSP_RESPONSE **resp, mhOCSPRespnseStatus_t
status)
{
int ret = 1;
@@ -2526,6 +2528,8 @@
/* Couldn't find match */
return SSL_TLSEXT_ERR_ALERT_FATAL;
}
+# endif /* OPENSSL_NO_OCSP */
+#endif /* OPENSSL_NO_TLSEXT */
/* 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 +2629,7 @@
return APR_SUCCESS;
}
+#ifndef OPENSSL_NO_TLSEXT
static int alpn_select_callback(SSL *ssl,
const unsigned char **out,
unsigned char *outlen,
@@ -2653,6 +2658,7 @@
return SSL_TLSEXT_ERR_ALERT_FATAL;
}
+#endif /* OPENSSL_NO_TLSEXT */
/**
* Inits the OpenSSL context.
@@ -2703,11 +2709,13 @@
#endif
#if OPENSSL_VERSION_NUMBER >= 0x10002000L /* >= 1.0.2 */
+# ifndef OPENSSL_NO_TLSEXT
if (cctx->serv_ctx->alpn) {
SSL_CTX_set_alpn_select_cb(ssl_ctx->ctx,
alpn_select_callback,
cctx->serv_ctx);
}
+# endif
#endif
if (cctx->protocols == mhProtoSSLv2) {
@@ -2774,10 +2782,12 @@
}
#ifndef OPENSSL_NO_TLSEXT
+# ifndef 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);
}
+# endif
#endif
SSL_CTX_set_mode(ssl_ctx->ctx, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER
Index: test/test_ssl.c
===================================================================
--- test/test_ssl.c (revision 1774544)
+++ test/test_ssl.c (working copy)
@@ -1996,7 +1996,11 @@
handler_ctx, tb->pool);
CuAssertTrue(tc, tb->result_flags & TEST_RESULT_SERVERCERTCB_CALLED);
+#ifndef OPENSSL_NO_TLSEXT
+# ifndef OPENSSL_NO_OCSP
CuAssertTrue(tc, tb->result_flags & TEST_RESULT_OCSP_CHECK_SUCCESSFUL);
+# endif
+#endif
}
/* Validate that the subject's CN containing a '\0' byte is reported as failure
@@ -2164,6 +2168,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 +2208,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 +2258,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)
