OpenSSL 1.1 breaks configure and more

Christoph Berg <myon@debian.org> writes:

as reported by Debian's OpenSSL maintainers, PostgreSQL is failing to
build against a snapshot of the upcoming 1.1.0 version.

The errors you report make it sound like they broke API compatibility
wholesale. Was that really their intent? If so, where are the changes
documented?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 06/27/2016 05:24 PM, Tom Lane wrote:

Christoph Berg <myon@debian.org> writes:

as reported by Debian's OpenSSL maintainers, PostgreSQL is failing to
build against a snapshot of the upcoming 1.1.0 version.

The errors you report make it sound like they broke API compatibility
wholesale. Was that really their intent? If so, where are the changes
documented?

I do not see that they have documented the removal of the
SSL_library_init symbol anywhere. They changed the function into a macro
in the following commit. I guess we have to check for some other symbol,
like SSL_new.

https://github.com/openssl/openssl/commit/7fa792d14d06cdaca18f225b1d2d8daf8ed24fd7

They have also, which is in the release notes, broken API compatibility
when they made the BIO and BIO_METHOD structs opaque. This will probably
require some ugly ugly #ifs or compatibility macros from us.

They also seem to have broken our OpenSSL thread safety callback when
they added their new threading API and removed the CRYPTO_LOCK define. I
have reported this in their issue tracker
(https://github.com/openssl/openssl/issues/1260).

In addition to this there are a couple of deprecated functions
(DH_generate_parameters() and OPENSSL_config()), but they look pretty
easy to handle.

I think much of the above is missing from the release notes I have
found. I hope they will be more complete at the time of the release. I
am working on a patch to handle these API changes.

- https://www.openssl.org/news/openssl-1.1.0-notes.html
- https://wiki.openssl.org/index.php/1.1_API_Changes

Andreas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: Andreas Karlsson 2016-06-27 <8a0a5959-0b83-3dc8-d9e7-66ce8c1c5bc7@proxel.se>

The errors you report make it sound like they broke API compatibility
wholesale. Was that really their intent? If so, where are the changes
documented?

I do not see that they have documented the removal of the SSL_library_init
symbol anywhere. They changed the function into a macro in the following
commit. I guess we have to check for some other symbol, like SSL_new.

I'm not an autoconf expert, but as said in the original mail, I could
get the SSL_library_init check to work, even if that's a macro now:

-      AC_CHECK_LIB(ssl,    SSL_library_init, [], [AC_MSG_ERROR([library 'ssl' is required for OpenSSL])])
+      AC_CHECK_LIB([ssl],  [SSL_library_init])

(I haven't investigated if that's due to the quoting, the removal of
the extra arguments, or simply because I reran autoreconf.)

I think much of the above is missing from the release notes I have found. I
hope they will be more complete at the time of the release. I am working on
a patch to handle these API changes.

- https://www.openssl.org/news/openssl-1.1.0-notes.html
- https://wiki.openssl.org/index.php/1.1_API_Changes

Nod, I was also disappointed when browsing the API changes document,
given it didn't mention any of the problems I was seeing.

Christoph

On 06/27/2016 08:12 PM, Christoph Berg wrote:

Re: Andreas Karlsson 2016-06-27 <8a0a5959-0b83-3dc8-d9e7-66ce8c1c5bc7@proxel.se>

The errors you report make it sound like they broke API compatibility
wholesale. Was that really their intent? If so, where are the changes
documented?

I do not see that they have documented the removal of the SSL_library_init
symbol anywhere. They changed the function into a macro in the following
commit. I guess we have to check for some other symbol, like SSL_new.

I'm not an autoconf expert, but as said in the original mail, I could
get the SSL_library_init check to work, even if that's a macro now:

Yes, we could do that, but I do not think we should check for the
existence of a backwards compatibility macro. Actually I think we may
want to skip much of the OpenSSL initialization code when compiling
against OpenSSL 1.1 since they have now added automatic initialization
of the library. Instead I think we should check for something we
actually will use like SSL_CTX_new().

Andreas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Jun 28, 2016 at 3:21 AM, Andreas Karlsson <andreas@proxel.se> wrote:

Yes, we could do that, but I do not think we should check for the existence
of a backwards compatibility macro. Actually I think we may want to skip
much of the OpenSSL initialization code when compiling against OpenSSL 1.1
since they have now added automatic initialization of the library. Instead I
think we should check for something we actually will use like SSL_CTX_new().

Agreed. Changing the routine being checked may be a good idea in this
case, and we surely want to check for something that is used in the
frontend and the backend. So why not something more generic like
SSL_read or SSL_write?
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

Here is an initial set of patches related to OpenSSL 1.1. Everything
should still build fine on older OpenSSL versions (and did when I tested
with 1.0.2h).

0001-Fixes-for-compiling-with-OpenSSL-1.1.patch

This patch fixes the code so it builds with OpenSSL 1.1 (except the
CRYPTO_LOCK issue I have reported to the OpenSSL team).

- Makes our configure script check for SSL_new instead
- Uses functions instead of direct access to struct members

0002-Define-CRYPTO_LOCK-for-OpenSSL-1.1-compat.patch

Fix for the removal of the CRYPTO_LOCK define. I am trying to convince
them to add the define back. :)

0003-Remove-OpenSSL-1.1-deprecation-warnings.patch

Silence all warnings. This commit changes more things and is not
necessary for getting PostgreSQL to build against 1.1.

- Silences deprecation other warnings related to that OpenSSL 1.1 now
1) automatically initializes the library and 2) no longer uses the
locking callback.
- Silences deprecation warning when generating DH parameters.

Andreas

On Fri, Jul 1, 2016 at 9:27 AM, Andreas Karlsson <andreas@proxel.se> wrote:

Hi,

Here is an initial set of patches related to OpenSSL 1.1. Everything should
still build fine on older OpenSSL versions (and did when I tested with
1.0.2h).

0001-Fixes-for-compiling-with-OpenSSL-1.1.patch

This patch fixes the code so it builds with OpenSSL 1.1 (except the
CRYPTO_LOCK issue I have reported to the OpenSSL team).

- Makes our configure script check for SSL_new instead
- Uses functions instead of direct access to struct members

0002-Define-CRYPTO_LOCK-for-OpenSSL-1.1-compat.patch

Fix for the removal of the CRYPTO_LOCK define. I am trying to convince them
to add the define back. :)

0003-Remove-OpenSSL-1.1-deprecation-warnings.patch

Silence all warnings. This commit changes more things and is not necessary
for getting PostgreSQL to build against 1.1.

- Silences deprecation other warnings related to that OpenSSL 1.1 now 1)
automatically initializes the library and 2) no longer uses the locking
callback.
- Silences deprecation warning when generating DH parameters.

Those patches are going to need a careful review by looking at the
areas they are changing, and a backpatch. On Arch there is no test
package available except in AUR. And that's the pre3 release, OpenSSL
folks are on pre5 now with their beta2. It would be annoying to
compile it manually, but if there is no other way... Is Debian up to
date with 1.1.0 beta2 in its snapshot packages?
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Jul 1, 2016 at 4:08 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Fri, Jul 1, 2016 at 9:27 AM, Andreas Karlsson <andreas@proxel.se>
wrote:

Hi,

Here is an initial set of patches related to OpenSSL 1.1. Everything

should

still build fine on older OpenSSL versions (and did when I tested with
1.0.2h).

0001-Fixes-for-compiling-with-OpenSSL-1.1.patch

This patch fixes the code so it builds with OpenSSL 1.1 (except the
CRYPTO_LOCK issue I have reported to the OpenSSL team).

- Makes our configure script check for SSL_new instead
- Uses functions instead of direct access to struct members

0002-Define-CRYPTO_LOCK-for-OpenSSL-1.1-compat.patch

Fix for the removal of the CRYPTO_LOCK define. I am trying to convince

them

to add the define back. :)

0003-Remove-OpenSSL-1.1-deprecation-warnings.patch

Silence all warnings. This commit changes more things and is not

necessary

for getting PostgreSQL to build against 1.1.

- Silences deprecation other warnings related to that OpenSSL 1.1 now 1)
automatically initializes the library and 2) no longer uses the locking
callback.
- Silences deprecation warning when generating DH parameters.

Those patches are going to need a careful review by looking at the
areas they are changing, and a backpatch. On Arch there is no test
package available except in AUR. And that's the pre3 release, OpenSSL
folks are on pre5 now with their beta2. It would be annoying to
compile it manually, but if there is no other way... Is Debian up to
date with 1.1.0 beta2 in its snapshot packages?

Debian testing is still on 1.0.2h.
Debian experimental is on 1.1.0pre5.

Not sure here beta2 enters the discussion, it's not mentioned anywhere on
their site?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Fri, Jul 1, 2016 at 5:02 PM, Magnus Hagander <magnus@hagander.net> wrote:

Debian testing is still on 1.0.2h.
Debian experimental is on 1.1.0pre5.

Not sure here beta2 enters the discussion, it's not mentioned anywhere on
their site?

Thanks. From the main page of openssl.org, pre5 is beta2.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Jul 1, 2016 at 10:10 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Fri, Jul 1, 2016 at 5:02 PM, Magnus Hagander <magnus@hagander.net>
wrote:

Debian testing is still on 1.0.2h.
Debian experimental is on 1.1.0pre5.

Not sure here beta2 enters the discussion, it's not mentioned anywhere on
their site?

Thanks. From the main page of openssl.org, pre5 is beta2.

Hah. And it's not mentioned on their download page. I see they continue
down their path of confusing version numbering.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Re: Andreas Karlsson 2016-07-01 <688a438c-ccc2-0431-7100-26e418fc3bca@proxel.se>

Hi,

Here is an initial set of patches related to OpenSSL 1.1. Everything should
still build fine on older OpenSSL versions (and did when I tested with
1.0.2h).

Hi Andreas,

thanks for the patches. I applied all there patches on top of HEAD
(10c0558f). The server builds and passes "make check", pgcrypto still
needs work, though:

./configure --with-openssl
make world

gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o openssl.o openssl.c
openssl.c:205:13: error: field ‘ctx’ has incomplete type
EVP_MD_CTX ctx;
^
openssl.c: In function ‘digest_free’:
openssl.c:253:2: warning: implicit declaration of function ‘EVP_MD_CTX_cleanup’ [-Wimplicit-function-declaration]
EVP_MD_CTX_cleanup(&digest->ctx);
^
openssl.c: In function ‘init_openssl_rand’:
openssl.c:990:24: warning: implicit declaration of function ‘RAND_SSLeay’ [-Wimplicit-function-declaration]
RAND_set_rand_method(RAND_SSLeay());
^
openssl.c:990:24: warning: passing argument 1 of ‘RAND_set_rand_method’ makes pointer from integer without a cast [-Wint-conversion]
In file included from openssl.c:40:0:
/usr/include/openssl/rand.h:41:5: note: expected ‘const RAND_METHOD * {aka const struct rand_meth_st *}’ but argument is of type ‘int’
int RAND_set_rand_method(const RAND_METHOD *meth);
^
openssl.c: In function ‘px_get_pseudo_random_bytes’:
openssl.c:1017:2: warning: ‘RAND_pseudo_bytes’ is deprecated [-Wdeprecated-declarations]
res = RAND_pseudo_bytes(dst, count);
^
In file included from openssl.c:40:0:
/usr/include/openssl/rand.h:51:5: note: declared here
DEPRECATEDIN_1_1_0(int RAND_pseudo_bytes(unsigned char *buf, int num))
^
openssl.c: In function ‘digest_block_size’:
openssl.c:222:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
openssl.c: In function ‘digest_result_size’:
openssl.c:214:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
<eingebaut>: die Regel für Ziel „openssl.o“ scheiterte
make[2]: *** [openssl.o] Fehler 1
make[2]: Verzeichnis „/home/cbe/projects/postgresql/pg/master/contrib/pgcrypto“ wird verlassen

ii libssl-dev:amd64 1.1.0~pre5-4 amd64 Secure Sockets Layer toolkit - development files
ii libssl1.0.0:amd64 1.0.2d-1 amd64 Secure Sockets Layer toolkit - shared libraries
ii libssl1.0.2:amd64 1.0.2h-1 amd64 Secure Sockets Layer toolkit - shared libraries
ii libssl1.1:amd64 1.1.0~pre5-4 amd64 Secure Sockets Layer toolkit - shared libraries
ii openssl 1.0.2h-1 amd64 Secure Sockets Layer toolkit - cryptographic utility

Christoph

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 07/01/2016 11:41 AM, Christoph Berg wrote:

thanks for the patches. I applied all there patches on top of HEAD
(10c0558f). The server builds and passes "make check", pgcrypto still
needs work, though:

Thanks, I had forgotten pgcrypto.

When fixing pgcrypto I noticed that the OpenSSL team has deprecated
RAND_pseudo_bytes() and recommend using RAND_bytes() instead (see
302d38e3f73d5fd2ba2fd30bb7798778cb9f18dd).

As far as I can tell the only difference is that RAND_bytes() adds an
error to the error queue if there is not enough entropy for generating
secure data. And since we already always use strong random with the
Fortuna algorithm, why not just drop px_get_pseudo_random_bytes()? It
feels like a potential security problem with to me unclear benefit.

I also found that client CA loading is broken in OpenSSL 1.1-pre5
(reported as https://github.com/openssl/openssl/pull/1279). This might
be good to be aware of when testing my patches.

Attached a new set of patches:

0001-Fixes-for-compiling-with-OpenSSL-1.1-v2.patch

The fixes necessary to build with OpenSSL 1.1. Mostly fixes surrounding
direct access to struct fields.

0002-Remove-OpenSSL-1.1-deprecation-warnings-v2.patch

Fix deprecation warnings. Mostly trusting OpenSSL 1.1 to handle
threading and initialization automatically.

0003-Remove-px_get_pseudo_random_bytes-v2.patch

Remove the px_get_pseudo_random_bytes() from pgcrypto. Also silcences
deprecation warning about RAND_pseudo_bytes().

0004-Define-CRYPTO_LOCK-for-OpenSSL-1.1-compat-v2.patch

Useful if you want to play around with
0001-Fixes-for-compiling-with-OpenSSL-1.1-v2.patch before they release a
new version where CRYPTO_LOCK is added back. See
https://github.com/openssl/openssl/issues/1260

Andreas

Thanks for this effort.

static BIO_METHOD *
my_BIO_s_socket(void)
{
-	if (!my_bio_initialized)
+	if (!my_bio_methods)
{
-		memcpy(&my_bio_methods, BIO_s_socket(), sizeof(BIO_METHOD));
-		my_bio_methods.bread = my_sock_read;
-		my_bio_methods.bwrite = my_sock_write;
-		my_bio_initialized = true;
+		BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
+#if SSLEAY_VERSION_NUMBER >= 0x10100000L
+		my_bio_methods = BIO_meth_new(BIO_TYPE_SOCKET, "pgsocket");
+		BIO_meth_set_write(my_bio_methods, my_sock_write);
+		BIO_meth_set_read(my_bio_methods, my_sock_read);
+		BIO_meth_set_gets(my_bio_methods, BIO_meth_get_gets(biom));
+		BIO_meth_set_ctrl(my_bio_methods, BIO_meth_get_ctrl(biom));
+		BIO_meth_set_create(my_bio_methods, BIO_meth_get_create(biom));
+		BIO_meth_set_destroy(my_bio_methods, BIO_meth_get_destroy(biom));
+		BIO_meth_set_callback_ctrl(my_bio_methods, BIO_meth_get_callback_ctrl(biom));
+#else
+		my_bio_methods = malloc(sizeof(BIO_METHOD));
+		memcpy(my_bio_methods, biom, sizeof(BIO_METHOD));
+		my_bio_methods->bread = my_sock_read;
+		my_bio_methods->bwrite = my_sock_write;
+#endif

Generally, version number tests sprinkled all over the place are not
terribly nice. I think it would be better to get configure to define a
symbol like HAVE_BIO_METH_NEW. Not sure about the other hunks in this
patch; perhaps HAVE_BIO_SET_DATA, and #define both those macros if not.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 07/02/2016 02:28 AM, Alvaro Herrera wrote:

static BIO_METHOD *
my_BIO_s_socket(void)
{
-	if (!my_bio_initialized)
+	if (!my_bio_methods)
{
-		memcpy(&my_bio_methods, BIO_s_socket(), sizeof(BIO_METHOD));
-		my_bio_methods.bread = my_sock_read;
-		my_bio_methods.bwrite = my_sock_write;
-		my_bio_initialized = true;
+		BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
+#if SSLEAY_VERSION_NUMBER >= 0x10100000L
+		my_bio_methods = BIO_meth_new(BIO_TYPE_SOCKET, "pgsocket");
+		BIO_meth_set_write(my_bio_methods, my_sock_write);
+		BIO_meth_set_read(my_bio_methods, my_sock_read);
+		BIO_meth_set_gets(my_bio_methods, BIO_meth_get_gets(biom));
+		BIO_meth_set_ctrl(my_bio_methods, BIO_meth_get_ctrl(biom));
+		BIO_meth_set_create(my_bio_methods, BIO_meth_get_create(biom));
+		BIO_meth_set_destroy(my_bio_methods, BIO_meth_get_destroy(biom));
+		BIO_meth_set_callback_ctrl(my_bio_methods, BIO_meth_get_callback_ctrl(biom));
+#else
+		my_bio_methods = malloc(sizeof(BIO_METHOD));
+		memcpy(my_bio_methods, biom, sizeof(BIO_METHOD));
+		my_bio_methods->bread = my_sock_read;
+		my_bio_methods->bwrite = my_sock_write;
+#endif

Generally, version number tests sprinkled all over the place are not
terribly nice. I think it would be better to get configure to define a
symbol like HAVE_BIO_METH_NEW. Not sure about the other hunks in this
patch; perhaps HAVE_BIO_SET_DATA, and #define both those macros if not.

Agreed, that it is not nice. I followed what the previous code did, but
I do not like the inflation of this kind of #ifs with my OpenSSL 1.1
patches. I will try to see if I can figure out some good symbols.

Essentially the API changes which require ifdefs are:

- Opaque struts (we see an example above with the BIO struct)
- Renaming of RAND_SSLeay()
- Deprecation of DH_generate_parameters()
- Automatic initialization
- Automatic handling of threading

I do not like the idea of having a define per struct they have made
opaque in 1.1, but I think one define for all structs could be fine
(something like HAVE_OPENSSL_OPAQUE_STRUCTS). What do you think?

Andreas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 07/02/2016 02:45 AM, Andreas Karlsson wrote:

On 07/02/2016 02:28 AM, Alvaro Herrera wrote:

Generally, version number tests sprinkled all over the place are not
terribly nice. I think it would be better to get configure to define a
symbol like HAVE_BIO_METH_NEW. Not sure about the other hunks in this
patch; perhaps HAVE_BIO_SET_DATA, and #define both those macros if not.

Agreed, that it is not nice. I followed what the previous code did, but
I do not like the inflation of this kind of #ifs with my OpenSSL 1.1
patches. I will try to see if I can figure out some good symbols.

Essentially the API changes which require ifdefs are:

- Opaque struts (we see an example above with the BIO struct)
- Renaming of RAND_SSLeay()
- Deprecation of DH_generate_parameters()
- Automatic initialization
- Automatic handling of threading

I do not like the idea of having a define per struct they have made
opaque in 1.1, but I think one define for all structs could be fine
(something like HAVE_OPENSSL_OPAQUE_STRUCTS). What do you think?

Looking at my code again I noticed it is just the BIO and BIO_METHOD
structs which needed #ifs. The rest could be handled with changing the
code to work in both old and new versions. If it is just two structs it
might be fine to have two symbols, hm ..

Andreas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: Andreas Karlsson 2016-07-02 <a5f4b79e-a9ea-200d-e17e-2da3ad187e5b@proxel.se>

On 07/01/2016 11:41 AM, Christoph Berg wrote:

thanks for the patches. I applied all there patches on top of HEAD
(10c0558f). The server builds and passes "make check", pgcrypto still
needs work, though:

Thanks, I had forgotten pgcrypto.

pgcrypto works now as well, thanks!

Re: Alvaro Herrera 2016-07-02 <20160702002846.GA376611@alvherre.pgsql>

Generally, version number tests sprinkled all over the place are not
terribly nice. I think it would be better to get configure to define a
symbol like HAVE_BIO_METH_NEW. Not sure about the other hunks in this
patch; perhaps HAVE_BIO_SET_DATA, and #define both those macros if not.

Otoh these symbols are strictly version-dependant on OpenSSL, it's not
like one of the symbols would appear or disappear for other reasons
(like different TLS implementation, or different operating system).

Once we decide (in 10 years?) that the minimum supported OpenSSL
version is >= 1.1, we can just drop the version checks. If these are
converted to feature tests now, it will be much harder to remember at
which point they can be dropped.

Christoph

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, 1 Jul 2016 02:27:03 +0200
Andreas Karlsson <andreas@proxel.se> wrote:

0003-Remove-OpenSSL-1.1-deprecation-warnings.patch

Silence all warnings. This commit changes more things and is not
necessary for getting PostgreSQL to build against 1.1.

This patch breaks feature, which exists in PostgreSQL since 8.2 -
support for SSL ciphers, provided by loadable modules such as Russian
national standard (GOST) algorithms, and support for cryptographic
hardware tokens (which are also supported by loadble modules called
engines in OpenSSL).

Call for OPENSSL_config was added to PostgreSQL for this purpose - it
loads default OpenSSL configuration file, where such things as crypto
hardware modules can be configured.

If we wish to keep this functionality, we need to explicitely call

OPENSSL_init_ssl(INIT_LOAD_CONFIG,NULL) instead of deprecated
OPENSSL_config in 1.1.0

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 07/05/2016 11:13 AM, Victor Wagner wrote:

On Fri, 1 Jul 2016 02:27:03 +0200
Andreas Karlsson <andreas@proxel.se> wrote:

0003-Remove-OpenSSL-1.1-deprecation-warnings.patch

Silence all warnings. This commit changes more things and is not
necessary for getting PostgreSQL to build against 1.1.

This patch breaks feature, which exists in PostgreSQL since 8.2 -
support for SSL ciphers, provided by loadable modules such as Russian
national standard (GOST) algorithms, and support for cryptographic
hardware tokens (which are also supported by loadble modules called
engines in OpenSSL).

Call for OPENSSL_config was added to PostgreSQL for this purpose - it
loads default OpenSSL configuration file, where such things as crypto
hardware modules can be configured.

If we wish to keep this functionality, we need to explicitely call

OPENSSL_init_ssl(INIT_LOAD_CONFIG,NULL) instead of deprecated
OPENSSL_config in 1.1.0

Thanks for testing the patches!

I have attached a new set of patches which this is fixed. I have also
skipped the last patch since OpenSSL has fixed the two issues I have
mentioned earlier in this thread.

Andreas

On 07/05/2016 04:46 PM, Andreas Karlsson wrote:

@@ -280,8 +287,9 @@ px_find_digest(const char *name, PX_MD **res)
digest = px_alloc(sizeof(*digest));
digest->algo = md;

-	EVP_MD_CTX_init(&digest->ctx);
-	if (EVP_DigestInit_ex(&digest->ctx, digest->algo, NULL) == 0)
+	digest->ctx = EVP_MD_CTX_create();
+	EVP_MD_CTX_init(digest->ctx);
+	if (EVP_DigestInit_ex(digest->ctx, digest->algo, NULL) == 0)
return -1;

h = px_alloc(sizeof(*h));

Now that we're calling EVP_MD_CTX_create((), which allocates memory, are
we risking memory leaks? It has always been part of the contract that
you have to call px_md_free(), for any context returned by
px_find_digest(), but I wonder just how careful we have been about that.
Before this, you would probably get away with it without leaking, if the
digest implementation didn't allocate any extra memory or other resources.

At least pg_digest and try_unix_std functions call px_find_digest(), and
then do more palloc()s which could elog() if you run out of memory,
leaking th digest struct. Highly unlikely, but I think it would be
fairly straightforward to reorder those calls to eliminate the risk, so
we probably should.

@@ -854,6 +858,25 @@ load_dh_buffer(const char *buffer, size_t len)
return dh;
}

+static DH  *
+generate_dh_params(int prime_len, int generator)
+{
+#if SSLEAY_VERSION_NUMBER >= 0x00908000L
+	DH *dh;
+
+	if ((dh = DH_new()) == NULL)
+		return NULL;
+
+	if (DH_generate_parameters_ex(dh, prime_len, generator, NULL))
+		return dh;
+
+	DH_free(dh);
+	return NULL;
+#else
+	return DH_generate_parameters(prime_len, generator, NULL, NULL);
+#endif
+}
+

I think now would be a good time to drop support for OpenSSL versions
older than 0.9.8. OpenSSL don't even support 0.9.8 anymore, although
there are probably distributions out there that still provide patches
for it. But OpenSSL 0.9.7 and older are really not interesting for
PostgreSQL 10 anymore, I think.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers