Thread: OpenSSL 1.1 breaks configure and more
Hi, as reported by Debian's OpenSSL maintainers, PostgreSQL is failing to build against a snapshot of the upcoming 1.1.0 version. The report was for 9.5.3, but I can reproduce it in HEAD as well: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=828510 > OpenSSL 1.1.0 is about to released. During a rebuild of all packages using > OpenSSL this package fail to build. A log of that build can be found at: > https://breakpoint.cc/openssl-1.1-rebuild-2016-05-29/Attempted/postgresql-9.5_9.5.3-1_amd64-20160529-1510 > > On https://wiki.openssl.org/index.php/1.1_API_Changes you can see various of the > reasons why it might fail. There are also updated man pages at > https://www.openssl.org/docs/manmaster/ that should contain useful information. > > There is a libssl-dev package available in experimental that contains a recent > snapshot, I suggest you try building against that to see if everything works. $ ./configure --with-openssl checking for CRYPTO_new_ex_data in -lcrypto... yes checking for SSL_library_init in -lssl... no configure: error: library 'ssl' is required for OpenSSL I can get one step further by tweaking configure.in and running autoreconf, but then compilation fails further down: - AC_CHECK_LIB(ssl, SSL_library_init, [], [AC_MSG_ERROR([library 'ssl' is required for OpenSSL])]) + AC_CHECK_LIB([ssl], [SSL_library_init]) make -C common all make[4]: Verzeichnis „/home/cbe/projects/postgresql/pg/master/src/backend/access/common“ wird betreten gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security-fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 -I../../../../src/include -D_GNU_SOURCE -c-o printtup.o printtup.c In file included from ../../../../src/include/libpq/libpq-be.h:25:0, from ../../../../src/include/libpq/libpq.h:21, from printtup.c:19: /usr/include/openssl/ssl.h:1740:26: error: expected identifier or ‘(’ before numeric constant__owur const COMP_METHOD *SSL_get_current_compression(SSL*s); ^ Christoph
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
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
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
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
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
Attachment
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
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?
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
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.
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
isof 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 hereDEPRECATEDIN_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
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
Attachment
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
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
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
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
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
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
Attachment
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
On 8/26/16 5:31 AM, Heikki Linnakangas wrote: > 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. CentOS 5 currently ships 0.9.8e. That's usually the oldest OS we want to support eagerly. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 8/26/16 5:31 AM, Heikki Linnakangas wrote:
>> 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.
> CentOS 5 currently ships 0.9.8e. That's usually the oldest OS we want
> to support eagerly.
Also, I get this on fully-up-to-date OS X (El Capitan):
$ openssl version
OpenSSL 0.9.8zh 14 Jan 2016
Worth noting though is that without -Wno-deprecated-declarations, you
find that Apple has sprinkled the entire OpenSSL API with deprecation
warnings. That suggests that their plan for the future is to drop it
rather than update it. Should we be thinking ahead to that?
regards, tom lane
PS: I still have 0.9.7 on some of my buildfarm critters. But I could
either update them, or stop using --with-openssl there.
On 08/26/2016 07:44 PM, Tom Lane wrote: > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >> On 8/26/16 5:31 AM, Heikki Linnakangas wrote: >>> 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. > >> CentOS 5 currently ships 0.9.8e. That's usually the oldest OS we want >> to support eagerly. > > Also, I get this on fully-up-to-date OS X (El Capitan): > > $ openssl version > OpenSSL 0.9.8zh 14 Jan 2016 Ok, sold, let's remove support for OpenSSL < 0.9.8. > Worth noting though is that without -Wno-deprecated-declarations, you > find that Apple has sprinkled the entire OpenSSL API with deprecation > warnings. That suggests that their plan for the future is to drop it > rather than update it. Should we be thinking ahead to that? Yeah, they want people to move to their own SSL library [1]. I doubt they will actually remove it any time soon, but who knows. It would be a good project for someone with an OS X system and some spare time, to write a patch to build with OS X's native SSL library instead of OpenSSL. The code is structured nicely to enable that now. [1] I couldn't find any official statement, but lots of blog posts saying the same thing. - Heikki
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> Yeah, they want people to move to their own SSL library [1].
> [1] I couldn't find any official statement, but lots of blog posts
> saying the same thing.
As I recall, the deprecation warning messages said that in so many words.
That probably counts as an official statement ...
regards, tom lane
On 08/26/2016 07:04 PM, Heikki Linnakangas wrote: > On 08/26/2016 07:44 PM, Tom Lane wrote: >> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >>> On 8/26/16 5:31 AM, Heikki Linnakangas wrote: >>>> 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. >> >>> CentOS 5 currently ships 0.9.8e. That's usually the oldest OS we want >>> to support eagerly. >> >> Also, I get this on fully-up-to-date OS X (El Capitan): >> >> $ openssl version >> OpenSSL 0.9.8zh 14 Jan 2016 > > Ok, sold, let's remove support for OpenSSL < 0.9.8. I have attached a patch which removes the < 0.9.8 compatibility code. Should we also add a version check to configure? We do not have any such check currently. Andreas
Attachment
On Sat, Aug 27, 2016 at 2:04 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote: > On 08/26/2016 07:44 PM, Tom Lane wrote: >> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >> Also, I get this on fully-up-to-date OS X (El Capitan): >> >> $ openssl version >> OpenSSL 0.9.8zh 14 Jan 2016 > > > Ok, sold, let's remove support for OpenSSL < 0.9.8. Yes I think it's a wiser plan to not brush up newer versions than that. >> Worth noting though is that without -Wno-deprecated-declarations, you >> find that Apple has sprinkled the entire OpenSSL API with deprecation >> warnings. That suggests that their plan for the future is to drop it >> rather than update it. Should we be thinking ahead to that? > > > Yeah, they want people to move to their own SSL library [1]. I doubt they > will actually remove it any time soon, but who knows. It would be a good > project for someone with an OS X system and some spare time, to write a > patch to build with OS X's native SSL library instead of OpenSSL. The code > is structured nicely to enable that now. > > [1] I couldn't find any official statement, but lots of blog posts saying > the same thing. As well on El Capitan: $ ssh -V OpenSSH_6.9p1, LibreSSL 2.1.8 So could it be possible that it would be a switch from openssl to libressl instead? -- Michael
On 8/26/16 9:26 PM, Andreas Karlsson wrote: > I have attached a patch which removes the < 0.9.8 compatibility code. > Should we also add a version check to configure? We do not have any such > check currently. I think that is not necessary. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 08/27/2016 05:15 PM, Peter Eisentraut wrote: > On 8/26/16 9:26 PM, Andreas Karlsson wrote: >> I have attached a patch which removes the < 0.9.8 compatibility code. >> Should we also add a version check to configure? We do not have any such >> check currently. > > I think that is not necessary. I was going to change the configure test to check for a different function that we use, that's only present in 0.9.8 and later. But the only such functions were related to ECDH, and the use of those functions is inside "#ifndef OPENSSL_NO_ECDH", so they're not suitable for the autoconf test. So I gave up. If you try to build with 0.9.7, you'll get compilation errors because of those ECDH symbols, and with 0.9.6, probably on some other symbols. Pushed with some small doc fixes, thanks Andreas! I'll continue reviewing the rest of the patches. - Heikki
On 08/29/2016 08:22 PM, Heikki Linnakangas wrote: > On 08/27/2016 05:15 PM, Peter Eisentraut wrote: >> On 8/26/16 9:26 PM, Andreas Karlsson wrote: >>> I have attached a patch which removes the < 0.9.8 compatibility code. >>> Should we also add a version check to configure? We do not have any such >>> check currently. >> >> I think that is not necessary. > > I was going to change the configure test to check for a different > function that we use, that's only present in 0.9.8 and later. But the > only such functions were related to ECDH, and the use of those functions > is inside "#ifndef OPENSSL_NO_ECDH", so they're not suitable for the > autoconf test. So I gave up. If you try to build with 0.9.7, you'll get > compilation errors because of those ECDH symbols, and with 0.9.6, > probably on some other symbols. > > Pushed with some small doc fixes, thanks Andreas! I'll continue > reviewing the rest of the patches. Buildfarm animals "locust" and "prairiedog" are not happy with this. They seem to be using OpenSSL 0.9.7, as they failed with errors related to those ECDH calls: be-secure-openssl.c: In function 'initialize_ecdh': be-secure-openssl.c:978: error: 'EC_KEY' undeclared (first use in this function) be-secure-openssl.c:978: error: (Each undeclared identifier is reported only once be-secure-openssl.c:978: error: for each function it appears in.) be-secure-openssl.c:978: error: 'ecdh' undeclared (first use in this function) be-secure-openssl.c:979: warning: ISO C90 forbids mixed declarations and code be-secure-openssl.c:986: warning: implicit declaration of function 'EC_KEY_new_by_curve_name' be-secure-openssl.c:991: error: 'SSL_OP_SINGLE_ECDH_USE' undeclared (first use in this function) be-secure-openssl.c:992: warning: implicit declaration of function 'SSL_CTX_set_tmp_ecdh' be-secure-openssl.c:993: warning: implicit declaration of function 'EC_KEY_free' I only now noticed that Tom said upthread that he still has a buildfarm critter using 0.9.7 (that's prairiedog). Sorry for the breakage. It would be easy to put the version check back to still support 0.9.7, most of the changes in this commit was thanks to removing support for 0.9.6. But that'd complicate the upcoming 1.1.0 support patch slightly, so let's stick to the plan and drop the support for <= 0.9.7 Tom, Rémi, can you fix locust and prairiedog, please, by updating OpenSSL or removing --with-openssl? - Heikki
Heikki Linnakangas <hlinnaka@iki.fi> writes:
> Buildfarm animals "locust" and "prairiedog" are not happy with this.
> They seem to be using OpenSSL 0.9.7, as they failed with errors related
> to those ECDH calls:
prairiedog definitely is, and since locust is also an ancient OS X
version, that's not too surprising also. (I imagine gaur/pademelon
will fall over too, next time I turn it on.)
> Tom, R�mi, can you fix locust and prairiedog, please, by updating
> OpenSSL or removing --with-openssl?
Roger, will do (probably just the latter for today).
regards, tom lane
> Le 29 août 2016 à 19:46, Heikki Linnakangas <hlinnaka@iki.fi> a écrit : > > > Tom, Rémi, can you fix locust and prairiedog, please, by updating OpenSSL or removing --with-openssl? > Hi, Should be OK for locust on next build. Rémi
On 08/29/2016 07:22 PM, Heikki Linnakangas wrote: > Pushed with some small doc fixes, thanks Andreas! I'll continue > reviewing the rest of the patches. Thanks! Andreas
On 08/26/2016 11:31 AM, Heikki Linnakangas wrote: > 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. Since px_find_digest() calls palloc() later in the function there is a slim possibility of memory leaks. How do we generally handle that things not allocated with palloc() may leak when something calls elog()? I have attached new versions of the patches which are rebased on master, with slightly improves error handling in px_find_digest(), and handles the deprecation of ASN1_STRING_data(). Andreas
Attachment
On 08/30/2016 03:26 AM, Andreas Karlsson wrote: > On 08/26/2016 11:31 AM, Heikki Linnakangas wrote: >> 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. > > Since px_find_digest() calls palloc() later in the function there is a > slim possibility of memory leaks. Yep. That palloc() would be easy to move to before the EVP_MD_CTX_new() call. And some of the calls to px_find_digest() could likewise be rearranged. But there are some more complicated callers. pgp_encrypt(), for example, builds a pipeline of multiple "mbuf" filters, and one of those filters uses px_find_digest(). > How do we generally handle that things > not allocated with palloc() may leak when something calls elog()? There's the ResourceOwner mechanism, see src/backend/utils/resowner/. That would be the proper way to do this. Call RegisterResourceReleaseCallback() when the context is allocated, and have the callback free it. One pitfall to watch out for is that RegisterResourceReleaseCallback() itself calls palloc(), and can error out, so you have to do things in such an order that you don't leak in that case either. Want to take a stab at that? Another approach is put each allocated context in a list or array in a global variable, and to register a callback to be called at end-of-(sub)transaction, which closes all the contexts. But the resource owner mechanism is probably easier. There's also PG_TRY-CATCH, that you could maybe use in the callers of px_find_digest(), to make sure they call px_free_digest() even on error. But that also seems difficult to use with the pgp_encrypt() pipeline. > I have attached new versions of the patches which are rebased on master, > with slightly improves error handling in px_find_digest(), and handles > the deprecation of ASN1_STRING_data(). Thanks! PS. I just remembered that I've wanted to refactor the pgcrypto calls for symmetric encryption to use the newer EVP API for some time, and even posted a patch for that (https://www.postgresql.org/message-id/561274F1.1030000@iki.fi). I dropped the ball back then, but I think I'll go ahead and do that now, once we get these other OpenSSL changes in. - Heikki
On 08/30/2016 08:42 AM, Heikki Linnakangas wrote: > There's the ResourceOwner mechanism, see src/backend/utils/resowner/. > That would be the proper way to do this. Call > RegisterResourceReleaseCallback() when the context is allocated, and > have the callback free it. One pitfall to watch out for is that > RegisterResourceReleaseCallback() itself calls palloc(), and can error > out, so you have to do things in such an order that you don't leak in > that case either. > > Want to take a stab at that? > > Another approach is put each allocated context in a list or array in a > global variable, and to register a callback to be called at > end-of-(sub)transaction, which closes all the contexts. But the resource > owner mechanism is probably easier. > > There's also PG_TRY-CATCH, that you could maybe use in the callers of > px_find_digest(), to make sure they call px_free_digest() even on error. > But that also seems difficult to use with the pgp_encrypt() pipeline. Sure, I have attached a patch where I try to use it. > PS. I just remembered that I've wanted to refactor the pgcrypto calls > for symmetric encryption to use the newer EVP API for some time, and > even posted a patch for that > (https://www.postgresql.org/message-id/561274F1.1030000@iki.fi). I > dropped the ball back then, but I think I'll go ahead and do that now, > once we get these other OpenSSL changes in. Nice! Andreas
Attachment
Andreas Karlsson <andreas@proxel.se> writes: > On 08/30/2016 08:42 AM, Heikki Linnakangas wrote: >> PS. I just remembered that I've wanted to refactor the pgcrypto calls >> for symmetric encryption to use the newer EVP API for some time, and >> even posted a patch for that >> (https://www.postgresql.org/message-id/561274F1.1030000@iki.fi). I >> dropped the ball back then, but I think I'll go ahead and do that now, >> once we get these other OpenSSL changes in. > Nice! Judging by the number of people who have popped up recently with their own OpenSSL 1.1 patches, I think there is going to be a lot of demand for back-patching some sort of 1.1 support into our back branches. All this talk of refactoring does not sound very back-patchable. Should we be thinking of what we can extract that is back-patchable? regards, tom lane
On 09/05/2016 02:23 AM, Tom Lane wrote: > Judging by the number of people who have popped up recently with their > own OpenSSL 1.1 patches, I think there is going to be a lot of demand for > back-patching some sort of 1.1 support into our back branches. All this > talk of refactoring does not sound very back-patchable. Should we be > thinking of what we can extract that is back-patchable? My idea is that the first of my four patches contains the minimum changes needed to add support for 1.1 and tries to do as little refactoring as possible while the other patches refactor things. I am not sure about if anything of the other patches should be backpatched. Andreas
On Mon, Sep 5, 2016 at 9:32 AM, Andreas Karlsson <andreas@proxel.se> wrote: > On 09/05/2016 02:23 AM, Tom Lane wrote: >> >> Judging by the number of people who have popped up recently with their >> own OpenSSL 1.1 patches, I think there is going to be a lot of demand for >> back-patching some sort of 1.1 support into our back branches. All this >> talk of refactoring does not sound very back-patchable. Should we be >> thinking of what we can extract that is back-patchable? > > My idea is that the first of my four patches contains the minimum changes > needed to add support for 1.1 and tries to do as little refactoring as > possible while the other patches refactor things. I am not sure about if > anything of the other patches should be backpatched. From what I can see of the 4 patches proposed, those are not that much invasive, so a backpatch of those is really doable. But yes let's keep the refactoring only for HEAD. That's definitely the safest approach. -- Michael
On 09/05/2016 03:12 AM, Andreas Karlsson wrote: > On 08/30/2016 08:42 AM, Heikki Linnakangas wrote: >> There's the ResourceOwner mechanism, see src/backend/utils/resowner/. >> That would be the proper way to do this. Call >> RegisterResourceReleaseCallback() when the context is allocated, and >> have the callback free it. One pitfall to watch out for is that >> RegisterResourceReleaseCallback() itself calls palloc(), and can error >> out, so you have to do things in such an order that you don't leak in >> that case either. >> >> Want to take a stab at that? >> >> Another approach is put each allocated context in a list or array in a >> global variable, and to register a callback to be called at >> end-of-(sub)transaction, which closes all the contexts. But the resource >> owner mechanism is probably easier. >> >> There's also PG_TRY-CATCH, that you could maybe use in the callers of >> px_find_digest(), to make sure they call px_free_digest() even on error. >> But that also seems difficult to use with the pgp_encrypt() pipeline. > > Sure, I have attached a patch where I try to use it. Thanks! Unfortunately the callback mechanism is a bit more complicated to use than that. The list of registered callbacks is global, so the callback gets called for every ResourceOwner that's closed, not just the one that was active when you registered it. Also, unregistering the callback from within the callback is not safe. I fixed those things in the (first) attached patch. On 09/05/2016 03:23 AM, Tom Lane wrote: > Judging by the number of people who have popped up recently with their > own OpenSSL 1.1 patches, I think there is going to be a lot of demand for > back-patching some sort of 1.1 support into our back branches. All this > talk of refactoring does not sound very back-patchable. Should we be > thinking of what we can extract that is back-patchable? Yes, I think you're right. The minimum set of changes is the first of the attached patches. It includes Andreas' first patch, with the configure changes and other changes needed to compile, and a working version of the resourceowner callback mechanism to make sure we don't leak OpenSSL handles in pgcrypto. Maybe we could get away without the resourceowner mechanism, and just accept the minor memory leaks on the rare error case (out-of-memory might be the only situation where that happen). Or #ifdef it so that we keep the old embed-in-palloced-struct approach for OpenSSL versions older than 1.1. But on the whole, I'd prefer to keep the code the same in all branches. The second patch attached here includes Andreas' second and third patches, to silence deprecation warnings. That's not strictly required, but seems safe enough that I think we should back-patch that too. It needs one additional #ifdef version check in generate_dh_params(), if we want it to still work with OpenSSL 0.9.7, but that's easy. I'm assuming we want to still support it in back-branches, even though we just dropped it from master. - Heikki
Attachment
On 09/05/2016 02:52 PM, Heikki Linnakangas wrote: > On 09/05/2016 03:23 AM, Tom Lane wrote: >> Judging by the number of people who have popped up recently with their >> own OpenSSL 1.1 patches, I think there is going to be a lot of demand for >> back-patching some sort of 1.1 support into our back branches. All this >> talk of refactoring does not sound very back-patchable. Should we be >> thinking of what we can extract that is back-patchable? > > Yes, I think you're right. I planned to commit this today, but while reading through it and testing, I ended up doing a bunch more changes, so this deserves another round of review. Changes since last version: * Added more error checks to the my_BIO_s_socket() function. Check for NULL result from malloc(). Check the return code of BIO_meth_set_*() functions; looking at OpenSSL sources, they always succeed, but all the test/example programs that come with OpenSSL do check them. * Use BIO_get_new_index() to get the index number for the wrapper BIO. * Also call BIO_meth_set_puts(). It was missing in previous patch versions. * Fixed src/test/ssl test suite to also work with OpenSSL 1.1.0. * Changed all references (in existing code) to SSLEAY_VERSION_NUMBER into OPENSSL_VERSION_NUMBER, for consistency. * Squashed all into one patch. I intend to apply this to all supported branches, so please have a look! This is now against REL9_6_STABLE, but there should be little difference between branches in the code that this touches. - Heikki
Attachment
On Tue, Sep 13, 2016 at 1:51 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote: > I planned to commit this today, but while reading through it and testing, I > ended up doing a bunch more changes, so this deserves another round of > review. OK, I am giving it a try. Note to people using OSX: at least for brew there is the formula openssl@1.1 that you can use with the following flags: CFLAGS="-I/usr/local/opt/openssl@1.1/include" LDFLAGS="-L/usr/local/opt/openssl@1.1/lib" Postgres is not the only broken thing, so they kept the formula openssl to 1.0.2. > Changes since last version: > > * Added more error checks to the my_BIO_s_socket() function. Check for NULL > result from malloc(). Check the return code of BIO_meth_set_*() functions; > looking at OpenSSL sources, they always succeed, but all the test/example > programs that come with OpenSSL do check them. > > * Use BIO_get_new_index() to get the index number for the wrapper BIO. > > * Also call BIO_meth_set_puts(). It was missing in previous patch versions. > > * Fixed src/test/ssl test suite to also work with OpenSSL 1.1.0. > > * Changed all references (in existing code) to SSLEAY_VERSION_NUMBER into > OPENSSL_VERSION_NUMBER, for consistency. > > * Squashed all into one patch. > > I intend to apply this to all supported branches, so please have a look! > This is now against REL9_6_STABLE, but there should be little difference > between branches in the code that this touches. I just took a look at this patch, testing it on the way with 1.1.0 and 1.0.2. And it looks in pretty good shape. + ResourceOwner owner; + struct OSSLDigest *next; + struct OSSLDigest *prev; This could be done as well with a list of pg_list, the cost being a couple of extra calls to switch memory contexts, but it would simplify free_openssldigest when cleaning up an entry. I guessed you already thought about that but discarded it? -- Michael
On 09/12/2016 06:51 PM, Heikki Linnakangas wrote: > Changes since last version: > > * Added more error checks to the my_BIO_s_socket() function. Check for > NULL result from malloc(). Check the return code of BIO_meth_set_*() > functions; looking at OpenSSL sources, they always succeed, but all the > test/example programs that come with OpenSSL do check them. > > * Use BIO_get_new_index() to get the index number for the wrapper BIO. > > * Also call BIO_meth_set_puts(). It was missing in previous patch versions. > > * Fixed src/test/ssl test suite to also work with OpenSSL 1.1.0. > > * Changed all references (in existing code) to SSLEAY_VERSION_NUMBER > into OPENSSL_VERSION_NUMBER, for consistency. > > * Squashed all into one patch. > > I intend to apply this to all supported branches, so please have a look! > This is now against REL9_6_STABLE, but there should be little difference > between branches in the code that this touches. This patch no longer seems to apply to head after the removed support of 0.9.6. Is that intentional? Andreas
On 09/15/2016 02:03 AM, Andreas Karlsson wrote: > On 09/12/2016 06:51 PM, Heikki Linnakangas wrote: >> Changes since last version: >> >> * Added more error checks to the my_BIO_s_socket() function. Check for >> NULL result from malloc(). Check the return code of BIO_meth_set_*() >> functions; looking at OpenSSL sources, they always succeed, but all the >> test/example programs that come with OpenSSL do check them. >> >> * Use BIO_get_new_index() to get the index number for the wrapper BIO. >> >> * Also call BIO_meth_set_puts(). It was missing in previous patch >> versions. >> >> * Fixed src/test/ssl test suite to also work with OpenSSL 1.1.0. >> >> * Changed all references (in existing code) to SSLEAY_VERSION_NUMBER >> into OPENSSL_VERSION_NUMBER, for consistency. >> >> * Squashed all into one patch. >> >> I intend to apply this to all supported branches, so please have a look! >> This is now against REL9_6_STABLE, but there should be little difference >> between branches in the code that this touches. > > This patch no longer seems to apply to head after the removed support of > 0.9.6. Is that intentional? Never mind. I just failed at reading. Now for a review: It looks generally good but I think I saw one error. In fe-secure-openssl.c your code still calls SSL_library_init() in OpenSSL 1.1. I think it should be enough to just call OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL) like you do in be-secure. Andreas
On 09/15/2016 03:16 AM, Andreas Karlsson wrote: > Now for a review: > > It looks generally good but I think I saw one error. In > fe-secure-openssl.c your code still calls SSL_library_init() in OpenSSL > 1.1. I think it should be enough to just call > OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL) like you do in be-secure. Ok, fixed that, and committed. Thanks everyone! I backpatched this to 9.5, but not further than that. The functions this modified were moved around in 9.5, so the patch wouldn't apply as is. It wouldn't be difficult to back-patch further if there's demand, but I'm not eager to do that until someone complains. - Heikki
On Thu, Sep 15, 2016 at 8:57 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote: > I backpatched this to 9.5, but not further than that. The functions this > modified were moved around in 9.5, so the patch wouldn't apply as is. It > wouldn't be difficult to back-patch further if there's demand, but I'm not > eager to do that until someone complains. Not going older than 9.5 may be fine: https://www.openssl.org/blog/blog/2014/12/23/the-new-release-strategy/ https://wiki.freebsd.org/OpenSSL As far as I can see 1.0.2 would be supported until Dec 2019, so that would just overlap with 9.4's EOL. -- Michael
Re: Michael Paquier 2016-09-15 <CAB7nPqQu1GpMzkB4S6XO0_+1cAUx==RDVF70vCmDytuA=nCHiQ@mail.gmail.com> > On Thu, Sep 15, 2016 at 8:57 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote: > > I backpatched this to 9.5, but not further than that. The functions this > > modified were moved around in 9.5, so the patch wouldn't apply as is. It > > wouldn't be difficult to back-patch further if there's demand, but I'm not > > eager to do that until someone complains. > > Not going older than 9.5 may be fine: > https://www.openssl.org/blog/blog/2014/12/23/the-new-release-strategy/ > https://wiki.freebsd.org/OpenSSL > As far as I can see 1.0.2 would be supported until Dec 2019, so that > would just overlap with 9.4's EOL. I'm afraid it's not that easy - Debian 9 (stretch) will release at the beginning of next year, and apt.postgresql.org will want to build 9.2/9.3/9.4 for that distribution. I guess yum.postgresql.org will have the same problem with the next Fedora release. Christoph
Christoph Berg wrote: > Re: Michael Paquier 2016-09-15 <CAB7nPqQu1GpMzkB4S6XO0_+1cAUx==RDVF70vCmDytuA=nCHiQ@mail.gmail.com> > > On Thu, Sep 15, 2016 at 8:57 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote: > > > I backpatched this to 9.5, but not further than that. The functions this > > > modified were moved around in 9.5, so the patch wouldn't apply as is. It > > > wouldn't be difficult to back-patch further if there's demand, but I'm not > > > eager to do that until someone complains. > > > > Not going older than 9.5 may be fine: > > https://www.openssl.org/blog/blog/2014/12/23/the-new-release-strategy/ > > https://wiki.freebsd.org/OpenSSL > > As far as I can see 1.0.2 would be supported until Dec 2019, so that > > would just overlap with 9.4's EOL. > > I'm afraid it's not that easy - Debian 9 (stretch) will release at the > beginning of next year, and apt.postgresql.org will want to build > 9.2/9.3/9.4 for that distribution. I guess yum.postgresql.org will > have the same problem with the next Fedora release. I suppose some interested party could grab the patch that Heikki committed to the new branches and produce a back-patch that can be applied to the older branches. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 09/15/2016 05:33 PM, Christoph Berg wrote: > Re: Michael Paquier 2016-09-15 <CAB7nPqQu1GpMzkB4S6XO0_+1cAUx==RDVF70vCmDytuA=nCHiQ@mail.gmail.com> >> On Thu, Sep 15, 2016 at 8:57 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote: >>> I backpatched this to 9.5, but not further than that. The functions this >>> modified were moved around in 9.5, so the patch wouldn't apply as is. It >>> wouldn't be difficult to back-patch further if there's demand, but I'm not >>> eager to do that until someone complains. >> >> Not going older than 9.5 may be fine: >> https://www.openssl.org/blog/blog/2014/12/23/the-new-release-strategy/ >> https://wiki.freebsd.org/OpenSSL >> As far as I can see 1.0.2 would be supported until Dec 2019, so that >> would just overlap with 9.4's EOL. > > I'm afraid it's not that easy - Debian 9 (stretch) will release at the > beginning of next year, and apt.postgresql.org will want to build > 9.2/9.3/9.4 for that distribution. I guess yum.postgresql.org will > have the same problem with the next Fedora release. Can you elaborate? Are you saying that Debian 9 (strect) will not ship OpenSSL 1.0.2 anymore, and will require using OpenSSL 1.1.0? - Heikki
Re: Heikki Linnakangas 2016-09-15 <7e4991a9-410f-5e1f-2a3a-e918e4a4bbbb@iki.fi> > > I'm afraid it's not that easy - Debian 9 (stretch) will release at the > > beginning of next year, and apt.postgresql.org will want to build > > 9.2/9.3/9.4 for that distribution. I guess yum.postgresql.org will > > have the same problem with the next Fedora release. > > Can you elaborate? Are you saying that Debian 9 (strect) will not ship > OpenSSL 1.0.2 anymore, and will require using OpenSSL 1.1.0? I thought that was the plan, but upon asking on #debian-devel, it seems it's not set yet. I'll ask the maintainers directly and report back. Christoph
On 09/15/2016 05:38 PM, Alvaro Herrera wrote: > I suppose some interested party could grab the patch that Heikki > committed to the new branches and produce a back-patch that can be > applied to the older branches. Here is the result of backporting the sum of the two patches on top of REL9_4_STABLE. Not sure if we need this, but if we do we can apply this patch. Andreas
Attachment
Andreas Karlsson <andreas@proxel.se> writes:
> On 09/15/2016 05:38 PM, Alvaro Herrera wrote:
>> I suppose some interested party could grab the patch that Heikki
>> committed to the new branches and produce a back-patch that can be
>> applied to the older branches.
> Here is the result of backporting the sum of the two patches on top of
> REL9_4_STABLE. Not sure if we need this, but if we do we can apply this
> patch.
If someone's done the legwork, I think we would be well advised to
back-patch. Maybe not bother with 9.1 though.
regards, tom lane
Re: To Heikki Linnakangas 2016-09-15 <20160915213406.2mjlhcg7px3saynq@msg.df7cb.de> > > Can you elaborate? Are you saying that Debian 9 (strect) will not ship > > OpenSSL 1.0.2 anymore, and will require using OpenSSL 1.1.0? > > I thought that was the plan, but upon asking on #debian-devel, it > seems it's not set yet. I'll ask the maintainers directly and report > back. The plan is to ship only OpenSSL 1.1 in Stretch. (The list of packages not yet ported is enormous, though, so I'm not yet sure it will really happen.) https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=827061 Re: Tom Lane 2016-09-16 <17025.1473977329@sss.pgh.pa.us> > > Here is the result of backporting the sum of the two patches on top of > > REL9_4_STABLE. Not sure if we need this, but if we do we can apply this > > patch. > > If someone's done the legwork, I think we would be well advised to > back-patch. Maybe not bother with 9.1 though. Thanks for the patch! I just tried to apply it to 9.2. There was a conflict in configure.in which was trivial to resolve. Another conflict in contrib/pgcrypto/pgcrypto.c was not applicable because the code doesn't seem to exist (didn't try very hard though). Ignoring the contrib conflict, it still didn't compile: /home/cbe/projects/postgresql/debian/9.2/build/../src/backend/libpq/be-secure.c: In function ‘secure_write’: /home/cbe/projects/postgresql/debian/9.2/build/../src/backend/libpq/be-secure.c:342:17: error: dereferencing pointer to incompletetype ‘SSL {aka struct ssl_st}’ if (port->ssl->state != SSL_ST_OK) ^~ /home/cbe/projects/postgresql/debian/9.2/build/../src/backend/libpq/be-secure.c:342:28: error: ‘SSL_ST_OK’ undeclared (firstuse in this function) if (port->ssl->state != SSL_ST_OK) ^~~~~~~~~ Christoph
On 09/16/2016 04:11 PM, Christoph Berg wrote:
> Thanks for the patch!
>
> I just tried to apply it to 9.2. There was a conflict in configure.in which was
> trivial to resolve.
>
> Another conflict in contrib/pgcrypto/pgcrypto.c was not applicable
> because the code doesn't seem to exist (didn't try very hard though).
>
> Ignoring the contrib conflict, it still didn't compile:
>
> /home/cbe/projects/postgresql/debian/9.2/build/../src/backend/libpq/be-secure.c: In function ‘secure_write’:
> /home/cbe/projects/postgresql/debian/9.2/build/../src/backend/libpq/be-secure.c:342:17: error: dereferencing pointer
toincomplete type ‘SSL {aka struct ssl_st}’
> if (port->ssl->state != SSL_ST_OK)
> ^~
> /home/cbe/projects/postgresql/debian/9.2/build/../src/backend/libpq/be-secure.c:342:28: error: ‘SSL_ST_OK’ undeclared
(firstuse in this function)
> if (port->ssl->state != SSL_ST_OK)
> ^~~~~~~~~
This is related to the renegotiation which was first fixed and later
removed in the 9.4 cycle, but intentionally not backported. It seems
like OpenSSL refactored the state machine in 1.1 which is why the code
above breaks.
I am not entirely sure I follow what the old code in 9.3 and 9.2 is
strying to do and why it messes directly with the state of the statemachine.
Andreas
Andreas Karlsson <andreas@proxel.se> writes:
> On 09/15/2016 05:38 PM, Alvaro Herrera wrote:
>> I suppose some interested party could grab the patch that Heikki
>> committed to the new branches and produce a back-patch that can be
>> applied to the older branches.
> Here is the result of backporting the sum of the two patches on top of
> REL9_4_STABLE. Not sure if we need this, but if we do we can apply this
> patch.
I've pushed this into 9.4 with trivial corrections (fix merge failure
against a later patch, and sync the autoconf output files with the
actual contents of configure.in). I've tested it locally against
openssl 1.0.1e and 1.1.0e, but not anything older. What I did to test
was to copy the 9.5-branch src/test/ssl/ stuff into 9.4 and run it.
I saw failures on the tests for Subject Alternative Name, which is
unsurprising since we added that support as a feature in 9.5, but
everything else passed. Unless the buildfarm turns up problems,
I think we're ok there.
I tried to push the code into 9.3, and saw the same problems Christoph
mentioned for 9.2: it compiles fine against 1.0.1e, but the references
to port->ssl->state don't work with 1.1. The reason that's OK in 9.4
is not that we removed SSL negotiation; that didn't happen until 9.5.
Rather, it's because this 9.4 commit got rid of the bogus code:
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Branch: master Release: REL9_4_BR [31cf1a1a4] 2013-10-10 23:45:20 -0300
Rework SSL renegotiation code
If we want to go any further back with 1.1 support, we have a range
of options:
1. Back-patch that patch, probably also including the followup adjustments
in 86029b31e and 36a3be654.
2. Add #if's to use 31cf1a1a4's coding with OpenSSL >= 1.1, while keeping
the older code for use when built against older OpenSSLs.
3. Conditionally disable renegotiation altogether with OpenSSL >= 1.1,
thus adopting 9.5 not 9.4 behavior when using newer OpenSSL.
I think #3 would be fairly weird unless we also changed 9.4 similarly.
But there's some argument for doing that: we don't really have any field
experience with using renegotiation with OpenSSL 1.1, so we don't know
that what is in the 9.4 branch right now actually works with 1.1.
On the other hand, it would also be the most work of these options,
since we'd have to do things like adding conditional behavior in guc.c.
Thoughts?
For the archives' sake, attached is the 9.3-adapted version of the
patch so far.
regards, tom lane
diff --git a/configure b/configure
index 0702667..e548722 100755
*** a/configure
--- b/configure
*************** $as_echo "$as_me: error: library 'crypto
*** 9524,9532 ****
fi
! { $as_echo "$as_me:$LINENO: checking for SSL_library_init in -lssl" >&5
! $as_echo_n "checking for SSL_library_init in -lssl... " >&6; }
! if test "${ac_cv_lib_ssl_SSL_library_init+set}" = set; then
$as_echo_n "(cached) " >&6
else
ac_check_lib_save_LIBS=$LIBS
--- 9524,9532 ----
fi
! { $as_echo "$as_me:$LINENO: checking for SSL_new in -lssl" >&5
! $as_echo_n "checking for SSL_new in -lssl... " >&6; }
! if test "${ac_cv_lib_ssl_SSL_new+set}" = set; then
$as_echo_n "(cached) " >&6
else
ac_check_lib_save_LIBS=$LIBS
*************** cat >>conftest.$ac_ext <<_ACEOF
*** 9544,9554 ****
#ifdef __cplusplus
extern "C"
#endif
! char SSL_library_init ();
int
main ()
{
! return SSL_library_init ();
;
return 0;
}
--- 9544,9554 ----
#ifdef __cplusplus
extern "C"
#endif
! char SSL_new ();
int
main ()
{
! return SSL_new ();
;
return 0;
}
*************** $as_echo "$ac_try_echo") >&5
*** 9574,9585 ****
test "$cross_compiling" = yes ||
$as_test_x conftest$ac_exeext
}; then
! ac_cv_lib_ssl_SSL_library_init=yes
else
$as_echo "$as_me: failed program was:" >&5
sed 's/^/| /' conftest.$ac_ext >&5
! ac_cv_lib_ssl_SSL_library_init=no
fi
rm -rf conftest.dSYM
--- 9574,9585 ----
test "$cross_compiling" = yes ||
$as_test_x conftest$ac_exeext
}; then
! ac_cv_lib_ssl_SSL_new=yes
else
$as_echo "$as_me: failed program was:" >&5
sed 's/^/| /' conftest.$ac_ext >&5
! ac_cv_lib_ssl_SSL_new=no
fi
rm -rf conftest.dSYM
*************** rm -f core conftest.err conftest.$ac_obj
*** 9587,9595 ****
conftest$ac_exeext conftest.$ac_ext
LIBS=$ac_check_lib_save_LIBS
fi
! { $as_echo "$as_me:$LINENO: result: $ac_cv_lib_ssl_SSL_library_init" >&5
! $as_echo "$ac_cv_lib_ssl_SSL_library_init" >&6; }
! if test "x$ac_cv_lib_ssl_SSL_library_init" = x""yes; then
cat >>confdefs.h <<_ACEOF
#define HAVE_LIBSSL 1
_ACEOF
--- 9587,9595 ----
conftest$ac_exeext conftest.$ac_ext
LIBS=$ac_check_lib_save_LIBS
fi
! { $as_echo "$as_me:$LINENO: result: $ac_cv_lib_ssl_SSL_new" >&5
! $as_echo "$ac_cv_lib_ssl_SSL_new" >&6; }
! if test "x$ac_cv_lib_ssl_SSL_new" = x""yes; then
cat >>confdefs.h <<_ACEOF
#define HAVE_LIBSSL 1
_ACEOF
*************** $as_echo "$as_me: error: library 'eay32'
*** 9694,9702 ****
{ (exit 1); exit 1; }; }
fi
! { $as_echo "$as_me:$LINENO: checking for library containing SSL_library_init" >&5
! $as_echo_n "checking for library containing SSL_library_init... " >&6; }
! if test "${ac_cv_search_SSL_library_init+set}" = set; then
$as_echo_n "(cached) " >&6
else
ac_func_search_save_LIBS=$LIBS
--- 9694,9702 ----
{ (exit 1); exit 1; }; }
fi
! { $as_echo "$as_me:$LINENO: checking for library containing SSL_new" >&5
! $as_echo_n "checking for library containing SSL_new... " >&6; }
! if test "${ac_cv_search_SSL_new+set}" = set; then
$as_echo_n "(cached) " >&6
else
ac_func_search_save_LIBS=$LIBS
*************** cat >>conftest.$ac_ext <<_ACEOF
*** 9713,9723 ****
#ifdef __cplusplus
extern "C"
#endif
! char SSL_library_init ();
int
main ()
{
! return SSL_library_init ();
;
return 0;
}
--- 9713,9723 ----
#ifdef __cplusplus
extern "C"
#endif
! char SSL_new ();
int
main ()
{
! return SSL_new ();
;
return 0;
}
*************** $as_echo "$ac_try_echo") >&5
*** 9750,9756 ****
test "$cross_compiling" = yes ||
$as_test_x conftest$ac_exeext
}; then
! ac_cv_search_SSL_library_init=$ac_res
else
$as_echo "$as_me: failed program was:" >&5
sed 's/^/| /' conftest.$ac_ext >&5
--- 9750,9756 ----
test "$cross_compiling" = yes ||
$as_test_x conftest$ac_exeext
}; then
! ac_cv_search_SSL_new=$ac_res
else
$as_echo "$as_me: failed program was:" >&5
sed 's/^/| /' conftest.$ac_ext >&5
*************** fi
*** 9761,9781 ****
rm -rf conftest.dSYM
rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
conftest$ac_exeext
! if test "${ac_cv_search_SSL_library_init+set}" = set; then
break
fi
done
! if test "${ac_cv_search_SSL_library_init+set}" = set; then
:
else
! ac_cv_search_SSL_library_init=no
fi
rm conftest.$ac_ext
LIBS=$ac_func_search_save_LIBS
fi
! { $as_echo "$as_me:$LINENO: result: $ac_cv_search_SSL_library_init" >&5
! $as_echo "$ac_cv_search_SSL_library_init" >&6; }
! ac_res=$ac_cv_search_SSL_library_init
if test "$ac_res" != no; then
test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
--- 9761,9781 ----
rm -rf conftest.dSYM
rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
conftest$ac_exeext
! if test "${ac_cv_search_SSL_new+set}" = set; then
break
fi
done
! if test "${ac_cv_search_SSL_new+set}" = set; then
:
else
! ac_cv_search_SSL_new=no
fi
rm conftest.$ac_ext
LIBS=$ac_func_search_save_LIBS
fi
! { $as_echo "$as_me:$LINENO: result: $ac_cv_search_SSL_new" >&5
! $as_echo "$ac_cv_search_SSL_new" >&6; }
! ac_res=$ac_cv_search_SSL_new
if test "$ac_res" != no; then
test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
*************** $as_echo "$as_me: error: library 'ssleay
*** 9786,9791 ****
--- 9786,10004 ----
fi
fi
+ # Functions introduced in OpenSSL 1.1.0. We used to check for
+ # OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
+ # defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it
+ # doesn't have these OpenSSL 1.1.0 functions. So check for individual
+ # functions.
+
+
+
+
+ for ac_func in OPENSSL_init_ssl BIO_meth_new ASN1_STRING_get0_data RAND_OpenSSL
+ do
+ as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
+ { $as_echo "$as_me:$LINENO: checking for $ac_func" >&5
+ $as_echo_n "checking for $ac_func... " >&6; }
+ if { as_var=$as_ac_var; eval "test \"\${$as_var+set}\" = set"; }; then
+ $as_echo_n "(cached) " >&6
+ else
+ cat >conftest.$ac_ext <<_ACEOF
+ /* confdefs.h. */
+ _ACEOF
+ cat confdefs.h >>conftest.$ac_ext
+ cat >>conftest.$ac_ext <<_ACEOF
+ /* end confdefs.h. */
+ /* Define $ac_func to an innocuous variant, in case <limits.h> declares $ac_func.
+ For example, HP-UX 11i <limits.h> declares gettimeofday. */
+ #define $ac_func innocuous_$ac_func
+
+ /* System header to define __stub macros and hopefully few prototypes,
+ which can conflict with char $ac_func (); below.
+ Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
+ <limits.h> exists even on freestanding compilers. */
+
+ #ifdef __STDC__
+ # include <limits.h>
+ #else
+ # include <assert.h>
+ #endif
+
+ #undef $ac_func
+
+ /* Override any GCC internal prototype to avoid an error.
+ Use char because int might match the return type of a GCC
+ builtin and then its argument prototype would still apply. */
+ #ifdef __cplusplus
+ extern "C"
+ #endif
+ char $ac_func ();
+ /* The GNU C library defines this for functions which it implements
+ to always fail with ENOSYS. Some functions are actually named
+ something starting with __ and the normal name is an alias. */
+ #if defined __stub_$ac_func || defined __stub___$ac_func
+ choke me
+ #endif
+
+ int
+ main ()
+ {
+ return $ac_func ();
+ ;
+ return 0;
+ }
+ _ACEOF
+ rm -f conftest.$ac_objext conftest$ac_exeext
+ if { (ac_try="$ac_link"
+ case "(($ac_try" in
+ *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
+ *) ac_try_echo=$ac_try;;
+ esac
+ eval ac_try_echo="\"\$as_me:$LINENO: $ac_try_echo\""
+ $as_echo "$ac_try_echo") >&5
+ (eval "$ac_link") 2>conftest.er1
+ ac_status=$?
+ grep -v '^ *+' conftest.er1 >conftest.err
+ rm -f conftest.er1
+ cat conftest.err >&5
+ $as_echo "$as_me:$LINENO: \$? = $ac_status" >&5
+ (exit $ac_status); } && {
+ test -z "$ac_c_werror_flag" ||
+ test ! -s conftest.err
+ } && test -s conftest$ac_exeext && {
+ test "$cross_compiling" = yes ||
+ $as_test_x conftest$ac_exeext
+ }; then
+ eval "$as_ac_var=yes"
+ else
+ $as_echo "$as_me: failed program was:" >&5
+ sed 's/^/| /' conftest.$ac_ext >&5
+
+ eval "$as_ac_var=no"
+ fi
+
+ rm -rf conftest.dSYM
+ rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
+ conftest$ac_exeext conftest.$ac_ext
+ fi
+ ac_res=`eval 'as_val=${'$as_ac_var'}
+ $as_echo "$as_val"'`
+ { $as_echo "$as_me:$LINENO: result: $ac_res" >&5
+ $as_echo "$ac_res" >&6; }
+ as_val=`eval 'as_val=${'$as_ac_var'}
+ $as_echo "$as_val"'`
+ if test "x$as_val" = x""yes; then
+ cat >>confdefs.h <<_ACEOF
+ #define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1
+ _ACEOF
+
+ fi
+ done
+
+ # OpenSSL versions before 1.1.0 required setting callback functions, for
+ # thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
+ # function was removed.
+
+ for ac_func in CRYPTO_lock
+ do
+ as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
+ { $as_echo "$as_me:$LINENO: checking for $ac_func" >&5
+ $as_echo_n "checking for $ac_func... " >&6; }
+ if { as_var=$as_ac_var; eval "test \"\${$as_var+set}\" = set"; }; then
+ $as_echo_n "(cached) " >&6
+ else
+ cat >conftest.$ac_ext <<_ACEOF
+ /* confdefs.h. */
+ _ACEOF
+ cat confdefs.h >>conftest.$ac_ext
+ cat >>conftest.$ac_ext <<_ACEOF
+ /* end confdefs.h. */
+ /* Define $ac_func to an innocuous variant, in case <limits.h> declares $ac_func.
+ For example, HP-UX 11i <limits.h> declares gettimeofday. */
+ #define $ac_func innocuous_$ac_func
+
+ /* System header to define __stub macros and hopefully few prototypes,
+ which can conflict with char $ac_func (); below.
+ Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
+ <limits.h> exists even on freestanding compilers. */
+
+ #ifdef __STDC__
+ # include <limits.h>
+ #else
+ # include <assert.h>
+ #endif
+
+ #undef $ac_func
+
+ /* Override any GCC internal prototype to avoid an error.
+ Use char because int might match the return type of a GCC
+ builtin and then its argument prototype would still apply. */
+ #ifdef __cplusplus
+ extern "C"
+ #endif
+ char $ac_func ();
+ /* The GNU C library defines this for functions which it implements
+ to always fail with ENOSYS. Some functions are actually named
+ something starting with __ and the normal name is an alias. */
+ #if defined __stub_$ac_func || defined __stub___$ac_func
+ choke me
+ #endif
+
+ int
+ main ()
+ {
+ return $ac_func ();
+ ;
+ return 0;
+ }
+ _ACEOF
+ rm -f conftest.$ac_objext conftest$ac_exeext
+ if { (ac_try="$ac_link"
+ case "(($ac_try" in
+ *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
+ *) ac_try_echo=$ac_try;;
+ esac
+ eval ac_try_echo="\"\$as_me:$LINENO: $ac_try_echo\""
+ $as_echo "$ac_try_echo") >&5
+ (eval "$ac_link") 2>conftest.er1
+ ac_status=$?
+ grep -v '^ *+' conftest.er1 >conftest.err
+ rm -f conftest.er1
+ cat conftest.err >&5
+ $as_echo "$as_me:$LINENO: \$? = $ac_status" >&5
+ (exit $ac_status); } && {
+ test -z "$ac_c_werror_flag" ||
+ test ! -s conftest.err
+ } && test -s conftest$ac_exeext && {
+ test "$cross_compiling" = yes ||
+ $as_test_x conftest$ac_exeext
+ }; then
+ eval "$as_ac_var=yes"
+ else
+ $as_echo "$as_me: failed program was:" >&5
+ sed 's/^/| /' conftest.$ac_ext >&5
+
+ eval "$as_ac_var=no"
+ fi
+
+ rm -rf conftest.dSYM
+ rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
+ conftest$ac_exeext conftest.$ac_ext
+ fi
+ ac_res=`eval 'as_val=${'$as_ac_var'}
+ $as_echo "$as_val"'`
+ { $as_echo "$as_me:$LINENO: result: $ac_res" >&5
+ $as_echo "$ac_res" >&6; }
+ as_val=`eval 'as_val=${'$as_ac_var'}
+ $as_echo "$as_val"'`
+ if test "x$as_val" = x""yes; then
+ cat >>confdefs.h <<_ACEOF
+ #define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1
+ _ACEOF
+
+ fi
+ done
+
fi
if test "$with_pam" = yes ; then
diff --git a/configure.in b/configure.in
index e00adc9..aad7598 100644
*** a/configure.in
--- b/configure.in
*************** if test "$with_openssl" = yes ; then
*** 951,961 ****
dnl Order matters!
if test "$PORTNAME" != "win32"; then
AC_CHECK_LIB(crypto, CRYPTO_new_ex_data, [], [AC_MSG_ERROR([library 'crypto' is required for OpenSSL])])
! AC_CHECK_LIB(ssl, SSL_library_init, [], [AC_MSG_ERROR([library 'ssl' is required for OpenSSL])])
else
AC_SEARCH_LIBS(CRYPTO_new_ex_data, eay32 crypto, [], [AC_MSG_ERROR([library 'eay32' or 'crypto' is required for
OpenSSL])])
! AC_SEARCH_LIBS(SSL_library_init, ssleay32 ssl, [], [AC_MSG_ERROR([library 'ssleay32' or 'ssl' is required for
OpenSSL])])
fi
fi
if test "$with_pam" = yes ; then
--- 951,971 ----
dnl Order matters!
if test "$PORTNAME" != "win32"; then
AC_CHECK_LIB(crypto, CRYPTO_new_ex_data, [], [AC_MSG_ERROR([library 'crypto' is required for OpenSSL])])
! AC_CHECK_LIB(ssl, SSL_new, [], [AC_MSG_ERROR([library 'ssl' is required for OpenSSL])])
else
AC_SEARCH_LIBS(CRYPTO_new_ex_data, eay32 crypto, [], [AC_MSG_ERROR([library 'eay32' or 'crypto' is required for
OpenSSL])])
! AC_SEARCH_LIBS(SSL_new, ssleay32 ssl, [], [AC_MSG_ERROR([library 'ssleay32' or 'ssl' is required for OpenSSL])])
fi
+ # Functions introduced in OpenSSL 1.1.0. We used to check for
+ # OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
+ # defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it
+ # doesn't have these OpenSSL 1.1.0 functions. So check for individual
+ # functions.
+ AC_CHECK_FUNCS([OPENSSL_init_ssl BIO_meth_new ASN1_STRING_get0_data RAND_OpenSSL])
+ # OpenSSL versions before 1.1.0 required setting callback functions, for
+ # thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
+ # function was removed.
+ AC_CHECK_FUNCS([CRYPTO_lock])
fi
if test "$with_pam" = yes ; then
diff --git a/contrib/pgcrypto/internal.c b/contrib/pgcrypto/internal.c
index cb8ba26..02ff976 100644
*** a/contrib/pgcrypto/internal.c
--- b/contrib/pgcrypto/internal.c
*************** px_find_cipher(const char *name, PX_Ciph
*** 620,634 ****
* Randomness provider
*/
- /*
- * Use always strong randomness.
- */
- int
- px_get_pseudo_random_bytes(uint8 *dst, unsigned count)
- {
- return px_get_random_bytes(dst, count);
- }
-
static time_t seed_time = 0;
static time_t check_time = 0;
--- 620,625 ----
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index e49dbaf..9ae45b2 100644
*** a/contrib/pgcrypto/openssl.c
--- b/contrib/pgcrypto/openssl.c
***************
*** 40,45 ****
--- 40,48 ----
#include <openssl/rand.h>
#include <openssl/err.h>
+ #include "utils/memutils.h"
+ #include "utils/resowner.h"
+
/*
* Max lengths we might want to handle.
*/
*************** compat_find_digest(const char *name, PX_
*** 199,216 ****
* Hashes
*/
typedef struct OSSLDigest
{
const EVP_MD *algo;
! EVP_MD_CTX ctx;
} OSSLDigest;
static unsigned
digest_result_size(PX_MD *h)
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! return EVP_MD_CTX_size(&digest->ctx);
}
static unsigned
--- 202,274 ----
* Hashes
*/
+ /*
+ * To make sure we don't leak OpenSSL handles on abort, we keep OSSLDigest
+ * objects in a linked list, allocated in TopMemoryContext. We use the
+ * ResourceOwner mechanism to free them on abort.
+ */
typedef struct OSSLDigest
{
const EVP_MD *algo;
! EVP_MD_CTX *ctx;
!
! ResourceOwner owner;
! struct OSSLDigest *next;
! struct OSSLDigest *prev;
} OSSLDigest;
+ static OSSLDigest *open_digests = NULL;
+ static bool resowner_callback_registered = false;
+
+ static void
+ free_openssldigest(OSSLDigest *digest)
+ {
+ EVP_MD_CTX_destroy(digest->ctx);
+ if (digest->prev)
+ digest->prev->next = digest->next;
+ else
+ open_digests = digest->next;
+ if (digest->next)
+ digest->next->prev = digest->prev;
+ pfree(digest);
+ }
+
+ /*
+ * Close any open OpenSSL handles on abort.
+ */
+ static void
+ digest_free_callback(ResourceReleasePhase phase,
+ bool isCommit,
+ bool isTopLevel,
+ void *arg)
+ {
+ OSSLDigest *curr;
+ OSSLDigest *next;
+
+ if (phase != RESOURCE_RELEASE_AFTER_LOCKS)
+ return;
+
+ next = open_digests;
+ while (next)
+ {
+ curr = next;
+ next = curr->next;
+
+ if (curr->owner == CurrentResourceOwner)
+ {
+ if (isCommit)
+ elog(WARNING, "pgcrypto digest reference leak: digest %p still referenced", curr);
+ free_openssldigest(curr);
+ }
+ }
+ }
+
static unsigned
digest_result_size(PX_MD *h)
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! return EVP_MD_CTX_size(digest->ctx);
}
static unsigned
*************** digest_block_size(PX_MD *h)
*** 218,224 ****
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! return EVP_MD_CTX_block_size(&digest->ctx);
}
static void
--- 276,282 ----
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! return EVP_MD_CTX_block_size(digest->ctx);
}
static void
*************** digest_reset(PX_MD *h)
*** 226,232 ****
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! EVP_DigestInit_ex(&digest->ctx, digest->algo, NULL);
}
static void
--- 284,290 ----
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! EVP_DigestInit_ex(digest->ctx, digest->algo, NULL);
}
static void
*************** digest_update(PX_MD *h, const uint8 *dat
*** 234,240 ****
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! EVP_DigestUpdate(&digest->ctx, data, dlen);
}
static void
--- 292,298 ----
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! EVP_DigestUpdate(digest->ctx, data, dlen);
}
static void
*************** digest_finish(PX_MD *h, uint8 *dst)
*** 242,248 ****
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! EVP_DigestFinal_ex(&digest->ctx, dst, NULL);
}
static void
--- 300,306 ----
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! EVP_DigestFinal_ex(digest->ctx, dst, NULL);
}
static void
*************** digest_free(PX_MD *h)
*** 250,258 ****
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! EVP_MD_CTX_cleanup(&digest->ctx);
!
! px_free(digest);
px_free(h);
}
--- 308,314 ----
{
OSSLDigest *digest = (OSSLDigest *) h->p.ptr;
! free_openssldigest(digest);
px_free(h);
}
*************** int
*** 264,269 ****
--- 320,326 ----
px_find_digest(const char *name, PX_MD **res)
{
const EVP_MD *md;
+ EVP_MD_CTX *ctx;
PX_MD *h;
OSSLDigest *digest;
*************** px_find_digest(const char *name, PX_MD *
*** 273,289 ****
OpenSSL_add_all_algorithms();
}
md = EVP_get_digestbyname(name);
if (md == NULL)
return compat_find_digest(name, 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)
return -1;
h = px_alloc(sizeof(*h));
h->result_size = digest_result_size;
h->block_size = digest_block_size;
--- 330,372 ----
OpenSSL_add_all_algorithms();
}
+ if (!resowner_callback_registered)
+ {
+ RegisterResourceReleaseCallback(digest_free_callback, NULL);
+ resowner_callback_registered = true;
+ }
+
md = EVP_get_digestbyname(name);
if (md == NULL)
return compat_find_digest(name, res);
! /*
! * Create an OSSLDigest object, an OpenSSL MD object, and a PX_MD object.
! * The order is crucial, to make sure we don't leak anything on
! * out-of-memory or other error.
! */
! digest = MemoryContextAlloc(TopMemoryContext, sizeof(*digest));
! ctx = EVP_MD_CTX_create();
! if (!ctx)
! {
! pfree(digest);
return -1;
+ }
+ if (EVP_DigestInit_ex(ctx, md, NULL) == 0)
+ {
+ pfree(digest);
+ return -1;
+ }
+
+ digest->algo = md;
+ digest->ctx = ctx;
+ digest->owner = CurrentResourceOwner;
+ digest->next = open_digests;
+ digest->prev = NULL;
+ open_digests = digest;
+ /* The PX_MD object is allocated in the current memory context. */
h = px_alloc(sizeof(*h));
h->result_size = digest_result_size;
h->block_size = digest_block_size;
*************** static void
*** 987,993 ****
--- 1070,1082 ----
init_openssl_rand(void)
{
if (RAND_get_rand_method() == NULL)
+ {
+ #ifdef HAVE_RAND_OPENSSL
+ RAND_set_rand_method(RAND_OpenSSL());
+ #else
RAND_set_rand_method(RAND_SSLeay());
+ #endif
+ }
openssl_random_init = 1;
}
*************** px_get_random_bytes(uint8 *dst, unsigned
*** 1007,1027 ****
}
int
- px_get_pseudo_random_bytes(uint8 *dst, unsigned count)
- {
- int res;
-
- if (!openssl_random_init)
- init_openssl_rand();
-
- res = RAND_pseudo_bytes(dst, count);
- if (res == 0 || res == 1)
- return count;
-
- return PXE_OSSL_RAND_ERROR;
- }
-
- int
px_add_entropy(const uint8 *data, unsigned count)
{
/*
--- 1096,1101 ----
diff --git a/contrib/pgcrypto/pgp-s2k.c b/contrib/pgcrypto/pgp-s2k.c
index 5f47e79..fb651fc 100644
*** a/contrib/pgcrypto/pgp-s2k.c
--- b/contrib/pgcrypto/pgp-s2k.c
*************** pgp_s2k_fill(PGP_S2K *s2k, int mode, int
*** 223,235 ****
case 0:
break;
case 1:
! res = px_get_pseudo_random_bytes(s2k->salt, PGP_S2K_SALT);
break;
case 3:
! res = px_get_pseudo_random_bytes(s2k->salt, PGP_S2K_SALT);
if (res < 0)
break;
! res = px_get_pseudo_random_bytes(&tmp, 1);
if (res < 0)
break;
s2k->iter = decide_count(tmp);
--- 223,235 ----
case 0:
break;
case 1:
! res = px_get_random_bytes(s2k->salt, PGP_S2K_SALT);
break;
case 3:
! res = px_get_random_bytes(s2k->salt, PGP_S2K_SALT);
if (res < 0)
break;
! res = px_get_random_bytes(&tmp, 1);
if (res < 0)
break;
s2k->iter = decide_count(tmp);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index e3246fc..3d42393 100644
*** a/contrib/pgcrypto/px-crypt.c
--- b/contrib/pgcrypto/px-crypt.c
*************** px_gen_salt(const char *salt_type, char
*** 153,159 ****
return PXE_BAD_SALT_ROUNDS;
}
! res = px_get_pseudo_random_bytes((uint8 *) rbuf, g->input_len);
if (res < 0)
return res;
--- 153,159 ----
return PXE_BAD_SALT_ROUNDS;
}
! res = px_get_random_bytes((uint8 *) rbuf, g->input_len);
if (res < 0)
return res;
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index d237d97..fa889eb 100644
*** a/contrib/pgcrypto/px.h
--- b/contrib/pgcrypto/px.h
*************** int px_find_cipher(const char *name, P
*** 190,196 ****
int px_find_combo(const char *name, PX_Combo **res);
int px_get_random_bytes(uint8 *dst, unsigned count);
- int px_get_pseudo_random_bytes(uint8 *dst, unsigned count);
int px_add_entropy(const uint8 *data, unsigned count);
unsigned px_acquire_system_randomness(uint8 *dst);
--- 190,195 ----
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 82a608d..0122b0d 100644
*** a/src/backend/libpq/be-secure.c
--- b/src/backend/libpq/be-secure.c
***************
*** 66,72 ****
#ifdef USE_SSL
#include <openssl/ssl.h>
#include <openssl/dh.h>
! #if SSLEAY_VERSION_NUMBER >= 0x0907000L
#include <openssl/conf.h>
#endif
#endif /* USE_SSL */
--- 66,72 ----
#ifdef USE_SSL
#include <openssl/ssl.h>
#include <openssl/dh.h>
! #if OPENSSL_VERSION_NUMBER >= 0x0907000L
#include <openssl/conf.h>
#endif
#endif /* USE_SSL */
***************
*** 80,85 ****
--- 80,86 ----
static DH *load_dh_file(int keylength);
static DH *load_dh_buffer(const char *, size_t);
+ static DH *generate_dh_parameters(int prime_len, int generator);
static DH *tmp_dh_cb(SSL *s, int is_export, int keylength);
static int verify_cb(int, X509_STORE_CTX *);
static void info_cb(const SSL *ssl, int type, int args);
*************** wloop:
*** 423,430 ****
* to retry; do we need to adopt their logic for that?
*/
! static bool my_bio_initialized = false;
! static BIO_METHOD my_bio_methods;
static int
my_sock_read(BIO *h, char *buf, int size)
--- 424,430 ----
* to retry; do we need to adopt their logic for that?
*/
! static BIO_METHOD *my_bio_methods = NULL;
static int
my_sock_read(BIO *h, char *buf, int size)
*************** my_sock_read(BIO *h, char *buf, int size
*** 435,441 ****
if (buf != NULL)
{
! res = recv(h->num, buf, size, 0);
BIO_clear_retry_flags(h);
if (res <= 0)
{
--- 435,441 ----
if (buf != NULL)
{
! res = recv(BIO_get_fd(h, NULL), buf, size, 0);
BIO_clear_retry_flags(h);
if (res <= 0)
{
*************** my_sock_write(BIO *h, const char *buf, i
*** 457,463 ****
{
int res = 0;
! res = send(h->num, buf, size, 0);
BIO_clear_retry_flags(h);
if (res <= 0)
{
--- 457,463 ----
{
int res = 0;
! res = send(BIO_get_fd(h, NULL), buf, size, 0);
BIO_clear_retry_flags(h);
if (res <= 0)
{
*************** my_sock_write(BIO *h, const char *buf, i
*** 473,486 ****
static BIO_METHOD *
my_BIO_s_socket(void)
{
! if (!my_bio_initialized)
{
! 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;
}
! return &my_bio_methods;
}
/* This should exactly match openssl's SSL_set_fd except for using my BIO */
--- 473,513 ----
static BIO_METHOD *
my_BIO_s_socket(void)
{
! if (!my_bio_methods)
{
! BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
! #ifdef HAVE_BIO_METH_NEW
! int my_bio_index;
!
! my_bio_index = BIO_get_new_index();
! if (my_bio_index == -1)
! return NULL;
! my_bio_methods = BIO_meth_new(my_bio_index, "PostgreSQL backend socket");
! if (!my_bio_methods)
! return NULL;
! if (!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_puts(my_bio_methods, BIO_meth_get_puts(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)))
! {
! BIO_meth_free(my_bio_methods);
! my_bio_methods = NULL;
! return NULL;
! }
! #else
! my_bio_methods = malloc(sizeof(BIO_METHOD));
! if (!my_bio_methods)
! return NULL;
! memcpy(my_bio_methods, biom, sizeof(BIO_METHOD));
! my_bio_methods->bread = my_sock_read;
! my_bio_methods->bwrite = my_sock_write;
! #endif
}
! return my_bio_methods;
}
/* This should exactly match openssl's SSL_set_fd except for using my BIO */
*************** static int
*** 488,496 ****
my_SSL_set_fd(SSL *s, int fd)
{
int ret = 0;
! BIO *bio = NULL;
! bio = BIO_new(my_BIO_s_socket());
if (bio == NULL)
{
--- 515,530 ----
my_SSL_set_fd(SSL *s, int fd)
{
int ret = 0;
! BIO *bio;
! BIO_METHOD *bio_method;
! bio_method = my_BIO_s_socket();
! if (bio_method == NULL)
! {
! SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
! goto err;
! }
! bio = BIO_new(bio_method);
if (bio == NULL)
{
*************** load_dh_buffer(const char *buffer, size_
*** 590,595 ****
--- 624,654 ----
}
/*
+ * Generate DH parameters.
+ *
+ * Last resort if we can't load precomputed nor hardcoded
+ * parameters.
+ */
+ static DH *
+ generate_dh_parameters(int prime_len, int generator)
+ {
+ #if (OPENSSL_VERSION_NUMBER >= 0x0090800fL)
+ 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
+ }
+
+ /*
* Generate an ephemeral DH key. Because this can take a long
* time to compute, we can use precomputed parameters of the
* common key sizes.
*************** tmp_dh_cb(SSL *s, int is_export, int key
*** 658,664 ****
ereport(DEBUG2,
(errmsg_internal("DH: generating parameters (%d bits)",
keylength)));
! r = DH_generate_parameters(keylength, DH_GENERATOR_2, NULL, NULL);
}
return r;
--- 717,723 ----
ereport(DEBUG2,
(errmsg_internal("DH: generating parameters (%d bits)",
keylength)));
! r = generate_dh_parameters(keylength, DH_GENERATOR_2);
}
return r;
*************** initialize_SSL(void)
*** 737,747 ****
--- 796,810 ----
if (!SSL_context)
{
+ #ifdef HAVE_OPENSSL_INIT_SSL
+ OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL);
+ #else
#if SSLEAY_VERSION_NUMBER >= 0x0907000L
OPENSSL_config(NULL);
#endif
SSL_library_init();
SSL_load_error_strings();
+ #endif
/*
* We use SSLv23_method() because it can negotiate use of the highest
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 985e5a7..872d39f 100644
*** a/src/include/pg_config.h.in
--- b/src/include/pg_config.h.in
***************
*** 87,92 ****
--- 87,98 ----
/* Define to 1 if you have the `append_history' function. */
#undef HAVE_APPEND_HISTORY
+ /* Define to 1 if you have the `ASN1_STRING_get0_data' function. */
+ #undef HAVE_ASN1_STRING_GET0_DATA
+
+ /* Define to 1 if you have the `BIO_meth_new' function. */
+ #undef HAVE_BIO_METH_NEW
+
/* Define to 1 if you have the `cbrt' function. */
#undef HAVE_CBRT
***************
*** 99,104 ****
--- 105,113 ----
/* Define to 1 if you have the `crypt' function. */
#undef HAVE_CRYPT
+ /* Define to 1 if you have the `CRYPTO_lock' function. */
+ #undef HAVE_CRYPTO_LOCK
+
/* Define to 1 if you have the <crypt.h> header file. */
#undef HAVE_CRYPT_H
***************
*** 357,362 ****
--- 366,374 ----
/* Define to 1 if you have the <net/if.h> header file. */
#undef HAVE_NET_IF_H
+ /* Define to 1 if you have the `OPENSSL_init_ssl' function. */
+ #undef HAVE_OPENSSL_INIT_SSL
+
/* Define to 1 if you have the <ossp/uuid.h> header file. */
#undef HAVE_OSSP_UUID_H
***************
*** 396,401 ****
--- 408,416 ----
/* Define to 1 if you have the `random' function. */
#undef HAVE_RANDOM
+ /* Define to 1 if you have the `RAND_OpenSSL' function. */
+ #undef HAVE_RAND_OPENSSL
+
/* Define to 1 if you have the <readline.h> header file. */
#undef HAVE_READLINE_H
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 9e7ae47..a52a206 100644
*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
***************
*** 58,64 ****
#ifdef USE_SSL
#include <openssl/ssl.h>
! #if (SSLEAY_VERSION_NUMBER >= 0x00907000L)
#include <openssl/conf.h>
#endif
#ifdef USE_SSL_ENGINE
--- 58,64 ----
#ifdef USE_SSL
#include <openssl/ssl.h>
! #if (OPENSSL_VERSION_NUMBER >= 0x00907000L)
#include <openssl/conf.h>
#endif
#ifdef USE_SSL_ENGINE
*************** verify_peer_name_matches_certificate(PGc
*** 835,843 ****
return result;
}
! #ifdef ENABLE_THREAD_SAFETY
/*
! * Callback functions for OpenSSL internal locking
*/
static unsigned long
--- 835,847 ----
return result;
}
! #if defined(ENABLE_THREAD_SAFETY) && defined(HAVE_CRYPTO_LOCK)
/*
! * Callback functions for OpenSSL internal locking. (OpenSSL 1.1.0
! * does its own locking, and doesn't need these anymore. The
! * CRYPTO_lock() function was removed in 1.1.0, when the callbacks
! * were made obsolete, so we assume that if CRYPTO_lock() exists,
! * the callbacks are still required.)
*/
static unsigned long
*************** pq_lockingcallback(int mode, int n, cons
*** 867,873 ****
PGTHREAD_ERROR("failed to unlock mutex");
}
}
! #endif /* ENABLE_THREAD_SAFETY */
/*
* Initialize SSL library.
--- 871,877 ----
PGTHREAD_ERROR("failed to unlock mutex");
}
}
! #endif /* ENABLE_THREAD_SAFETY && HAVE_CRYPTO_LOCK */
/*
* Initialize SSL library.
*************** init_ssl_system(PGconn *conn)
*** 905,910 ****
--- 909,915 ----
if (pthread_mutex_lock(&ssl_config_mutex))
return -1;
+ #ifdef HAVE_CRYPTO_LOCK
if (pq_init_crypto_lib)
{
/*
*************** init_ssl_system(PGconn *conn)
*** 940,956 ****
CRYPTO_set_locking_callback(pq_lockingcallback);
}
}
#endif /* ENABLE_THREAD_SAFETY */
if (!ssl_lib_initialized)
{
if (pq_init_ssl_lib)
{
! #if SSLEAY_VERSION_NUMBER >= 0x00907000L
OPENSSL_config(NULL);
#endif
SSL_library_init();
SSL_load_error_strings();
}
ssl_lib_initialized = true;
}
--- 945,966 ----
CRYPTO_set_locking_callback(pq_lockingcallback);
}
}
+ #endif /* HAVE_CRYPTO_LOCK */
#endif /* ENABLE_THREAD_SAFETY */
if (!ssl_lib_initialized)
{
if (pq_init_ssl_lib)
{
! #ifdef HAVE_OPENSSL_INIT_SSL
! OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL);
! #else
! #if OPENSSL_VERSION_NUMBER >= 0x00907000L
OPENSSL_config(NULL);
#endif
SSL_library_init();
SSL_load_error_strings();
+ #endif
}
ssl_lib_initialized = true;
}
*************** init_ssl_system(PGconn *conn)
*** 970,981 ****
* if we had any.)
*
* Callbacks are only set when we're compiled in threadsafe mode, so
! * we only need to remove them in this case.
*/
static void
destroy_ssl_system(void)
{
! #ifdef ENABLE_THREAD_SAFETY
/* Mutex is created in initialize_ssl_system() */
if (pthread_mutex_lock(&ssl_config_mutex))
return;
--- 980,992 ----
* if we had any.)
*
* Callbacks are only set when we're compiled in threadsafe mode, so
! * we only need to remove them in this case. They are also not needed
! * with OpenSSL 1.1.0 anymore.
*/
static void
destroy_ssl_system(void)
{
! #if defined(ENABLE_THREAD_SAFETY) && defined(HAVE_CRYPTO_LOCK)
/* Mutex is created in initialize_ssl_system() */
if (pthread_mutex_lock(&ssl_config_mutex))
return;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index f14cf2b..ba1bfac 100644
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
*************** typedef struct
*** 77,83 ****
#include <openssl/ssl.h>
#include <openssl/err.h>
! #if (SSLEAY_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
#define USE_SSL_ENGINE
#endif
#endif /* USE_SSL */
--- 77,83 ----
#include <openssl/ssl.h>
#include <openssl/err.h>
! #if (OPENSSL_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
#define USE_SSL_ENGINE
#endif
#endif /* USE_SSL */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I wrote: > If we want to go any further back with 1.1 support, we have a range > of options: > 1. Back-patch that patch, probably also including the followup adjustments > in 86029b31e and 36a3be654. > 2. Add #if's to use 31cf1a1a4's coding with OpenSSL >= 1.1, while keeping > the older code for use when built against older OpenSSLs. > 3. Conditionally disable renegotiation altogether with OpenSSL >= 1.1, > thus adopting 9.5 not 9.4 behavior when using newer OpenSSL. > I think #3 would be fairly weird unless we also changed 9.4 similarly. > But there's some argument for doing that: we don't really have any field > experience with using renegotiation with OpenSSL 1.1, so we don't know > that what is in the 9.4 branch right now actually works with 1.1. > On the other hand, it would also be the most work of these options, > since we'd have to do things like adding conditional behavior in guc.c. I did some simple testing and can say that at least the successful path in the 9.4 code seems to be fine with 1.1; in particular I see no sign of the misbehavior discussed in https://www.postgresql.org/message-id/flat/20150126101405.GA31719%40awork2.anarazel.de Given Heikki's opinion that that was an OpenSSL bug, perhaps they fixed the bug. Certainly we don't seem to have committed any of the workaround patches discussed in that thread. At this point I'd be inclined to reject option #3: aside from being more work, it'd be a pain to document, and confusing for users. I have a slight preference for #1 over #2 --- we'd intended to back-patch Alvaro's fixes once they had survived some field use, and I know of no evidence that 9.4 is worse than the older branches. regards, tom lane
On 04/16/2017 03:14 AM, Tom Lane wrote: > 1. Back-patch that patch, probably also including the followup adjustments > in 86029b31e and 36a3be654. > > 2. Add #if's to use 31cf1a1a4's coding with OpenSSL >= 1.1, while keeping > the older code for use when built against older OpenSSLs. > > 3. Conditionally disable renegotiation altogether with OpenSSL >= 1.1, > thus adopting 9.5 not 9.4 behavior when using newer OpenSSL. > > [...] > > Thoughts? Given that I cannot recall seeing any complaints about the behavior of 9.4 compared to 9.3 I am leaning towards #1. That way there are fewer different versions of our OpenSSL code. Andreas
Andreas Karlsson <andreas@proxel.se> writes:
> On 04/16/2017 03:14 AM, Tom Lane wrote:
>> 1. Back-patch that patch, probably also including the followup adjustments
>> in 86029b31e and 36a3be654.
> Given that I cannot recall seeing any complaints about the behavior of
> 9.4 compared to 9.3 I am leaning towards #1. That way there are fewer
> different versions of our OpenSSL code.
Yeah, I was thinking about that point too. Barring objections I'll
do #1 and then move forward with the openssl 1.1 backport.
regards, tom lane