From c2fbf9424561b5e3e9d20fda245c321f97c8f818 Mon Sep 17 00:00:00 2001 From: Andrew Jackson Date: Sat, 28 Mar 2026 23:29:48 -0500 Subject: [PATCH] Add ldapservice connection parameter Currently there exists, only in pg_service.conf, the ability to look up connection parameters from a centralized LDAP server. This patch expands the usability of this be allowing it to be specified directly in a connection string instead of only in a pg_service.conf file. This adds the PGLDAPSERVICE env var that provides an envvar interface to this functionality. Also the functionality has been moved to conninfo_add_defaults after review. Also 2 tests have been added. One to validate the env var functionality and one to validate that this parameter is ignored when used in pg_service.conf files. -- v5 patch changes This patch changes the relevant naming from ldapservice to ldapserviceurl. I also add a comment above the new ldapservicelookup call that explains the reasoning for ignoring all non zero return codes. Also I increase the value of dispsize for ldapserviceurl from 20 to 64. It could be reasonable to increase it past 64, was originally thinking about bumping it to 1024 but I am not sure what these values are used for in the codebase and thought it was safer to just bump it to the currently existing max for any parameter. Moves the placement of ldapserviceurl to below the GSS options and adds a comment explaining that the parameter is exposed even in non LDAP builds. This was done to reflect similar comments in GSS/SSL, though this comment does not exist for the new OAUTH parameters so this may not have been a good choice. Also Add filename tags to filename reference in docs -- v6 patch changes - Updated libpq.sgml with additional verbiage around corner cases interactions between different parameter settings, etc - Changed existing guard in service file parse from "ldap" to "ldap:" to accomodate new connection parameter that begin with "ldap" - Add error raising for ldapserviceurl found during pg_service.conf parse - add libpq append error message - Expanded unit tests - Added 6 new LDAP entries for different test scenarios - Ensured that we are asserting expected_stdout for every test - Added set of unit tests that cover error conditions in ldapServiceLookup that are currently not tested in postgres codebase - Added 5 new tests that cover corner cases of new ldapserviceurl functionality --- doc/src/sgml/libpq.sgml | 43 +++- src/interfaces/libpq/fe-connect.c | 43 +++- src/interfaces/libpq/libpq-int.h | 1 + .../t/003_ldap_connection_param_lookup.pl | 229 ++++++++++++++++++ 4 files changed, 309 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 6db823808fc..01308321020 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -2350,6 +2350,19 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + + ldapserviceurl + + + This option specifies an LDAP query that can be used to reference connection parameters + stored on an LDAP server. Any connection parameter that is looked up in this way is + overridden by explicitly named connection parameters or environment variables. This + option cannot be specified in a pg_service.conf file. This + functionality is described in more detail in . + + + + target_session_attrs @@ -9170,6 +9183,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) + + + + PGLDAPSERVICEURL + + PGLDAPSERVICEURL behaves the same as the + connection parameter. + + + @@ -9659,12 +9682,20 @@ user=admin LDAP connection parameter lookup uses the connection service file pg_service.conf (see ). A line in a - pg_service.conf stanza that starts with - ldap:// will be recognized as an LDAP URL and an - LDAP query will be performed. The result must be a list of - keyword = value pairs which will be used to set - connection options. The URL must conform to + linkend="libpq-pgservice"/>) or the + connection parameter. + A line in a pg_service.conf stanza that starts with + ldap:// or an explicitly provided + connection parameter + will be recognized as an LDAP URL and an LDAP query will be performed. + Please note that if both methods are used the in conjunction any parameter + found via will take precedence + over results found in the lookup from the LDAP URL specified in + pg_service.conf. Also please note that any + value + that is found during an LDAP lookup will be ignored. The result of an LDAP lookup + must be a list of keyword = value pairs which will be used to + set connection options. The URL must conform to RFC 1959 and be of the form diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index db9b4c8edbf..95fa2f8fdc5 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -376,6 +376,15 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "GSS-delegation", "", 1, offsetof(struct pg_conn, gssdelegation)}, + /* + * As with SSL and GSS options, ldapserviceurl is exposed even in builds + * that do not have support + */ + {"ldapserviceurl", "PGLDAPSERVICEURL", NULL, NULL, + "Database-LDAP-Service", "", 64, + offsetof(struct pg_conn, pgldapserviceurl)}, + + {"replication", NULL, NULL, NULL, "Replication", "D", 5, offsetof(struct pg_conn, replication)}, @@ -6134,7 +6143,10 @@ parseServiceFile(const char *serviceFile, bool found_keyword; #ifdef USE_LDAP - if (strncmp(line, "ldap", 4) == 0) + /* + * Is this a potential ldapurl or a ldapserviceurl parameter? + */ + if (strncmp(line, "ldap:", 5) == 0) { int rc = ldapServiceLookup(line, options, errorMessage); @@ -6176,6 +6188,16 @@ parseServiceFile(const char *serviceFile, goto exit; } + if (strcmp(key, "ldapserviceurl") == 0) + { + libpq_append_error(errorMessage, + "ldapserviceurl parameters are not supported in service file \"%s\", line %d", + serviceFile, + linenr); + result = 3; + goto exit; + } + if (strcmp(key, "servicefile") == 0) { libpq_append_error(errorMessage, @@ -6726,6 +6748,25 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage) PQconninfoOption *sslmode_default = NULL, *sslrootcert = NULL; char *tmp; +#ifdef USE_LDAP + const char *ldapserviceurl = conninfo_getval(options, "ldapserviceurl"); + + if (ldapserviceurl == NULL) + ldapserviceurl = getenv("PGLDAPSERVICEURL"); + + if (ldapserviceurl != NULL) + { + + /* + * ldapServiceLookup has 4 potential return values. We only care here + * if it succeeded, if it failed we dont care why, return failure. + */ + if (ldapServiceLookup(ldapserviceurl, options, errorMessage) != 0){ + libpq_append_error(errorMessage, "ldapserviceurl lookup failed"); + return false; + } + } +#endif /* * If there's a service spec, use it to obtain any not-explicitly-given diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index bd7eb59f5f8..f8b10635d41 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -392,6 +392,7 @@ struct pg_conn char *pgservice; /* Postgres service, if any */ char *pgservicefile; /* path to a service file containing * service(s) */ + char *pgldapserviceurl; /* Postgres LDAP service URL, if any */ char *pguser; /* Postgres username and password, if any */ char *pgpass; char *pgpassfile; /* path to a file containing password(s) */ diff --git a/src/test/ldap/t/003_ldap_connection_param_lookup.pl b/src/test/ldap/t/003_ldap_connection_param_lookup.pl index 359fc7a998a..a9469fe011f 100644 --- a/src/test/ldap/t/003_ldap_connection_param_lookup.pl +++ b/src/test/ldap/t/003_ldap_connection_param_lookup.pl @@ -56,6 +56,85 @@ changetype:add objectclass:top objectclass:device cn:mydatabase +description:application_name=application_name_from_ldap +description:host=} . $node->host . qq{ +description:port=} . $node->port . qq{ +}); + +# Valid ldap record with geqo and application_name set +# to test how parameters are set/overridden +append_to_file( + $ldif_valid, qq{ +version:1 +dn:cn=mydatabasegeqooff,dc=example,dc=net +changetype:add +objectclass:top +objectclass:device +cn:mydatabasegeqooff +description:application_name=application_name_from_database2 +description:options=--geqo=off +}); + +# Invalid LDAP record with no entries +append_to_file( + $ldif_valid, qq{ +version:1 +dn:cn=noentries,dc=example,dc=net +changetype:add +objectclass:top +objectclass:device +cn:noentries +}); + +# Invalid LDAP record with missing = after application_name +append_to_file( + $ldif_valid, qq{ +version:1 +dn:cn=missingequal,dc=example,dc=net +changetype:add +objectclass:top +objectclass:device +cn:missingequal +description:application_name +}); + +# Invalid LDAP record with non existent parameter +append_to_file( + $ldif_valid, qq{ +version:1 +dn:cn=invalidconnoptionservice,dc=example,dc=net +changetype:add +objectclass:top +objectclass:device +cn:invalidconnoptionservice +description:invalidconnoption=1 +}); + +# Invalid LDAP record with missing = in options parameter +append_to_file( + $ldif_valid, qq{ +version:1 +dn:cn=pgoptionmissingequals,dc=example,dc=net +changetype:add +objectclass:top +objectclass:device +cn:pgoptionmissingequals +description:options=--geqo off +description:host=} . $node->host . qq{ +description:port=} . $node->port . qq{ +}); + +# Valid LDAP record which has ldapserviceurl parameter +append_to_file( + $ldif_valid, qq{ +version:1 +dn:cn=ldapurlinldapurl,dc=example,dc=net +changetype:add +objectclass:top +objectclass:device +cn:ldapurlinldapurl +description:ldapserviceurl=ldap://localhost:1234/dc=example,dc=net?description?one?(cn=mydatabasegeqooff) +description:application_name=application_name_from_ldapurlinldapurl description:host=} . $node->host . qq{ description:port=} . $node->port . qq{ }); @@ -80,6 +159,16 @@ append_to_file( $srvfile_valid, qq{ [my_srv] ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase) + +[my_srv_geqo_off] +ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabasegeqooff) + +[ldapserviceurl_in_pg_service_test] +ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase) + +[pg_service_application_name] +application_name=pg_service +options=--geqo=off }); # File defined with no contents, used as default value for @@ -196,10 +285,150 @@ local $ENV{PGSERVICEFILE} = "$srvfile_empty"; expected_stdout => qr/definition of service "undefined-service" not found/); + delete $ENV{PGSERVICE}; + + unlink($srvfile_default); +} + +# Test ldapserviceurl connection parameter +{ + # Create copy of valid file + my $srvfile_default = "$td/pg_service.conf"; + copy($srvfile_valid, $srvfile_default); + + $dummy_node->connect_ok( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase)", + 'connection with correct "ldapserviceurl"', + sql => "SELECT 'connect2_4', current_setting('application_name')", + expected_stdout => qr/connect2_4\|application_name_from_ldap/); + + $dummy_node->connect_ok( + "postgres://?ldapserviceurl=ldap%3A%2F%2Flocalhost%3A$ldap_port%2Fdc%3Dexample%2Cdc%3Dnet%3Fdescription%3Fone%3F%28cn%3Dmydatabase%29", + 'connection with correct "ldapserviceurl" in uri format', + sql => "SELECT 'connect2_5', current_setting('application_name')", + expected_stdout => qr/connect2_5\|application_name_from_ldap/); + + local $ENV{PGLDAPSERVICEURL} = "ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase)"; + + $dummy_node->connect_ok( + "", + 'connection with correct "ldapserviceurl" provided by env var', + sql => "SELECT 'connect2_6', current_setting('application_name')", + expected_stdout => qr/connect2_6\|application_name_from_ldap/); + + $dummy_node->connect_fails( + 'ldapserviceurl=ldap://localhost:invalidport/dc=example,dc=net?description?one?(cn=mydatabase)', + 'connection fails with because valid PGLDAPSERVICEURL env var overridden by explicit parameter with bad port', + expected_stderr => qr/invalid LDAP URL .*: invalid port number/); + + delete $ENV{PGLDAPSERVICEURL}; + + $dummy_node->connect_ok( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase) application_name=explicit_parameter", + 'connection with correct "ldapservice" but with application_name overridden by explicit connection parameter', + sql => "SELECT 'connect2_7', current_setting('application_name')", + expected_stdout => qr/connect2_7\|explicit_parameter/); + + $dummy_node->connect_ok( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase) service=pg_service_application_name", + 'connection with correct "ldapservice" string, service file application_name overridden by ldap, geqo off set by service file', + sql => "SELECT 'connect2_8', current_setting('application_name'), current_setting('geqo');", + expected_stdout => qr/connect2_8\|application_name_from_ldap\|off/); + + # test that geqo grabbed from service file service my_srv_geqo_off via LDAP lookup + # conflicting application names present in both LDAP services queried (ldapserviceurl and service file) + # application name from service file is ignored because it has already been set by ldapserviceurl lookup + $dummy_node->connect_ok( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase) service=my_srv_geqo_off", + 'connection with 2 LDAP lookups (pg_service.conf and ldapserviceurl)', + sql => "SELECT 'connect2_9', current_setting('application_name'), current_setting('geqo');", + expected_stdout => qr/connect2_9\|application_name_from_ldap\|off/); + + $dummy_node->connect_fails( + 'service=ldapserviceurl_in_pg_service_test', + 'connection fails with ldapserviceurl specified in pg_service.conf file', + expected_stderr => qr/ldapserviceurl parameters are not supported in service file .*, line .*/ ); + + $dummy_node->connect_ok( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=ldapurlinldapurl)", + 'ldapserviceurl that points to another ldapserviceurl will be ignored', + sql => "SELECT 'connect2_10', current_setting('application_name'), current_setting('geqo');", + expected_stdout=> qr/connect2_10\|application_name_from_ldapurlinldapurl\|on/); + # Remove default pg_service.conf. unlink($srvfile_default); } +# negative tests for logic inside ldapServiceLookup +{ + + $dummy_node->connect_fails( + "ldapserviceurl=http://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase)", + 'ldapserviceurl that doesnt begin with ldap:// fails', + expected_stderr => qr/invalid LDAP URL .*: scheme must be ldap:\/\//); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:$ldap_port/", + 'ldapserviceurl that does not specify distinguished name', + expected_stderr => qr/invalid LDAP URL .*: missing distinguished name/); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?", + 'ldapserviceurl that does not specify attribute', + expected_stderr => qr/invalid LDAP URL .*: must have exactly one attribute/); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description", + 'ldapserviceurl that does not provide scope', + expected_stderr => qr/invalid LDAP URL .*: must have search scope \(base\/one\/sub\)/); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one", + 'ldapserviceurl that does not specify filter', + expected_stderr => qr/invalid LDAP URL .*: no filter/); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:invalidportnumber/dc=example,dc=net?description?one?(cn=mydatabase)", + 'ldapserviceurl contains invalid port number', + expected_stderr => qr/invalid LDAP URL .*: invalid port number/); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description,dn?one?(cn=mydatabase)", + 'ldapserviceurl that contains 2 attributes', + expected_stderr => qr/invalid LDAP URL .*: must have exactly one attribute/); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?invalidscope?(cn=mydatabase)", + 'ldapserviceurl that does not provide valid scope', + expected_stderr => qr/invalid LDAP URL .*: must have search scope \(base\/one\/sub\)/); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?sub?(cn=*)", + 'ldapserviceurl that contains more than one entry', + expected_stderr => qr/more than one entry found on LDAP lookup/); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=doesnotexist)", + 'ldapserviceurl with no ldap entry', + expected_stderr => qr/no entry found on LDAP lookup/); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=noentries)", + 'ldapserviceurl that has no connection attributes', + expected_stderr => qr/attribute has no values on LDAP lookup/); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=missingequal)", + 'ldapservicesurl that has missing equal sign', + expected_stderr => qr/missing \"\=\" after \"application_name\n\" in connection info string/); + + $dummy_node->connect_fails( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=invalidconnoptionservice)", + 'ldapserviceurl that contains nonexistent connection option', + expected_stderr => qr/invalid connection option \"invalidconnoption\"/); + +} + $node->teardown_node; done_testing(); -- 2.51.2