Re: [PATCH v20] GSSAPI encryption support

Stephen Frost <sfrost@snowman.net> writes:

> * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
>
>> Or maybe we avoid that, and you rename be-secure-gssapi.c to just
>> be-gssapi.c and also combine that with the contents of
>> be-gssapi-common.c.
>
> I don't know why we would need to, or want to, combine
> be-secure-gssapi.c and be-gssapi-common.c, they do have different
> roles in that be-gssapi-common.c is used even if you aren't doing
> encryption, while bs-secure-gssapi.c is specifically for handling the
> encryption side of things.
>
> Then again, be-gssapi-common.c does currently only have the one
> function in it, and it's not like there's an option to compile for
> *just* GSS authentication (and not encryption), or at least, I don't
> think that would be a useful option to have..  Robbie?

Yeah, I think I'm opposed to making that an option.

Worth pointing out here: up until v6, I had this structured differently,
with all the GSSAPI code in fe-gssapi.c and be-gssapi.c.  The current
separation was suggested by Michael Paquier for ease of reading and to
keep the code churn down.

>> (And similarly in libpq.)
>
> I do agree that we should try to keep the frontend and backend code
> structures similar in this area, that seems to make sense.

Well, I don't know if it's an argument in either direction, but: on the
frontend we have about twice as much shared code in fe-gssapi-common.c
(pg_GSS_have_ccache() and pg_GSS_load_servicename()).

>> I don't see any tests in the patch.  We have a Kerberos test suite at
>> src/test/kerberos/ and an SSL test suite at src/test/ssl/.  You can
>> get some ideas there.
>
> Yeah, I was going to comment on that as well.  We definitely should
> implement tests around this.

Attached.  Please note that I don't really speak perl.  There's a pile
of duplicated code in 002_enc.pl that probably should be shared between
the two.  (It would also I think be possible for 001_auth.pl to set up
the KDC and for 002_enc.pl to then use it.)

Thanks,
--Robbie
From 42ab1ccae8e517934866ee923d80554ef1996709 Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Tue, 5 Mar 2019 22:54:11 -0500
Subject: [PATCH] Add tests for GSSAPI/krb5 encryption

---
 src/test/kerberos/t/002_enc.pl | 197 +++++++++++++++++++++++++++++++++
 1 file changed, 197 insertions(+)
 create mode 100644 src/test/kerberos/t/002_enc.pl

diff --git a/src/test/kerberos/t/002_enc.pl b/src/test/kerberos/t/002_enc.pl
new file mode 100644
index 0000000000..927abe15e4
--- /dev/null
+++ b/src/test/kerberos/t/002_enc.pl
@@ -0,0 +1,197 @@
+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, $gssmode, $expected_res, $test_name) = @_;
+
+    my $res = $node->psql(
+        "postgres",
+        "SELECT 1",
+        extra_params => [
+            "-d",
+            $node->connstr("postgres") . " host=$host hostaddr=$hostaddr gssmode=$gssmode",
+            "-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{hostgss 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{hostgss 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{hostnogss 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.20.1