From: Robbie Harwood <rharwood@redhat.com>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH v10] GSSAPI encryption support
In-Reply-To: 
 <CAB7nPqTtJrFYy6Z5fyRiKg8DNdQ5bpCH_Qmbarhx0rSoK2RvQA@mail.gmail.com>
References: <jlg1tgq1ktm.fsf@thriss.redhat.com>
 <jlgpoua3syy.fsf@thriss.redhat.com>
 <CAB7nPqTtJrFYy6Z5fyRiKg8DNdQ5bpCH_Qmbarhx0rSoK2RvQA@mail.gmail.com>
User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/24.5.1
 (x86_64-pc-linux-gnu)
Date: Fri, 01 Apr 2016 15:00:07 -0400
Message-ID: <jlgmvpd40js.fsf@thriss.redhat.com>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

--=-=-=
Content-Type: text/plain

Michael Paquier <michael.paquier@gmail.com> writes:

> On Fri, Apr 1, 2016 at 12:31 PM, Robbie Harwood <rharwood@redhat.com> wrote:
>
>> - Fixed buffering of large replies on the serverside.  This should fix
>>   the traceback that was being seen.  The issue had to do with the
>>   difference between the server and client calling conventions for the
>>   _read and _write functions.
>
> This does not fix the issue. I am still able to crash the server with
> the same trace.

Interesting.  I guess I'll keep trying to reproduce this.

>> - Move gss_encrypt out of the GUCs and into connection-specific logic.
>>   Thanks to Tom Lane for pointing me in the right direction here.
>
> +   /* delay processing until after AUTH_REQ_OK has been sent */
> +   if (port->gss->gss_encrypt != NULL)
> +       port->gss->encrypt = !strcmp(port->gss->gss_encrypt, "on");
>
> You should use parse_bool here, because contrary to libpq, clients
> should be able to use other values like "1", "0", "off", 'N', 'Y',
> etc.

Missed that function somehow.  Will fix.

>> - Replace writev() with two calls to _raw_write().  I'm not attached to
>>   this design; if someone has a preference for allocating a buffer and
>>   making a single write from that, I could be persuaded.  I don't know
>>   what the performance tradeoffs are.
>
> Relying on pqsecure_raw_write and pqsecure_raw_read is a better bet
> IMO, there is already some low-level error handling.

Indeed, it looks like it should especially lead to better behavior on
Windows.

>  static ConfigVariable *ProcessConfigFileInternal(GucContext context,
>                           bool applySettings, int elevel);
>
> -
>  /*
>
> Spurious noise.

Indeed, will fix.

--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJW/sU4AAoJECUy+RdqlaRCf1YP/A9o0xr+/Qpxnt4E04SsTAi4
rYZtRKsHnJPgvV4Yc5x9ZSDqKrqpAswOylyZ4DTKVfh/d1FhBIFUTRAaOuV24lmj
mD0XsIdHg9I9yj0kzrEP9QuPE2hwFfAhEajMdrZFRb3AMspmQTrPEhvktefOuirk
fY2LCU7QWUd39Qfm5KZUuiJpsYF1K2lNyliBonz4Vx33D2+CcMDx7oGZWzlxypIW
yVlegQk7fAvLa2oJ5mpcGWcl4zfzKJOkqy90tgva8KW6/AzlQr8F5X6wAnHYmEk+
6ReVuAggoA5IXeWVn/N/3xmEJQXqoCTqUC2g7DLxO21tKP/N2X7i/RFYJSe0pzPK
B/VvEMMzC0Qgpvlg13YtzrrdEgiD4c2Lm8ArOXsM1fEtT0KR6ZQf2OZ+kYzveMgf
LG8ILPFqd53YlvetrKTxIh7YSEeJccjwsk4JqOQOIInY0eSaaV2MIiGz2fiUl7Zj
2Vi0umyHKvtJEiynJT+IGgthZumTp0uWUNsa48ZH/nA+GF8U2OdE3FzZlXwyUv2t
onxSm9CZAieAoOZKuE5Y2lVxGFF8H+TPfrSgeN9l3ITYVUcgNyB+10rV/r4Oq5CA
sGDb8W1wSFNsGX54wL1TAefD607Whbg3admke1dGl/Vr+82ydShafeDc3XZ9rSAn
l+aDsPWk6UyHPb5o/oZ5
=5ZmH
-----END PGP SIGNATURE-----
--=-=-=--