From: Robbie Harwood <rharwood@redhat.com>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org, Stephen Frost <sfrost@snowman.net>
Cc: Nico Williams <nico@cryptonector.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: David Steele <david@pgmasters.net>
Subject: Re: [PATCH v20] GSSAPI encryption support
Date: Mon, 18 Feb 2019 10:02:27 -0500
Message-ID: <jlgzhqtqgvw.fsf@redhat.com> (raw)
In-Reply-To: <20190216041026.gpkfn4e6wrozliyb@alap3.anarazel.de>
References: <jlg1tgq1ktm.fsf@thriss.redhat.com>
<jlgy38mir81.fsf@redhat.com>
<20190216041026.gpkfn4e6wrozliyb@alap3.anarazel.de>
Andres Freund <andres@anarazel.de> writes:
> On 2018-12-18 14:12:46 -0500, Robbie Harwood wrote:
>
>> Subject: [PATCH] libpq GSSAPI encryption support
>
> Could some of these be split into separate patches that could be more
> eagerly merged? This is a somewhat large patch...
What splits do you propose? (It's been a single patch since v3 as per
your request in id:20151003161810.GD30738@alap3.anarazel.de and Michael
Paquier's request in
id:CAB7nPqTJD-tTrM1Vu8P55_4kKVeDX8DFz9v1D_XsQQvR_xA5qQ@mail.gmail.com).
>> diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
>> index a06fc7dc82..f4f196e3b4 100644
>> --- a/src/interfaces/libpq/fe-secure.c
>> +++ b/src/interfaces/libpq/fe-secure.c
>> @@ -220,6 +220,13 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)
>> n = pgtls_read(conn, ptr, len);
>> }
>> else
>> +#endif
>> +#ifdef ENABLE_GSS
>> + if (conn->gssenc)
>> + {
>> + n = pg_GSS_read(conn, ptr, len);
>> + }
>> + else
>> #endif
>> {
>> n = pqsecure_raw_read(conn, ptr, len);
>> @@ -287,7 +294,7 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
>> * to determine whether to continue/retry after error.
>> */
>> ssize_t
>> -pqsecure_write(PGconn *conn, const void *ptr, size_t len)
>> +pqsecure_write(PGconn *conn, void *ptr, size_t len)
>> {
>> ssize_t n;
>>
>> @@ -297,6 +304,13 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
>> n = pgtls_write(conn, ptr, len);
>> }
>> else
>> +#endif
>> +#ifdef ENABLE_GSS
>> + if (conn->gssenc)
>> + {
>> + n = pg_GSS_write(conn, ptr, len);
>> + }
>> + else
>> #endif
>> {
>> n = pqsecure_raw_write(conn, ptr, len);
>
> Not a fan of this. Seems like we'll grow more and more such branches
> over time? Wouldn't the right thing be to have callbacks in PGconn
> (and similarly in the backend)?
Is that really a problem? Each branch is only seven lines, which is a
lot less than adding callback support will be. And we'll only get a new
branch when we want to support a new encryption protocol - unlike
authentication, there aren't too many of those.
> Seems like if that's done reasonably it'd also make integration of
> compression easier, because that could just layer itself between
> encryption and wire?
The current interface would allow a compress/decompress call in a way
that makes sense to me (here for write, ignoring ifdefs):
ssize_t pqsecure_write(PGConn *conn, void *ptr, size_t len)
{
ssize_t n;
if (conn->compress)
{
do_compression(conn, &ptr, &len);
}
if (conn->ssl_in_use)
{
n = pgtls_write(conn, ptr, len);
}
else if (conn->gssenc)
{
n = pg_GSS_write(conn, ptr, len);
}
else
{
n = pqsecure_raw_write(conn, ptr, len);
}
return n;
}
(pqsecure_read would look similarly, with decompression as the last step
instead of the first.)
Thanks,
--Robbie
Attachments:
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: rharwood@redhat.com, andres@anarazel.de, sfrost@snowman.net, nico@cryptonector.com, michael@paquier.xyz, alvherre@2ndquadrant.com, david@pgmasters.net
Subject: Re: [PATCH v20] GSSAPI encryption support
In-Reply-To: <jlgzhqtqgvw.fsf@redhat.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox