On 2023-01-02 Mo 09:45, Andrew Dunstan wrote:
> On 2023-01-01 Su 18:31, Andrew Dunstan wrote:
>> On 2023-01-01 Su 14:02, Thomas Munro wrote:
>>> On Mon, Jan 2, 2023 at 3:04 AM Andrew Dunstan <and...@dunslane.net> wrote:
>>>> On 2022-12-19 Mo 11:16, Andrew Dunstan wrote:
>>>>> There is currently no test for the use of ldapbindpasswd in the
>>>>> pg_hba.conf file. This patch, mostly the work of John Naylor, remedies
>>>>> that.
>>>>>
>>>>>
>>>> This currently has failures on the cfbot for meson builds on FBSD13 and
>>>> Debian Bullseye, but it's not at all clear why. In both cases it fails
>>>> where the ldap server is started.
>>> I think it's failing when using meson. I guess it fails to fail on
>>> macOS only because you need to add a new path for Homebrew/ARM like
>>> commit 14d63dd2, so it's skipping (it'd be nice if we didn't need
>>> another copy of all that logic). Trying locally... it looks like
>>> slapd is failing silently, and with some tracing I can see it's
>>> sending an error message to my syslog daemon, which logged:
>>>
>>> 2023-01-02T07:50:20.853019+13:00 x1 slapd[153599]: main: TLS init def
>>> ctx failed: -1
>>>
>>> Ah, it looks like this test is relying on "slapd-certs", which doesn't
>>> exist:
>>>
>>> tmunro@x1:~/projects/postgresql/build$ ls testrun/ldap/001_auth/data/
>>> ldap.conf ldappassword openldap-data portlock slapd-certs slapd.conf
>>> tmunro@x1:~/projects/postgresql/build$ ls testrun/ldap/002_bindpasswd/data/
>>> portlock slapd.conf
>>>
>>> I didn't look closely, but apparently there is something wrong in the
>>> part that copies certs from the ssl test? Not sure why it works for
>>> autoconf...
>>
>> Let's see how we fare with this patch.
>>
>>
> Not so well :-(. This version tries to make the tests totally
> independent, as they should be. That's an attempt to get the cfbot to go
> green, but I am intending to refactor this code substantially so the
> common bits are in a module each test file will load.
>
>
This version factors out the creation of the LDAP server into a separate
perl Module. That makes both the existing test script and the new test
script a lot shorter, and will be useful for the nearby patch for a hook
for the ldapbindpassword.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
diff --git a/src/test/ldap/LdapServer.pm b/src/test/ldap/LdapServer.pm
new file mode 100644
index 0000000000..a7198e06e5
--- /dev/null
+++ b/src/test/ldap/LdapServer.pm
@@ -0,0 +1,318 @@
+
+############################################################################
+#
+# LdapServer.pm
+#
+# Module to set up an LDAP server for testing pg_hba.conf ldap authentication
+#
+# Copyright (c) 2023, PostgreSQL Global Development Group
+#
+############################################################################
+
+=pod
+
+=head1 NAME
+
+LdapServer - class for an LDAP server for testing pg_hba.conf authentication
+
+=head1 SYNOPSIS
+
+ use LdapServer;
+
+ # have we found openldap binaies suitable for setting up a server?
+ my $ldap_binaries_found = $LdapServer::setup;
+
+ # create a server with the given root password and auth type
+ # (users or anonymous)
+ my $server = LdapServer->new($root_password, $auth_type);
+
+ # Add the contents of an LDIF file to the server
+ $server->ldapadd_file ($path_to_ldif_data);
+
+ # set the Ldap password for a user
+ $server->ldapsetpw($user, $password);
+
+ # get details of some settings for the server
+ my @properties = $server->prop($propname1, $propname2, ...);
+
+=head1 DESCRIPTION
+
+ LdapServer tests in its INIT phase for the presence of suitable openldap
+ binaries. Its constructor method sets up and runs an LDAP server, and any
+ servers that are set up are terminated during its END phase.
+
+=cut
+
+package LdapServer;
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+use File::Copy;
+use File::Basename;
+
+# private variables
+my ($slapd, $ldap_schema_dir, @servers);
+
+# visible variable
+our ($setup);
+
+INIT
+{
+ $setup = 1;
+ if ($^O eq 'darwin' && -d '/opt/homebrew/opt/openldap')
+ {
+ # typical paths for Homebrew on ARM
+ $slapd = '/opt/homebrew/opt/openldap/libexec/slapd';
+ $ldap_schema_dir = '/opt/homebrew/etc/openldap/schema';
+ }
+ elsif ($^O eq 'darwin' && -d '/usr/local/opt/openldap')
+ {
+ # typical paths for Homebrew on Intel
+ $slapd = '/usr/local/opt/openldap/libexec/slapd';
+ $ldap_schema_dir = '/usr/local/etc/openldap/schema';
+ }
+ elsif ($^O eq 'darwin' && -d '/opt/local/etc/openldap')
+ {
+ # typical paths for MacPorts
+ $slapd = '/opt/local/libexec/slapd';
+ $ldap_schema_dir = '/opt/local/etc/openldap/schema';
+ }
+ elsif ($^O eq 'linux')
+ {
+ $slapd = '/usr/sbin/slapd';
+ $ldap_schema_dir = '/etc/ldap/schema' if -d '/etc/ldap/schema';
+ $ldap_schema_dir = '/etc/openldap/schema' if -d '/etc/openldap/schema';
+ }
+ elsif ($^O eq 'freebsd')
+ {
+ $slapd = '/usr/local/libexec/slapd';
+ $ldap_schema_dir = '/usr/local/etc/openldap/schema';
+ }
+ elsif ($^O eq 'openbsd')
+ {
+ $slapd = '/usr/local/libexec/slapd';
+ $ldap_schema_dir = '/usr/local/share/examples/openldap/schema';
+ }
+ else
+ {
+ $setup = 0;
+ }
+}
+
+END
+{
+ foreach my $server (@servers)
+ {
+ next unless -f $server->{pidfile};
+ my $pid = slurp_file($server->{pidfile});
+ chomp $pid;
+ kill 'INT', $pid;
+ }
+}
+
+=pod
+
+=head1 METHODS
+
+=over
+
+=item LdapServer->new($rootpw, $auth_type)
+
+Create a new LDAP server.
+
+The rootpw can be used when authenticating with the ldapbindpasswd option.
+
+The auth_type is either 'users' or 'anonymous'.
+
+=back
+
+=cut
+
+sub new
+{
+ die "no suitable binaries found" unless $setup;
+
+ my $class = shift;
+ my $rootpw = shift;
+ my $authtype = shift; # 'users' or 'anonymous'
+ my $testname = basename((caller)[1],'.pl');
+ my $self = {};
+
+ my $test_temp = PostgreSQL::Test::Utils::tempdir("ldap-$testname");
+
+ my $ldap_datadir = "$test_temp/openldap-data";
+ my $slapd_certs = "$test_temp/slapd-certs";
+ my $slapd_pidfile = "$test_temp/slapd.pid";
+ my $slapd_logfile =
+ "${PostgreSQL::Test::Utils::log_path}/slapd-$testname.log";
+ my $ldap_server = 'localhost';
+ my $ldap_port = PostgreSQL::Test::Cluster::get_free_port();
+ my $ldaps_port = PostgreSQL::Test::Cluster::get_free_port();
+ my $ldap_url = "ldap://$ldap_server:$ldap_port";;
+ my $ldaps_url = "ldaps://$ldap_server:$ldaps_port";
+ my $ldap_basedn = 'dc=example,dc=net';
+ my $ldap_rootdn = 'cn=Manager,dc=example,dc=net';
+ my $ldap_rootpw = $rootpw;
+ my $ldap_pwfile = "$test_temp/ldappassword";
+
+ (my $conf = <<"EOC") =~ s/^\t\t//gm;
+ include $ldap_schema_dir/core.schema
+ include $ldap_schema_dir/cosine.schema
+ include $ldap_schema_dir/nis.schema
+ include $ldap_schema_dir/inetorgperson.schema
+
+ pidfile $slapd_pidfile
+ logfile $slapd_logfile
+
+ access to *
+ by * read
+ by $authtype auth
+
+ database ldif
+ directory $ldap_datadir
+
+ TLSCACertificateFile $slapd_certs/ca.crt
+ TLSCertificateFile $slapd_certs/server.crt
+ TLSCertificateKeyFile $slapd_certs/server.key
+
+ suffix "dc=example,dc=net"
+ rootdn "$ldap_rootdn"
+ rootpw "$ldap_rootpw"
+EOC
+ append_to_file($slapd_conf, $conf);
+
+ mkdir $ldap_datadir or die "making $ldap_datadir: $!";
+ mkdir $slapd_certs or die "making $slapd_certs: $!";
+
+ my $certdir = dirname(__FILE__) . "/../ssl/ssl";
+
+ copy "$certdir/server_ca.crt", "$slapd_certs/ca.crt"
+ || die "copying ca.crt: $!";
+ # check we actually have the file, as copy() sometimes gives a false success
+ -f "$slapd_certs/ca.crt" || die "copying ca.crt (error unknown)";
+ copy "$certdir/server-cn-only.crt", "$slapd_certs/server.crt"
+ || die "copying server.crt: $!";
+ copy "$certdir/server-cn-only.key", "$slapd_certs/server.key"
+ || die "copying server.key: $!";
+
+ append_to_file($ldap_pwfile, $ldap_rootpw);
+ chmod 0600, $ldap_pwfile or die "chmod on $ldap_pwfile";
+
+ system_or_bail $slapd, '-f', $slapd_conf, '-h', "$ldap_url $ldaps_url";
+
+ # wait until slapd accepts requests
+ my $retries = 0;
+ while (1)
+ {
+ last
+ if (
+ system_log(
+ "ldapsearch", "-sbase",
+ "-H", $ldap_url,
+ "-b", $ldap_basedn,
+ "-D", $ldap_rootdn,
+ "-y", $ldap_pwfile,
+ "-n", "'objectclass=*'") == 0);
+ die "cannot connect to slapd" if ++$retries >= 300;
+ note "waiting for slapd to accept requests...";
+ Time::HiRes::usleep(1000000);
+ }
+
+ $self->{pidfile} = $slapd_pidfile;
+ $self->{pwfile} = $ldap_pwfile;
+ $self->{url} = $ldap_url;
+ $self->{s_url} = $ldaps_url;
+ $self->{server} = $ldap_server;
+ $self->{port} = $ldap_port;
+ $self->{s_port} = $ldaps_port;
+ $self->{basedn} = $ldap_basedn;
+ $self->{rootdn} = $ldap_rootdn;
+
+ bless $self, $class;
+ push @servers, $self;
+ return $self;
+}
+
+# private routine to set up the environment for methods below
+sub _ldapenv
+{
+ my $self = shift;
+ my %env = %ENV;
+ $env{'LDAPURI'} = $self->{url};
+ $env{'LDAPBINDDN'} = $self->{rootdn};
+ return %env;
+}
+
+=pod
+
+=over
+
+=item ldap_add(filename)
+
+filename is the path to a file containing LDIF data which is added to the LDAP
+server.
+
+=back
+
+=cut
+
+sub ldapadd_file
+{
+ my $self = shift;
+ my $file = shift;
+
+ local %ENV = $self->_ldapenv;
+
+ system_or_bail 'ldapadd', '-x', '-y', $self->{pwfile}, '-f', $file;
+}
+
+=pod
+
+=over
+
+=item ldapsetpw(user, password)
+
+Set the user's password in the LDAP server
+
+=back
+
+=cut
+
+sub ldapsetpw
+{
+ my $self = shift;
+ my $user = shift;
+ my $password = shift;
+
+ local %ENV = $self->_ldapenv;
+
+ system_or_bail 'ldappasswd', '-x', '-y', $self->{pwfile}, '-s', $password,
+ $user;
+}
+
+=pod
+
+=over
+
+=item prop(name1, ...)
+
+Returns the list of values for the specified properties of the instance, such
+as 'url', 'port', 'basedn'.
+
+=back
+
+=cut
+
+sub prop
+{
+ my $self = shift;
+ my @settings;
+ push @settings, $self->{$_} foreach (@_);
+ return @settings;
+}
+
+1;
diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
index 39736e5116..4a6c4c9339 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -3,170 +3,45 @@
use strict;
use warnings;
+
+use FindBin; use lib "$FindBin::RealBin/..";
+
use File::Copy;
+use File::Basename;
+use LdapServer;
use PostgreSQL::Test::Utils;
use PostgreSQL::Test::Cluster;
use Test::More;
-
-my ($slapd, $ldap_bin_dir, $ldap_schema_dir);
-
-$ldap_bin_dir = undef; # usually in PATH
-
if ($ENV{with_ldap} ne 'yes')
{
plan skip_all => 'LDAP not supported by this build';
}
elsif ($ENV{PG_TEST_EXTRA} !~ /\bldap\b/)
{
- plan skip_all => 'Potentially unsafe test LDAP not enabled in PG_TEST_EXTRA';
+ plan skip_all =>
+ 'Potentially unsafe test LDAP not enabled in PG_TEST_EXTRA';
}
-elsif ($^O eq 'darwin' && -d '/opt/homebrew/opt/openldap')
-{
- # typical paths for Homebrew on ARM
- $slapd = '/opt/homebrew/opt/openldap/libexec/slapd';
- $ldap_schema_dir = '/opt/homebrew/etc/openldap/schema';
-}
-elsif ($^O eq 'darwin' && -d '/usr/local/opt/openldap')
-{
- # typical paths for Homebrew on Intel
- $slapd = '/usr/local/opt/openldap/libexec/slapd';
- $ldap_schema_dir = '/usr/local/etc/openldap/schema';
-}
-elsif ($^O eq 'darwin' && -d '/opt/local/etc/openldap')
-{
- # typical paths for MacPorts
- $slapd = '/opt/local/libexec/slapd';
- $ldap_schema_dir = '/opt/local/etc/openldap/schema';
-}
-elsif ($^O eq 'linux')
-{
- $slapd = '/usr/sbin/slapd';
- $ldap_schema_dir = '/etc/ldap/schema' if -d '/etc/ldap/schema';
- $ldap_schema_dir = '/etc/openldap/schema' if -d '/etc/openldap/schema';
-}
-elsif ($^O eq 'freebsd')
-{
- $slapd = '/usr/local/libexec/slapd';
- $ldap_schema_dir = '/usr/local/etc/openldap/schema';
-}
-elsif ($^O eq 'openbsd')
-{
- $slapd = '/usr/local/libexec/slapd';
- $ldap_schema_dir = '/usr/local/share/examples/openldap/schema';
-}
-else
+elsif (! $LdapServer::setup)
{
plan skip_all =>
"ldap tests not supported on $^O or dependencies not installed";
}
-# make your own edits here
-#$slapd = '';
-#$ldap_bin_dir = '';
-#$ldap_schema_dir = '';
+note "setting up LDAP server";
-$ENV{PATH} = "$ldap_bin_dir:$ENV{PATH}" if $ldap_bin_dir;
+my $ldap_rootpw = 'secret';
+my $ldap = LdapServer->new($ldap_rootpw,'anonymous'); # use anonymous auth
+$ldap->ldapadd_file('authdata.ldif');
+$ldap->ldapsetpw('uid=test1,dc=example,dc=net','secret1');
+$ldap->ldapsetpw('uid=test2,dc=example,dc=net','secret2');
-my $ldap_datadir = "${PostgreSQL::Test::Utils::tmp_check}/openldap-data";
-my $slapd_certs = "${PostgreSQL::Test::Utils::tmp_check}/slapd-certs";
-my $slapd_conf = "${PostgreSQL::Test::Utils::tmp_check}/slapd.conf";
-my $slapd_pidfile = "${PostgreSQL::Test::Utils::tmp_check}/slapd.pid";
-my $slapd_logfile = "${PostgreSQL::Test::Utils::log_path}/slapd.log";
-my $ldap_conf = "${PostgreSQL::Test::Utils::tmp_check}/ldap.conf";
-my $ldap_server = 'localhost';
-my $ldap_port = PostgreSQL::Test::Cluster::get_free_port();
-my $ldaps_port = PostgreSQL::Test::Cluster::get_free_port();
-my $ldap_url = "ldap://$ldap_server:$ldap_port";;
-my $ldaps_url = "ldaps://$ldap_server:$ldaps_port";
-my $ldap_basedn = 'dc=example,dc=net';
-my $ldap_rootdn = 'cn=Manager,dc=example,dc=net';
-my $ldap_rootpw = 'secret';
-my $ldap_pwfile = "${PostgreSQL::Test::Utils::tmp_check}/ldappassword";
-
-note "setting up slapd";
-
-append_to_file(
- $slapd_conf,
- qq{include $ldap_schema_dir/core.schema
-include $ldap_schema_dir/cosine.schema
-include $ldap_schema_dir/nis.schema
-include $ldap_schema_dir/inetorgperson.schema
-
-pidfile $slapd_pidfile
-logfile $slapd_logfile
-
-access to *
- by * read
- by anonymous auth
-
-database ldif
-directory $ldap_datadir
-
-TLSCACertificateFile $slapd_certs/ca.crt
-TLSCertificateFile $slapd_certs/server.crt
-TLSCertificateKeyFile $slapd_certs/server.key
-
-suffix "dc=example,dc=net"
-rootdn "$ldap_rootdn"
-rootpw $ldap_rootpw});
+my ($ldap_server, $ldap_port, $ldaps_port, $ldap_url, $ldaps_url, $ldap_basedn,
+ $ldap_rootdn) =
+ $ldap->prop(qw(server port s_port url s_url basedn rootdn));
# don't bother to check the server's cert (though perhaps we should)
-append_to_file(
- $ldap_conf,
- qq{TLS_REQCERT never
-});
-
-mkdir $ldap_datadir or die;
-mkdir $slapd_certs or die;
-
-# use existing certs from nearby SSL test suite
-copy "../ssl/ssl/server_ca.crt", "$slapd_certs/ca.crt"
- || die "copying ca.crt: $!";
-copy "../ssl/ssl/server-cn-only.crt", "$slapd_certs/server.crt"
- || die "copying server.crt: $!";;
-copy "../ssl/ssl/server-cn-only.key", "$slapd_certs/server.key"
- || die "copying server.key: $!";;
-
-system_or_bail $slapd, '-f', $slapd_conf, '-h', "$ldap_url $ldaps_url";
-
-END
-{
- kill 'INT', `cat $slapd_pidfile` if -f $slapd_pidfile;
-}
-
-append_to_file($ldap_pwfile, $ldap_rootpw);
-chmod 0600, $ldap_pwfile or die;
-
-# wait until slapd accepts requests
-my $retries = 0;
-while (1)
-{
- last
- if (
- system_log(
- "ldapsearch", "-sbase",
- "-H", $ldap_url,
- "-b", $ldap_basedn,
- "-D", $ldap_rootdn,
- "-y", $ldap_pwfile,
- "-n", "'objectclass=*'") == 0);
- die "cannot connect to slapd" if ++$retries >= 300;
- note "waiting for slapd to accept requests...";
- Time::HiRes::usleep(1000000);
-}
-
-$ENV{'LDAPURI'} = $ldap_url;
-$ENV{'LDAPBINDDN'} = $ldap_rootdn;
-$ENV{'LDAPCONF'} = $ldap_conf;
-
-note "loading LDAP data";
-
-system_or_bail 'ldapadd', '-x', '-y', $ldap_pwfile, '-f', 'authdata.ldif';
-system_or_bail 'ldappasswd', '-x', '-y', $ldap_pwfile, '-s', 'secret1',
- 'uid=test1,dc=example,dc=net';
-system_or_bail 'ldappasswd', '-x', '-y', $ldap_pwfile, '-s', 'secret2',
- 'uid=test2,dc=example,dc=net';
+$ENV{'LDAPTLS_REQCERT'} = "never";
note "setting up PostgreSQL instance";
diff --git a/src/test/ldap/t/002_bindpasswd.pl b/src/test/ldap/t/002_bindpasswd.pl
new file mode 100644
index 0000000000..112149344a
--- /dev/null
+++ b/src/test/ldap/t/002_bindpasswd.pl
@@ -0,0 +1,92 @@
+
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+use FindBin; use lib "$FindBin::RealBin/..";
+
+use File::Copy;
+use File::Basename;
+use LdapServer;
+use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+if ($ENV{with_ldap} ne 'yes')
+{
+ plan skip_all => 'LDAP not supported by this build';
+}
+elsif ($ENV{PG_TEST_EXTRA} !~ /\bldap\b/)
+{
+ plan skip_all =>
+ 'Potentially unsafe test LDAP not enabled in PG_TEST_EXTRA';
+}
+elsif (! $LdapServer::setup)
+{
+ plan skip_all =>
+ "ldap tests not supported on $^O or dependencies not installed";
+}
+
+note "setting up LDAP server";
+
+my $ldap_rootpw = 'secret';
+my $ldap = LdapServer->new($ldap_rootpw,'users'); # no anonymous auth
+$ldap->ldapadd_file('authdata.ldif');
+$ldap->ldapsetpw('uid=test1,dc=example,dc=net','secret1');
+$ldap->ldapsetpw('uid=test2,dc=example,dc=net','secret2');
+
+my ($ldap_server, $ldap_port, $ldap_basedn, $ldap_rootdn) =
+ $ldap->prop(qw(server port basedn rootdn));
+
+note "setting up PostgreSQL instance";
+
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+$node->append_conf('postgresql.conf', "log_connections = on\n");
+$node->start;
+
+$node->safe_psql('postgres', 'CREATE USER test0;');
+$node->safe_psql('postgres', 'CREATE USER test1;');
+$node->safe_psql('postgres', 'CREATE USER "te...@example.net";');
+
+note "running tests";
+
+sub test_access
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+ my ($node, $role, $expected_res, $test_name, %params) = @_;
+ my $connstr = "user=$role";
+
+ if ($expected_res eq 0)
+ {
+ $node->connect_ok($connstr, $test_name, %params);
+ }
+ else
+ {
+ # No checks of the error message, only the status code.
+ $node->connect_fails($connstr, $test_name, %params);
+ }
+}
+
+note "use ldapbindpasswd";
+
+unlink($node->data_dir . '/pg_hba.conf');
+$node->append_conf('pg_hba.conf',
+ qq{local all all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapbasedn="$ldap_basedn" ldapbinddn="$ldap_rootdn ldapbindpasswd=wrong}
+);
+$node->restart;
+
+$ENV{"PGPASSWORD"} = 'secret1';
+test_access($node, 'test1', 2, 'search+bind authentication fails with wrong ldapbindpasswd');
+
+unlink($node->data_dir . '/pg_hba.conf');
+$node->append_conf('pg_hba.conf',
+ qq{local all all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapbasedn="$ldap_basedn" ldapbinddn="$ldap_rootdn" ldapbindpasswd="$ldap_rootpw"}
+);
+$node->restart;
+
+test_access($node, 'test1', 0, 'search+bind authentication succeeds with ldapbindpasswd');
+
+done_testing();
