Обсуждение: Kerberos test suite
Here is a patch with a test suite for the Kerberos/GSSAPI authentication functionality. It's very similar in principle to the recently added LDAP tests, and similar caveats apply. You will need the client and server parts of a krb5 package installation, possibly named krb5-workstation and krb5-server, or perhaps krb5-user and krb5-kdc. (If it appears to hang for you in the "setting up Kerberos" step, you might need more entropy/wait a while. That problem appears to be limited to some virtual machine setups, but the specifics are not clear.) -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
On Wed, Feb 14, 2018 at 09:27:04AM -0500, Peter Eisentraut wrote:
> Here is a patch with a test suite for the Kerberos/GSSAPI authentication
> functionality. It's very similar in principle to the recently added
> LDAP tests, and similar caveats apply.
>
> You will need the client and server parts of a krb5 package
> installation, possibly named krb5-workstation and krb5-server, or
> perhaps krb5-user and krb5-kdc.
Thanks. Could you document that on the README please? krb5-user and
krb5-kdc is a split from Debian. For darwin, are you using macports or
homebrew? I would assume the later, and it would be nice to precise
that in the README as well. On Debian you need to install as well
krb5-admin-server as it includes kadmin.local which the test needs.
Once I understood that I have been able to run the tests.
> (If it appears to hang for you in the "setting up Kerberos" step, you
> might need more entropy/wait a while. That problem appears to be
> limited to some virtual machine setups, but the specifics are not
> clear.)
That's one of those "move your mouse" or "type randomly your keyboard"
to generate more entropy for the installation setup?
You have forgotten to update ALWAYS_SUBDIRS in src/test/Makefile.
+my ($stdout, $krb5_version);
+IPC::Run::run [ 'krb5-config', '--version' ], '>', \$stdout or die
"could not execute krb5-config";
+$stdout =~ m/Kerberos 5 release ([0-9]+\.[0-9]+)/ or die "could not get
Kerberos version";
+$krb5_version = $1;
Time for a new routine command_log which executes the command, then
returns stdout and stderr to the caller?
+system_or_bail 'echo secret1 | kinit test1';
Using IPC::Run stuff would be better here.
@@ -1153,6 +1152,11 @@ sub psql
$params{on_error_stop} = 1 unless defined $params{on_error_stop};
$params{on_error_die} = 0 unless defined $params{on_error_die};
+ $connstr .= ' host=localhost' if defined $params{tcpip};
+
+ my @psql_params =
+ ('psql', '-XAtq', '-d', $connstr, '-f', '-');
This bit I don't like. Wouldn't it be enough to abuse of extra_params
and use a custom connection string? The last value wins in a psql
command.
--
Michael
Вложения
On Thu, Feb 15, 2018 at 3:27 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> Here is a patch with a test suite for the Kerberos/GSSAPI authentication
> functionality. It's very similar in principle to the recently added
> LDAP tests, and similar caveats apply.
+not run by default. You might need to adjust some paths in the test
+file to have it find MIT Kerberos in a place that hadn't been thought
+of yet.
FWIW it passes for me if I add this:
+elsif ($^O eq 'freebsd')
+{
+ $krb5_bin_dir = '/usr/local/bin';
+ $krb5_sbin_dir = '/usr/local/sbin';
+}
One thing that surprised me is that your test runs my system's
installed psql command out of my $PATH, not the one under test. Is
that expected?
--
Thomas Munro
http://www.enterprisedb.com
On 2/27/18 00:56, Thomas Munro wrote:
> FWIW it passes for me if I add this:
>
> +elsif ($^O eq 'freebsd')
> +{
> + $krb5_bin_dir = '/usr/local/bin';
> + $krb5_sbin_dir = '/usr/local/sbin';
I suppose you only need the second one, right? The first one should be
in the path.
> +}
>
> One thing that surprised me is that your test runs my system's
> installed psql command out of my $PATH, not the one under test. Is
> that expected?
Oh, you probably have a psql in /usr/local/bin, which we prepend to the
path, per the above. We should probably append instead. The ldap test
suite has the same issue.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Mar 1, 2018 at 4:43 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> On 2/27/18 00:56, Thomas Munro wrote:
>> FWIW it passes for me if I add this:
>>
>> +elsif ($^O eq 'freebsd')
>> +{
>> + $krb5_bin_dir = '/usr/local/bin';
>> + $krb5_sbin_dir = '/usr/local/sbin';
>
> I suppose you only need the second one, right? The first one should be
> in the path.
Erm. Turned out I had a couple of different versions in my path, but
your test only works with the binaries in the paths shown above (the
binaries installed from ports, or rather pkg install krb5, not the
ones from the base system).
munro@asterix $ /usr/bin/krb5-config --version
FreeBSD heimdal 1.1.0
munro@asterix $ /usr/local/bin/krb5-config --version
Kerberos 5 release 1.15.2
>> One thing that surprised me is that your test runs my system's
>> installed psql command out of my $PATH, not the one under test. Is
>> that expected?
>
> Oh, you probably have a psql in /usr/local/bin, which we prepend to the
> path, per the above. We should probably append instead. The ldap test
> suite has the same issue.
Hmm. Not sure if appending is right either, since then you wouldn't
get the krb5_bin_dir etc in preference to the user's preexisting PATH,
so you'd break my attempt to use krb5-config 1.15.2 instead of heimdal
1.1.0. Maybe you need to prepend the user's krb5_bin_dir etc, and
then finally prepend the tmp install bin dir wherever that happens?
Or maybe you want to build absolute paths from the paths you have? I
didn't look at how this perl code is structured...
--
Thomas Munro
http://www.enterprisedb.com
Michael Paquier <michael@paquier.xyz> writes: > On Wed, Feb 14, 2018 at 09:27:04AM -0500, Peter Eisentraut wrote: > >> (If it appears to hang for you in the "setting up Kerberos" step, you >> might need more entropy/wait a while. That problem appears to be >> limited to some virtual machine setups, but the specifics are not >> clear.) > > That's one of those "move your mouse" or "type randomly your keyboard" > to generate more entropy for the installation setup? Right. Whether this message can show up will depend on how the krb5 was built (and how new it is). krb5 > 1.15 has support for getrandom(), so on most Linux distros, if the machine has successfully ever created 256 bits of entropy, it won't block. On other platforms, use of /dev/urandom requires a build flag. Thanks, --Robbie
Вложения
Thomas Munro <thomas.munro@enterprisedb.com> writes:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
>> On 2/27/18 00:56, Thomas Munro wrote:
>>> FWIW it passes for me if I add this:
>>>
>>> +elsif ($^O eq 'freebsd')
>>> +{
>>> + $krb5_bin_dir = '/usr/local/bin';
>>> + $krb5_sbin_dir = '/usr/local/sbin';
>>
>> I suppose you only need the second one, right? The first one should be
>> in the path.
>
> Erm. Turned out I had a couple of different versions in my path, but
> your test only works with the binaries in the paths shown above (the
> binaries installed from ports, or rather pkg install krb5, not the
> ones from the base system).
>
> munro@asterix $ /usr/bin/krb5-config --version
> FreeBSD heimdal 1.1.0
This one's Heimdal; the test is intended to only work with MIT. Perhaps
it should check the output of krb5-config instead of failing
confusingly?
> munro@asterix $ /usr/local/bin/krb5-config --version
> Kerberos 5 release 1.15.2
Thanks,
--Robbie
Вложения
On 2/27/18 00:21, Michael Paquier wrote:
> Thanks. Could you document that on the README please? krb5-user and
> krb5-kdc is a split from Debian. For darwin, are you using macports or
> homebrew? I would assume the later, and it would be nice to precise
> that in the README as well. On Debian you need to install as well
> krb5-admin-server as it includes kadmin.local which the test needs.
> Once I understood that I have been able to run the tests.
Added to README.
> You have forgotten to update ALWAYS_SUBDIRS in src/test/Makefile.
Fixed, and updated to reflect recent changes with PG_TEST_EXTRA etc.
> +my ($stdout, $krb5_version);
> +IPC::Run::run [ 'krb5-config', '--version' ], '>', \$stdout or die
> "could not execute krb5-config";
> +$stdout =~ m/Kerberos 5 release ([0-9]+\.[0-9]+)/ or die "could not get
> Kerberos version";
> +$krb5_version = $1;
> Time for a new routine command_log which executes the command, then
> returns stdout and stderr to the caller?
I didn't do anything about this. Do we have any more places where that
would be needed right now?
> +system_or_bail 'echo secret1 | kinit test1';
> Using IPC::Run stuff would be better here.
done
> @@ -1153,6 +1152,11 @@ sub psql
> $params{on_error_stop} = 1 unless defined $params{on_error_stop};
> $params{on_error_die} = 0 unless defined $params{on_error_die};
>
> + $connstr .= ' host=localhost' if defined $params{tcpip};
> +
> + my @psql_params =
> + ('psql', '-XAtq', '-d', $connstr, '-f', '-');
> This bit I don't like. Wouldn't it be enough to abuse of extra_params
> and use a custom connection string? The last value wins in a psql
> command.
done
Also included feedback from Thomas Munro.
New patch attached.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
On Tue, Mar 6, 2018 at 8:45 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > New patch attached. Passes here. LGTM. Only complaint is your assumption that 'darwin' implies HomeBrew installation paths, but you already did that in other tests before this one. Perhaps we can sort that out globally in some future patch. PS Once this lands I'll adjust my Commitfest testing bot to use PG_TEST_EXTRA='kerberos ldap ssl'. -- Thomas Munro http://www.enterprisedb.com
On Mon, Mar 05, 2018 at 02:45:07PM -0500, Peter Eisentraut wrote:
> On 2/27/18 00:21, Michael Paquier wrote:
>> +my ($stdout, $krb5_version);
>> +IPC::Run::run [ 'krb5-config', '--version' ], '>', \$stdout or die
>> "could not execute krb5-config";
>> +$stdout =~ m/Kerberos 5 release ([0-9]+\.[0-9]+)/ or die "could not get
>> Kerberos version";
>> +$krb5_version = $1;
>> Time for a new routine command_log which executes the command, then
>> returns stdout and stderr to the caller?
>
> I didn't do anything about this. Do we have any more places where that
> would be needed right now?
I don't mind if this happens later on. I can send a patch as well if
necessary. There are a couple of places where things can be
consolidated using a single entry point:
- 010_pg_archivecleanup.pl
- PostgresNode::psql, where we could merge things.
- PostgresNode::poll_query_until
- PostgresNode::pg_recvlogical_upto
- TestLib::check_pg_config
- TestLib::program_help_ok
- TestLib::program_version_ok
- TestLib::program_options_handling_ok
- TestLib::command_like
- TestLib::command_like_safe
- TestLib::command_fails_like
- TestLib::command_checks_all
+if ($ENV{with_gssapi} eq 'yes')
+{
+ plan tests => 4;
+}
+else
+{
+ plan skip_all => 'GSSAPI/Kerberos not supported by this build';
+}
Thanks for adding this sanity check.
+my $kdc_port = int(rand() * 16384) + 49152;
That may not be worth worrying as that's an ephemeral port range but it
can make the test a bit unstable...
Perhaps the tests should be skipped on Windows or just produce an error?
Like LDAP tests, libraries are supported on Windows but the hardcoded
paths make things harder to handle there.
--
Michael
Вложения
On 3/5/18 16:34, Thomas Munro wrote: > On Tue, Mar 6, 2018 at 8:45 AM, Peter Eisentraut > <peter.eisentraut@2ndquadrant.com> wrote: >> New patch attached. > > Passes here. LGTM. committed > Only complaint is your assumption that 'darwin' implies HomeBrew > installation paths, but you already did that in other tests before > this one. Perhaps we can sort that out globally in some future patch. right -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 3/5/18 21:08, Michael Paquier wrote: > +my $kdc_port = int(rand() * 16384) + 49152; > That may not be worth worrying as that's an ephemeral port range but it > can make the test a bit unstable... This is what we've been using in the other tests as well. It's clearly not optimal, but making it more robust in a platform-agnostic way seems tricky. > Perhaps the tests should be skipped on Windows or just produce an error? > Like LDAP tests, libraries are supported on Windows but the hardcoded > paths make things harder to handle there. Hmm, why couldn't someone install MIT krb5 on Windows and give it a try? We don't need to block the platform outright. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: > On 3/5/18 21:08, Michael Paquier wrote: > >> Perhaps the tests should be skipped on Windows or just produce an error? >> Like LDAP tests, libraries are supported on Windows but the hardcoded >> paths make things harder to handle there. > > Hmm, why couldn't someone install MIT krb5 on Windows and give it a try? > We don't need to block the platform outright. krb5kdc doesn't support windows; only the client portion works there. Thanks, --Robbie
Вложения
Peter Eisentraut wrote: > On 3/5/18 21:08, Michael Paquier wrote: > > +my $kdc_port = int(rand() * 16384) + 49152; > > That may not be worth worrying as that's an ephemeral port range but it > > can make the test a bit unstable... > > This is what we've been using in the other tests as well. It's clearly > not optimal, but making it more robust in a platform-agnostic way seems > tricky. We have port-testing code in PostgresNode.pm::get_new_node; maybe that could be taking out of there into TestLib.pm? -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Mar 06, 2018 at 10:58:54AM -0500, Peter Eisentraut wrote:
> On 3/5/18 16:34, Thomas Munro wrote:
> > On Tue, Mar 6, 2018 at 8:45 AM, Peter Eisentraut
> > <peter.eisentraut@2ndquadrant.com> wrote:
> >> New patch attached.
> >
> > Passes here. LGTM.
>
> committed
This fails on my machine, where /etc/hosts has:
127.0.0.1 localhost.localdomain localhost
::1 localhost6.localdomain6 localhost6
This is CentOS 7, but I may have written that myself. First failure:
psql: FATAL: no pg_hba.conf entry for host "127.0.0.1", user "test1", database "postgres", SSL off
not ok 3 - succeeds with mapping
Bypassing that, by recognizing localhost.localdomain in pg_hba.conf, unearths:
psql: GSSAPI continuation error: Unspecified GSS failure. Minor code may provide more information
GSSAPI continuation error: Server krbtgt/LOCALDOMAIN@EXAMPLE.COM not found in Kerberos database
not ok 3 - succeeds with mapping
On the client side, Kerberos is canonicalizing "localhost" to
"localhost.localdomain" as part of constructing the service principal.
"$service_principal = "$ENV{with_krb_srvnam}/localhost.localdomain" was a
quick workaround. For the long-term fix, let's use hostaddr= and a fictitious
host=, as attached. This makes us independent of local name resolution and
IPv6 configuration, and it's more like how PostgresNode operates on systems
that use TCP instead of unix_socket_directories (Windows). I considered
adding dns_canonicalize_hostname to $krb5_config, but that is new as of
krb5-1.12 and does not help the pg_hba.conf side of the problem.