On Tue, Mar 15, 2016 at 3:12 PM, David Steele <da...@pgmasters.net> wrote:
> On 3/8/16 5:44 PM, Robbie Harwood wrote:
>> Here's yet another version of GSSAPI encryption support.

This looks far more stable than last versions, cool to see the
progress. pgbench -C does not complain on my side so that's a good
thing. This is not yet a detailed review, there are a couple of things
that I find strange in what you did and are potential subject to bugs,
but I need a bit of time to let that mature a bit. This is not
something yet committable, but I really like the direction that the
patch is taking.

For now, regarding 0002:
-    * Flush message so client will see it, except for AUTH_REQ_OK, which need
-    * not be sent until we are ready for queries.
+    * In most cases, we do not need to send AUTH_REQ_OK until we are ready
+    * for queries, but if we are doing GSSAPI encryption that request must go
+    * out now.
-   if (areq != AUTH_REQ_OK)
-       pq_flush();
+   pq_flush();
Er, this sends unconditionally the message without caring about the
protocol, and so this is incorrect, no?

+#ifdef ENABLE_GSS
+       if (conn->gss->buf.data)
+           pfree(conn->gss->buf.data);
+       if (conn->gss->writebuf.data)
+           pfree(conn->gss->writebuf.data);
You should use resetStringInfo here.

> OK, everything seems to be working fine with a 9.6 client and server so
> next I tested older clients and I got this error:
> $ /usr/lib/postgresql/9.1/bin/psql -h localhost \
>   -U vagr...@pgmasters.net postgres
> psql: FATAL:  GSSAPI did no provide required flags
> There's a small typo in the error message, BTW.

And in 0003, the previous error is caused by that:
+   target_flags = GSS_C_MUTUAL_FLAG | GSS_C_REPLAY_FLAG |
+   if ((gflags & target_flags) != target_flags)
+   {
+       ereport(FATAL, (errmsg("GSSAPI did no provide required flags")));
+       return STATUS_ERROR;
+   }
Yeah, this is a recipe for protocol incompatibility and here the
connection context is not yet fully defined I believe. We need to be

-       maj_stat = gss_accept_sec_context(
-                                         &min_stat,
+       maj_stat = gss_accept_sec_context(&min_stat,
This is just noise.

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to