MIME-Version: 1.0
References: <88986722-5A72-4DEC-8750-BDBF67FF8C01@yesql.se>
 <7E77028B-5A3A-436B-9046-8E9992E9F94A@yesql.se>
 <Z1jeINLtB05YMVOY@paquier.xyz>
 <E51258CD-C7B2-4BB1-AB9C-A3DD2B6592D0@yesql.se>
 <F4222274-15C0-40E4-84FB-8FC1355BB8B2@yesql.se>
 <CAOYmi+mSrV8hRaQkvGDf1Df4cmpv5SeTbTxppyxeonMe6MW8nA@mail.gmail.com>
 <0BC5B9B1-6503-4563-AAC6-33DEF264AE3F@yesql.se>
 <aa7gx3mychf3m2g67mbslzbxjy3if4enpcflstoa5pol3432x5@ugqz45gsvurq>
 <F585452A-1F69-4658-A7E8-D1273AC663ED@yesql.se>
 <aLT9xVsTVp0j3zWy@paquier.xyz>
 <80F4F8F4-8E4F-4B6F-866B-D837057C1192@yesql.se>
 <D9F6F14D-FCB1-48B5-9CBC-AE89FA0AEF4C@yesql.se>
 <CAOYmi+m2Ks7D4obtXay3y-UNn6CkTNrmr_zWC25vKTdesatafA@mail.gmail.com>
 <0C53C316-C24E-4307-807B-D825CA3F7254@yesql.se>
 <DD77A37A-BFCA-4819-A029-1A41E2443D8E@gmail.com>
 <378D83FA-338C-4EA1-BC60-397BE08D0F01@yesql.se>
 <2025112617144938459246@163.com>
 <0217DEFA-9684-4A77-A005-D30EBEF155C4@yesql.se>
 <afb479af-2fe7-4790-ad11-437a326329b5@iki.fi>
 <5D0E78E0-EA79-480E-ABD3-B1EF0156BF8B@yesql.se>
 <f621264b-26a6-4b7a-8982-3820b7d6d397@iki.fi>
 <CAGECzQTWH-bzHcdPo=i09TL_P6_HBBNEkBmr+rpN_J9zVfR2Fw@mail.gmail.com>
 <785C0B88-7068-4576-AF55-251D06CEC112@yesql.se>
 <CAOYmi+=u=vS1beiog6p5e843uVdout9qZY=pRj4vo=jCVwgGTA@mail.gmail.com>
 <FB2759EB-C8DB-49FA-800F-7010C65EB8DF@yesql.se>
 <CAOYmi+mZ=i55iH44zPqidZfoNDLwPBMD=PUtD03LR2ut+zMEag@mail.gmail.com>
 <01412917-C42E-4238-97E2-707C32940DDD@yesql.se>
 <CAOYmi+m05X5fRaeV7w3y4VOePnJJQrihK9A_6ma3e5Pesa5mXA@mail.gmail.com>
 <F3BFF481-E9FD-4893-92D2-925BE2229401@yesql.se>
 <CAOYmi+nsqYiXLxN7G4A5edBJWXZ8qD=zFnaE2bEzuMj2_xBT7Q@mail.gmail.com>
 <B481FA78-0173-4D0F-84C5-EDFB613B6428@yesql.se>
 <AE0BE080-02E8-4EF2-980B-15302FA1599E@yesql.se>
 <A4CFF972-B404-4422-A9BF-E0E2797CCBF3@yesql.se>
In-Reply-To: <A4CFF972-B404-4422-A9BF-E0E2797CCBF3@yesql.se>
From: Zsolt Parragi <zsolt.parragi@percona.com>
Date: Fri, 13 Mar 2026 21:12:43 +0000
Message-ID: 
 <CAN4CZFOGiKAX_gD4Ck+o79n5hAVr3UmuTbYwaABcCuhWA3UehQ@mail.gmail.com>
Subject: Re: Serverside SNI support in libpq
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Jacob Champion <jacob.champion@enterprisedb.com>,
 Jelte Fennema-Nio <postgres@jeltef.nl>,
	Heikki Linnakangas <hlinnaka@iki.fi>, Dewei Dai <daidewei1970@163.com>,
	"li.evan.chao" <li.evan.chao@gmail.com>,
 Michael Paquier <michael@paquier.xyz>,
	Andres Freund <andres@anarazel.de>,
 Pgsql Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAN4CZFOGiKAX_gD4Ck%2Bo79n5hAVr3UmuTbYwaABcCuhWA3UehQ%40mail.gmail.com>
Precedence: bulk

Hello

Originally I started looking at this thread because Jacob mentioned it
relates to the custom OAuth / HBA variable question[1]. While I do see
the relation, I don't have many practical ideas.

I was thinking about suggesting using a "key=value, key=value ..."
file style instead of the fixed table, both for easier later
generalization, and because it aligns better to modern configuration
formats (and personally I always find these columnar config files
harder to read)
However, it would also differ from other existing postgres config
files and wouldn't offer a clear initial advantage, so it doesn't seem
like a good practical choice. I'm still mentioning this for
completeness, but mostly I'll focus on a more practical review:

+  check_nook => 'check_ssl_sni',

This seems to be a typo?


+ if (SSL_client_hello_get0_ext(ssl, TLSEXT_TYPE_server_name, &tlsext, &left))
+ {
+ if (left <= 2)
+ {
+ *al = SSL_AD_MISSING_EXTENSION;
+ return 0;
+ }

 ... and later error returns in this if block seem to use the wrong
error code to me: truncated length, length mismatch, empty list,
length exceeding remaining data...

missing_extension:  Sent by endpoints that receive a handshake
      message not containing an extension that is mandatory to send for
      the offered TLS version or other negotiated parameters.

decode_error:  A message could not be decoded because some field was
      out of the specified range or the length of the message was
      incorrect.  This alert is used for errors where the message does
      not conform to the formal protocol syntax.  This alert should
      never be observed in communication between proper implementations,
      except when messages were corrupted in the network.

Since we are already inside the if which verifies that the extension
is present, shouldn't all of these report decode_error?

+ if (!ssl_update_ssl(ssl, install_config))
+ {
+ ereport(COMMERROR,
+ errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("failed to switch to SSL configuration for host, terminating
connection"));
+ return SSL_CLIENT_HELLO_ERROR;
+ }


Isn't there a missing *al = assignment here?


+ /*
+ * There should be no more tokens after this, if there are break
+ * parsing and report error to avoid silently accepting incorrect
+ * config.
+ */
+ if (tokens->length > 1)
+ {
+ ereport(elevel,
+ errcode(ERRCODE_CONFIG_FILE_ERROR),
+ errmsg("extra fields at end of line"),
+ errcontext("line %d of configuration file \"%s\"",
+    tok_line->line_num, tok_line->file_name));
+ return NULL;
+ }


The comment suggests that this aims to prevent any additional text on
the line, but this parses:

localhost server.crt server.key server.crt "cmd" on TRAILING_TEXT MORE_TEXT


+ /* SSL Passphrase Command (optional) */
+ field = lnext(tok_line->fields, field);
+ if (field)
+ {
+ tokens = lfirst(field);
+ token = linitial(tokens);

Isn't a length > 1 error check missing from here?

[1]: https://www.postgresql.org/message-id/CAN4CZFM3b8u5uNNNsY6XCya257u%2BDofms3su9f11iMCxvCacag%40mail.gmail.com