Hello, There was a brief discussion [1] back in February on allowing user mapping for LDAP, in order to open up some more complex authorization logic (and slightly reduce the need for LDAP-to-Postgres user synchronization). Attached is an implementation of this that separates the LDAP authentication and authorization identities, and lets the client control the former with an `ldapuser` connection option or its associated PGLDAPUSER envvar.
This isn't as useful as, say, authorization based on LDAP attributes or group membership, but it seems like a necessary step towards that feature, since we'll need to separate authn and authz anyway. This provides some feature parity with other auth methods like gss, and it solves the "let anyone who can authenticate against LDAP connect as X role" use case trivially. There is precedent for allowing the DBA to choose whether to map a bare username or the "full" identity expansion -- see for example include_realm=1 for gss and clientname=DN for cert -- so I added an ldap_map_dn=1 option which can be used to map the whole DN. (I'm not entirely convinced that it's useful, but maybe there are some deployments that put authorization information into the LDAP topology, like "everyone in this particular subtree is an admin".) Unlike include_realm, this is only allowed with an explicit map. I don't anticipate people using a full DN as a database username, and I'm worried that doing that without normalization could cause some major confusion and/or security problems. When using a newer client with an older server, the new `ldapuser` option will cause a connection failure. For the case where PGUSER and PGLDAPUSER are identical, that failure is technically unnecessary, and I briefly considered stripping the `ldapuser` option from the connection string in that case so that we could have wider compatibility. I now think that's a bad idea, because suddenly introducing a hard connection failure (or new success) just because your PGLDAPUSER variable changed would be a major support hazard. The TODO is still in the code to remind me to have the conversation. WDYT? --Jacob [1] https://www.postgresql.org/message-id/94f6b945f9ca8cabe2b9d2a38ec19dca6f90a083.camel%40vmware.com
From 24d58279bb85340a55ed618a60a20b0483ac3a0a Mon Sep 17 00:00:00 2001 From: Jacob Champion <pchamp...@vmware.com> Date: Mon, 30 Aug 2021 16:29:59 -0700 Subject: [PATCH] Allow user name mapping with LDAP Enable the `map` HBA option for the ldap auth method. To make effective use of this from the client side, the ldapuser connection option (and a corresponding environment variable, PGLDAPUSER) has been added; it defaults to the PGUSER. For more advanced mapping, the ldap_map_dn HBA option can be set to use the full user Distinguished Name during user mapping. (This parallels the include_realm=1 and clientname=DN options.) --- doc/src/sgml/client-auth.sgml | 45 ++++++++- doc/src/sgml/libpq.sgml | 32 ++++++ src/backend/libpq/auth.c | 38 ++++++-- src/backend/libpq/hba.c | 29 +++++- src/backend/postmaster/postmaster.c | 2 + src/include/libpq/hba.h | 1 + src/include/libpq/libpq-be.h | 5 + src/interfaces/libpq/fe-connect.c | 6 ++ src/interfaces/libpq/fe-protocol3.c | 2 + src/interfaces/libpq/libpq-int.h | 1 + src/test/ldap/t/001_auth.pl | 145 +++++++++++++++++++++++++++- 11 files changed, 291 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 02f0489112..d902eb9d01 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -1624,10 +1624,11 @@ omicron bryanh guest1 This authentication method operates similarly to <literal>password</literal> except that it uses LDAP as the password verification method. LDAP is used only to validate - the user name/password pairs. Therefore the user must already - exist in the database before LDAP can be used for - authentication. - </para> + the user name/password pairs. Therefore database users must already + exist in the database before LDAP can be used for authentication. User name + mapping can be used to allow the LDAP user name to be different from the + database user name. +</para> <para> LDAP authentication can operate in two modes. In the first mode, @@ -1703,6 +1704,42 @@ omicron bryanh guest1 </para> </listitem> </varlistentry> + <varlistentry> + <term><literal>map</literal></term> + <listitem> + <para> + Allows for mapping between LDAP and database user names. See + <xref linkend="auth-username-maps"/> for details. + </para> + </listitem> + </varlistentry> + <varlistentry> + <term><literal>ldap_map_dn</literal></term> + <listitem> + <para> + Set to 1 to use the user's full Distinguished Name (e.g. + <literal>uid=someuser,dc=example,dc=com</literal>) during user name + mapping. When set to 0 (the default), only the bare username (in this + example, <literal>someuser</literal>) will be mapped. This option may + only be used in conjunction with the <literal>map</literal> option. + </para> + <note> + <para> + When using regular expression matching in combination with this option, + care should be taken to correctly anchor the regular expression in + order to avoid false positives. For example, the anchored expression + <literal>/,dc=example,dc=com$</literal> will match only DNs underneath + the <literal>example.com</literal> subtree, whereas the unanchored + <literal>/dc=example,dc=com</literal> could match + <literal>uid=mal,dc=example,dc=community,dc=invalid</literal> + or any number of DNs that are not rooted at + <literal>example.com</literal>. (Whether these DNs exist in the LDAP + tree to be abused is dependent on the LDAP deployment and outside the + scope of this documentation.) + </para> + </note> + </listitem> + </varlistentry> </variablelist> </para> diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index b449c834a9..8a0ec715e4 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1455,6 +1455,28 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname </listitem> </varlistentry> + <varlistentry id="libpq-connect-ldapuser" xreflabel="ldapuser"> + <term><literal>ldapuser</literal></term> + <listitem> + <para> + When connecting to servers that use LDAP authentication, this option + sets the username that should be used to bind to the LDAP server. + By default, the <productname>PostgreSQL</productname> user name + (<xref linkend="libpq-connect-user"/>) is used to bind. + </para> + + <note> + <para> + For this option to be useful, the server must be appropriately + configured with a user name map (<xref linkend="auth-username-maps"/>). + Server versions prior to 15 always use the + <productname>PostgreSQL</productname> user name to bind and cannot make + use of this option. + </para> + </note> + </listitem> + </varlistentry> + <varlistentry id="libpq-connect-gssencmode" xreflabel="gssencmode"> <term><literal>gssencmode</literal></term> <listitem> @@ -7918,6 +7940,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) </para> </listitem> + <listitem> + <para> + <indexterm> + <primary><envar>PGLDAPUSER</envar></primary> + </indexterm> + <envar>PGLDAPUSER</envar> behaves the same as the + <xref linkend="libpq-connect-ldapuser"/> connection parameter. + </para> + </listitem> + <listitem> <para> <indexterm> diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 8cc23ef7fb..eeb5a6d46f 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -2479,8 +2479,10 @@ CheckLDAPAuth(Port *port) char *passwd; LDAP *ldap; int r; + const char *ldapuser; char *fulluser; const char *server_name; + const char *map_name; #ifdef HAVE_LDAP_INITIALIZE @@ -2532,6 +2534,11 @@ CheckLDAPAuth(Port *port) return STATUS_ERROR; } + /* If a PGLDAPUSER was not provided, default to PGUSER. */ + ldapuser = port->ldapuser; + if (!ldapuser || !ldapuser[0]) + ldapuser = port->user_name; + if (port->hba->ldapbasedn) { /* @@ -2543,7 +2550,7 @@ CheckLDAPAuth(Port *port) LDAPMessage *entry; char *attributes[] = {LDAP_NO_ATTRS, NULL}; char *dn; - char *c; + const char *c; int count; /* @@ -2552,7 +2559,7 @@ CheckLDAPAuth(Port *port) * them would make it possible to inject any kind of custom filters in * the LDAP filter. */ - for (c = port->user_name; *c; c++) + for (c = ldapuser; *c; c++) { if (*c == '*' || *c == '(' || @@ -2590,11 +2597,11 @@ CheckLDAPAuth(Port *port) /* Build a custom filter or a single attribute filter? */ if (port->hba->ldapsearchfilter) - filter = FormatSearchFilter(port->hba->ldapsearchfilter, port->user_name); + filter = FormatSearchFilter(port->hba->ldapsearchfilter, ldapuser); else if (port->hba->ldapsearchattribute) - filter = psprintf("(%s=%s)", port->hba->ldapsearchattribute, port->user_name); + filter = psprintf("(%s=%s)", port->hba->ldapsearchattribute, ldapuser); else - filter = psprintf("(uid=%s)", port->user_name); + filter = psprintf("(uid=%s)", ldapuser); r = ldap_search_s(ldap, port->hba->ldapbasedn, @@ -2621,12 +2628,12 @@ CheckLDAPAuth(Port *port) { if (count == 0) ereport(LOG, - (errmsg("LDAP user \"%s\" does not exist", port->user_name), + (errmsg("LDAP user \"%s\" does not exist", ldapuser), errdetail("LDAP search for filter \"%s\" on server \"%s\" returned no entries.", filter, server_name))); else ereport(LOG, - (errmsg("LDAP user \"%s\" is not unique", port->user_name), + (errmsg("LDAP user \"%s\" is not unique", ldapuser), errdetail_plural("LDAP search for filter \"%s\" on server \"%s\" returned %d entry.", "LDAP search for filter \"%s\" on server \"%s\" returned %d entries.", count, @@ -2691,7 +2698,7 @@ CheckLDAPAuth(Port *port) else fulluser = psprintf("%s%s%s", port->hba->ldapprefix ? port->hba->ldapprefix : "", - port->user_name, + ldapuser, port->hba->ldapsuffix ? port->hba->ldapsuffix : ""); r = ldap_simple_bind_s(ldap, fulluser, passwd); @@ -2713,9 +2720,22 @@ CheckLDAPAuth(Port *port) ldap_unbind(ldap); pfree(passwd); + + /* + * Check the usermap for authorization. If ldap_map_dn is set and the admin + * has set up an explicit map, we use the full distinguised name during the + * mapping. Otherwise we just check the bare LDAP username. + */ + map_name = ldapuser; + if (port->hba->usermap && port->hba->usermap[0] && port->hba->ldap_map_dn) + map_name = fulluser; + + r = check_usermap(port->hba->usermap, port->user_name, map_name, + pg_krb_caseins_users); + pfree(fulluser); - return STATUS_OK; + return r; } /* diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 3be8778d21..da9e1ae509 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -1596,6 +1596,24 @@ parse_hba_line(TokenizedLine *tok_line, int elevel) *err_msg = "cannot use ldapsearchattribute together with ldapsearchfilter"; return NULL; } + + /* + * Setting ldap_map_dn=1 tells the server to use the full DN when + * mapping users, but it doesn't do anything if there's no usermap + * defined. To prevent confusion, don't allow this to be set without a + * map. + */ + if (parsedline->ldap_map_dn + && !(parsedline->usermap && parsedline->usermap[0])) + { + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("authentication option \"ldap_map_dn\" requires argument \"map\" to be set"), + errcontext("line %d of configuration file \"%s\"", + line_num, HbaFileName))); + *err_msg = "authentication option \"ldap_map_dn\" requires argument \"map\" to be set"; + return NULL; + } } if (parsedline->auth_method == uaRADIUS) @@ -1713,8 +1731,9 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, hbaline->auth_method != uaPeer && hbaline->auth_method != uaGSS && hbaline->auth_method != uaSSPI && + hbaline->auth_method != uaLDAP && hbaline->auth_method != uaCert) - INVALID_AUTH_OPTION("map", gettext_noop("ident, peer, gssapi, sspi, and cert")); + INVALID_AUTH_OPTION("map", gettext_noop("ident, peer, gssapi, sspi, ldap, and cert")); hbaline->usermap = pstrdup(val); } else if (strcmp(name, "clientcert") == 0) @@ -1931,6 +1950,14 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaLDAP, "ldapsuffix", "ldap"); hbaline->ldapsuffix = pstrdup(val); } + else if (strcmp(name, "ldap_map_dn") == 0) + { + REQUIRE_AUTH_OPTION(uaLDAP, "ldap_map_dn", "ldap"); + if (strcmp(val, "1") == 0) + hbaline->ldap_map_dn = true; + else + hbaline->ldap_map_dn = false; + } else if (strcmp(name, "krb_realm") == 0) { if (hbaline->auth_method != uaGSS && diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 9c2c98614a..6a1c1249d6 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -2165,6 +2165,8 @@ retry1: port->database_name = pstrdup(valptr); else if (strcmp(nameptr, "user") == 0) port->user_name = pstrdup(valptr); + else if (strcmp(nameptr, "ldapuser") == 0) + port->ldapuser = pstrdup(valptr); else if (strcmp(nameptr, "options") == 0) port->cmdline_options = pstrdup(valptr); else if (strcmp(nameptr, "replication") == 0) diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index 8d9f3821b1..8729056f09 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -106,6 +106,7 @@ typedef struct HbaLine int ldapscope; char *ldapprefix; char *ldapsuffix; + bool ldap_map_dn; ClientCertMode clientcert; ClientCertName clientcertname; char *krb_realm; diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 02015efe13..26f764aae9 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -203,6 +203,11 @@ typedef struct Port void *gss; #endif + /* + * LDAP structures. + */ + const char *ldapuser; + /* * SSL structures. */ diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 49eec3e835..a94a0fe47b 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -344,6 +344,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "Target-Session-Attrs", "", 15, /* sizeof("prefer-standby") = 15 */ offsetof(struct pg_conn, target_session_attrs)}, + {"ldapuser", "PGLDAPUSER", NULL, NULL, + "LDAP-User", "", 20, + offsetof(struct pg_conn, pgldapuser)}, + /* Terminating entry --- MUST BE LAST */ {NULL, NULL, NULL, NULL, NULL, NULL, 0} @@ -1436,6 +1440,8 @@ connectOptions2(PGconn *conn) goto oom_error; } + /* TODO: unset pgldapuser if it's the same as pguser, for compatibility? */ + /* * Only if we get this far is it appropriate to try to connect. (We need a * state flag, rather than just the boolean result of this function, in diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index 9ab3bf1fcb..42e4e535ea 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -2220,6 +2220,8 @@ build_startup_packet(const PGconn *conn, char *packet, ADD_STARTUP_OPTION("replication", conn->replication); if (conn->pgoptions && conn->pgoptions[0]) ADD_STARTUP_OPTION("options", conn->pgoptions); + if (conn->pgldapuser && conn->pgldapuser[0]) + ADD_STARTUP_OPTION("ldapuser", conn->pgldapuser); if (conn->send_appname) { /* Use appname if present, otherwise use fallback */ diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 490458adef..ea8e23da08 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -390,6 +390,7 @@ struct pg_conn char *krbsrvname; /* Kerberos service name */ char *gsslib; /* What GSS library to use ("gssapi" or * "sspi") */ + char *pgldapuser; /* LDAP username, if not pguser */ char *ssl_min_protocol_version; /* minimum TLS protocol version */ char *ssl_max_protocol_version; /* maximum TLS protocol version */ char *target_session_attrs; /* desired session properties */ diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl index 1d1282f8dc..01b55c9616 100644 --- a/src/test/ldap/t/001_auth.pl +++ b/src/test/ldap/t/001_auth.pl @@ -9,7 +9,7 @@ use Test::More; if ($ENV{with_ldap} eq 'yes') { - plan tests => 28; + plan tests => 60; } else { @@ -206,6 +206,77 @@ test_access( qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/ ],); +note "ident mapping"; + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf('pg_hba.conf', + qq{local all all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapprefix="uid=" ldapsuffix=",dc=example,dc=net" map=mymap} +); +$node->restart; + +$ENV{"PGPASSWORD"} = 'secret1'; +test_access( + $node, 'test1', 2, + 'fails without mapping', + log_like => [ + qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/, + qr/no match in usermap "mymap" for user "test1"/, + ]); + +$node->append_conf('pg_ident.conf', qq{mymap /^ test1}); +$node->restart; + +test_access( + $node, 'test1', 0, + 'succeeds with mapping', + log_like => [ + qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/, + ]); + +$ENV{"PGPASSWORD"} = 'secret2'; +test_access( + $node, 'test2', 2, + 'fails with unmapped role', + log_like => [ + qr/connection authenticated: identity="uid=test2,dc=example,dc=net" method=ldap/, + qr/no match in usermap "mymap" for user "test2" authenticated as "test2"/, + ]); + +$ENV{"PGLDAPUSER"} = 'test2'; +test_access( + $node, 'test1', 0, + 'succeeds with different PGLDAPUSER', + log_like => [ + qr/connection authenticated: identity="uid=test2,dc=example,dc=net" method=ldap/, + qr/connection authorized: user=test1/, + ]); +delete $ENV{"PGLDAPUSER"}; + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf('pg_hba.conf', + qq{local all all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapprefix="uid=" ldapsuffix=",dc=example,dc=net" map=mymap ldap_map_dn=1} +); +unlink($node->data_dir . '/pg_ident.conf'); +$node->append_conf('pg_ident.conf', qq{mymap "/,dc=example,dc=net\$" test1}); +$node->restart; + +$ENV{"PGPASSWORD"} = 'secret1'; +test_access( + $node, 'test1', 0, + 'succeeds with full DN mapping', + log_like => [ + qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/, + ]); + +$ENV{"PGPASSWORD"} = 'secret2'; +test_access( + $node, 'test2', 2, + 'fails with unmapped role for full DN mapping', + log_like => [ + qr/connection authenticated: identity="uid=test2,dc=example,dc=net" method=ldap/, + qr/no match in usermap "mymap" for user "test2" authenticated as "uid=test2,dc=example,dc=net"/, + ]); + note "search+bind"; unlink($node->data_dir . '/pg_hba.conf'); @@ -227,6 +298,78 @@ test_access( qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/ ],); +note "search+bind ident mapping"; + +unlink($node->data_dir . '/pg_ident.conf'); +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" map=mymap} +); +$node->restart; + +$ENV{"PGPASSWORD"} = 'secret1'; +test_access( + $node, 'test1', 2, + 'fails without mapping', + log_like => [ + qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/, + qr/no match in usermap "mymap" for user "test1" authenticated as "test1"/, + ]); + +$node->append_conf('pg_ident.conf', qq{mymap /^ test1}); +$node->restart; + +test_access( + $node, 'test1', 0, + 'succeeds with mapping', + log_like => [ + qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/, + ]); + +$ENV{"PGPASSWORD"} = 'secret2'; +test_access( + $node, 'test2', 2, + 'fails with unmapped role', + log_like => [ + qr/connection authenticated: identity="uid=test2,dc=example,dc=net" method=ldap/, + qr/no match in usermap "mymap" for user "test2" authenticated as "test2"/, + ]); + +$ENV{"PGLDAPUSER"} = 'test2'; +test_access( + $node, 'test1', 0, + 'succeeds with different PGLDAPUSER', + log_like => [ + qr/connection authenticated: identity="uid=test2,dc=example,dc=net" method=ldap/, + qr/connection authorized: user=test1/, + ]); +delete $ENV{"PGLDAPUSER"}; + +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" map=mymap ldap_map_dn=1} +); +unlink($node->data_dir . '/pg_ident.conf'); +$node->append_conf('pg_ident.conf', qq{mymap "/,dc=example,dc=net\$" test1}); +$node->restart; + +$ENV{"PGPASSWORD"} = 'secret1'; +test_access( + $node, 'test1', 0, + 'succeeds with full DN mapping', + log_like => [ + qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/, + ]); + +$ENV{"PGPASSWORD"} = 'secret2'; +test_access( + $node, 'test2', 2, + 'fails with unmapped role for full DN mapping', + log_like => [ + qr/connection authenticated: identity="uid=test2,dc=example,dc=net" method=ldap/, + qr/no match in usermap "mymap" for user "test2" authenticated as "uid=test2,dc=example,dc=net"/, + ]); + note "multiple servers"; unlink($node->data_dir . '/pg_hba.conf'); -- 2.25.1