Discussion:
OpenSSL 1.1.0 support
(too old to reply)
Dirkjan Bussink
2016-08-29 12:22:23 UTC
Permalink
Hi all,

I've attached a patch for support for OpenSSL 1.1.0. That version changes quite a few things, mostly it makes a lot of the structures now opaque and private and provides functions to interact with them. Most of this change consists of using these new functions on OpenSSL 1.1.0 and newer.

There are a few things worth calling out specifically in this patch:

- I was not 100% clear on the handshake logic. It looked like it tried to detect if a handshake was ever attempted and distinguish that from a failure case. I've used the new state mechanism available through SSL_get_state to hopefully mimic similar behavior. I might have gotten this totally wrong though.
- The Makefile change was needed so that linking the OpenSSL bits also pulls in dl if needed (OpenSSL uses this itself). Also OpenSSL will now use pthread by default, so maybe that also should be added? Although I've used USE_PTHREAD_PSHARED for now in testing to link that.
- The code guarded with #ifdef SSL_CTX_get_tlsext_status_arg ideally would also use that macro, but there seems to be a closing brace missing at https://github.com/openssl/openssl/blob/fddfc0afc84728f8a5140685163e66ce6471742d/include/openssl/tls1.h#L300-L301 so it throws an error. That's why I've used the implementation of that macro in the code instead.

What this does not address at the moment is fixing the use of deprecated functions. These are the warnings still present with these changes:

[jessie-amd64] src/ssl_sock.c: In function 'ssl_tlsext_ticket_key_cb':
[jessie-amd64] src/ssl_sock.c:492:3: warning: 'RAND_pseudo_bytes' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/rand.h:47) [-Wdeprecated-declarations]
[jessie-amd64] if(!RAND_pseudo_bytes(iv, EVP_MAX_IV_LENGTH))
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c: In function 'ssl_sock_prepare_ctx':
[jessie-amd64] src/ssl_sock.c:2731:3: warning: 'TLSv1_server_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1597) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(ctx, TLSv1_server_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:2734:3: warning: 'TLSv1_1_server_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1603) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(ctx, TLSv1_1_server_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:2738:3: warning: 'TLSv1_2_server_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1609) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(ctx, TLSv1_2_server_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:2824:13: warning: assignment discards 'const' qualifier from pointer target type
[jessie-amd64] cipher = sk_SSL_CIPHER_value(ciphers, idx);
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c: In function 'ssl_sock_prepare_srv_ctx':
[jessie-amd64] src/ssl_sock.c:3111:3: warning: 'TLSv1_client_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1598) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(srv->ssl_ctx.ctx, TLSv1_client_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:3114:3: warning: 'TLSv1_1_client_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1604) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(srv->ssl_ctx.ctx, TLSv1_1_client_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:3118:3: warning: 'TLSv1_2_client_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1610) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(srv->ssl_ctx.ctx, TLSv1_2_client_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c: In function '__ssl_sock_deinit':
[jessie-amd64] src/ssl_sock.c:6254:9: warning: 'ERR_remove_state' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/err.h:247) [-Wdeprecated-declarations]
[jessie-amd64] ERR_remove_state(0);
[jessie-amd64]


Let me know what you all think.

Cheers,

Dirkjan
Willy Tarreau
2016-08-29 13:15:51 UTC
Permalink
Hi Dirkjan,

CCing Emeric who will know better than me how to respond, but I have some
questions at the end.
Post by Dirkjan Bussink
Hi all,
I've attached a patch for support for OpenSSL 1.1.0. That version changes quite a few things, mostly it makes a lot of the structures now opaque and private and provides functions to interact with them. Most of this change consists of using these new functions on OpenSSL 1.1.0 and newer.
- I was not 100% clear on the handshake logic. It looked like it tried to detect if a handshake was ever attempted and distinguish that from a failure case. I've used the new state mechanism available through SSL_get_state to hopefully mimic similar behavior. I might have gotten this totally wrong though.
- The Makefile change was needed so that linking the OpenSSL bits also pulls in dl if needed (OpenSSL uses this itself). Also OpenSSL will now use pthread by default, so maybe that also should be added? Although I've used USE_PTHREAD_PSHARED for now in testing to link that.
- The code guarded with #ifdef SSL_CTX_get_tlsext_status_arg ideally would also use that macro, but there seems to be a closing brace missing at https://github.com/openssl/openssl/blob/fddfc0afc84728f8a5140685163e66ce6471742d/include/openssl/tls1.h#L300-L301 so it throws an error. That's why I've used the implementation of that macro in the code instead.
[jessie-amd64] src/ssl_sock.c:492:3: warning: 'RAND_pseudo_bytes' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/rand.h:47) [-Wdeprecated-declarations]
[jessie-amd64] if(!RAND_pseudo_bytes(iv, EVP_MAX_IV_LENGTH))
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:2731:3: warning: 'TLSv1_server_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1597) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(ctx, TLSv1_server_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:2734:3: warning: 'TLSv1_1_server_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1603) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(ctx, TLSv1_1_server_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:2738:3: warning: 'TLSv1_2_server_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1609) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(ctx, TLSv1_2_server_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:2824:13: warning: assignment discards 'const' qualifier from pointer target type
[jessie-amd64] cipher = sk_SSL_CIPHER_value(ciphers, idx);
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:3111:3: warning: 'TLSv1_client_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1598) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(srv->ssl_ctx.ctx, TLSv1_client_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:3114:3: warning: 'TLSv1_1_client_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1604) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(srv->ssl_ctx.ctx, TLSv1_1_client_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:3118:3: warning: 'TLSv1_2_client_method' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/ssl.h:1610) [-Wdeprecated-declarations]
[jessie-amd64] SSL_CTX_set_ssl_version(srv->ssl_ctx.ctx, TLSv1_2_client_method());
[jessie-amd64] ^
[jessie-amd64] src/ssl_sock.c:6254:9: warning: 'ERR_remove_state' is deprecated (declared at /data/build/jessie-amd64/packages/haproxy/tmp-install/openssl-static/include/openssl/err.h:247) [-Wdeprecated-declarations]
[jessie-amd64] ERR_remove_state(0);
[jessie-amd64]
Let me know what you all think.
Since the API experienced a major change and causes a lot of #ifdefs,
what do you think about proceeding differently and definining these
API functions/macros for older versions so that only the new code
remains instead ? It seems to me this should be quite possible.
Example below :

+#if (OPENSSL_VERSION_NUMBER >= 0x1010000fL)
+ const unsigned char *sid_ctx_data;
+
+ sid_data = SSL_SESSION_get_id(sess, &sid_length);
+ sid_ctx_data = SSL_SESSION_get0_id_context(sess, &sid_ctx_length);
+ SSL_SESSION_set1_id(sess, sid_data, 0);
+ SSL_SESSION_set1_id_context(sess, sid_ctx_data, 0);
+#else
sid_length = sess->session_id_length;
- sess->session_id_length = 0;
sid_ctx_length = sess->sid_ctx_length;
+ sess->session_id_length = 0;
sess->sid_ctx_length = 0;
+ sid_data = sess->session_id;
+#endif

We'd have to declare SSL_SESSION_get_id(), SSL_SESSION_get0_id_context()
and SSL_SESSION_set1_id() if OPENSSL_VERSION_NUMBER < 0x1010000fL and
that would be done, we'd only keep the new version.

It's interesting to work this way because it avoids the issues that happen
when fixing bugs only in one half of the ifdef without realizing that the
other half needs a different fix.

The part below is an even more obvious candidate :

+#if (OPENSSL_VERSION_NUMBER >= 0x1010000fL)
+ SSL_SESSION_set1_id(sess, sid_data, sid_length);
+ SSL_SESSION_set1_id_context(sess, sid_ctx_data, sid_ctx_length);
+#else
sess->session_id_length = sid_length;
sess->sid_ctx_length = sid_ctx_length;
+#endif

And here we see an example where you had to do it but it would rather be
centralized at the top of ssl_sock.h for example :

- if (!ctx->tlsext_status_cb) {
+#ifndef SSL_CTX_get_tlsext_status_cb
+# define SSL_CTX_get_tlsext_status_cb(ctx, cb) \
+ *cb = (void (*) (void))ctx->tlsext_status_cb;
+#endif
+ SSL_CTX_get_tlsext_status_cb(ctx, &callback);

Thanks!
Willy
Dirkjan Bussink
2016-09-17 20:56:13 UTC
Permalink
Hi Willy,
Post by Willy Tarreau
Hi Dirkjan,
CCing Emeric who will know better than me how to respond, but I have some
questions at the end.
I've gone for a somewhat different approach in this patch with a compatibility header file. It defines some inline functions from 1.1.0 when they are not available. Right now it implements the minimal things that HAProxy needs, so they aren't full replacements of what OpenSSL 1.1.0 would provide.

It's in a separate header file, was not sure if this was something that should go in include/proto/ssl_sock.h then. Also brings up the question what the minimum version of OpenSSL is that is supported? A lot of the functions used are already available in 1.0.0 (which is already EOL), so not sure if the compatibility code is needed for 0.9.8 then?

Is this more the approach you were thinking about?

Cheers,

Dirkjan
Willy Tarreau
2016-10-26 09:40:06 UTC
Permalink
Hi Dirkjan,

sorry for the delay.
Post by Dirkjan Bussink
I've gone for a somewhat different approach in this patch with a
compatibility header file. It defines some inline functions from 1.1.0 when
they are not available. Right now it implements the minimal things that
HAProxy needs, so they aren't full replacements of what OpenSSL 1.1.0 would
provide.
I definitely like this approach, and to be clear I want it merged before
1.7 release.
Post by Dirkjan Bussink
It's in a separate header file, was not sure if this was something that
should go in include/proto/ssl_sock.h then. Also brings up the question what
the minimum version of OpenSSL is that is supported? A lot of the functions
used are already available in 1.0.0 (which is already EOL), so not sure if
the compatibility code is needed for 0.9.8 then?
Yes I'd rather have it work with 0.9.8 as it's still supported and used by
some LTS distros. For example, RHEL5's regular support is due till March 2017
and extended support till november 2020. It's not very likely that such users
will decide to upgrade to 1.7 now especially since some of the SSL-related
features only work with newer versions. But if we can make it work as it used
to with limited effort it's better. Is your current patch capable of supporting
0.9.8 ?
Post by Dirkjan Bussink
Is this more the approach you were thinking about?
Yes definitely. Also your new patch is much more readable and will make it
easier to drop older versions in the future, I like it. I'll ping Emeric
again since we got no response (ie I'll yell in the office) :-)

Thanks!
Willy
Dirkjan Bussink
2016-11-07 12:02:33 UTC
Permalink
Hi Willy,
Post by Willy Tarreau
Yes I'd rather have it work with 0.9.8 as it's still supported and used by
some LTS distros. For example, RHEL5's regular support is due till March 2017
and extended support till november 2020. It's not very likely that such users
will decide to upgrade to 1.7 now especially since some of the SSL-related
features only work with newer versions. But if we can make it work as it used
to with limited effort it's better. Is your current patch capable of supporting
0.9.8 ?
I don't think there's a reason it couldn't work. I haven't tested it though yet, only against 1.0.1 and 1.1.0 at this point.
Post by Willy Tarreau
Yes definitely. Also your new patch is much more readable and will make it
easier to drop older versions in the future, I like it. I'll ping Emeric
again since we got no response (ie I'll yell in the office) :-)
Alright, hopefully it's not too complicated to get this in then.

Cheers,

Dirkjan
Willy Tarreau
2016-11-08 17:58:40 UTC
Permalink
Just resending, I noticed that my message didn't make it through the list,
and no, it was not caught by the anti-spam :-)
Post by Willy Tarreau
Hi Dirkjan,
Post by Dirkjan Bussink
Hi Willy,
Post by Willy Tarreau
Yes I'd rather have it work with 0.9.8 as it's still supported and used by
some LTS distros. For example, RHEL5's regular support is due till March 2017
and extended support till november 2020. It's not very likely that such users
will decide to upgrade to 1.7 now especially since some of the SSL-related
features only work with newer versions. But if we can make it work as it used
to with limited effort it's better. Is your current patch capable of supporting
0.9.8 ?
I don't think there's a reason it couldn't work. I haven't tested it though yet, only against 1.0.1 and 1.1.0 at this point.
OK so I got a few build errors with openssl-0.9.8za but I think they're
not impossible to get rid of, because every time it was just a matter of
having some of these functions already declard in openssl (which sounds
quite strange), hence the "conflicting type" declaration. What I don't
understand is why I do find them both in 0.9.8 and 1.0.1 still they don't
cause this build failure in 1.0.1. Any insight would be welcome, as I'm
sure we're pretty close from something working. I checked the sources and
didn't see any suspicious #if. Maybe we should rearrange our own compat/openssl
so that we don't redefine them on 0.9.8 ? I have not tested on 1.0.0 either
but I think it's really dead even on LTS distros.
Thanks,
Willy
------------------------------------------------
include/compat/openssl.h:31:36: error: static declaration of 'SSL_SESSION_get_id' follows non-static declaration
/dev/shm/openssl-0.9.8za/include/openssl/ssl.h:1433:22: note: previous declaration of 'SSL_SESSION_get_id' was here
include/compat/openssl.h:37:32: error: conflicting types for 'X509_NAME_get_entry'
In file included from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183:0,
/dev/shm/openssl-0.9.8za/include/openssl/x509.h:1106:18: note: previous declaration of 'X509_NAME_get_entry' was here
include/compat/openssl.h:42:28: error: conflicting types for 'X509_NAME_ENTRY_get_object'
In file included from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183:0,
/dev/shm/openssl-0.9.8za/include/openssl/x509.h:1127:15: note: previous declaration of 'X509_NAME_ENTRY_get_object' was here
include/compat/openssl.h:47:28: error: conflicting types for 'X509_NAME_ENTRY_get_data'
In file included from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183:0,
/dev/shm/openssl-0.9.8za/include/openssl/x509.h:1128:15: note: previous declaration of 'X509_NAME_ENTRY_get_data' was here
include/compat/openssl.h:52:19: error: conflicting types for 'ASN1_STRING_length'
In file included from /dev/shm/openssl-0.9.8za/include/openssl/objects.h:960:0,
from /dev/shm/openssl-0.9.8za/include/openssl/evp.h:98,
from /dev/shm/openssl-0.9.8za/include/openssl/x509.h:73,
from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183,
/dev/shm/openssl-0.9.8za/include/openssl/asn1.h:795:5: note: previous declaration of 'ASN1_STRING_length' was here
include/compat/openssl.h:57:19: error: static declaration of 'X509_NAME_entry_count' follows non-static declaration
In file included from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183:0,
/dev/shm/openssl-0.9.8za/include/openssl/x509.h:1095:7: note: previous declaration of 'X509_NAME_entry_count' was here
include/compat/openssl.h:60:1: error: expected ';' before '}' token
include/compat/openssl.h:68:20: error: conflicting types for 'X509_ALGOR_get0'
In file included from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183:0,
/dev/shm/openssl-0.9.8za/include/openssl/x509.h:871:6: note: previous declaration of 'X509_ALGOR_get0' was here
include/compat/openssl.h:31:36: error: static declaration of 'SSL_SESSION_get_id' follows non-static declaration
In file included from include/types/listener.h:29:0,
from include/types/global.h:31,
/dev/shm/openssl-0.9.8za/include/openssl/ssl.h:1433:22: note: previous declaration of 'SSL_SESSION_get_id' was here
include/compat/openssl.h:37:32: error: conflicting types for 'X509_NAME_get_entry'
In file included from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183:0,
from include/types/listener.h:29,
from include/types/global.h:31,
/dev/shm/openssl-0.9.8za/include/openssl/x509.h:1106:18: note: previous declaration of 'X509_NAME_get_entry' was here
include/compat/openssl.h:42:28: error: conflicting types for 'X509_NAME_ENTRY_get_object'
In file included from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183:0,
from include/types/listener.h:29,
from include/types/global.h:31,
/dev/shm/openssl-0.9.8za/include/openssl/x509.h:1127:15: note: previous declaration of 'X509_NAME_ENTRY_get_object' was here
include/compat/openssl.h:47:28: error: conflicting types for 'X509_NAME_ENTRY_get_data'
In file included from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183:0,
from include/types/listener.h:29,
from include/types/global.h:31,
/dev/shm/openssl-0.9.8za/include/openssl/x509.h:1128:15: note: previous declaration of 'X509_NAME_ENTRY_get_data' was here
include/compat/openssl.h:52:19: error: conflicting types for 'ASN1_STRING_length'
In file included from /dev/shm/openssl-0.9.8za/include/openssl/objects.h:960:0,
from /dev/shm/openssl-0.9.8za/include/openssl/evp.h:98,
from /dev/shm/openssl-0.9.8za/include/openssl/x509.h:73,
from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183,
from include/types/listener.h:29,
from include/types/global.h:31,
/dev/shm/openssl-0.9.8za/include/openssl/asn1.h:795:5: note: previous declaration of 'ASN1_STRING_length' was here
include/compat/openssl.h:57:19: error: static declaration of 'X509_NAME_entry_count' follows non-static declaration
In file included from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183:0,
from include/types/listener.h:29,
from include/types/global.h:31,
/dev/shm/openssl-0.9.8za/include/openssl/x509.h:1095:7: note: previous declaration of 'X509_NAME_entry_count' was here
include/compat/openssl.h:60:1: error: expected ';' before '}' token
include/compat/openssl.h:68:20: error: conflicting types for 'X509_ALGOR_get0'
In file included from /dev/shm/openssl-0.9.8za/include/openssl/ssl.h:183:0,
from include/types/listener.h:29,
from include/types/global.h:31,
/dev/shm/openssl-0.9.8za/include/openssl/x509.h:871:6: note: previous declaration of 'X509_ALGOR_get0' was here
src/shctx.c:660:2: warning: passing argument 2 of 'SSL_CTX_sess_set_get_cb' from incompatible pointer type [enabled by default]
In file included from include/types/listener.h:29:0,
from include/types/global.h:31,
/dev/shm/openssl-0.9.8za/include/openssl/ssl.h:849:6: note: expected 'struct SSL_SESSION * (*)(struct ssl_st *, unsigned char *, int, int *)' but argument is of type 'struct SSL_SESSION * (*)(struct SSL *, const unsigned char *, int, int *)'
make: *** [src/shctx.o] Error 1
make: *** Waiting for unfinished jobs....
make: *** [src/ssl_sock.o] Error 1
------------------------------------------------
Willy Tarreau
2016-11-08 20:12:21 UTC
Permalink
Hi Dirkjan,

I finally merged your patch after discussing with Emeric. He's fine with
it as well. Both of us think that the breakage of openssl 0.9.8 is not a
showstopper at the moment and that the best way to know if/how it needs to
be fixed is to let it go in the wild. Given that openssl 1.1.0 was released
3 months ago and 0.9.8 has been dead for almost a year, I prefer that our
new haproxy version focuses on the future than on the past, especially with
the reduced life of new versions (eg: 1.0.2 only has two more years left).

I got two annoying warnings affecting either older versions or newer ones,
each time due to a const being put at the right place in a function prototype.
So I added another patch to deal with this, it defines a new macro called
__OPENSSL_110_CONST__ which equals "const" on 1.1.0 and nothing on older
versions.

I also moved the include file to include/proto/openssl-compat.h since it
only defines prototypes. But the main reason I must confess is that the
previous choice (include/compat/openssl.h) was causing me a lot of pain by
breaking my auto-completion, and after spending a few hours on it, I had
to forfeit on trying to get my fingers to switch "comm<tab>" instead of the
shorter "c<tab>" they've been trained to for a decade :-/

I'm appending the 3 patches I added on top of yours and that I merged as
one single patch. As you said, 1.1.0 builds with a few "deprecated" warnings
which are still OK for now as the affected functions are part of the API and
that we can clean later. I tested 1.0.1 and 1.0.2 and they were OK as well.

Now if anyone is interested in trying to port the code to 0.9.8 (should not be
too hard, just a few more #ifdef to add), proposals are welcome.

Thanks!
Willy
Apollon Oikonomopoulos
2016-11-09 09:44:41 UTC
Permalink
Hi Willy, Dirkjan,
Post by Willy Tarreau
Hi Dirkjan,
I finally merged your patch after discussing with Emeric. He's fine with
it as well.
Thanks for this. Is it too much of a hassle to ask for a 1.6 backport?

We currently have a release-critical bug in Debian for OpenSSL 1.1
compatibility[1], so it would greatly help us. I could go ahead and try
to make a backport myself, however I admit I'm a bit reluctant to touch
OpenSSL-related code at this point.

Thanks!
Apollon

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=828337
Willy Tarreau
2016-11-09 11:07:26 UTC
Permalink
Post by Apollon Oikonomopoulos
Hi Willy, Dirkjan,
Post by Willy Tarreau
Hi Dirkjan,
I finally merged your patch after discussing with Emeric. He's fine with
it as well.
Thanks for this. Is it too much of a hassle to ask for a 1.6 backport?
Given that it breaks support for older versions (0.9.8 at least), for
now it's out of question. And it has received only limited testing. If
we manage to stabilise the patch to properly handle all versions where
1.6 currently works, then maybe the question could be reconsidered.
Post by Apollon Oikonomopoulos
We currently have a release-critical bug in Debian for OpenSSL 1.1
compatibility[1], so it would greatly help us. I could go ahead and try
to make a backport myself, however I admit I'm a bit reluctant to touch
OpenSSL-related code at this point.
You should definitely avoid it, the testing is insufficient for now.

Another, better option would be to upgrade the haproxy package to 1.7 for
the next debian release so that it matches the new openssl version as well.
There are (too) few changes in 1.7 compared to 1.6, it mostly accumulated
all the fixes that resulted from the bugs coming with the new architecture
brought in 1.6. I consider 1.7 almost as stable as 1.6, and will encourage
users to upgrade. I don't know how much time left you have to decide on a
version for a new distro (I don't know the process at all).

Thanks,
Willy
Apollon Oikonomopoulos
2016-11-09 11:28:07 UTC
Permalink
Post by Willy Tarreau
Post by Apollon Oikonomopoulos
Thanks for this. Is it too much of a hassle to ask for a 1.6 backport?
Given that it breaks support for older versions (0.9.8 at least), for
now it's out of question. And it has received only limited testing. If
we manage to stabilise the patch to properly handle all versions where
1.6 currently works, then maybe the question could be reconsidered.
Agreed, thanks for the clarification.
Post by Willy Tarreau
Post by Apollon Oikonomopoulos
We currently have a release-critical bug in Debian for OpenSSL 1.1
compatibility[1], so it would greatly help us. I could go ahead and try
to make a backport myself, however I admit I'm a bit reluctant to touch
OpenSSL-related code at this point.
You should definitely avoid it, the testing is insufficient for now.
Another, better option would be to upgrade the haproxy package to 1.7 for
the next debian release so that it matches the new openssl version as well.
There are (too) few changes in 1.7 compared to 1.6, it mostly accumulated
all the fixes that resulted from the bugs coming with the new architecture
brought in 1.6. I consider 1.7 almost as stable as 1.6, and will encourage
users to upgrade. I don't know how much time left you have to decide on a
version for a new distro (I don't know the process at all).
Let's say that we must have settled with a stable-enough version by
early December. Is there a chance there will be a final 1.7 release by
then?

Regards,
Apollon
Willy Tarreau
2016-11-09 13:10:02 UTC
Permalink
Post by Apollon Oikonomopoulos
Let's say that we must have settled with a stable-enough version by
early December. Is there a chance there will be a final 1.7 release by
then?
Definitely. I hope to have it maybe at the end of next week. Given
that I'm always late when releasing compared to what I announce, use
end of month as a safe target :-)

Cheers,
Willy
Dirkjan Bussink
2016-11-17 14:40:13 UTC
Permalink
Hi Willy,

Thanks for getting this merged!

Cheers,

Dirkjan
Post by Willy Tarreau
Hi Dirkjan,
I finally merged your patch after discussing with Emeric. He's fine with
it as well. Both of us think that the breakage of openssl 0.9.8 is not a
showstopper at the moment and that the best way to know if/how it needs to
be fixed is to let it go in the wild. Given that openssl 1.1.0 was released
3 months ago and 0.9.8 has been dead for almost a year, I prefer that our
new haproxy version focuses on the future than on the past, especially with
the reduced life of new versions (eg: 1.0.2 only has two more years left).
I got two annoying warnings affecting either older versions or newer ones,
each time due to a const being put at the right place in a function prototype.
So I added another patch to deal with this, it defines a new macro called
__OPENSSL_110_CONST__ which equals "const" on 1.1.0 and nothing on older
versions.
I also moved the include file to include/proto/openssl-compat.h since it
only defines prototypes. But the main reason I must confess is that the
previous choice (include/compat/openssl.h) was causing me a lot of pain by
breaking my auto-completion, and after spending a few hours on it, I had
to forfeit on trying to get my fingers to switch "comm<tab>" instead of the
shorter "c<tab>" they've been trained to for a decade :-/
I'm appending the 3 patches I added on top of yours and that I merged as
one single patch. As you said, 1.1.0 builds with a few "deprecated" warnings
which are still OK for now as the affected functions are part of the API and
that we can clean later. I tested 1.0.1 and 1.0.2 and they were OK as well.
Now if anyone is interested in trying to port the code to 0.9.8 (should not be
too hard, just a few more #ifdef to add), proposals are welcome.
Thanks!
Willy
<0001-WIP-ssl-1.1.0-protect-compat-openssl.h-against-multi.patch><0002-WIP-ssl-1.1.0-add-a-const-type-modifier-only-for-1.1.patch><0003-WIP-ssl-1.1.0-move-include-to-proto-openssl-compat.h.patch>
Continue reading on narkive:
Loading...