Andres Freund <and...@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.gd30...@alap3.anarazel.de and Michael
Paquier's request in
id:cab7npqtjd-ttrm1vu8p55_4kkvedx8dfz9v1d_xsqqvr_xa...@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
signature.asc
Description: PGP signature
