Обсуждение: [MASSMAIL]pgsql: Enhance libpq encryption negotiation tests with new GUC

Enhance libpq encryption negotiation tests with new GUC

The new "log_connection_negotiation" server option causes the server
to print messages to the log when it receives a SSLRequest or
GSSENCRequest packet from the client. Together with "log_connections",
it gives a trace of how a connection and encryption is
negotiatated. Use the option in the libpq_encryption test, to verify
in more detail how libpq negotiates encryption with different
gssencmode and sslmode options.

This revealed a couple of cases where libpq retries encryption or
authentication, when it should already know that it cannot succeed.  I
marked them with XXX comments in the test tables. They only happen
when the connection was going to fail anyway, and only with rare
combinations of options, so they're not serious.

Discussion: https://www.postgresql.org/message-id/CAEze2Wja8VUoZygCepwUeiCrWa4jP316k0mvJrOW4PFmWP0Tcw@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/705843d294d5d3bc5cce4001596df4df5f1c8b59

Modified Files
--------------
src/backend/tcop/backend_startup.c                 |  23 ++
src/backend/utils/misc/guc_tables.c                |  11 +
.../libpq_encryption/t/001_negotiate_encryption.pl | 380 ++++++++++++---------
3 files changed, 255 insertions(+), 159 deletions(-)

At Sun, 07 Apr 2024 23:50:08 +0000, Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote in 
> Enhance libpq encryption negotiation tests with new GUC

This commit adds the following messages:

> gettext_noop("Log details of pre-authentication connection handshake."),

Similar to a nearby commit, other messages with a similar context use
the phrase "Logs <something>". Wouldn't it be better to align this
message with existing ones?

diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 83e3a59d7e..4584829992 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1227,7 +1227,7 @@ struct config_bool ConfigureNamesBool[] =
     },
     {
         {"trace_connection_negotiation", PGC_POSTMASTER, DEVELOPER_OPTIONS,
-            gettext_noop("Log details of pre-authentication connection handshake."),
+            gettext_noop("Logs details of pre-authentication connection handshake."),
             NULL,
             GUC_NOT_IN_SAMPLE
         },

regards

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

On 08/04/2024 09:40, Kyotaro Horiguchi wrote:
> At Sun, 07 Apr 2024 23:50:08 +0000, Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote in
>> Enhance libpq encryption negotiation tests with new GUC
> 
> This commit adds the following messages:
> 
>> gettext_noop("Log details of pre-authentication connection handshake."),
> 
> Similar to a nearby commit, other messages with a similar context use
> the phrase "Logs <something>". Wouldn't it be better to align this
> message with existing ones?
> 
> diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
> index 83e3a59d7e..4584829992 100644
> --- a/src/backend/utils/misc/guc_tables.c
> +++ b/src/backend/utils/misc/guc_tables.c
> @@ -1227,7 +1227,7 @@ struct config_bool ConfigureNamesBool[] =
>       },
>       {
>           {"trace_connection_negotiation", PGC_POSTMASTER, DEVELOPER_OPTIONS,
> -            gettext_noop("Log details of pre-authentication connection handshake."),
> +            gettext_noop("Logs details of pre-authentication connection handshake."),
>               NULL,
>               GUC_NOT_IN_SAMPLE
>           },

We're not very consistent about it, there's also:

log_temp_files: Log the use of temporary files larger than this number 
of kilobytes.
trace_syncscan: Generate debugging output for synchronized scanning.
trace_sort: Emit information about resource usage in sorting.
backtrace_functions: Log backtrace for errors in these functions.
backtrace_on_internal_error: Log backtrace for any error with error code 
XX000 (internal error).

But I agree the "Logs ..." phrasing is more common, so committed.

-- 
Heikki Linnakangas
Neon (https://neon.tech)