I hope I didn't joggle your elbow reviewing this, Jacob, but I spent
some time rebase and fix various little things:
- Incorporated Matthias's test changes
- Squashed the client, server and documentation patches. Not much point
in keeping them separate, as one requires the other, and if you're only
interested e.g. in the server parts, just look at src/backend.
- Squashed some of my refactorings with the main patches, because I'm
certain enough that they're desirable. I kept the last libpq state
machine refactoring separate though. I'm pretty sure we need a
refactoring like that, but I'm not 100% sure about the details.
- Added some comments to the new state machine logic in fe-connect.c.
- Removed the XXX comments about TLS alerts.
- Removed the "Allow pipelining data after ssl request" patch
- Reordered the patches so that the first two patches add the tests
different combinations of sslmode, gssencmode and server support. That
could be committed separately, without the rest of the patches. A later
patch expands the tests for the new sslnegotiation option.
The tests are still not distinguishing whether a connection was
established in direct or negotiated mode. So if we e.g. had a bug that
accidentally disabled direct SSL connection completely and always used
negotiated mode, the tests would still pass. I'd like to see some tests
that would catch that.
--
Heikki Linnakangas
Neon (https://neon.tech)
From c3b88ffb05a2a8b50e1af3220bf8f524e8bbae46 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 v8 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 2a81ce8834b..9d3fac83aaa 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;
-# 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 90140f1093281691bdba185d96f3b07e27d4a50e 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 v8 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 060188083d483d725939d77efb51ccd7d16e7ba2 Mon Sep 17 00:00:00 2001
From: Greg Stark <st...@mit.edu>
Date: Wed, 15 Mar 2023 15:51:02 -0400
Subject: [PATCH v8 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/postmaster/postmaster.c | 84 +++++++++++++++++++++--
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, 375 insertions(+), 30 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 1d8998efb2a..88bcfe44d8e 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1691,10 +1691,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>
@@ -1721,6 +1724,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>
@@ -1954,11 +2026,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>
@@ -8149,6 +8223,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 c606bf34473..caa502b6373 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -1134,15 +1134,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/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index f3c09e8dc0d..ab82faf6aa0 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -423,6 +423,7 @@ static void BackendRun(Port *port) pg_attribute_noreturn();
static void ExitPostmaster(int status) pg_attribute_noreturn();
static int ServerLoop(void);
static int BackendStartup(Port *port);
+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 processCancelRequest(Port *port, void *pkt);
@@ -1922,6 +1923,69 @@ ServerLoop(void)
}
}
+/*
+ * 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.
*
@@ -2035,8 +2099,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 */
@@ -2044,11 +2113,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")));
@@ -2089,7 +2157,7 @@ retry1:
GSSok = 'G';
#endif
- while (send(port->sock, &GSSok, 1, 0) != 1)
+ while (secure_write(port, &GSSok, 1) != 1)
{
if (errno == EINTR)
continue;
@@ -4358,7 +4426,9 @@ BackendInitialize(Port *port)
* 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
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 47d66d55241..1f7ce041359 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -227,6 +227,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;
#ifdef USE_SSL
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 6171a0d17a5..79c5f6b754b 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -80,7 +80,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 d4e10a0c4f3..7bd371fafb5 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)},
@@ -1517,10 +1522,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;
}
@@ -1643,6 +1675,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
@@ -2730,11 +2777,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;
}
@@ -3192,6 +3240,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
@@ -3268,9 +3339,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
@@ -3376,6 +3449,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
@@ -4371,6 +4459,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 defc415fa3f..8dd7a9af457 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -78,7 +78,8 @@ typedef enum
CONNECTION_CONSUME, /* Consuming any extra messages. */
CONNECTION_GSS_STARTUP, /* Negotiating GSSAPI. */
CONNECTION_CHECK_TARGET, /* Checking target server properties. */
- CONNECTION_CHECK_STANDBY /* Checking if server is in standby mode. */
+ CONNECTION_CHECK_STANDBY, /* Checking if server is in standby mode. */
+ 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 82c18f870d2..640a3ae0f5a 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 */
@@ -549,6 +551,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 3a6f22bdd0b231cc645b67cccd7511d6a38095c4 Mon Sep 17 00:00:00 2001
From: Greg Stark <st...@mit.edu>
Date: Mon, 20 Mar 2023 14:09:30 -0400
Subject: [PATCH v8 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/postmaster/postmaster.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 e12b1cc9e3b..afde13c2fad 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/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index ab82faf6aa0..cc6d7b477e2 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1958,6 +1958,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)
{
@@ -1972,6 +1977,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 45013582a74..b7f7b2837aa 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 edcc0282b2d..880cd9438cb 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 5c906e48068..07f13f9c04e 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 1f7ce041359..53c08370a4c 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -218,6 +218,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 79c5f6b754b..b65b32f2093 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -123,6 +123,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 7bd371fafb5..4bbed0451ed 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)},
@@ -4473,6 +4477,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 81108822620..a8fd4c19efb 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
@@ -1736,6 +1755,7 @@ PQsslAttributeNames(PGconn *conn)
"cipher",
"compression",
"protocol",
+ "alpn",
NULL
};
static const char *const empty_attrs[] = {NULL};
@@ -1790,6 +1810,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 640a3ae0f5a..97d78f02b92 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 c3e21363f0d15adbbd8f6a90fba0e38c36c60188 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 v8 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 0718d379ca12459469d503340bf35b1acf0cdbc2 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 v8 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 4bbed0451ed..5b42384d897 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);
@@ -1679,21 +1685,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
@@ -2777,16 +2768,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;
}
@@ -2812,6 +2796,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)
{
@@ -3126,18 +3138,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.
@@ -3186,30 +3186,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);
@@ -3224,12 +3225,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
@@ -3245,39 +3240,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;
@@ -3302,8 +3280,11 @@ keep_going: /* We will come back to here until there is
#endif /* USE_SSL */
/*
- * 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)
@@ -3344,10 +3325,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
@@ -3377,34 +3357,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')
{
@@ -3429,6 +3389,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.
*/
@@ -3457,32 +3425,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;
@@ -3501,7 +3444,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);
@@ -3525,9 +3468,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 */
@@ -3535,17 +3476,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')
{
@@ -3577,18 +3509,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;
@@ -3764,55 +3685,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)
{
@@ -4252,6 +4125,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 a8fd4c19efb..9bfab813bea 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)
@@ -1843,9 +1845,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)
{
@@ -1867,6 +1870,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 8dd7a9af457..595973fd8de 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -72,14 +72,13 @@ 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. */
CONNECTION_GSS_STARTUP, /* Negotiating GSSAPI. */
CONNECTION_CHECK_TARGET, /* Checking target server properties. */
CONNECTION_CHECK_STANDBY, /* Checking if server is in standby mode. */
- 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 97d78f02b92..d3105e6c8d0 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
{
@@ -546,17 +553,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 */
@@ -579,7 +586,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
