Stephen Frost <sfr...@snowman.net> writes: > * Peter Eisentraut (peter.eisentr...@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
signature.asc
Description: PGP signature
>From 42ab1ccae8e517934866ee923d80554ef1996709 Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharw...@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
