[PATCH] add ssl_protocols configuration option

Started by Dag-Erling Smørgravabout 11 years ago41 messages
6 attachment(s)

The attached patches add an ssl_protocols configuration option which
control which versions of SSL or TLS the server will use. The syntax is
similar to Apache's SSLProtocols directive, except that the list is
colon-separated instead of whitespace-separated, although that is easy
to change if it proves unpopular.

Summary of the patch:

- In src/backend/libpq/be-secure.c:
- Add an SSLProtocols variable for the option.
- Add a function, parse_SSL_protocols(), that parses an ssl_protocols
string and returns a bitmask suitable for SSL_CTX_set_options().
- Change initialize_SSL() to call parse_SSL_protocols() and pass the
result to SSL_CTX_set_options().
- In src/backend/utils/misc/guc.c:
- Add an extern declaration for SSLProtocols.
- Add an entry in the ConfigureNamesString array for the
ssl_protocols option.
- In src/backend/utils/misc/postgresql.conf.sample:
- Add a sample ssl_protocols line.
- In doc/src/sgml/config.sgml:
- Document the ssl_protocols option.

The file names are slightly different in 9.5, since be-secure.c was
split in two and the declaration was moved into libpq.h.

The default is "ALL:-SSLv2" in 9.0-9.3 and "ALL:-SSL" in 9.4 and up.
This corresponds to the current hardcoded values, so the default
behavior is unchanged, but the admin now has the option to select a
different settings, e.g. if a serious vulnerability is found in TLS 1.0.

Attachments:

postgresql-master-ssl-protocols.difftext/x-patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6ee17d8..7233a73 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1027,6 +1027,34 @@ include_dir 'conf.d'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-ssl-protocols" xreflabel="ssl_protocols">
+      <term><varname>ssl_protocols</varname> (<type>string</type>)</term>
+      <indexterm>
+       <primary><varname>ssl_protocols</> configuration parameter</primary>
+      </indexterm>
+      <listitem>
+       <para>
+        Specifies a colon-separated list of <acronym>SSL</> protocols that are
+        allowed to be used on secure connections. Each entry in the list can
+        be prefixed by a <literal>+</> (add to the current list)
+        or <literal>-</> (remove from the current list). If no prefix is used,
+        the list is replaced with the specified protocol.
+       </para>
+       <para>
+        The full list of supported protocols can be found in the
+        the <application>openssl</> manual page.  In addition to the names of
+        individual protocols, the following keywords can be
+        used: <literal>ALL</> (all protocols supported by the underlying
+        crypto library), <literal>SSL</> (all supported versions
+        of <acronym>SSL</>) and <literal>TLS</> (all supported versions
+        of <acronym>TLS</>).
+       </para>
+       <para>
+        The default is <literal>ALL:-SSL</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-ssl-ciphers" xreflabel="ssl_ciphers">
       <term><varname>ssl_ciphers</varname> (<type>string</type>)
       <indexterm>
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index b05364c..f440b77 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -87,6 +87,7 @@ static int	verify_cb(int, X509_STORE_CTX *);
 static void info_cb(const SSL *ssl, int type, int args);
 static void initialize_ecdh(void);
 static const char *SSLerrmessage(void);
+static long parse_SSL_protocols(const char *str);
 
 /* are we in the middle of a renegotiation? */
 static bool in_ssl_renegotiation = false;
@@ -245,15 +246,16 @@ be_tls_init(void)
 							SSLerrmessage())));
 	}
 
-	/* set up ephemeral DH keys, and disallow SSL v2/v3 while at it */
+	/* set up ephemeral DH keys */
 	SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb);
-	SSL_CTX_set_options(SSL_context,
-						SSL_OP_SINGLE_DH_USE |
-						SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
+	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE);
 
 	/* set up ephemeral ECDH keys */
 	initialize_ecdh();
 
+	/* set up the allowed protocol list */
+	SSL_CTX_set_options(SSL_context, parse_SSL_protocols(SSLProtocols));
+
 	/* set up the allowed cipher list */
 	if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
 		elog(FATAL, "could not set the cipher list (no valid ciphers available)");
@@ -1053,3 +1055,106 @@ SSLerrmessage(void)
 	snprintf(errbuf, sizeof(errbuf), _("SSL error code %lu"), errcode);
 	return errbuf;
 }
+
+
+/*
+ *	Parse the SSL allowed protocol list
+ *
+ *	The logic here is inverted.  OpenSSL does not take a list of
+ *	protocols to use, but a list of protocols to avoid.  We use the
+ *	same bits with the opposite meaning, then invert the result.
+ */
+
+#define SSL_PROTO_SSLv2		SSL_OP_NO_SSLv2
+#define SSL_PROTO_SSLv3		SSL_OP_NO_SSLv3
+#define SSL_PROTO_SSL		(SSL_PROTO_SSLv2 | SSL_PROTO_SSLv3)
+#define SSL_PROTO_TLSv1		SSL_OP_NO_TLSv1
+#ifdef SSL_OP_NO_TLSv1_1
+#define SSL_PROTO_TLSv1_1	SSL_OP_NO_TLSv1_1
+#else
+#define SSL_PROTO_TLSv1_1	0
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+#define SSL_PROTO_TLSv1_2	SSL_OP_NO_TLSv1_2
+#else
+#define SSL_PROTO_TLSv1_2	0
+#endif
+#define SSL_PROTO_TLS		(SSL_PROTO_TLSv1 | SSL_PROTO_TLSv1_1 | SSL_PROTO_TLSv1_2)
+#define SSL_PROTO_ALL		(SSL_PROTO_SSL | SSL_PROTO_TLS)
+#define SSL_PROTO_NONE		0
+
+#define str_is_token(str, tok, len) \
+	(len == sizeof(tok) - 1 && pg_strncasecmp(str, tok, len) == 0)
+
+static long
+parse_SSL_protocols(const char *str)
+{
+	long current, result;
+	const char *p, *q;
+	int action;
+
+	/*
+	 * Iterate over the colon-separated list of protocols.  If a protocol is
+	 * preceded by a +, it is added to the list.  If it is preceded by a -, it
+	 * is removed from the list.  If it is not preceded by anything, the list
+	 * is set to exactly that protocol.  "ALL" can be used to indicate all
+	 * protocols, "NONE" to indicate no protocols, "SSL" to indicate all SSL
+	 * protocols and "TLS" to indicate all TLS protocols.  The parser accepts
+	 * "SSLv2", "SSLv3" and "SSL", but they are removed after parsing.
+	 */
+	result = SSL_PROTO_NONE;
+	for (p = q = str; *q; p = q + 1) {
+		for (q = p; *q && *q != ':'; ++q)
+			/* nothing */ ;
+		if (*p == '-' || *p == '+')
+			action = *p++;
+		else
+			action = '=';
+		if (str_is_token(p, "ALL", q - p))
+			current = SSL_PROTO_ALL;
+		else if (str_is_token(p, "NONE", q - p))
+			current = SSL_PROTO_NONE;
+		else if (str_is_token(p, SSL_TXT_SSLV2, q - p))
+			current = SSL_PROTO_SSLv2;
+		else if (str_is_token(p, SSL_TXT_SSLV3, q - p))
+			current = SSL_PROTO_SSLv3;
+		else if (str_is_token(p, "SSL", q - p))
+			current = SSL_PROTO_SSL;
+		else if (str_is_token(p, SSL_TXT_TLSV1, q - p))
+			current = SSL_PROTO_TLSv1;
+#ifdef SSL_OP_NO_TLSv1_1
+		else if (str_is_token(p, SSL_TXT_TLSV1_1, q - p))
+			current = SSL_PROTO_TLSv1_1;
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+		else if (str_is_token(p, SSL_TXT_TLSV1_2, q - p))
+			current = SSL_PROTO_TLSv1_2;
+#endif
+		else if (str_is_token(p, "TLS", q - p))
+			current = SSL_PROTO_TLS;
+		else
+			elog(FATAL, "invalid SSL protocol list");
+		switch (action) {
+		case '+':
+			result |= current;
+			break;
+		case '-':
+			result &= ~current;
+			break;
+		default:
+			result = current;
+			break;
+		}
+	}
+	/* forcibly disallow SSLv2 and SSLv3 */
+	if (result & SSL_PROTO_SSL) {
+		elog(WARNING, "removing SSLv2 and SSLv3 from SSL protocol list");
+		result &= ~SSL_PROTO_SSL;
+	}
+	/* check the result */
+	if (result == 0)
+		elog(FATAL, "could not set the protocol list (no valid protocols available)");
+	elog(DEBUG2, "enabling SSL protocols: %lx", result);
+	/* return the inverse */
+	return (SSL_PROTO_ALL & ~result);
+}
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 41ec1ad..a5a5f3f 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -52,7 +52,8 @@ int			ssl_renegotiation_limit;
 bool ssl_loaded_verify_locations = false;
 #endif
 
-/* GUC variable controlling SSL cipher list */
+/* GUC variables controlling SSL protocol and cipher list */
+char	   *SSLProtocols = NULL;
 char	   *SSLCipherSuites = NULL;
 
 /* GUC variable for default ECHD curve. */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index dca533a..3a34be1 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3239,6 +3239,21 @@ static struct config_string ConfigureNamesString[] =
 	},
 
 	{
+		{"ssl_protocols", PGC_POSTMASTER, CONN_AUTH_SECURITY,
+			gettext_noop("Sets the list of allowed SSL protocols."),
+			NULL,
+			GUC_SUPERUSER_ONLY
+		},
+		&SSLProtocols,
+#ifdef USE_SSL
+		"ALL:-SSL",
+#else
+		"none",
+#endif
+		NULL, NULL, NULL
+	},
+
+	{
 		{"ssl_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY,
 			gettext_noop("Sets the list of allowed SSL ciphers."),
 			NULL,
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index dac6776..6b21fc5 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -79,6 +79,7 @@
 
 #authentication_timeout = 1min		# 1s-600s
 #ssl = off				# (change requires restart)
+#ssl_protocols = 'ALL:-SSL'		# allowed SSL protocols
 #ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers
 					# (change requires restart)
 #ssl_prefer_server_ciphers = on		# (change requires restart)
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 5da9d8d..15d0c3f 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -88,6 +88,7 @@ extern ssize_t secure_raw_write(Port *port, const void *ptr, size_t len);
 extern bool ssl_loaded_verify_locations;
 
 /* GUCs */
+extern char *SSLProtocols;
 extern char *SSLCipherSuites;
 extern char *SSLECDHCurve;
 extern bool SSLPreferServerCiphers;
postgresql-9.4-ssl-protocols.difftext/x-patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 2fb9217..50bd51f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1027,6 +1027,34 @@ include_dir 'conf.d'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-ssl-protocols" xreflabel="ssl_protocols">
+      <term><varname>ssl_protocols</varname> (<type>string</type>)</term>
+      <indexterm>
+       <primary><varname>ssl_protocols</> configuration parameter</primary>
+      </indexterm>
+      <listitem>
+       <para>
+        Specifies a colon-separated list of <acronym>SSL</> protocols that are
+        allowed to be used on secure connections. Each entry in the list can
+        be prefixed by a <literal>+</> (add to the current list)
+        or <literal>-</> (remove from the current list). If no prefix is used,
+        the list is replaced with the specified protocol.
+       </para>
+       <para>
+        The full list of supported protocols can be found in the
+        the <application>openssl</> manual page.  In addition to the names of
+        individual protocols, the following keywords can be
+        used: <literal>ALL</> (all protocols supported by the underlying
+        crypto library), <literal>SSL</> (all supported versions
+        of <acronym>SSL</>) and <literal>TLS</> (all supported versions
+        of <acronym>TLS</>).
+       </para>
+       <para>
+        The default is <literal>ALL:-SSL</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-ssl-ciphers" xreflabel="ssl_ciphers">
       <term><varname>ssl_ciphers</varname> (<type>string</type>)
       <indexterm>
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 89c30d0..acebdc2 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -90,6 +90,7 @@ static void initialize_SSL(void);
 static int	open_server_SSL(Port *);
 static void close_SSL(Port *);
 static const char *SSLerrmessage(void);
+static long parse_SSL_protocols(const char *str);
 #endif
 
 char	   *ssl_cert_file;
@@ -112,7 +113,8 @@ static SSL_CTX *SSL_context = NULL;
 static bool ssl_loaded_verify_locations = false;
 #endif
 
-/* GUC variable controlling SSL cipher list */
+/* GUC variables controlling SSL protocol and cipher list */
+char	   *SSLProtocols = NULL;
 char	   *SSLCipherSuites = NULL;
 
 /* GUC variable for default ECHD curve. */
@@ -887,15 +889,16 @@ initialize_SSL(void)
 							SSLerrmessage())));
 	}
 
-	/* set up ephemeral DH keys, and disallow SSL v2/v3 while at it */
+	/* set up ephemeral DH keys */
 	SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb);
-	SSL_CTX_set_options(SSL_context,
-						SSL_OP_SINGLE_DH_USE |
-						SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
+	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE);
 
 	/* set up ephemeral ECDH keys */
 	initialize_ecdh();
 
+	/* set up the allowed protocol list */
+	SSL_CTX_set_options(SSL_context, parse_SSL_protocols(SSLProtocols));
+
 	/* set up the allowed cipher list */
 	if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
 		elog(FATAL, "could not set the cipher list (no valid ciphers available)");
@@ -1158,4 +1161,106 @@ SSLerrmessage(void)
 	return errbuf;
 }
 
+/*
+ *	Parse the SSL allowed protocol list
+ *
+ *	The logic here is inverted.  OpenSSL does not take a list of
+ *	protocols to use, but a list of protocols to avoid.  We use the
+ *	same bits with the opposite meaning, then invert the result.
+ */
+
+#define SSL_PROTO_SSLv2		SSL_OP_NO_SSLv2
+#define SSL_PROTO_SSLv3		SSL_OP_NO_SSLv3
+#define SSL_PROTO_SSL		(SSL_PROTO_SSLv2 | SSL_PROTO_SSLv3)
+#define SSL_PROTO_TLSv1		SSL_OP_NO_TLSv1
+#ifdef SSL_OP_NO_TLSv1_1
+#define SSL_PROTO_TLSv1_1	SSL_OP_NO_TLSv1_1
+#else
+#define SSL_PROTO_TLSv1_1	0
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+#define SSL_PROTO_TLSv1_2	SSL_OP_NO_TLSv1_2
+#else
+#define SSL_PROTO_TLSv1_2	0
+#endif
+#define SSL_PROTO_TLS		(SSL_PROTO_TLSv1 | SSL_PROTO_TLSv1_1 | SSL_PROTO_TLSv1_2)
+#define SSL_PROTO_ALL		(SSL_PROTO_SSL | SSL_PROTO_TLS)
+#define SSL_PROTO_NONE		0
+
+#define str_is_token(str, tok, len) \
+	(len == sizeof(tok) - 1 && pg_strncasecmp(str, tok, len) == 0)
+
+static long
+parse_SSL_protocols(const char *str)
+{
+	long current, result;
+	const char *p, *q;
+	int action;
+
+	/*
+	 * Iterate over the colon-separated list of protocols.  If a protocol is
+	 * preceded by a +, it is added to the list.  If it is preceded by a -, it
+	 * is removed from the list.  If it is not preceded by anything, the list
+	 * is set to exactly that protocol.  "ALL" can be used to indicate all
+	 * protocols, "NONE" to indicate no protocols "SSL" to indicate all SSL
+	 * protocols and "TLS" to indicate all TLS protocols.  The parser accepts
+	 * "SSLv2", "SSLv3" and "SSL", but they are removed after parsing.
+	 */
+	result = SSL_PROTO_NONE;
+	for (p = q = str; *q; p = q + 1) {
+		for (q = p; *q && *q != ':'; ++q)
+			/* nothing */ ;
+		if (*p == '-' || *p == '+')
+			action = *p++;
+		else
+			action = '=';
+		if (str_is_token(p, "ALL", q - p))
+			current = SSL_PROTO_ALL;
+		else if (str_is_token(p, "NONE", q - p))
+			current = SSL_PROTO_NONE;
+		else if (str_is_token(p, SSL_TXT_SSLV2, q - p))
+			current = SSL_PROTO_SSLv2;
+		else if (str_is_token(p, SSL_TXT_SSLV3, q - p))
+			current = SSL_PROTO_SSLv3;
+		else if (str_is_token(p, "SSL", q - p))
+			current = SSL_PROTO_SSL;
+		else if (str_is_token(p, SSL_TXT_TLSV1, q - p))
+			current = SSL_PROTO_TLSv1;
+#ifdef SSL_OP_NO_TLSv1_1
+		else if (str_is_token(p, SSL_TXT_TLSV1_1, q - p))
+			current = SSL_PROTO_TLSv1_1;
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+		else if (str_is_token(p, SSL_TXT_TLSV1_2, q - p))
+			current = SSL_PROTO_TLSv1_2;
+#endif
+		else if (str_is_token(p, "TLS", q - p))
+			current = SSL_PROTO_TLS;
+		else
+			elog(FATAL, "invalid SSL protocol list");
+		switch (action) {
+		case '+':
+			result |= current;
+			break;
+		case '-':
+			result &= ~current;
+			break;
+		default:
+			result = current;
+			break;
+		}
+	}
+	/* forcibly disallow SSLv2 and SSLv3 */
+	if (result & SSL_PROTO_SSL) {
+		elog(WARNING, "removing SSLv2 and SSLv3 from SSL protocol list");
+		result &= ~SSL_PROTO_SSL;
+	}
+	/* check the result */
+	if (result == 0)
+		elog(FATAL, "could not set the protocol list (no valid protocols available)");
+	elog(DEBUG2, "enabling SSL protocols: %lx", result);
+	/* return the inverse */
+	return (SSL_PROTO_ALL & ~result);
+}
+
 #endif   /* USE_SSL */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index ea58cf6..3d1b668 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -125,6 +125,7 @@ extern char *default_tablespace;
 extern char *temp_tablespaces;
 extern bool ignore_checksum_failure;
 extern bool synchronize_seqscans;
+extern char *SSLProtocols;
 extern char *SSLCipherSuites;
 extern char *SSLECDHCurve;
 extern bool SSLPreferServerCiphers;
@@ -3215,6 +3216,21 @@ static struct config_string ConfigureNamesString[] =
 	},
 
 	{
+		{"ssl_protocols", PGC_POSTMASTER, CONN_AUTH_SECURITY,
+			gettext_noop("Sets the list of allowed SSL protocols."),
+			NULL,
+			GUC_SUPERUSER_ONLY
+		},
+		&SSLProtocols,
+#ifdef USE_SSL
+		"ALL:-SSL",
+#else
+		"none",
+#endif
+		NULL, NULL, NULL
+	},
+
+	{
 		{"ssl_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY,
 			gettext_noop("Sets the list of allowed SSL ciphers."),
 			NULL,
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 8d5bb19..d161113 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -79,6 +79,7 @@
 
 #authentication_timeout = 1min		# 1s-600s
 #ssl = off				# (change requires restart)
+#ssl_protocols = 'ALL:-SSL'		# allowed SSL protocols
 #ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers
 					# (change requires restart)
 #ssl_prefer_server_ciphers = on		# (change requires restart)
postgresql-9.3-ssl-protocols.difftext/x-patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c9276a3..ab2941b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -873,6 +873,34 @@ include 'filename'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-ssl-protocols" xreflabel="ssl_protocols">
+      <term><varname>ssl_protocols</varname> (<type>string</type>)</term>
+      <indexterm>
+       <primary><varname>ssl_protocols</> configuration parameter</primary>
+      </indexterm>
+      <listitem>
+       <para>
+        Specifies a colon-separated list of <acronym>SSL</> protocols that are
+        allowed to be used on secure connections. Each entry in the list can
+        be prefixed by a <literal>+</> (add to the current list)
+        or <literal>-</> (remove from the current list). If no prefix is used,
+        the list is replaced with the specified protocol.
+       </para>
+       <para>
+        The full list of supported protocols can be found in the
+        the <application>openssl</> manual page.  In addition to the names of
+        individual protocols, the following keywords can be
+        used: <literal>ALL</> (all protocols supported by the underlying
+        crypto library), <literal>SSL</> (all supported versions
+        of <acronym>SSL</>) and <literal>TLS</> (all supported versions
+        of <acronym>TLS</>).
+       </para>
+       <para>
+        The default is <literal>ALL:-SSLv2</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-ssl-ciphers" xreflabel="ssl_ciphers">
       <term><varname>ssl_ciphers</varname> (<type>string</type>)</term>
       <indexterm>
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index d1beda8..c5d1558 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -87,6 +87,7 @@ static void initialize_SSL(void);
 static int	open_server_SSL(Port *);
 static void close_SSL(Port *);
 static const char *SSLerrmessage(void);
+static long parse_SSL_protocols(const char *str);
 #endif
 
 char	   *ssl_cert_file;
@@ -106,7 +107,8 @@ static SSL_CTX *SSL_context = NULL;
 static bool ssl_loaded_verify_locations = false;
 #endif
 
-/* GUC variable controlling SSL cipher list */
+/* GUC variables controlling SSL protocol and cipher list */
+char	   *SSLProtocols = NULL;
 char	   *SSLCipherSuites = NULL;
 
 /* ------------------------------------------------------------ */
@@ -793,9 +795,12 @@ initialize_SSL(void)
 							SSLerrmessage())));
 	}
 
-	/* set up ephemeral DH keys, and disallow SSL v2 while at it */
+	/* set up ephemeral DH keys */
 	SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb);
-	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE | SSL_OP_NO_SSLv2);
+	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE);
+
+	/* set up the allowed protocol list */
+	SSL_CTX_set_options(SSL_context, parse_SSL_protocols(SSLProtocols));
 
 	/* set up the allowed cipher list */
 	if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
@@ -1055,4 +1060,107 @@ SSLerrmessage(void)
 	return errbuf;
 }
 
+/*
+ *	Parse the SSL allowed protocol list
+ *
+ *	The logic here is inverted.  OpenSSL does not take a list of
+ *	protocols to use, but a list of protocols to avoid.  We use the
+ *	same bits with the opposite meaning, then invert the result.
+ */
+
+#define SSL_PROTO_SSLv2		SSL_OP_NO_SSLv2
+#define SSL_PROTO_SSLv3		SSL_OP_NO_SSLv3
+#define SSL_PROTO_SSL		(SSL_PROTO_SSLv2 | SSL_PROTO_SSLv3)
+#define SSL_PROTO_TLSv1		SSL_OP_NO_TLSv1
+#ifdef SSL_OP_NO_TLSv1_1
+#define SSL_PROTO_TLSv1_1	SSL_OP_NO_TLSv1_1
+#else
+#define SSL_PROTO_TLSv1_1	0
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+#define SSL_PROTO_TLSv1_2	SSL_OP_NO_TLSv1_2
+#else
+#define SSL_PROTO_TLSv1_2	0
+#endif
+#define SSL_PROTO_TLS		(SSL_PROTO_TLSv1 | SSL_PROTO_TLSv1_1 | SSL_PROTO_TLSv1_2)
+#define SSL_PROTO_ALL		(SSL_PROTO_SSL | SSL_PROTO_TLS)
+#define SSL_PROTO_NONE		0
+
+#define str_is_token(str, tok, len) \
+	(len == sizeof(tok) - 1 && pg_strncasecmp(str, tok, len) == 0)
+
+static long
+parse_SSL_protocols(const char *str)
+{
+	long current, result;
+	const char *p, *q;
+	int action;
+
+	/*
+	 * Iterate over the colon-separated list of protocols.  If a
+	 * protocol is preceded by a +, it is added to the list.  If it is
+	 * preceded by a -, it is removed from the list.  If it is not
+	 * preceded by anything, the list is set to exactly that protocol.
+	 * "ALL" can be used to indicate all protocols, "NONE" to indicate
+	 * no protocols, "SSL" to indicate all SSL protocols and "TLS" to
+	 * indicate all TLS protocols.  The parser accepts "SSLv2", but it
+	 * is removed after parsing.
+	 */
+	result = SSL_PROTO_NONE;
+	for (p = q = str; *q; p = q + 1) {
+		for (q = p; *q && *q != ':'; ++q)
+			/* nothing */ ;
+		if (*p == '-' || *p == '+')
+			action = *p++;
+		else
+			action = '=';
+		if (str_is_token(p, "ALL", q - p))
+			current = SSL_PROTO_ALL;
+		else if (str_is_token(p, "NONE", q - p))
+			current = SSL_PROTO_NONE;
+		else if (str_is_token(p, SSL_TXT_SSLV2, q - p))
+			current = SSL_PROTO_SSLv2;
+		else if (str_is_token(p, SSL_TXT_SSLV3, q - p))
+			current = SSL_PROTO_SSLv3;
+		else if (str_is_token(p, "SSL", q - p))
+			current = SSL_PROTO_SSL;
+		else if (str_is_token(p, SSL_TXT_TLSV1, q - p))
+			current = SSL_PROTO_TLSv1;
+#ifdef SSL_OP_NO_TLSv1_1
+		else if (str_is_token(p, SSL_TXT_TLSV1_1, q - p))
+			current = SSL_PROTO_TLSv1_1;
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+		else if (str_is_token(p, SSL_TXT_TLSV1_2, q - p))
+			current = SSL_PROTO_TLSv1_2;
+#endif
+		else if (str_is_token(p, "TLS", q - p))
+			current = SSL_PROTO_TLS;
+		else
+			elog(FATAL, "invalid SSL protocol list");
+		switch (action) {
+		case '+':
+			result |= current;
+			break;
+		case '-':
+			result &= ~current;
+			break;
+		default:
+			result = current;
+			break;
+		}
+	}
+	/* forcibly disallow SSLv2 */
+	if (result & SSL_PROTO_SSLv2) {
+		elog(WARNING, "removing SSLv2 from SSL protocol list");
+		result &= ~SSL_PROTO_SSLv2;
+	}
+	/* check the result */
+	if (result == 0)
+		elog(FATAL, "could not set the protocol list (no valid protocols available)");
+	elog(DEBUG2, "enabling SSL protocols: %lx", result);
+	/* return the inverse */
+	return (SSL_PROTO_ALL & ~result);
+}
+
 #endif   /* USE_SSL */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 2b6527f..42d4ad1 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -125,6 +125,7 @@ extern char *temp_tablespaces;
 extern bool ignore_checksum_failure;
 extern bool synchronize_seqscans;
 extern int	ssl_renegotiation_limit;
+extern char *SSLProtocols;
 extern char *SSLCipherSuites;
 
 #ifdef TRACE_SORT
@@ -3117,6 +3118,21 @@ static struct config_string ConfigureNamesString[] =
 	},
 
 	{
+		{"ssl_protocols", PGC_POSTMASTER, CONN_AUTH_SECURITY,
+			gettext_noop("Sets the list of allowed SSL protocols."),
+			NULL,
+			GUC_SUPERUSER_ONLY
+		},
+		&SSLProtocols,
+#ifdef USE_SSL
+		"ALL:-SSLv2",
+#else
+		"none",
+#endif
+		NULL, NULL, NULL
+	},
+
+	{
 		{"ssl_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY,
 			gettext_noop("Sets the list of allowed SSL ciphers."),
 			NULL,
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 12f1cba..64d38a9 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -79,6 +79,7 @@
 
 #authentication_timeout = 1min		# 1s-600s
 #ssl = off				# (change requires restart)
+#ssl_protocols = 'ALL:-SSLv2'		# allowed SSL protocols
 #ssl_ciphers = 'DEFAULT:!LOW:!EXP:!MD5:@STRENGTH'	# allowed SSL ciphers
 					# (change requires restart)
 #ssl_renegotiation_limit = 512MB	# amount of data between renegotiations
postgresql-9.2-ssl-protocols.difftext/x-patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 51d7da9..628f523 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -789,6 +789,34 @@ SET ENABLE_SEQSCAN TO OFF;
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-ssl-protocols" xreflabel="ssl_protocols">
+      <term><varname>ssl_protocols</varname> (<type>string</type>)</term>
+      <indexterm>
+       <primary><varname>ssl_protocols</> configuration parameter</primary>
+      </indexterm>
+      <listitem>
+       <para>
+        Specifies a colon-separated list of <acronym>SSL</> protocols that are
+        allowed to be used on secure connections. Each entry in the list can
+        be prefixed by a <literal>+</> (add to the current list)
+        or <literal>-</> (remove from the current list). If no prefix is used,
+        the list is replaced with the specified protocol.
+       </para>
+       <para>
+        The full list of supported protocols can be found in the
+        the <application>openssl</> manual page.  In addition to the names of
+        individual protocols, the following keywords can be
+        used: <literal>ALL</> (all protocols supported by the underlying
+        crypto library), <literal>SSL</> (all supported versions
+        of <acronym>SSL</>) and <literal>TLS</> (all supported versions
+        of <acronym>TLS</>).
+       </para>
+       <para>
+        The default is <literal>ALL:-SSLv2</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-ssl-ciphers" xreflabel="ssl_ciphers">
       <term><varname>ssl_ciphers</varname> (<type>string</type>)</term>
       <indexterm>
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index bccea54..9b8871b 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -87,6 +87,7 @@ static void initialize_SSL(void);
 static int	open_server_SSL(Port *);
 static void close_SSL(Port *);
 static const char *SSLerrmessage(void);
+static long parse_SSL_protocols(const char *str);
 #endif
 
 char	   *ssl_cert_file;
@@ -106,7 +107,8 @@ static SSL_CTX *SSL_context = NULL;
 static bool ssl_loaded_verify_locations = false;
 #endif
 
-/* GUC variable controlling SSL cipher list */
+/* GUC variables controlling SSL protocol and cipher list */
+char	   *SSLProtocols = NULL;
 char	   *SSLCipherSuites = NULL;
 
 /* ------------------------------------------------------------ */
@@ -793,9 +795,12 @@ initialize_SSL(void)
 							SSLerrmessage())));
 	}
 
-	/* set up ephemeral DH keys, and disallow SSL v2 while at it */
+	/* set up ephemeral DH keys */
 	SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb);
-	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE | SSL_OP_NO_SSLv2);
+	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE);
+
+	/* set up the allowed protocol list */
+	SSL_CTX_set_options(SSL_context, parse_SSL_protocols(SSLProtocols));
 
 	/* set up the allowed cipher list */
 	if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
@@ -1055,4 +1060,107 @@ SSLerrmessage(void)
 	return errbuf;
 }
 
+/*
+ *	Parse the SSL allowed protocol list
+ *
+ *	The logic here is inverted.  OpenSSL does not take a list of
+ *	protocols to use, but a list of protocols to avoid.  We use the
+ *	same bits with the opposite meaning, then invert the result.
+ */
+
+#define SSL_PROTO_SSLv2		SSL_OP_NO_SSLv2
+#define SSL_PROTO_SSLv3		SSL_OP_NO_SSLv3
+#define SSL_PROTO_SSL		(SSL_PROTO_SSLv2 | SSL_PROTO_SSLv3)
+#define SSL_PROTO_TLSv1		SSL_OP_NO_TLSv1
+#ifdef SSL_OP_NO_TLSv1_1
+#define SSL_PROTO_TLSv1_1	SSL_OP_NO_TLSv1_1
+#else
+#define SSL_PROTO_TLSv1_1	0
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+#define SSL_PROTO_TLSv1_2	SSL_OP_NO_TLSv1_2
+#else
+#define SSL_PROTO_TLSv1_2	0
+#endif
+#define SSL_PROTO_TLS		(SSL_PROTO_TLSv1 | SSL_PROTO_TLSv1_1 | SSL_PROTO_TLSv1_2)
+#define SSL_PROTO_ALL		(SSL_PROTO_SSL | SSL_PROTO_TLS)
+#define SSL_PROTO_NONE		0
+
+#define str_is_token(str, tok, len) \
+	(len == sizeof(tok) - 1 && pg_strncasecmp(str, tok, len) == 0)
+
+static long
+parse_SSL_protocols(const char *str)
+{
+	long current, result;
+	const char *p, *q;
+	int action;
+
+	/*
+	 * Iterate over the colon-separated list of protocols.  If a
+	 * protocol is preceded by a +, it is added to the list.  If it is
+	 * preceded by a -, it is removed from the list.  If it is not
+	 * preceded by anything, the list is set to exactly that protocol.
+	 * "ALL" can be used to indicate all protocols, "NONE" to indicate
+	 * no protocols, "SSL" to indicate all SSL protocols and "TLS" to
+	 * indicate all TLS protocols.  The parser accepts "SSLv2", but it
+	 * is removed after parsing.
+	 */
+	result = SSL_PROTO_NONE;
+	for (p = q = str; *q; p = q + 1) {
+		for (q = p; *q && *q != ':'; ++q)
+			/* nothing */ ;
+		if (*p == '-' || *p == '+')
+			action = *p++;
+		else
+			action = '=';
+		if (str_is_token(p, "ALL", q - p))
+			current = SSL_PROTO_ALL;
+		else if (str_is_token(p, "NONE", q - p))
+			current = SSL_PROTO_NONE;
+		else if (str_is_token(p, SSL_TXT_SSLV2, q - p))
+			current = SSL_PROTO_SSLv2;
+		else if (str_is_token(p, SSL_TXT_SSLV3, q - p))
+			current = SSL_PROTO_SSLv3;
+		else if (str_is_token(p, "SSL", q - p))
+			current = SSL_PROTO_SSL;
+		else if (str_is_token(p, SSL_TXT_TLSV1, q - p))
+			current = SSL_PROTO_TLSv1;
+#ifdef SSL_OP_NO_TLSv1_1
+		else if (str_is_token(p, SSL_TXT_TLSV1_1, q - p))
+			current = SSL_PROTO_TLSv1_1;
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+		else if (str_is_token(p, SSL_TXT_TLSV1_2, q - p))
+			current = SSL_PROTO_TLSv1_2;
+#endif
+		else if (str_is_token(p, "TLS", q - p))
+			current = SSL_PROTO_TLS;
+		else
+			elog(FATAL, "invalid SSL protocol list");
+		switch (action) {
+		case '+':
+			result |= current;
+			break;
+		case '-':
+			result &= ~current;
+			break;
+		default:
+			result = current;
+			break;
+		}
+	}
+	/* forcibly disallow SSLv2 */
+	if (result & SSL_PROTO_SSLv2) {
+		elog(WARNING, "removing SSLv2 from SSL protocol list");
+		result &= ~SSL_PROTO_SSLv2;
+	}
+	/* check the result */
+	if (result == 0)
+		elog(FATAL, "could not set the protocol list (no valid protocols available)");
+	elog(DEBUG2, "enabling SSL protocols: %lx", result);
+	/* return the inverse */
+	return (SSL_PROTO_ALL & ~result);
+}
+
 #endif   /* USE_SSL */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index e5ee0f8..cc8edbc 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -132,6 +132,7 @@ extern char *default_tablespace;
 extern char *temp_tablespaces;
 extern bool synchronize_seqscans;
 extern int	ssl_renegotiation_limit;
+extern char *SSLProtocols;
 extern char *SSLCipherSuites;
 
 #ifdef TRACE_SORT
@@ -3043,6 +3044,21 @@ static struct config_string ConfigureNamesString[] =
 	},
 
 	{
+		{"ssl_protocols", PGC_POSTMASTER, CONN_AUTH_SECURITY,
+			gettext_noop("Sets the list of allowed SSL protocols."),
+			NULL,
+			GUC_SUPERUSER_ONLY
+		},
+		&SSLProtocols,
+#ifdef USE_SSL
+		"ALL:-SSLv2",
+#else
+		"none",
+#endif
+		NULL, NULL, NULL
+	},
+
+	{
 		{"ssl_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY,
 			gettext_noop("Sets the list of allowed SSL ciphers."),
 			NULL,
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 6fe6924..8cca6b9 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -78,6 +78,7 @@
 
 #authentication_timeout = 1min		# 1s-600s
 #ssl = off				# (change requires restart)
+#ssl_protocols = 'ALL:-SSLv2'		# allowed SSL protocols
 #ssl_ciphers = 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH'	# allowed SSL ciphers
 					# (change requires restart)
 #ssl_renegotiation_limit = 512MB	# amount of data between renegotiations
postgresql-9.1-ssl-protocols.difftext/x-patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 179c60e..4ae3ae3 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -689,6 +689,34 @@ SET ENABLE_SEQSCAN TO OFF;
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-ssl-protocols" xreflabel="ssl_protocols">
+      <term><varname>ssl_protocols</varname> (<type>string</type>)</term>
+      <indexterm>
+       <primary><varname>ssl_protocols</> configuration parameter</primary>
+      </indexterm>
+      <listitem>
+       <para>
+        Specifies a colon-separated list of <acronym>SSL</> protocols that are
+        allowed to be used on secure connections. Each entry in the list can
+        be prefixed by a <literal>+</> (add to the current list)
+        or <literal>-</> (remove from the current list). If no prefix is used,
+        the list is replaced with the specified protocol.
+       </para>
+       <para>
+        The full list of supported protocols can be found in the
+        the <application>openssl</> manual page.  In addition to the names of
+        individual protocols, the following keywords can be
+        used: <literal>ALL</> (all protocols supported by the underlying
+        crypto library), <literal>SSL</> (all supported versions
+        of <acronym>SSL</>) and <literal>TLS</> (all supported versions
+        of <acronym>TLS</>).
+       </para>
+       <para>
+        The default is <literal>ALL:-SSLv2</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-ssl-ciphers" xreflabel="ssl_ciphers">
       <term><varname>ssl_ciphers</varname> (<type>string</type>)</term>
       <indexterm>
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 6e09496..1d3c09b 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -92,6 +92,7 @@ static void initialize_SSL(void);
 static int	open_server_SSL(Port *);
 static void close_SSL(Port *);
 static const char *SSLerrmessage(void);
+static long parse_SSL_protocols(const char *str);
 #endif
 
 /*
@@ -106,7 +107,8 @@ static SSL_CTX *SSL_context = NULL;
 static bool ssl_loaded_verify_locations = false;
 #endif
 
-/* GUC variable controlling SSL cipher list */
+/* GUC variables controlling SSL protocol and cipher list */
+char	   *SSLProtocols = NULL;
 char	   *SSLCipherSuites = NULL;
 
 /* ------------------------------------------------------------ */
@@ -793,9 +795,12 @@ initialize_SSL(void)
 							SSLerrmessage())));
 	}
 
-	/* set up ephemeral DH keys, and disallow SSL v2 while at it */
+	/* set up ephemeral DH keys */
 	SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb);
-	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE | SSL_OP_NO_SSLv2);
+	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE);
+
+	/* set up the allowed protocol list */
+	SSL_CTX_set_options(SSL_context, parse_SSL_protocols(SSLProtocols));
 
 	/* set up the allowed cipher list */
 	if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
@@ -1074,4 +1079,107 @@ SSLerrmessage(void)
 	return errbuf;
 }
 
+/*
+ *	Parse the SSL allowed protocol list
+ *
+ *	The logic here is inverted.  OpenSSL does not take a list of
+ *	protocols to use, but a list of protocols to avoid.  We use the
+ *	same bits with the opposite meaning, then invert the result.
+ */
+
+#define SSL_PROTO_SSLv2		SSL_OP_NO_SSLv2
+#define SSL_PROTO_SSLv3		SSL_OP_NO_SSLv3
+#define SSL_PROTO_SSL		(SSL_PROTO_SSLv2 | SSL_PROTO_SSLv3)
+#define SSL_PROTO_TLSv1		SSL_OP_NO_TLSv1
+#ifdef SSL_OP_NO_TLSv1_1
+#define SSL_PROTO_TLSv1_1	SSL_OP_NO_TLSv1_1
+#else
+#define SSL_PROTO_TLSv1_1	0
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+#define SSL_PROTO_TLSv1_2	SSL_OP_NO_TLSv1_2
+#else
+#define SSL_PROTO_TLSv1_2	0
+#endif
+#define SSL_PROTO_TLS		(SSL_PROTO_TLSv1 | SSL_PROTO_TLSv1_1 | SSL_PROTO_TLSv1_2)
+#define SSL_PROTO_ALL		(SSL_PROTO_SSL | SSL_PROTO_TLS)
+#define SSL_PROTO_NONE		0
+
+#define str_is_token(str, tok, len) \
+	(len == sizeof(tok) - 1 && pg_strncasecmp(str, tok, len) == 0)
+
+static long
+parse_SSL_protocols(const char *str)
+{
+	long current, result;
+	const char *p, *q;
+	int action;
+
+	/*
+	 * Iterate over the colon-separated list of protocols.  If a
+	 * protocol is preceded by a +, it is added to the list.  If it is
+	 * preceded by a -, it is removed from the list.  If it is not
+	 * preceded by anything, the list is set to exactly that protocol.
+	 * "ALL" can be used to indicate all protocols, "NONE" to indicate
+	 * no protocols, "SSL" to indicate all SSL protocols and "TLS" to
+	 * indicate all TLS protocols.  The parser accepts "SSLv2", but it
+	 * is removed after parsing.
+	 */
+	result = SSL_PROTO_NONE;
+	for (p = q = str; *q; p = q + 1) {
+		for (q = p; *q && *q != ':'; ++q)
+			/* nothing */ ;
+		if (*p == '-' || *p == '+')
+			action = *p++;
+		else
+			action = '=';
+		if (str_is_token(p, "ALL", q - p))
+			current = SSL_PROTO_ALL;
+		else if (str_is_token(p, "NONE", q - p))
+			current = SSL_PROTO_NONE;
+		else if (str_is_token(p, SSL_TXT_SSLV2, q - p))
+			current = SSL_PROTO_SSLv2;
+		else if (str_is_token(p, SSL_TXT_SSLV3, q - p))
+			current = SSL_PROTO_SSLv3;
+		else if (str_is_token(p, "SSL", q - p))
+			current = SSL_PROTO_SSL;
+		else if (str_is_token(p, SSL_TXT_TLSV1, q - p))
+			current = SSL_PROTO_TLSv1;
+#ifdef SSL_OP_NO_TLSv1_1
+		else if (str_is_token(p, SSL_TXT_TLSV1_1, q - p))
+			current = SSL_PROTO_TLSv1_1;
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+		else if (str_is_token(p, SSL_TXT_TLSV1_2, q - p))
+			current = SSL_PROTO_TLSv1_2;
+#endif
+		else if (str_is_token(p, "TLS", q - p))
+			current = SSL_PROTO_TLS;
+		else
+			elog(FATAL, "invalid SSL protocol list");
+		switch (action) {
+		case '+':
+			result |= current;
+			break;
+		case '-':
+			result &= ~current;
+			break;
+		default:
+			result = current;
+			break;
+		}
+	}
+	/* forcibly disallow SSLv2 */
+	if (result & SSL_PROTO_SSLv2) {
+		elog(WARNING, "removing SSLv2 from SSL protocol list");
+		result &= ~SSL_PROTO_SSLv2;
+	}
+	/* check the result */
+	if (result == 0)
+		elog(FATAL, "could not set the protocol list (no valid protocols available)");
+	elog(DEBUG2, "enabling SSL protocols: %lx", result);
+	/* return the inverse */
+	return (SSL_PROTO_ALL & ~result);
+}
+
 #endif   /* USE_SSL */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a66a7d9..c2dc017 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -131,6 +131,7 @@ extern char *temp_tablespaces;
 extern bool synchronize_seqscans;
 extern bool fullPageWrites;
 extern int	ssl_renegotiation_limit;
+extern char *SSLProtocols;
 extern char *SSLCipherSuites;
 
 #ifdef TRACE_SORT
@@ -2987,6 +2988,21 @@ static struct config_string ConfigureNamesString[] =
 	},
 
 	{
+		{"ssl_protocols", PGC_POSTMASTER, CONN_AUTH_SECURITY,
+			gettext_noop("Sets the list of allowed SSL protocols."),
+			NULL,
+			GUC_SUPERUSER_ONLY
+		},
+		&SSLProtocols,
+#ifdef USE_SSL
+		"ALL:-SSLv2",
+#else
+		"none",
+#endif
+		NULL, NULL, NULL
+	},
+
+	{
 		{"ssl_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY,
 			gettext_noop("Sets the list of allowed SSL ciphers."),
 			NULL,
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 82c8ae4..8acc5a5 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -78,6 +78,7 @@
 
 #authentication_timeout = 1min		# 1s-600s
 #ssl = off				# (change requires restart)
+#ssl_protocols = 'ALL:-SSLv2'		# allowed SSL protocols
 #ssl_ciphers = 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH'	# allowed SSL ciphers
 					# (change requires restart)
 #ssl_renegotiation_limit = 512MB	# amount of data between renegotiations
postgresql-9.0-ssl-protocols.difftext/x-patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 2f37a29..71efa6f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -685,6 +685,34 @@ SET ENABLE_SEQSCAN TO OFF;
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-ssl-protocols" xreflabel="ssl_protocols">
+      <term><varname>ssl_protocols</varname> (<type>string</type>)</term>
+      <indexterm>
+       <primary><varname>ssl_protocols</> configuration parameter</primary>
+      </indexterm>
+      <listitem>
+       <para>
+        Specifies a colon-separated list of <acronym>SSL</> protocols that are
+        allowed to be used on secure connections. Each entry in the list can
+        be prefixed by a <literal>+</> (add to the current list)
+        or <literal>-</> (remove from the current list). If no prefix is used,
+        the list is replaced with the specified protocol.
+       </para>
+       <para>
+        The full list of supported protocols can be found in the
+        the <application>openssl</> manual page.  In addition to the names of
+        individual protocols, the following keywords can be
+        used: <literal>ALL</> (all protocols supported by the underlying
+        crypto library), <literal>SSL</> (all supported versions
+        of <acronym>SSL</>) and <literal>TLS</> (all supported versions
+        of <acronym>TLS</>).
+       </para>
+       <para>
+        The default is <literal>ALL:-SSLv2</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-ssl-ciphers" xreflabel="ssl_ciphers">
       <term><varname>ssl_ciphers</varname> (<type>string</type>)</term>
       <indexterm>
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 28e3102..e10bcdd 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -92,6 +92,7 @@ static void initialize_SSL(void);
 static int	open_server_SSL(Port *);
 static void close_SSL(Port *);
 static const char *SSLerrmessage(void);
+static long parse_SSL_protocols(const char *str);
 #endif
 
 /*
@@ -105,7 +106,8 @@ int			ssl_renegotiation_limit;
 static SSL_CTX *SSL_context = NULL;
 static bool ssl_loaded_verify_locations = false;
 
-/* GUC variable controlling SSL cipher list */
+/* GUC variables controlling SSL protocol and cipher list */
+char	   *SSLProtocols = NULL;
 char	   *SSLCipherSuites = NULL;
 #endif
 
@@ -793,9 +795,12 @@ initialize_SSL(void)
 							SSLerrmessage())));
 	}
 
-	/* set up ephemeral DH keys, and disallow SSL v2 while at it */
+	/* set up ephemeral DH keys */
 	SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb);
-	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE | SSL_OP_NO_SSLv2);
+	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE);
+
+	/* set up the allowed protocol list */
+	SSL_CTX_set_options(SSL_context, parse_SSL_protocols(SSLProtocols));
 
 	/* set up the allowed cipher list */
 	if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
@@ -1074,4 +1079,107 @@ SSLerrmessage(void)
 	return errbuf;
 }
 
+/*
+ *	Parse the SSL allowed protocol list
+ *
+ *	The logic here is inverted.  OpenSSL does not take a list of
+ *	protocols to use, but a list of protocols to avoid.  We use the
+ *	same bits with the opposite meaning, then invert the result.
+ */
+
+#define SSL_PROTO_SSLv2		SSL_OP_NO_SSLv2
+#define SSL_PROTO_SSLv3		SSL_OP_NO_SSLv3
+#define SSL_PROTO_SSL		(SSL_PROTO_SSLv2 | SSL_PROTO_SSLv3)
+#define SSL_PROTO_TLSv1		SSL_OP_NO_TLSv1
+#ifdef SSL_OP_NO_TLSv1_1
+#define SSL_PROTO_TLSv1_1	SSL_OP_NO_TLSv1_1
+#else
+#define SSL_PROTO_TLSv1_1	0
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+#define SSL_PROTO_TLSv1_2	SSL_OP_NO_TLSv1_2
+#else
+#define SSL_PROTO_TLSv1_2	0
+#endif
+#define SSL_PROTO_TLS		(SSL_PROTO_TLSv1 | SSL_PROTO_TLSv1_1 | SSL_PROTO_TLSv1_2)
+#define SSL_PROTO_ALL		(SSL_PROTO_SSL | SSL_PROTO_TLS)
+#define SSL_PROTO_NONE		0
+
+#define str_is_token(str, tok, len) \
+	(len == sizeof(tok) - 1 && pg_strncasecmp(str, tok, len) == 0)
+
+static long
+parse_SSL_protocols(const char *str)
+{
+	long current, result;
+	const char *p, *q;
+	int action;
+
+	/*
+	 * Iterate over the colon-separated list of protocols.  If a
+	 * protocol is preceded by a +, it is added to the list.  If it is
+	 * preceded by a -, it is removed from the list.  If it is not
+	 * preceded by anything, the list is set to exactly that protocol.
+	 * "ALL" can be used to indicate all protocols, "NONE" to indicate
+	 * no protocols, "SSL" to indicate all SSL protocols and "TLS" to
+	 * indicate all TLS protocols.  The parser accepts "SSLv2", but it
+	 * is removed after parsing.
+	 */
+	result = SSL_PROTO_NONE;
+	for (p = q = str; *q; p = q + 1) {
+		for (q = p; *q && *q != ':'; ++q)
+			/* nothing */ ;
+		if (*p == '-' || *p == '+')
+			action = *p++;
+		else
+			action = '=';
+		if (str_is_token(p, "ALL", q - p))
+			current = SSL_PROTO_ALL;
+		else if (str_is_token(p, "NONE", q - p))
+			current = SSL_PROTO_NONE;
+		else if (str_is_token(p, SSL_TXT_SSLV2, q - p))
+			current = SSL_PROTO_SSLv2;
+		else if (str_is_token(p, SSL_TXT_SSLV3, q - p))
+			current = SSL_PROTO_SSLv3;
+		else if (str_is_token(p, "SSL", q - p))
+			current = SSL_PROTO_SSL;
+		else if (str_is_token(p, SSL_TXT_TLSV1, q - p))
+			current = SSL_PROTO_TLSv1;
+#ifdef SSL_OP_NO_TLSv1_1
+		else if (str_is_token(p, SSL_TXT_TLSV1_1, q - p))
+			current = SSL_PROTO_TLSv1_1;
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
+		else if (str_is_token(p, SSL_TXT_TLSV1_2, q - p))
+			current = SSL_PROTO_TLSv1_2;
+#endif
+		else if (str_is_token(p, "TLS", q - p))
+			current = SSL_PROTO_TLS;
+		else
+			elog(FATAL, "invalid SSL protocol list");
+		switch (action) {
+		case '+':
+			result |= current;
+			break;
+		case '-':
+			result &= ~current;
+			break;
+		default:
+			result = current;
+			break;
+		}
+	}
+	/* forcibly disallow SSLv2 */
+	if (result & SSL_PROTO_SSLv2) {
+		elog(WARNING, "removing SSLv2 from SSL protocol list");
+		result &= ~SSL_PROTO_SSLv2;
+	}
+	/* check the result */
+	if (result == 0)
+		elog(FATAL, "could not set the protocol list (no valid protocols available)");
+	elog(DEBUG2, "enabling SSL protocols: %lx", result);
+	/* return the inverse */
+	return (SSL_PROTO_ALL & ~result);
+}
+
 #endif   /* USE_SSL */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 718de95..563267e 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -130,6 +130,7 @@ extern bool optimize_bounded_sort;
 #endif
 
 #ifdef USE_SSL
+extern char *SSLProtocols;
 extern char *SSLCipherSuites;
 #endif
 
@@ -2638,6 +2639,21 @@ static struct config_string ConfigureNamesString[] =
 
 #ifdef USE_SSL
 	{
+		{"ssl_protocols", PGC_POSTMASTER, CONN_AUTH_SECURITY,
+			gettext_noop("Sets the list of allowed SSL protocols."),
+			NULL,
+			GUC_SUPERUSER_ONLY
+		},
+		&SSLProtocols,
+#ifdef USE_SSL
+		"ALL:-SSLv2",
+#else
+		"none",
+#endif
+		NULL, NULL, NULL
+	},
+
+	{
 		{"ssl_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY,
 			gettext_noop("Sets the list of allowed SSL ciphers."),
 			NULL,
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 155af1c..b2e1023 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -78,6 +78,7 @@
 
 #authentication_timeout = 1min		# 1s-600s
 #ssl = off				# (change requires restart)
+#ssl_protocols = 'ALL:-SSLv2'		# allowed SSL protocols
 #ssl_ciphers = 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH'	# allowed SSL ciphers
 					# (change requires restart)
 #ssl_renegotiation_limit = 512MB	# amount of data between renegotiations
#2Michael Paquier
michael.paquier@gmail.com
In reply to: Dag-Erling Smørgrav (#1)
Re: [PATCH] add ssl_protocols configuration option

On Fri, Oct 17, 2014 at 7:58 PM, Dag-Erling Smørgrav <des@des.no> wrote:

The default is "ALL:-SSLv2" in 9.0-9.3 and "ALL:-SSL" in 9.4 and up.
This corresponds to the current hardcoded values, so the default
behavior is unchanged, but the admin now has the option to select a
different settings, e.g. if a serious vulnerability is found in TLS 1.0.

Please note that new features can only be added to the version currently in
development, aka 9.5. You should as well register your patch to the current
commit fest, I think you are still in time:
https://commitfest.postgresql.org/action/commitfest_view?id=24
--
Michael

In reply to: Michael Paquier (#2)
Re: [PATCH] add ssl_protocols configuration option

Michael Paquier <michael.paquier@gmail.com> writes:

Please note that new features can only be added to the version
currently in development, aka 9.5.

I understand this policy. However, this new feature a) has absolutely
no impact unless the admin makes a conscious decision to use it and b)
will make life much easier for everyone if a POODLE-like vulnerability
is discovered in TLS.

You should as well register your patch to the current commit fest, I
think you are still in time:
https://commitfest.postgresql.org/action/commitfest_view?id=24

Thanks for reminding me.

DES
--
Dag-Erling Smørgrav - des@des.no

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

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Dag-Erling Smørgrav (#3)
Re: [PATCH] add ssl_protocols configuration option

Dag-Erling Sm�rgrav wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

Please note that new features can only be added to the version
currently in development, aka 9.5.

I understand this policy. However, this new feature a) has absolutely
no impact unless the admin makes a conscious decision to use it and b)
will make life much easier for everyone if a POODLE-like vulnerability
is discovered in TLS.

I see this as vaguely related to
/messages/by-id/20131114202733.GB7583@eldon.alvh.no-ip.org
where we want to have SSL behavior configurable in the back branches due
to renegotiation issues: there was talk in that thread about introducing
new GUC variables in back branches to control the behavior. The current
patch really doesn't add much in the way of features (SSLv3 support and
so on already exist in back branches) --- what it does add is a way to
control whether these are used.

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

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

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#4)
Re: [PATCH] add ssl_protocols configuration option

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Dag-Erling Sm�rgrav wrote:

I understand this policy. However, this new feature a) has absolutely
no impact unless the admin makes a conscious decision to use it and b)
will make life much easier for everyone if a POODLE-like vulnerability
is discovered in TLS.

I see this as vaguely related to
/messages/by-id/20131114202733.GB7583@eldon.alvh.no-ip.org
where we want to have SSL behavior configurable in the back branches due
to renegotiation issues: there was talk in that thread about introducing
new GUC variables in back branches to control the behavior. The current
patch really doesn't add much in the way of features (SSLv3 support and
so on already exist in back branches) --- what it does add is a way to
control whether these are used.

This looks to me like re-fighting the last war. Such a GUC has zero value
*unless* some situation exactly like the POODLE bug comes up again, and
the odds of that are not high.

Moreover, the GUC could easily be misused to decrease rather than increase
one's security, if it's carelessly set. Remember that we only recently
fixed bugs that prevented use of the latest TLS version. If we have a
setting like this, I fully anticipate that people will set it to "only use
TLS 1.2" and then forget that they ever did that; years from now they'll
still be using 1.2 when it's deprecated.

So I think the argument that this is a useful security contribution is
pretty weak; and on the whole we don't need another marginally-useful GUC.

regards, tom lane

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

#6Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#5)
Re: [PATCH] add ssl_protocols configuration option

On Fri, Oct 17, 2014 at 7:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Dag-Erling Smørgrav wrote:

I understand this policy. However, this new feature a) has absolutely
no impact unless the admin makes a conscious decision to use it and b)
will make life much easier for everyone if a POODLE-like vulnerability
is discovered in TLS.

I see this as vaguely related to
/messages/by-id/20131114202733.GB7583@eldon.alvh.no-ip.org
where we want to have SSL behavior configurable in the back branches due
to renegotiation issues: there was talk in that thread about introducing
new GUC variables in back branches to control the behavior. The current
patch really doesn't add much in the way of features (SSLv3 support and
so on already exist in back branches) --- what it does add is a way to
control whether these are used.

This looks to me like re-fighting the last war. Such a GUC has zero value
*unless* some situation exactly like the POODLE bug comes up again, and
the odds of that are not high.

Moreover, the GUC could easily be misused to decrease rather than increase
one's security, if it's carelessly set. Remember that we only recently
fixed bugs that prevented use of the latest TLS version. If we have a
setting like this, I fully anticipate that people will set it to "only use
TLS 1.2" and then forget that they ever did that; years from now they'll
still be using 1.2 when it's deprecated.

If anything, I think the default should be "default", and then we have
that map out to something. Because once you've initdb'ed, the config
file wil be stuck with a default and we can't change that in a minor
release *if* something like POODLE shows up again. It would require an
admin action, and in this case, it would make us less secure. If we
had a GUC that took the keyword "default" which would mean "whatever
the current version of postgresql thinks is the best" then we could
change the default in a security update if something showed up.

In a case like POODLE we probably wouldn't have done it anyway, as it
doesn't affect our connections (we're not http) and it would have the
potential of breaking some third party clients. However, if something
really bad showed up, we might want to do that.

So I think the argument that this is a useful security contribution is
pretty weak; and on the whole we don't need another marginally-useful GUC.

Having the guc could certainly be useful in some cases. If we do, we
should of course *also* have a corresponding configuration option in
libpq, so I'd say this patch is incomplete if we do want to do it.

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

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

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#6)
Re: [PATCH] add ssl_protocols configuration option

Magnus Hagander <magnus@hagander.net> writes:

If anything, I think the default should be "default", and then we have
that map out to something. Because once you've initdb'ed, the config
file wil be stuck with a default and we can't change that in a minor
release *if* something like POODLE shows up again. It would require an
admin action, and in this case, it would make us less secure. If we
had a GUC that took the keyword "default" which would mean "whatever
the current version of postgresql thinks is the best" then we could
change the default in a security update if something showed up.

That's pretty much isomorphic to just setting the value in the code,
no?

Having the guc could certainly be useful in some cases. If we do, we
should of course *also* have a corresponding configuration option in
libpq, so I'd say this patch is incomplete if we do want to do it.

True. But both of those scenarios posit that no *code* changes are
needed, which is infrequently the case.

And you still have the problem that if an admin does change the setting
away from "default", and forgets to revert that after his next update,
he could in the long run be less secure not more so. Client-side
settings would likely be even harder to get rid of than server-side.

And in the end, if we set values like this from PG --- whether
hard-wired or via a GUC --- the SSL library people will have exactly
the same perspective with regards to *our* values. And not without
reason; we were forcing very obsolete settings up till recently,
because nobody had looked at the issue for a decade. I see no reason
to expect that that history won't repeat itself.

regards, tom lane

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

#8Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#7)
Re: [PATCH] add ssl_protocols configuration option

On Sun, Oct 19, 2014 at 6:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

If anything, I think the default should be "default", and then we have
that map out to something. Because once you've initdb'ed, the config
file wil be stuck with a default and we can't change that in a minor
release *if* something like POODLE shows up again. It would require an
admin action, and in this case, it would make us less secure. If we
had a GUC that took the keyword "default" which would mean "whatever
the current version of postgresql thinks is the best" then we could
change the default in a security update if something showed up.

That's pretty much isomorphic to just setting the value in the code,
no?

No, it would let the user (temporarily) override it.

Having the guc could certainly be useful in some cases. If we do, we
should of course *also* have a corresponding configuration option in
libpq, so I'd say this patch is incomplete if we do want to do it.

True. But both of those scenarios posit that no *code* changes are
needed, which is infrequently the case.

Definitely - it's still very borderline if it's useful.

And you still have the problem that if an admin does change the setting
away from "default", and forgets to revert that after his next update,
he could in the long run be less secure not more so. Client-side
settings would likely be even harder to get rid of than server-side.

True.

And in the end, if we set values like this from PG --- whether
hard-wired or via a GUC --- the SSL library people will have exactly
the same perspective with regards to *our* values. And not without
reason; we were forcing very obsolete settings up till recently,
because nobody had looked at the issue for a decade. I see no reason
to expect that that history won't repeat itself.

The best part would be if we could just leave it up to the SSL
library, but at least the openssl one doesn't have an API that lets us
do that, right? We *have* to pick something...

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

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

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#8)
Re: [PATCH] add ssl_protocols configuration option

Magnus Hagander <magnus@hagander.net> writes:

On Sun, Oct 19, 2014 at 6:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

And in the end, if we set values like this from PG --- whether
hard-wired or via a GUC --- the SSL library people will have exactly
the same perspective with regards to *our* values. And not without
reason; we were forcing very obsolete settings up till recently,
because nobody had looked at the issue for a decade. I see no reason
to expect that that history won't repeat itself.

The best part would be if we could just leave it up to the SSL
library, but at least the openssl one doesn't have an API that lets us
do that, right? We *have* to pick something...

As far as protocol version goes, I think our existing coding basically
says "prefer newest available version, but at least TLS 1.0". I think
that's probably a reasonable approach.

If the patch exposed a GUC that set a "minimum" version, rather than
calling out specific acceptable protocols, it might be less risky.

regards, tom lane

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

#10Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#9)
Re: [PATCH] add ssl_protocols configuration option

On Oct 19, 2014 9:18 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

On Sun, Oct 19, 2014 at 6:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

And in the end, if we set values like this from PG --- whether
hard-wired or via a GUC --- the SSL library people will have exactly
the same perspective with regards to *our* values. And not without
reason; we were forcing very obsolete settings up till recently,
because nobody had looked at the issue for a decade. I see no reason
to expect that that history won't repeat itself.

The best part would be if we could just leave it up to the SSL
library, but at least the openssl one doesn't have an API that lets us
do that, right? We *have* to pick something...

As far as protocol version goes, I think our existing coding basically
says "prefer newest available version, but at least TLS 1.0". I think
that's probably a reasonable approach.

Yes, it does that. Though it only does it on 9.4,but with the facts we know
now, what 9.4+ does is perfectly safe.

/Magnus

In reply to: Tom Lane (#5)
Re: [PATCH] add ssl_protocols configuration option

Tom Lane <tgl@sss.pgh.pa.us> writes:

This looks to me like re-fighting the last war. Such a GUC has zero value
*unless* some situation exactly like the POODLE bug comes up again, and
the odds of that are not high.

Many people would have said the exact same thing before POODLE, and
BEAST, and CRIME, and Heartbleed. You never know what sort of bugs or
weaknesses will show up or when; all you know is that there are a lot of
people working very hard to find these things and exploit them, and that
they *will* succeeded, again and again and again. You can gamble that
PostgreSQL will not be vulnerable due to specific details of its
protocol or how it uses TLS, but that's a gamble which you will
eventually lose.

Moreover, the GUC could easily be misused to decrease rather than increase
one's security, if it's carelessly set.

That's the user's responsibility.

DES
--
Dag-Erling Smørgrav - des@des.no

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

In reply to: Magnus Hagander (#6)
Re: [PATCH] add ssl_protocols configuration option

Magnus Hagander <magnus@hagander.net> writes:

If anything, I think the default should be "default", and then we have
that map out to something. Because once you've initdb'ed, the config
file wil be stuck with a default and we can't change that in a minor
release *if* something like POODLE shows up again.

Actually, I had that in an earlier version of the patch, but I thought
it was too obscure / circular. I can easily re-add it.

In a case like POODLE we probably wouldn't have done it anyway, as it
doesn't affect our connections (we're not http)

If I understand correctly, imaps has been shown to be vulnerable as
well, so I wouldn't be so sure.

Having the guc could certainly be useful in some cases. If we do, we
should of course *also* have a corresponding configuration option in
libpq, so I'd say this patch is incomplete if we do want to do it.

I can update the patch to include the client side.

DES
--
Dag-Erling Smørgrav - des@des.no

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

In reply to: Tom Lane (#7)
Re: [PATCH] add ssl_protocols configuration option

Tom Lane <tgl@sss.pgh.pa.us> writes:

And in the end, if we set values like this from PG --- whether
hard-wired or via a GUC --- the SSL library people will have exactly
the same perspective with regards to *our* values. And not without
reason; we were forcing very obsolete settings up till recently,
because nobody had looked at the issue for a decade. I see no reason
to expect that that history won't repeat itself.

I'm not sure what you're saying here, but - I'm not sure how familiar
you are with the OpenSSL API, but it's insecure by default. There is
*no other choice* for an application than to explicitly select which
protocols it wants to use (or at least which protocols it wants to
avoid). And you can't change OpenSSL, because a ton of old crappy
software is going to break.

DES
--
Dag-Erling Smørgrav - des@des.no

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

In reply to: Tom Lane (#9)
Re: [PATCH] add ssl_protocols configuration option

Tom Lane <tgl@sss.pgh.pa.us> writes:

As far as protocol version goes, I think our existing coding basically
says "prefer newest available version, but at least TLS 1.0". I think
that's probably a reasonable approach.

The client side forces TLS 1.0:

SSL_context = SSL_CTX_new(TLSv1_method());

In typical OpenSSL fashion, this does *not* mean 1.0 or higher. It
means 1.0 exactly.

If the patch exposed a GUC that set a "minimum" version, rather than
calling out specific acceptable protocols, it might be less risky.

Not necessarily. Someone might find a weakness in TLS 1.1 which is not
present in 1.0 because it involves a specific algorithm or mode that 1.0
does not support.

DES
--
Dag-Erling Smørgrav - des@des.no

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

In reply to: Magnus Hagander (#10)
Re: [PATCH] add ssl_protocols configuration option

Magnus Hagander <magnus@hagander.net> writes:

Yes, it does that. Though it only does it on 9.4,but with the facts we
know now, what 9.4+ does is perfectly safe.

Agreed.

DES
--
Dag-Erling Smørgrav - des@des.no

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

#16Michael Paquier
michael.paquier@gmail.com
In reply to: Dag-Erling Smørgrav (#11)
Re: [PATCH] add ssl_protocols configuration option

On Wed, Oct 22, 2014 at 3:12 PM, Dag-Erling Smørgrav <des@des.no> wrote:

Tom Lane <tgl@sss.pgh.pa.us> writes:

This looks to me like re-fighting the last war. Such a GUC has zero

value

*unless* some situation exactly like the POODLE bug comes up again, and
the odds of that are not high.

Many people would have said the exact same thing before POODLE, and
BEAST, and CRIME, and Heartbleed. You never know what sort of bugs or
weaknesses will show up or when; all you know is that there are a lot of
people working very hard to find these things and exploit them, and that
they *will* succeeded, again and again and again. You can gamble that
PostgreSQL will not be vulnerable due to specific details of its
protocol or how it uses TLS, but that's a gamble which you will
eventually lose.

There are some companies, without naming them, that have increased the
resources dedicated to analyze existing security protocols and libraries,
so even if the chances are low, IMO the occurence that similar problems
show up are getting to increase wit the time.

Moreover, the GUC could easily be misused to decrease rather than

increase

one's security, if it's carelessly set.

That's the user's responsibility.

I actually just had a user knocking about having a way to control the
protocols used by server... So, changing my opinion on the matter, that
would be nice to have even such a parameter on back-branches, with
'default' to let the server decide which one is better.
--
Michael

#17Martijn van Oosterhout
kleptog@svana.org
In reply to: Dag-Erling Smørgrav (#12)
Re: [PATCH] add ssl_protocols configuration option

On Wed, Oct 22, 2014 at 03:14:26PM +0200, Dag-Erling Smørgrav wrote:

In a case like POODLE we probably wouldn't have done it anyway, as it
doesn't affect our connections (we're not http)

If I understand correctly, imaps has been shown to be vulnerable as
well, so I wouldn't be so sure.

Reference? It's an SSL3 specific attack, so most servers are not
vulnerable because TLS will negotiate to the highest supported
protocol. So unless one of the client/server doesn't support TLS1.0
there's no issue. The only reason http is vulnerable is because
browsers do protocol downgrading, something strictly forbidden by the
spec.

Additionally, the man-in-the-middle must be able to control the padding
in the startup packet, which just isn't possible (no scripting language
in the client).

Since you can already specify the cipher list, couldn't you just add
-SSLv3 to the cipher list and be done?

Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/

He who writes carelessly confesses thereby at the very outset that he does
not attach much importance to his own thoughts.

-- Arthur Schopenhauer

In reply to: Martijn van Oosterhout (#17)
Re: [PATCH] add ssl_protocols configuration option

Martijn van Oosterhout <kleptog@svana.org> writes:

Dag-Erling Smørgrav <des@des.no> writes:

If I understand correctly, imaps has been shown to be vulnerable as
well, so I wouldn't be so sure.

Reference?

Sorry, no reference. I was told that Thunderbird was vulnerable to
POODLE when talking imaps.

Since you can already specify the cipher list, couldn't you just add
-SSLv3 to the cipher list and be done?

I didn't want to change the existing behavior; all I wanted was to give
users a way to do so if they wish.

DES
--
Dag-Erling Smørgrav - des@des.no

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

#19Martijn van Oosterhout
kleptog@svana.org
In reply to: Dag-Erling Smørgrav (#18)
Re: [PATCH] add ssl_protocols configuration option

On Wed, Oct 22, 2014 at 09:36:59PM +0200, Dag-Erling Smørgrav wrote:

Martijn van Oosterhout <kleptog@svana.org> writes:

Dag-Erling Smørgrav <des@des.no> writes:

If I understand correctly, imaps has been shown to be vulnerable as
well, so I wouldn't be so sure.

Reference?

Sorry, no reference. I was told that Thunderbird was vulnerable to
POODLE when talking imaps.

Ugh, found it. It does the same connection fallback stuff as firefox.

https://securityblog.redhat.com/2014/10/20/can-ssl-3-0-be-fixed-an-analysis-of-the-poodle-attack/

Since you can already specify the cipher list, couldn't you just add
-SSLv3 to the cipher list and be done?

I didn't want to change the existing behavior; all I wanted was to give
users a way to do so if they wish.

I think we should just disable SSL3.0 altogether. The only way this
could cause problems is if people are using PostgreSQL with an OpenSSL
library from last century. As for client libraries, even Windows XP
supports TLS1.0.

Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/

He who writes carelessly confesses thereby at the very outset that he does
not attach much importance to his own thoughts.

-- Arthur Schopenhauer

In reply to: Martijn van Oosterhout (#19)
Re: [PATCH] add ssl_protocols configuration option

Martijn van Oosterhout <kleptog@svana.org> writes:

Dag-Erling Smørgrav <des@des.no> writes:

Martijn van Oosterhout <kleptog@svana.org> writes:

Since you can already specify the cipher list, couldn't you just
add -SSLv3 to the cipher list and be done?

I didn't want to change the existing behavior; all I wanted was to
give users a way to do so if they wish.

I think we should just disable SSL3.0 altogether. The only way this
could cause problems is if people are using PostgreSQL with an OpenSSL
library from last century. As for client libraries, even Windows XP
supports TLS1.0.

As far as I'm concerned (i.e. as far as FreeBSD and the University of
Oslo are concerned), I couldn't care less about anything older than
0.9.8, which is what FreeBSD 8 and RHEL5 have, but I don't feel
comfortable making that decision for other people. On the gripping
hand, no currently supported version of libpq uses anything older than
TLS; 9.0 through 9.3 use TLS 1.0 only while 9.4 uses TLS 1.0 or higher.

DES
--
Dag-Erling Smørgrav - des@des.no

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

#21Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Dag-Erling Smørgrav (#20)
Re: [PATCH] add ssl_protocols configuration option

Dag-Erling Sm�rgrav wrote:

Martijn van Oosterhout <kleptog@svana.org> writes:

Dag-Erling Sm�rgrav <des@des.no> writes:

Martijn van Oosterhout <kleptog@svana.org> writes:

Since you can already specify the cipher list, couldn't you just
add -SSLv3 to the cipher list and be done?

I didn't want to change the existing behavior; all I wanted was to
give users a way to do so if they wish.

I think we should just disable SSL3.0 altogether. The only way this
could cause problems is if people are using PostgreSQL with an OpenSSL
library from last century. As for client libraries, even Windows XP
supports TLS1.0.

As far as I'm concerned (i.e. as far as FreeBSD and the University of
Oslo are concerned), I couldn't care less about anything older than
0.9.8, which is what FreeBSD 8 and RHEL5 have, but I don't feel
comfortable making that decision for other people. On the gripping
hand, no currently supported version of libpq uses anything older than
TLS; 9.0 through 9.3 use TLS 1.0 only while 9.4 uses TLS 1.0 or higher.

OpenSSL just announced a week or two ago that they're abandoning support
for 0.9.8 by the end of next year[1]http://openssl.6102.n7.nabble.com/OpenSSL-0-9-8-End-Of-Life-Announcement-td54155.html, which means its replacements have
been around for a really long time. I think it's fine to drop 0.9.7
support --- we already dropped support for 0.9.6 with the renegotiation
rework[2]/messages/by-id/20130712203252.GH29206@eldon.alvh.no-ip.org in the 9.4 timeframe.

OpenSSL 0.9.7 has already not gotten fixes for all the latest flurry of
security issues, so anyone *is* using SSL but not at least the 0.9.8
branch, they are in trouble.

[1]: http://openssl.6102.n7.nabble.com/OpenSSL-0-9-8-End-Of-Life-Announcement-td54155.html
[2]: /messages/by-id/20130712203252.GH29206@eldon.alvh.no-ip.org

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

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

In reply to: Alvaro Herrera (#21)
Re: [PATCH] add ssl_protocols configuration option

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

OpenSSL just announced a week or two ago that they're abandoning support
for 0.9.8 by the end of next year[1], which means its replacements have
been around for a really long time.

RHEL5 still has 0.9.8e with backported patches and will be supported
until 2017-03-31.

FreeBSD 8.4, 9.1, 9.2 and 9.3 all have 0.9.8y with backported patches.
8.4, 9.1 and 9.2 all expire before OpenSSL 0.9.8, but 9.3 will be
supported until 2016-12-31.

0.9.8 and 1.0.1 are not binary compatible, so upgrading is *not* an
option. We (as in FreeBSD) will have to make do - either develop our
own patches or adapt RedHat's.

OpenSSL 0.9.7 has already not gotten fixes for all the latest flurry of
security issues, so anyone *is* using SSL but not at least the 0.9.8
branch, they are in trouble.

The latest 0.9.8 still only has TLS 1.0, unless they're planning to
backport 1.1 and 1.2 (which I seriously doubt).

DES
--
Dag-Erling Smørgrav - des@des.no

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

#23Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dag-Erling Smørgrav (#22)
Re: [PATCH] add ssl_protocols configuration option

=?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= <des@des.no> writes:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

OpenSSL 0.9.7 has already not gotten fixes for all the latest flurry of
security issues, so anyone *is* using SSL but not at least the 0.9.8
branch, they are in trouble.

The latest 0.9.8 still only has TLS 1.0, unless they're planning to
backport 1.1 and 1.2 (which I seriously doubt).

The upshot of this conversation still seems to be that we don't need to
do anything. Unless I'm misunderstanding something:

(1) No currently supported (or even recently supported) version of either
the backend or libpq will select protocol less than TLS 1.0 unless forced
to via (poorly chosen) configuration settings.

(2) Anyone who is feeling paranoid about shutting off SSLv3 despite (1)
can do so via the existing ssl_ciphers GUC parameter.

Seems to me that's sufficient, not only for now but for the future;
existing OpenSSL practice is that the ciphers string includes categories
corresponding to protocol versions, so you can shut off an old
protocol version there if you need to.

regards, tom lane

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

In reply to: Tom Lane (#23)
Re: [PATCH] add ssl_protocols configuration option

Tom Lane <tgl@sss.pgh.pa.us> writes:

Anyone who is feeling paranoid about shutting off SSLv3 despite (1)
can do so via the existing ssl_ciphers GUC parameter [...] the ciphers
string includes categories corresponding to protocol versions, so you
can shut off an old protocol version there if you need to.

The overlap between SSL 3.0 and TLS 1.0 is 100%:

% openssl ciphers SSLv2 | md5
fe5ff23432f119364a1126ca0776c5db
% openssl ciphers SSLv3 | md5
bde4e4a10b9c3f323c0632ad067e293a
% openssl ciphers TLSv1 | md5
bde4e4a10b9c3f323c0632ad067e293a
% openssl ciphers TLSv1.2 | md5
26c375b6bdefb018b9dd7df463658320

Thus, if you disable all SSL 3.0 ciphers, you also disable TLS 1.0.

DES
--
Dag-Erling Smørgrav - des@des.no

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

#25Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#5)
Re: [PATCH] add ssl_protocols configuration option

On Fri, Oct 17, 2014 at 1:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

This looks to me like re-fighting the last war. Such a GUC has zero value
*unless* some situation exactly like the POODLE bug comes up again, and
the odds of that are not high.

I think it's pretty common for flaws to be discovered in particular
protocols - usually, but not always, the oldest ones. The cost of
adding a new GUC seems pretty small tom me compared to the appealing
potential for users to secure their installation by changing a
configuration setting rather than, say, having to wait for new
packages to be available to install.

Moreover, the GUC could easily be misused to decrease rather than increase
one's security, if it's carelessly set. Remember that we only recently
fixed bugs that prevented use of the latest TLS version. If we have a
setting like this, I fully anticipate that people will set it to "only use
TLS 1.2" and then forget that they ever did that; years from now they'll
still be using 1.2 when it's deprecated.

checkpoint_segments can easily be misused to decrease rather than
increase performance, and autovacuum_naptime can easily be misused to
destroy rather than improve autovacuum behavior. If we only added
GUCs that couldn't be used to make things worse, we'd have no GUCs.
The bottom line for me is that OpenSSL has (a) a seemingly
never-ending supply of serious security flaws and (b) an exposed knob
intended to help users of OpenSSL mitigate the effects of those flaws.
Exposing that knob to our users seems like a good plan; supporting
alternate SSL implementations might be an even better one, not that
the two are mutually exclusive.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

#26Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Dag-Erling Smørgrav (#22)
Re: [PATCH] add ssl_protocols configuration option

Dag-Erling Sm�rgrav wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

OpenSSL just announced a week or two ago that they're abandoning support
for 0.9.8 by the end of next year[1], which means its replacements have
been around for a really long time.

RHEL5 still has 0.9.8e with backported patches and will be supported
until 2017-03-31.

FreeBSD 8.4, 9.1, 9.2 and 9.3 all have 0.9.8y with backported patches.
8.4, 9.1 and 9.2 all expire before OpenSSL 0.9.8, but 9.3 will be
supported until 2016-12-31.

0.9.8 and 1.0.1 are not binary compatible, so upgrading is *not* an
option. We (as in FreeBSD) will have to make do - either develop our
own patches or adapt RedHat's.

I think you misread me. I was saying to desupport 0.9.7, not 0.9.8.

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

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

#27Alex Shulgin
ash@commandprompt.com
In reply to: Dag-Erling Smørgrav (#1)
Re: [PATCH] add ssl_protocols configuration option

Dag-Erling Smørgrav <des@des.no> writes:

The attached patches add an ssl_protocols configuration option which
control which versions of SSL or TLS the server will use. The syntax is
similar to Apache's SSLProtocols directive, except that the list is
colon-separated instead of whitespace-separated, although that is easy
to change if it proves unpopular.

Hello,

Here is my review of the patch against master branch:

* The patch applies and compiles cleanly, except for sgml docs:

postgres.xml:66141: element varlistentry: validity error : Element
varlistentry content does not follow the DTD, expecting (term+ ,
listitem), got (term indexterm listitem)

</para></listitem></varlistentry><varlistentry
^

The fix is to move <indexterm> under the <term> tag in the material
added to config.sgml by the patch.

* The patch works as advertised, though the only way to verify that
connections made with the protocol disabled by the GUC are indeed
rejected is to edit fe-secure-openssl.c to only allow specific TLS
versions. Adding configuration on the libpq side as suggested in the
original discussion might help here.

* The code allows specifying SSLv2 and SSLv3 in the GUC, but removes
them forcibly after parsing the complete string (a warning is issued).
Should we also add a note about this to the documentation?

* In case the list of allowed protocols comes empty a FATAL error
message is logged: "could not set the protocol list (no valid
protocols available)". I think it's worth changing that to "could not
set SSL protocol list..." All other related error/warning logs
include "SSL".

* The added code follows conventions and looks good to me. I didn't
spot any problems in the parser. I've tried a good deal of different
values and all seem to be parsed correctly.

I would try to apply patches for older branches if there is consensus
that we really need this patch and we want to back-patch it.

Thanks.
--
Alex

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

In reply to: Alex Shulgin (#27)
Re: [PATCH] add ssl_protocols configuration option

Alex Shulgin <ash@commandprompt.com> writes:

* The patch works as advertised, though the only way to verify that
connections made with the protocol disabled by the GUC are indeed
rejected is to edit fe-secure-openssl.c to only allow specific TLS
versions. Adding configuration on the libpq side as suggested in the
original discussion might help here.

I can easily do that, but I won't have time until next week or so.

DES
--
Dag-Erling Smørgrav - des@des.no

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

#29Magnus Hagander
magnus@hagander.net
In reply to: Alex Shulgin (#27)
Re: [PATCH] add ssl_protocols configuration option

On Wed, Nov 19, 2014 at 4:34 PM, Alex Shulgin <ash@commandprompt.com> wrote:

Dag-Erling Smørgrav <des@des.no> writes:

The attached patches add an ssl_protocols configuration option which
control which versions of SSL or TLS the server will use. The syntax is
similar to Apache's SSLProtocols directive, except that the list is
colon-separated instead of whitespace-separated, although that is easy
to change if it proves unpopular.

Hello,

Here is my review of the patch against master branch:

* The code allows specifying SSLv2 and SSLv3 in the GUC, but removes
them forcibly after parsing the complete string (a warning is issued).
Should we also add a note about this to the documentation?

I see no reason to accept them at all, if we're going to reject them
later anyway.

We can argue (as was done earlier in this thread) if we can drop SSL
3.0 completely -- but we can *definitely* drop SSLv2, and we should.
But anything that we're going to reject at a later stage anyway, we
should reject early.

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

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

In reply to: Magnus Hagander (#29)
Re: [PATCH] add ssl_protocols configuration option

Magnus Hagander <magnus@hagander.net> writes:

Alex Shulgin <ash@commandprompt.com> writes:

* The code allows specifying SSLv2 and SSLv3 in the GUC, but removes
them forcibly after parsing the complete string (a warning is issued).
Should we also add a note about this to the documentation?

I see no reason to accept them at all, if we're going to reject them
later anyway.

We can argue (as was done earlier in this thread) if we can drop SSL
3.0 completely -- but we can *definitely* drop SSLv2, and we should.
But anything that we're going to reject at a later stage anyway, we
should reject early.

It's not really "early or late", but rather "within the loop or at the
end of it". From the users' perspective, the difference is that they
get (to paraphrase) "SSLv2 is not allowed" instead of "syntax error" and
that they can use constructs such as "ALL:-SSLv2".

DES
--
Dag-Erling Smørgrav - des@des.no

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

#31Magnus Hagander
magnus@hagander.net
In reply to: Dag-Erling Smørgrav (#30)
Re: [PATCH] add ssl_protocols configuration option

On Thu, Nov 20, 2014 at 10:19 AM, Dag-Erling Smørgrav <des@des.no> wrote:

Magnus Hagander <magnus@hagander.net> writes:

Alex Shulgin <ash@commandprompt.com> writes:

* The code allows specifying SSLv2 and SSLv3 in the GUC, but removes
them forcibly after parsing the complete string (a warning is issued).
Should we also add a note about this to the documentation?

I see no reason to accept them at all, if we're going to reject them
later anyway.

We can argue (as was done earlier in this thread) if we can drop SSL
3.0 completely -- but we can *definitely* drop SSLv2, and we should.
But anything that we're going to reject at a later stage anyway, we
should reject early.

It's not really "early or late", but rather "within the loop or at the
end of it". From the users' perspective, the difference is that they
get (to paraphrase) "SSLv2 is not allowed" instead of "syntax error" and
that they can use constructs such as "ALL:-SSLv2".

Ah, I see now - I hadn't looked at the code, just the review comment.
It's a "fallout" from the reverse logic in openssl. Then it makes a
lot more sense.

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

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

#32Alex Shulgin
ash@commandprompt.com
In reply to: Dag-Erling Smørgrav (#28)
Re: [PATCH] add ssl_protocols configuration option

Dag-Erling Smørgrav <des@des.no> writes:

Alex Shulgin <ash@commandprompt.com> writes:

* The patch works as advertised, though the only way to verify that
connections made with the protocol disabled by the GUC are indeed
rejected is to edit fe-secure-openssl.c to only allow specific TLS
versions. Adding configuration on the libpq side as suggested in the
original discussion might help here.

I can easily do that, but I won't have time until next week or so.

I can do that too, just need a hint where to look at in libpq/psql to
add the option.

For SSL we have sslmode and sslcompression, etc. in conninfo, so adding
sslprotocols seems to be an option. As an aside note: should we also
expose a parameter to choose SSL ciphers (would be a separate patch)?

--
Alex

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

In reply to: Alex Shulgin (#32)
Re: [PATCH] add ssl_protocols configuration option

Alex Shulgin <ash@commandprompt.com> writes:

I can do that too, just need a hint where to look at in libpq/psql to
add the option.

The place to *enforce* the option is src/interfaces/libpq/fe-secure.c
(look for SSLv23_method() and SSL_CTX_set_options()). I haven't looked
into how to set it.

DES
--
Dag-Erling Smørgrav - des@des.no

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

#34Alex Shulgin
ash@commandprompt.com
In reply to: Dag-Erling Smørgrav (#33)
Re: [PATCH] add ssl_protocols configuration option

Dag-Erling Smørgrav <des@des.no> writes:

Alex Shulgin <ash@commandprompt.com> writes:

I can do that too, just need a hint where to look at in libpq/psql to
add the option.

The place to *enforce* the option is src/interfaces/libpq/fe-secure.c
(look for SSLv23_method() and SSL_CTX_set_options()). I haven't looked
into how to set it.

Yes, I've figured it out. Guess we'd better share the ssl_protocol
value parser code between libpq and the backend. Any precedent?

--
Alex

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

#35Alex Shulgin
ash@commandprompt.com
In reply to: Alex Shulgin (#34)
1 attachment(s)
Re: [PATCH] add ssl_protocols configuration option

Alex Shulgin <ash@commandprompt.com> writes:

I can do that too, just need a hint where to look at in libpq/psql to
add the option.

The place to *enforce* the option is src/interfaces/libpq/fe-secure.c
(look for SSLv23_method() and SSL_CTX_set_options()). I haven't looked
into how to set it.

Yes, I've figured it out. Guess we'd better share the ssl_protocol
value parser code between libpq and the backend. Any precedent?

OK, looks like I've come up with something workable: I've added
sslprotocol connection string keyword similar to pre-existing
sslcompression, etc.

Please see attached v2 of the original patch. I'm having doubts about
the name of openssl.h header though, libpq-openssl.h?

--
Alex

Attachments:

postgresql-master-ssl-protocols-v2.difftext/x-diffDownload
>From 7cd60e962ce5e7fc10dc52ed9c92b0b2b5c4c7f1 Mon Sep 17 00:00:00 2001
From: Alex Shulgin <ash@commandprompt.com>
Date: Wed, 19 Nov 2014 19:49:29 +0300
Subject: [PATCH] DRAFT: ssl_protocols GUC

---
 doc/src/sgml/config.sgml                      |  29 ++++++
 doc/src/sgml/libpq.sgml                       |  25 ++++++
 src/backend/libpq/Makefile                    |   2 +-
 src/backend/libpq/be-secure-openssl.c         |  29 ++++--
 src/backend/libpq/be-secure.c                 |   3 +-
 src/backend/libpq/openssl.c                   | 124 ++++++++++++++++++++++++++
 src/backend/utils/misc/guc.c                  |  15 ++++
 src/backend/utils/misc/postgresql.conf.sample |   1 +
 src/include/libpq/libpq.h                     |   1 +
 src/include/libpq/openssl.h                   |  50 +++++++++++
 src/interfaces/libpq/Makefile                 |   8 +-
 src/interfaces/libpq/fe-connect.c             |   4 +
 src/interfaces/libpq/fe-secure-openssl.c      |  18 +++-
 src/interfaces/libpq/libpq-int.h              |   1 +
 14 files changed, 300 insertions(+), 10 deletions(-)
 create mode 100644 src/backend/libpq/openssl.c
 create mode 100644 src/include/libpq/openssl.h

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
new file mode 100644
index ab8c263..8b701df
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*************** include_dir 'conf.d'
*** 1027,1032 ****
--- 1027,1061 ----
        </listitem>
       </varlistentry>
  
+      <varlistentry id="guc-ssl-protocols" xreflabel="ssl_protocols">
+       <term><varname>ssl_protocols</varname> (<type>string</type>)
+       <indexterm>
+        <primary><varname>ssl_protocols</> configuration parameter</primary>
+       </indexterm>
+       </term>
+       <listitem>
+        <para>
+         Specifies a colon-separated list of <acronym>SSL</> protocols that are
+         allowed to be used on secure connections. Each entry in the list can
+         be prefixed by a <literal>+</> (add to the current list)
+         or <literal>-</> (remove from the current list). If no prefix is used,
+         the list is replaced with the specified protocol.
+        </para>
+        <para>
+         The full list of supported protocols can be found in the
+         the <application>openssl</> manual page.  In addition to the names of
+         individual protocols, the following keywords can be
+         used: <literal>ALL</> (all protocols supported by the underlying
+         crypto library), <literal>SSL</> (all supported versions
+         of <acronym>SSL</>) and <literal>TLS</> (all supported versions
+         of <acronym>TLS</>).
+        </para>
+        <para>
+         The default is <literal>ALL:-SSL</literal>.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
       <varlistentry id="guc-ssl-ciphers" xreflabel="ssl_ciphers">
        <term><varname>ssl_ciphers</varname> (<type>string</type>)
        <indexterm>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
new file mode 100644
index e23e91d..9ea71a4
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 1230,1235 ****
--- 1230,1250 ----
        </listitem>
       </varlistentry>
  
+      <varlistentry id="libpq-connect-sslprotocols" xreflabel="sslprotocols">
+       <term><literal>sslprotocols</literal></term>
+       <listitem>
+        <para>
+         Specifies a colon-separated list of <acronym>SSL</> protocols that are
+         allowed to be used on secure connections.
+         See <xref linkend="guc-ssl-protocols"> server configuration parameter
+         for format.
+        </para>
+        <para>
+         Defaults to <literal>ALL:-SSL</>.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
       <varlistentry id="libpq-connect-sslcompression" xreflabel="sslcompression">
        <term><literal>sslcompression</literal></term>
        <listitem>
*************** myEventProc(PGEventId evtId, void *evtIn
*** 6711,6716 ****
--- 6726,6741 ----
       </para>
      </listitem>
  
+     <listitem>
+      <para>
+       <indexterm>
+        <primary><envar>PGSSLPROTOCOLS</envar></primary>
+       </indexterm>
+       <envar>PGSSLPROTOCOLS</envar> behaves the same as the <xref
+       linkend="libpq-connect-sslprotocols"> connection parameter.
+      </para>
+     </listitem>
+ 
      <listitem>
       <para>
        <indexterm>
diff --git a/src/backend/libpq/Makefile b/src/backend/libpq/Makefile
new file mode 100644
index 09410c4..1c058ec
*** a/src/backend/libpq/Makefile
--- b/src/backend/libpq/Makefile
*************** OBJS = be-fsstubs.o be-secure.o auth.o c
*** 18,24 ****
         pqformat.o pqmq.o pqsignal.o
  
  ifeq ($(with_openssl),yes)
! OBJS += be-secure-openssl.o
  endif
  
  include $(top_srcdir)/src/backend/common.mk
--- 18,24 ----
         pqformat.o pqmq.o pqsignal.o
  
  ifeq ($(with_openssl),yes)
! OBJS += be-secure-openssl.o openssl.o
  endif
  
  include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
new file mode 100644
index b05364c..3801455
*** a/src/backend/libpq/be-secure-openssl.c
--- b/src/backend/libpq/be-secure-openssl.c
***************
*** 71,76 ****
--- 71,77 ----
  #endif
  
  #include "libpq/libpq.h"
+ #include "libpq/openssl.h"
  #include "tcop/tcopprot.h"
  #include "utils/memutils.h"
  
*************** KWbuHn491xNO25CQWMtem80uKw+pTnisBRF/454n
*** 169,175 ****
  void
  be_tls_init(void)
  {
! 	struct stat buf;
  
  	STACK_OF(X509_NAME) *root_cert_list = NULL;
  
--- 170,178 ----
  void
  be_tls_init(void)
  {
! 	struct stat		buf;
! 	long			protocol_options,
! 					disallow_ssl_options;
  
  	STACK_OF(X509_NAME) *root_cert_list = NULL;
  
*************** be_tls_init(void)
*** 245,259 ****
  							SSLerrmessage())));
  	}
  
! 	/* set up ephemeral DH keys, and disallow SSL v2/v3 while at it */
  	SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb);
! 	SSL_CTX_set_options(SSL_context,
! 						SSL_OP_SINGLE_DH_USE |
! 						SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
  
  	/* set up ephemeral ECDH keys */
  	initialize_ecdh();
  
  	/* set up the allowed cipher list */
  	if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
  		elog(FATAL, "could not set the cipher list (no valid ciphers available)");
--- 248,278 ----
  							SSLerrmessage())));
  	}
  
! 	/* set up ephemeral DH keys */
  	SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb);
! 	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE);
  
  	/* set up ephemeral ECDH keys */
  	initialize_ecdh();
  
+ 	/* set up the allowed protocol list */
+ 	if (!options_from_ssl_protocols_string(SSLProtocols, &protocol_options))
+ 		ereport(FATAL,
+ 				(errmsg("invalid SSL protocol list: %s", SSLProtocols),
+ 				 errhint("are you missing 'ALL:' before '-BADPROTO'?")));
+ 
+ 	/* forcibly disallow SSLv2 and SSLv3 */
+ 	disallow_ssl_options = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;
+ 
+ 	if ((protocol_options & disallow_ssl_options) != disallow_ssl_options)
+ 	{
+ 		elog(WARNING, "removing SSLv2 and SSLv3 from SSL protocol list");
+ 		protocol_options |= disallow_ssl_options;
+ 	}
+ 	elog(DEBUG2, "disabling SSL protocols: %lx", protocol_options);
+ 
+ 	SSL_CTX_set_options(SSL_context, protocol_options);
+ 
  	/* set up the allowed cipher list */
  	if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
  		elog(FATAL, "could not set the cipher list (no valid ciphers available)");
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
new file mode 100644
index 41ec1ad..a5a5f3f
*** a/src/backend/libpq/be-secure.c
--- b/src/backend/libpq/be-secure.c
*************** int			ssl_renegotiation_limit;
*** 52,58 ****
  bool ssl_loaded_verify_locations = false;
  #endif
  
! /* GUC variable controlling SSL cipher list */
  char	   *SSLCipherSuites = NULL;
  
  /* GUC variable for default ECHD curve. */
--- 52,59 ----
  bool ssl_loaded_verify_locations = false;
  #endif
  
! /* GUC variables controlling SSL protocol and cipher list */
! char	   *SSLProtocols = NULL;
  char	   *SSLCipherSuites = NULL;
  
  /* GUC variable for default ECHD curve. */
diff --git a/src/backend/libpq/openssl.c b/src/backend/libpq/openssl.c
new file mode 100644
index ...12cf7c0
*** a/src/backend/libpq/openssl.c
--- b/src/backend/libpq/openssl.c
***************
*** 0 ****
--- 1,124 ----
+ /*-------------------------------------------------------------------------
+  *
+  * openssl.c
+  *	  shared functions for OpenSSL support in the backend and frontend.
+  *
+  *
+  * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
+  * Portions Copyright (c) 1994, Regents of the University of California
+  *
+  *
+  * IDENTIFICATION
+  *	  src/backend/libpq/openssl.c
+  *
+  *	  Contains functions that both backend and frontend can use to setup a
+  *	  secure connection.
+  *
+  *-------------------------------------------------------------------------
+  */
+ 
+ #include "postgres.h"
+ 
+ #include <openssl/ssl.h>
+ #if SSLEAY_VERSION_NUMBER >= 0x0907000L
+ #include <openssl/conf.h>
+ #endif
+ 
+ #include "libpq/libpq.h"
+ #include "libpq/openssl.h"
+ 
+ /*
+  *	Parse the SSL allowed protocol list
+  *
+  *	The logic here is inverted.  OpenSSL does not take a list of
+  *	protocols to use, but a list of protocols to avoid.  We use the
+  *	same bits with the opposite meaning, then invert the result.
+  */
+ 
+ #define SSL_PROTO_SSLv2		SSL_OP_NO_SSLv2
+ #define SSL_PROTO_SSLv3		SSL_OP_NO_SSLv3
+ #define SSL_PROTO_SSL		(SSL_PROTO_SSLv2 | SSL_PROTO_SSLv3)
+ #define SSL_PROTO_TLSv1		SSL_OP_NO_TLSv1
+ #ifdef SSL_OP_NO_TLSv1_1
+ #define SSL_PROTO_TLSv1_1	SSL_OP_NO_TLSv1_1
+ #else
+ #define SSL_PROTO_TLSv1_1	0
+ #endif
+ #ifdef SSL_OP_NO_TLSv1_2
+ #define SSL_PROTO_TLSv1_2	SSL_OP_NO_TLSv1_2
+ #else
+ #define SSL_PROTO_TLSv1_2	0
+ #endif
+ #define SSL_PROTO_TLS		(SSL_PROTO_TLSv1 | SSL_PROTO_TLSv1_1 | SSL_PROTO_TLSv1_2)
+ #define SSL_PROTO_ALL		(SSL_PROTO_SSL | SSL_PROTO_TLS)
+ #define SSL_PROTO_NONE		0
+ 
+ #define str_is_token(str, tok, len) \
+ 	(len == sizeof(tok) - 1 && pg_strncasecmp(str, tok, len) == 0)
+ 
+ 
+ bool
+ options_from_ssl_protocols_string(const char *str, long *options)
+ {
+ 	long result, current;
+ 	const char *p, *q;
+ 	int action;
+ 
+ 	/*
+ 	 * Iterate over the colon-separated list of protocols.  If a protocol is
+ 	 * preceded by a +, it is added to the list.  If it is preceded by a -, it
+ 	 * is removed from the list.  If it is not preceded by anything, the list
+ 	 * is set to exactly that protocol.  "ALL" can be used to indicate all
+ 	 * protocols, "NONE" to indicate no protocols, "SSL" to indicate all SSL
+ 	 * protocols and "TLS" to indicate all TLS protocols.
+ 	 */
+ 	result = SSL_PROTO_NONE;
+ 	for (p = q = str; *q; p = q + 1) {
+ 		for (q = p; *q && *q != ':'; ++q)
+ 			/* nothing */ ;
+ 		if (*p == '-' || *p == '+')
+ 			action = *p++;
+ 		else
+ 			action = '=';
+ 		if (str_is_token(p, "ALL", q - p))
+ 			current = SSL_PROTO_ALL;
+ 		else if (str_is_token(p, "NONE", q - p))
+ 			current = SSL_PROTO_NONE;
+ 		else if (str_is_token(p, SSL_TXT_SSLV2, q - p))
+ 			current = SSL_PROTO_SSLv2;
+ 		else if (str_is_token(p, SSL_TXT_SSLV3, q - p))
+ 			current = SSL_PROTO_SSLv3;
+ 		else if (str_is_token(p, "SSL", q - p))
+ 			current = SSL_PROTO_SSL;
+ 		else if (str_is_token(p, SSL_TXT_TLSV1, q - p))
+ 			current = SSL_PROTO_TLSv1;
+ #ifdef SSL_OP_NO_TLSv1_1
+ 		else if (str_is_token(p, SSL_TXT_TLSV1_1, q - p))
+ 			current = SSL_PROTO_TLSv1_1;
+ #endif
+ #ifdef SSL_OP_NO_TLSv1_2
+ 		else if (str_is_token(p, SSL_TXT_TLSV1_2, q - p))
+ 			current = SSL_PROTO_TLSv1_2;
+ #endif
+ 		else if (str_is_token(p, "TLS", q - p))
+ 			current = SSL_PROTO_TLS;
+ 		else
+ 			return false;
+ 		switch (action) {
+ 		case '+':
+ 			result |= current;
+ 			break;
+ 		case '-':
+ 			result &= ~current;
+ 			break;
+ 		default:
+ 			result = current;
+ 			break;
+ 		}
+ 	}
+ 	if (result == SSL_PROTO_NONE)
+ 		return false;
+ 
+ 	*options = (SSL_PROTO_ALL & ~result);
+ 	return true;
+ }
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
new file mode 100644
index d4d74ba..77b285c
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** static struct config_string ConfigureNam
*** 3235,3240 ****
--- 3235,3255 ----
  	},
  
  	{
+ 		{"ssl_protocols", PGC_POSTMASTER, CONN_AUTH_SECURITY,
+ 			gettext_noop("Sets the list of allowed SSL protocols."),
+ 			NULL,
+ 			GUC_SUPERUSER_ONLY
+ 		},
+ 		&SSLProtocols,
+ #ifdef USE_SSL
+ 		"ALL:-SSL",
+ #else
+ 		"none",
+ #endif
+ 		NULL, NULL, NULL
+ 	},
+ 
+ 	{
  		{"ssl_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY,
  			gettext_noop("Sets the list of allowed SSL ciphers."),
  			NULL,
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
new file mode 100644
index 4a89cb7..9c2abdb
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 79,84 ****
--- 79,85 ----
  
  #authentication_timeout = 1min		# 1s-600s
  #ssl = off				# (change requires restart)
+ #ssl_protocols = 'ALL:-SSL'		# allowed SSL protocols
  #ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers
  					# (change requires restart)
  #ssl_prefer_server_ciphers = on		# (change requires restart)
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
new file mode 100644
index 2a61a9e..eb8dfee
*** a/src/include/libpq/libpq.h
--- b/src/include/libpq/libpq.h
*************** extern ssize_t secure_raw_write(Port *po
*** 108,113 ****
--- 108,114 ----
  extern bool ssl_loaded_verify_locations;
  
  /* GUCs */
+ extern char *SSLProtocols;
  extern char *SSLCipherSuites;
  extern char *SSLECDHCurve;
  extern bool SSLPreferServerCiphers;
diff --git a/src/include/libpq/openssl.h b/src/include/libpq/openssl.h
new file mode 100644
index ...8067774
*** a/src/include/libpq/openssl.h
--- b/src/include/libpq/openssl.h
***************
*** 0 ****
--- 1,50 ----
+ /*-------------------------------------------------------------------------
+  *
+  * openssl.h
+  *	  declarations of shared functions for OpenSSL support in the backend and
+  *	  frontend.
+  *
+  *
+  * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
+  * Portions Copyright (c) 1994, Regents of the University of California
+  *
+  * src/include/libpq/openssl.h
+  *
+  *-------------------------------------------------------------------------
+  */
+ #ifndef LIBPQ_OPENSSL_H
+ #define LIBPQ_OPENSSL_H
+ 
+ /*
+  * Parse the list of SSL protocols in the colon-separated format and return a
+  * bitmask of options to be passed to SSL_CTX_set_options().
+  *
+  * Recognized protocol names are SSLv2, SSLv3 and TLSv1.
+  *
+  * In case the OpenSSL version defines SSL_OP_NO_TLSv1_1, the string TLSv1.1
+  * is also recognized (ditto for SSL_OP_NO_TLSv1_2 and TLSv1.2).
+  *
+  * Additionally, SSL means all supported SSL versions, the same goes for TLS.
+  * Furthermore, ALL means all supported protocols (and NONE, well you've
+  * guessed it.)
+  *
+  * A plus sign before the protocol name includes a protocol in the list, minus
+  * removes, otherwise replaces the whole list.
+  *
+  * Example: ALL:-SSL
+  *
+  * The above example includes all supported protocols except all SSL versions
+  * (SSLv2 and SSLv3).
+  *
+  * Since OpenSSL API only let you *exclude* protocols in the options, the
+  * resulting bitmask will have SSL_OP_NO_<protocol> flags set for which there
+  * were protocols listed with a minus sign.
+  *
+  * Returns true on success, false otherwise (i.e. unrecognized protocol
+  * found in the list).
+  *
+  * On success, stores the result of parsing in the options out-parameter.
+  */
+ extern bool options_from_ssl_protocols_string(const char *str, long *options);
+ 
+ #endif /* LIBPQ_OPENSSL_H */
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
new file mode 100644
index 18d9b85..15c00a3
*** a/src/interfaces/libpq/Makefile
--- b/src/interfaces/libpq/Makefile
*************** OBJS += chklocale.o inet_net_ntop.o nobl
*** 40,51 ****
--- 40,55 ----
  # libpgport C files that are needed if identified by configure
  OBJS += $(filter crypt.o getaddrinfo.o getpeereid.o inet_aton.o open.o system.o snprintf.o strerror.o strlcpy.o win32error.o win32setlocale.o, $(LIBOBJS))
  # backend/libpq
+ backend_c = ip.c md5.c
  OBJS += ip.o md5.o
  # utils/mb
  OBJS += encnames.o wchar.o
  
  ifeq ($(with_openssl),yes)
  OBJS += fe-secure-openssl.o
+ # backend/libpq
+ backend_c += openssl.c
+ OBJS += openssl.o
  endif
  
  ifeq ($(PORTNAME), cygwin)
*************** backend_src = $(top_srcdir)/src/backend
*** 96,102 ****
  chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c system.c pgsleep.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
  	rm -f $@ && $(LN_S) $< .
  
! ip.c md5.c: % : $(backend_src)/libpq/%
  	rm -f $@ && $(LN_S) $< .
  
  encnames.c wchar.c: % : $(backend_src)/utils/mb/%
--- 100,106 ----
  chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c system.c pgsleep.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
  	rm -f $@ && $(LN_S) $< .
  
! $(backend_c): % : $(backend_src)/libpq/%
  	rm -f $@ && $(LN_S) $< .
  
  encnames.c wchar.c: % : $(backend_src)/utils/mb/%
*************** clean distclean: clean-lib
*** 156,162 ****
  	rm -f inet_net_ntop.c noblock.c pgstrcasecmp.c pqsignal.c thread.c
  	rm -f chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c open.c system.c snprintf.c strerror.c strlcpy.c win32error.c win32setlocale.c
  	rm -f pgsleep.c
! 	rm -f md5.c ip.c
  	rm -f encnames.c wchar.c
  
  maintainer-clean: distclean maintainer-clean-lib
--- 160,166 ----
  	rm -f inet_net_ntop.c noblock.c pgstrcasecmp.c pqsignal.c thread.c
  	rm -f chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c open.c system.c snprintf.c strerror.c strlcpy.c win32error.c win32setlocale.c
  	rm -f pgsleep.c
! 	rm -f $(backend_c)
  	rm -f encnames.c wchar.c
  
  maintainer-clean: distclean maintainer-clean-lib
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
new file mode 100644
index 703cbac..0e10a58
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
*************** static const internalPQconninfoOption PQ
*** 254,259 ****
--- 254,263 ----
  		"SSL-Mode", "", 12,		/* sizeof("verify-full") == 12 */
  	offsetof(struct pg_conn, sslmode)},
  
+ 	{"sslprotocols", "PGSSLPROTOCOLS", "ALL:-SSL", NULL,
+ 		"SSL-Protocols", "", 10,
+ 	offsetof(struct pg_conn, sslprotocols)},
+ 
  	{"sslcompression", "PGSSLCOMPRESSION", "1", NULL,
  		"SSL-Compression", "", 1,
  	offsetof(struct pg_conn, sslcompression)},
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
new file mode 100644
index 78aa46d..0d174bd
*** a/src/interfaces/libpq/fe-secure-openssl.c
--- b/src/interfaces/libpq/fe-secure-openssl.c
***************
*** 29,34 ****
--- 29,35 ----
  #include "libpq-fe.h"
  #include "fe-auth.h"
  #include "libpq-int.h"
+ #include "libpq/openssl.h"
  
  #ifdef WIN32
  #include "win32.h"
*************** pgtls_init(PGconn *conn)
*** 806,811 ****
--- 807,814 ----
  
  	if (!SSL_context)
  	{
+ 		long	protocol_options;
+ 
  		if (pq_init_ssl_lib)
  		{
  #if SSLEAY_VERSION_NUMBER >= 0x00907000L
*************** pgtls_init(PGconn *conn)
*** 836,843 ****
  			return -1;
  		}
  
  		/* Disable old protocol versions */
! 		SSL_CTX_set_options(SSL_context, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
  
  		/*
  		 * Disable OpenSSL's moving-write-buffer sanity check, because it
--- 839,859 ----
  			return -1;
  		}
  
+ 		/* set up the allowed protocol list */
+ 		if (!options_from_ssl_protocols_string(conn->sslprotocols,
+ 											   &protocol_options))
+ 		{
+ 			printfPQExpBuffer(&conn->errorMessage,
+ 							  libpq_gettext("invalid SSL protocols list (are you missing 'ALL:' before '-BADPROTO'?)\n"));
+ #ifdef ENABLE_THREAD_SAFETY
+ 			pthread_mutex_unlock(&ssl_config_mutex);
+ #endif
+ 			return -1;
+ 		}
+ 
  		/* Disable old protocol versions */
! 		protocol_options |= SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;
! 		SSL_CTX_set_options(SSL_context, protocol_options);
  
  		/*
  		 * Disable OpenSSL's moving-write-buffer sanity check, because it
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
new file mode 100644
index 4ef46ff..d7c2b78
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
*************** struct pg_conn
*** 324,329 ****
--- 324,330 ----
  	char	   *keepalives_count;		/* maximum number of TCP keepalive
  										 * retransmits */
  	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
+ 	char	   *sslprotocols;	/* SSL protocols (e.g, "ALL:-SSL") */
  	char	   *sslcompression; /* SSL compression (0 or 1) */
  	char	   *sslkey;			/* client key filename */
  	char	   *sslcert;		/* client certificate filename */
-- 
2.1.0

In reply to: Alex Shulgin (#35)
Re: [PATCH] add ssl_protocols configuration option

Alex Shulgin <ash@commandprompt.com> writes:

OK, looks like I've come up with something workable: I've added
sslprotocol connection string keyword similar to pre-existing
sslcompression, etc. Please see attached v2 of the original patch.
I'm having doubts about the name of openssl.h header though,
libpq-openssl.h?

Perhaps ssloptions.[ch], unless you plan to add non-option-related code
there later?

BTW, there is no Regent code in your openssl.c, so the copyright
statement is incorrect.

DES
--
Dag-Erling Smørgrav - des@des.no

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

#37Alex Shulgin
ash@commandprompt.com
In reply to: Dag-Erling Smørgrav (#36)
Re: [PATCH] add ssl_protocols configuration option

Dag-Erling Smørgrav <des@des.no> writes:

Alex Shulgin <ash@commandprompt.com> writes:

OK, looks like I've come up with something workable: I've added
sslprotocol connection string keyword similar to pre-existing
sslcompression, etc. Please see attached v2 of the original patch.
I'm having doubts about the name of openssl.h header though,
libpq-openssl.h?

Perhaps ssloptions.[ch], unless you plan to add non-option-related code
there later?

I don't think anything else than common options-related code fits in
there, so ssloptions.c makes sense to me.

BTW, there is no Regent code in your openssl.c, so the copyright
statement is incorrect.

Good catch, I just blindly copied that from some other file.

--
Alex

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

#38Michael Paquier
michael.paquier@gmail.com
In reply to: Alex Shulgin (#37)
Re: [PATCH] add ssl_protocols configuration option

On Thu, Nov 27, 2014 at 8:51 PM, Alex Shulgin <ash@commandprompt.com> wrote:

Dag-Erling Smørgrav <des@des.no> writes:

Alex Shulgin <ash@commandprompt.com> writes:

OK, looks like I've come up with something workable: I've added
sslprotocol connection string keyword similar to pre-existing
sslcompression, etc. Please see attached v2 of the original patch.
I'm having doubts about the name of openssl.h header though,
libpq-openssl.h?

Perhaps ssloptions.[ch], unless you plan to add non-option-related code
there later?

I don't think anything else than common options-related code fits in
there, so ssloptions.c makes sense to me.

BTW, there is no Regent code in your openssl.c, so the copyright
statement is incorrect.

Good catch, I just blindly copied that from some other file.

There have been arguments in favor and against this patch... But
marking it as returned with feedback because of a lack of activity in
the last couple of weeks. Feel free to update if you think that's not
correct, and please move this patch to commit fest 2014-12.
--
Michael

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

#39Alex Shulgin
ash@commandprompt.com
In reply to: Michael Paquier (#38)
1 attachment(s)
Re: [PATCH] add ssl_protocols configuration option

Michael Paquier <michael.paquier@gmail.com> writes:

Perhaps ssloptions.[ch], unless you plan to add non-option-related code
there later?

I don't think anything else than common options-related code fits in
there, so ssloptions.c makes sense to me.

BTW, there is no Regent code in your openssl.c, so the copyright
statement is incorrect.

Good catch, I just blindly copied that from some other file.

There have been arguments in favor and against this patch... But
marking it as returned with feedback because of a lack of activity in
the last couple of weeks. Feel free to update if you think that's not
correct, and please move this patch to commit fest 2014-12.

Attached is a new version that addresses the earlier feedback: renamed
the added *.[ch] files and removed incorrect copyright line.

I'm moving this to the current CF.

--
Alex

Attachments:

ssl-protocols-v3.difftext/x-diffDownload
>From 18388300f9ed34fa47c66b8a2da098aeb19a7f71 Mon Sep 17 00:00:00 2001
From: Alex Shulgin <ash@commandprompt.com>
Date: Wed, 19 Nov 2014 19:49:29 +0300
Subject: [PATCH] DRAFT: ssl_protocols GUC

---
 doc/src/sgml/config.sgml                      |  29 ++++++
 doc/src/sgml/libpq.sgml                       |  25 ++++++
 src/backend/libpq/Makefile                    |   2 +-
 src/backend/libpq/be-secure-openssl.c         |  29 ++++--
 src/backend/libpq/be-secure.c                 |   3 +-
 src/backend/libpq/ssloptions.c                | 123 ++++++++++++++++++++++++++
 src/backend/utils/misc/guc.c                  |  15 ++++
 src/backend/utils/misc/postgresql.conf.sample |   1 +
 src/include/libpq/libpq.h                     |   1 +
 src/include/libpq/ssloptions.h                |  48 ++++++++++
 src/interfaces/libpq/Makefile                 |   8 +-
 src/interfaces/libpq/fe-connect.c             |   4 +
 src/interfaces/libpq/fe-secure-openssl.c      |  18 +++-
 src/interfaces/libpq/libpq-int.h              |   1 +
 14 files changed, 297 insertions(+), 10 deletions(-)
 create mode 100644 src/backend/libpq/ssloptions.c
 create mode 100644 src/include/libpq/ssloptions.h

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
new file mode 100644
index 48ae3e4..699f0f9
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*************** include_dir 'conf.d'
*** 1055,1060 ****
--- 1055,1089 ----
        </listitem>
       </varlistentry>
  
+      <varlistentry id="guc-ssl-protocols" xreflabel="ssl_protocols">
+       <term><varname>ssl_protocols</varname> (<type>string</type>)
+       <indexterm>
+        <primary><varname>ssl_protocols</> configuration parameter</primary>
+       </indexterm>
+       </term>
+       <listitem>
+        <para>
+         Specifies a colon-separated list of <acronym>SSL</> protocols that are
+         allowed to be used on secure connections. Each entry in the list can
+         be prefixed by a <literal>+</> (add to the current list)
+         or <literal>-</> (remove from the current list). If no prefix is used,
+         the list is replaced with the specified protocol.
+        </para>
+        <para>
+         The full list of supported protocols can be found in the
+         the <application>openssl</> manual page.  In addition to the names of
+         individual protocols, the following keywords can be
+         used: <literal>ALL</> (all protocols supported by the underlying
+         crypto library), <literal>SSL</> (all supported versions
+         of <acronym>SSL</>) and <literal>TLS</> (all supported versions
+         of <acronym>TLS</>).
+        </para>
+        <para>
+         The default is <literal>ALL:-SSL</literal>.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
       <varlistentry id="guc-ssl-ciphers" xreflabel="ssl_ciphers">
        <term><varname>ssl_ciphers</varname> (<type>string</type>)
        <indexterm>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
new file mode 100644
index d829a4b..62ee0b4
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 1230,1235 ****
--- 1230,1250 ----
        </listitem>
       </varlistentry>
  
+      <varlistentry id="libpq-connect-sslprotocols" xreflabel="sslprotocols">
+       <term><literal>sslprotocols</literal></term>
+       <listitem>
+        <para>
+         Specifies a colon-separated list of <acronym>SSL</> protocols that are
+         allowed to be used on secure connections.
+         See <xref linkend="guc-ssl-protocols"> server configuration parameter
+         for format.
+        </para>
+        <para>
+         Defaults to <literal>ALL:-SSL</>.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
       <varlistentry id="libpq-connect-sslcompression" xreflabel="sslcompression">
        <term><literal>sslcompression</literal></term>
        <listitem>
*************** myEventProc(PGEventId evtId, void *evtIn
*** 6693,6698 ****
--- 6708,6723 ----
       </para>
      </listitem>
  
+     <listitem>
+      <para>
+       <indexterm>
+        <primary><envar>PGSSLPROTOCOLS</envar></primary>
+       </indexterm>
+       <envar>PGSSLPROTOCOLS</envar> behaves the same as the <xref
+       linkend="libpq-connect-sslprotocols"> connection parameter.
+      </para>
+     </listitem>
+ 
      <listitem>
       <para>
        <indexterm>
diff --git a/src/backend/libpq/Makefile b/src/backend/libpq/Makefile
new file mode 100644
index 09410c4..62abd46
*** a/src/backend/libpq/Makefile
--- b/src/backend/libpq/Makefile
*************** OBJS = be-fsstubs.o be-secure.o auth.o c
*** 18,24 ****
         pqformat.o pqmq.o pqsignal.o
  
  ifeq ($(with_openssl),yes)
! OBJS += be-secure-openssl.o
  endif
  
  include $(top_srcdir)/src/backend/common.mk
--- 18,24 ----
         pqformat.o pqmq.o pqsignal.o
  
  ifeq ($(with_openssl),yes)
! OBJS += be-secure-openssl.o ssloptions.o
  endif
  
  include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
new file mode 100644
index b05364c..ad7e0f6
*** a/src/backend/libpq/be-secure-openssl.c
--- b/src/backend/libpq/be-secure-openssl.c
***************
*** 71,76 ****
--- 71,77 ----
  #endif
  
  #include "libpq/libpq.h"
+ #include "libpq/ssloptions.h"
  #include "tcop/tcopprot.h"
  #include "utils/memutils.h"
  
*************** KWbuHn491xNO25CQWMtem80uKw+pTnisBRF/454n
*** 169,175 ****
  void
  be_tls_init(void)
  {
! 	struct stat buf;
  
  	STACK_OF(X509_NAME) *root_cert_list = NULL;
  
--- 170,178 ----
  void
  be_tls_init(void)
  {
! 	struct stat		buf;
! 	long			protocol_options,
! 					disallow_ssl_options;
  
  	STACK_OF(X509_NAME) *root_cert_list = NULL;
  
*************** be_tls_init(void)
*** 245,259 ****
  							SSLerrmessage())));
  	}
  
! 	/* set up ephemeral DH keys, and disallow SSL v2/v3 while at it */
  	SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb);
! 	SSL_CTX_set_options(SSL_context,
! 						SSL_OP_SINGLE_DH_USE |
! 						SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
  
  	/* set up ephemeral ECDH keys */
  	initialize_ecdh();
  
  	/* set up the allowed cipher list */
  	if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
  		elog(FATAL, "could not set the cipher list (no valid ciphers available)");
--- 248,278 ----
  							SSLerrmessage())));
  	}
  
! 	/* set up ephemeral DH keys */
  	SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb);
! 	SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE);
  
  	/* set up ephemeral ECDH keys */
  	initialize_ecdh();
  
+ 	/* set up the allowed protocol list */
+ 	if (!options_from_ssl_protocols_string(SSLProtocols, &protocol_options))
+ 		ereport(FATAL,
+ 				(errmsg("invalid SSL protocol list: %s", SSLProtocols),
+ 				 errhint("are you missing 'ALL:' before '-BADPROTO'?")));
+ 
+ 	/* forcibly disallow SSLv2 and SSLv3 */
+ 	disallow_ssl_options = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;
+ 
+ 	if ((protocol_options & disallow_ssl_options) != disallow_ssl_options)
+ 	{
+ 		elog(WARNING, "removing SSLv2 and SSLv3 from SSL protocol list");
+ 		protocol_options |= disallow_ssl_options;
+ 	}
+ 	elog(DEBUG2, "disabling SSL protocols: %lx", protocol_options);
+ 
+ 	SSL_CTX_set_options(SSL_context, protocol_options);
+ 
  	/* set up the allowed cipher list */
  	if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
  		elog(FATAL, "could not set the cipher list (no valid ciphers available)");
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
new file mode 100644
index 41ec1ad..a5a5f3f
*** a/src/backend/libpq/be-secure.c
--- b/src/backend/libpq/be-secure.c
*************** int			ssl_renegotiation_limit;
*** 52,58 ****
  bool ssl_loaded_verify_locations = false;
  #endif
  
! /* GUC variable controlling SSL cipher list */
  char	   *SSLCipherSuites = NULL;
  
  /* GUC variable for default ECHD curve. */
--- 52,59 ----
  bool ssl_loaded_verify_locations = false;
  #endif
  
! /* GUC variables controlling SSL protocol and cipher list */
! char	   *SSLProtocols = NULL;
  char	   *SSLCipherSuites = NULL;
  
  /* GUC variable for default ECHD curve. */
diff --git a/src/backend/libpq/ssloptions.c b/src/backend/libpq/ssloptions.c
new file mode 100644
index ...e161c1e
*** a/src/backend/libpq/ssloptions.c
--- b/src/backend/libpq/ssloptions.c
***************
*** 0 ****
--- 1,123 ----
+ /*-------------------------------------------------------------------------
+  *
+  * ssloptions.c
+  *	  shared functions for setting OpenSSL options.
+  *
+  *
+  * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
+  *
+  *
+  * IDENTIFICATION
+  *	  src/backend/libpq/ssloptions.c
+  *
+  *	  Contains functions that both backend and frontend can use to setup
+  *	  options for a secure connection.
+  *
+  *-------------------------------------------------------------------------
+  */
+ 
+ #include "postgres.h"
+ 
+ #include <openssl/ssl.h>
+ #if SSLEAY_VERSION_NUMBER >= 0x0907000L
+ #include <openssl/conf.h>
+ #endif
+ 
+ #include "libpq/libpq.h"
+ #include "libpq/ssloptions.h"
+ 
+ /*
+  *	Parse the SSL allowed protocol list
+  *
+  *	The logic here is inverted.  OpenSSL does not take a list of
+  *	protocols to use, but a list of protocols to avoid.  We use the
+  *	same bits with the opposite meaning, then invert the result.
+  */
+ 
+ #define SSL_PROTO_SSLv2		SSL_OP_NO_SSLv2
+ #define SSL_PROTO_SSLv3		SSL_OP_NO_SSLv3
+ #define SSL_PROTO_SSL		(SSL_PROTO_SSLv2 | SSL_PROTO_SSLv3)
+ #define SSL_PROTO_TLSv1		SSL_OP_NO_TLSv1
+ #ifdef SSL_OP_NO_TLSv1_1
+ #define SSL_PROTO_TLSv1_1	SSL_OP_NO_TLSv1_1
+ #else
+ #define SSL_PROTO_TLSv1_1	0
+ #endif
+ #ifdef SSL_OP_NO_TLSv1_2
+ #define SSL_PROTO_TLSv1_2	SSL_OP_NO_TLSv1_2
+ #else
+ #define SSL_PROTO_TLSv1_2	0
+ #endif
+ #define SSL_PROTO_TLS		(SSL_PROTO_TLSv1 | SSL_PROTO_TLSv1_1 | SSL_PROTO_TLSv1_2)
+ #define SSL_PROTO_ALL		(SSL_PROTO_SSL | SSL_PROTO_TLS)
+ #define SSL_PROTO_NONE		0
+ 
+ #define str_is_token(str, tok, len) \
+ 	(len == sizeof(tok) - 1 && pg_strncasecmp(str, tok, len) == 0)
+ 
+ 
+ bool
+ options_from_ssl_protocols_string(const char *str, long *options)
+ {
+ 	long result, current;
+ 	const char *p, *q;
+ 	int action;
+ 
+ 	/*
+ 	 * Iterate over the colon-separated list of protocols.  If a protocol is
+ 	 * preceded by a +, it is added to the list.  If it is preceded by a -, it
+ 	 * is removed from the list.  If it is not preceded by anything, the list
+ 	 * is set to exactly that protocol.  "ALL" can be used to indicate all
+ 	 * protocols, "NONE" to indicate no protocols, "SSL" to indicate all SSL
+ 	 * protocols and "TLS" to indicate all TLS protocols.
+ 	 */
+ 	result = SSL_PROTO_NONE;
+ 	for (p = q = str; *q; p = q + 1) {
+ 		for (q = p; *q && *q != ':'; ++q)
+ 			/* nothing */ ;
+ 		if (*p == '-' || *p == '+')
+ 			action = *p++;
+ 		else
+ 			action = '=';
+ 		if (str_is_token(p, "ALL", q - p))
+ 			current = SSL_PROTO_ALL;
+ 		else if (str_is_token(p, "NONE", q - p))
+ 			current = SSL_PROTO_NONE;
+ 		else if (str_is_token(p, SSL_TXT_SSLV2, q - p))
+ 			current = SSL_PROTO_SSLv2;
+ 		else if (str_is_token(p, SSL_TXT_SSLV3, q - p))
+ 			current = SSL_PROTO_SSLv3;
+ 		else if (str_is_token(p, "SSL", q - p))
+ 			current = SSL_PROTO_SSL;
+ 		else if (str_is_token(p, SSL_TXT_TLSV1, q - p))
+ 			current = SSL_PROTO_TLSv1;
+ #ifdef SSL_OP_NO_TLSv1_1
+ 		else if (str_is_token(p, SSL_TXT_TLSV1_1, q - p))
+ 			current = SSL_PROTO_TLSv1_1;
+ #endif
+ #ifdef SSL_OP_NO_TLSv1_2
+ 		else if (str_is_token(p, SSL_TXT_TLSV1_2, q - p))
+ 			current = SSL_PROTO_TLSv1_2;
+ #endif
+ 		else if (str_is_token(p, "TLS", q - p))
+ 			current = SSL_PROTO_TLS;
+ 		else
+ 			return false;
+ 		switch (action) {
+ 		case '+':
+ 			result |= current;
+ 			break;
+ 		case '-':
+ 			result &= ~current;
+ 			break;
+ 		default:
+ 			result = current;
+ 			break;
+ 		}
+ 	}
+ 	if (result == SSL_PROTO_NONE)
+ 		return false;
+ 
+ 	*options = (SSL_PROTO_ALL & ~result);
+ 	return true;
+ }
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
new file mode 100644
index b1bff7f..c54c55b
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** static struct config_string ConfigureNam
*** 3245,3250 ****
--- 3245,3265 ----
  	},
  
  	{
+ 		{"ssl_protocols", PGC_POSTMASTER, CONN_AUTH_SECURITY,
+ 			gettext_noop("Sets the list of allowed SSL protocols."),
+ 			NULL,
+ 			GUC_SUPERUSER_ONLY
+ 		},
+ 		&SSLProtocols,
+ #ifdef USE_SSL
+ 		"ALL:-SSL",
+ #else
+ 		"none",
+ #endif
+ 		NULL, NULL, NULL
+ 	},
+ 
+ 	{
  		{"ssl_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY,
  			gettext_noop("Sets the list of allowed SSL ciphers."),
  			NULL,
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
new file mode 100644
index b053659..bb44aa6
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 79,84 ****
--- 79,85 ----
  
  #authentication_timeout = 1min		# 1s-600s
  #ssl = off				# (change requires restart)
+ #ssl_protocols = 'ALL:-SSL'		# allowed SSL protocols
  #ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers
  					# (change requires restart)
  #ssl_prefer_server_ciphers = on		# (change requires restart)
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
new file mode 100644
index 2a61a9e..eb8dfee
*** a/src/include/libpq/libpq.h
--- b/src/include/libpq/libpq.h
*************** extern ssize_t secure_raw_write(Port *po
*** 108,113 ****
--- 108,114 ----
  extern bool ssl_loaded_verify_locations;
  
  /* GUCs */
+ extern char *SSLProtocols;
  extern char *SSLCipherSuites;
  extern char *SSLECDHCurve;
  extern bool SSLPreferServerCiphers;
diff --git a/src/include/libpq/ssloptions.h b/src/include/libpq/ssloptions.h
new file mode 100644
index ...92e8561
*** a/src/include/libpq/ssloptions.h
--- b/src/include/libpq/ssloptions.h
***************
*** 0 ****
--- 1,48 ----
+ /*-------------------------------------------------------------------------
+  *
+  * ssloptions.h
+  *	  declarations of shared functions for setting OpenSSL options.
+  *
+  *
+  * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
+  *
+  * src/include/libpq/ssloptions.h
+  *
+  *-------------------------------------------------------------------------
+  */
+ #ifndef SSLOPTIONS_H
+ #define SSLOPTIONS_H
+ 
+ /*
+  * Parse the list of SSL protocols in the colon-separated format and return a
+  * bitmask of options to be passed to SSL_CTX_set_options().
+  *
+  * Recognized protocol names are SSLv2, SSLv3 and TLSv1.
+  *
+  * In case the OpenSSL version defines SSL_OP_NO_TLSv1_1, the string TLSv1.1
+  * is also recognized (ditto for SSL_OP_NO_TLSv1_2 and TLSv1.2).
+  *
+  * Additionally, SSL means all supported SSL versions, the same goes for TLS.
+  * Furthermore, ALL means all supported protocols (and NONE, well you've
+  * guessed it.)
+  *
+  * A plus sign before the protocol name includes a protocol in the list, minus
+  * removes, otherwise replaces the whole list.
+  *
+  * Example: ALL:-SSL
+  *
+  * The above example includes all supported protocols except all SSL versions
+  * (SSLv2 and SSLv3).
+  *
+  * Since OpenSSL API only let you *exclude* protocols in the options, the
+  * resulting bitmask will have SSL_OP_NO_<protocol> flags set for which there
+  * were protocols listed with a minus sign.
+  *
+  * Returns true on success, false otherwise (i.e. unrecognized protocol
+  * found in the list).
+  *
+  * On success, stores the result of parsing in the options out-parameter.
+  */
+ extern bool options_from_ssl_protocols_string(const char *str, long *options);
+ 
+ #endif /* SSLOPTIONS_H */
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
new file mode 100644
index 18d9b85..31acdc3
*** a/src/interfaces/libpq/Makefile
--- b/src/interfaces/libpq/Makefile
*************** OBJS += chklocale.o inet_net_ntop.o nobl
*** 40,51 ****
--- 40,55 ----
  # libpgport C files that are needed if identified by configure
  OBJS += $(filter crypt.o getaddrinfo.o getpeereid.o inet_aton.o open.o system.o snprintf.o strerror.o strlcpy.o win32error.o win32setlocale.o, $(LIBOBJS))
  # backend/libpq
+ backend_c = ip.c md5.c
  OBJS += ip.o md5.o
  # utils/mb
  OBJS += encnames.o wchar.o
  
  ifeq ($(with_openssl),yes)
  OBJS += fe-secure-openssl.o
+ # backend/libpq
+ backend_c += ssloptions.c
+ OBJS += ssloptions.o
  endif
  
  ifeq ($(PORTNAME), cygwin)
*************** backend_src = $(top_srcdir)/src/backend
*** 96,102 ****
  chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c system.c pgsleep.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
  	rm -f $@ && $(LN_S) $< .
  
! ip.c md5.c: % : $(backend_src)/libpq/%
  	rm -f $@ && $(LN_S) $< .
  
  encnames.c wchar.c: % : $(backend_src)/utils/mb/%
--- 100,106 ----
  chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c system.c pgsleep.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
  	rm -f $@ && $(LN_S) $< .
  
! $(backend_c): % : $(backend_src)/libpq/%
  	rm -f $@ && $(LN_S) $< .
  
  encnames.c wchar.c: % : $(backend_src)/utils/mb/%
*************** clean distclean: clean-lib
*** 156,162 ****
  	rm -f inet_net_ntop.c noblock.c pgstrcasecmp.c pqsignal.c thread.c
  	rm -f chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c open.c system.c snprintf.c strerror.c strlcpy.c win32error.c win32setlocale.c
  	rm -f pgsleep.c
! 	rm -f md5.c ip.c
  	rm -f encnames.c wchar.c
  
  maintainer-clean: distclean maintainer-clean-lib
--- 160,166 ----
  	rm -f inet_net_ntop.c noblock.c pgstrcasecmp.c pqsignal.c thread.c
  	rm -f chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c open.c system.c snprintf.c strerror.c strlcpy.c win32error.c win32setlocale.c
  	rm -f pgsleep.c
! 	rm -f $(backend_c)
  	rm -f encnames.c wchar.c
  
  maintainer-clean: distclean maintainer-clean-lib
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
new file mode 100644
index 3bac2bc..33dc826
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
*************** static const internalPQconninfoOption PQ
*** 254,259 ****
--- 254,263 ----
  		"SSL-Mode", "", 12,		/* sizeof("verify-full") == 12 */
  	offsetof(struct pg_conn, sslmode)},
  
+ 	{"sslprotocols", "PGSSLPROTOCOLS", "ALL:-SSL", NULL,
+ 		"SSL-Protocols", "", 10,
+ 	offsetof(struct pg_conn, sslprotocols)},
+ 
  	{"sslcompression", "PGSSLCOMPRESSION", "1", NULL,
  		"SSL-Compression", "", 1,
  	offsetof(struct pg_conn, sslcompression)},
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
new file mode 100644
index 7fcc1f0..85c405a
*** a/src/interfaces/libpq/fe-secure-openssl.c
--- b/src/interfaces/libpq/fe-secure-openssl.c
***************
*** 29,34 ****
--- 29,35 ----
  #include "libpq-fe.h"
  #include "fe-auth.h"
  #include "libpq-int.h"
+ #include "libpq/ssloptions.h"
  
  #ifdef WIN32
  #include "win32.h"
*************** pgtls_init(PGconn *conn)
*** 815,820 ****
--- 816,823 ----
  
  	if (!SSL_context)
  	{
+ 		long	protocol_options;
+ 
  		if (pq_init_ssl_lib)
  		{
  #if SSLEAY_VERSION_NUMBER >= 0x00907000L
*************** pgtls_init(PGconn *conn)
*** 845,852 ****
  			return -1;
  		}
  
  		/* Disable old protocol versions */
! 		SSL_CTX_set_options(SSL_context, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
  
  		/*
  		 * Disable OpenSSL's moving-write-buffer sanity check, because it
--- 848,868 ----
  			return -1;
  		}
  
+ 		/* set up the allowed protocol list */
+ 		if (!options_from_ssl_protocols_string(conn->sslprotocols,
+ 											   &protocol_options))
+ 		{
+ 			printfPQExpBuffer(&conn->errorMessage,
+ 							  libpq_gettext("invalid SSL protocols list (are you missing 'ALL:' before '-BADPROTO'?)\n"));
+ #ifdef ENABLE_THREAD_SAFETY
+ 			pthread_mutex_unlock(&ssl_config_mutex);
+ #endif
+ 			return -1;
+ 		}
+ 
  		/* Disable old protocol versions */
! 		protocol_options |= SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;
! 		SSL_CTX_set_options(SSL_context, protocol_options);
  
  		/*
  		 * Disable OpenSSL's moving-write-buffer sanity check, because it
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
new file mode 100644
index c345571..28015e9
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
*************** struct pg_conn
*** 324,329 ****
--- 324,330 ----
  	char	   *keepalives_count;		/* maximum number of TCP keepalive
  										 * retransmits */
  	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
+ 	char	   *sslprotocols;	/* SSL protocols (e.g, "ALL:-SSL") */
  	char	   *sslcompression; /* SSL compression (0 or 1) */
  	char	   *sslkey;			/* client key filename */
  	char	   *sslcert;		/* client certificate filename */
-- 
2.1.0

#40Michael Paquier
michael.paquier@gmail.com
In reply to: Alex Shulgin (#39)
Re: [PATCH] add ssl_protocols configuration option

On Mon, Dec 15, 2014 at 11:15 PM, Alex Shulgin <ash@commandprompt.com> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

Perhaps ssloptions.[ch], unless you plan to add non-option-related code
there later?

I don't think anything else than common options-related code fits in
there, so ssloptions.c makes sense to me.

BTW, there is no Regent code in your openssl.c, so the copyright
statement is incorrect.

Good catch, I just blindly copied that from some other file.

There have been arguments in favor and against this patch... But
marking it as returned with feedback because of a lack of activity in
the last couple of weeks. Feel free to update if you think that's not
correct, and please move this patch to commit fest 2014-12.

Attached is a new version that addresses the earlier feedback: renamed
the added *.[ch] files and removed incorrect copyright line.

I'm moving this to the current CF.

This patch is statuquo since the beginning of this CF, at the
arguments are standing the same. If there is nothing happening maybe
it would be better to mark it as returned with feedback? Thoughts?
--
Michael

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

#41Michael Paquier
michael.paquier@gmail.com
In reply to: Michael Paquier (#40)
Re: [PATCH] add ssl_protocols configuration option

On Thu, Jan 15, 2015 at 4:17 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Mon, Dec 15, 2014 at 11:15 PM, Alex Shulgin <ash@commandprompt.com>
wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

Perhaps ssloptions.[ch], unless you plan to add non-option-related

code

there later?

I don't think anything else than common options-related code fits in
there, so ssloptions.c makes sense to me.

BTW, there is no Regent code in your openssl.c, so the copyright
statement is incorrect.

Good catch, I just blindly copied that from some other file.

There have been arguments in favor and against this patch... But
marking it as returned with feedback because of a lack of activity in
the last couple of weeks. Feel free to update if you think that's not
correct, and please move this patch to commit fest 2014-12.

Attached is a new version that addresses the earlier feedback: renamed
the added *.[ch] files and removed incorrect copyright line.

I'm moving this to the current CF.

This patch is statuquo since the beginning of this CF, at the
arguments are standing the same. If there is nothing happening maybe
it would be better to mark it as returned with feedback? Thoughts?

I am not sure where we are moving on here, and if anybody cares much about
this patch... Hence marked as rejected. Feel free to complain...
--
Michael