Removal of support for OpenSSL 0.9.8 and 1.0.0

Started by Michael Paquierabout 6 years ago15 messages
#1Michael Paquier
michael@paquier.xyz
3 attachment(s)

Hi all,

So, I have been looking at what we could clean up by removing support
for OpenSSL 0.9.8 and 1.0.0. Here are my notes:
1) SSL_get_current_compression exists before 0.9.8, and we don't
actually make use of its configure check. So I think that it could
just be removed, as per patch 0001.
2) SSL_clear_options exists since 0.9.8, so we should not even need the
configure checks. Still, it is defined as a macro from 0.9.8 to
1.0.2, and then it has switched to a function in 1.1.0, so we fail to
detect it on past versions of OpenSSL (LibreSSL has forked at the
point of 1.0.1g, so it uses only a macro). There is an extra take
though. Daniel has mentioned that here:
/messages/by-id/98F7F99E-1129-41D8-B86B-FE3B1E286881@yesql.se
Note also that a364dfa has also added a tweak in fe-secure-openssl.c
for cases where we don't have SSL_clear_options(). This refers to
NetBSD 5.1. Peter, do you recall which version of LibreSSL was
involved here? From a lookup at the code of LibreSSL, the function
has always been around as a macro. Anyway, 0002 is more subject to
discussions regarding this last point.

Then comes the actual changes across the major versions:
1) SSL_CTX_set_options, which has been added in 0.9.8f, so this could
get removed in be-secure-openssl.c.
2) These functions are new as of 1.0.2:
X509_get_signature_nid
3) These functions are new as of 1.1.0:
- SSL_CTX_set_min_proto_version, SSL_CTX_set_max_proto_version (still
for the fallback functions we have it sounds better to keep the extra
checks on the TLSvXX definitions.)
- BIO_meth_new
- BIO_get_data
- OPENSSL_init_ssl
- ASN1_STRING_get0_data
From the point of view of the code, the cleanup is not actually that
amazing I am afraid, a jump directly to 1.1.0 would remove much more
because the breakages were wider when we integrated it. Anyway, those
cleanups are part of 0003. I thought that this would have resulted in
more cleanup :(

I think that 0001 is a fix we need to do, 0002 is debatable still
LibreSSL should support it and we fail to detect SSL_clear_options
properly, and 0003 does not really much additional value. Or we put
into the balance for 0003 the argument that we can use TLSv1.2 for all
configurations, which is safer but we have the configuration to
enforce it.

Thoughts?
--
Michael

Attachments:

0001-Remove-configure-checks-for-SSL_get_current_compress.patchtext/x-diff; charset=us-asciiDownload
From 463046ea6f27af59035e15135191ef87d3c9ec29 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Thu, 5 Dec 2019 16:41:59 +0900
Subject: [PATCH 1/3] Remove configure checks for SSL_get_current_compression
 in OpenSSL

This function is supported down to OpenSSL 0.9.8, which is the oldest
version supported on HEAD.
---
 configure                     | 2 +-
 configure.in                  | 2 +-
 src/include/pg_config.h.in    | 3 ---
 src/include/pg_config.h.win32 | 3 ---
 src/include/port.h            | 4 ----
 5 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/configure b/configure
index 1d88983b34..56c4aaa95b 100755
--- a/configure
+++ b/configure
@@ -12094,7 +12094,7 @@ else
 fi
 
   fi
-  for ac_func in SSL_clear_options SSL_get_current_compression X509_get_signature_nid
+  for ac_func in SSL_clear_options X509_get_signature_nid
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.in b/configure.in
index a2cb20b5e3..9fd9c390e6 100644
--- a/configure.in
+++ b/configure.in
@@ -1186,7 +1186,7 @@ if test "$with_openssl" = yes ; then
      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
-  AC_CHECK_FUNCS([SSL_clear_options SSL_get_current_compression X509_get_signature_nid])
+  AC_CHECK_FUNCS([SSL_clear_options X509_get_signature_nid])
   # 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
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index c208dcdfc7..0d77f2aafd 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -515,9 +515,6 @@
 /* Define to 1 if you have the `SSL_clear_options' function. */
 #undef HAVE_SSL_CLEAR_OPTIONS
 
-/* Define to 1 if you have the `SSL_get_current_compression' function. */
-#undef HAVE_SSL_GET_CURRENT_COMPRESSION
-
 /* Define to 1 if stdbool.h conforms to C99. */
 #undef HAVE_STDBOOL_H
 
diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index 6c98ef4916..467fb89ee6 100644
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -373,9 +373,6 @@
 /* Define to 1 if you have the `SSL_clear_options' function. */
 #define HAVE_SSL_CLEAR_OPTIONS 1
 
-/* Define to 1 if you have the `SSL_get_current_compression' function. */
-#define HAVE_SSL_GET_CURRENT_COMPRESSION 1
-
 /* Define to 1 if stdbool.h conforms to C99. */
 #define HAVE_STDBOOL_H 1
 
diff --git a/src/include/port.h b/src/include/port.h
index 10dcb5f0a6..bfd2e2759f 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -446,10 +446,6 @@ extern void unsetenv(const char *name);
 extern void srandom(unsigned int seed);
 #endif
 
-#ifndef HAVE_SSL_GET_CURRENT_COMPRESSION
-#define SSL_get_current_compression(x) 0
-#endif
-
 #ifndef HAVE_DLOPEN
 extern void *dlopen(const char *file, int mode);
 extern void *dlsym(void *handle, const char *symbol);
-- 
2.24.0

0002-Remove-configure-checks-for-SSL_clear_options-in-Ope.patchtext/x-diff; charset=us-asciiDownload
From 4ad5e60ace01b5360a031ae75b400a565960b591 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Thu, 5 Dec 2019 16:47:27 +0900
Subject: [PATCH 2/3] Remove configure checks for SSL_clear_options in OpenSSL

This function is supported down to OpenSSL 0.9.8, which is the oldest
version supported on HEAD.  Note that it is defined as a macro from
OpenSSL 0.9.8, where it has been introduced, to 1.0.2, and that it is a
function in 1.1.0 and newer versions.
---
 configure                                | 10 +++++-----
 configure.in                             |  3 ++-
 src/include/pg_config.h.in               |  3 ---
 src/include/pg_config.h.win32            |  3 ---
 src/interfaces/libpq/fe-secure-openssl.c |  9 ---------
 5 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/configure b/configure
index 56c4aaa95b..7cc159c332 100755
--- a/configure
+++ b/configure
@@ -12094,13 +12094,13 @@ else
 fi
 
   fi
-  for ac_func in SSL_clear_options X509_get_signature_nid
+  # Function introduced in OpenSSL 1.0.2
+  for ac_func in X509_get_signature_nid
 do :
-  as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
-ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
-if eval test \"x\$"$as_ac_var"\" = x"yes"; then :
+  ac_fn_c_check_func "$LINENO" "X509_get_signature_nid" "ac_cv_func_X509_get_signature_nid"
+if test "x$ac_cv_func_X509_get_signature_nid" = xyes; then :
   cat >>confdefs.h <<_ACEOF
-#define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1
+#define HAVE_X509_GET_SIGNATURE_NID 1
 _ACEOF
 
 fi
diff --git a/configure.in b/configure.in
index 9fd9c390e6..d56fed1096 100644
--- a/configure.in
+++ b/configure.in
@@ -1186,7 +1186,8 @@ if test "$with_openssl" = yes ; then
      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
-  AC_CHECK_FUNCS([SSL_clear_options X509_get_signature_nid])
+  # Function introduced in OpenSSL 1.0.2
+  AC_CHECK_FUNCS([X509_get_signature_nid])
   # 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
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 0d77f2aafd..050c48b108 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -512,9 +512,6 @@
 /* Define to 1 if you have the `srandom' function. */
 #undef HAVE_SRANDOM
 
-/* Define to 1 if you have the `SSL_clear_options' function. */
-#undef HAVE_SSL_CLEAR_OPTIONS
-
 /* Define to 1 if stdbool.h conforms to C99. */
 #undef HAVE_STDBOOL_H
 
diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index 467fb89ee6..09cedd0bda 100644
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -370,9 +370,6 @@
 /* Define to 1 if you have the `srandom' function. */
 /* #undef HAVE_SRANDOM */
 
-/* Define to 1 if you have the `SSL_clear_options' function. */
-#define HAVE_SSL_CLEAR_OPTIONS 1
-
 /* Define to 1 if stdbool.h conforms to C99. */
 #define HAVE_STDBOOL_H 1
 
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index cba81f63c0..c71da75cfd 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1198,17 +1198,8 @@ initialize_SSL(PGconn *conn)
 #ifdef SSL_OP_NO_COMPRESSION
 	if (conn->sslcompression && conn->sslcompression[0] == '0')
 		SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
-
-	/*
-	 * Mainline OpenSSL introduced SSL_clear_options() before
-	 * SSL_OP_NO_COMPRESSION, so this following #ifdef should not be
-	 * necessary, but some old NetBSD version have a locally modified libssl
-	 * that has SSL_OP_NO_COMPRESSION but not SSL_clear_options().
-	 */
-#ifdef HAVE_SSL_CLEAR_OPTIONS
 	else
 		SSL_clear_options(conn->ssl, SSL_OP_NO_COMPRESSION);
-#endif
 #endif
 
 	return 0;
-- 
2.24.0

0003-Remove-code-older-than-OpenSSL-0.9.8-and-1.0.0.patchtext/x-diff; charset=us-asciiDownload
From f96d45a2589c8412b12233a5a4a6c820f5b2b35a Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Thu, 5 Dec 2019 17:26:05 +0900
Subject: [PATCH 3/3] Remove code older than OpenSSL 0.9.8 and 1.0.0

---
 doc/src/sgml/installation.sgml           | 2 +-
 src/backend/libpq/be-secure-openssl.c    | 2 --
 src/interfaces/libpq/fe-secure-openssl.c | 5 +----
 src/test/ssl/t/SSLServer.pm              | 4 ----
 4 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 9c10a897f1..d4904bf5a0 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -254,7 +254,7 @@ su - postgres
       encrypted client connections.  <productname>OpenSSL</productname> is
       also required for random number generation on platforms that do not
       have <filename>/dev/urandom</filename> (except Windows).  The minimum
-      version required is 0.9.8.
+      version required is 1.0.1.
      </para>
     </listitem>
 
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 629919cc6e..c065b1cd8b 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -223,9 +223,7 @@ be_tls_init(bool isServerStart)
 	}
 
 	/* disallow SSL session tickets */
-#ifdef SSL_OP_NO_TICKET			/* added in OpenSSL 0.9.8f */
 	SSL_CTX_set_options(context, SSL_OP_NO_TICKET);
-#endif
 
 	/* disallow SSL session caching, too */
 	SSL_CTX_set_session_cache_mode(context, SSL_SESS_CACHE_OFF);
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index c71da75cfd..14d781d5ca 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1192,15 +1192,12 @@ initialize_SSL(PGconn *conn)
 		SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, verify_cb);
 
 	/*
-	 * Set compression option if the OpenSSL version used supports it (from
-	 * 1.0.0 on).
+	 * Set compression option if necessary.
 	 */
-#ifdef SSL_OP_NO_COMPRESSION
 	if (conn->sslcompression && conn->sslcompression[0] == '0')
 		SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
 	else
 		SSL_clear_options(conn->ssl, SSL_OP_NO_COMPRESSION);
-#endif
 
 	return 0;
 }
diff --git a/src/test/ssl/t/SSLServer.pm b/src/test/ssl/t/SSLServer.pm
index 26b5964f4f..005955a2ff 100644
--- a/src/test/ssl/t/SSLServer.pm
+++ b/src/test/ssl/t/SSLServer.pm
@@ -132,10 +132,6 @@ sub configure_test_server_for_ssl
 	print $conf "listen_addresses='$serverhost'\n";
 	print $conf "log_statement=all\n";
 
-	# Accept even old TLS versions so that builds with older OpenSSL
-	# can run the test suite.
-	print $conf "ssl_min_protocol_version='TLSv1'\n";
-
 	# enable SSL and set up server key
 	print $conf "include 'sslconfig.conf'\n";
 
-- 
2.24.0

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#1)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

On 5 Dec 2019, at 09:32, Michael Paquier <michael@paquier.xyz> wrote:

From the point of view of the code, the cleanup is not actually that
amazing I am afraid, a jump directly to 1.1.0 would remove much more
because the breakages were wider when we integrated it. Anyway, those
cleanups are part of 0003. I thought that this would have resulted in
more cleanup :(

While expected, it's still disappointing. Until we can drop 1.0.2 there isn't
too much to gain, and that will likely be reasonably far into the future given
that it's the final version that can run the FIPS module.

I think that 0001 is a fix we need to do

+1

0002 is debatable still LibreSSL should support it and we fail to detect
SSL_clear_options properly,

LibreSSL has SSL_clear_options in all versions, as does OpenSSL even at the
level of support we have as of now. The only issue with 0002 is support for
older NetBSD releases, as is documented in the comment and commit message.

NetBSD 5.x shipped a custom OpenSSL identified as 0.9.9, which is a version of
their own invention. NetBSD 6.0 (which shipped in October 2012) ships 1.0.1u,
which has SSL_clear_options as well as SSL_OP_NO_COMPRESSION. So, this patch
is not really debateable if we are dropping support for 0.9.8 and 1.0.0.

opossum is the only animal in the buildfarm on NetBSD 5, and it has been silent
for close to a year now (coypu is on NetBSD 8). Requiring opossum to build
without OpenSSL (if/when) it comes back seems a reasonable ask. NetBSD 5.x has
been EOL for quite some time too.

+1 on applying this instead of trying to fix the autoconf check.

0003 does not really much additional value. Or we put
into the balance for 0003 the argument that we can use TLSv1.2 for all
configurations, which is safer but we have the configuration to
enforce it.

I think the TLSv1.2 argument is the most compelling one, the changes are a wash
in terms of code maintainability. Raising the minimum supported version
doesn't really have any downsides though, and should be quite uncontroversial
(and as noted in the other thread, prariedog and gaur are ready for the change
so buildfarm breakage should be limited to an animal that doesnt build
anymore). Overall, +1.

cheers ./daniel

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#2)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

Daniel Gustafsson <daniel@yesql.se> writes:

On 5 Dec 2019, at 09:32, Michael Paquier <michael@paquier.xyz> wrote:
From the point of view of the code, the cleanup is not actually that
amazing I am afraid, a jump directly to 1.1.0 would remove much more
because the breakages were wider when we integrated it. Anyway, those
cleanups are part of 0003. I thought that this would have resulted in
more cleanup :(

While expected, it's still disappointing. Until we can drop 1.0.2 there isn't
too much to gain, and that will likely be reasonably far into the future given
that it's the final version that can run the FIPS module.

Yeah; also as mentioned in the other thread, 1.0.1 is still in use
in RHEL 6, so it's hard to consider dropping that for at least another
year. I concur with the conclusion that we can stop worrying about
NetBSD 5, though.

I see nothing to object to in this patch set.

regards, tom lane

#4Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#3)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

On Thu, Dec 05, 2019 at 10:38:55AM -0500, Tom Lane wrote:

Yeah; also as mentioned in the other thread, 1.0.1 is still in use
in RHEL 6, so it's hard to consider dropping that for at least another
year. I concur with the conclusion that we can stop worrying about
NetBSD 5, though.

Thanks. Another argument in favor of dropping 1.0.0 and 0.9.8 is that
it is a pain to check an OpenSSL patch across that many versions,
multiplied by the number of Postgres branches in need of patching :)

I see nothing to object to in this patch set.

I have applied 0001 on HEAD for now as that's a simple cleanup (I
would not backpatch that though). For 0002, I would prefer be sure
that we reach the right conclusion. My take is to:
1) Apply 0002 only on HEAD to remove the check for clear_options.
2) Apply something like Daniel's patch in [1]/messages/by-id/3C636E88-44C7-40C6-ABA3-1B236E0A74DE@yesql.se -- Michael for REL_12_STABLE and
REL_11_STABLE as we care about this routine since e3bdb2d to not mess
up with past versions of NetBSD which worked previously on our
released branches. (The patch looks fine at quick glance and I
haven't tested it yet)

[1]: /messages/by-id/3C636E88-44C7-40C6-ABA3-1B236E0A74DE@yesql.se -- Michael
--
Michael

#5Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#4)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

On Fri, Dec 06, 2019 at 10:33:23AM +0900, Michael Paquier wrote:

Thanks. Another argument in favor of dropping 1.0.0 and 0.9.8 is that
it is a pain to check an OpenSSL patch across that many versions,
multiplied by the number of Postgres branches in need of patching :)

I have done nothing for 0003 yet. Let's wait a bit and see if others
have more arguments in favor of it or not.

I have applied 0001 on HEAD for now as that's a simple cleanup (I
would not backpatch that though). For 0002, I would prefer be sure
that we reach the right conclusion. My take is to:
1) Apply 0002 only on HEAD to remove the check for clear_options.
2) Apply something like Daniel's patch in [1] for REL_12_STABLE and
REL_11_STABLE as we care about this routine since e3bdb2d to not mess
up with past versions of NetBSD which worked previously on our
released branches. (The patch looks fine at quick glance and I
haven't tested it yet)

0002 is now applied, and did things as described in the above
paragraph.
--
Michael

#6Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#4)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

On 6 Dec 2019, at 02:33, Michael Paquier <michael@paquier.xyz> wrote:

Another argument in favor of dropping 1.0.0 and 0.9.8 is that
it is a pain to check an OpenSSL patch across that many versions,
multiplied by the number of Postgres branches in need of patching :)

That is indeed a very good argument.

cheers ./daniel

#7Michael Paquier
michael@paquier.xyz
In reply to: Daniel Gustafsson (#6)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

On Fri, Dec 06, 2019 at 09:21:55AM +0100, Daniel Gustafsson wrote:

On 6 Dec 2019, at 02:33, Michael Paquier <michael@paquier.xyz> wrote:

Another argument in favor of dropping 1.0.0 and 0.9.8 is that
it is a pain to check an OpenSSL patch across that many versions,
multiplied by the number of Postgres branches in need of patching :)

That is indeed a very good argument.

Sorry for letting this thread down for a couple of weeks, but I was
hesitating to apply the last patch of the series as the cleanup of the
code related to OpenSSL 0.9.8 and 1.0.0 is not that much. An extra
argument in favor of the removal is that this can allow more shaving
of past Python versions, as proposed by Peter here:
/messages/by-id/98b69261-298c-13d2-f34d-836fd9c29b21@2ndquadrant.com

So, let's do it. I don't think that I'll be able to do anything this
week about it, but that should be fine by the end of next week. Are
there any objections or comments?

For now, please note that I have added an entry in the CF app:
https://commitfest.postgresql.org/26/2413/
--
Michael

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#7)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

Michael Paquier <michael@paquier.xyz> writes:

Sorry for letting this thread down for a couple of weeks, but I was
hesitating to apply the last patch of the series as the cleanup of the
code related to OpenSSL 0.9.8 and 1.0.0 is not that much. An extra
argument in favor of the removal is that this can allow more shaving
of past Python versions, as proposed by Peter here:
/messages/by-id/98b69261-298c-13d2-f34d-836fd9c29b21@2ndquadrant.com

So, let's do it.

FWIW, I'm not sure I see why there's a connection between moving up
the minimum Python version and minimum OpenSSL version. Nobody is
installing bleeding-edge Postgres on RHEL5, not even me ;-), so I
don't especially buy Peter's line of reasoning.

I'm perfectly okay with doing both things in HEAD, I just don't
see that doing one is an argument for or against doing the other.

regards, tom lane

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#7)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

Michael Paquier <michael@paquier.xyz> writes:

For now, please note that I have added an entry in the CF app:
https://commitfest.postgresql.org/26/2413/

BTW, the referenced patch only removes the configure check for
SSL_get_current_compression, which is fine, but is that even
moving any goalposts? The proposed commit message says the
function exists down to 0.9.8, which is already our minimum.
There's nothing to debate here if that statement is accurate.

regards, tom lane

#10Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#9)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

On Thu, Jan 02, 2020 at 09:30:42AM -0500, Tom Lane wrote:

BTW, the referenced patch only removes the configure check for
SSL_get_current_compression, which is fine, but is that even
moving any goalposts? The proposed commit message says the
function exists down to 0.9.8, which is already our minimum.
There's nothing to debate here if that statement is accurate.

Oops, sorry for the confusion. There are three patches in the set.
0001 has been already applied as of 28f4bba, and 0002 as of 7d0bcb0
(backpatched with a different fix from Daniel to allow the build to
still work). The actual patch I am proposing to finish merging is
0003 as posted here, which is the remaining piece:
/messages/by-id/20191205083252.GE5064@paquier.xyz
--
Michael

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#10)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

Michael Paquier <michael@paquier.xyz> writes:

On Thu, Jan 02, 2020 at 09:30:42AM -0500, Tom Lane wrote:

BTW, the referenced patch only removes the configure check for
SSL_get_current_compression

The actual patch I am proposing to finish merging is
0003 as posted here, which is the remaining piece:
/messages/by-id/20191205083252.GE5064@paquier.xyz

Ah. The CF app doesn't understand that (and hence the cfbot ditto),
so you might want to repost just the currently-proposed patch to get
the cfbot to try it.

regards, tom lane

#12Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#11)
1 attachment(s)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

On Thu, Jan 02, 2020 at 11:45:37PM -0500, Tom Lane wrote:

Ah. The CF app doesn't understand that (and hence the cfbot ditto),
so you might want to repost just the currently-proposed patch to get
the cfbot to try it.

Yes, let's do that. Here you go with a v2. While on it, I have
noticed in the docs a mention to OpenSSL 1.0.0 regarding our
sslcompression parameter in libpq, so a paragraph can be removed.
--
Michael

Attachments:

v2-0001-Remove-code-older-than-OpenSSL-0.9.8-and-1.0.0.patchtext/x-diff; charset=us-asciiDownload
From d693a4d78b80d7927d1859e95cf0a3b16766240a Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Fri, 3 Jan 2020 15:47:26 +0900
Subject: [PATCH v2] Remove code older than OpenSSL 0.9.8 and 1.0.0

---
 doc/src/sgml/installation.sgml           | 2 +-
 doc/src/sgml/libpq.sgml                  | 4 ----
 src/backend/libpq/be-secure-openssl.c    | 2 --
 src/interfaces/libpq/fe-secure-openssl.c | 5 +----
 src/test/ssl/t/SSLServer.pm              | 4 ----
 5 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 9c10a897f1..d4904bf5a0 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -254,7 +254,7 @@ su - postgres
       encrypted client connections.  <productname>OpenSSL</productname> is
       also required for random number generation on platforms that do not
       have <filename>/dev/urandom</filename> (except Windows).  The minimum
-      version required is 0.9.8.
+      version required is 1.0.1.
      </para>
     </listitem>
 
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 66b09da06f..64cff49c4d 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1608,10 +1608,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
         compression by default, and many operating system distributions
         disable it in prior versions as well, so setting this parameter to on
         will not have any effect if the server does not accept compression.
-        On the other hand, <productname>OpenSSL</productname> before 1.0.0
-        does not support disabling compression, so this parameter is ignored
-        with those versions, and whether compression is used depends on the
-        server.
        </para>
 
        <para>
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 7ad32116ea..62f1fcab2b 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -223,9 +223,7 @@ be_tls_init(bool isServerStart)
 	}
 
 	/* disallow SSL session tickets */
-#ifdef SSL_OP_NO_TICKET			/* added in OpenSSL 0.9.8f */
 	SSL_CTX_set_options(context, SSL_OP_NO_TICKET);
-#endif
 
 	/* disallow SSL session caching, too */
 	SSL_CTX_set_session_cache_mode(context, SSL_SESS_CACHE_OFF);
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index ce8e252c09..0e84fc8ac6 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1192,15 +1192,12 @@ initialize_SSL(PGconn *conn)
 		SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, verify_cb);
 
 	/*
-	 * Set compression option if the OpenSSL version used supports it (from
-	 * 1.0.0 on).
+	 * Set compression option if necessary.
 	 */
-#ifdef SSL_OP_NO_COMPRESSION
 	if (conn->sslcompression && conn->sslcompression[0] == '0')
 		SSL_set_options(conn->ssl, SSL_OP_NO_COMPRESSION);
 	else
 		SSL_clear_options(conn->ssl, SSL_OP_NO_COMPRESSION);
-#endif
 
 	return 0;
 }
diff --git a/src/test/ssl/t/SSLServer.pm b/src/test/ssl/t/SSLServer.pm
index 26b5964f4f..005955a2ff 100644
--- a/src/test/ssl/t/SSLServer.pm
+++ b/src/test/ssl/t/SSLServer.pm
@@ -132,10 +132,6 @@ sub configure_test_server_for_ssl
 	print $conf "listen_addresses='$serverhost'\n";
 	print $conf "log_statement=all\n";
 
-	# Accept even old TLS versions so that builds with older OpenSSL
-	# can run the test suite.
-	print $conf "ssl_min_protocol_version='TLSv1'\n";
-
 	# enable SSL and set up server key
 	print $conf "include 'sslconfig.conf'\n";
 
-- 
2.24.1

#13Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#12)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

On 3 Jan 2020, at 07:49, Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Jan 02, 2020 at 11:45:37PM -0500, Tom Lane wrote:

Ah. The CF app doesn't understand that (and hence the cfbot ditto),
so you might want to repost just the currently-proposed patch to get
the cfbot to try it.

Yes, let's do that. Here you go with a v2. While on it, I have
noticed in the docs a mention to OpenSSL 1.0.0 regarding our
sslcompression parameter in libpq, so a paragraph can be removed.

LGTM, switching to ready for committer.

cheers ./daniel

#14Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#8)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

On Thu, Jan 02, 2020 at 09:22:47AM -0500, Tom Lane wrote:

FWIW, I'm not sure I see why there's a connection between moving up
the minimum Python version and minimum OpenSSL version. Nobody is
installing bleeding-edge Postgres on RHEL5, not even me ;-), so I
don't especially buy Peter's line of reasoning.

It seems to me that the line of reasoning was to consider RHEL5 in the
garbage for all our dependencies, in a consistent way.

I'm perfectly okay with doing both things in HEAD, I just don't
see that doing one is an argument for or against doing the other.

Yes, right. That would be the case if we had direct dependencies
between both, but that has never been the case AFAIK.
--
Michael

#15Michael Paquier
michael@paquier.xyz
In reply to: Daniel Gustafsson (#13)
Re: Removal of support for OpenSSL 0.9.8 and 1.0.0

On Fri, Jan 03, 2020 at 10:57:54PM +0100, Daniel Gustafsson wrote:

LGTM, switching to ready for committer.

Thanks Daniel. I have looked at that stuff again, and committed the
patch.
--
Michael