On 2022-12-19 Mo 11:29, Andrew Dunstan wrote:
> This patch, mostly the work of John Naylor, provides a hook whereby a
> module can modify the ldapbindpasswd before it is handed to the ldap
> server. This is similar in concept to the ssl_passphrase_callback
> feature, and allows the user not to have to put the cleartext password
> in the pg_hba.conf file. A trivial test is added which provides an
> example of such a module.
Updated to take advantage of refactoring of ldap tests.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
From f9605b4703e970cf4eac9e26513a7dfca4040b4b Mon Sep 17 00:00:00 2001
From: Andrew Dunstan <and...@dunslane.net>
Date: Mon, 23 Jan 2023 14:05:32 -0500
Subject: [PATCH] Add a password handling hook for ldapbindpasswd
This hook allows for interception of the ldapbindpasswd in the
pg_hba.conf file before it is passed on to the ldap server. The hook
function receives a copy of the password as specified in the config file
and hands back a pointer to a password string. the default handler
simply hands back its input. A test is supplied which implements a
module that makes a trivial (rot13) modifiction of the input. The
benefit here is that the clear text password no longer needs to be
stored in the config file. This is similar in concept to the
ssl_passphrase_callback feature.
John Naylor, test modified by Andrew Dunstan.
---
src/backend/libpq/auth.c | 12 +-
src/include/libpq/auth.h | 6 +
src/test/modules/Makefile | 11 ++
src/test/modules/ldap_password_func/Makefile | 25 +++++
.../ldap_password_func/ldap_password_func.c | 64 +++++++++++
.../modules/ldap_password_func/meson.build | 35 ++++++
.../t/001_mutated_bindpasswd.pl | 103 ++++++++++++++++++
src/test/modules/meson.build | 1 +
8 files changed, 256 insertions(+), 1 deletion(-)
create mode 100644 src/test/modules/ldap_password_func/Makefile
create mode 100644 src/test/modules/ldap_password_func/ldap_password_func.c
create mode 100644 src/test/modules/ldap_password_func/meson.build
create mode 100644 src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 25b3a781cd..9463bf5a62 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -144,6 +144,10 @@ static int CheckLDAPAuth(Port *port);
#define LDAP_OPT_DIAGNOSTIC_MESSAGE LDAP_OPT_ERROR_STRING
#endif
+/* Default LDAP password mutator hook, can be overridden by a shared library */
+static char* dummy_ldap_password_mutator(char* input);
+auth_password_hook_typ ldap_password_hook = dummy_ldap_password_mutator;
+
#endif /* USE_LDAP */
/*----------------------------------------------------------------
@@ -2370,6 +2374,12 @@ InitializeLDAPConnection(Port *port, LDAP **ldap)
#define LDAPS_PORT 636
#endif
+static char*
+dummy_ldap_password_mutator(char * input)
+{
+ return input;
+}
+
/*
* Return a newly allocated C string copied from "pattern" with all
* occurrences of the placeholder "$username" replaced with "user_name".
@@ -2498,7 +2508,7 @@ CheckLDAPAuth(Port *port)
*/
r = ldap_simple_bind_s(ldap,
port->hba->ldapbinddn ? port->hba->ldapbinddn : "",
- port->hba->ldapbindpasswd ? port->hba->ldapbindpasswd : "");
+ port->hba->ldapbindpasswd ? ldap_password_hook(port->hba->ldapbindpasswd) : "");
if (r != LDAP_SUCCESS)
{
ereport(LOG,
diff --git a/src/include/libpq/auth.h b/src/include/libpq/auth.h
index 137bee7c45..9f2f3ed00d 100644
--- a/src/include/libpq/auth.h
+++ b/src/include/libpq/auth.h
@@ -28,4 +28,10 @@ extern void sendAuthRequest(Port *port, AuthRequest areq, const char *extradata,
typedef void (*ClientAuthentication_hook_type) (Port *, int);
extern PGDLLIMPORT ClientAuthentication_hook_type ClientAuthentication_hook;
+/* hook type for password manglers */
+typedef char* (* auth_password_hook_typ)(char* input);
+
+/* Default LDAP password mutator hook, can be overridden by a shared library */
+extern PGDLLIMPORT auth_password_hook_typ ldap_password_hook;
+
#endif /* AUTH_H */
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index c629cbe383..79e3033ec2 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -42,5 +42,16 @@ else
ALWAYS_SUBDIRS += ssl_passphrase_callback
endif
+# Test runs an LDAP server, so only run if ldap is in PG_TEST_EXTRA
+ifeq ($(with_ldap),yes)
+ifneq (,$(filter ldap,$(PG_TEST_EXTRA)))
+SUBDIRS += ldap_password_func
+else
+ALWAYS_SUBDIRS += ldap_password_func
+endif
+else
+ALWAYS_SUBDIRS += ldap_password_func
+endif
+
$(recurse)
$(recurse_always)
diff --git a/src/test/modules/ldap_password_func/Makefile b/src/test/modules/ldap_password_func/Makefile
new file mode 100644
index 0000000000..3324e04248
--- /dev/null
+++ b/src/test/modules/ldap_password_func/Makefile
@@ -0,0 +1,25 @@
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+# ldap_password_func Makefile
+
+export with_ldap
+
+MODULE_big = ldap_password_func
+OBJS = ldap_password_func.o $(WIN32RES)
+PGFILEDESC = "set hook to mutate ldapbindpasswd"
+
+TAP_TESTS = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/ldap_password_func
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
+
+
+
diff --git a/src/test/modules/ldap_password_func/ldap_password_func.c b/src/test/modules/ldap_password_func/ldap_password_func.c
new file mode 100644
index 0000000000..8a8ae40187
--- /dev/null
+++ b/src/test/modules/ldap_password_func/ldap_password_func.c
@@ -0,0 +1,64 @@
+/*-------------------------------------------------------------------------
+ *
+ * Copyright (c) 2022, PostgreSQL Global Development Group
+ *
+ * ldap_password_func.c
+ *
+ * Loadable PostgreSQL module to mutate the ldapbindpasswd. This
+ * implementation just hands back the configured password rot13'd.
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include <float.h>
+#include <stdio.h>
+
+#include "libpq/libpq.h"
+#include "libpq/libpq-be.h"
+#include "libpq/auth.h"
+#include "utils/guc.h"
+
+PG_MODULE_MAGIC;
+
+void _PG_init(void);
+void _PG_fini(void);
+
+/* hook function */
+static char* rot13_passphrase(char *password);
+
+/*
+ * Module load callback
+ */
+void
+_PG_init(void)
+{
+ ldap_password_hook = rot13_passphrase;
+}
+
+void
+_PG_fini(void)
+{
+ /* do nothing yet */
+}
+
+static char*
+rot13_passphrase(char *pw)
+{
+ size_t size = strlen(pw) + 1;
+
+ char* new_pw = (char*) palloc(size);
+ strlcpy(new_pw, pw, size);
+ for (char *p = new_pw; *p; p++)
+ {
+ char c = *p;
+
+ if ((c >= 'a' && c <= 'm') || (c >= 'A' && c <= 'M'))
+ *p = c + 13;
+ else if ((c >= 'n' && c <= 'z') || (c >= 'N' && c <= 'Z'))
+ *p = c - 13;
+ }
+
+ return new_pw;
+}
diff --git a/src/test/modules/ldap_password_func/meson.build b/src/test/modules/ldap_password_func/meson.build
new file mode 100644
index 0000000000..8ab6d6e8ac
--- /dev/null
+++ b/src/test/modules/ldap_password_func/meson.build
@@ -0,0 +1,35 @@
+if not ldap.found()
+ subdir_done()
+endif
+
+# FIXME: prevent install during main install, but not during test :/
+
+ldap_password_func_sources = files(
+ 'ldap_password_func.c',
+)
+
+if host_system == 'windows'
+ ldap_password_func_sources += rc_lib_gen.process(win32ver_rc, extra_args: [
+ '--NAME', 'ldap_password_func',
+ '--FILEDESC', 'set hook to mutate ldapbindpassw',])
+endif
+
+ldap_password_func = shared_module('ldap_password_func',
+ ldap_password_func_sources,
+ kwargs: pg_mod_args + {
+ 'dependencies': [ldap, pg_mod_args['dependencies']],
+ },
+)
+testprep_targets += ldap_password_func
+
+tests += {
+ 'name': 'ldap_password_func',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_mutated_bindpasswd.pl',
+ ],
+ 'env': {'with_ldap': 'yes'}
+ },
+}
diff --git a/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl b/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl
new file mode 100644
index 0000000000..4174292d2d
--- /dev/null
+++ b/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl
@@ -0,0 +1,103 @@
+
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use File::Copy;
+use FindBin;
+use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+use lib "$FindBin::RealBin/../../../ldap";
+use LdapServer;
+
+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';
+}
+elsif (!$LdapServer::setup)
+{
+ plan skip_all =>
+ "ldap tests not supported on $^O or dependencies not installed";
+}
+
+my $clear_ldap_rootpw = "FooBaR1";
+my $rot13_ldap_rootpw = "SbbOnE1";
+
+my $ldap = LdapServer->new($clear_ldap_rootpw, 'users'); # no anonymous auth
+$ldap->ldapadd_file("$FindBin::RealBin/../../../ldap/authdata.ldif");
+$ldap->ldapsetpw('uid=test1,dc=example,dc=net', 'secret1');
+
+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->append_conf('postgresql.conf', "shared_preload_libraries = 'ldap_password_func'");
+$node->start;
+
+$node->safe_psql('postgres', 'CREATE USER test1;');
+
+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";
+
+$ENV{"PGPASSWORD"} = 'secret1';
+
+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;
+
+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="$clear_ldap_rootpw"}
+);
+$node->restart;
+
+test_access($node, 'test1', 2, 'search+bind authentication fails with clear password');
+
+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="$rot13_ldap_rootpw"}
+);
+$node->restart;
+
+test_access($node, 'test1', 0, 'search+bind authentication succeeds with rot13ed password');
+
+done_testing();
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 1baa6b558d..dcb82ed68f 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -5,6 +5,7 @@ subdir('commit_ts')
subdir('delay_execution')
subdir('dummy_index_am')
subdir('dummy_seclabel')
+subdir('ldap_password_func')
subdir('libpq_pipeline')
subdir('plsample')
subdir('snapshot_too_old')
--
2.34.1
