From d66e68207e998c9b6180bee5fb55019f35fdacf4 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 13 Nov 2019 13:41:04 -0500 Subject: [PATCH] Avoid downcasing/truncation of RADIUS authentication parameters. Commit 6b76f1bb5 changed all the RADIUS auth parameters to be lists rather than single values. But its use of SplitIdentifierString to parse the list format was not very carefully thought through, because that function thinks it's parsing SQL identifiers, which means it will (a) downcase the strings and (b) truncate them to be shorter than NAMEDATALEN. While downcasing should be harmless for the server names and ports, it's just wrong for the shared secrets, and probably for the NAS Identifier strings as well. The truncation aspect is at least potentially a problem too, though typical values for these parameters would fit in 63 bytes. Fortunately, we now have a function SplitGUCList that is exactly the same except for not doing the two unwanted things, so fixing this is a trivial matter of calling that function instead. While here, improve the documentation to show how to double-quote the parameter values. I failed to resist the temptation to do some copy-editing as well. Report and patch from Marcos David (bug #16106); doc changes by me. Back-patch to v10 where the aforesaid commit came in, since this is arguably a regression from our previous behavior with RADIUS auth. Discussion: https://postgr.es/m/16106-7d319e4295d08e70@postgresql.org --- doc/src/sgml/client-auth.sgml | 39 ++++++++++++++++++++++------------- src/backend/libpq/hba.c | 8 +++---- 2 files changed, 29 insertions(+), 18 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 1e456e7796..fd26beb5e9 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -1751,7 +1751,7 @@ host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapse user name, password (encrypted) and NAS Identifier. The request will be encrypted using a secret shared with the server. The RADIUS server will respond to - this server with either Access Accept or + this request with either Access Accept or Access Reject. There is no support for RADIUS accounting. @@ -1760,11 +1760,11 @@ host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapse be tried sequentially. If a negative response is received from a server, the authentication will fail. If no response is received, the next server in the list will be tried. To specify multiple - servers, put the names within quotes and separate the server names - with a comma. If multiple servers are specified, all other RADIUS - options can also be given as a comma separate list, to apply - individual values to each server. They can also be specified as - a single value, in which case this value will apply to all servers. + servers, separate the server names with commas and surround the list + with double quotes. If multiple servers are specified, the other + RADIUS options can also be given as comma-separated lists, to provide + individual values for each server. They can also be specified as + a single value, in which case that value will apply to all servers. @@ -1774,7 +1774,7 @@ host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapse radiusservers - The name or IP addresses of the RADIUS servers to connect to. + The DNS names or IP addresses of the RADIUS servers to connect to. This parameter is required. @@ -1785,7 +1785,7 @@ host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapse The shared secrets used when talking securely to the RADIUS - server. This must have exactly the same value on the PostgreSQL + servers. This must have exactly the same value on the PostgreSQL and RADIUS servers. It is recommended that this be a string of at least 16 characters. This parameter is required. @@ -1805,8 +1805,9 @@ host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapse radiusports - The port number on the RADIUS servers to connect to. If no port - is specified, the default port 1812 will be used. + The port numbers to connect to on the RADIUS servers. If no port + is specified, the default RADIUS port (1812) + will be used. @@ -1815,10 +1816,10 @@ host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapse radiusidentifiers - The string used as NAS Identifier in the RADIUS - requests. This parameter can be used as a second parameter - identifying for example which database user the user is attempting - to authenticate as, which can be used for policy matching on + The strings to be used as NAS Identifier in the + RADIUS requests. This parameter can be used, for example, to + identify which database cluster the user is attempting to connect + to, which can be useful for policy matching on the RADIUS server. If no identifier is specified, the default postgresql will be used. @@ -1827,6 +1828,16 @@ host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapse + + + If it is necessary to have a comma or whitespace in a RADIUS parameter + value, that can be done by putting double quotes around the value, but + it is tedious because two layers of double-quoting are now required. + An example of putting whitespace into RADIUS secret strings is: + +host ... radius radiusservers="server1,server2" radiussecrets="""secret one"",""secret two""" + + diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 0129dd24d0..95569cf878 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -1881,7 +1881,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaRADIUS, "radiusservers", "radius"); - if (!SplitIdentifierString(dupval, ',', &parsed_servers)) + if (!SplitGUCList(dupval, ',', &parsed_servers)) { /* syntax error in list */ ereport(elevel, @@ -1930,7 +1930,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaRADIUS, "radiusports", "radius"); - if (!SplitIdentifierString(dupval, ',', &parsed_ports)) + if (!SplitGUCList(dupval, ',', &parsed_ports)) { ereport(elevel, (errcode(ERRCODE_CONFIG_FILE_ERROR), @@ -1965,7 +1965,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaRADIUS, "radiussecrets", "radius"); - if (!SplitIdentifierString(dupval, ',', &parsed_secrets)) + if (!SplitGUCList(dupval, ',', &parsed_secrets)) { /* syntax error in list */ ereport(elevel, @@ -1987,7 +1987,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaRADIUS, "radiusidentifiers", "radius"); - if (!SplitIdentifierString(dupval, ',', &parsed_identifiers)) + if (!SplitGUCList(dupval, ',', &parsed_identifiers)) { /* syntax error in list */ ereport(elevel,