On 28/03/2024 13:15, Matthias van de Meent wrote:
On Tue, 5 Mar 2024 at 15:08, Heikki Linnakangas <hlinn...@iki.fi> wrote:
I hope I didn't joggle your elbow reviewing this, Jacob, but I spent
some time rebase and fix various little things:
With the recent changes to backend startup committed by you, this
patchset has gotten major apply failures.
Could you provide a new version of the patchset so that it can be
reviewed in the context of current HEAD?
Here you are.
--
Heikki Linnakangas
Neon (https://neon.tech)
From 83484696e470ab130bcd3038f0e28d494065071a Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Tue, 5 Mar 2024 15:40:30 +0200
Subject: [PATCH v9 1/6] Move Kerberos module
So that we can reuse it in new tests.
---
src/test/kerberos/t/001_auth.pl | 174 ++--------------
src/test/perl/PostgreSQL/Test/Kerberos.pm | 229 ++++++++++++++++++++++
2 files changed, 240 insertions(+), 163 deletions(-)
create mode 100644 src/test/perl/PostgreSQL/Test/Kerberos.pm
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index e51e87d0a2e..d4f1ec58092 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -21,6 +21,7 @@ use strict;
use warnings FATAL => 'all';
use PostgreSQL::Test::Utils;
use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Kerberos;
use Test::More;
use Time::HiRes qw(usleep);
@@ -34,177 +35,27 @@ elsif (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\bkerberos\b/)
'Potentially unsafe test GSSAPI/Kerberos not enabled in PG_TEST_EXTRA';
}
-my ($krb5_bin_dir, $krb5_sbin_dir);
-
-if ($^O eq 'darwin' && -d "/opt/homebrew")
-{
- # typical paths for Homebrew on ARM
- $krb5_bin_dir = '/opt/homebrew/opt/krb5/bin';
- $krb5_sbin_dir = '/opt/homebrew/opt/krb5/sbin';
-}
-elsif ($^O eq 'darwin')
-{
- # typical paths for Homebrew on Intel
- $krb5_bin_dir = '/usr/local/opt/krb5/bin';
- $krb5_sbin_dir = '/usr/local/opt/krb5/sbin';
-}
-elsif ($^O eq 'freebsd')
-{
- $krb5_bin_dir = '/usr/local/bin';
- $krb5_sbin_dir = '/usr/local/sbin';
-}
-elsif ($^O eq 'linux')
-{
- $krb5_sbin_dir = '/usr/sbin';
-}
-
-my $krb5_config = 'krb5-config';
-my $kinit = 'kinit';
-my $klist = 'klist';
-my $kdb5_util = 'kdb5_util';
-my $kadmin_local = 'kadmin.local';
-my $krb5kdc = 'krb5kdc';
-
-if ($krb5_bin_dir && -d $krb5_bin_dir)
-{
- $krb5_config = $krb5_bin_dir . '/' . $krb5_config;
- $kinit = $krb5_bin_dir . '/' . $kinit;
- $klist = $krb5_bin_dir . '/' . $klist;
-}
-if ($krb5_sbin_dir && -d $krb5_sbin_dir)
-{
- $kdb5_util = $krb5_sbin_dir . '/' . $kdb5_util;
- $kadmin_local = $krb5_sbin_dir . '/' . $kadmin_local;
- $krb5kdc = $krb5_sbin_dir . '/' . $krb5kdc;
-}
-
-my $host = 'auth-test-localhost.postgresql.example.com';
-my $hostaddr = '127.0.0.1';
-my $realm = 'EXAMPLE.COM';
-
-my $krb5_conf = "${PostgreSQL::Test::Utils::tmp_check}/krb5.conf";
-my $kdc_conf = "${PostgreSQL::Test::Utils::tmp_check}/kdc.conf";
-my $krb5_cache = "${PostgreSQL::Test::Utils::tmp_check}/krb5cc";
-my $krb5_log = "${PostgreSQL::Test::Utils::log_path}/krb5libs.log";
-my $kdc_log = "${PostgreSQL::Test::Utils::log_path}/krb5kdc.log";
-my $kdc_port = PostgreSQL::Test::Cluster::get_free_port();
-my $kdc_datadir = "${PostgreSQL::Test::Utils::tmp_check}/krb5kdc";
-my $kdc_pidfile = "${PostgreSQL::Test::Utils::tmp_check}/krb5kdc.pid";
-my $keytab = "${PostgreSQL::Test::Utils::tmp_check}/krb5.keytab";
-
my $pgpass = "${PostgreSQL::Test::Utils::tmp_check}/.pgpass";
my $dbname = 'postgres';
my $username = 'test1';
my $application = '001_auth.pl';
-note "setting up Kerberos";
-
-my ($stdout, $krb5_version);
-run_log [ $krb5_config, '--version' ], '>', \$stdout
- or BAIL_OUT("could not execute krb5-config");
-BAIL_OUT("Heimdal is not supported") if $stdout =~ m/heimdal/;
-$stdout =~ m/Kerberos 5 release ([0-9]+\.[0-9]+)/
- or BAIL_OUT("could not get Kerberos version");
-$krb5_version = $1;
-
# Construct a pgpass file to make sure we don't use it
append_to_file($pgpass, '*:*:*:*:abc123');
chmod 0600, $pgpass or die $!;
-# Build the krb5.conf to use.
-#
-# Explicitly specify the default (test) realm and the KDC for
-# that realm to avoid the Kerberos library trying to look up
-# that information in DNS, and also because we're using a
-# non-standard KDC port.
-#
-# Also explicitly disable DNS lookups since this isn't really
-# our domain and we shouldn't be causing random DNS requests
-# to be sent out (not to mention that broken DNS environments
-# can cause the tests to take an extra long time and timeout).
-#
-# Reverse DNS is explicitly disabled to avoid any issue with a
-# captive portal or other cases where the reverse DNS succeeds
-# and the Kerberos library uses that as the canonical name of
-# the host and then tries to acquire a cross-realm ticket.
-append_to_file(
- $krb5_conf,
- qq![logging]
-default = FILE:$krb5_log
-kdc = FILE:$kdc_log
-
-[libdefaults]
-dns_lookup_realm = false
-dns_lookup_kdc = false
-default_realm = $realm
-forwardable = false
-rdns = false
-
-[realms]
-$realm = {
- kdc = $hostaddr:$kdc_port
-}
-!);
-
-append_to_file(
- $kdc_conf,
- qq![kdcdefaults]
-!);
-
-# For new-enough versions of krb5, use the _listen settings rather
-# than the _ports settings so that we can bind to localhost only.
-if ($krb5_version >= 1.15)
-{
- append_to_file(
- $kdc_conf,
- qq!kdc_listen = $hostaddr:$kdc_port
-kdc_tcp_listen = $hostaddr:$kdc_port
-!);
-}
-else
-{
- append_to_file(
- $kdc_conf,
- qq!kdc_ports = $kdc_port
-kdc_tcp_ports = $kdc_port
-!);
-}
-append_to_file(
- $kdc_conf,
- qq!
-[realms]
-$realm = {
- database_name = $kdc_datadir/principal
- admin_keytab = FILE:$kdc_datadir/kadm5.keytab
- acl_file = $kdc_datadir/kadm5.acl
- key_stash_file = $kdc_datadir/_k5.$realm
-}!);
-
-mkdir $kdc_datadir or die;
-
-# Ensure that we use test's config and cache files, not global ones.
-$ENV{'KRB5_CONFIG'} = $krb5_conf;
-$ENV{'KRB5_KDC_PROFILE'} = $kdc_conf;
-$ENV{'KRB5CCNAME'} = $krb5_cache;
+note "setting up Kerberos";
-my $service_principal = "$ENV{with_krb_srvnam}/$host";
+my $host = 'auth-test-localhost.postgresql.example.com';
+my $hostaddr = '127.0.0.1';
+my $realm = 'EXAMPLE.COM';
-system_or_bail $kdb5_util, 'create', '-s', '-P', 'secret0';
+my $krb = PostgreSQL::Test::Kerberos->new($host, $hostaddr, $realm);
my $test1_password = 'secret1';
-system_or_bail $kadmin_local, '-q', "addprinc -pw $test1_password test1";
-
-system_or_bail $kadmin_local, '-q', "addprinc -randkey $service_principal";
-system_or_bail $kadmin_local, '-q', "ktadd -k $keytab $service_principal";
-
-system_or_bail $krb5kdc, '-P', $kdc_pidfile;
-
-END
-{
- kill 'INT', `cat $kdc_pidfile` if defined($kdc_pidfile) && -f $kdc_pidfile;
-}
+$krb->create_principal('test1', $test1_password);
note "setting up PostgreSQL instance";
@@ -213,7 +64,7 @@ $node->init;
$node->append_conf(
'postgresql.conf', qq{
listen_addresses = '$hostaddr'
-krb_server_keyfile = '$keytab'
+krb_server_keyfile = '$krb->{keytab}'
log_connections = on
lc_messages = 'C'
});
@@ -327,8 +178,7 @@ $node->restart;
test_access($node, 'test1', 'SELECT true', 2, '', 'fails without ticket');
-run_log [ $kinit, 'test1' ], \$test1_password or BAIL_OUT($?);
-run_log [ $klist, '-f' ] or BAIL_OUT($?);
+$krb->create_ticket('test1', $test1_password);
test_access(
$node,
@@ -470,10 +320,8 @@ $node->append_conf(
hostgssenc all all $hostaddr/32 gss map=mymap
});
-string_replace_file($krb5_conf, "forwardable = false", "forwardable = true");
-
-run_log [ $kinit, 'test1' ], \$test1_password or BAIL_OUT($?);
-run_log [ $klist, '-f' ] or BAIL_OUT($?);
+# Re-create the ticket, with the forwardable flag set
+$krb->create_ticket('test1', $test1_password, forwardable => 1);
test_access(
$node,
diff --git a/src/test/perl/PostgreSQL/Test/Kerberos.pm b/src/test/perl/PostgreSQL/Test/Kerberos.pm
new file mode 100644
index 00000000000..5bb00c76f14
--- /dev/null
+++ b/src/test/perl/PostgreSQL/Test/Kerberos.pm
@@ -0,0 +1,229 @@
+
+# Copyright (c) 2021-2024, PostgreSQL Global Development Group
+
+# Sets up a stand-alone KDC for testing PostgreSQL GSSAPI / Kerberos
+# functionality.
+
+package PostgreSQL::Test::Kerberos;
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+
+our ($krb5_bin_dir, $krb5_sbin_dir, $krb5_config, $kinit, $klist,
+ $kdb5_util, $kadmin_local, $krb5kdc,
+ $krb5_conf, $kdc_conf, $krb5_cache, $krb5_log, $kdc_log,
+ $kdc_port, $kdc_datadir, $kdc_pidfile, $keytab);
+
+INIT
+{
+ if ($^O eq 'darwin' && -d "/opt/homebrew")
+ {
+ # typical paths for Homebrew on ARM
+ $krb5_bin_dir = '/opt/homebrew/opt/krb5/bin';
+ $krb5_sbin_dir = '/opt/homebrew/opt/krb5/sbin';
+ }
+ elsif ($^O eq 'darwin')
+ {
+ # typical paths for Homebrew on Intel
+ $krb5_bin_dir = '/usr/local/opt/krb5/bin';
+ $krb5_sbin_dir = '/usr/local/opt/krb5/sbin';
+ }
+ elsif ($^O eq 'freebsd')
+ {
+ $krb5_bin_dir = '/usr/local/bin';
+ $krb5_sbin_dir = '/usr/local/sbin';
+ }
+ elsif ($^O eq 'linux')
+ {
+ $krb5_sbin_dir = '/usr/sbin';
+ }
+
+ $krb5_config = 'krb5-config';
+ $kinit = 'kinit';
+ $klist = 'klist';
+ $kdb5_util = 'kdb5_util';
+ $kadmin_local = 'kadmin.local';
+ $krb5kdc = 'krb5kdc';
+
+ if ($krb5_bin_dir && -d $krb5_bin_dir)
+ {
+ $krb5_config = $krb5_bin_dir . '/' . $krb5_config;
+ $kinit = $krb5_bin_dir . '/' . $kinit;
+ $klist = $krb5_bin_dir . '/' . $klist;
+ }
+ if ($krb5_sbin_dir && -d $krb5_sbin_dir)
+ {
+ $kdb5_util = $krb5_sbin_dir . '/' . $kdb5_util;
+ $kadmin_local = $krb5_sbin_dir . '/' . $kadmin_local;
+ $krb5kdc = $krb5_sbin_dir . '/' . $krb5kdc;
+ }
+
+ $krb5_conf = "${PostgreSQL::Test::Utils::tmp_check}/krb5.conf";
+ $kdc_conf = "${PostgreSQL::Test::Utils::tmp_check}/kdc.conf";
+ $krb5_cache = "${PostgreSQL::Test::Utils::tmp_check}/krb5cc";
+ $krb5_log = "${PostgreSQL::Test::Utils::log_path}/krb5libs.log";
+ $kdc_log = "${PostgreSQL::Test::Utils::log_path}/krb5kdc.log";
+ $kdc_port = PostgreSQL::Test::Cluster::get_free_port();
+ $kdc_datadir = "${PostgreSQL::Test::Utils::tmp_check}/krb5kdc";
+ $kdc_pidfile = "${PostgreSQL::Test::Utils::tmp_check}/krb5kdc.pid";
+ $keytab = "${PostgreSQL::Test::Utils::tmp_check}/krb5.keytab";
+}
+
+=pod
+
+=item PostgreSQL::Test::Kerberos->new(host, hostaddr, realm, %params)
+
+Sets up a new Kerberos realm and KDC. This function assigns a free
+port for the KDC. The KDC will be shut down automatically when the
+test script exits.
+
+=over
+
+=item host => 'auth-test-localhost.postgresql.example.com'
+
+Hostname to use in the service principal.
+
+=item hostaddr => '127.0.0.1'
+
+Network interface the KDC will listen on.
+
+=item realm => 'EXAMPLE.COM'
+
+Name of the Kerberos realm.
+
+=back
+
+=cut
+
+sub new
+{
+ my $class = shift;
+ my ($host, $hostaddr, $realm) = @_;
+
+ my ($stdout, $krb5_version);
+ run_log [ $krb5_config, '--version' ], '>', \$stdout
+ or BAIL_OUT("could not execute krb5-config");
+ BAIL_OUT("Heimdal is not supported") if $stdout =~ m/heimdal/;
+ $stdout =~ m/Kerberos 5 release ([0-9]+\.[0-9]+)/
+ or BAIL_OUT("could not get Kerberos version");
+ $krb5_version = $1;
+
+ # Build the krb5.conf to use.
+ #
+ # Explicitly specify the default (test) realm and the KDC for
+ # that realm to avoid the Kerberos library trying to look up
+ # that information in DNS, and also because we're using a
+ # non-standard KDC port.
+ #
+ # Also explicitly disable DNS lookups since this isn't really
+ # our domain and we shouldn't be causing random DNS requests
+ # to be sent out (not to mention that broken DNS environments
+ # can cause the tests to take an extra long time and timeout).
+ #
+ # Reverse DNS is explicitly disabled to avoid any issue with a
+ # captive portal or other cases where the reverse DNS succeeds
+ # and the Kerberos library uses that as the canonical name of
+ # the host and then tries to acquire a cross-realm ticket.
+ append_to_file(
+ $krb5_conf,
+ qq![logging]
+default = FILE:$krb5_log
+kdc = FILE:$kdc_log
+
+[libdefaults]
+dns_lookup_realm = false
+dns_lookup_kdc = false
+default_realm = $realm
+forwardable = false
+rdns = false
+
+[realms]
+$realm = {
+ kdc = $hostaddr:$kdc_port
+}
+!);
+
+ append_to_file(
+ $kdc_conf,
+ qq![kdcdefaults]
+!);
+
+ # For new-enough versions of krb5, use the _listen settings rather
+ # than the _ports settings so that we can bind to localhost only.
+ if ($krb5_version >= 1.15)
+ {
+ append_to_file(
+ $kdc_conf,
+ qq!kdc_listen = $hostaddr:$kdc_port
+kdc_tcp_listen = $hostaddr:$kdc_port
+!);
+ }
+ else
+ {
+ append_to_file(
+ $kdc_conf,
+ qq!kdc_ports = $kdc_port
+kdc_tcp_ports = $kdc_port
+!);
+ }
+ append_to_file(
+ $kdc_conf,
+ qq!
+[realms]
+$realm = {
+ database_name = $kdc_datadir/principal
+ admin_keytab = FILE:$kdc_datadir/kadm5.keytab
+ acl_file = $kdc_datadir/kadm5.acl
+ key_stash_file = $kdc_datadir/_k5.$realm
+}!);
+
+ mkdir $kdc_datadir or die;
+
+ # Ensure that we use test's config and cache files, not global ones.
+ $ENV{'KRB5_CONFIG'} = $krb5_conf;
+ $ENV{'KRB5_KDC_PROFILE'} = $kdc_conf;
+ $ENV{'KRB5CCNAME'} = $krb5_cache;
+
+ my $service_principal = "$ENV{with_krb_srvnam}/$host";
+
+ system_or_bail $kdb5_util, 'create', '-s', '-P', 'secret0';
+
+ system_or_bail $kadmin_local, '-q', "addprinc -randkey $service_principal";
+ system_or_bail $kadmin_local, '-q', "ktadd -k $keytab $service_principal";
+
+ system_or_bail $krb5kdc, '-P', $kdc_pidfile;
+
+ my $self = {};
+ $self->{keytab} = $keytab;
+
+ bless $self, $class;
+
+ return $self;
+}
+
+sub create_principal
+{
+ my ($self, $principal, $password) = @_;
+
+ system_or_bail $kadmin_local, '-q', "addprinc -pw $password $principal";
+}
+
+sub create_ticket
+{
+ my ($self, $principal, $password, %params) = @_;
+
+ my @cmd = ($kinit, $principal);
+
+ push @cmd, '-f' if ($params{forwardable});
+
+ run_log [@cmd], \$password or BAIL_OUT($?);
+ run_log [ $klist, '-f' ] or BAIL_OUT($?);
+}
+
+END
+{
+ kill 'INT', `cat $kdc_pidfile` if defined($kdc_pidfile) && -f $kdc_pidfile;
+}
+
+1;
--
2.39.2
From e52c701931f5c4b67d2014d6d27c925501f15464 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Tue, 5 Mar 2024 15:41:09 +0200
Subject: [PATCH v9 2/6] Add tests for libpq choosing encryption mode
Author: Heikki Linnakangas, Matthias van de Meent
Discussion: XX
---
src/test/libpq_encryption/Makefile | 25 ++
src/test/libpq_encryption/README | 23 ++
src/test/libpq_encryption/meson.build | 19 +
.../t/001_negotiate_encryption.pl | 369 ++++++++++++++++++
src/test/perl/PostgreSQL/Test/Kerberos.pm | 6 +-
5 files changed, 439 insertions(+), 3 deletions(-)
create mode 100644 src/test/libpq_encryption/Makefile
create mode 100644 src/test/libpq_encryption/README
create mode 100644 src/test/libpq_encryption/meson.build
create mode 100644 src/test/libpq_encryption/t/001_negotiate_encryption.pl
diff --git a/src/test/libpq_encryption/Makefile b/src/test/libpq_encryption/Makefile
new file mode 100644
index 00000000000..710929c4cce
--- /dev/null
+++ b/src/test/libpq_encryption/Makefile
@@ -0,0 +1,25 @@
+#-------------------------------------------------------------------------
+#
+# Makefile for src/test/libpq_encryption
+#
+# Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+# Portions Copyright (c) 1994, Regents of the University of California
+#
+# src/test/ldap/libpq_encryption
+#
+#-------------------------------------------------------------------------
+
+subdir = src/test/libpq_encryption
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+export OPENSSL with_ssl with_gssapi with_krb_srvnam
+
+check:
+ $(prove_check)
+
+installcheck:
+ $(prove_installcheck)
+
+clean distclean:
+ rm -rf tmp_check
diff --git a/src/test/libpq_encryption/README b/src/test/libpq_encryption/README
new file mode 100644
index 00000000000..66430b54316
--- /dev/null
+++ b/src/test/libpq_encryption/README
@@ -0,0 +1,23 @@
+src/test/libpq_encryption/README
+
+Tests for negotiating network encryption method
+===============================================
+
+
+Running the tests
+=================
+
+NOTE: You must have given the --enable-tap-tests argument to configure.
+
+Run
+ make check PG_TEST_EXTRA=libpq_encryption
+
+XXX You can use "make installcheck" if you previously did "make install".
+In that case, the code in the installation tree is tested. With
+"make check", a temporary installation tree is built from the current
+sources and then tested.
+
+XXX Either way, this test initializes, starts, and stops a test Postgres
+cluster, as well as a test LDAP server.
+
+See src/test/perl/README for more info about running these tests.
diff --git a/src/test/libpq_encryption/meson.build b/src/test/libpq_encryption/meson.build
new file mode 100644
index 00000000000..04f479e9fe7
--- /dev/null
+++ b/src/test/libpq_encryption/meson.build
@@ -0,0 +1,19 @@
+# Copyright (c) 2022-2024, PostgreSQL Global Development Group
+
+tests += {
+ 'name': 'ldap',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_auth.pl',
+ 't/002_bindpasswd.pl',
+ ],
+ 'env': {
+ 'with_ssl': ssl_library,
+ 'OPENSSL': openssl.found() ? openssl.path() : '',
+ 'with_gssapi': gssapi.found() ? 'yes' : 'no',
+ 'with_krb_srvnam': 'postgres',
+ },
+ },
+}
diff --git a/src/test/libpq_encryption/t/001_negotiate_encryption.pl b/src/test/libpq_encryption/t/001_negotiate_encryption.pl
new file mode 100644
index 00000000000..b8646c5bc97
--- /dev/null
+++ b/src/test/libpq_encryption/t/001_negotiate_encryption.pl
@@ -0,0 +1,369 @@
+
+# Copyright (c) 2021-2024, PostgreSQL Global Development Group
+
+# Test negotiation of SSL and GSSAPI encryption
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Kerberos;
+use File::Basename;
+use File::Copy;
+use Test::More;
+
+if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\blibpq_encryption\b/)
+{
+ plan skip_all =>
+ 'Potentially unsafe test libpq_encryption not enabled in PG_TEST_EXTRA';
+}
+
+my $host = 'enc-test-localhost.postgresql.example.com';
+my $hostaddr = '127.0.0.1';
+my $servercidr = '127.0.0.1/32';
+
+note "setting up PostgreSQL instance";
+
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+$node->append_conf(
+ 'postgresql.conf', qq{
+listen_addresses = '$hostaddr'
+log_connections = on
+lc_messages = 'C'
+});
+my $pgdata = $node->data_dir;
+
+my $dbname = 'postgres';
+my $username = 'enctest';
+my $application = '001_negotiate_encryption.pl';
+
+my $gssuser_password = 'secret1';
+
+my $krb;
+
+my $ssl_supported = $ENV{with_ssl} eq 'openssl';
+my $gss_supported = $ENV{with_gssapi} eq 'yes';
+
+if ($gss_supported != 0)
+{
+ note "setting up Kerberos";
+
+ my $realm = 'EXAMPLE.COM';
+ $krb = PostgreSQL::Test::Kerberos->new($host, $hostaddr, $realm);
+ $node->append_conf('postgresql.conf', "krb_server_keyfile = '$krb->{keytab}'\n");
+}
+
+if ($ssl_supported != 0)
+{
+ my $certdir = dirname(__FILE__) . "/../../ssl/ssl";
+
+ copy "$certdir/server-cn-only.crt", "$pgdata/server.crt"
+ || die "copying server.crt: $!";
+ copy "$certdir/server-cn-only.key", "$pgdata/server.key"
+ || die "copying server.key: $!";
+ chmod(0600, "$pgdata/server.key");
+
+ # Start with SSL disabled.
+ $node->append_conf('postgresql.conf', "ssl = off\n");
+}
+
+$node->start;
+
+$node->safe_psql('postgres', 'CREATE USER localuser;');
+$node->safe_psql('postgres', 'CREATE USER testuser;');
+$node->safe_psql('postgres', 'CREATE USER ssluser;');
+$node->safe_psql('postgres', 'CREATE USER nossluser;');
+$node->safe_psql('postgres', 'CREATE USER gssuser;');
+$node->safe_psql('postgres', 'CREATE USER nogssuser;');
+
+my $unixdir = $node->safe_psql('postgres', 'SHOW unix_socket_directories;');
+chomp($unixdir);
+
+$node->safe_psql('postgres', q{
+CREATE FUNCTION current_enc() RETURNS text LANGUAGE plpgsql AS $$
+DECLARE
+ ssl_in_use bool;
+ gss_in_use bool;
+BEGIN
+ ssl_in_use = (SELECT ssl FROM pg_stat_ssl WHERE pid = pg_backend_pid());
+ gss_in_use = (SELECT encrypted FROM pg_stat_gssapi WHERE pid = pg_backend_pid());
+
+ raise log 'ssl % gss %', ssl_in_use, gss_in_use;
+
+ IF ssl_in_use AND gss_in_use THEN
+ RETURN 'ssl+gss'; -- shouldn't happen
+ ELSIF ssl_in_use THEN
+ RETURN 'ssl';
+ ELSIF gss_in_use THEN
+ RETURN 'gss';
+ ELSE
+ RETURN 'plain';
+ END IF;
+END;
+$$;
+});
+
+# Only accept SSL connections from $servercidr. Our tests don't depend on this
+# but seems best to keep it as narrow as possible for security reasons.
+#
+# When connecting to certdb, also check the client certificate.
+open my $hba, '>', "$pgdata/pg_hba.conf";
+print $hba qq{
+# TYPE DATABASE USER ADDRESS METHOD OPTIONS
+local postgres localuser trust
+host postgres testuser $servercidr trust
+hostnossl postgres nossluser $servercidr trust
+hostnogssenc postgres nogssuser $servercidr trust
+};
+
+print $hba qq{
+hostssl postgres ssluser $servercidr trust
+} if ($ssl_supported != 0);
+
+print $hba qq{
+hostgssenc postgres gssuser $servercidr trust
+} if ($gss_supported != 0);
+close $hba;
+$node->reload;
+
+sub connect_test
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+ my ($node, $connstr, $expected_enc, @expect_log_msgs)
+ = @_;
+
+ my $test_name = " '$connstr' -> $expected_enc";
+
+ my $connstr_full = "";
+ $connstr_full .= "dbname=postgres " unless $connstr =~ m/dbname=/;
+ $connstr_full .= "host=$host hostaddr=$hostaddr " unless $connstr =~ m/host=/;
+ $connstr_full .= $connstr;
+
+ my $log_location = -s $node->logfile;
+
+ my ($ret, $stdout, $stderr) = $node->psql(
+ 'postgres',
+ 'SELECT current_enc()',
+ extra_params => ['-w'],
+ connstr => "$connstr_full",
+ on_error_stop => 0);
+
+ my $result = $ret == 0 ? $stdout : 'fail';
+
+ is($result, $expected_enc, $test_name);
+
+ if (@expect_log_msgs)
+ {
+ # Match every message literally.
+ my @regexes = map { qr/\Q$_\E/ } @expect_log_msgs;
+ my %params = ();
+ $params{log_like} = \@regexes;
+ $node->log_check($test_name, $log_location, %params);
+ }
+}
+
+# Return the encryption mode that we expect to be chosen by libpq,
+# when connecting with given the user, gssmode, sslmode settings.
+sub resolve_connection_type
+{
+ my ($config) = @_;
+ my $user = $config->{user};
+ my $gssmode = $config->{gssmode};
+ my $sslmode = $config->{sslmode};
+
+ my @conntypes = qw(plain);
+
+ # Add connection types supported by the server to the pool
+ push(@conntypes, "ssl") if $config->{server_ssl} == 1;
+ push(@conntypes, "gss") if $config->{server_gss} == 1;
+
+ # User configurations:
+ # gssuser/ssluser require the relevant connection type,
+ @conntypes = grep {/gss/} @conntypes if $user eq 'gssuser';
+ @conntypes = grep {/ssl/} @conntypes if $user eq 'ssluser';
+
+ # nogssuser/nossluser require anything but the relevant connection type.
+ @conntypes = grep {!/gss/} @conntypes if $user eq 'nogssuser';
+ @conntypes = grep {!/ssl/} @conntypes if $user eq 'nossluser';
+
+ print STDOUT "After user filter: @conntypes\n";
+
+ # remove disabled connection modes
+ @conntypes = grep {!/gss/} @conntypes if $gssmode eq 'disable';
+ @conntypes = grep {!/ssl/} @conntypes if $sslmode eq 'disable';
+
+ # If gssmode=require, drop all non-GSS modes.
+ if ($gssmode eq 'require')
+ {
+ @conntypes = grep {/gss/} @conntypes;
+ }
+
+ # If sslmode=require, drop plain mode.
+ #
+ # NOTE: GSS is also allowed with sslmode=require.
+ if ($sslmode eq 'require')
+ {
+ @conntypes = grep {!/plain/} @conntypes;
+ }
+
+ print STDOUT "After mode require filter: @conntypes\n";
+
+ # Handle priorities of the various types.
+ # Note that this doesn't need to care about require/disable/etc, those
+ # filters were applied before we get here.
+ # Also note that preference is 1 > 2 > 3 > 4 > 5, so first preference
+ # without ssl or gss 'prefer/require' is plain connections.
+ my %order = (plain=>3, gss=>4, ssl=>5);
+
+ $order{ssl} = 2 if $sslmode eq "prefer";
+ $order{gss} = 1 if $gssmode eq "prefer";
+ @conntypes = sort { $order{$a} cmp $order{$b} } @conntypes;
+
+ # If there are no connection types available after filtering requirements,
+ # the connection fails.
+ return "fail" if @conntypes == 0;
+ # Else, we get to connect using the connection type with the highest
+ # priority.
+ return $conntypes[0];
+}
+
+# First test with SSL disabled in the server
+
+# Test the cube of parameters: user, sslmode, and gssencmode
+sub test_modes
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+ my ($pg_node, $node_conf,
+ $test_users, $ssl_modes, $gss_modes) = @_;
+
+ foreach my $test_user (@{$test_users})
+ {
+ foreach my $client_mode (@{$ssl_modes})
+ {
+ foreach my $gssencmode (@{$gss_modes})
+ {
+ my %params = (
+ server_ssl=>$node_conf->{server_ssl},
+ server_gss=>$node_conf->{server_gss},
+ user=>$test_user,
+ sslmode=>$client_mode,
+ gssmode=>$gssencmode,
+ );
+ my $res = resolve_connection_type(\%params);
+ connect_test($pg_node, "user=$test_user sslmode=$client_mode gssencmode=$gssencmode", $res);
+ }
+ }
+ }
+}
+
+my $sslmodes = ['disable', 'allow', 'prefer', 'require'];
+my $gssencmodes = ['disable', 'prefer', 'require'];
+
+my $server_config = {
+ server_ssl => 0,
+ server_gss => 0,
+};
+
+note("Running tests with SSL and GSS disabled in server");
+test_modes($node, $server_config,
+ ['testuser'],
+ $sslmodes, $gssencmodes);
+
+# Enable SSL in the server
+SKIP:
+{
+ skip "SSL not supported by this build" if $ssl_supported == 0;
+
+ $node->adjust_conf('postgresql.conf', 'ssl', 'on');
+ $node->restart;
+ $server_config->{server_ssl} = 1;
+
+ note("Running tests with SSL enabled in server");
+ test_modes($node, $server_config,
+ ['testuser', 'ssluser', 'nossluser'],
+ $sslmodes, ['disable']);
+
+ $node->adjust_conf('postgresql.conf', 'ssl', 'off');
+ $node->reload;
+ $server_config->{server_ssl} = 0;
+}
+
+# Test GSSAPI
+SKIP:
+{
+ skip "GSSAPI/Kerberos not supported by this build" if $gss_supported == 0;
+
+ # No ticket
+ connect_test($node, 'user=testuser sslmode=disable gssencmode=require', 'fail');
+
+ $krb->create_principal('gssuser', $gssuser_password);
+ $krb->create_ticket('gssuser', $gssuser_password);
+ $server_config->{server_gss} = 1;
+
+ note("Running tests with GSS enabled in server");
+ test_modes($node, $server_config,
+ ['testuser', 'gssuser', 'nogssuser'],
+ $sslmodes, $gssencmodes);
+
+ # Check that logs match the expected 'no pg_hba.conf entry' line, too, as
+ # that is not tested by test_modes.
+ connect_test($node, 'user=nogssuser sslmode=prefer gssencmode=require', 'fail',
+ 'no pg_hba.conf entry for host "127.0.0.1", user "nogssuser", database "postgres", GSS encryption');
+
+ # With 'gssencmode=prefer', libpq will first negotiate GSSAPI
+ # encryption, but the connection will fail because pg_hba.conf
+ # forbids GSSAPI encryption for this user. It will then reconnect
+ # with SSL, but the server doesn't support it, so it will continue
+ # with no encryption.
+ connect_test($node, 'user=nogssuser sslmode=prefer gssencmode=prefer', 'plain',
+ 'no pg_hba.conf entry for host "127.0.0.1", user "nogssuser", database "postgres", GSS encryption');
+}
+
+# Server supports both SSL and GSSAPI
+SKIP:
+{
+ skip "GSSAPI/Kerberos or SSL not supported by this build" unless ($ssl_supported && $gss_supported);
+
+ # SSL is still disabled
+ connect_test($node, 'user=testuser sslmode=prefer gssencmode=prefer', 'gss');
+
+ # Enable SSL
+ $node->adjust_conf('postgresql.conf', 'ssl', 'on');
+ $node->reload;
+ $server_config->{server_ssl} = 1;
+
+ note("Running tests with both GSS and SSL enabled in server");
+ test_modes($node, $server_config,
+ ['testuser', 'gssuser', 'ssluser', 'nogssuser', 'nossluser'],
+ $sslmodes, $gssencmodes);
+
+ # Test case that server supports GSSAPI, but it's not allowed for
+ # this user. Special cased because we check output
+ connect_test($node, 'user=nogssuser sslmode=prefer gssencmode=require', 'fail',
+ 'no pg_hba.conf entry for host "127.0.0.1", user "nogssuser", database "postgres", GSS encryption');
+
+ # with 'gssencmode=prefer', libpq will first negotiate GSSAPI
+ # encryption, but the connection will fail because pg_hba.conf
+ # forbids GSSAPI encryption for this user. It will then reconnect
+ # with SSL.
+ connect_test($node, 'user=nogssuser sslmode=prefer gssencmode=prefer', 'ssl',
+ 'no pg_hba.conf entry for host "127.0.0.1", user "nogssuser", database "postgres", GSS encryption');
+
+ # Setting both sslmode=require and gssencmode=require fails if GSSAPI is not
+ # available.
+ connect_test($node, 'user=nogssuser sslmode=require gssencmode=require', 'fail');
+}
+
+# Test negotiation over unix domain sockets.
+SKIP:
+{
+ skip "Unix domain sockets not supported" unless ($unixdir ne "");
+
+ connect_test($node, "user=localuser sslmode=require gssencmode=prefer host=$unixdir", 'plain');
+ connect_test($node, "user=localuser sslmode=prefer gssencmode=require host=$unixdir", 'fail');
+}
+
+done_testing();
diff --git a/src/test/perl/PostgreSQL/Test/Kerberos.pm b/src/test/perl/PostgreSQL/Test/Kerberos.pm
index 5bb00c76f14..e6063703c3a 100644
--- a/src/test/perl/PostgreSQL/Test/Kerberos.pm
+++ b/src/test/perl/PostgreSQL/Test/Kerberos.pm
@@ -103,10 +103,10 @@ sub new
my ($stdout, $krb5_version);
run_log [ $krb5_config, '--version' ], '>', \$stdout
- or BAIL_OUT("could not execute krb5-config");
- BAIL_OUT("Heimdal is not supported") if $stdout =~ m/heimdal/;
+ or die("could not execute krb5-config");
+ die("Heimdal is not supported") if $stdout =~ m/heimdal/;
$stdout =~ m/Kerberos 5 release ([0-9]+\.[0-9]+)/
- or BAIL_OUT("could not get Kerberos version");
+ or die("could not get Kerberos version");
$krb5_version = $1;
# Build the krb5.conf to use.
--
2.39.2
From c050ee7cd559dc1a3575cb11b4013bf3e10839b9 Mon Sep 17 00:00:00 2001
From: Greg Stark <st...@mit.edu>
Date: Wed, 15 Mar 2023 15:51:02 -0400
Subject: [PATCH v9 3/6] Direct SSL connections, client and server support
Author: Greg Stark, Heikki Linnakangas
---
doc/src/sgml/libpq.sgml | 102 ++++++++++++++++++++++++++---
doc/src/sgml/protocol.sgml | 37 +++++++++++
src/backend/libpq/be-secure.c | 52 ++++++++++++++-
src/backend/libpq/pqcomm.c | 9 +--
src/backend/tcop/backend_startup.c | 83 +++++++++++++++++++++--
src/include/libpq/libpq-be.h | 13 ++++
src/include/libpq/libpq.h | 2 +-
src/interfaces/libpq/fe-connect.c | 99 ++++++++++++++++++++++++++--
src/interfaces/libpq/libpq-fe.h | 3 +-
src/interfaces/libpq/libpq-int.h | 4 ++
10 files changed, 374 insertions(+), 30 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index d3e87056f2c..0166355d834 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1700,10 +1700,13 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
Note that if <acronym>GSSAPI</acronym> encryption is possible,
that will be used in preference to <acronym>SSL</acronym>
encryption, regardless of the value of <literal>sslmode</literal>.
- To force use of <acronym>SSL</acronym> encryption in an
- environment that has working <acronym>GSSAPI</acronym>
- infrastructure (such as a Kerberos server), also
- set <literal>gssencmode</literal> to <literal>disable</literal>.
+ To negotiate <acronym>SSL</acronym> encryption in an environment that
+ has working <acronym>GSSAPI</acronym> infrastructure (such as a
+ Kerberos server), also set <literal>gssencmode</literal>
+ to <literal>disable</literal>.
+ Use of non-default values of <literal>sslnegotiation</literal> can
+ also cause <acronym>SSL</acronym> to be used instead of
+ negotiating <acronym>GSSAPI</acronym> encryption.
</para>
</listitem>
</varlistentry>
@@ -1730,6 +1733,75 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
</listitem>
</varlistentry>
+ <varlistentry id="libpq-connect-sslnegotiation" xreflabel="sslnegotiation">
+ <term><literal>sslnegotiation</literal></term>
+ <listitem>
+ <para>
+ This option controls whether <productname>PostgreSQL</productname>
+ will perform its protocol negotiation to request encryption from the
+ server or will just directly make a standard <acronym>SSL</acronym>
+ connection. Traditional <productname>PostgreSQL</productname>
+ protocol negotiation is the default and the most flexible with
+ different server configurations. If the server is known to support
+ direct <acronym>SSL</acronym> connections then the latter requires one
+ fewer round trip reducing connection latency and also allows the use
+ of protocol agnostic SSL network tools.
+ </para>
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>postgres</literal></term>
+ <listitem>
+ <para>
+ perform <productname>PostgreSQL</productname> protocol
+ negotiation. This is the default if the option is not provided.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>direct</literal></term>
+ <listitem>
+ <para>
+ first attempt to establish a standard SSL connection and if that
+ fails reconnect and perform the negotiation. This fallback
+ process adds significant latency if the initial SSL connection
+ fails.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>requiredirect</literal></term>
+ <listitem>
+ <para>
+ attempt to establish a standard SSL connection and if that fails
+ return a connection failure immediately.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ If <literal>sslmode</literal> set to <literal>disable</literal>
+ or <literal>allow</literal> then <literal>sslnegotiation</literal> is
+ ignored. If <literal>gssencmode</literal> is set
+ to <literal>require</literal> then <literal>sslnegotiation</literal>
+ must be the default <literal>postgres</literal> value.
+ </para>
+
+ <para>
+ Moreover, note if <literal>gssencmode</literal> is set
+ to <literal>prefer</literal> and <literal>sslnegotiation</literal>
+ to <literal>direct</literal> then the effective preference will be
+ direct <acronym>SSL</acronym> connections, followed by
+ negotiated <acronym>GSS</acronym> connections, followed by
+ negotiated <acronym>SSL</acronym> connections, possibly followed
+ lastly by unencrypted connections.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-connect-sslcompression" xreflabel="sslcompression">
<term><literal>sslcompression</literal></term>
<listitem>
@@ -1963,11 +2035,13 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
<para>
The Server Name Indication can be used by SSL-aware proxies to route
- connections without having to decrypt the SSL stream. (Note that this
- requires a proxy that is aware of the PostgreSQL protocol handshake,
- not just any SSL proxy.) However, <acronym>SNI</acronym> makes the
- destination host name appear in cleartext in the network traffic, so
- it might be undesirable in some cases.
+ connections without having to decrypt the SSL stream. (Note that
+ unless the proxy is aware of the PostgreSQL protocol handshake this
+ would require setting <literal>sslnegotiation</literal>
+ to <literal>direct</literal> or <literal>requiredirect</literal>.)
+ However, <acronym>SNI</acronym> makes the destination host name appear
+ in cleartext in the network traffic, so it might be undesirable in
+ some cases.
</para>
</listitem>
</varlistentry>
@@ -8585,6 +8659,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
</para>
</listitem>
+ <listitem>
+ <para>
+ <indexterm>
+ <primary><envar>PGSSLNEGOTIATION</envar></primary>
+ </indexterm>
+ <envar>PGSSLNEGOTIATION</envar> behaves the same as the <xref
+ linkend="libpq-connect-sslnegotiation"/> connection parameter.
+ </para>
+ </listitem>
+
<listitem>
<para>
<indexterm>
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index a5cb19357f5..b215602c5ed 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1529,17 +1529,54 @@ SELCT 1/0;<!-- this typo is intentional -->
bytes.
</para>
+ <para>
+ Likewise the server expects the client to not begin
+ the <acronym>SSL</acronym> negotiation until it receives the server's
+ single byte response to the <acronym>SSL</acronym> request. If the
+ client begins the <acronym>SSL</acronym> negotiation immediately without
+ waiting for the server response to be received it can reduce connection
+ latency by one round-trip. However this comes at the cost of not being
+ able to handle the case where the server sends a negative response to the
+ <acronym>SSL</acronym> request. In that case instead of continuing with either GSSAPI or an
+ unencrypted connection or a protocol error the server will simply
+ disconnect.
+ </para>
+
<para>
An initial SSLRequest can also be used in a connection that is being
opened to send a CancelRequest message.
</para>
+ <para>
+ A second alternate way to initiate <acronym>SSL</acronym> encryption is
+ available. The server will recognize connections which immediately
+ begin <acronym>SSL</acronym> negotiation without any previous SSLRequest
+ packets. Once the <acronym>SSL</acronym> connection is established the
+ server will expect a normal startup-request packet and continue
+ negotiation over the encrypted channel. In this case any other requests
+ for encryption will be refused. This method is not preferred for general
+ purpose tools as it cannot negotiate the best connection encryption
+ available or handle unencrypted connections. However it is useful for
+ environments where both the server and client are controlled together.
+ In that case it avoids one round trip of latency and allows the use of
+ network tools that depend on standard <acronym>SSL</acronym> connections.
+ When using <acronym>SSL</acronym> connections in this style the client is
+ required to use the ALPN extension defined
+ by <ulink url="https://tools.ietf.org/html/rfc7301";>RFC 7301</ulink> to
+ protect against protocol confusion attacks.
+ The <productname>PostgreSQL</productname> protocol is "TBD-pgsql" as
+ registered
+ at <ulink url="https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids";>IANA
+ TLS ALPN Protocol IDs</ulink> registry.
+ </para>
+
<para>
While the protocol itself does not provide a way for the server to
force <acronym>SSL</acronym> encryption, the administrator can
configure the server to reject unencrypted sessions as a byproduct
of authentication checking.
</para>
+
</sect2>
<sect2 id="protocol-flow-gssapi">
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 5612c29f8b2..1d1329d1d95 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -109,18 +109,51 @@ secure_loaded_verify_locations(void)
int
secure_open_server(Port *port)
{
+#ifdef USE_SSL
int r = 0;
+ ssize_t len;
+
+ /* push unencrypted buffered data back through SSL setup */
+ len = pq_buffer_has_data();
+ if (len > 0)
+ {
+ char *buf = palloc(len);
+
+ pq_startmsgread();
+ if (pq_getbytes(buf, len) == EOF)
+ return STATUS_ERROR; /* shouldn't be possible */
+ pq_endmsgread();
+ port->raw_buf = buf;
+ port->raw_buf_remaining = len;
+ port->raw_buf_consumed = 0;
+ }
+ Assert(pq_buffer_has_data() == 0);
-#ifdef USE_SSL
r = be_tls_open_server(port);
+ if (port->raw_buf_remaining > 0)
+ {
+ /*
+ * This shouldn't be possible -- it would mean the client sent
+ * encrypted data before we established a session key...
+ */
+ elog(LOG, "Buffered unencrypted data remains after negotiating native SSL connection");
+ return STATUS_ERROR;
+ }
+ if (port->raw_buf != NULL)
+ {
+ pfree(port->raw_buf);
+ port->raw_buf = NULL;
+ }
+
ereport(DEBUG2,
(errmsg_internal("SSL connection from DN:\"%s\" CN:\"%s\"",
port->peer_dn ? port->peer_dn : "(anonymous)",
port->peer_cn ? port->peer_cn : "(anonymous)")));
-#endif
-
return r;
+#else
+ return 0;
+#endif
}
/*
@@ -232,6 +265,19 @@ secure_raw_read(Port *port, void *ptr, size_t len)
{
ssize_t n;
+ /* Read from the "unread" buffered data first. c.f. libpq-be.h */
+ if (port->raw_buf_remaining > 0)
+ {
+ /* consume up to len bytes from the raw_buf */
+ if (len > port->raw_buf_remaining)
+ len = port->raw_buf_remaining;
+ Assert(port->raw_buf);
+ memcpy(ptr, port->raw_buf + port->raw_buf_consumed, len);
+ port->raw_buf_consumed += len;
+ port->raw_buf_remaining -= len;
+ return len;
+ }
+
/*
* Try to read from the socket without blocking. If it succeeds we're
* done, otherwise we'll wait for the socket using the latch mechanism.
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 6497100a1a4..58f6e3da4f8 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -1114,15 +1114,16 @@ pq_discardbytes(size_t len)
}
/* --------------------------------
- * pq_buffer_has_data - is any buffered data available to read?
+ * pq_buffer_has_data - return number of bytes in receive buffer
*
- * This will *not* attempt to read more data.
+ * This will *not* attempt to read more data. And reading up to that number of
+ * bytes should not cause reading any more data either.
* --------------------------------
*/
-bool
+size_t
pq_buffer_has_data(void)
{
- return (PqRecvPointer < PqRecvLength);
+ return (PqRecvLength - PqRecvPointer);
}
diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c
index 0b9f899cd8b..d2ce8e47a8e 100644
--- a/src/backend/tcop/backend_startup.c
+++ b/src/backend/tcop/backend_startup.c
@@ -38,6 +38,7 @@
#include "utils/timeout.h"
static void BackendInitialize(ClientSocket *client_sock, CAC_state cac);
+static int ProcessSSLStartup(Port *port);
static int ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done);
static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options);
static void process_startup_packet_die(SIGNAL_ARGS);
@@ -252,7 +253,9 @@ BackendInitialize(ClientSocket *client_sock, CAC_state cac)
* Receive the startup packet (which might turn out to be a cancel request
* packet).
*/
- status = ProcessStartupPacket(port, false, false);
+ status = ProcessSSLStartup(port);
+ if (status == STATUS_OK)
+ status = ProcessStartupPacket(port, false, false);
/*
* If we're going to reject the connection due to database state, say so
@@ -344,6 +347,68 @@ BackendInitialize(ClientSocket *client_sock, CAC_state cac)
set_ps_display("initializing");
}
+/*
+ * Check for a native direct SSL connection.
+ *
+ * This happens before startup packets so we are careful not to actual read
+ * any bytes from the stream if it's not a direct SSL connection.
+ */
+static int
+ProcessSSLStartup(Port *port)
+{
+ int firstbyte;
+
+ Assert(!port->ssl_in_use);
+
+ pq_startmsgread();
+ firstbyte = pq_peekbyte();
+ pq_endmsgread();
+ if (firstbyte == EOF)
+ {
+ /*
+ * Like in ProcessStartupPacket, if we get no data at all, don't
+ * clutter the log with a complaint.
+ */
+ return STATUS_ERROR;
+ }
+
+ /*
+ * First byte indicates standard SSL handshake message
+ *
+ * (It can't be a Postgres startup length because in network byte order
+ * that would be a startup packet hundreds of megabytes long)
+ */
+ if (firstbyte == 0x16)
+ {
+ elog(LOG, "Detected direct SSL handshake");
+
+#ifdef USE_SSL
+ if (!LoadedSSL || port->laddr.addr.ss_family == AF_UNIX)
+ {
+ /* SSL not supported */
+ return STATUS_ERROR;
+ }
+ else if (secure_open_server(port) == -1)
+ {
+ /*
+ * we assume secure_open_server() sent an appropriate TLS alert
+ * already
+ */
+ return STATUS_ERROR;
+ }
+#else
+ /* SSL not supported by this build */
+ return STATUS_ERROR;
+#endif
+ }
+
+ if (port->ssl_in_use)
+ ereport(DEBUG2,
+ (errmsg_internal("Direct SSL connection established")));
+
+ return STATUS_OK;
+}
+
/*
* Read a client's startup packet and do something according to it.
*
@@ -465,8 +530,13 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
char SSLok;
#ifdef USE_SSL
- /* No SSL when disabled or on Unix sockets */
- if (!LoadedSSL || port->laddr.addr.ss_family == AF_UNIX)
+
+ /*
+ * No SSL when disabled or on Unix sockets.
+ *
+ * Also no SSL negotiation if we already have a direct SSL connection
+ */
+ if (!LoadedSSL || port->laddr.addr.ss_family == AF_UNIX || port->ssl_in_use)
SSLok = 'N';
else
SSLok = 'S'; /* Support for SSL */
@@ -474,11 +544,10 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
SSLok = 'N'; /* No support for SSL */
#endif
-retry1:
- if (send(port->sock, &SSLok, 1, 0) != 1)
+ while (secure_write(port, &SSLok, 1) != 1)
{
if (errno == EINTR)
- goto retry1; /* if interrupted, just retry */
+ continue; /* if interrupted, just retry */
ereport(COMMERROR,
(errcode_for_socket_access(),
errmsg("failed to send SSL negotiation response: %m")));
@@ -519,7 +588,7 @@ retry1:
GSSok = 'G';
#endif
- while (send(port->sock, &GSSok, 1, 0) != 1)
+ while (secure_write(port, &GSSok, 1) != 1)
{
if (errno == EINTR)
continue;
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 4dce7677510..86f773999ed 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -212,6 +212,19 @@ typedef struct Port
SSL *ssl;
X509 *peer;
#endif
+
+ /*
+ * This is a bit of a hack. The raw_buf is data that was previously read
+ * and buffered in a higher layer but then "unread" and needs to be read
+ * again while establishing an SSL connection via the SSL library layer.
+ *
+ * There's no API to "unread", the upper layer just places the data in the
+ * Port structure in raw_buf and sets raw_buf_remaining to the amount of
+ * bytes unread and raw_buf_consumed to 0.
+ */
+ char *raw_buf;
+ ssize_t raw_buf_consumed,
+ raw_buf_remaining;
} Port;
/*
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index be054b59dd1..a9c89e8179b 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -79,7 +79,7 @@ extern int pq_getmessage(StringInfo s, int maxlen);
extern int pq_getbyte(void);
extern int pq_peekbyte(void);
extern int pq_getbyte_if_available(unsigned char *c);
-extern bool pq_buffer_has_data(void);
+extern size_t pq_buffer_has_data(void);
extern int pq_putmessage_v2(char msgtype, const char *s, size_t len);
extern bool pq_check_connection(void);
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 01e49c6975e..a559c80e258 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -129,6 +129,7 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
#define DefaultSSLMode "disable"
#define DefaultSSLCertMode "disable"
#endif
+#define DefaultSSLNegotiation "postgres"
#ifdef ENABLE_GSS
#include "fe-gssapi-common.h"
#define DefaultGSSMode "prefer"
@@ -272,6 +273,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
"SSL-Mode", "", 12, /* sizeof("verify-full") == 12 */
offsetof(struct pg_conn, sslmode)},
+ {"sslnegotiation", "PGSSLNEGOTIATION", DefaultSSLNegotiation, NULL,
+ "SSL-Negotiation", "", 14, /* strlen("requiredirect") == 14 */
+ offsetof(struct pg_conn, sslnegotiation)},
+
{"sslcompression", "PGSSLCOMPRESSION", "0", NULL,
"SSL-Compression", "", 1,
offsetof(struct pg_conn, sslcompression)},
@@ -1565,10 +1570,37 @@ pqConnectOptions2(PGconn *conn)
}
#endif
}
+
+ /*
+ * validate sslnegotiation option, default is "postgres" for the postgres
+ * style negotiated connection with an extra round trip but more options.
+ */
+ if (conn->sslnegotiation)
+ {
+ if (strcmp(conn->sslnegotiation, "postgres") != 0
+ && strcmp(conn->sslnegotiation, "direct") != 0
+ && strcmp(conn->sslnegotiation, "requiredirect") != 0)
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
+ "sslnegotiation", conn->sslnegotiation);
+ return false;
+ }
+
+#ifndef USE_SSL
+ if (conn->sslnegotiation[0] != 'p')
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "sslnegotiation value \"%s\" invalid when SSL support is not compiled in",
+ conn->sslnegotiation);
+ return false;
+ }
+#endif
+ }
else
{
- conn->sslmode = strdup(DefaultSSLMode);
- if (!conn->sslmode)
+ conn->sslnegotiation = strdup(DefaultSSLNegotiation);
+ if (!conn->sslnegotiation)
goto oom_error;
}
@@ -1691,6 +1723,21 @@ pqConnectOptions2(PGconn *conn)
conn->gssencmode);
return false;
}
+#endif
+#ifdef USE_SSL
+
+ /*
+ * GSS is incompatible with direct SSL connections so it requires the
+ * default postgres style connection ssl negotiation
+ */
+ if (strcmp(conn->gssencmode, "require") == 0 &&
+ strcmp(conn->sslnegotiation, "postgres") != 0)
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "gssencmode value \"%s\" invalid when Direct SSL negotiation is enabled",
+ conn->gssencmode);
+ return false;
+ }
#endif
}
else
@@ -2793,11 +2840,12 @@ keep_going: /* We will come back to here until there is
/* initialize these values based on SSL mode */
conn->allow_ssl_try = (conn->sslmode[0] != 'd'); /* "disable" */
conn->wait_ssl_try = (conn->sslmode[0] == 'a'); /* "allow" */
+ /* direct ssl is incompatible with "allow" or "disabled" ssl */
+ conn->allow_direct_ssl_try = conn->allow_ssl_try && !conn->wait_ssl_try && (conn->sslnegotiation[0] != 'p');
#endif
#ifdef ENABLE_GSS
conn->try_gss = (conn->gssencmode[0] != 'd'); /* "disable" */
#endif
-
reset_connection_state_machine = false;
need_new_connection = true;
}
@@ -3255,6 +3303,29 @@ keep_going: /* We will come back to here until there is
if (pqsecure_initialize(conn, false, true) < 0)
goto error_return;
+ /*
+ * If SSL is enabled and direct SSL connections are enabled
+ * and we haven't already established an SSL connection (or
+ * already tried a direct connection and failed or succeeded)
+ * then try just enabling SSL directly.
+ *
+ * If we fail then we'll either fail the connection (if
+ * sslnegotiation is set to requiredirect or turn
+ * allow_direct_ssl_try to false
+ */
+ if (conn->allow_ssl_try
+ && !conn->wait_ssl_try
+ && conn->allow_direct_ssl_try
+ && !conn->ssl_in_use
+#ifdef ENABLE_GSS
+ && !conn->gssenc
+#endif
+ )
+ {
+ conn->status = CONNECTION_SSL_STARTUP;
+ return PGRES_POLLING_WRITING;
+ }
+
/*
* If SSL is enabled and we haven't already got encryption of
* some sort running, request SSL instead of sending the
@@ -3354,9 +3425,11 @@ keep_going: /* We will come back to here until there is
/*
* On first time through, get the postmaster's response to our
- * SSL negotiation packet.
+ * SSL negotiation packet. If we are trying a direct ssl
+ * connection skip reading the negotiation packet and go
+ * straight to initiating an ssl connection.
*/
- if (!conn->ssl_in_use)
+ if (!conn->ssl_in_use && !conn->allow_direct_ssl_try)
{
/*
* We use pqReadData here since it has the logic to
@@ -3462,6 +3535,21 @@ keep_going: /* We will come back to here until there is
}
if (pollres == PGRES_POLLING_FAILED)
{
+ /*
+ * Failed direct ssl connection, possibly try a new
+ * connection with postgres negotiation
+ */
+ if (conn->allow_direct_ssl_try)
+ {
+ /* if it's requiredirect then it's a hard failure */
+ if (conn->sslnegotiation[0] == 'r')
+ goto error_return;
+ /* otherwise only retry using postgres connection */
+ conn->allow_direct_ssl_try = false;
+ need_new_connection = true;
+ goto keep_going;
+ }
+
/*
* Failed ... if sslmode is "prefer" then do a non-SSL
* retry
@@ -4464,6 +4552,7 @@ freePGconn(PGconn *conn)
free(conn->keepalives_interval);
free(conn->keepalives_count);
free(conn->sslmode);
+ free(conn->sslnegotiation);
free(conn->sslcert);
free(conn->sslkey);
if (conn->sslpassword)
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 09b485bd2bc..ef6ce9d0c0b 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -80,8 +80,9 @@ typedef enum
CONNECTION_CHECK_TARGET, /* Internal state: checking target server
* properties. */
CONNECTION_CHECK_STANDBY, /* Checking if server is in standby mode. */
- CONNECTION_ALLOCATED /* Waiting for connection attempt to be
+ CONNECTION_ALLOCATED, /* Waiting for connection attempt to be
* started. */
+ CONNECTION_DIRECT_SSL_STARTUP /* Starting SSL without PG Negotiation. */
} ConnStatusType;
typedef enum
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 9c05f11a6e9..78a39dccfce 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -388,6 +388,8 @@ struct pg_conn
char *keepalives_count; /* maximum number of TCP keepalive
* retransmits */
char *sslmode; /* SSL mode (require,prefer,allow,disable) */
+ char *sslnegotiation; /* SSL initiation style
+ * (postgres,direct,requiredirect) */
char *sslcompression; /* SSL compression (0 or 1) */
char *sslkey; /* client key filename */
char *sslcert; /* client certificate filename */
@@ -553,6 +555,8 @@ struct pg_conn
bool ssl_cert_sent; /* Did we send one in reply? */
#ifdef USE_SSL
+ bool allow_direct_ssl_try; /* Try to make a direct SSL connection
+ * without an "SSL negotiation packet" */
bool allow_ssl_try; /* Allowed to try SSL negotiation */
bool wait_ssl_try; /* Delay SSL negotiation until after
* attempting normal connection */
--
2.39.2
From f0bac7c748804693818746fd99b4c09e3e9e3317 Mon Sep 17 00:00:00 2001
From: Greg Stark <st...@mit.edu>
Date: Mon, 20 Mar 2023 14:09:30 -0400
Subject: [PATCH v9 4/6] Direct SSL connections ALPN support
Move code to check for alpn
- if non-direct SSL is used, and client sends an unexpected ALPN
protocol, that's now an error ?
Author: Greg Stark, Heikki Linnakangas
---
src/backend/libpq/be-secure-openssl.c | 84 +++++++++++++++++++
src/backend/libpq/be-secure.c | 3 +
src/backend/tcop/backend_startup.c | 13 +++
src/backend/utils/misc/guc_tables.c | 9 ++
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/bin/psql/command.c | 7 +-
src/include/libpq/libpq-be.h | 1 +
src/include/libpq/libpq.h | 1 +
src/include/libpq/pqcomm.h | 19 +++++
src/interfaces/libpq/fe-connect.c | 5 ++
src/interfaces/libpq/fe-secure-openssl.c | 35 ++++++++
src/interfaces/libpq/libpq-int.h | 1 +
12 files changed, 177 insertions(+), 2 deletions(-)
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 72e43af3537..6f950f9c03c 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -67,6 +67,12 @@ static int ssl_external_passwd_cb(char *buf, int size, int rwflag, void *userdat
static int dummy_ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata);
static int verify_cb(int ok, X509_STORE_CTX *ctx);
static void info_cb(const SSL *ssl, int type, int args);
+static int alpn_cb(SSL *ssl,
+ const unsigned char **out,
+ unsigned char *outlen,
+ const unsigned char *in,
+ unsigned int inlen,
+ void *userdata);
static bool initialize_dh(SSL_CTX *context, bool isServerStart);
static bool initialize_ecdh(SSL_CTX *context, bool isServerStart);
static const char *SSLerrmessage(unsigned long ecode);
@@ -432,6 +438,16 @@ be_tls_open_server(Port *port)
/* set up debugging/info callback */
SSL_CTX_set_info_callback(SSL_context, info_cb);
+ if (ssl_enable_alpn)
+ {
+ elog(DEBUG2, "Enabling OpenSSL ALPN callback");
+ SSL_CTX_set_alpn_select_cb(SSL_context, alpn_cb, port);
+ }
+ else
+ {
+ elog(DEBUG2, "OpenSSL ALPN is disabled, not setting callback");
+ }
+
if (!(port->ssl = SSL_new(SSL_context)))
{
ereport(COMMERROR,
@@ -571,6 +587,32 @@ aloop:
return -1;
}
+ /* Get the protocol selected by ALPN */
+ port->alpn_used = false;
+ {
+ const unsigned char *selected;
+ unsigned int len;
+
+ SSL_get0_alpn_selected(port->ssl, &selected, &len);
+
+ /* If ALPN is used, check that we negotiated the expected protocol */
+ if (selected != NULL)
+ {
+ if (len == strlen(PG_ALPN_PROTOCOL) &&
+ memcmp(selected, PG_ALPN_PROTOCOL, strlen(PG_ALPN_PROTOCOL)) == 0)
+ {
+ port->alpn_used = true;
+ }
+ else
+ {
+ /* shouldn't happen */
+ ereport(COMMERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("Received SSL connection request with unexpected ALPN protocol")));
+ }
+ }
+ }
+
/* Get client certificate, if available. */
port->peer = SSL_get_peer_certificate(port->ssl);
@@ -1259,6 +1301,48 @@ info_cb(const SSL *ssl, int type, int args)
}
}
+/* See pqcomm.h comments on OpenSSL implementation of ALPN (RFC 7301) */
+static const unsigned char alpn_protos[] = PG_ALPN_PROTOCOL_VECTOR;
+
+/*
+ * Server callback for ALPN negotiation. We use the standard "helper" function
+ * even though currently we only accept one value.
+ */
+static int
+alpn_cb(SSL *ssl,
+ const unsigned char **out,
+ unsigned char *outlen,
+ const unsigned char *in,
+ unsigned int inlen,
+ void *userdata)
+{
+ /*
+ * Why does OpenSSL provide a helper function that requires a nonconst
+ * vector when the callback is declared to take a const vector? What are
+ * we to do with that?
+ */
+ int retval;
+
+ Assert(userdata != NULL);
+ Assert(out != NULL);
+ Assert(outlen != NULL);
+ Assert(in != NULL);
+
+ retval = SSL_select_next_proto((unsigned char **) out, outlen,
+ alpn_protos, sizeof(alpn_protos),
+ in, inlen);
+ if (*out == NULL || *outlen > sizeof(alpn_protos) || outlen <= 0)
+ return SSL_TLSEXT_ERR_NOACK; /* can't happen */
+
+ if (retval == OPENSSL_NPN_NEGOTIATED)
+ return SSL_TLSEXT_ERR_OK;
+ else if (retval == OPENSSL_NPN_NO_OVERLAP)
+ return SSL_TLSEXT_ERR_NOACK;
+ else
+ return SSL_TLSEXT_ERR_NOACK;
+}
+
+
/*
* Set DH parameters for generating ephemeral DH keys. The
* DH parameters can take a long time to compute, so they must be
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 1d1329d1d95..20a1a4ad551 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -58,6 +58,9 @@ bool SSLPreferServerCiphers;
int ssl_min_protocol_version = PG_TLS1_2_VERSION;
int ssl_max_protocol_version = PG_TLS_ANY;
+/* GUC variable: if false ignore ALPN negotiation */
+bool ssl_enable_alpn = true;
+
/* ------------------------------------------------------------ */
/* Procedures common to all secure sessions */
/* ------------------------------------------------------------ */
diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c
index d2ce8e47a8e..5801d7576ba 100644
--- a/src/backend/tcop/backend_startup.c
+++ b/src/backend/tcop/backend_startup.c
@@ -382,6 +382,11 @@ ProcessSSLStartup(Port *port)
{
elog(LOG, "Detected direct SSL handshake");
+ if (!ssl_enable_alpn)
+ {
+ elog(WARNING, "Received direct SSL connection without ssl_enable_alpn enabled");
+ }
+
#ifdef USE_SSL
if (!LoadedSSL || port->laddr.addr.ss_family == AF_UNIX)
{
@@ -396,6 +401,14 @@ ProcessSSLStartup(Port *port)
*/
return STATUS_ERROR;
}
+
+ if (!port->alpn_used)
+ {
+ ereport(COMMERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("Received direct SSL connection request without required ALPN protocol negotiation extension")));
+ return STATUS_ERROR;
+ }
#else
/* SSL not supported by this build */
return STATUS_ERROR;
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index abd9029451f..ad917303770 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1085,6 +1085,15 @@ struct config_bool ConfigureNamesBool[] =
true,
NULL, NULL, NULL
},
+ {
+ {"ssl_enable_alpn", PGC_SIGHUP, CONN_AUTH_SSL,
+ gettext_noop("Respond to TLS ALPN Extension Requests."),
+ NULL,
+ },
+ &ssl_enable_alpn,
+ true,
+ NULL, NULL, NULL
+ },
{
{"fsync", PGC_SIGHUP, WAL_SETTINGS,
gettext_noop("Forces synchronization of updates to disk."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 2244ee52f79..34f0c3b56cf 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -127,6 +127,7 @@
#ssl_dh_params_file = ''
#ssl_passphrase_command = ''
#ssl_passphrase_command_supports_reload = off
+#ssl_enable_alpn = on
#------------------------------------------------------------------------------
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9b0fa041f73..5607dea8049 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3814,6 +3814,7 @@ printSSLInfo(void)
const char *protocol;
const char *cipher;
const char *compression;
+ const char *alpn;
if (!PQsslInUse(pset.db))
return; /* no SSL */
@@ -3821,11 +3822,13 @@ printSSLInfo(void)
protocol = PQsslAttribute(pset.db, "protocol");
cipher = PQsslAttribute(pset.db, "cipher");
compression = PQsslAttribute(pset.db, "compression");
+ alpn = PQsslAttribute(pset.db, "alpn");
- printf(_("SSL connection (protocol: %s, cipher: %s, compression: %s)\n"),
+ printf(_("SSL connection (protocol: %s, cipher: %s, compression: %s, ALPN: %s)\n"),
protocol ? protocol : _("unknown"),
cipher ? cipher : _("unknown"),
- (compression && strcmp(compression, "off") != 0) ? _("on") : _("off"));
+ (compression && strcmp(compression, "off") != 0) ? _("on") : _("off"),
+ alpn ? alpn : _("none"));
}
/*
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 86f773999ed..a2414c401bb 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -203,6 +203,7 @@ typedef struct Port
char *peer_cn;
char *peer_dn;
bool peer_cert_valid;
+ bool alpn_used;
/*
* OpenSSL structures. (Keep these last so that the locations of other
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index a9c89e8179b..91a61e9eacb 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -122,6 +122,7 @@ extern PGDLLIMPORT char *SSLECDHCurve;
extern PGDLLIMPORT bool SSLPreferServerCiphers;
extern PGDLLIMPORT int ssl_min_protocol_version;
extern PGDLLIMPORT int ssl_max_protocol_version;
+extern PGDLLIMPORT bool ssl_enable_alpn;
enum ssl_protocol_versions
{
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index 9ae469c86c4..fb93c820530 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -139,6 +139,25 @@ typedef struct CancelRequestPacket
uint32 cancelAuthCode; /* secret key to authorize cancel */
} CancelRequestPacket;
+/* Application-Layer Protocol Negotiation is required for direct connections
+ * to avoid protocol confusion attacks (e.g https://alpaca-attack.com/).
+ *
+ * ALPN is specified in RFC 7301
+ *
+ * This string should be registered at:
+ * https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids
+ *
+ * OpenSSL uses this wire-format for the list of alpn protocols even in the
+ * API. Both server and client take the same format parameter but the client
+ * actually sends it to the server as-is and the server it specifies the
+ * preference order to use to choose the one selected to send back.
+ *
+ * c.f. https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_alpn_select_cb.html
+ *
+ * The #define can be used to initialize a char[] vector to use directly in the API
+ */
+#define PG_ALPN_PROTOCOL "TBD-pgsql"
+#define PG_ALPN_PROTOCOL_VECTOR { 9, 'T','B','D','-','p','g','s','q','l' }
/*
* A client can also start by sending a SSL or GSSAPI negotiation request to
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index a559c80e258..620a2abbfa3 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -313,6 +313,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
"SSL-SNI", "", 1,
offsetof(struct pg_conn, sslsni)},
+ {"sslalpn", "PGSSLALPN", "1", NULL,
+ "SSL-ALPN", "", 1,
+ offsetof(struct pg_conn, sslalpn)},
+
{"requirepeer", "PGREQUIREPEER", NULL, NULL,
"Require-Peer", "", 10,
offsetof(struct pg_conn, requirepeer)},
@@ -4566,6 +4570,7 @@ freePGconn(PGconn *conn)
free(conn->sslcrldir);
free(conn->sslcompression);
free(conn->sslsni);
+ free(conn->sslalpn);
free(conn->requirepeer);
free(conn->require_auth);
free(conn->ssl_min_protocol_version);
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index bf45a8edc31..9aad2ac605c 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -885,6 +885,9 @@ destroy_ssl_system(void)
#endif
}
+/* See pqcomm.h comments on OpenSSL implementation of ALPN (RFC 7301) */
+static unsigned char alpn_protos[] = PG_ALPN_PROTOCOL_VECTOR;
+
/*
* Create per-connection SSL object, and load the client certificate,
* private key, and trusted CA certs.
@@ -1234,6 +1237,22 @@ initialize_SSL(PGconn *conn)
}
}
+ if (conn->sslalpn && conn->sslalpn[0] == '1')
+ {
+ int retval;
+
+ retval = SSL_set_alpn_protos(conn->ssl, alpn_protos, sizeof(alpn_protos));
+
+ if (retval != 0)
+ {
+ char *err = SSLerrmessage(ERR_get_error());
+
+ libpq_append_conn_error(conn, "could not set ssl alpn extension: %s", err);
+ SSLerrfree(err);
+ return -1;
+ }
+ }
+
/*
* Read the SSL key. If a key is specified, treat it as an engine:key
* combination if there is colon present - we don't support files with
@@ -1753,6 +1772,7 @@ PQsslAttributeNames(PGconn *conn)
"cipher",
"compression",
"protocol",
+ "alpn",
NULL
};
static const char *const empty_attrs[] = {NULL};
@@ -1807,6 +1827,21 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
if (strcmp(attribute_name, "protocol") == 0)
return SSL_get_version(conn->ssl);
+ if (strcmp(attribute_name, "alpn") == 0)
+ {
+ const unsigned char *data;
+ unsigned int len;
+ static char alpn_str[256]; /* alpn doesn't support longer than 255
+ * bytes */
+
+ SSL_get0_alpn_selected(conn->ssl, &data, &len);
+ if (data == NULL || len == 0 || len > sizeof(alpn_str) - 1)
+ return NULL;
+ memcpy(alpn_str, data, len);
+ alpn_str[len] = 0;
+ return alpn_str;
+ }
+
return NULL; /* unknown attribute */
}
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 78a39dccfce..def49a68af3 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -399,6 +399,7 @@ struct pg_conn
char *sslcrl; /* certificate revocation list filename */
char *sslcrldir; /* certificate revocation list directory name */
char *sslsni; /* use SSL SNI extension (0 or 1) */
+ char *sslalpn; /* use SSL ALPN extension (0 or 1) */
char *requirepeer; /* required peer credentials for local sockets */
char *gssencmode; /* GSS mode (require,prefer,disable) */
char *krbsrvname; /* Kerberos service name */
--
2.39.2
From 5d744b1f29502a1e47064b1a929b96e909191f89 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Wed, 10 Jan 2024 10:25:08 +0200
Subject: [PATCH v9 5/6] Add tests for sslnegotiation
Author: Heikki Linnakangas, Matthias van de Meent
---
.../t/001_negotiate_encryption.pl | 25 +++++++++++++------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/src/test/libpq_encryption/t/001_negotiate_encryption.pl b/src/test/libpq_encryption/t/001_negotiate_encryption.pl
index b8646c5bc97..b8b4cd2726d 100644
--- a/src/test/libpq_encryption/t/001_negotiate_encryption.pl
+++ b/src/test/libpq_encryption/t/001_negotiate_encryption.pl
@@ -231,13 +231,13 @@ sub resolve_connection_type
# First test with SSL disabled in the server
-# Test the cube of parameters: user, sslmode, and gssencmode
+# Test the cube of parameters: user, sslmode, sslnegotiation and gssencmode
sub test_modes
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($pg_node, $node_conf,
- $test_users, $ssl_modes, $gss_modes) = @_;
+ $test_users, $ssl_modes, $ssl_negotiations, $gss_modes) = @_;
foreach my $test_user (@{$test_users})
{
@@ -253,13 +253,18 @@ sub test_modes
gssmode=>$gssencmode,
);
my $res = resolve_connection_type(\%params);
- connect_test($pg_node, "user=$test_user sslmode=$client_mode gssencmode=$gssencmode", $res);
+ # Negotiation type doesn't matter for supported connection types
+ foreach my $negotiation (@{$ssl_negotiations})
+ {
+ connect_test($pg_node, "user=$test_user sslmode=$client_mode sslnegotiation=$negotiation gssencmode=$gssencmode", $res);
+ }
}
}
}
}
my $sslmodes = ['disable', 'allow', 'prefer', 'require'];
+my $sslnegotiations = ['postgres', 'direct', 'requiredirect'];
my $gssencmodes = ['disable', 'prefer', 'require'];
my $server_config = {
@@ -270,7 +275,7 @@ my $server_config = {
note("Running tests with SSL and GSS disabled in server");
test_modes($node, $server_config,
['testuser'],
- $sslmodes, $gssencmodes);
+ $sslmodes, $sslnegotiations, $gssencmodes);
# Enable SSL in the server
SKIP:
@@ -284,7 +289,7 @@ SKIP:
note("Running tests with SSL enabled in server");
test_modes($node, $server_config,
['testuser', 'ssluser', 'nossluser'],
- $sslmodes, ['disable']);
+ $sslmodes, $sslnegotiations, ['disable']);
$node->adjust_conf('postgresql.conf', 'ssl', 'off');
$node->reload;
@@ -306,7 +311,7 @@ SKIP:
note("Running tests with GSS enabled in server");
test_modes($node, $server_config,
['testuser', 'gssuser', 'nogssuser'],
- $sslmodes, $gssencmodes);
+ $sslmodes, $sslnegotiations, $gssencmodes);
# Check that logs match the expected 'no pg_hba.conf entry' line, too, as
# that is not tested by test_modes.
@@ -320,6 +325,10 @@ SKIP:
# with no encryption.
connect_test($node, 'user=nogssuser sslmode=prefer gssencmode=prefer', 'plain',
'no pg_hba.conf entry for host "127.0.0.1", user "nogssuser", database "postgres", GSS encryption');
+ connect_test($node, 'user=nogssuser sslmode=prefer gssencmode=prefer sslnegotiation=direct', 'plain',
+ 'no pg_hba.conf entry for host "127.0.0.1", user "nogssuser", database "postgres", GSS encryption');
+ connect_test($node, 'user=nogssuser sslmode=prefer gssencmode=prefer sslnegotiation=requiredirect', 'plain',
+ 'no pg_hba.conf entry for host "127.0.0.1", user "nogssuser", database "postgres", GSS encryption');
}
# Server supports both SSL and GSSAPI
@@ -329,6 +338,8 @@ SKIP:
# SSL is still disabled
connect_test($node, 'user=testuser sslmode=prefer gssencmode=prefer', 'gss');
+ connect_test($node, 'user=testuser sslmode=prefer gssencmode=prefer sslnegotiation=direct', 'gss');
+ connect_test($node, 'user=testuser sslmode=prefer gssencmode=prefer sslnegotiation=requiredirect', 'gss');
# Enable SSL
$node->adjust_conf('postgresql.conf', 'ssl', 'on');
@@ -338,7 +349,7 @@ SKIP:
note("Running tests with both GSS and SSL enabled in server");
test_modes($node, $server_config,
['testuser', 'gssuser', 'ssluser', 'nogssuser', 'nossluser'],
- $sslmodes, $gssencmodes);
+ $sslmodes, $sslnegotiations, $gssencmodes);
# Test case that server supports GSSAPI, but it's not allowed for
# this user. Special cased because we check output
--
2.39.2
From 8254d98b20bad0eb87769da825f02a9d6b126a58 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Tue, 5 Mar 2024 16:03:52 +0200
Subject: [PATCH v9 6/6] WIP: refactor state machine in libpq
---
src/interfaces/libpq/fe-connect.c | 506 +++++++++++++----------
src/interfaces/libpq/fe-secure-openssl.c | 12 +-
src/interfaces/libpq/libpq-fe.h | 3 +-
src/interfaces/libpq/libpq-int.h | 18 +-
4 files changed, 315 insertions(+), 224 deletions(-)
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 620a2abbfa3..edc324dad05 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -396,6 +396,12 @@ static const char uri_designator[] = "postgresql://";
static const char short_uri_designator[] = "postgres://";
static bool connectOptions1(PGconn *conn, const char *conninfo);
+static bool init_allowed_encryption_methods(PGconn *conn);
+#if defined(USE_SSL) || defined(USE_GSS)
+static int encryption_negotiation_failed(PGconn *conn);
+#endif
+static bool connection_failed(PGconn *conn);
+static bool select_next_encryption_method(PGconn *conn, bool negotiation_failure);
static PGPing internal_ping(PGconn *conn);
static void pqFreeCommandQueue(PGcmdQueueEntry *queue);
static bool fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
@@ -1727,21 +1733,6 @@ pqConnectOptions2(PGconn *conn)
conn->gssencmode);
return false;
}
-#endif
-#ifdef USE_SSL
-
- /*
- * GSS is incompatible with direct SSL connections so it requires the
- * default postgres style connection ssl negotiation
- */
- if (strcmp(conn->gssencmode, "require") == 0 &&
- strcmp(conn->sslnegotiation, "postgres") != 0)
- {
- conn->status = CONNECTION_BAD;
- libpq_append_conn_error(conn, "gssencmode value \"%s\" invalid when Direct SSL negotiation is enabled",
- conn->gssencmode);
- return false;
- }
#endif
}
else
@@ -2840,16 +2831,9 @@ keep_going: /* We will come back to here until there is
*/
conn->pversion = PG_PROTOCOL(3, 0);
conn->send_appname = true;
-#ifdef USE_SSL
- /* initialize these values based on SSL mode */
- conn->allow_ssl_try = (conn->sslmode[0] != 'd'); /* "disable" */
- conn->wait_ssl_try = (conn->sslmode[0] == 'a'); /* "allow" */
- /* direct ssl is incompatible with "allow" or "disabled" ssl */
- conn->allow_direct_ssl_try = conn->allow_ssl_try && !conn->wait_ssl_try && (conn->sslnegotiation[0] != 'p');
-#endif
-#ifdef ENABLE_GSS
- conn->try_gss = (conn->gssencmode[0] != 'd'); /* "disable" */
-#endif
+ conn->failed_enc_methods = 0;
+ conn->current_enc_method = 0;
+ conn->allowed_enc_methods = 0;
reset_connection_state_machine = false;
need_new_connection = true;
}
@@ -2875,6 +2859,34 @@ keep_going: /* We will come back to here until there is
need_new_connection = false;
}
+ /* Decide what to do next, if SSL or GSS negotiation fails */
+#define ENCRYPTION_NEGOTIATION_FAILED() \
+ do { \
+ switch (encryption_negotiation_failed(conn)) \
+ { \
+ case 0: \
+ goto error_return; \
+ case 1: \
+ conn->status = CONNECTION_MADE; \
+ return PGRES_POLLING_WRITING; \
+ case 2: \
+ need_new_connection = true; \
+ goto keep_going; \
+ } \
+ } while(0);
+
+ /* Decide what to do next, if connection fails */
+#define CONNECTION_FAILED() \
+ do { \
+ if (connection_failed(conn)) \
+ { \
+ need_new_connection = true; \
+ goto keep_going; \
+ } \
+ else \
+ goto error_return; \
+ } while(0);
+
/* Now try to advance the state machine for this connection */
switch (conn->status)
{
@@ -3189,18 +3201,6 @@ keep_going: /* We will come back to here until there is
goto error_return;
}
- /*
- * Make sure we can write before advancing to next step.
- */
- conn->status = CONNECTION_MADE;
- return PGRES_POLLING_WRITING;
- }
-
- case CONNECTION_MADE:
- {
- char *startpacket;
- int packetlen;
-
/*
* Implement requirepeer check, if requested and it's a
* Unix-domain socket.
@@ -3249,30 +3249,31 @@ keep_going: /* We will come back to here until there is
#endif /* WIN32 */
}
- if (conn->raddr.addr.ss_family == AF_UNIX)
- {
- /* Don't request SSL or GSSAPI over Unix sockets */
-#ifdef USE_SSL
- conn->allow_ssl_try = false;
-#endif
-#ifdef ENABLE_GSS
- conn->try_gss = false;
-#endif
- }
+ /* Choose encryption method to try first */
+ if (!init_allowed_encryption_methods(conn))
+ goto error_return;
+
+ /*
+ * Make sure we can write before advancing to next step.
+ */
+ conn->status = CONNECTION_MADE;
+ return PGRES_POLLING_WRITING;
+ }
+
+ case CONNECTION_MADE:
+ {
+ char *startpacket;
+ int packetlen;
#ifdef ENABLE_GSS
/*
- * If GSSAPI encryption is enabled, then call
- * pg_GSS_have_cred_cache() which will return true if we can
- * acquire credentials (and give us a handle to use in
- * conn->gcred), and then send a packet to the server asking
- * for GSSAPI Encryption (and skip past SSL negotiation and
- * regular startup below).
+ * If GSSAPI encryption is enabled, send a packet to the
+ * server asking for GSSAPI Encryption and proceed with GSSAPI
+ * handshake. We will come back here after GSSAPI encryption
+ * has been established, with conn->gctx set.
*/
- if (conn->try_gss && !conn->gctx)
- conn->try_gss = pg_GSS_have_cred_cache(&conn->gcred);
- if (conn->try_gss && !conn->gctx)
+ if (conn->current_enc_method == ENC_GSSAPI && !conn->gctx)
{
ProtocolVersion pv = pg_hton32(NEGOTIATE_GSS_CODE);
@@ -3287,12 +3288,6 @@ keep_going: /* We will come back to here until there is
conn->status = CONNECTION_GSS_STARTUP;
return PGRES_POLLING_READING;
}
- else if (!conn->gctx && conn->gssencmode[0] == 'r')
- {
- libpq_append_conn_error(conn,
- "GSSAPI encryption required but was impossible (possibly no credential cache, no server support, or using a local socket)");
- goto error_return;
- }
#endif
#ifdef USE_SSL
@@ -3308,39 +3303,22 @@ keep_going: /* We will come back to here until there is
goto error_return;
/*
- * If SSL is enabled and direct SSL connections are enabled
- * and we haven't already established an SSL connection (or
- * already tried a direct connection and failed or succeeded)
- * then try just enabling SSL directly.
- *
- * If we fail then we'll either fail the connection (if
- * sslnegotiation is set to requiredirect or turn
- * allow_direct_ssl_try to false
+ * If direct SSL is enabled, jump right into SSL handshake. We
+ * will come back here after SSL encryption has been
+ * established, with ssl_in_use set.
*/
- if (conn->allow_ssl_try
- && !conn->wait_ssl_try
- && conn->allow_direct_ssl_try
- && !conn->ssl_in_use
-#ifdef ENABLE_GSS
- && !conn->gssenc
-#endif
- )
+ if (conn->current_enc_method == ENC_DIRECT_SSL && !conn->ssl_in_use)
{
conn->status = CONNECTION_SSL_STARTUP;
return PGRES_POLLING_WRITING;
}
/*
- * If SSL is enabled and we haven't already got encryption of
- * some sort running, request SSL instead of sending the
- * startup message.
+ * If negotiated SSL is enabled, request SSL and proceed with
+ * SSL handshake. We will come back here after SSL encryption
+ * has been established, with ssl_in_use set.
*/
- if (conn->allow_ssl_try && !conn->wait_ssl_try &&
- !conn->ssl_in_use
-#ifdef ENABLE_GSS
- && !conn->gssenc
-#endif
- )
+ if (conn->current_enc_method == ENC_NEGOTIATED_SSL && !conn->ssl_in_use)
{
ProtocolVersion pv;
@@ -3388,8 +3366,11 @@ keep_going: /* We will come back to here until there is
}
/*
- * Build the startup packet.
+ * We have now established encryption, or we are happy to
+ * proceed without.
*/
+
+ /* Build the startup packet. */
startpacket = pqBuildStartupPacket3(conn, &packetlen,
EnvironmentOptions);
if (!startpacket)
@@ -3430,10 +3411,9 @@ keep_going: /* We will come back to here until there is
/*
* On first time through, get the postmaster's response to our
* SSL negotiation packet. If we are trying a direct ssl
- * connection skip reading the negotiation packet and go
- * straight to initiating an ssl connection.
+ * connection, go straight to initiating ssl.
*/
- if (!conn->ssl_in_use && !conn->allow_direct_ssl_try)
+ if (!conn->ssl_in_use && conn->current_enc_method == ENC_NEGOTIATED_SSL)
{
/*
* We use pqReadData here since it has the logic to
@@ -3463,34 +3443,14 @@ keep_going: /* We will come back to here until there is
{
/* mark byte consumed */
conn->inStart = conn->inCursor;
-
- /*
- * Set up global SSL state if required. The crypto
- * state has already been set if libpq took care of
- * doing that, so there is no need to make that happen
- * again.
- */
- if (pqsecure_initialize(conn, true, false) != 0)
- goto error_return;
}
else if (SSLok == 'N')
{
/* mark byte consumed */
conn->inStart = conn->inCursor;
/* OK to do without SSL? */
- if (conn->sslmode[0] == 'r' || /* "require" */
- conn->sslmode[0] == 'v') /* "verify-ca" or
- * "verify-full" */
- {
- /* Require SSL, but server does not want it */
- libpq_append_conn_error(conn, "server does not support SSL, but SSL was required");
- goto error_return;
- }
- /* Otherwise, proceed with normal startup */
- conn->allow_ssl_try = false;
/* We can proceed using this connection */
- conn->status = CONNECTION_MADE;
- return PGRES_POLLING_WRITING;
+ ENCRYPTION_NEGOTIATION_FAILED();
}
else if (SSLok == 'E')
{
@@ -3515,6 +3475,14 @@ keep_going: /* We will come back to here until there is
}
}
+ /*
+ * Set up global SSL state if required. The crypto state has
+ * already been set if libpq took care of doing that, so there
+ * is no need to make that happen again.
+ */
+ if (pqsecure_initialize(conn, true, false) != 0)
+ goto error_return;
+
/*
* Begin or continue the SSL negotiation process.
*/
@@ -3543,32 +3511,7 @@ keep_going: /* We will come back to here until there is
* Failed direct ssl connection, possibly try a new
* connection with postgres negotiation
*/
- if (conn->allow_direct_ssl_try)
- {
- /* if it's requiredirect then it's a hard failure */
- if (conn->sslnegotiation[0] == 'r')
- goto error_return;
- /* otherwise only retry using postgres connection */
- conn->allow_direct_ssl_try = false;
- need_new_connection = true;
- goto keep_going;
- }
-
- /*
- * Failed ... if sslmode is "prefer" then do a non-SSL
- * retry
- */
- if (conn->sslmode[0] == 'p' /* "prefer" */
- && conn->allow_ssl_try /* redundant? */
- && !conn->wait_ssl_try) /* redundant? */
- {
- /* only retry once */
- conn->allow_ssl_try = false;
- need_new_connection = true;
- goto keep_going;
- }
- /* Else it's a hard failure */
- goto error_return;
+ CONNECTION_FAILED();
}
/* Else, return POLLING_READING or POLLING_WRITING status */
return pollres;
@@ -3587,7 +3530,7 @@ keep_going: /* We will come back to here until there is
* If we haven't yet, get the postmaster's response to our
* negotiation packet
*/
- if (conn->try_gss && !conn->gctx)
+ if (!conn->gctx)
{
char gss_ok;
int rdresult = pqReadData(conn);
@@ -3611,9 +3554,7 @@ keep_going: /* We will come back to here until there is
* error message on retry). Server gets fussy if we
* don't hang up the socket, though.
*/
- conn->try_gss = false;
- need_new_connection = true;
- goto keep_going;
+ CONNECTION_FAILED();
}
/* mark byte consumed */
@@ -3621,17 +3562,8 @@ keep_going: /* We will come back to here until there is
if (gss_ok == 'N')
{
- /* Server doesn't want GSSAPI; fall back if we can */
- if (conn->gssencmode[0] == 'r')
- {
- libpq_append_conn_error(conn, "server doesn't support GSSAPI encryption, but it was required");
- goto error_return;
- }
-
- conn->try_gss = false;
/* We can proceed using this connection */
- conn->status = CONNECTION_MADE;
- return PGRES_POLLING_WRITING;
+ ENCRYPTION_NEGOTIATION_FAILED();
}
else if (gss_ok != 'G')
{
@@ -3663,18 +3595,7 @@ keep_going: /* We will come back to here until there is
}
else if (pollres == PGRES_POLLING_FAILED)
{
- if (conn->gssencmode[0] == 'p')
- {
- /*
- * We failed, but we can retry on "prefer". Have to
- * drop the current connection to do so, though.
- */
- conn->try_gss = false;
- need_new_connection = true;
- goto keep_going;
- }
- /* Else it's a hard failure */
- goto error_return;
+ CONNECTION_FAILED();
}
/* Else, return POLLING_READING or POLLING_WRITING status */
return pollres;
@@ -3850,55 +3771,7 @@ keep_going: /* We will come back to here until there is
/* Check to see if we should mention pgpassfile */
pgpassfileWarning(conn);
-#ifdef ENABLE_GSS
-
- /*
- * If gssencmode is "prefer" and we're using GSSAPI, retry
- * without it.
- */
- if (conn->gssenc && conn->gssencmode[0] == 'p')
- {
- /* only retry once */
- conn->try_gss = false;
- need_new_connection = true;
- goto keep_going;
- }
-#endif
-
-#ifdef USE_SSL
-
- /*
- * if sslmode is "allow" and we haven't tried an SSL
- * connection already, then retry with an SSL connection
- */
- if (conn->sslmode[0] == 'a' /* "allow" */
- && !conn->ssl_in_use
- && conn->allow_ssl_try
- && conn->wait_ssl_try)
- {
- /* only retry once */
- conn->wait_ssl_try = false;
- need_new_connection = true;
- goto keep_going;
- }
-
- /*
- * if sslmode is "prefer" and we're in an SSL connection,
- * then do a non-SSL retry
- */
- if (conn->sslmode[0] == 'p' /* "prefer" */
- && conn->ssl_in_use
- && conn->allow_ssl_try /* redundant? */
- && !conn->wait_ssl_try) /* redundant? */
- {
- /* only retry once */
- conn->allow_ssl_try = false;
- need_new_connection = true;
- goto keep_going;
- }
-#endif
-
- goto error_return;
+ CONNECTION_FAILED();
}
else if (beresp == PqMsg_NegotiateProtocolVersion)
{
@@ -4344,6 +4217,213 @@ error_return:
return PGRES_POLLING_FAILED;
}
+static bool
+init_allowed_encryption_methods(PGconn *conn)
+{
+ if (conn->raddr.addr.ss_family == AF_UNIX)
+ {
+ /* Don't request SSL or GSSAPI over Unix sockets */
+ conn->allowed_enc_methods &= ~(ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL | ENC_GSSAPI);
+
+ /* to give a better error message */
+
+ /*
+ * XXX: we probably should not do this. sslmode=require works
+ * differently
+ */
+ if (conn->gssencmode[0] == 'r')
+ {
+ libpq_append_conn_error(conn,
+ "GSSAPI encryption required but it is not supported over a local socket)");
+ conn->allowed_enc_methods = 0;
+ conn->current_enc_method = ENC_ERROR;
+ return false;
+ }
+
+ conn->allowed_enc_methods = ENC_PLAINTEXT;
+ conn->current_enc_method = ENC_PLAINTEXT;
+ return true;
+ }
+
+ /* initialize these values based on sslmode and gssencmode */
+ conn->allowed_enc_methods = 0;
+
+#ifdef USE_SSL
+ /* sslmode anything but 'disable', and GSSAPI not required */
+ if (conn->sslmode[0] != 'd' && conn->gssencmode[0] != 'r')
+ {
+ if (conn->sslnegotiation[0] == 'p')
+ conn->allowed_enc_methods |= ENC_NEGOTIATED_SSL;
+ else if (conn->sslnegotiation[0] == 'd')
+ conn->allowed_enc_methods |= ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL;
+ else if (conn->sslnegotiation[0] == 'r')
+ conn->allowed_enc_methods |= ENC_DIRECT_SSL;
+ }
+#endif
+
+#ifdef ENABLE_GSS
+ if (conn->gssencmode[0] != 'd')
+ conn->allowed_enc_methods |= ENC_GSSAPI;
+#endif
+
+ if ((conn->sslmode[0] == 'd' || conn->sslmode[0] == 'p' || conn->sslmode[0] == 'a') &&
+ (conn->gssencmode[0] == 'd' || conn->gssencmode[0] == 'p'))
+ {
+ conn->allowed_enc_methods |= ENC_PLAINTEXT;
+ }
+
+ return select_next_encryption_method(conn, false);
+}
+
+/*
+ * Out-of-line portion of the ENCRYPTION_NEGOTIATION_FAILED() macro in the
+ * PQconnectPoll state machine.
+ *
+ * Return value:
+ * 0: connection failed and we are out of encryption methods to try. return an error
+ * 1: Retry with next connection method. The TCP connection is still valid and in
+ * known state, so we can proceed with the negotiating next method without
+ * reconnecting.
+ * 2: Disconnect, and retry with next connection method.
+ *
+ * conn->current_enc_method is updated to the next method to try.
+ */
+#if defined(USE_SSL) || defined(USE_GSS)
+static int
+encryption_negotiation_failed(PGconn *conn)
+{
+ Assert((conn->failed_enc_methods & conn->current_enc_method) == 0);
+ conn->failed_enc_methods |= conn->current_enc_method;
+
+ if (select_next_encryption_method(conn, true))
+ {
+ if (conn->current_enc_method == ENC_DIRECT_SSL)
+ return 2;
+ else
+ return 1;
+ }
+ else
+ return 0;
+}
+#endif
+
+/*
+ * Out-of-line portion of the CONNECTION_FAILED() macro
+ *
+ * Returns true, if we should retry the connection with different encryption method.
+ * conn->current_enc_method is updated to the next method to try.
+ */
+static bool
+connection_failed(PGconn *conn)
+{
+ Assert((conn->failed_enc_methods & conn->current_enc_method) == 0);
+ conn->failed_enc_methods |= conn->current_enc_method;
+
+ /*
+ * If the server reported an error after the SSL handshake, no point in
+ * retrying with negotiated vs direct SSL.
+ */
+ if ((conn->current_enc_method & (ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL)) != 0 &&
+ conn->ssl_handshake_started)
+ {
+ conn->failed_enc_methods |= (ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL) & conn->allowed_enc_methods;
+ }
+ else
+ conn->failed_enc_methods |= conn->current_enc_method;
+
+ return select_next_encryption_method(conn, false);
+}
+
+/*
+ * Choose the next encryption method to try. If this is a retry,
+ * conn->failed_enc_methods has already been updated. conn->current_enc_method
+ * is updated to the next method to try.
+ */
+static bool
+select_next_encryption_method(PGconn *conn, bool have_valid_connection)
+{
+ int remaining_methods;
+
+ remaining_methods = conn->allowed_enc_methods & ~conn->failed_enc_methods;
+
+ /*
+ * Try GSSAPI before SSL
+ */
+#ifdef ENABLE_GSS
+ if ((remaining_methods & ENC_GSSAPI) != 0)
+ {
+ /*
+ * If GSSAPI encryption is enabled, then call pg_GSS_have_cred_cache()
+ * which will return true if we can acquire credentials (and give us a
+ * handle to use in conn->gcred), and then send a packet to the server
+ * asking for GSSAPI Encryption (and skip past SSL negotiation and
+ * regular startup below).
+ */
+ if (!conn->gctx)
+ {
+ if (!pg_GSS_have_cred_cache(&conn->gcred))
+ {
+ conn->allowed_enc_methods &= ~ENC_GSSAPI;
+ remaining_methods &= ~ENC_GSSAPI;
+
+ if (conn->gssencmode[0] == 'r')
+ {
+ libpq_append_conn_error(conn,
+ "GSSAPI encryption required but no credential cache");
+ }
+ }
+ }
+ if ((remaining_methods & ENC_GSSAPI) != 0)
+ {
+ conn->current_enc_method = ENC_GSSAPI;
+ return true;
+ }
+ }
+#endif
+
+ /* With sslmode=allow, try plaintext connection before SSL. */
+ if (conn->sslmode[0] == 'a' && (remaining_methods & ENC_PLAINTEXT) != 0)
+ {
+ conn->current_enc_method = ENC_PLAINTEXT;
+ return true;
+ }
+
+ /*
+ * Try SSL. If enabled, try direct SSL. Unless we have a valid TCP
+ * connection that failed negotiating GSSAPI encryption; in that case we
+ * prefer to reuse the connection with negotiated SSL, instead of
+ * reconnecting to do direct SSL. The point of direct SSL is to avoid the
+ * roundtrip from the negotiation, but reconnecting would also incur a
+ * roundtrip.
+ */
+ if (have_valid_connection && (remaining_methods & ENC_NEGOTIATED_SSL) != 0)
+ {
+ conn->current_enc_method = ENC_NEGOTIATED_SSL;
+ return true;
+ }
+
+ if ((remaining_methods & ENC_DIRECT_SSL) != 0)
+ {
+ conn->current_enc_method = ENC_DIRECT_SSL;
+ return true;
+ }
+
+ if ((remaining_methods & ENC_NEGOTIATED_SSL) != 0)
+ {
+ conn->current_enc_method = ENC_NEGOTIATED_SSL;
+ return true;
+ }
+
+ if (conn->sslmode[0] != 'a' && (remaining_methods & ENC_PLAINTEXT) != 0)
+ {
+ conn->current_enc_method = ENC_PLAINTEXT;
+ return true;
+ }
+
+ /* No more options */
+ conn->current_enc_method = ENC_ERROR;
+ return false;
+}
/*
* internal_ping
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 9aad2ac605c..336325b020d 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1484,6 +1484,7 @@ open_client_SSL(PGconn *conn)
SOCK_ERRNO_SET(0);
ERR_clear_error();
r = SSL_connect(conn->ssl);
+
if (r <= 0)
{
int save_errno = SOCK_ERRNO;
@@ -1587,7 +1588,7 @@ open_client_SSL(PGconn *conn)
/*
* We already checked the server certificate in initialize_SSL() using
- * SSL_CTX_set_verify(), if root.crt exists.
+ * SSL_set_verify(), if root.crt exists.
*/
/* get server certificate */
@@ -1631,6 +1632,7 @@ pgtls_close(PGconn *conn)
SSL_free(conn->ssl);
conn->ssl = NULL;
conn->ssl_in_use = false;
+ conn->ssl_handshake_started = false;
destroy_needed = true;
}
@@ -1654,7 +1656,7 @@ pgtls_close(PGconn *conn)
{
/*
* In the non-SSL case, just remove the crypto callbacks if the
- * connection has then loaded. This code path has no dependency on
+ * connection has loaded them. This code path has no dependency on
* any pending SSL calls.
*/
if (conn->crypto_loaded)
@@ -1860,9 +1862,10 @@ static BIO_METHOD *my_bio_methods;
static int
my_sock_read(BIO *h, char *buf, int size)
{
+ PGconn *conn = (PGconn *) BIO_get_app_data(h);
int res;
- res = pqsecure_raw_read((PGconn *) BIO_get_app_data(h), buf, size);
+ res = pqsecure_raw_read(conn, buf, size);
BIO_clear_retry_flags(h);
if (res < 0)
{
@@ -1884,6 +1887,9 @@ my_sock_read(BIO *h, char *buf, int size)
}
}
+ if (res > 0)
+ conn->ssl_handshake_started = true;
+
return res;
}
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index ef6ce9d0c0b..4fdd2cbee8c 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -72,7 +72,7 @@ typedef enum
CONNECTION_AUTH_OK, /* Received authentication; waiting for
* backend startup. */
CONNECTION_SETENV, /* This state is no longer used. */
- CONNECTION_SSL_STARTUP, /* Negotiating SSL. */
+ CONNECTION_SSL_STARTUP, /* Performing SSL handshake. */
CONNECTION_NEEDED, /* Internal state: connect() needed. */
CONNECTION_CHECK_WRITABLE, /* Checking if session is read-write. */
CONNECTION_CONSUME, /* Consuming any extra messages. */
@@ -82,7 +82,6 @@ typedef enum
CONNECTION_CHECK_STANDBY, /* Checking if server is in standby mode. */
CONNECTION_ALLOCATED, /* Waiting for connection attempt to be
* started. */
- CONNECTION_DIRECT_SSL_STARTUP /* Starting SSL without PG Negotiation. */
} ConnStatusType;
typedef enum
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index def49a68af3..9c5bd898fd4 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -76,6 +76,7 @@ typedef struct
#include <openssl/ssl.h>
#include <openssl/err.h>
+
#ifndef OPENSSL_NO_ENGINE
#define USE_SSL_ENGINE
#endif
@@ -231,6 +232,12 @@ typedef enum
PGASYNC_PIPELINE_IDLE, /* "Idle" between commands in pipeline mode */
} PGAsyncStatusType;
+#define ENC_ERROR 0
+#define ENC_DIRECT_SSL 0x01
+#define ENC_GSSAPI 0x02
+#define ENC_NEGOTIATED_SSL 0x04
+#define ENC_PLAINTEXT 0x08
+
/* Target server type (decoded value of target_session_attrs) */
typedef enum
{
@@ -550,17 +557,17 @@ struct pg_conn
void *sasl_state;
int scram_sha_256_iterations;
+ uint8 allowed_enc_methods;
+ uint8 failed_enc_methods;
+ uint8 current_enc_method;
+
/* SSL structures */
bool ssl_in_use;
+ bool ssl_handshake_started;
bool ssl_cert_requested; /* Did the server ask us for a cert? */
bool ssl_cert_sent; /* Did we send one in reply? */
#ifdef USE_SSL
- bool allow_direct_ssl_try; /* Try to make a direct SSL connection
- * without an "SSL negotiation packet" */
- bool allow_ssl_try; /* Allowed to try SSL negotiation */
- bool wait_ssl_try; /* Delay SSL negotiation until after
- * attempting normal connection */
#ifdef USE_OPENSSL
SSL *ssl; /* SSL status, if have SSL connection */
X509 *peer; /* X509 cert of server */
@@ -583,7 +590,6 @@ struct pg_conn
gss_name_t gtarg_nam; /* GSS target name */
/* The following are encryption-only */
- bool try_gss; /* GSS attempting permitted */
bool gssenc; /* GSS encryption is usable */
gss_cred_id_t gcred; /* GSS credential temp storage. */
--
2.39.2
