Greetings,

* Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
> On 2019-04-09 09:32, Peter Eisentraut wrote:
> > On 2019-04-09 04:51, Stephen Frost wrote:
> >>> Running just 002_enc.pl by itself passes the tests!
> >> Great!  I think what I'll do is work to incorporate the two tests back
> >> into one script, to avoid whatever the race condition or other confusion
> >> is happening on macOS here.
> > 
> > That seems reasonable.  It also avoids the large amount of duplicate
> > setup code.
> 
> Another problem is that the two test files cannot be run in parallel
> because they use the same hardcoded data directories.  That would have
> to be replaced by temporary directories.

The tests are really fast enough with one KDC that I don't think it
makes sense to have two independent tests.

> The race condition alluded to above appears to be simply that at the end
> of the test the kdc is shut down by kill -INT but nothing waits for that
> to finish before a new one is started by the next test.  So there is
> potential for all kinds of confusion.

Ah, good to know that's what was happening.

> Putting it all in one test file seems to be the easiest way out.

Please find attached a patch which updates the protocol.sgml docs that
Michael mentioned before, and merges the tests into one test file (while
adding in some additional tests to make sure that the server also agrees
with what our expectations are, using the pg_stat_gssapi view).

I'll push this soon unless there are concerns.  If you get a chance to
test the patch out, that would be great.  It's working happily for me
locally.

Thanks!

Stephen
From e0745810ba1582601f2c71db3b67a8cfc2480546 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfr...@snowman.net>
Date: Sun, 14 Apr 2019 21:14:42 -0400
Subject: [PATCH] GSSAPI: Improve documentation and tests

The GSSAPI encryption patch neglected to update the protocol
documentation to describe how to set up a GSSAPI encrypted connection
from a client to the server, so fix that by adding the appropriate
documentation to protocol.sgml.

The tests added for encryption support were overly long and couldn't be
run in parallel due to race conditions; this was largely because each
test was setting up its own KDC to perform the tests.  Instead, merge
the authentication tests and the encryption tests into one test file,
where we only create one KDC to run the tests with.  Also, have the
tests check what the server's opinion is of the connection and if it was
GSS authenticated or encrypted using the previously added pg_stat_gssapi
view.

In passing, fix the libpq label for GSSENC-Mode to be consistent with
the "PGGSSENCMODE" environment variable.

Missing protocol documentation pointed out by Michael Paquier.
Issues with the tests pointed out by Tom Lane and Peter Eisentraut.

Refactored tests and added documentation by me.
---
 doc/src/sgml/protocol.sgml        | 103 ++++++++++++++++
 src/interfaces/libpq/fe-connect.c |   2 +-
 src/test/kerberos/t/001_auth.pl   |  48 ++++++--
 src/test/kerberos/t/002_enc.pl    | 197 ------------------------------
 4 files changed, 141 insertions(+), 209 deletions(-)
 delete mode 100644 src/test/kerberos/t/002_enc.pl

diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 09893d96b5..204dca7c33 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1479,6 +1479,72 @@ SELCT 1/0;
     of authentication checking.
    </para>
   </sect2>
+
+  <sect2>
+   <title><acronym>GSSAPI</acronym> Session Encryption</title>
+
+   <para>
+    If <productname>PostgreSQL</productname> was built with
+    <acronym>GSSAPI</acronym> support, frontend/backend communications
+    can be encrypted using <acronym>GSSAPI</acronym>.  This provides
+    communication security in environments where attackers might be
+    able to capture the session traffic. For more information on
+    encrypting <productname>PostgreSQL</productname> sessions with
+    <acronym>GSSAPI</acronym>, see <xref linkend="gssapi-enc"/>.
+   </para>
+
+   <para>
+    To initiate a <acronym>GSSAPI</acronym>-encrypted connection, the
+    frontend initially sends a GSSENCRequest message rather than a
+    StartupMessage.  The server then responds with a single byte
+    containing <literal>G</literal> or <literal>N</literal>, indicating that it
+    is willing or unwilling to perform <acronym>GSSAPI</acronym> encryption,
+    respectively.  The frontend might close the connection at this point
+    if it is dissatisfied with the response.  To continue after
+    <literal>G</literal>, perform a <acronym>GSSAPI</acronym> initialization by
+    calling gss_init_sec_context() in a loop and sending the result to the
+    server, starting with an empty input and then with each result from the
+    server, until it returns no output.  When sending the results of
+    gss_init_sec_context() to the server, prepend the length of the message as a
+    four byte integer in network byte order.  If this is successful, then use
+    gss_wrap() to encrypt the usual StartupMessage and all subsequent data,
+    prepending the length of the result from gss_wrap() as a four byte integer
+    in network byte order to the actual encrypted payload.  Note that the server
+    will only accept encrypted packets from the client which are less than 16KB;
+    gss_wrap_size_limit() should be used by the client to determine the size of
+    the unencrypted message which will fit within this limit and larger messages
+    should be broken up into multiple gss_wrap() calls.  Typical segments are
+    8KB of unencrypted data, resulting in encrypted packets of slightly larger
+    than 8KB but well within the 16KB maximum.  The server can be expected to
+    not send encrypted packets of larger than 16KB to the client.  To continue
+    after <literal>N</literal>, send the usual StartupMessage and proceed
+    without encryption.
+   </para>
+
+   <para>
+    The frontend should also be prepared to handle an ErrorMessage
+    response to GSSENCRequest from the server.  This would only occur if
+    the server predates the addition of <acronym>GSSAPI</acronym> encryption
+    support to <productname>PostgreSQL</productname>.  In this case the
+    connection must be closed, but the frontend might choose to open a fresh
+    connection and proceed without requesting <acronym>GSSAPI</acronym>
+    encryption.  Given the length limits specified above, the ErrorMessage can
+    not be confused with a proper response from the server with an appropriate
+    length.
+   </para>
+
+   <para>
+    An initial GSSENCRequest can also be used in a connection that is being
+    opened to send a CancelRequest message.
+   </para>
+
+   <para>
+    While the protocol itself does not provide a way for the server to
+    force <acronym>GSSAPI</acronym> encryption, the administrator can
+    configure the server to reject unencrypted sessions as a byproduct
+    of authentication checking.
+   </para>
+  </sect2>
  </sect1>
 
 <sect1 id="sasl-authentication">
@@ -5714,6 +5780,43 @@ SSLRequest (F)
 </listitem>
 </varlistentry>
 
+<varlistentry>
+<term>
+GSSENCRequest (F)
+</term>
+<listitem>
+<para>
+
+<variablelist>
+<varlistentry>
+<term>
+        Int32(8)
+</term>
+<listitem>
+<para>
+                Length of message contents in bytes, including self.
+</para>
+</listitem>
+</varlistentry>
+<varlistentry>
+<term>
+        Int32(80877104)
+</term>
+<listitem>
+<para>
+                The <acronym>GSSAPI</acronym> Encryption request code.  The value is chosen to contain
+                <literal>1234</literal> in the most significant 16 bits, and <literal>5680</literal> in the
+                least significant 16 bits.  (To avoid confusion, this code
+                must not be the same as any protocol version number.)
+</para>
+</listitem>
+</varlistentry>
+</variablelist>
+
+</para>
+</listitem>
+</varlistentry>
+
 
 <varlistentry>
 <term>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 87df79880a..b89df08c29 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -313,7 +313,7 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 	 * and "prefer".
 	 */
 	{"gssencmode", "PGGSSENCMODE", DefaultGSSMode, NULL,
-		"GSS-Mode", "", 7,		/* sizeof("disable") == 7 */
+		"GSSENC-Mode", "", 7,		/* sizeof("disable") == 7 */
 	offsetof(struct pg_conn, gssencmode)},
 
 #if defined(ENABLE_GSS) || defined(ENABLE_SSPI)
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index 1be89aef4f..b702a79343 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -6,7 +6,7 @@ use Test::More;
 
 if ($ENV{with_gssapi} eq 'yes')
 {
-	plan tests => 4;
+	plan tests => 12;
 }
 else
 {
@@ -155,18 +155,24 @@ note "running tests";
 
 sub test_access
 {
-	my ($node, $role, $expected_res, $test_name) = @_;
+	my ($node, $role, $server_check, $expected_res, $gssencmode, $test_name) = @_;
 
 	# need to connect over TCP/IP for Kerberos
-	my $res = $node->psql(
+	my ($res, $stdoutres, $stderrres) = $node->psql(
 		'postgres',
-		'SELECT 1',
+		"$server_check",
 		extra_params => [
-			'-d',
-			$node->connstr('postgres') . " host=$host hostaddr=$hostaddr",
+			'-XAtd',
+			$node->connstr('postgres') . " host=$host hostaddr=$hostaddr $gssencmode",
 			'-U', $role
 		]);
-	is($res, $expected_res, $test_name);
+
+	# If we get a query result back, it should be true.
+	if ($res == $expected_res and $res eq 0) {
+		is($stdoutres, "t", $test_name);
+	} else {
+		is($res, $expected_res, $test_name);
+	}
 	return;
 }
 
@@ -175,16 +181,36 @@ $node->append_conf('pg_hba.conf',
 	qq{host all all $hostaddr/32 gss map=mymap});
 $node->restart;
 
-test_access($node, 'test1', 2, 'fails without ticket');
+test_access($node, 'test1', 'SELECT true', 2, '', 'fails without ticket');
 
 run_log [ $kinit, 'test1' ], \$test1_password or BAIL_OUT($?);
 
-test_access($node, 'test1', 2, 'fails without mapping');
+test_access($node, 'test1', 'SELECT true', 2, '', 'fails without mapping');
 
 $node->append_conf('pg_ident.conf', qq{mymap  /^(.*)\@$realm\$  \\1});
 $node->restart;
 
-test_access($node, 'test1', 0, 'succeeds with mapping');
+test_access($node, 'test1', 'SELECT gss_authenticated AND encrypted from pg_stat_gssapi where pid = pg_backend_pid();', 0, '', 'succeeds with mapping with default gssencmode and host hba');
+test_access($node, "test1", 'SELECT gss_authenticated AND encrypted from pg_stat_gssapi where pid = pg_backend_pid();', 0, "gssencmode=prefer", "succeeds with GSS-encrypted access preferred with host hba");
+test_access($node, "test1", 'SELECT gss_authenticated AND encrypted from pg_stat_gssapi where pid = pg_backend_pid();', 0, "gssencmode=require", "succeeds with GSS-encrypted access required with host hba");
+
+unlink($node->data_dir . '/pg_hba.conf');
+$node->append_conf('pg_hba.conf',
+	qq{hostgssenc all all $hostaddr/32 gss map=mymap});
+$node->restart;
+
+test_access($node, "test1", 'SELECT gss_authenticated AND encrypted from pg_stat_gssapi where pid = pg_backend_pid();', 0, "gssencmode=prefer", "succeeds with GSS-encrypted access preferred and hostgssenc hba");
+test_access($node, "test1", 'SELECT gss_authenticated AND encrypted from pg_stat_gssapi where pid = pg_backend_pid();', 0, "gssencmode=require", "succeeds with GSS-encrypted access required and hostgssenc hba");
+test_access($node, "test1", 'SELECT true', 2, "gssencmode=disable", "fails with GSS encryption disabled and hostgssenc hba");
+
+unlink($node->data_dir . '/pg_hba.conf');
+$node->append_conf('pg_hba.conf',
+	qq{hostnogssenc all all $hostaddr/32 gss map=mymap});
+$node->restart;
+
+test_access($node, "test1", 'SELECT gss_authenticated and not encrypted from pg_stat_gssapi where pid = pg_backend_pid();', 0, "gssencmode=prefer", "succeeds with GSS-encrypted access preferred and hostnogssenc hba, but no encryption");
+test_access($node, "test1", 'SELECT true', 2, "gssencmode=require", "fails with GSS-encrypted access required and hostnogssenc hba");
+test_access($node, "test1", 'SELECT gss_authenticated and not encrypted from pg_stat_gssapi where pid = pg_backend_pid();', 0, "gssencmode=disable", "succeeds with GSS encryption disabled and hostnogssenc hba");
 
 truncate($node->data_dir . '/pg_ident.conf', 0);
 unlink($node->data_dir . '/pg_hba.conf');
@@ -192,4 +218,4 @@ $node->append_conf('pg_hba.conf',
 	qq{host all all $hostaddr/32 gss include_realm=0});
 $node->restart;
 
-test_access($node, 'test1', 0, 'succeeds with include_realm=0');
+test_access($node, 'test1', 'SELECT gss_authenticated AND encrypted from pg_stat_gssapi where pid = pg_backend_pid();', 0, '', 'succeeds with include_realm=0 and defaults');
diff --git a/src/test/kerberos/t/002_enc.pl b/src/test/kerberos/t/002_enc.pl
deleted file mode 100644
index 1a7dc30a81..0000000000
--- a/src/test/kerberos/t/002_enc.pl
+++ /dev/null
@@ -1,197 +0,0 @@
-use strict;
-use warnings;
-use TestLib;
-use PostgresNode;
-use Test::More;
-use File::Path 'remove_tree';
-
-if ($ENV{with_gssapi} eq 'yes')
-{
-	plan tests => 5;
-}
-else
-{
-	plan skip_all => 'GSSAPI/Kerberos not supported by this build';
-}
-
-my ($krb5_bin_dir, $krb5_sbin_dir);
-
-if ($^O eq 'darwin')
-{
-	$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 $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;
-}
-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   = "${TestLib::tmp_check}/krb5.conf";
-my $kdc_conf    = "${TestLib::tmp_check}/kdc.conf";
-my $krb5_log    = "${TestLib::tmp_check}/krb5libs.log";
-my $kdc_log     = "${TestLib::tmp_check}/krb5kdc.log";
-my $kdc_port    = int(rand() * 16384) + 49152;
-my $kdc_datadir = "${TestLib::tmp_check}/krb5kdc";
-my $kdc_pidfile = "${TestLib::tmp_check}/krb5kdc.pid";
-my $keytab      = "${TestLib::tmp_check}/krb5.keytab";
-
-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;
-
-append_to_file(
-	$krb5_conf,
-	qq![logging]
-default = FILE:$krb5_log
-kdc = FILE:$kdc_log
-
-[libdefaults]
-default_realm = $realm
-
-[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
-}!);
-
-remove_tree $kdc_datadir;
-mkdir $kdc_datadir or die;
-
-$ENV{'KRB5_CONFIG'}      = $krb5_conf;
-$ENV{'KRB5_KDC_PROFILE'} = $kdc_conf;
-
-my $service_principal = "$ENV{with_krb_srvnam}/$host";
-
-system_or_bail $kdb5_util, 'create', '-s', '-P', 'secret0';
-
-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 -f $kdc_pidfile;
-}
-
-note "setting up PostgreSQL instance";
-
-my $node = get_new_node('node');
-$node->init;
-$node->append_conf('postgresql.conf', "listen_addresses = 'localhost'");
-$node->append_conf('postgresql.conf', "krb_server_keyfile = '$keytab'");
-$node->start;
-
-$node->safe_psql('postgres', 'CREATE USER test1;');
-
-note "running tests";
-
-sub test_access
-{
-	my ($node, $gssencmode, $expected_res, $test_name) = @_;
-
-	my $res = $node->psql(
-		"postgres",
-		"SELECT 1",
-		extra_params => [
-			"-d",
-			$node->connstr("postgres") . " host=$host hostaddr=$hostaddr gssencmode=$gssencmode",
-			"-U", "test1",
-		]);
-	is($res, $expected_res, $test_name);
-	return;
-}
-
-unlink($node->data_dir . "/pg_ident.conf");
-$node->append_conf("pg_ident.conf", 'mymap /^(.*)@EXAMPLE.COM$ \1');
-run_log [ $kinit, 'test1' ], \$test1_password or BAIL_OUT($?);
-
-unlink($node->data_dir . '/pg_hba.conf');
-$node->append_conf('pg_hba.conf',
-				   qq{hostgssenc all all $hostaddr/32 gss map=mymap});
-$node->restart;
-test_access($node, "require", 0, "GSS-encrypted access");
-test_access($node, "disable", 2, "GSS encryption disabled");
-
-unlink($node->data_dir . "/pg_hba.conf");
-$node->append_conf("pg_hba.conf", qq{hostgssenc all all $hostaddr/32 trust});
-$node->restart;
-test_access($node, "require", 0, "GSS encryption without auth");
-
-unlink($node->data_dir . "/pg_hba.conf");
-$node->append_conf("pg_hba.conf",
-				   qq{hostnogssenc all all localhost gss map=mymap});
-$node->restart;
-test_access($node, "prefer", 0, "GSS unencrypted fallback");
-
-# Check that the client can prevent fallback.
-test_access($node, "require", 2, "GSS unencrypted fallback prevention");
-- 
2.19.1

Attachment: signature.asc
Description: PGP signature

Reply via email to