Comparing the existing {be,fe}-secure-openssl.c with the proposed
{be,fe}-secure-gnutls.c, and with half an eye on the previously proposed
Apple Secure Transport implementation, I have identified a few more
areas of refactoring that should be done in order to avoid excessive
copy-and-pasting in the new implementations:

0001-Add-installcheck-support-to-more-test-suites.patch

This will help with interoperability testing, because you can then
create an installation with mixed SSL implementations and run the test
suite against it.

0002-Split-out-documentation-of-SSL-parameters-into-their.patch

Prepares and cleans up the documentation a bit before the addition of
new things, as discussed elsewhere.

0003-Move-EDH-support-to-common-files.patch

To avoid copy-and-paste, and also because the EDH explanation doesn't
really belong in a file header comment.  Maybe the whole thing is known
well enough nowadays that we can just remove the explanation.

0004-Move-SSL-API-comments-to-header-files.patch
0005-Extract-common-bits-from-OpenSSL-implementation.patch

Move copy-and-paste avoidance.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 5b5f568bf05b2424ceb522c0a6d2f94487fea3b7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Fri, 19 Jan 2018 12:17:35 -0500
Subject: [PATCH 1/5] Add installcheck support to more test suites

---
 src/test/authentication/Makefile | 3 +++
 src/test/ldap/Makefile           | 3 +++
 src/test/recovery/Makefile       | 3 +++
 src/test/ssl/Makefile            | 3 +++
 src/test/subscription/Makefile   | 3 +++
 5 files changed, 15 insertions(+)

diff --git a/src/test/authentication/Makefile b/src/test/authentication/Makefile
index a435b13057..218452ec76 100644
--- a/src/test/authentication/Makefile
+++ b/src/test/authentication/Makefile
@@ -16,5 +16,8 @@ include $(top_builddir)/src/Makefile.global
 check:
        $(prove_check)
 
+installcheck:
+       $(prove_installcheck)
+
 clean distclean maintainer-clean:
        rm -rf tmp_check
diff --git a/src/test/ldap/Makefile b/src/test/ldap/Makefile
index 50e3c17e95..fef5742b82 100644
--- a/src/test/ldap/Makefile
+++ b/src/test/ldap/Makefile
@@ -16,5 +16,8 @@ include $(top_builddir)/src/Makefile.global
 check:
        $(prove_check)
 
+installcheck:
+       $(prove_installcheck)
+
 clean distclean maintainer-clean:
        rm -rf tmp_check
diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile
index aecf37d89a..daf79a0b1f 100644
--- a/src/test/recovery/Makefile
+++ b/src/test/recovery/Makefile
@@ -18,5 +18,8 @@ include $(top_builddir)/src/Makefile.global
 check:
        $(prove_check)
 
+installcheck:
+       $(prove_installcheck)
+
 clean distclean maintainer-clean:
        rm -rf tmp_check
diff --git a/src/test/ssl/Makefile b/src/test/ssl/Makefile
index 4886e901d0..4e9095529a 100644
--- a/src/test/ssl/Makefile
+++ b/src/test/ssl/Makefile
@@ -132,3 +132,6 @@ clean distclean maintainer-clean:
 
 check:
        $(prove_check)
+
+installcheck:
+       $(prove_installcheck)
diff --git a/src/test/subscription/Makefile b/src/test/subscription/Makefile
index 25c48e470d..0f3d2098ad 100644
--- a/src/test/subscription/Makefile
+++ b/src/test/subscription/Makefile
@@ -18,5 +18,8 @@ EXTRA_INSTALL = contrib/hstore
 check:
        $(prove_check)
 
+installcheck:
+       $(prove_installcheck)
+
 clean distclean maintainer-clean:
        rm -rf tmp_check

base-commit: 4e54dd2e0a750352ce2a5c45d1cc9183e887eec3
-- 
2.15.1

From d9160fc3cfa0e9725c857469fc1f156452a77922 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 18 Jan 2018 19:12:05 -0500
Subject: [PATCH 2/5] Split out documentation of SSL parameters into their own
 section

Split the "Authentication and Security" section into two separate
sections "Authentication" and "SSL".  The latter part has gotten much
longer over time, and doesn't primarily have to do with authentication.
---
 doc/src/sgml/config.sgml       | 233 +++++++++++++++++++++--------------------
 src/backend/utils/misc/guc.c   |  38 +++----
 src/include/utils/guc_tables.h |   3 +-
 3 files changed, 143 insertions(+), 131 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 37a61a13c8..907bf1471d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -924,8 +924,9 @@ <title>Connection Settings</title>
 
      </variablelist>
      </sect2>
-     <sect2 id="runtime-config-connection-security">
-     <title>Security and Authentication</title>
+
+     <sect2 id="runtime-config-connection-authentication">
+     <title>Authentication</title>
 
      <variablelist>
      <varlistentry id="guc-authentication-timeout" 
xreflabel="authentication_timeout">
@@ -950,6 +951,123 @@ <title>Security and Authentication</title>
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-password-encryption" 
xreflabel="password_encryption">
+      <term><varname>password_encryption</varname> (<type>enum</type>)
+      <indexterm>
+       <primary><varname>password_encryption</varname> configuration 
parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        When a password is specified in <xref linkend="sql-createrole"/> or
+        <xref linkend="sql-alterrole"/>, this parameter determines the 
algorithm
+        to use to encrypt the password. The default value is 
<literal>md5</literal>,
+        which stores the password as an MD5 hash (<literal>on</literal> is also
+        accepted, as alias for <literal>md5</literal>). Setting this parameter 
to
+        <literal>scram-sha-256</literal> will encrypt the password with 
SCRAM-SHA-256.
+       </para>
+       <para>
+        Note that older clients might lack support for the SCRAM authentication
+        mechanism, and hence not work with passwords encrypted with
+        SCRAM-SHA-256.  See <xref linkend="auth-password"/> for more details.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-krb-server-keyfile" xreflabel="krb_server_keyfile">
+      <term><varname>krb_server_keyfile</varname> (<type>string</type>)
+      <indexterm>
+       <primary><varname>krb_server_keyfile</varname> configuration 
parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Sets the location of the Kerberos server key file. See
+        <xref linkend="gssapi-auth"/>
+        for details. This parameter can only be set in the
+        <filename>postgresql.conf</filename> file or on the server command 
line.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-krb-caseins-users" xreflabel="krb_caseins_users">
+      <term><varname>krb_caseins_users</varname> (<type>boolean</type>)
+      <indexterm>
+       <primary><varname>krb_caseins_users</varname> configuration 
parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Sets whether GSSAPI user names should be treated
+        case-insensitively.
+        The default is <literal>off</literal> (case sensitive). This parameter 
can only be
+        set in the <filename>postgresql.conf</filename> file or on the server 
command line.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-db-user-namespace" xreflabel="db_user_namespace">
+      <term><varname>db_user_namespace</varname> (<type>boolean</type>)
+      <indexterm>
+       <primary><varname>db_user_namespace</varname> configuration 
parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        This parameter enables per-database user names.  It is off by default.
+        This parameter can only be set in the 
<filename>postgresql.conf</filename>
+        file or on the server command line.
+       </para>
+
+       <para>
+        If this is on, you should create users as 
<replaceable>username@dbname</replaceable>.
+        When <replaceable>username</replaceable> is passed by a connecting 
client,
+        <literal>@</literal> and the database name are appended to the user
+        name and that database-specific user name is looked up by the
+        server. Note that when you create users with names containing
+        <literal>@</literal> within the SQL environment, you will need to
+        quote the user name.
+       </para>
+
+       <para>
+        With this parameter enabled, you can still create ordinary global
+        users.  Simply append <literal>@</literal> when specifying the user
+        name in the client, e.g. <literal>joe@</literal>.  The 
<literal>@</literal>
+        will be stripped off before the user name is looked up by the
+        server.
+       </para>
+
+       <para>
+        <varname>db_user_namespace</varname> causes the client's and
+        server's user name representation to differ.
+        Authentication checks are always done with the server's user name
+        so authentication methods must be configured for the
+        server's user name, not the client's.  Because
+        <literal>md5</literal> uses the user name as salt on both the
+        client and server, <literal>md5</literal> cannot be used with
+        <varname>db_user_namespace</varname>.
+       </para>
+
+       <note>
+        <para>
+         This feature is intended as a temporary measure until a
+         complete solution is found.  At that time, this option will
+         be removed.
+        </para>
+       </note>
+      </listitem>
+     </varlistentry>
+     </variablelist>
+     </sect2>
+
+     <sect2 id="runtime-config-connection-ssl">
+     <title>SSL</title>
+
+     <para>
+      See <xref linkend="ssl-tcp"/> for more information about setting up SSL.
+     </para>
+
+     <variablelist>
      <varlistentry id="guc-ssl" xreflabel="ssl">
       <term><varname>ssl</varname> (<type>boolean</type>)
       <indexterm>
@@ -958,8 +1076,7 @@ <title>Security and Authentication</title>
       </term>
       <listitem>
        <para>
-        Enables <acronym>SSL</acronym> connections. Please read
-        <xref linkend="ssl-tcp"/> before using this.
+        Enables <acronym>SSL</acronym> connections.
         This parameter can only be set in the 
<filename>postgresql.conf</filename>
         file or on the server command line.
         The default is <literal>off</literal>.
@@ -1172,29 +1289,6 @@ <title>Security and Authentication</title>
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-password-encryption" 
xreflabel="password_encryption">
-      <term><varname>password_encryption</varname> (<type>enum</type>)
-      <indexterm>
-       <primary><varname>password_encryption</varname> configuration 
parameter</primary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        When a password is specified in <xref linkend="sql-createrole"/> or
-        <xref linkend="sql-alterrole"/>, this parameter determines the 
algorithm
-        to use to encrypt the password. The default value is 
<literal>md5</literal>,
-        which stores the password as an MD5 hash (<literal>on</literal> is also
-        accepted, as alias for <literal>md5</literal>). Setting this parameter 
to
-        <literal>scram-sha-256</literal> will encrypt the password with 
SCRAM-SHA-256.
-       </para>
-       <para>
-        Note that older clients might lack support for the SCRAM authentication
-        mechanism, and hence not work with passwords encrypted with
-        SCRAM-SHA-256.  See <xref linkend="auth-password"/> for more details.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-ssl-dh-params-file" xreflabel="ssl_dh_params_file">
       <term><varname>ssl_dh_params_file</varname> (<type>string</type>)
       <indexterm>
@@ -1218,91 +1312,6 @@ <title>Security and Authentication</title>
        </para>
       </listitem>
      </varlistentry>
-
-     <varlistentry id="guc-krb-server-keyfile" xreflabel="krb_server_keyfile">
-      <term><varname>krb_server_keyfile</varname> (<type>string</type>)
-      <indexterm>
-       <primary><varname>krb_server_keyfile</varname> configuration 
parameter</primary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Sets the location of the Kerberos server key file. See
-        <xref linkend="gssapi-auth"/>
-        for details. This parameter can only be set in the
-        <filename>postgresql.conf</filename> file or on the server command 
line.
-       </para>
-      </listitem>
-     </varlistentry>
-
-     <varlistentry id="guc-krb-caseins-users" xreflabel="krb_caseins_users">
-      <term><varname>krb_caseins_users</varname> (<type>boolean</type>)
-      <indexterm>
-       <primary><varname>krb_caseins_users</varname> configuration 
parameter</primary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Sets whether GSSAPI user names should be treated
-        case-insensitively.
-        The default is <literal>off</literal> (case sensitive). This parameter 
can only be
-        set in the <filename>postgresql.conf</filename> file or on the server 
command line.
-       </para>
-      </listitem>
-     </varlistentry>
-
-     <varlistentry id="guc-db-user-namespace" xreflabel="db_user_namespace">
-      <term><varname>db_user_namespace</varname> (<type>boolean</type>)
-      <indexterm>
-       <primary><varname>db_user_namespace</varname> configuration 
parameter</primary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        This parameter enables per-database user names.  It is off by default.
-        This parameter can only be set in the 
<filename>postgresql.conf</filename>
-        file or on the server command line.
-       </para>
-
-       <para>
-        If this is on, you should create users as 
<replaceable>username@dbname</replaceable>.
-        When <replaceable>username</replaceable> is passed by a connecting 
client,
-        <literal>@</literal> and the database name are appended to the user
-        name and that database-specific user name is looked up by the
-        server. Note that when you create users with names containing
-        <literal>@</literal> within the SQL environment, you will need to
-        quote the user name.
-       </para>
-
-       <para>
-        With this parameter enabled, you can still create ordinary global
-        users.  Simply append <literal>@</literal> when specifying the user
-        name in the client, e.g. <literal>joe@</literal>.  The 
<literal>@</literal>
-        will be stripped off before the user name is looked up by the
-        server.
-       </para>
-
-       <para>
-        <varname>db_user_namespace</varname> causes the client's and
-        server's user name representation to differ.
-        Authentication checks are always done with the server's user name
-        so authentication methods must be configured for the
-        server's user name, not the client's.  Because
-        <literal>md5</literal> uses the user name as salt on both the
-        client and server, <literal>md5</literal> cannot be used with
-        <varname>db_user_namespace</varname>.
-       </para>
-
-       <note>
-        <para>
-         This feature is intended as a temporary measure until a
-         complete solution is found.  At that time, this option will
-         be removed.
-        </para>
-       </note>
-      </listitem>
-     </varlistentry>
-
     </variablelist>
     </sect2>
    </sect1>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 72f6be329e..28f17e3e5f 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -573,8 +573,10 @@ const char *const config_group_names[] =
        gettext_noop("Connections and Authentication"),
        /* CONN_AUTH_SETTINGS */
        gettext_noop("Connections and Authentication / Connection Settings"),
-       /* CONN_AUTH_SECURITY */
-       gettext_noop("Connections and Authentication / Security and 
Authentication"),
+       /* CONN_AUTH_AUTH */
+       gettext_noop("Connections and Authentication / Authentication"),
+       /* CONN_AUTH_SSL */
+       gettext_noop("Connections and Authentication / SSL"),
        /* RESOURCES */
        gettext_noop("Resource Usage"),
        /* RESOURCES_MEM */
@@ -978,7 +980,7 @@ static struct config_bool ConfigureNamesBool[] =
                NULL, NULL, NULL
        },
        {
-               {"ssl", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"ssl", PGC_SIGHUP, CONN_AUTH_SSL,
                        gettext_noop("Enables SSL connections."),
                        NULL
                },
@@ -987,7 +989,7 @@ static struct config_bool ConfigureNamesBool[] =
                check_ssl, NULL, NULL
        },
        {
-               {"ssl_prefer_server_ciphers", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"ssl_prefer_server_ciphers", PGC_SIGHUP, CONN_AUTH_SSL,
                        gettext_noop("Give priority to server ciphersuite 
order."),
                        NULL
                },
@@ -1378,7 +1380,7 @@ static struct config_bool ConfigureNamesBool[] =
                NULL, NULL, NULL
        },
        {
-               {"db_user_namespace", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH,
                        gettext_noop("Enables per-database user names."),
                        NULL
                },
@@ -1425,7 +1427,7 @@ static struct config_bool ConfigureNamesBool[] =
                check_transaction_deferrable, NULL, NULL
        },
        {
-               {"row_security", PGC_USERSET, CONN_AUTH_SECURITY,
+               {"row_security", PGC_USERSET, CLIENT_CONN_STATEMENT,
                        gettext_noop("Enable row security."),
                        gettext_noop("When enabled, row security will be 
applied to all users.")
                },
@@ -1548,7 +1550,7 @@ static struct config_bool ConfigureNamesBool[] =
        },
 
        {
-               {"krb_caseins_users", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"krb_caseins_users", PGC_SIGHUP, CONN_AUTH_AUTH,
                        gettext_noop("Sets whether Kerberos and GSSAPI user 
names should be treated as case-insensitive."),
                        NULL
                },
@@ -2247,7 +2249,7 @@ static struct config_int ConfigureNamesInt[] =
        },
 
        {
-               {"authentication_timeout", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"authentication_timeout", PGC_SIGHUP, CONN_AUTH_AUTH,
                        gettext_noop("Sets the maximum allowed time to complete 
client authentication."),
                        NULL,
                        GUC_UNIT_S
@@ -2797,7 +2799,7 @@ static struct config_int ConfigureNamesInt[] =
        },
 
        {
-               {"ssl_renegotiation_limit", PGC_USERSET, CONN_AUTH_SECURITY,
+               {"ssl_renegotiation_limit", PGC_USERSET, CONN_AUTH_SSL,
                        gettext_noop("SSL renegotiation is no longer supported; 
this can only be 0."),
                        NULL,
                        GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE | 
GUC_DISALLOW_IN_FILE,
@@ -3170,7 +3172,7 @@ static struct config_string ConfigureNamesString[] =
        },
 
        {
-               {"krb_server_keyfile", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"krb_server_keyfile", PGC_SIGHUP, CONN_AUTH_AUTH,
                        gettext_noop("Sets the location of the Kerberos server 
key file."),
                        NULL,
                        GUC_SUPERUSER_ONLY
@@ -3530,7 +3532,7 @@ static struct config_string ConfigureNamesString[] =
        },
 
        {
-               {"ssl_cert_file", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"ssl_cert_file", PGC_SIGHUP, CONN_AUTH_SSL,
                        gettext_noop("Location of the SSL server certificate 
file."),
                        NULL
                },
@@ -3540,7 +3542,7 @@ static struct config_string ConfigureNamesString[] =
        },
 
        {
-               {"ssl_key_file", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"ssl_key_file", PGC_SIGHUP, CONN_AUTH_SSL,
                        gettext_noop("Location of the SSL server private key 
file."),
                        NULL
                },
@@ -3550,7 +3552,7 @@ static struct config_string ConfigureNamesString[] =
        },
 
        {
-               {"ssl_ca_file", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"ssl_ca_file", PGC_SIGHUP, CONN_AUTH_SSL,
                        gettext_noop("Location of the SSL certificate authority 
file."),
                        NULL
                },
@@ -3560,7 +3562,7 @@ static struct config_string ConfigureNamesString[] =
        },
 
        {
-               {"ssl_crl_file", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"ssl_crl_file", PGC_SIGHUP, CONN_AUTH_SSL,
                        gettext_noop("Location of the SSL certificate 
revocation list file."),
                        NULL
                },
@@ -3602,7 +3604,7 @@ static struct config_string ConfigureNamesString[] =
        },
 
        {
-               {"ssl_ciphers", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"ssl_ciphers", PGC_SIGHUP, CONN_AUTH_SSL,
                        gettext_noop("Sets the list of allowed SSL ciphers."),
                        NULL,
                        GUC_SUPERUSER_ONLY
@@ -3617,7 +3619,7 @@ static struct config_string ConfigureNamesString[] =
        },
 
        {
-               {"ssl_ecdh_curve", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"ssl_ecdh_curve", PGC_SIGHUP, CONN_AUTH_SSL,
                        gettext_noop("Sets the curve to use for ECDH."),
                        NULL,
                        GUC_SUPERUSER_ONLY
@@ -3632,7 +3634,7 @@ static struct config_string ConfigureNamesString[] =
        },
 
        {
-               {"ssl_dh_params_file", PGC_SIGHUP, CONN_AUTH_SECURITY,
+               {"ssl_dh_params_file", PGC_SIGHUP, CONN_AUTH_SSL,
                        gettext_noop("Location of the SSL DH parameters file."),
                        NULL,
                        GUC_SUPERUSER_ONLY
@@ -3932,7 +3934,7 @@ static struct config_enum ConfigureNamesEnum[] =
        },
 
        {
-               {"password_encryption", PGC_USERSET, CONN_AUTH_SECURITY,
+               {"password_encryption", PGC_USERSET, CONN_AUTH_AUTH,
                        gettext_noop("Encrypt passwords."),
                        gettext_noop("When a password is specified in CREATE 
USER or "
                                                 "ALTER USER without writing 
either ENCRYPTED or UNENCRYPTED, "
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index 04de6a383a..668d9efd35 100644
--- a/src/include/utils/guc_tables.h
+++ b/src/include/utils/guc_tables.h
@@ -56,7 +56,8 @@ enum config_group
        FILE_LOCATIONS,
        CONN_AUTH,
        CONN_AUTH_SETTINGS,
-       CONN_AUTH_SECURITY,
+       CONN_AUTH_AUTH,
+       CONN_AUTH_SSL,
        RESOURCES,
        RESOURCES_MEM,
        RESOURCES_DISK,
-- 
2.15.1

From e69297ca31c532077d6c7f0e34338e8d1432cb78 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Fri, 19 Jan 2018 12:18:42 -0500
Subject: [PATCH 3/5] Move EDH support to common files

The EDH support is not really specific to the OpenSSL implementation, so
move the support and documentation comments to common files.
---
 src/backend/libpq/README.SSL          | 22 +++++++++++++
 src/backend/libpq/be-secure-openssl.c | 58 +----------------------------------
 src/include/libpq/libpq-be.h          | 19 ++++++++++++
 3 files changed, 42 insertions(+), 57 deletions(-)

diff --git a/src/backend/libpq/README.SSL b/src/backend/libpq/README.SSL
index 53dc9dd005..d84a434a6e 100644
--- a/src/backend/libpq/README.SSL
+++ b/src/backend/libpq/README.SSL
@@ -58,3 +58,25 @@ SSL
    Fail with unknown
 
 ---------------------------------------------------------------------------
+
+Ephemeral DH
+============
+
+Since the server static private key ($DataDir/server.key) will
+normally be stored unencrypted so that the database backend can
+restart automatically, it is important that we select an algorithm
+that continues to provide confidentiality even if the attacker has the
+server's private key.  Ephemeral DH (EDH) keys provide this and more
+(Perfect Forward Secrecy aka PFS).
+
+N.B., the static private key should still be protected to the largest
+extent possible, to minimize the risk of impersonations.
+
+Another benefit of EDH is that it allows the backend and clients to
+use DSA keys.  DSA keys can only provide digital signatures, not
+encryption, and are often acceptable in jurisdictions where RSA keys
+are unacceptable.
+
+The downside to EDH is that it makes it impossible to use ssldump(1)
+if there's a problem establishing an SSL session.  In this case you'll
+need to temporarily disable EDH (see initialize_dh()).
diff --git a/src/backend/libpq/be-secure-openssl.c 
b/src/backend/libpq/be-secure-openssl.c
index fc6e8a0a88..450a2f614c 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -11,28 +11,6 @@
  * IDENTIFICATION
  *       src/backend/libpq/be-secure-openssl.c
  *
- *       Since the server static private key ($DataDir/server.key)
- *       will normally be stored unencrypted so that the database
- *       backend can restart automatically, it is important that
- *       we select an algorithm that continues to provide confidentiality
- *       even if the attacker has the server's private key.  Ephemeral
- *       DH (EDH) keys provide this and more (Perfect Forward Secrecy
- *       aka PFS).
- *
- *       N.B., the static private key should still be protected to
- *       the largest extent possible, to minimize the risk of
- *       impersonations.
- *
- *       Another benefit of EDH is that it allows the backend and
- *       clients to use DSA keys.  DSA keys can only provide digital
- *       signatures, not encryption, and are often acceptable in
- *       jurisdictions where RSA keys are unacceptable.
- *
- *       The downside to EDH is that it makes it impossible to
- *       use ssldump(1) if there's a problem establishing an SSL
- *       session.  In this case you'll need to temporarily disable
- *       EDH (see initialize_dh()).
- *
  *-------------------------------------------------------------------------
  */
 
@@ -87,40 +65,6 @@ static SSL_CTX *SSL_context = NULL;
 static bool SSL_initialized = false;
 static bool ssl_passwd_cb_called = false;
 
-/* ------------------------------------------------------------ */
-/*                                              Hardcoded values               
                                */
-/* ------------------------------------------------------------ */
-
-/*
- *     Hardcoded DH parameters, used in ephemeral DH keying.
- *     As discussed above, EDH protects the confidentiality of
- *     sessions even if the static private key is compromised,
- *     so we are *highly* motivated to ensure that we can use
- *     EDH even if the DBA has not provided custom DH parameters.
- *
- *     We could refuse SSL connections unless a good DH parameter
- *     file exists, but some clients may quietly renegotiate an
- *     unsecured connection without fully informing the user.
- *     Very uncool. Alternatively, the system could refuse to start
- *     if a DH parameters is not specified, but this would tend to
- *     piss off DBAs.
- *
- *     If you want to create your own hardcoded DH parameters
- *     for fun and profit, review "Assigned Number for SKIP
- *     Protocols" (http://www.skip-vpn.org/spec/numbers.html)
- *     for suggestions.
- */
-
-static const char file_dh2048[] =
-"-----BEGIN DH PARAMETERS-----\n\
-MIIBCAKCAQEA9kJXtwh/CBdyorrWqULzBej5UxE5T7bxbrlLOCDaAadWoxTpj0BV\n\
-89AHxstDqZSt90xkhkn4DIO9ZekX1KHTUPj1WV/cdlJPPT2N286Z4VeSWc39uK50\n\
-T8X8dryDxUcwYc58yWb/Ffm7/ZFexwGq01uejaClcjrUGvC/RgBYK+X0iP1YTknb\n\
-zSC0neSRBzZrM2w4DUUdD3yIsxx8Wy2O9vPJI8BD8KVbGI2Ou1WMuF040zT9fBdX\n\
-Q6MdGGzeMyEstSr/POGxKUAYEY18hKcKctaGxAMZyAcpesqVDNmWn6vQClCbAkbT\n\
-CD1mpF1Bn5x8vYlLIhkmuquiXsNV6TILOwIBAg==\n\
------END DH PARAMETERS-----\n";
-
 
 /* ------------------------------------------------------------ */
 /*                                              Public interface               
                                */
@@ -1080,7 +1024,7 @@ initialize_dh(SSL_CTX *context, bool isServerStart)
        if (ssl_dh_params_file[0])
                dh = load_dh_file(ssl_dh_params_file, isServerStart);
        if (!dh)
-               dh = load_dh_buffer(file_dh2048, sizeof file_dh2048);
+               dh = load_dh_buffer(FILE_DH2048, sizeof(FILE_DH2048));
        if (!dh)
        {
                ereport(isServerStart ? FATAL : LOG,
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 49cb263110..a38849b0d0 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -193,6 +193,25 @@ typedef struct Port
 } Port;
 
 #ifdef USE_SSL
+/*
+ *     Hardcoded DH parameters, used in ephemeral DH keying.  (See also
+ *     README.SSL for more details on EDH.)
+ *
+ *     If you want to create your own hardcoded DH parameters
+ *     for fun and profit, review "Assigned Number for SKIP
+ *     Protocols" (http://www.skip-vpn.org/spec/numbers.html)
+ *     for suggestions.
+ */
+#define FILE_DH2048 \
+"-----BEGIN DH PARAMETERS-----\n\
+MIIBCAKCAQEA9kJXtwh/CBdyorrWqULzBej5UxE5T7bxbrlLOCDaAadWoxTpj0BV\n\
+89AHxstDqZSt90xkhkn4DIO9ZekX1KHTUPj1WV/cdlJPPT2N286Z4VeSWc39uK50\n\
+T8X8dryDxUcwYc58yWb/Ffm7/ZFexwGq01uejaClcjrUGvC/RgBYK+X0iP1YTknb\n\
+zSC0neSRBzZrM2w4DUUdD3yIsxx8Wy2O9vPJI8BD8KVbGI2Ou1WMuF040zT9fBdX\n\
+Q6MdGGzeMyEstSr/POGxKUAYEY18hKcKctaGxAMZyAcpesqVDNmWn6vQClCbAkbT\n\
+CD1mpF1Bn5x8vYlLIhkmuquiXsNV6TILOwIBAg==\n\
+-----END DH PARAMETERS-----\n"
+
 /*
  * These functions are implemented by the glue code specific to each
  * SSL implementation (e.g. be-secure-openssl.c)
-- 
2.15.1

From 7255b557fd14713574cf646718eacdaffb7328ed Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 18 Jan 2018 19:53:22 -0500
Subject: [PATCH 4/5] Move SSL API comments to header files

Move the documentation of the SSL API calls are supposed to do into the
headers files, instead of keeping them in the files for the OpenSSL
implementation.  That way, they don't have to be duplicated or be
inconsistent when other implementations are added.
---
 src/backend/libpq/be-secure-openssl.c    | 38 --------------------
 src/include/libpq/libpq-be.h             | 46 ++++++++++++++++++++++++
 src/interfaces/libpq/fe-secure-openssl.c | 57 ++++-------------------------
 src/interfaces/libpq/libpq-int.h         | 62 +++++++++++++++++++++++++++++++-
 4 files changed, 113 insertions(+), 90 deletions(-)

diff --git a/src/backend/libpq/be-secure-openssl.c 
b/src/backend/libpq/be-secure-openssl.c
index 450a2f614c..f550ec82a9 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -70,13 +70,6 @@ static bool ssl_passwd_cb_called = false;
 /*                                              Public interface               
                                */
 /* ------------------------------------------------------------ */
 
-/*
- *     Initialize global SSL context.
- *
- * If isServerStart is true, report any errors as FATAL (so we don't return).
- * Otherwise, log errors at LOG level and return -1 to indicate trouble,
- * preserving the old SSL state if any.  Returns 0 if OK.
- */
 int
 be_tls_init(bool isServerStart)
 {
@@ -356,9 +349,6 @@ be_tls_init(bool isServerStart)
        return -1;
 }
 
-/*
- *     Destroy global SSL context, if any.
- */
 void
 be_tls_destroy(void)
 {
@@ -368,9 +358,6 @@ be_tls_destroy(void)
        ssl_loaded_verify_locations = false;
 }
 
-/*
- *     Attempt to negotiate SSL connection.
- */
 int
 be_tls_open_server(Port *port)
 {
@@ -539,9 +526,6 @@ be_tls_open_server(Port *port)
        return 0;
 }
 
-/*
- *     Close SSL connection.
- */
 void
 be_tls_close(Port *port)
 {
@@ -566,9 +550,6 @@ be_tls_close(Port *port)
        }
 }
 
-/*
- *     Read data from a secure connection.
- */
 ssize_t
 be_tls_read(Port *port, void *ptr, size_t len, int *waitfor)
 {
@@ -628,9 +609,6 @@ be_tls_read(Port *port, void *ptr, size_t len, int *waitfor)
        return n;
 }
 
-/*
- *     Write data to a secure connection.
- */
 ssize_t
 be_tls_write(Port *port, void *ptr, size_t len, int *waitfor)
 {
@@ -1106,9 +1084,6 @@ SSLerrmessage(unsigned long ecode)
        return errbuf;
 }
 
-/*
- * Return information about the SSL connection
- */
 int
 be_tls_get_cipher_bits(Port *port)
 {
@@ -1159,12 +1134,6 @@ be_tls_get_peerdn_name(Port *port, char *ptr, size_t len)
                ptr[0] = '\0';
 }
 
-/*
- * Routine to get the expected TLS Finished message information from the
- * client, useful for authorization when doing channel binding.
- *
- * Result is a palloc'd copy of the TLS Finished message with its size.
- */
 char *
 be_tls_get_peer_finished(Port *port, size_t *len)
 {
@@ -1183,13 +1152,6 @@ be_tls_get_peer_finished(Port *port, size_t *len)
        return result;
 }
 
-/*
- * Get the server certificate hash for SCRAM channel binding type
- * tls-server-end-point.
- *
- * The result is a palloc'd hash of the server certificate with its
- * size, and NULL if there is no certificate available.
- */
 char *
 be_tls_get_certificate_hash(Port *port, size_t *len)
 {
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index a38849b0d0..584f794b9e 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -216,19 +216,65 @@ CD1mpF1Bn5x8vYlLIhkmuquiXsNV6TILOwIBAg==\n\
  * These functions are implemented by the glue code specific to each
  * SSL implementation (e.g. be-secure-openssl.c)
  */
+
+/*
+ * Initialize global SSL context.
+ *
+ * If isServerStart is true, report any errors as FATAL (so we don't return).
+ * Otherwise, log errors at LOG level and return -1 to indicate trouble,
+ * preserving the old SSL state if any.  Returns 0 if OK.
+ */
 extern int     be_tls_init(bool isServerStart);
+
+/*
+ * Destroy global SSL context, if any.
+ */
 extern void be_tls_destroy(void);
+
+/*
+ * Attempt to negotiate SSL connection.
+ */
 extern int     be_tls_open_server(Port *port);
+
+/*
+ * Close SSL connection.
+ */
 extern void be_tls_close(Port *port);
+
+/*
+ * Read data from a secure connection.
+ */
 extern ssize_t be_tls_read(Port *port, void *ptr, size_t len, int *waitfor);
+
+/*
+ * Write data to a secure connection.
+ */
 extern ssize_t be_tls_write(Port *port, void *ptr, size_t len, int *waitfor);
 
+/*
+ * Return information about the SSL connection.
+ */
 extern int     be_tls_get_cipher_bits(Port *port);
 extern bool be_tls_get_compression(Port *port);
 extern void be_tls_get_version(Port *port, char *ptr, size_t len);
 extern void be_tls_get_cipher(Port *port, char *ptr, size_t len);
 extern void be_tls_get_peerdn_name(Port *port, char *ptr, size_t len);
+
+/*
+ * Get the expected TLS Finished message information from the client, useful
+ * for authorization when doing channel binding.
+ *
+ * Result is a palloc'd copy of the TLS Finished message with its size.
+ */
 extern char *be_tls_get_peer_finished(Port *port, size_t *len);
+
+/*
+ * Get the server certificate hash for SCRAM channel binding type
+ * tls-server-end-point.
+ *
+ * The result is a palloc'd hash of the server certificate with its
+ * size, and NULL if there is no certificate available.
+ */
 extern char *be_tls_get_certificate_hash(Port *port, size_t *len);
 #endif
 
diff --git a/src/interfaces/libpq/fe-secure-openssl.c 
b/src/interfaces/libpq/fe-secure-openssl.c
index b50bfd144a..eb13120941 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -98,10 +98,6 @@ static long win32_ssl_create_mutex = 0;
 /*                      Procedures common to all secure sessions               
        */
 /* ------------------------------------------------------------ */
 
-/*
- *     Exported function to allow application to tell us it's already
- *     initialized OpenSSL and/or libcrypto.
- */
 void
 pgtls_init_library(bool do_ssl, int do_crypto)
 {
@@ -119,9 +115,6 @@ pgtls_init_library(bool do_ssl, int do_crypto)
        pq_init_crypto_lib = do_crypto;
 }
 
-/*
- *     Begin or continue negotiating a secure session.
- */
 PostgresPollingStatusType
 pgtls_open_client(PGconn *conn)
 {
@@ -144,22 +137,6 @@ pgtls_open_client(PGconn *conn)
        return open_client_SSL(conn);
 }
 
-/*
- *     Is there unread data waiting in the SSL read buffer?
- */
-bool
-pgtls_read_pending(PGconn *conn)
-{
-       return SSL_pending(conn->ssl);
-}
-
-/*
- *     Read data from a secure connection.
- *
- * On failure, this function is responsible for putting a suitable message
- * into conn->errorMessage.  The caller must still inspect errno, but only
- * to determine whether to continue/retry after error.
- */
 ssize_t
 pgtls_read(PGconn *conn, void *ptr, size_t len)
 {
@@ -284,13 +261,12 @@ pgtls_read(PGconn *conn, void *ptr, size_t len)
        return n;
 }
 
-/*
- *     Write data to a secure connection.
- *
- * On failure, this function is responsible for putting a suitable message
- * into conn->errorMessage.  The caller must still inspect errno, but only
- * to determine whether to continue/retry after error.
- */
+bool
+pgtls_read_pending(PGconn *conn)
+{
+       return SSL_pending(conn->ssl);
+}
+
 ssize_t
 pgtls_write(PGconn *conn, const void *ptr, size_t len)
 {
@@ -393,12 +369,6 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
        return n;
 }
 
-/*
- *     Get the TLS finish message sent during last handshake
- *
- * This information is useful for callers doing channel binding during
- * authentication.
- */
 char *
 pgtls_get_finished(PGconn *conn, size_t *len)
 {
@@ -419,13 +389,6 @@ pgtls_get_finished(PGconn *conn, size_t *len)
        return result;
 }
 
-/*
- * Get the hash of the server certificate, for SCRAM channel binding type
- * tls-server-end-point.
- *
- * NULL is sent back to the caller in the event of an error, with an
- * error message for the caller to consume.
- */
 char *
 pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
 {
@@ -854,11 +817,6 @@ pq_lockingcallback(int mode, int n, const char *file, int 
line)
  * If the caller has told us (through PQinitOpenSSL) that he's taking care
  * of libcrypto, we expect that callbacks are already set, and won't try to
  * override it.
- *
- * The conn parameter is only used to be able to pass back an error
- * message - no connection-local setup is made here.
- *
- * Returns 0 if OK, -1 on failure (with a message in conn->errorMessage).
  */
 int
 pgtls_init(PGconn *conn)
@@ -1493,9 +1451,6 @@ open_client_SSL(PGconn *conn)
        return PGRES_POLLING_OK;
 }
 
-/*
- *     Close SSL connection.
- */
 void
 pgtls_close(PGconn *conn)
 {
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 4e354098b3..b3492b033a 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -661,19 +661,79 @@ extern void pq_reset_sigpipe(sigset_t *osigset, bool 
sigpipe_pending,
                                 bool got_epipe);
 #endif
 
+/* === SSL === */
+
 /*
- * The SSL implementation provides these functions (fe-secure-openssl.c)
+ * The SSL implementation provides these functions.
+ */
+
+/*
+ *     Implementation of PQinitSSL().
  */
 extern void pgtls_init_library(bool do_ssl, int do_crypto);
+
+/*
+ * Initialize SSL library.
+ *
+ * The conn parameter is only used to be able to pass back an error
+ * message - no connection-local setup is made here.
+ *
+ * Returns 0 if OK, -1 on failure (with a message in conn->errorMessage).
+ */
 extern int     pgtls_init(PGconn *conn);
+
+/*
+ *     Begin or continue negotiating a secure session.
+ */
 extern PostgresPollingStatusType pgtls_open_client(PGconn *conn);
+
+/*
+ *     Close SSL connection.
+ */
 extern void pgtls_close(PGconn *conn);
+
+/*
+ *     Read data from a secure connection.
+ *
+ * On failure, this function is responsible for putting a suitable message
+ * into conn->errorMessage.  The caller must still inspect errno, but only
+ * to determine whether to continue/retry after error.
+ */
 extern ssize_t pgtls_read(PGconn *conn, void *ptr, size_t len);
+
+/*
+ *     Is there unread data waiting in the SSL read buffer?
+ */
 extern bool pgtls_read_pending(PGconn *conn);
+
+/*
+ *     Write data to a secure connection.
+ *
+ * On failure, this function is responsible for putting a suitable message
+ * into conn->errorMessage.  The caller must still inspect errno, but only
+ * to determine whether to continue/retry after error.
+ */
 extern ssize_t pgtls_write(PGconn *conn, const void *ptr, size_t len);
+
+/*
+ * Get the TLS finish message sent during last handshake.
+ *
+ * This information is useful for callers doing channel binding during
+ * authentication.
+ */
 extern char *pgtls_get_finished(PGconn *conn, size_t *len);
+
+/*
+ * Get the hash of the server certificate, for SCRAM channel binding type
+ * tls-server-end-point.
+ *
+ * NULL is sent back to the caller in the event of an error, with an
+ * error message for the caller to consume.
+ */
 extern char *pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len);
 
+/* === miscellaneous macros === */
+
 /*
  * this is so that we can check if a connection is non-blocking internally
  * without the overhead of a function call
-- 
2.15.1

From 4ce82a167b3a7bc4b0f00f8794441d4d7bcb9edd Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Fri, 19 Jan 2018 10:17:56 -0500
Subject: [PATCH 5/5] Extract common bits from OpenSSL implementation

Some things in be-secure-openssl.c and fe-secure-openssl.c were not
actually specific to OpenSSL but could also be used by other
implementations.  In order to avoid copy-and-pasting, move some of that
code to common files.
---
 src/backend/libpq/be-secure-openssl.c    | 62 +---------------------------
 src/backend/libpq/be-secure.c            | 71 ++++++++++++++++++++++++++++++++
 src/include/libpq/libpq.h                |  1 +
 src/interfaces/libpq/fe-secure-openssl.c |  8 ----
 src/interfaces/libpq/fe-secure.c         | 14 ++++---
 5 files changed, 81 insertions(+), 75 deletions(-)

diff --git a/src/backend/libpq/be-secure-openssl.c 
b/src/backend/libpq/be-secure-openssl.c
index f550ec82a9..02601da6c8 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -75,7 +75,6 @@ be_tls_init(bool isServerStart)
 {
        STACK_OF(X509_NAME) *root_cert_list = NULL;
        SSL_CTX    *context;
-       struct stat buf;
 
        /* This stuff need be done only once. */
        if (!SSL_initialized)
@@ -133,63 +132,8 @@ be_tls_init(bool isServerStart)
                goto error;
        }
 
-       if (stat(ssl_key_file, &buf) != 0)
-       {
-               ereport(isServerStart ? FATAL : LOG,
-                               (errcode_for_file_access(),
-                                errmsg("could not access private key file 
\"%s\": %m",
-                                               ssl_key_file)));
+       if (!check_ssl_key_file_permissions(ssl_key_file, isServerStart))
                goto error;
-       }
-
-       if (!S_ISREG(buf.st_mode))
-       {
-               ereport(isServerStart ? FATAL : LOG,
-                               (errcode(ERRCODE_CONFIG_FILE_ERROR),
-                                errmsg("private key file \"%s\" is not a 
regular file",
-                                               ssl_key_file)));
-               goto error;
-       }
-
-       /*
-        * Refuse to load key files owned by users other than us or root.
-        *
-        * XXX surely we can check this on Windows somehow, too.
-        */
-#if !defined(WIN32) && !defined(__CYGWIN__)
-       if (buf.st_uid != geteuid() && buf.st_uid != 0)
-       {
-               ereport(isServerStart ? FATAL : LOG,
-                               (errcode(ERRCODE_CONFIG_FILE_ERROR),
-                                errmsg("private key file \"%s\" must be owned 
by the database user or root",
-                                               ssl_key_file)));
-               goto error;
-       }
-#endif
-
-       /*
-        * Require no public access to key file. If the file is owned by us,
-        * require mode 0600 or less. If owned by root, require 0640 or less to
-        * allow read access through our gid, or a supplementary gid that allows
-        * to read system-wide certificates.
-        *
-        * XXX temporarily suppress check when on Windows, because there may not
-        * be proper support for Unix-y file permissions.  Need to think of a
-        * reasonable check to apply on Windows.  (See also the data directory
-        * permission check in postmaster.c)
-        */
-#if !defined(WIN32) && !defined(__CYGWIN__)
-       if ((buf.st_uid == geteuid() && buf.st_mode & (S_IRWXG | S_IRWXO)) ||
-               (buf.st_uid == 0 && buf.st_mode & (S_IWGRP | S_IXGRP | 
S_IRWXO)))
-       {
-               ereport(isServerStart ? FATAL : LOG,
-                               (errcode(ERRCODE_CONFIG_FILE_ERROR),
-                                errmsg("private key file \"%s\" has group or 
world access",
-                                               ssl_key_file),
-                                errdetail("File must have permissions u=rw 
(0600) or less if owned by the database user, or permissions u=rw,g=r (0640) or 
less if owned by root.")));
-               goto error;
-       }
-#endif
 
        /*
         * OK, try to load the private key file.
@@ -516,10 +460,6 @@ be_tls_open_server(Port *port)
                port->peer_cert_valid = true;
        }
 
-       ereport(DEBUG2,
-                       (errmsg("SSL connection from \"%s\"",
-                                       port->peer_cn ? port->peer_cn : 
"(anonymous)")));
-
        /* set up debugging/info callback */
        SSL_CTX_set_info_callback(SSL_context, info_cb);
 
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index eb42ea1a1e..76c0a9e39b 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -114,6 +114,10 @@ secure_open_server(Port *port)
 
 #ifdef USE_SSL
        r = be_tls_open_server(port);
+
+       ereport(DEBUG2,
+                       (errmsg("SSL connection from \"%s\"",
+                                       port->peer_cn ? port->peer_cn : 
"(anonymous)")));
 #endif
 
        return r;
@@ -314,3 +318,70 @@ secure_raw_write(Port *port, const void *ptr, size_t len)
 
        return n;
 }
+
+bool
+check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart)
+{
+       int                     loglevel = isServerStart ? FATAL : LOG;
+       struct stat     buf;
+
+       if (stat(ssl_key_file, &buf) != 0)
+       {
+               ereport(loglevel,
+                               (errcode_for_file_access(),
+                                errmsg("could not access private key file 
\"%s\": %m",
+                                               ssl_key_file)));
+               return false;
+       }
+
+       if (!S_ISREG(buf.st_mode))
+       {
+               ereport(loglevel,
+                               (errcode(ERRCODE_CONFIG_FILE_ERROR),
+                                errmsg("private key file \"%s\" is not a 
regular file",
+                                               ssl_key_file)));
+               return false;
+       }
+
+       /*
+        * Refuse to load key files owned by users other than us or root.
+        *
+        * XXX surely we can check this on Windows somehow, too.
+        */
+#if !defined(WIN32) && !defined(__CYGWIN__)
+       if (buf.st_uid != geteuid() && buf.st_uid != 0)
+       {
+               ereport(loglevel,
+                               (errcode(ERRCODE_CONFIG_FILE_ERROR),
+                                errmsg("private key file \"%s\" must be owned 
by the database user or root",
+                                               ssl_key_file)));
+               return false;
+       }
+#endif
+
+       /*
+        * Require no public access to key file. If the file is owned by us,
+        * require mode 0600 or less. If owned by root, require 0640 or less to
+        * allow read access through our gid, or a supplementary gid that allows
+        * to read system-wide certificates.
+        *
+        * XXX temporarily suppress check when on Windows, because there may not
+        * be proper support for Unix-y file permissions.  Need to think of a
+        * reasonable check to apply on Windows.  (See also the data directory
+        * permission check in postmaster.c)
+        */
+#if !defined(WIN32) && !defined(__CYGWIN__)
+       if ((buf.st_uid == geteuid() && buf.st_mode & (S_IRWXG | S_IRWXO)) ||
+               (buf.st_uid == 0 && buf.st_mode & (S_IWGRP | S_IXGRP | 
S_IRWXO)))
+       {
+               ereport(loglevel,
+                               (errcode(ERRCODE_CONFIG_FILE_ERROR),
+                                errmsg("private key file \"%s\" has group or 
world access",
+                                               ssl_key_file),
+                                errdetail("File must have permissions u=rw 
(0600) or less if owned by the database user, or permissions u=rw,g=r (0640) or 
less if owned by root.")));
+               return false;
+       }
+#endif
+
+       return true;
+}
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 2e7725db21..255222acd7 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -90,6 +90,7 @@ extern ssize_t secure_read(Port *port, void *ptr, size_t len);
 extern ssize_t secure_write(Port *port, void *ptr, size_t len);
 extern ssize_t secure_raw_read(Port *port, void *ptr, size_t len);
 extern ssize_t secure_raw_write(Port *port, const void *ptr, size_t len);
+extern bool check_ssl_key_file_permissions(const char *ssl_key_file, bool 
isServerStart);
 
 extern bool ssl_loaded_verify_locations;
 
diff --git a/src/interfaces/libpq/fe-secure-openssl.c 
b/src/interfaces/libpq/fe-secure-openssl.c
index eb13120941..9ab317320a 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1547,14 +1547,6 @@ SSLerrfree(char *buf)
 /*                                     SSL information functions               
                        */
 /* ------------------------------------------------------------ */
 
-int
-PQsslInUse(PGconn *conn)
-{
-       if (!conn)
-               return 0;
-       return conn->ssl_in_use;
-}
-
 /*
  *     Return pointer to OpenSSL object.
  */
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index ec6c65a4b4..cfb77f6d85 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -129,6 +129,14 @@ struct sigpipe_info
 /* ------------------------------------------------------------ */
 
 
+int
+PQsslInUse(PGconn *conn)
+{
+       if (!conn)
+               return 0;
+       return conn->ssl_in_use;
+}
+
 /*
  *     Exported function to allow application to tell us it's already
  *     initialized OpenSSL.
@@ -384,12 +392,6 @@ pqsecure_raw_write(PGconn *conn, const void *ptr, size_t 
len)
 /* Dummy versions of SSL info functions, when built without SSL support */
 #ifndef USE_SSL
 
-int
-PQsslInUse(PGconn *conn)
-{
-       return 0;
-}
-
 void *
 PQgetssl(PGconn *conn)
 {
-- 
2.15.1

Reply via email to