ProcessStartupPacket(): database_name and user_name truncation

Started by Drouvot, Bertrandover 2 years ago18 messages
#1Drouvot, Bertrand
bertranddrouvot.pg@gmail.com
1 attachment(s)

Hi hackers,

Please find attached a patch to truncate (in ProcessStartupPacket())
the port->database_name and port->user_name in such a way to not break
multibyte character boundary.

Indeed, currently, one could create a database that way:

postgres=# create database ääääääääääääääääääääääääääääääää;
NOTICE: identifier "ääääääääääääääääääääääääääääääää" will be truncated to "äääääääääääääääääääääääääääääää"
CREATE DATABASE

The database name has been truncated from 64 bytes to 62 bytes thanks to pg_mbcliplen()
which ensures to not break multibyte character boundary.

postgres=# select datname, OCTET_LENGTH(datname),encoding from pg_database;
datname | octet_length | encoding
---------------------------------+--------------+----------
äääääääääääääääääääääääääääääää | 62 | 6

Trying to connect with the 64 bytes name:

$ psql -d ääääääääääääääääääääääääääääääää
psql: error: connection to server on socket "/tmp/.s.PGSQL.55448" failed: FATAL: database "äääääääääääääääääääääääääääääää" does not exist

It fails because the truncation done in ProcessStartupPacket():

"
if (strlen(port→database_name) >= NAMEDATALEN)
port→database_name[NAMEDATALEN - 1] = '\0';
"

does not take care about multibyte character boundary.

On the other hand it works with non multibyte character involved:

postgres=# create database abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijke;
NOTICE: identifier "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijke" will be truncated to "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijk"
CREATE DATABASE

postgres=# select datname, OCTET_LENGTH(datname),encoding from pg_database;
datname | octet_length | encoding
-----------------------------------------------------------------+--------------+----------
abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijk | 63 | 6

The database name is truncated to 63 bytes and then using the 64 bytes name would work:

$ psql -d abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijke
psql (16beta1)
Type "help" for help.

The comment in ProcessStartupPacket() states:

"
/*
* Truncate given database and user names to length of a Postgres name.
* This avoids lookup failures when overlength names are given.
*/
"

The last sentence is not right in case of mutlibyte character (as seen
in the first example).

About the patch:

As the database encoding is not known yet in ProcessStartupPacket() (
and we are even not sure the database provided does exist), the proposed
patch does not rely on pg_mbcliplen() but on pg_encoding_mbcliplen().

The proposed patch does use the client encoding that it retrieves that way:

- use the one requested in the startup packet (if we come across it)
- use the one from the locale (if we did not find a client encoding request
in the startup packet)
- use PG_SQL_ASCII (if none of the above have been satisfied)

Happy to discuss any other thoughts or suggestions if any.

With the proposed patch in place, using the first example above (and the
64 bytes name) we would get:

$ PGCLIENTENCODING=LATIN1 psql -d ääääääääääääääääääääääääääääääää
psql: error: connection to server on socket "/tmp/.s.PGSQL.55448" failed: FATAL: database "äääääääääääääääääääääääääääääää" does not exist

but this one would allow us to connect:

$ PGCLIENTENCODING=UTF8 psql -d ääääääääääääääääääääääääääääääää
psql (16beta1)
Type "help" for help.

The patch does not provide documentation update or related TAP test (but could be added
if we feel the need).

Looking forward to your feedback,

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachments:

v1-0001-multibyte-truncation-for-database-and-user-name.patchtext/plain; charset=UTF-8; name=v1-0001-multibyte-truncation-for-database-and-user-name.patchDownload
From 6ccb4f03f56dda9e9a2616c6e0875a97a8711d72 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Tue, 20 Jun 2023 10:26:16 +0000
Subject: [PATCH v1] multibyte truncation for database and user name

database_name and user_name truncation done in ProcessStartupPacket() do not
take care of multibyte character boundary. In case of multibyte, this somehow
breaks the initial goal to avoid lookup failures when overlength names are given.
---
 src/backend/postmaster/postmaster.c | 48 ++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 4 deletions(-)
 100.0% src/backend/postmaster/

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 4c49393fc5..72991c4eab 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1951,6 +1951,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 	char	   *buf;
 	ProtocolVersion proto;
 	MemoryContext oldcontext;
+	int			client_encoding = -1;
 
 	pq_startmsgread();
 
@@ -2238,6 +2239,18 @@ retry1:
 				{
 					port->application_name = pg_clean_ascii(valptr, 0);
 				}
+				/* Record the client_encoding if we come across it */
+				else if (strcmp(nameptr, "client_encoding") == 0)
+				{
+					client_encoding = pg_valid_client_encoding(valptr);
+
+					if (client_encoding < 0)
+						ereport(FATAL,
+								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+								 errmsg("invalid value for parameter \"%s\": \"%s\"",
+										"client_encoding",
+										valptr)));
+				}
 			}
 			offset = valoffset + strlen(valptr) + 1;
 		}
@@ -2291,13 +2304,40 @@ retry1:
 	}
 
 	/*
-	 * Truncate given database and user names to length of a Postgres name.
-	 * This avoids lookup failures when overlength names are given.
+	 * Given the client encoding, truncate given database and user names to
+	 * length of a Postgres name. This avoids lookup failures when overlength
+	 * names are given.
 	 */
+	if (client_encoding < 0)
+	{
+		/*
+		 * client_encoding has not been found in the startup packet so let's
+		 * try to get it from locale.
+		 */
+		client_encoding = pg_get_encoding_from_locale(NULL, true);
+
+		if (client_encoding < 0)
+			client_encoding = PG_SQL_ASCII;
+	}
+
 	if (strlen(port->database_name) >= NAMEDATALEN)
-		port->database_name[NAMEDATALEN - 1] = '\0';
+	{
+		int			newlen;
+
+		newlen = pg_encoding_mbcliplen(client_encoding, port->database_name,
+									   strlen(port->database_name),
+									   NAMEDATALEN - 1);
+		port->database_name[newlen] = '\0';
+	}
 	if (strlen(port->user_name) >= NAMEDATALEN)
-		port->user_name[NAMEDATALEN - 1] = '\0';
+	{
+		int			newlen;
+
+		newlen = pg_encoding_mbcliplen(client_encoding, port->user_name,
+									   strlen(port->user_name),
+									   NAMEDATALEN - 1);
+		port->user_name[newlen] = '\0';
+	}
 
 	if (am_walsender)
 		MyBackendType = B_WAL_SENDER;
-- 
2.34.1

#2Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Drouvot, Bertrand (#1)
Re: ProcessStartupPacket(): database_name and user_name truncation

At Wed, 21 Jun 2023 09:43:50 +0200, "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com> wrote in

Trying to connect with the 64 bytes name:

$ psql -d ääääääääääääääääääääääääääääääää
psql: error: connection to server on socket "/tmp/.s.PGSQL.55448"
failed: FATAL: database "äääääääääääääääääääääääääääääää" does not
exist

IMHO, I'm not sure we should allow connections without the exact name
being provided. In that sense, I think we might want to consider
outright rejecting the estblishment of a connection when the given
database name doesn't fit the startup packet, since the database with
the exact given name cannot be found.

While it is somewhat off-topic, I cannot establish a connection if the
console encoding differs from the template database even if I provide
the identical database name. (I don't mean I want that behavior to be
"fix"ed.)

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Kyotaro Horiguchi (#2)
Re: ProcessStartupPacket(): database_name and user_name truncation

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

At Wed, 21 Jun 2023 09:43:50 +0200, "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com> wrote in

Trying to connect with the 64 bytes name:
$ psql -d ääääääääääääääääääääääääääääääää
psql: error: connection to server on socket "/tmp/.s.PGSQL.55448"
failed: FATAL: database "äääääääääääääääääääääääääääääää" does not
exist

IMHO, I'm not sure we should allow connections without the exact name
being provided. In that sense, I think we might want to consider
outright rejecting the estblishment of a connection when the given
database name doesn't fit the startup packet, since the database with
the exact given name cannot be found.

I think I agree. I don't like the proposed patch at all, because it's
making completely unsupportable assumptions about what encoding the
names are given in. Simply failing to match when a name is overlength
sounds safer.

(Our whole story about what is the encoding of names in shared catalogs
is a mess. But this particular point doesn't seem like the place to
start if you want to clean that up.)

regards, tom lane

#4Drouvot, Bertrand
bertranddrouvot.pg@gmail.com
In reply to: Tom Lane (#3)
Re: ProcessStartupPacket(): database_name and user_name truncation

Hi,

On 6/21/23 3:43 PM, Tom Lane wrote:

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

At Wed, 21 Jun 2023 09:43:50 +0200, "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com> wrote in

Trying to connect with the 64 bytes name:
$ psql -d ääääääääääääääääääääääääääääääää
psql: error: connection to server on socket "/tmp/.s.PGSQL.55448"
failed: FATAL: database "äääääääääääääääääääääääääääääää" does not
exist

IMHO, I'm not sure we should allow connections without the exact name
being provided. In that sense, I think we might want to consider
outright rejecting the estblishment of a connection when the given
database name doesn't fit the startup packet, since the database with
the exact given name cannot be found.

I think I agree. I don't like the proposed patch at all, because it's
making completely unsupportable assumptions about what encoding the
names are given in. Simply failing to match when a name is overlength
sounds safer.

Yeah, that's another and "cleaner" option.

I'll propose a patch to make it failing even for the non multibyte case then (
so that multibyte and non multibyte behaves the same aka failing in case of overlength
name is detected).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

#5Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#3)
Re: ProcessStartupPacket(): database_name and user_name truncation

On Wed, Jun 21, 2023 at 09:43:38AM -0400, Tom Lane wrote:

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

IMHO, I'm not sure we should allow connections without the exact name
being provided. In that sense, I think we might want to consider
outright rejecting the estblishment of a connection when the given
database name doesn't fit the startup packet, since the database with
the exact given name cannot be found.

I think I agree. I don't like the proposed patch at all, because it's
making completely unsupportable assumptions about what encoding the
names are given in. Simply failing to match when a name is overlength
sounds safer.

+1. Even if these assumptions were supportable, IMHO it's probably not
worth the added complexity to keep the truncation consistent with CREATE
ROLE/DATABASE.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

#6Drouvot, Bertrand
bertranddrouvot.pg@gmail.com
In reply to: Drouvot, Bertrand (#4)
1 attachment(s)
Re: ProcessStartupPacket(): database_name and user_name truncation

On 6/21/23 4:22 PM, Drouvot, Bertrand wrote:

Hi,

On 6/21/23 3:43 PM, Tom Lane wrote:

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

At Wed, 21 Jun 2023 09:43:50 +0200, "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com> wrote in

Trying to connect with the 64 bytes name:
$ psql -d ääääääääääääääääääääääääääääääää
psql: error: connection to server on socket "/tmp/.s.PGSQL.55448"
failed: FATAL: database "äääääääääääääääääääääääääääääää" does not
exist

IMHO, I'm not sure we should allow connections without the exact name
being provided. In that sense, I think we might want to consider
outright rejecting the estblishment of a connection when the given
database name doesn't fit the startup packet, since the database with
the exact given name cannot be found.

I think I agree.  I don't like the proposed patch at all, because it's
making completely unsupportable assumptions about what encoding the
names are given in.  Simply failing to match when a name is overlength
sounds safer.

Yeah, that's another and "cleaner" option.

I'll propose a patch to make it failing even for the non multibyte case then (
so that multibyte and non multibyte behaves the same aka failing in case of overlength
name is detected).

Please find attached a patch doing so (which is basically a revert of d18c1d1f51).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachments:

v1-0001-Reject-incoming-username-and-database-name-in-cas.patchtext/plain; charset=UTF-8; name=v1-0001-Reject-incoming-username-and-database-name-in-cas.patchDownload
From 1a40e13385752ef05b9602d1040e73dbb14d0c7e Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Wed, 21 Jun 2023 18:28:13 +0000
Subject: [PATCH v1] Reject incoming username and database name in case of
 overlength

---
 src/backend/postmaster/postmaster.c | 9 ---------
 1 file changed, 9 deletions(-)
 100.0% src/backend/postmaster/

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 4c49393fc5..0b1de9efb2 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2290,15 +2290,6 @@ retry1:
 		}
 	}
 
-	/*
-	 * Truncate given database and user names to length of a Postgres name.
-	 * This avoids lookup failures when overlength names are given.
-	 */
-	if (strlen(port->database_name) >= NAMEDATALEN)
-		port->database_name[NAMEDATALEN - 1] = '\0';
-	if (strlen(port->user_name) >= NAMEDATALEN)
-		port->user_name[NAMEDATALEN - 1] = '\0';
-
 	if (am_walsender)
 		MyBackendType = B_WAL_SENDER;
 	else
-- 
2.34.1

#7Nathan Bossart
nathandbossart@gmail.com
In reply to: Drouvot, Bertrand (#6)
Re: ProcessStartupPacket(): database_name and user_name truncation

On Wed, Jun 21, 2023 at 09:02:49PM +0200, Drouvot, Bertrand wrote:

Please find attached a patch doing so (which is basically a revert of d18c1d1f51).

LGTM. I think this can wait for v17 since the current behavior has been
around since 2001 and AFAIK this is the first report. While it's arguably
a bug fix, the patch also breaks some cases that work today.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

#8Michael Paquier
michael@paquier.xyz
In reply to: Nathan Bossart (#7)
Re: ProcessStartupPacket(): database_name and user_name truncation

On Wed, Jun 21, 2023 at 12:55:15PM -0700, Nathan Bossart wrote:

LGTM. I think this can wait for v17 since the current behavior has been
around since 2001 and AFAIK this is the first report. While it's arguably
a bug fix, the patch also breaks some cases that work today.

Agreed that anything discussed on this thread does not warrant a
backpatch.
--
Michael

#9Drouvot, Bertrand
bertranddrouvot.pg@gmail.com
In reply to: Michael Paquier (#8)
Re: ProcessStartupPacket(): database_name and user_name truncation

Hi,

On 6/22/23 1:37 AM, Michael Paquier wrote:

On Wed, Jun 21, 2023 at 12:55:15PM -0700, Nathan Bossart wrote:

LGTM. I think this can wait for v17 since the current behavior has been
around since 2001 and AFAIK this is the first report. While it's arguably
a bug fix, the patch also breaks some cases that work today.

Agreed that anything discussed on this thread does not warrant a
backpatch.

Fully agree, the CF entry [1]https://commitfest.postgresql.org/43/4383/ has been tagged as "Target Version 17".

[1]: https://commitfest.postgresql.org/43/4383/

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

#10Nathan Bossart
nathandbossart@gmail.com
In reply to: Drouvot, Bertrand (#9)
Re: ProcessStartupPacket(): database_name and user_name truncation

After taking another look at this, I wonder if it'd be better to fail as
soon as we see the database or user name is too long instead of lugging
them around when authentication is destined to fail.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#10)
Re: ProcessStartupPacket(): database_name and user_name truncation

Nathan Bossart <nathandbossart@gmail.com> writes:

After taking another look at this, I wonder if it'd be better to fail as
soon as we see the database or user name is too long instead of lugging
them around when authentication is destined to fail.

If we're agreed that we aren't going to truncate these identifiers,
that seems like a reasonable way to handle it.

regards, tom lane

#12Drouvot, Bertrand
bertranddrouvot.pg@gmail.com
In reply to: Tom Lane (#11)
Re: ProcessStartupPacket(): database_name and user_name truncation

Hi,

On 6/30/23 5:54 PM, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

After taking another look at this, I wonder if it'd be better to fail as
soon as we see the database or user name is too long instead of lugging
them around when authentication is destined to fail.

If we're agreed that we aren't going to truncate these identifiers,
that seems like a reasonable way to handle it.

Yeah agree, thanks Nathan for the idea.
I'll work on a new patch version proposal.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

#13Drouvot, Bertrand
bertranddrouvot.pg@gmail.com
In reply to: Drouvot, Bertrand (#12)
1 attachment(s)
Re: ProcessStartupPacket(): database_name and user_name truncation

Hi,

On 6/30/23 7:32 PM, Drouvot, Bertrand wrote:

Hi,

On 6/30/23 5:54 PM, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

After taking another look at this, I wonder if it'd be better to fail as
soon as we see the database or user name is too long instead of lugging
them around when authentication is destined to fail.

If we're agreed that we aren't going to truncate these identifiers,
that seems like a reasonable way to handle it.

Yeah agree, thanks Nathan for the idea.
I'll work on a new patch version proposal.

Please find V2 attached where it's failing as soon as the database name or
user name are detected as overlength.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachments:

v2-0001-Reject-incoming-username-and-database-name-in-cas.patchtext/plain; charset=UTF-8; name=v2-0001-Reject-incoming-username-and-database-name-in-cas.patchDownload
From 7747032129fb66891805a8a2b5e06cbce8df2d2a Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Wed, 21 Jun 2023 18:28:13 +0000
Subject: [PATCH v2] Reject incoming username and database name in case of
 overlength

---
 src/backend/postmaster/postmaster.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)
 100.0% src/backend/postmaster/

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 4c49393fc5..03289f2093 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2183,9 +2183,25 @@ retry1:
 			valptr = buf + valoffset;
 
 			if (strcmp(nameptr, "database") == 0)
+			{
+				/* Overlength database name has been provided. */
+				if (strlen(valptr) >= NAMEDATALEN)
+					ereport(FATAL,
+							(errcode(ERRCODE_UNDEFINED_DATABASE),
+							 errmsg("database \"%s\" does not exist", valptr)));
+
 				port->database_name = pstrdup(valptr);
+			}
 			else if (strcmp(nameptr, "user") == 0)
+			{
+				/* Overlength user name has been provided. */
+				if (strlen(valptr) >= NAMEDATALEN)
+					ereport(FATAL,
+							(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+							 errmsg("role \"%s\" does not exist", valptr)));
+
 				port->user_name = pstrdup(valptr);
+			}
 			else if (strcmp(nameptr, "options") == 0)
 				port->cmdline_options = pstrdup(valptr);
 			else if (strcmp(nameptr, "replication") == 0)
@@ -2290,15 +2306,6 @@ retry1:
 		}
 	}
 
-	/*
-	 * Truncate given database and user names to length of a Postgres name.
-	 * This avoids lookup failures when overlength names are given.
-	 */
-	if (strlen(port->database_name) >= NAMEDATALEN)
-		port->database_name[NAMEDATALEN - 1] = '\0';
-	if (strlen(port->user_name) >= NAMEDATALEN)
-		port->user_name[NAMEDATALEN - 1] = '\0';
-
 	if (am_walsender)
 		MyBackendType = B_WAL_SENDER;
 	else
-- 
2.34.1

#14Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Drouvot, Bertrand (#12)
Re: ProcessStartupPacket(): database_name and user_name truncation

At Fri, 30 Jun 2023 19:32:50 +0200, "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com> wrote in

Hi,

On 6/30/23 5:54 PM, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

After taking another look at this, I wonder if it'd be better to fail
as
soon as we see the database or user name is too long instead of
lugging
them around when authentication is destined to fail.

For the record, if I understand Nathan correctly, it is what I
suggested in my initial post. If this is correct, +1 for the suggestion.

me> I think we might want to consider outright rejecting the
me> estblishment of a connection when the given database name doesn't
me> fit the startup packet

If we're agreed that we aren't going to truncate these identifiers,
that seems like a reasonable way to handle it.

Yeah agree, thanks Nathan for the idea.
I'll work on a new patch version proposal.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

#15Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Drouvot, Bertrand (#13)
Re: ProcessStartupPacket(): database_name and user_name truncation

At Mon, 03 Jul 2023 10:50:45 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in

For the record, if I understand Nathan correctly, it is what I
suggested in my initial post. If this is correct, +1 for the suggestion.

me> I think we might want to consider outright rejecting the
me> estblishment of a connection when the given database name doesn't
me> fit the startup packet

Mmm. It's bit wrong. "doesn't fit the startup packet" is "is long as a
database name".

At Sat, 1 Jul 2023 16:02:06 +0200, "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com> wrote in

Please find V2 attached where it's failing as soon as the database
name or
user name are detected as overlength.

I find another errocde "ERRCODE_INVALID_ROLE_SPECIFICATION". I don't
find a clear distinction between the usages of the two, but I think
.._ROLE_.. might be a better fit.

ERRCODE_INVALID_ROLE_SPACIFICATION:
auth.c:1507: "could not transnlate name"
auth.c:1526: "could not translate name"
auth.c:1539: "realm name too long"
auth.c:1554: "translated account name too long"

ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION:
postmaster.c:2268: "no PostgreSQL user name specified in startup packet"
miscinit.c:756: "role \"%s\" does not exist"
miscinit.c:764: "role with OID %u does not exist"
miscinit.c:794: "role \"%s\" is not permitted to log in"
auth.c:420: "connection requires a valid client certificate"
auth.c:461,468,528,536: "pg_hba.conf rejects ..."
auth.c:878: MD5 authentication is not supported when \"db_user_namespace\" is enabled"
auth-scram.c:1016: "SCRAM channel binding negotiation error"
auth-scram.c:1349: "SCRAM channel binding check failed"

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

#16Nathan Bossart
nathandbossart@gmail.com
In reply to: Drouvot, Bertrand (#13)
Re: ProcessStartupPacket(): database_name and user_name truncation

On Sat, Jul 01, 2023 at 04:02:06PM +0200, Drouvot, Bertrand wrote:

Please find V2 attached where it's failing as soon as the database name or
user name are detected as overlength.

Thanks, Bertrand. I chickened out and ended up committing v1 for now
(i.e., simply removing the truncation code). I didn't like the idea of
trying to keep the new error messages consistent with code in faraway
files, and the startup packet length limit is already pretty aggressive, so
I'm a little less concerned about lugging around long names. Plus, I think
v2 had some subtle interactions with db_user_namespace (maybe for the
better), but I didn't spend too much time looking at that since
db_user_namespace will likely be removed soon.

If anyone disagrees and wants to see the FATALs emitted from
ProcessStartupPacket() directly, please let me know and we can work on
adding them in a follow-up patch.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#16)
Re: ProcessStartupPacket(): database_name and user_name truncation

Nathan Bossart <nathandbossart@gmail.com> writes:

Thanks, Bertrand. I chickened out and ended up committing v1 for now
(i.e., simply removing the truncation code).

WFM.

If anyone disagrees and wants to see the FATALs emitted from
ProcessStartupPacket() directly, please let me know and we can work on
adding them in a follow-up patch.

I think the new behavior is fine.

regards, tom lane

#18Drouvot, Bertrand
bertranddrouvot.pg@gmail.com
In reply to: Nathan Bossart (#16)
Re: ProcessStartupPacket(): database_name and user_name truncation

Hi,

On 7/3/23 10:34 PM, Nathan Bossart wrote:

On Sat, Jul 01, 2023 at 04:02:06PM +0200, Drouvot, Bertrand wrote:

Please find V2 attached where it's failing as soon as the database name or
user name are detected as overlength.

Thanks, Bertrand. I chickened out and ended up committing v1 for now
(i.e., simply removing the truncation code). I didn't like the idea of
trying to keep the new error messages consistent with code in faraway
files, and the startup packet length limit is already pretty aggressive, so
I'm a little less concerned about lugging around long names. Plus, I think
v2 had some subtle interactions with db_user_namespace (maybe for the
better), but I didn't spend too much time looking at that since
db_user_namespace will likely be removed soon.

Thanks Nathan for the feedback and explanations, I think that makes fully sense.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com