From d081a4f99d8b776fdebae601f5579a6e8d93be33 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Tue, 14 May 2019 12:52:03 +1200 Subject: [PATCH] Add ldapbindpasswdfile option for pg_hba.conf. Some users would prefer to keep the bind password for their LDAP server in a separate file. --- doc/src/sgml/client-auth.sgml | 13 +++++++ src/backend/libpq/auth.c | 69 ++++++++++++++++++++++++++++++++++- src/backend/libpq/hba.c | 21 ++++++++--- src/include/libpq/hba.h | 1 + src/test/ldap/t/001_auth.pl | 47 +++++++++++++++++++++++- 5 files changed, 144 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index ffed887ab7..3820252d55 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -1525,6 +1525,9 @@ omicron bryanh guest1 performed over the subtree at ldapbasedn, and will try to do an exact match of the attribute specified in ldapsearchattribute. + As an alternative to ldapbindpasswd, the + password may be stored in a separate file specified with + ldapbindpasswdfile. Once the user has been found in this search, the server disconnects and re-binds to the directory as this user, using the password specified by the client, to verify that the @@ -1642,6 +1645,16 @@ omicron bryanh guest1 when doing search+bind authentication. + + + ldapbindpasswdfile + + + File containing the password for user to bind to the directory with to + perform the search when doing search+bind authentication, as an + alternative to ldapbindpasswd. + + ldapsearchattribute diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 62466be702..3036cef59e 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -35,6 +35,7 @@ #include "miscadmin.h" #include "port/pg_bswap.h" #include "replication/walsender.h" +#include "storage/fd.h" #include "storage/ipc.h" #include "utils/memutils.h" #include "utils/timestamp.h" @@ -2563,6 +2564,45 @@ FormatSearchFilter(const char *pattern, const char *user_name) return output.data; } +/* + * Read the entire contents of the password file into output. Don't trim any + * trailing newline character, because that is the convention established by + * the ldapsearch -y commandline tool. + */ +static int +read_ldapbindpasswdfile(const char *path, StringInfo output) +{ + int fd; + char buffer[80]; + ssize_t size; + + fd = OpenTransientFile(path, O_RDONLY); + if (fd < 0) + return -1; + + for (;;) + { + size = read(fd, buffer, sizeof(buffer)); + if (size < 0) + { + int save_errno = errno; + + CloseTransientFile(fd); + errno = save_errno; + + return -1; + } + else if (size == 0) + break; + appendBinaryStringInfo(output, buffer, size); + } + + if (CloseTransientFile(fd) < 0) + return -1; + + return 0; +} + /* * Perform LDAP authentication */ @@ -2638,6 +2678,8 @@ CheckLDAPAuth(Port *port) char *dn; char *c; int count; + const char *bind_passwd; + StringInfoData bind_passwd_buffer = {0}; /* * Disallow any characters that we would otherwise need to escape, @@ -2664,10 +2706,35 @@ CheckLDAPAuth(Port *port) /* * Bind with a pre-defined username/password (if available) for * searching. If none is specified, this turns into an anonymous bind. + * If the password was specified in a separate file, read it. */ + if (port->hba->ldapbindpasswdfile) + { + initStringInfo(&bind_passwd_buffer); + if (read_ldapbindpasswdfile(port->hba->ldapbindpasswdfile, + &bind_passwd_buffer)) + { + ereport(LOG, + (errmsg("could not read ldapbindpasswd from \"%s\": %m", + port->hba->ldapbindpasswdfile))); + ldap_unbind(ldap); + pfree(passwd); + return STATUS_ERROR; + } + bind_passwd = bind_passwd_buffer.data; + } + else if (port->hba->ldapbindpasswd) + bind_passwd = port->hba->ldapbindpasswd; + else + bind_passwd = ""; + r = ldap_simple_bind_s(ldap, port->hba->ldapbinddn ? port->hba->ldapbinddn : "", - port->hba->ldapbindpasswd ? port->hba->ldapbindpasswd : ""); + bind_passwd); + + if (bind_passwd_buffer.data) + pfree(bind_passwd_buffer.data); + if (r != LDAP_SUCCESS) { ereport(LOG, diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 37d5ad44a5..fd3945ba3a 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -1536,7 +1536,7 @@ parse_hba_line(TokenizedLine *tok_line, int elevel) /* * LDAP can operate in two modes: either with a direct bind, using * ldapprefix and ldapsuffix, or using a search+bind, using - * ldapbasedn, ldapbinddn, ldapbindpasswd and one of + * ldapbasedn, ldapbinddn, ldapbindpasswd/ldapbindpasswdfile and one of * ldapsearchattribute or ldapsearchfilter. Disallow mixing these * parameters. */ @@ -1545,15 +1545,16 @@ parse_hba_line(TokenizedLine *tok_line, int elevel) if (parsedline->ldapbasedn || parsedline->ldapbinddn || parsedline->ldapbindpasswd || + parsedline->ldapbindpasswdfile || parsedline->ldapsearchattribute || parsedline->ldapsearchfilter) { ereport(elevel, (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapsearchattribute, ldapsearchfilter, or ldapurl together with ldapprefix"), + errmsg("cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapbindpasswdfile, ldapsearchattribute, ldapsearchfilter, or ldapurl together with ldapprefix"), errcontext("line %d of configuration file \"%s\"", line_num, HbaFileName))); - *err_msg = "cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapsearchattribute, ldapsearchfilter, or ldapurl together with ldapprefix"; + *err_msg = "cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapbindpasswdfile, ldapsearchattribute, ldapsearchfilter, or ldapurl together with ldapprefix"; return NULL; } } @@ -1856,6 +1857,11 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaLDAP, "ldapbindpasswd", "ldap"); hbaline->ldapbindpasswd = pstrdup(val); } + else if (strcmp(name, "ldapbindpasswdfile") == 0) + { + REQUIRE_AUTH_OPTION(uaLDAP, "ldapbindpasswdfile", "ldap"); + hbaline->ldapbindpasswdfile = pstrdup(val); + } else if (strcmp(name, "ldapsearchattribute") == 0) { REQUIRE_AUTH_OPTION(uaLDAP, "ldapsearchattribute", "ldap"); @@ -2275,12 +2281,12 @@ load_hba(void) /* * This macro specifies the maximum number of authentication options * that are possible with any given authentication method that is supported. - * Currently LDAP supports 11, and there are 3 that are not dependent on + * Currently LDAP supports 12, and there are 3 that are not dependent on * the auth method here. It may not actually be possible to set all of them * at the same time, but we'll set the macro value high enough to be * conservative and avoid warnings from static analysis tools. */ -#define MAX_HBA_OPTIONS 14 +#define MAX_HBA_OPTIONS 15 /* * Create a text array listing the options specified in the HBA line. @@ -2352,6 +2358,11 @@ gethba_options(HbaLine *hba) CStringGetTextDatum(psprintf("ldapbindpasswd=%s", hba->ldapbindpasswd)); + if (hba->ldapbindpasswdfile) + options[noptions++] = + CStringGetTextDatum(psprintf("ldapbindpasswdfile=%s", + hba->ldapbindpasswdfile)); + if (hba->ldapsearchattribute) options[noptions++] = CStringGetTextDatum(psprintf("ldapsearchattribute=%s", diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index 186e433574..985fd98f11 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -89,6 +89,7 @@ typedef struct HbaLine int ldapport; char *ldapbinddn; char *ldapbindpasswd; + char *ldapbindpasswdfile; char *ldapsearchattribute; char *ldapsearchfilter; char *ldapbasedn; diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl index 6c02f2530b..bf92ace060 100644 --- a/src/test/ldap/t/001_auth.pl +++ b/src/test/ldap/t/001_auth.pl @@ -6,7 +6,7 @@ use Test::More; if ($ENV{with_ldap} eq 'yes') { - plan tests => 22; + plan tests => 27; } else { @@ -63,6 +63,7 @@ my $ldap_basedn = 'dc=example,dc=net'; my $ldap_rootdn = 'cn=Manager,dc=example,dc=net'; my $ldap_rootpw = 'secret'; my $ldap_pwfile = "${TestLib::tmp_check}/ldappassword"; +my $ldap_bpwfile = "${TestLib::tmp_check}/ldappassword.bad"; note "setting up slapd"; @@ -120,6 +121,8 @@ END append_to_file($ldap_pwfile, $ldap_rootpw); chmod 0600, $ldap_pwfile or die; +append_to_file($ldap_bpwfile, "this-is-a-bad-password"); + $ENV{'LDAPURI'} = $ldap_url; $ENV{'LDAPBINDDN'} = $ldap_rootdn; $ENV{'LDAPCONF'} = $ldap_conf; @@ -310,3 +313,45 @@ $node->restart; $ENV{"PGPASSWORD"} = 'secret1'; test_access($node, 'test1', 2, 'bad combination of LDAPS and StartTLS'); + +note "Non-anonymous bind"; + +# bad ldapbindpasswd fails (note "x" prepended to password) +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf('pg_hba.conf', + qq{local all all ldap ldapbinddn="$ldap_rootdn" ldapbindpasswd="x$ldap_rootpw" ldapurl="$ldap_url/$ldap_basedn?uid?sub"}); +$node->restart; + +test_access($node, 'test1', 2, 'bad ldapbindpasswd fails'); + +# good ldapbindpasswd succeeds +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf('pg_hba.conf', + qq{local all all ldap ldapbinddn="$ldap_rootdn" ldapbindpasswd="$ldap_rootpw" ldapurl="$ldap_url/$ldap_basedn?uid?sub"}); +$node->restart; + +test_access($node, 'test1', 0, 'good ldapbindpasswd succeeds'); + +# bad ldapbindpasswdfile path fails (note "x" prepended to path) +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf('pg_hba.conf', + qq{local all all ldap ldapbinddn="$ldap_rootdn" ldapbindpasswdfile="x$ldap_pwfile" ldapurl="$ldap_url/$ldap_basedn?uid?sub"}); +$node->restart; + +test_access($node, 'test1', 2, 'bad ldapbindpasswdpath path fails'); + +# bad ldapbindpasswdfile contents fails +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf('pg_hba.conf', + qq{local all all ldap ldapbinddn="$ldap_rootdn" ldapbindpasswdfile="$ldap_bpwfile" ldapurl="$ldap_url/$ldap_basedn?uid?sub"}); +$node->restart; + +test_access($node, 'test1', 2, 'bad ldapbindpasswdfile contents fails'); + +# good ldapbindpasswdfile succeeds +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf('pg_hba.conf', + qq{local all all ldap ldapbinddn="$ldap_rootdn" ldapbindpasswdfile="$ldap_pwfile" ldapurl="$ldap_url/$ldap_basedn?uid?sub"}); +$node->restart; + +test_access($node, 'test1', 0, 'good ldapbindpasswdfile succeeds'); -- 2.21.0