You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@serf.apache.org by James McCoy <ja...@debian.org> on 2016/06/12 18:56:07 UTC

[PATCH] Prepare serf for OpenSSL 1.1 release

OpenSSL is preparing a 1.1.0 release which introduces API and ABI
incompatibilities (described in an in-progress[0] wiki page).

[0]: https://wiki.openssl.org/index.php/1.1_API_Changes

A rebuild[1] of all Debian packages using OpenSSL found that serf is
affected by these changes, specifically making BIO/BIO_METHOD opaque and
removing the need for the locking functions[2].

[1]: https://breakpoint.cc/openssl-1.1-rebuild-2016-05-29/
[2]: https://github.com/openssl/openssl/blob/dae00d631fdaed48d88c454864abbd6ce99c63d6/include/openssl/crypto.h#L209-L216

The attached patches fix the build and pass the test suites both with
OpenSSL 1.0.2h and a pre-release of OpenSSL 1.1.0, but more eyes are
always good.

I'm including patches for both branches/1.3.x and trunk since there's a
bit of divergence between the two.

Cheers,
-- 
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB

Re: [PATCH] Prepare serf for OpenSSL 1.1 release

Posted by 'James McCoy' <ja...@debian.org>.
On Mon, Jun 13, 2016 at 10:07:51PM +0200, Bert Huijben wrote:
> 
> 
> > -----Original Message-----
> > From: James McCoy [mailto:vega.james@gmail.com] On Behalf Of James
> > McCoy
> > Sent: zondag 12 juni 2016 20:56
> > To: dev@serf.apache.org
> > Subject: [PATCH] Prepare serf for OpenSSL 1.1 release
> > 
> > OpenSSL is preparing a 1.1.0 release which introduces API and ABI
> > incompatibilities (described in an in-progress[0] wiki page).
> > 
> > [0]: https://wiki.openssl.org/index.php/1.1_API_Changes
> > 
> > A rebuild[1] of all Debian packages using OpenSSL found that serf is
> > affected by these changes, specifically making BIO/BIO_METHOD opaque and
> > removing the need for the locking functions[2].
> > 
> > [1]: https://breakpoint.cc/openssl-1.1-rebuild-2016-05-29/
> > [2]:
> > https://github.com/openssl/openssl/blob/dae00d631fdaed48d88c454864abbd6
> > ce99c63d6/include/openssl/crypto.h#L209-L216
> > 
> > The attached patches fix the build and pass the test suites both with
> > OpenSSL 1.0.2h and a pre-release of OpenSSL 1.1.0, but more eyes are
> > always good.
> 
> 	Hi,
> 
> I had a quick look at this patch. The patch looks like it fixes the
> issues you identified. I'm guessing the patch introduces a minor
> memory leak via the new calls to BIO_meth_new(), as there is no code
> path that destroys these now. 

Thanks for the review and spotting that.  Should be fixed in the updated
patch.

> For trunk I would prefer a patch with the USE_OPENSSL_1_1_API macro
> defined the other way around. Newer OpenSSL 1.X versions will most
> likely all use the new code path, while only 1.0 (and 0.x) will use
> the legacy code path.

Ok.  I've changed the macro to USE_LEGACY_OPENSSL and adjusted
accordingly.

> > I'm including patches for both branches/1.3.x and trunk since there's a
> > bit of divergence between the two.
> >
> 
> I tried to look at OpenSSL 1.1.0 earlier myself, but as my build
> depends on Subversion dependencies I got into a few other problems
> earlier. I got an easy workaround for APR, but Cyrus Sasl was a
> different story. Do you know if there is already a patch for that
> package?

Not that I've seen yet.  Upstream is pretty quiet and I know the folks
on the Debian side were looking for more help in general, so I'm not
sure if they'll have time to help.

I have a few other things on my plate, but if there's still a need when
I get past that I'll take a look at cyrus-sasl.

Cheers,
-- 
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB

RE: [PATCH] Prepare serf for OpenSSL 1.1 release

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: James McCoy [mailto:vega.james@gmail.com] On Behalf Of James
> McCoy
> Sent: zondag 12 juni 2016 20:56
> To: dev@serf.apache.org
> Subject: [PATCH] Prepare serf for OpenSSL 1.1 release
> 
> OpenSSL is preparing a 1.1.0 release which introduces API and ABI
> incompatibilities (described in an in-progress[0] wiki page).
> 
> [0]: https://wiki.openssl.org/index.php/1.1_API_Changes
> 
> A rebuild[1] of all Debian packages using OpenSSL found that serf is
> affected by these changes, specifically making BIO/BIO_METHOD opaque and
> removing the need for the locking functions[2].
> 
> [1]: https://breakpoint.cc/openssl-1.1-rebuild-2016-05-29/
> [2]:
> https://github.com/openssl/openssl/blob/dae00d631fdaed48d88c454864abbd6
> ce99c63d6/include/openssl/crypto.h#L209-L216
> 
> The attached patches fix the build and pass the test suites both with
> OpenSSL 1.0.2h and a pre-release of OpenSSL 1.1.0, but more eyes are
> always good.

	Hi,

I had a quick look at this patch. The patch looks like it fixes the issues you identified. I'm guessing the patch introduces a minor memory leak via the new calls to BIO_meth_new(), as there is no code path that destroys these now. 

For trunk I would prefer a patch with the USE_OPENSSL_1_1_API macro defined the other way around. Newer OpenSSL 1.X versions will most likely all use the new code path, while only 1.0 (and 0.x) will use the legacy code path.

> 
> I'm including patches for both branches/1.3.x and trunk since there's a
> bit of divergence between the two.
>

I tried to look at OpenSSL 1.1.0 earlier myself, but as my build depends on Subversion dependencies I got into a few other problems earlier. I got an easy workaround for APR, but Cyrus Sasl was a different story. Do you know if there is already a patch for that package? If there is one it would make it easier for me to test with OpenSSL 1.1.0 prereleases.

Thanks,

	Bert


> Cheers,
> --
> James
> GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB


Re: [PATCH] Prepare serf for OpenSSL 1.1 release

Posted by James McCoy <ja...@debian.org>.
On Sun, Jun 12, 2016 at 02:56:07PM -0400, James McCoy wrote:
> OpenSSL is preparing a 1.1.0 release which introduces API and ABI
> incompatibilities (described in an in-progress[0] wiki page).
> 
> [0]: https://wiki.openssl.org/index.php/1.1_API_Changes
> 
> A rebuild[1] of all Debian packages using OpenSSL found that serf is
> affected by these changes, specifically making BIO/BIO_METHOD opaque and
> removing the need for the locking functions[2].
> 
> [1]: https://breakpoint.cc/openssl-1.1-rebuild-2016-05-29/
> [2]: https://github.com/openssl/openssl/blob/dae00d631fdaed48d88c454864abbd6ce99c63d6/include/openssl/crypto.h#L209-L216
> 
> The attached patches fix the build and pass the test suites both with
> OpenSSL 1.0.2h and a pre-release of OpenSSL 1.1.0, but more eyes are
> always good.

Proposed commit messages below:

trunk
[[[
Adapt to OpenSSL 1.1.x API changes.

OpenSSL 1.1.x makes various types opaque, requiring the use of
accessors, and rewrote the state machine describing the handshake
process.  Of particular interest to serf are the BIO, BIO_METHOD, and
X509_STORE types.

* buckets/ssl_buckets.c
  (): New USE_OPENSSL_1_1_API define
  (): New X509_STORE_get0_param() define for use with pre-1.1.x OpenSSL
  (detect_renegotiate): Use SSL_get_state to check for the
    TLS_ST_SW_HELLO_REQ state, indicating the server is starting a new
    negotiation.
  (bio_set_data, bio_get_data): New functions to abstract access to
    the BIO data.
  (bio_bucket_read, bio_bucket_write, bio_file_read, bio_file_write,
   bio_file_gets): Use bio_get_data.
  (bio_bucket_create): Use BIO accessor functions when available.
  (bio_meth_bucket_new, bio_meth_file_new): New functions to abstract
    creation of BIO_METHOD.  With OpenSSL 1.1.x or newer, the BIO_meth_*
    functions are used to allocate a new BIO_METOD and set the
    callbacks, otherwise the pointers to the statically defined structs
    are used.
  (ocsp_callback): Use OCSP_response_status to get status instead of
    accessing internals of OCSP_RESPONSE struct.  Remove unused
    OCSP_RESPBYTES variable.
  (ssl_decrypt): Use SSL_get_state to check for the TLS_ST_OK state,
    indicating completed handshake.
  (init_ssl_libraries): Exclude threading code when OpenSSL 1.1.x is in
    use since OpenSSL now handles this appropriately without users of
    the library setting up locking functions.
  (ssl_need_client_cert, ssl_init_context, serf_ssl_load_cert_file,
   serf_ssl_add_crl_from_file): Use new bio_meth_*_new functions to
   provide the BIO_METHOD* to BIO_new().  Also use the bio_set_data
   function to set the data for the callback.

* test/MockHTTPinC/MockHTTP_server.c
  (): New USE_OPENSSL_1_1_API define
  (bio_set_data, bio_get_data): New functions to abstract access to
    the BIO data.
  (bio_apr_socket_read, bio_apr_socket_write): Use bio_get_data.
  (bio_apr_socket_create): Use BIO accessor functions when available.
  (bio_meth_apr_socket_new): New function to abstract creation of
    BIO_METHOD.  With OpenSSL 1.1.x or newer, the BIO_meth_* functions
    are used to allocate a new BIO_METOD and set the callbacks,
    otherwise the pointer to the statically defined struct is used.
  (initSSLCtx): Use new bio_meth_apr_socket_new function to
   provide the BIO_METHOD* to BIO_new().  Also use the bio_set_data
   function to set the data for the callback.
]]]

1.3.x
[[[
Adapt to OpenSSL 1.1.x API changes.

OpenSSL 1.1.x makes various types opaque, requiring the use of
accessors, and rewrote the state machine describing the handshake
process.  Of particular interest to serf are the BIO, BIO_METHOD, and
X509_STORE types.

* buckets/ssl_buckets.c
  (): New USE_OPENSSL_1_1_API define
  (): New X509_STORE_get0_param() define for use with pre-1.1.x OpenSSL
  (bio_set_data, bio_get_data): New functions to abstract access to
    the BIO data.
  (bio_bucket_read, bio_bucket_write, bio_file_read, bio_file_write,
   bio_file_gets): Use bio_get_data.
  (bio_bucket_create): Use BIO accessor functions when available.
  (bio_meth_bucket_new, bio_meth_file_new): New functions to abstract
    creation of BIO_METHOD.  With OpenSSL 1.1.x or newer, the BIO_meth_*
    functions are used to allocate a new BIO_METOD and set the
    callbacks, otherwise the pointers to the statically defined structs
    are used.
  (init_ssl_libraries): Exclude threading code when OpenSSL 1.1.x is in
    use since OpenSSL now handles this appropriately without users of
    the library setting up locking functions.
  (ssl_need_client_cert, ssl_init_context): Use new bio_meth_*_new
    functions to provide the BIO_METHOD* to BIO_new().  Also use the
    bio_set_data function to set the data for the callback.

* test/server/test_sslserver.c
  (): New USE_OPENSSL_1_1_API define
  (bio_set_data, bio_get_data): New functions to abstract access to
    the BIO data.
  (bio_apr_socket_read, bio_apr_socket_write): Use bio_get_data.
  (bio_apr_socket_create): Use BIO accessor functions when available.
  (bio_meth_apr_socket_new): New function to abstract creation of
    BIO_METHOD.  With OpenSSL 1.1.x or newer, the BIO_meth_* functions
    are used to allocate a new BIO_METOD and set the callbacks,
    otherwise the pointer to the statically defined struct is used.
  (validate_client_certificate): Use new bio_meth_apr_socket_new
    function to provide the BIO_METHOD* to BIO_new().  Also use the
    bio_set_data function to set the data for the callback.
]]]

Cheers,
-- 
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB