From: Robbie Harwood <rharwood@redhat.com>
To: Stephen Frost <sfrost@snowman.net>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org, Nico Williams <nico@cryptonector.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: David Steele <david@pgmasters.net>
Subject: Re: [PATCH v20] GSSAPI encryption support
Date: Tue, 05 Mar 2019 22:55:23 -0500
Message-ID: <jlg4l8gpsgk.fsf@redhat.com> (raw)
In-Reply-To: <20190223162751.GO6197@tamriel.snowman.net>
References: <jlg1tgq1ktm.fsf@thriss.redhat.com>
<jlgy38mir81.fsf@redhat.com>
<5ec3f734-810e-36e0-030f-af92efe135da@2ndquadrant.com>
<20190223162751.GO6197@tamriel.snowman.net>
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
Attachments:
signature.asc (application/pgp-signature, 832B) downloadAdd-tests-for-GSSAPI-krb5-encryption.patch (text/x-diff, 5.6K) download
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.plnew file mode 100644index 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: rharwood@redhat.com, sfrost@snowman.net, peter.eisentraut@2ndquadrant.com, nico@cryptonector.com, michael@paquier.xyz, alvherre@2ndquadrant.com, david@pgmasters.net
Subject: Re: [PATCH v20] GSSAPI encryption support
In-Reply-To: <jlg4l8gpsgk.fsf@redhat.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox