Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-24 Thread Michael Paquier
On Fri, Jun 23, 2023 at 10:41:06PM +0200, Peter Eisentraut wrote:
> Considering that, yes.

Thanks, applied to 11~13, then.
--
Michael


signature.asc
Description: PGP signature


Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-23 Thread Peter Eisentraut

On 23.06.23 00:22, Michael Paquier wrote:

Also, note that the documentation claims that the minimum version of
OpenSSL supported is 0.9.8, which is something that commit 9b7cd59 has
done, impacting Postgres 10~.  So your argument looks incorrect to me?


Considering that, yes.




Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-22 Thread Michael Paquier
On Thu, Jun 22, 2023 at 08:08:54PM +0200, Peter Eisentraut wrote:
> The message linked to above also says:
> 
>> I'm not sure.  I don't have a good sense of what OpenSSL versions we
>> claim to support in branches older than PG13.  We made a conscious
>> decision for 1.0.1 in PG13, but I seem to recall that that discussion
>> also revealed that the version assumptions before that were quite
>> inconsistent.  Code in PG12 and before makes references to OpenSSL as
>> old as 0.9.6.  But OpenSSL 3.0.0 will reject a compat level older than
>> 0.9.8.

Well, I highly doubt that anybody has tried to compile Postgres 12
with OpenSSL 0.9.7 for a few years.  If they attempt to do so, the
compilation fails:
: note: this is the location of the previous definition
In file included from ../../src/include/common/scram-common.h:16,
 from scram-common.c:23:
../../src/include/common/sha2.h:73:9: error: unknown type name ‘SHA256_CTX’
   73 | typedef SHA256_CTX pg_sha256_ctx;

One reason is that SHA256_CTX is defined in OpenSSL 0.9.8
crypto/sha/sha.h, but this exists only in fips-1.0 in OpenSSL 0.9.7,
while we rely on SHA256_CTX in src/common/ since SCRAM exists.

Also, note that the documentation claims that the minimum version of
OpenSSL supported is 0.9.8, which is something that commit 9b7cd59 has
done, impacting Postgres 10~.  So your argument looks incorrect to me?

Honestly, I see no reason to not move on with this and remove these
deprecation warnings as proposed by the last patches sent.  (I have
run builds with 0.9.8, FWIW.)
--
Michael


signature.asc
Description: PGP signature


Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-22 Thread Peter Eisentraut

On 22.06.23 01:53, Michael Paquier wrote:

Looking at the relevant thread from 2020, this was still at the point
where we did not consider supporting 3.0 for all the stable branches
because 3.0 was in alpha:
https://www.postgresql.org/message-id/3d4afcfc-0930-1389-b9f7-59bdf11fb...@2ndquadrant.com

However, recent fixes like cab553a have made that possible, and we do
build with OpenSSL 3.0 across the whole set of stable branches.
Regarding the versions of OpenSSL supported:
- REL_13_STABLE requires 1.0.1 since 7b283d0e1.
- REL_12_STABLE and REL_11_STABLE require 0.9.8.

For 0.9.8, OPENSSL_API_COMPAT needs to be set at 0x00908000L (see
upstream's CHANGES.md).  So I don't see a reason not to do as
suggested by Andres?


The message linked to above also says:

> I'm not sure.  I don't have a good sense of what OpenSSL versions we
> claim to support in branches older than PG13.  We made a conscious
> decision for 1.0.1 in PG13, but I seem to recall that that discussion
> also revealed that the version assumptions before that were quite
> inconsistent.  Code in PG12 and before makes references to OpenSSL as
> old as 0.9.6.  But OpenSSL 3.0.0 will reject a compat level older than
> 0.9.8.





Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-22 Thread Michael Paquier
On Thu, Jun 22, 2023 at 10:02:58AM +0200, Daniel Gustafsson wrote:
> These patches LGTM from reading,

Thanks for double-checking.

> but I think the Discussion link in the commit
> messages should refer to this thread as well.

Of course.
--
Michael


signature.asc
Description: PGP signature


Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-22 Thread Daniel Gustafsson
> On 22 Jun 2023, at 01:53, Michael Paquier  wrote:

> I have tested the attached patches across 11~13 with various versions
> of OpenSSL (OPENSSL_API_COMPAT exists since 1.1.0), and this is
> working here.  Note that I don't have a MSVC environment at hand to
> test this change on Windows, still `perl -cw Solution.pm` is OK with
> it.

These patches LGTM from reading, but I think the Discussion link in the commit
messages should refer to this thread as well.

--
Daniel Gustafsson





Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-21 Thread Michael Paquier
On Wed, Jun 21, 2023 at 10:11:33AM +0200, Peter Eisentraut wrote:
> Backpatching the OPENSSL_API_COMPAT change would set the minimum OpenSSL
> version to 1.0.1, which is newer than what was so far required in those
> branches.  That is the reason we didn't do this.

Looking at the relevant thread from 2020, this was still at the point
where we did not consider supporting 3.0 for all the stable branches
because 3.0 was in alpha:
https://www.postgresql.org/message-id/3d4afcfc-0930-1389-b9f7-59bdf11fb...@2ndquadrant.com

However, recent fixes like cab553a have made that possible, and we do
build with OpenSSL 3.0 across the whole set of stable branches.
Regarding the versions of OpenSSL supported:
- REL_13_STABLE requires 1.0.1 since 7b283d0e1.
- REL_12_STABLE and REL_11_STABLE require 0.9.8.

For 0.9.8, OPENSSL_API_COMPAT needs to be set at 0x00908000L (see
upstream's CHANGES.md).  So I don't see a reason not to do as
suggested by Andres?

I have tested the attached patches across 11~13 with various versions
of OpenSSL (OPENSSL_API_COMPAT exists since 1.1.0), and this is
working here.  Note that I don't have a MSVC environment at hand to
test this change on Windows, still `perl -cw Solution.pm` is OK with
it.

What do you think about the attached patch set (one for each branch)?
--
Michael
From 469d8c07cfcf22245b59cfe4573a70ea1720b8c1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Sun, 19 Jul 2020 12:14:42 +0200
Subject: [PATCH v2] Define OPENSSL_API_COMPAT

This avoids deprecation warnings from newer OpenSSL versions (3.0.0 in
particular).

Discussion: https://www.postgresql.org/message-id/flat/FEF81714-D479-4512-839B-C769D2605F8A%40yesql.se
---
 src/include/pg_config.h.in|  4 
 src/include/pg_config.h.win32 |  4 
 configure |  6 +-
 configure.in  |  3 +++
 src/tools/msvc/Solution.pm| 10 +-
 5 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 912132dbc5..157b504ea6 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -778,6 +778,10 @@
 /* Define bytes to use libc memset(). */
 #undef MEMSET_LOOP_LIMIT
 
+/* Define to the OpenSSL API version in use. This avoids deprecation warnings
+   from newer OpenSSL versions. */
+#undef OPENSSL_API_COMPAT
+
 /* Define to the address where bug reports for this package should be sent. */
 #undef PACKAGE_BUGREPORT
 
diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index 9510b98216..7fa151f41b 100644
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -628,6 +628,10 @@
 /* Define bytes to use libc memset(). */
 #define MEMSET_LOOP_LIMIT 1024
 
+/* Define to the OpenSSL API version in use. This avoids deprecation warnings
+   from newer OpenSSL versions. */
+#define OPENSSL_API_COMPAT 0x00908000L
+
 /* Define to the address where bug reports for this package should be sent. */
 #define PACKAGE_BUGREPORT "pgsql-b...@postgresql.org"
 
diff --git a/configure b/configure
index 1577cf7ad3..dae02c8687 100755
--- a/configure
+++ b/configure
@@ -12063,7 +12063,11 @@ fi
 fi
 
 if test "$with_openssl" = yes ; then
-if test "$PORTNAME" != "win32"; then
+# Minimum required OpenSSL version is 0.9.8
+
+$as_echo "#define OPENSSL_API_COMPAT 0x00908000L" >>confdefs.h
+
+  if test "$PORTNAME" != "win32"; then
  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for CRYPTO_new_ex_data in -lcrypto" >&5
 $as_echo_n "checking for CRYPTO_new_ex_data in -lcrypto... " >&6; }
 if ${ac_cv_lib_crypto_CRYPTO_new_ex_data+:} false; then :
diff --git a/configure.in b/configure.in
index 0b44e2119f..29de083fe8 100644
--- a/configure.in
+++ b/configure.in
@@ -1269,6 +1269,9 @@ fi
 
 if test "$with_openssl" = yes ; then
   dnl Order matters!
+  # Minimum required OpenSSL version is 0.9.8
+  AC_DEFINE(OPENSSL_API_COMPAT, [0x00908000L],
+[Define to the OpenSSL API version in use. This avoids deprecation warnings from newer OpenSSL versions.])
   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])])
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 984d63f5d7..c823655ed9 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -151,6 +151,8 @@ sub GenerateFiles
 {
 	my $self = shift;
 	my $bits = $self->{platform} eq 'Win32' ? 32 : 64;
+	my $openssl_api_compat;
+	my $ac_define_openssl_api_compat_found = 0;
 
 	# Parse configure.in to get version numbers
 	open(my $c, '<', "configure.in")
@@ -167,10 +169,15 @@ sub GenerateFiles
 			$self->{numver} = sprintf("%d%04d", $1, $2 ? $2 : 0);
 			$self->{majorver} = sprintf("%d", $1);
 		}
+		elsif (/\bAC_DEFINE\(OPENSSL_API_COMPAT, \[([0-9xL]+)\]/)
+		{
+			$ac_define_openssl_api_compat_found = 1;
+			

Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-21 Thread Andres Freund
Hi,

On 2023-06-21 10:11:33 +0200, Peter Eisentraut wrote:
> On 21.06.23 09:43, Michael Paquier wrote:
> > On Wed, Jun 21, 2023 at 09:16:38AM +0200, Daniel Gustafsson wrote:
> > > Agreed, I'd be more inclined to go with OPENSSL_API_COMPAT.  If we still 
> > > get
> > > warnings with that set then I feel those warrant special consideration 
> > > rather
> > > than a blanket suppression.
> > 
> > 4d3db136 seems to be OK on REL_13_STABLE with a direct cherry-pick.
> > REL_11_STABLE and REL_12_STABLE require two different changes:
> > - pg_config.h.win32 needs to list OPENSSL_API_COMPAT.
> > - Solution.pm needs an extra #define OPENSSL_API_COMPAT in
> > GenerateFiles() whose value can be retrieved from configure.in like in
> > 13~.
> > 
> > Anything I am missing perhaps?
> 
> Backpatching the OPENSSL_API_COMPAT change would set the minimum OpenSSL
> version to 1.0.1, which is newer than what was so far required in those
> branches.  That is the reason we didn't do this.

What's the problem with just setting a different version in those branches?

Greetings,

Andres Freund




Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-21 Thread Peter Eisentraut

On 21.06.23 09:43, Michael Paquier wrote:

On Wed, Jun 21, 2023 at 09:16:38AM +0200, Daniel Gustafsson wrote:

Agreed, I'd be more inclined to go with OPENSSL_API_COMPAT.  If we still get
warnings with that set then I feel those warrant special consideration rather
than a blanket suppression.


4d3db136 seems to be OK on REL_13_STABLE with a direct cherry-pick.
REL_11_STABLE and REL_12_STABLE require two different changes:
- pg_config.h.win32 needs to list OPENSSL_API_COMPAT.
- Solution.pm needs an extra #define OPENSSL_API_COMPAT in
GenerateFiles() whose value can be retrieved from configure.in like in
13~.

Anything I am missing perhaps?


Backpatching the OPENSSL_API_COMPAT change would set the minimum OpenSSL 
version to 1.0.1, which is newer than what was so far required in those 
branches.  That is the reason we didn't do this.






Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-21 Thread Michael Paquier
On Wed, Jun 21, 2023 at 09:16:38AM +0200, Daniel Gustafsson wrote:
> Agreed, I'd be more inclined to go with OPENSSL_API_COMPAT.  If we still get
> warnings with that set then I feel those warrant special consideration rather
> than a blanket suppression.

4d3db136 seems to be OK on REL_13_STABLE with a direct cherry-pick.
REL_11_STABLE and REL_12_STABLE require two different changes:
- pg_config.h.win32 needs to list OPENSSL_API_COMPAT.
- Solution.pm needs an extra #define OPENSSL_API_COMPAT in
GenerateFiles() whose value can be retrieved from configure.in like in
13~.

Anything I am missing perhaps?
--
Michael


signature.asc
Description: PGP signature


Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-21 Thread Daniel Gustafsson
> On 21 Jun 2023, at 07:44, Andres Freund  wrote:
> On 2023-06-21 11:53:44 +0900, Michael Paquier wrote:

>> I have been annoyed by these in the past when doing backpatches, as
>> this creates some noise, and the only place where this counts is
>> sha2_openssl.c.  Thoughts about doing something like the attached for
>> ~13?
> 
> Wouldn't the proper fix be to backpatch 4d3db13621b?

Agreed, I'd be more inclined to go with OPENSSL_API_COMPAT.  If we still get
warnings with that set then I feel those warrant special consideration rather
than a blanket suppression.

--
Daniel Gustafsson





Re: Remove deprecation warnings when compiling PG ~13 with OpenSSL 3.0~

2023-06-20 Thread Andres Freund
Hi,

On 2023-06-21 11:53:44 +0900, Michael Paquier wrote:
> Compiling Postgres up to 13 with OpenSSL 3.0 leads to a couple of
> compilation warnings with what OpenSSL considers as deprecated, like:
> sha2_openssl.c: In function pg_sha384_init
> sha2_openssl.c:70:9: warning: SHA384_Init is deprecated =
> Since OpenSSL 3.0 [-Wdeprecated-declarations]
>70 | SHA384_Init((SHA512_CTX *) ctx);
>   | ^~~
> /usr/include/openssl/sha.h:119:27: note: declared here
>   119 | OSSL_DEPRECATEDIN_3_0 int SHA384_Init(SHA512_CTX *c);
> 
> I was looking at the code of OpenSSL to see if there would be a way to
> silenced these, and found about OPENSSL_SUPPRESS_DEPRECATED.
> 
> I have been annoyed by these in the past when doing backpatches, as
> this creates some noise, and the only place where this counts is
> sha2_openssl.c.  Thoughts about doing something like the attached for
> ~13?

Wouldn't the proper fix be to backpatch 4d3db13621b? Just suppressing all
deprecations doesn't strike me as particularly wise, especially because we've
chosen a different path for 14+?

Greetings,

Andres Freund