You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@serf.apache.org by br...@apache.org on 2016/12/15 14:31:13 UTC
svn commit: r1774473 - in /serf/branches/ocsp-verification:
buckets/ssl_buckets.c serf_bucket_types.h test/test_ssl.c
Author: brane
Date: Thu Dec 15 14:31:13 2016
New Revision: 1774473
URL: http://svn.apache.org/viewvc?rev=1774473&view=rev
Log:
On the ocsp-verification branch: Implement request constructor,
export, import and acccessors.
* serf_bucket_types.h
(serf_ssl_ocsp_response_parse): Fix return type.
* buckets/ssl_buckets.c
(free_ocsp_request): New; pool cleanup function.
(serf_ssl_ocsp_request_t, serf_ssl_ocsp_response_t): Define types.
(serf_ssl_ocsp_request_create,
serf_ssl_ocsp_request_body,
serf_ssl_ocsp_request_body_size,
serf_ssl_ocsp_request_export,
serf_ssl_ocsp_request_import): Implement these functions.
(serf_ssl_ocsp_response_parse,
serf_ssl_ocsp_response_verify): Add fake implementations.
Modified:
serf/branches/ocsp-verification/buckets/ssl_buckets.c
serf/branches/ocsp-verification/serf_bucket_types.h
serf/branches/ocsp-verification/test/test_ssl.c
Modified: serf/branches/ocsp-verification/buckets/ssl_buckets.c
URL: http://svn.apache.org/viewvc/serf/branches/ocsp-verification/buckets/ssl_buckets.c?rev=1774473&r1=1774472&r2=1774473&view=diff
==============================================================================
--- serf/branches/ocsp-verification/buckets/ssl_buckets.c (original)
+++ serf/branches/ocsp-verification/buckets/ssl_buckets.c Thu Dec 15 14:31:13 2016
@@ -2614,3 +2614,260 @@ const serf_bucket_type_t serf_bucket_typ
serf_default_get_remaining,
serf_ssl_set_config,
};
+
+
+/*
+ * 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 */
+};
+
+
+#ifndef OPENSSL_NO_OCSP
+static apr_status_t free_ocsp_request(void *data)
+{
+ OCSP_REQUEST_free(data);
+ return APR_SUCCESS;
+}
+#endif /* OPENSSL_NO_OCSP */
+
+
+serf_ssl_ocsp_request_t *serf_ssl_ocsp_request_create(
+ const serf_ssl_certificate_t *server_cert,
+ const serf_ssl_certificate_t *issuer_cert,
+ int generate_nonce,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+#ifndef OPENSSL_NO_OCSP
+ X509 *const cert = server_cert->ssl_cert;
+ X509 *const issuer = issuer_cert->ssl_cert;
+
+ serf_ssl_ocsp_request_t *req = NULL;
+ OCSP_REQUEST *ocsp_req = NULL;
+ OCSP_CERTID *cert_id = NULL;
+ unsigned char *unused;
+ void *der;
+ int len;
+
+ if (X509_V_OK != X509_check_issued(issuer, cert))
+ goto cleanup;
+
+ /* TODO: Support other hash algorithms besides the default SHA1. */
+ cert_id = OCSP_cert_to_id(NULL, cert, issuer);
+ if (!cert_id)
+ goto cleanup;
+
+ ocsp_req = OCSP_REQUEST_new();
+ if (!ocsp_req)
+ goto cleanup;
+
+ if (!OCSP_request_add0_id(ocsp_req, cert_id))
+ goto cleanup;
+ cert_id = NULL;
+
+ if (generate_nonce) {
+ /* Generates a random nonce, using the internal random generator. */
+ if (!OCSP_request_add1_nonce(ocsp_req, NULL, -1))
+ goto cleanup;
+ }
+
+ /* Generate the DER form of the request. */
+ len = i2d_OCSP_REQUEST(ocsp_req, NULL);
+ if (len < 0)
+ goto cleanup;
+
+ unused = der = apr_palloc(result_pool, len);
+ len = i2d_OCSP_REQUEST(ocsp_req, &unused); /* unused is incremented */
+ if (len < 0)
+ goto cleanup;
+
+ req = apr_palloc(result_pool, sizeof(*req));
+ req->der_request = der;
+ req->der_request_size = len;
+ req->encoded_server_cert =
+ serf_ssl_cert_export2(server_cert, result_pool, scratch_pool);
+ req->encoded_issuer_cert =
+ serf_ssl_cert_export2(issuer_cert, result_pool, scratch_pool);
+
+ /* Now move the unencoded request to the result. */
+ req->request = ocsp_req;
+ apr_pool_cleanup_register(result_pool, ocsp_req,
+ free_ocsp_request,
+ apr_pool_cleanup_null);
+ ocsp_req = NULL;
+
+ cleanup:
+ if (ocsp_req)
+ OCSP_REQUEST_free(ocsp_req);
+ if (cert_id)
+ OCSP_CERTID_free(cert_id);
+ return req;
+#else
+ return NULL;
+#endif /* OPENSSL_NO_OCSP */
+}
+
+const void *serf_ssl_ocsp_request_body(
+ const serf_ssl_ocsp_request_t *ocsp_request)
+{
+#ifndef OPENSSL_NO_OCSP
+ return ocsp_request->der_request;
+#else
+ return NULL;
+#endif /* OPENSSL_NO_OCSP */
+}
+
+apr_size_t serf_ssl_ocsp_request_body_size(
+ const serf_ssl_ocsp_request_t *ocsp_request)
+{
+#ifndef OPENSSL_NO_OCSP
+ return ocsp_request->der_request_size;
+#else
+ return 0;
+#endif /* OPENSSL_NO_OCSP */
+}
+
+const char *serf_ssl_ocsp_request_export(
+ const serf_ssl_ocsp_request_t *ocsp_request,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+#ifndef OPENSSL_NO_OCSP
+
+ /*
+ The structure of the exported request is:
+
+ "Base64-server-cert" "\x1"
+ "Base64-issuer-cert" "\x1"
+ "Base64-DER-formatted-request" "\0"
+ */
+
+ const apr_size_t s_size = strlen(ocsp_request->encoded_server_cert);
+ const apr_size_t i_size = strlen(ocsp_request->encoded_issuer_cert);
+ const apr_size_t all_size = (
+ apr_base64_encode_len(ocsp_request->der_request_size)
+ + s_size + i_size + 3); /* Three terminator bytes */
+
+ char *const buffer = apr_palloc(result_pool, all_size);
+ char *p = buffer;
+
+ memcpy(p, ocsp_request->encoded_server_cert, s_size);
+ p += s_size;
+ *p++ = '\x1';
+
+ memcpy(p, ocsp_request->encoded_issuer_cert, i_size);
+ p += i_size;
+ *p++ = '\x1';
+
+ apr_base64_encode(p, ocsp_request->der_request,
+ ocsp_request->der_request_size);
+
+ return buffer;
+#else
+ return NULL;
+#endif /* OPENSSL_NO_OCSP */
+}
+
+serf_ssl_ocsp_request_t *serf_ssl_ocsp_request_import(
+ const char *encoded_ocsp_request,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+#ifndef OPENSSL_NO_OCSP
+ serf_ssl_ocsp_request_t *req = NULL;
+ const char *encoded_server_cert = encoded_ocsp_request;
+ const char *encoded_issuer_cert;
+ const char *end_server_cert;
+ const char *end_issuer_cert;
+
+ end_server_cert = strchr(encoded_server_cert, '\x1');
+ if (!end_server_cert)
+ return NULL;
+
+ encoded_issuer_cert = end_server_cert + 1;
+ end_issuer_cert = strchr(encoded_issuer_cert, '\x1');
+
+ if (end_issuer_cert) {
+ OCSP_REQUEST *ocsp_req;
+ const char *base64_request = end_issuer_cert + 1;
+ long der_request_size = apr_base64_decode_len(base64_request);
+ /* FIXME: Use scratch pool instead and pmemdup later? */
+ void *der_request = apr_palloc(result_pool, der_request_size);
+ const unsigned char *unused = der_request;
+
+ der_request_size = apr_base64_decode(der_request, base64_request);
+ ocsp_req = d2i_OCSP_REQUEST(NULL, &unused, der_request_size);
+ if (!ocsp_req)
+ return NULL;
+
+ req = apr_palloc(result_pool, sizeof(*req));
+ req->der_request = der_request;
+ req->der_request_size = der_request_size;
+ req->encoded_server_cert =
+ apr_pstrmemdup(result_pool, encoded_server_cert,
+ end_server_cert - encoded_server_cert);
+ req->encoded_issuer_cert =
+ apr_pstrmemdup(result_pool, encoded_issuer_cert,
+ end_issuer_cert - encoded_issuer_cert);
+
+ req->request = ocsp_req;
+ apr_pool_cleanup_register(result_pool, ocsp_req,
+ free_ocsp_request,
+ apr_pool_cleanup_null);
+ }
+
+ return req;
+#else
+ return NULL;
+#endif /* OPENSSL_NO_OCSP */
+}
+
+struct serf_ssl_ocsp_response_t {
+#ifndef OPENSSL_NO_OCSP
+ /* OpenSSL's internal representation of the OCSP response. */
+ OCSP_BASICRESP *response;
+#endif /* OPENSSL_NO_OCSP */
+};
+
+serf_ssl_ocsp_response_t *serf_ssl_ocsp_response_parse(
+ const void *ocsp_response_body,
+ apr_size_t ocsp_response_size,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+#ifndef OPENSSL_NO_OCSP
+ return NULL;
+#else
+ return NULL;
+#endif /* OPENSSL_NO_OCSP */
+}
+
+apr_status_t serf_ssl_ocsp_response_verify(
+ const serf_ssl_ocsp_response_t *ocsp_response,
+ const serf_ssl_ocsp_request_t *ocsp_request,
+ apr_time_t *this_update,
+ apr_time_t *next_update,
+ apr_time_t *produced_at,
+ apr_pool_t *scratch_pool)
+{
+#ifndef OPENSSL_NO_OCSP
+ return SERF_ERROR_SSL_OCSP_RESPONSE_INVALID;
+#else
+ return SERF_ERROR_SSL_OCSP_RESPONSE_INVALID;
+#endif /* OPENSSL_NO_OCSP */
+}
Modified: serf/branches/ocsp-verification/serf_bucket_types.h
URL: http://svn.apache.org/viewvc/serf/branches/ocsp-verification/serf_bucket_types.h?rev=1774473&r1=1774472&r2=1774473&view=diff
==============================================================================
--- serf/branches/ocsp-verification/serf_bucket_types.h (original)
+++ serf/branches/ocsp-verification/serf_bucket_types.h Thu Dec 15 14:31:13 2016
@@ -891,7 +891,7 @@ typedef struct serf_ssl_ocsp_response_t
*
* Returns @c NULL if the response body is not well-formed.
*/
-serf_ssl_ocsp_request_t *serf_ssl_ocsp_response_parse(
+serf_ssl_ocsp_response_t *serf_ssl_ocsp_response_parse(
const void *ocsp_response_body,
apr_size_t ocsp_response_size,
apr_pool_t *result_pool,
Modified: serf/branches/ocsp-verification/test/test_ssl.c
URL: http://svn.apache.org/viewvc/serf/branches/ocsp-verification/test/test_ssl.c?rev=1774473&r1=1774472&r2=1774473&view=diff
==============================================================================
--- serf/branches/ocsp-verification/test/test_ssl.c (original)
+++ serf/branches/ocsp-verification/test/test_ssl.c Thu Dec 15 14:31:13 2016
@@ -2303,6 +2303,103 @@ static void test_ssl_alpn_negotiate(CuTe
handler_ctx, tb->pool);
}
+
+#ifndef OPENSSL_NO_OCSP
+static void load_ocsp_test_certs(CuTest *tc,
+ serf_ssl_certificate_t **cert,
+ serf_ssl_certificate_t **issuer)
+{
+ test_baton_t *tb = tc->testBaton;
+ apr_status_t status;
+
+ status = serf_ssl_load_cert_file(
+ cert,
+ get_srcdir_file(tb->pool, "test/certs/serfserver_san_ocsp_cert.pem"),
+ tb->pool);
+ CuAssertIntEquals(tc, APR_SUCCESS, status);
+ CuAssertPtrNotNull(tc, *cert);
+
+ status = serf_ssl_load_cert_file(
+ issuer,
+ get_srcdir_file(tb->pool, "test/certs/serfcacert.pem"),
+ tb->pool);
+ CuAssertIntEquals(tc, APR_SUCCESS, status);
+ CuAssertPtrNotNull(tc, *issuer);
+}
+#endif /* OPENSSL_NO_OCSP */
+
+static void test_ssl_ocsp_request_create(CuTest *tc)
+{
+#ifndef OPENSSL_NO_OCSP
+ test_baton_t *tb = tc->testBaton;
+ serf_ssl_certificate_t *cert = NULL;
+ serf_ssl_certificate_t *issuer = NULL;
+ serf_ssl_ocsp_request_t *req = NULL;
+
+ load_ocsp_test_certs(tc, &cert, &issuer);
+
+ /* no nonce */
+ req = serf_ssl_ocsp_request_create(cert, issuer, 0, tb->pool, tb->pool);
+ CuAssertPtrNotNull(tc, req);
+
+ /* add nonce */
+ req = serf_ssl_ocsp_request_create(cert, issuer, 1, tb->pool, tb->pool);
+ CuAssertPtrNotNull(tc, req);
+
+ /* certs switched */
+ req = serf_ssl_ocsp_request_create(issuer, cert, 0, tb->pool, tb->pool);
+ CuAssertPtrEquals(tc, NULL, req);
+#else
+ CuTestAssertTrue(tc, 1);
+#endif /* OPENSSL_NO_OCSP */
+}
+
+
+static void test_ssl_ocsp_request_export_import(CuTest *tc)
+{
+#ifndef OPENSSL_NO_OCSP
+ test_baton_t *tb = tc->testBaton;
+ serf_ssl_certificate_t *cert = NULL;
+ serf_ssl_certificate_t *issuer = NULL;
+ serf_ssl_ocsp_request_t *req = NULL;
+ serf_ssl_ocsp_request_t *impreq = NULL;
+ const char *expreq = NULL;
+
+ load_ocsp_test_certs(tc, &cert, &issuer);
+
+ impreq = serf_ssl_ocsp_request_import("foo", tb->pool, tb->pool);
+ CuAssertPtrEquals(tc, NULL, impreq);
+
+ impreq = serf_ssl_ocsp_request_import("foo" "\x1" "bar", tb->pool, tb->pool);
+ CuAssertPtrEquals(tc, NULL, impreq);
+
+ impreq = serf_ssl_ocsp_request_import("foo" "\x1" "bar" "\x1" "baz", tb->pool, tb->pool);
+ CuAssertPtrEquals(tc, NULL, impreq);
+
+ req = serf_ssl_ocsp_request_create(cert, issuer, 0, tb->pool, tb->pool);
+ CuAssertPtrNotNull(tc, req);
+ CuAssertPtrNotNull(tc, serf_ssl_ocsp_request_body(req));
+ CuAssertTrue(tc, 0 < serf_ssl_ocsp_request_body_size(req));
+
+ expreq = serf_ssl_ocsp_request_export(req, tb->pool, tb->pool);
+ CuAssertPtrNotNull(tc, expreq);
+
+ impreq = serf_ssl_ocsp_request_import(expreq, tb->pool, tb->pool);
+ CuAssertPtrNotNull(tc, impreq);
+
+ CuAssertIntEquals(tc,
+ serf_ssl_ocsp_request_body_size(req),
+ serf_ssl_ocsp_request_body_size(impreq));
+ CuAssertTrue(tc,
+ 0 == memcmp(serf_ssl_ocsp_request_body(req),
+ serf_ssl_ocsp_request_body(impreq),
+ serf_ssl_ocsp_request_body_size(req)));
+#else
+ CuTestAssertTrue(tc, 1);
+#endif /* OPENSSL_NO_OCSP */
+}
+
+
CuSuite *test_ssl(void)
{
CuSuite *suite = CuSuiteNew();
@@ -2349,6 +2446,7 @@ CuSuite *test_ssl(void)
SUITE_ADD_TEST(suite, test_ssl_server_cert_with_san_and_empty_cb);
SUITE_ADD_TEST(suite, test_ssl_renegotiate);
SUITE_ADD_TEST(suite, test_ssl_alpn_negotiate);
-
+ SUITE_ADD_TEST(suite, test_ssl_ocsp_request_create);
+ SUITE_ADD_TEST(suite, test_ssl_ocsp_request_export_import);
return suite;
}
Re: svn commit: r1774473 - in /serf/branches/ocsp-verification:
buckets/ssl_buckets.c serf_bucket_types.h test/test_ssl.c
Posted by Branko Čibej <br...@apache.org>.
On 16.12.2016 20:23, Daniel Shahaf wrote:
> Branko \u010cibej wrote on Fri, Dec 16, 2016 at 13:56:34 +0100:
>> 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.
> Have you considered making it a configure-time error� to use an openssl
> that doesn't have those two symbols defined? At least, until somebody
> comes along and asks for them to be supported?
>
> The less knobs the better, and all that...
Truthfully, I have not considered that. Because the knobs are already in
the code on trunk. I'm not even really eager to make the knobs actually
work as advertised, because I've no need to, e.g., use an OpenSSL
without OCSP support -- quite the opposite in fact!
The only reason I've been fiddling with #ifdefs to make trunk compile is
a desire to keep things shipshape.
-- Brane
Re: svn commit: r1774473 - in /serf/branches/ocsp-verification:
buckets/ssl_buckets.c serf_bucket_types.h test/test_ssl.c
Posted by Daniel Shahaf <da...@apache.org>.
Branko \u010cibej wrote on Fri, Dec 16, 2016 at 13:56:34 +0100:
> 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.
Have you considered making it a configure-time error� to use an openssl
that doesn't have those two symbols defined? At least, until somebody
comes along and asks for them to be supported?
The less knobs the better, and all that...
Cheers,
Daniel
� Whatever the scons analogue of "configure-time" is.
Re: svn commit: r1774473 - in /serf/branches/ocsp-verification:
buckets/ssl_buckets.c serf_bucket_types.h test/test_ssl.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 16 December 2016 at 15:56, Branko Čibej <br...@apache.org> wrote:
> On 16.12.2016 13:35, Ivan Zhakov wrote:
>> On 16 December 2016 at 09:15, Branko Čibej <br...@apache.org> wrote:
>>> On 16.12.2016 06:45, Ivan Zhakov wrote:
>>>> On 16 December 2016 at 01:48, Branko Čibej <br...@apache.org> wrote:
>>>>> On 15.12.2016 18:16, Ivan Zhakov wrote:
>>>>>> On 15 December 2016 at 17:31, <br...@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
Re: svn commit: r1774473 - in /serf/branches/ocsp-verification:
buckets/ssl_buckets.c serf_bucket_types.h test/test_ssl.c
Posted by Branko Čibej <br...@apache.org>.
On 16.12.2016 13:35, Ivan Zhakov wrote:
> On 16 December 2016 at 09:15, Branko \u010cibej <br...@apache.org> wrote:
>> On 16.12.2016 06:45, Ivan Zhakov wrote:
>>> On 16 December 2016 at 01:48, Branko \u010cibej <br...@apache.org> wrote:
>>>> On 15.12.2016 18:16, Ivan Zhakov wrote:
>>>>> On 15 December 2016 at 17:31, <br...@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
Re: svn commit: r1774473 - in /serf/branches/ocsp-verification:
buckets/ssl_buckets.c serf_bucket_types.h test/test_ssl.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 16 December 2016 at 09:15, Branko Čibej <br...@apache.org> wrote:
> On 16.12.2016 06:45, Ivan Zhakov wrote:
>> On 16 December 2016 at 01:48, Branko Čibej <br...@apache.org> wrote:
>>> On 15.12.2016 18:16, Ivan Zhakov wrote:
>>>> On 15 December 2016 at 17:31, <br...@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.
--
Ivan Zhakov
Re: svn commit: r1774473 - in /serf/branches/ocsp-verification:
buckets/ssl_buckets.c serf_bucket_types.h test/test_ssl.c
Posted by Branko Čibej <br...@apache.org>.
On 16.12.2016 06:45, Ivan Zhakov wrote:
> On 16 December 2016 at 01:48, Branko \u010cibej <br...@apache.org> wrote:
>> On 15.12.2016 18:16, Ivan Zhakov wrote:
>>> On 15 December 2016 at 17:31, <br...@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.
-- Brane
Re: svn commit: r1774473 - in /serf/branches/ocsp-verification:
buckets/ssl_buckets.c serf_bucket_types.h test/test_ssl.c
Posted by Ivan Zhakov <ch...@gmail.com>.
On 16 December 2016 at 01:48, Branko Čibej <br...@apache.org> wrote:
> On 15.12.2016 18:16, Ivan Zhakov wrote:
>> On 15 December 2016 at 17:31, <br...@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
--
Ivan Zhakov
Re: svn commit: r1774473 - in /serf/branches/ocsp-verification:
buckets/ssl_buckets.c serf_bucket_types.h test/test_ssl.c
Posted by Branko Čibej <br...@apache.org>.
On 15.12.2016 18:16, Ivan Zhakov wrote:
> On 15 December 2016 at 17:31, <br...@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.
-- Brane
Re: svn commit: r1774473 - in /serf/branches/ocsp-verification:
buckets/ssl_buckets.c serf_bucket_types.h test/test_ssl.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 15 December 2016 at 17:31, <br...@apache.org> wrote:
> Author: brane
> Date: Thu Dec 15 14:31:13 2016
> New Revision: 1774473
>
> URL: http://svn.apache.org/viewvc?rev=1774473&view=rev
> Log:
> On the ocsp-verification branch: Implement request constructor,
> export, import and acccessors.
>
[...]
> Modified: serf/branches/ocsp-verification/buckets/ssl_buckets.c
> URL: http://svn.apache.org/viewvc/serf/branches/ocsp-verification/buckets/ssl_buckets.c?rev=1774473&r1=1774472&r2=1774473&view=diff
> ==============================================================================
> --- serf/branches/ocsp-verification/buckets/ssl_buckets.c (original)
> +++ serf/branches/ocsp-verification/buckets/ssl_buckets.c Thu Dec 15 14:31:13 2016
> @@ -2614,3 +2614,260 @@ const serf_bucket_type_t serf_bucket_typ
> serf_default_get_remaining,
> serf_ssl_set_config,
> };
> +
> +
> +/*
> + * 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.
--
Ivan Zhakov