From: Jacob Champion <pchampion@vmware.com>
To: Daniel Gustafsson <daniel@yesql.se>
CC: Heikki Linnakangas <hlinnaka@iki.fi>, Andres Freund <andres@anarazel.de>,
	Postgres hackers <pgsql-hackers@lists.postgresql.org>, Michael Paquier
	<michael@paquier.xyz>, Andrew Dunstan <andrew.dunstan@2ndquadrant.com>,
	Stephen Frost <sfrost@snowman.net>, Thomas Munro <thomas.munro@gmail.com>
Subject: Re: Support for NSS as a libpq TLS backend
Thread-Topic: Support for NSS as a libpq TLS backend
Thread-Index: 
 AQHWM1w61GrKUIsl6UCBcYrYNRBt46kJRsIAgAA+PQCAGQsiAIAEdGUAgAAqk4CAAbpsAIABgI0AgCnqVACAA5UBgIAAY+MAgBTX7ACAACGAAIASngCAgAAfeICABYLFgIAbyjgAgABy24CACw63gIAAAzWAgAwa5QCAA6HPgIAAKuGAgAYXDoCAACZBAIABTCSAgAALLgCAAcj/gIAA61UAgAU5IwCAAT6LgIAZy/oA
Date: Fri, 4 Dec 2020 00:57:26 +0000
Message-ID: <BB748015-45A4-4A43-B740-289E5592CA08@vmware.com>
References: <e3c34e7b-900e-7e32-f60b-a278a1eb743a@2ndQuadrant.com>
 <DA91E5F0-5F9D-41A7-A7A6-B91CDE0F1D63@yesql.se>
 <CAA8=A7-suZg90M7Fgyo1Rfymkrpdh_Z547RKXrbHirsAmojQSw@mail.gmail.com>
 <20200904012334.GF19499@paquier.xyz> <20200917074134.GX2873@paquier.xyz>
 <BAD11CCA-E712-4906-8A27-94C2AA0E696E@yesql.se>
 <20200929055939.GF7117@paquier.xyz>
 <DE377D9A-BFFC-492C-A2B8-2AE6EBBBE740@yesql.se>
 <411593A7-E037-474D-BFD7-D3D6683C1D46@yesql.se>
 <B1ADEE76-3E33-4E9D-B830-3AA89B1C42D2@yesql.se>
 <20201020191529.5yverw3ybaube3pg@alap3.anarazel.de>
 <907FA7D1-9BD9-464B-A6EC-DEEB53438D36@yesql.se>
 <7ed636a0-872c-3ea7-dc44-b35dd515bb87@iki.fi>
 <02260088-E0C9-49A2-ACD3-CE51D65A84E3@yesql.se>
 <0E64F3CB-4731-4FA8-82D8-D8590086654C@vmware.com>
 <F16F003C-EAFB-4FCA-9880-3D5E2B428F78@yesql.se>
 <1B0B31E4-AD37-475C-9374-7E24AA808479@vmware.com>
 <B506D54C-BBB5-49C0-BF9A-BACEFAD8520D@yesql.se>
 <5C27CF6F-5BD3-4639-AAA6-73465595B6DF@vmware.com>
 <AC495586-B79D-45DB-A2ED-2E4FC24ED3C7@vmware.com>
 <FBA0C365-3B19-4E40-98F4-8B0064DB7127@vmware.com>
 <7B5A88DF-863A-4B00-B841-0285C1FF70DF@yesql.se>
 <5F24348B-10BA-4AA1-999B-1E2AB22B4997@vmware.com>
 <94E22878-6289-43D5-A674-804F6CB23782@yesql.se>
In-Reply-To: <94E22878-6289-43D5-A674-804F6CB23782@yesql.se>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <76189C268934FF44BDD5DA59252ABCFE@namprd05.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 
 c0e85573-c5f5-4ee9-2df2-08d897ef91bb
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Dec 2020 00:57:26.9537
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 
 9JieAHI+lsRN9rQar25qMjLE0wehyv6fpQVqtoVAGVaNyM/cIrmH4JVJ3poWDITOyI5V1iKjSKvuOfNLXbJ3IQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR05MB3454
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Precedence: bulk

On Nov 17, 2020, at 7:00 AM, Daniel Gustafsson <daniel@yesql.se> wrote:
>=20
> Nice, thanks for the fix!  I've incorporated your patch into the attached=
 v20
> which also fixes client side error reporting to be more readable.

I was testing handshake failure modes and noticed that some FATAL
messages are being sent through to the client in cleartext. The OpenSSL
implementation doesn't do this, because it logs handshake problems at
COMMERROR level. Should we switch all those ereport() calls in the NSS
be_tls_open_server() to COMMERROR as well (and return explicitly), to
avoid this? Or was there a reason for logging at FATAL/ERROR level?

Related note, at the end of be_tls_open_server():

>     ...
>     port->ssl_in_use =3D true;
>     return 0;
>=20
> error:
>     return 1;
> }

This needs to return -1 in the error case; the only caller of
secure_open_server() does a direct `result =3D=3D -1` comparison rather tha=
n
checking `result !=3D 0`.

--Jacob=