rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix timing-dependent failure in GSSAPI data transmission. 

When using GSSAPI encryption in non-blocking mode, libpq sometimes
failed with "GSSAPI caller failed to retransmit all data needing
to be retried".  The cause is that pqPutMsgEnd rounds its transmit
request down to an even multiple of 8K, and sometimes that can lead
to not requesting a write of data that was requested to be written
(but reported as not written) earlier.  That can upset pg_GSS_write's
logic for dealing with not-yet-written data, since it's possible
the data in question had already been incorporated into an encrypted
packet that we weren't able to send during the previous call.

We could fix this with a one-or-two-line hack to disable pqPutMsgEnd's
round-down behavior, but that seems like making the caller work around
a behavior that pg_GSS_write shouldn't expose in this way.  Instead,
adjust pg_GSS_write to never report a partial write: it either
reports a complete write, or reflects the failure of the lower-level
pqsecure_raw_write call.  The requirement still exists for the caller
to present at least as much data as on the previous call, but with
the caller-visible write start point not moving there is no temptation
for it to present less.  We lose some ability to reclaim buffer space
early, but I doubt that that will make much difference in practice.

This also gets rid of a rather dubious assumption that "any
interesting failure condition (from pqsecure_raw_write) will recur
on the next try".  We've not seen failure reports traceable to that,
but I've never trusted it particularly and am glad to remove it.

Make the same adjustments to the equivalent backend routine
be_gssapi_write().  It is probable that there's no bug on the backend
side, since we don't have a notion of nonblock mode there; but we
should keep the logic the same to ease future maintenance.

Per bug #18210 from Lars Kanis.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/18210-4c6d0b14627f2eb8@postgresql.org
This commit is contained in:
Tom Lane 2023-11-23 13:30:18 -05:00
parent 50c67c2019
commit d053a879bb
3 changed files with 56 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
src/backend/libpq/be-secure-gssapi.c
View File

@ -60,8 +60,8 @@ static char *PqGSSSendBuffer; /* Encrypted data waiting to be sent */
static int PqGSSSendLength; /* End of data available in PqGSSSendBuffer */
static int PqGSSSendNext; /* Next index to send a byte from
* PqGSSSendBuffer */
static int PqGSSSendConsumed; /* Number of *unencrypted* bytes consumed for
* current contents of PqGSSSendBuffer */
static int PqGSSSendConsumed; /* Number of source bytes encrypted but not
* yet reported as sent */
static char *PqGSSRecvBuffer; /* Received, encrypted data */
static int PqGSSRecvLength; /* End of data available in PqGSSRecvBuffer */
@ -83,8 +83,8 @@ static uint32 PqGSSMaxPktSize; /* Maximum size we can encrypt and fit the
*
* On success, returns the number of data bytes consumed (possibly less than
* len). On failure, returns -1 with errno set appropriately. For retryable
* errors, caller should call again (passing the same data) once the socket
* is ready.
* errors, caller should call again (passing the same or more data) once the
* socket is ready.
*
* Dealing with fatal errors here is a bit tricky: we can't invoke elog(FATAL)
* since it would try to write to the client, probably resulting in infinite
@ -98,19 +98,25 @@ be_gssapi_write(Port *port, void *ptr, size_t len)
minor;
gss_buffer_desc input,
output;
size_t bytes_sent = 0;
size_t bytes_to_encrypt;
size_t bytes_encrypted;
gss_ctx_id_t gctx = port->gss->ctx;
/*
* When we get a failure, we must not tell the caller we have successfully
* transmitted everything, else it won't retry. Hence a "success"
* (positive) return value must only count source bytes corresponding to
* fully-transmitted encrypted packets. The amount of source data
* corresponding to the current partly-transmitted packet is remembered in
* When we get a retryable failure, we must not tell the caller we have
* successfully transmitted everything, else it won't retry. For
* simplicity, we claim we haven't transmitted anything until we have
* successfully transmitted all "len" bytes. Between calls, the amount of
* the current input data that's already been encrypted and placed into
* PqGSSSendBuffer (and perhaps transmitted) is remembered in
* PqGSSSendConsumed. On a retry, the caller *must* be sending that data
* again, so if it offers a len less than that, something is wrong.
*
* Note: it may seem attractive to report partial write completion once
* we've successfully sent any encrypted packets. However, that can cause
* problems for callers; notably, pqPutMsgEnd's heuristic to send only
* full 8K blocks interacts badly with such a hack. We won't save much,
* typically, by letting callers discard data early, so don't risk it.
*/
if (len < PqGSSSendConsumed)
{
@ -118,6 +124,7 @@ be_gssapi_write(Port *port, void *ptr, size_t len)
errno = ECONNRESET;
return -1;
}
/* Discount whatever source data we already encrypted. */
bytes_to_encrypt = len - PqGSSSendConsumed;
bytes_encrypted = PqGSSSendConsumed;
@ -146,33 +153,20 @@ be_gssapi_write(Port *port, void *ptr, size_t len)
ret = secure_raw_write(port, PqGSSSendBuffer + PqGSSSendNext, amount);
if (ret <= 0)
{
/*
* Report any previously-sent data; if there was none, reflect
* the secure_raw_write result up to our caller. When there
* was some, we're effectively assuming that any interesting
* failure condition will recur on the next try.
*/
if (bytes_sent)
return bytes_sent;
return ret;
}
/*
* Check if this was a partial write, and if so, move forward that
* far in our buffer and try again.
*/
if (ret != amount)
if (ret < amount)
{
PqGSSSendNext += ret;
continue;
}
/* We've successfully sent whatever data was in that packet. */
bytes_sent += PqGSSSendConsumed;
/* All encrypted data was sent, our buffer is empty now. */
PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0;
/* We've successfully sent whatever data was in the buffer. */
PqGSSSendLength = PqGSSSendNext = 0;
}
/*
@ -196,7 +190,10 @@ be_gssapi_write(Port *port, void *ptr, size_t len)
output.value = NULL;
output.length = 0;
/* Create the next encrypted packet */
/*
* Create the next encrypted packet. Any failure here is considered a
* hard failure, so we return -1 even if some data has been sent.
*/
major = gss_wrap(&minor, gctx, 1, GSS_C_QOP_DEFAULT,
&input, &conf_state, &output);
if (major != GSS_S_COMPLETE)
@ -239,10 +236,13 @@ be_gssapi_write(Port *port, void *ptr, size_t len)
}
/* If we get here, our counters should all match up. */
Assert(bytes_sent == len);
Assert(bytes_sent == bytes_encrypted);
Assert(len == PqGSSSendConsumed);
Assert(len == bytes_encrypted);
return bytes_sent;
/* We're reporting all the data as sent, so reset PqGSSSendConsumed. */
PqGSSSendConsumed = 0;
return bytes_encrypted;
}
/*

52
src/interfaces/libpq/fe-secure-gssapi.c
View File

@ -79,8 +79,8 @@
* On success, returns the number of data bytes consumed (possibly less than
* len). On failure, returns -1 with errno set appropriately. If the errno
* indicates a non-retryable error, a message is added to conn->errorMessage.
* For retryable errors, caller should call again (passing the same data)
* once the socket is ready.
* For retryable errors, caller should call again (passing the same or more
* data) once the socket is ready.
*/
ssize_t
pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
@ -90,19 +90,25 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
gss_buffer_desc input,
output = GSS_C_EMPTY_BUFFER;
ssize_t ret = -1;
size_t bytes_sent = 0;
size_t bytes_to_encrypt;
size_t bytes_encrypted;
gss_ctx_id_t gctx = conn->gctx;
/*
* When we get a failure, we must not tell the caller we have successfully
* transmitted everything, else it won't retry. Hence a "success"
* (positive) return value must only count source bytes corresponding to
* fully-transmitted encrypted packets. The amount of source data
* corresponding to the current partly-transmitted packet is remembered in
* When we get a retryable failure, we must not tell the caller we have
* successfully transmitted everything, else it won't retry. For
* simplicity, we claim we haven't transmitted anything until we have
* successfully transmitted all "len" bytes. Between calls, the amount of
* the current input data that's already been encrypted and placed into
* PqGSSSendBuffer (and perhaps transmitted) is remembered in
* PqGSSSendConsumed. On a retry, the caller *must* be sending that data
* again, so if it offers a len less than that, something is wrong.
*
* Note: it may seem attractive to report partial write completion once
* we've successfully sent any encrypted packets. However, that can cause
* problems for callers; notably, pqPutMsgEnd's heuristic to send only
* full 8K blocks interacts badly with such a hack. We won't save much,
* typically, by letting callers discard data early, so don't risk it.
*/
if (len < PqGSSSendConsumed)
{
@ -140,33 +146,20 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
retval = pqsecure_raw_write(conn, PqGSSSendBuffer + PqGSSSendNext, amount);
if (retval <= 0)
{
/*
* Report any previously-sent data; if there was none, reflect
* the pqsecure_raw_write result up to our caller. When there
* was some, we're effectively assuming that any interesting
* failure condition will recur on the next try.
*/
if (bytes_sent)
return bytes_sent;
return retval;
}
/*
* Check if this was a partial write, and if so, move forward that
* far in our buffer and try again.
*/
if (retval != amount)
if (retval < amount)
{
PqGSSSendNext += retval;
continue;
}
/* We've successfully sent whatever data was in that packet. */
bytes_sent += PqGSSSendConsumed;
/* All encrypted data was sent, our buffer is empty now. */
PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0;
/* We've successfully sent whatever data was in the buffer. */
PqGSSSendLength = PqGSSSendNext = 0;
}
/*
@ -192,7 +185,7 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
/*
* Create the next encrypted packet. Any failure here is considered a
* hard failure, so we return -1 even if bytes_sent > 0.
* hard failure, so we return -1 even if some data has been sent.
*/
major = gss_wrap(&minor, gctx, 1, GSS_C_QOP_DEFAULT,
&input, &conf_state, &output);
@ -236,10 +229,13 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
}
/* If we get here, our counters should all match up. */
Assert(bytes_sent == len);
Assert(bytes_sent == bytes_encrypted);
Assert(len == PqGSSSendConsumed);
Assert(len == bytes_encrypted);
ret = bytes_sent;
/* We're reporting all the data as sent, so reset PqGSSSendConsumed. */
PqGSSSendConsumed = 0;
ret = bytes_encrypted;
cleanup:
/* Release GSSAPI buffer storage, if we didn't already */

4
src/interfaces/libpq/libpq-int.h
View File

@ -583,8 +583,8 @@ struct pg_conn
int gss_SendLength; /* End of data available in gss_SendBuffer */
int gss_SendNext; /* Next index to send a byte from
* gss_SendBuffer */
int gss_SendConsumed; /* Number of *unencrypted* bytes consumed
* for current contents of gss_SendBuffer */
int gss_SendConsumed; /* Number of source bytes encrypted but
* not yet reported as sent */
char *gss_RecvBuffer; /* Received, encrypted data */
int gss_RecvLength; /* End of data available in gss_RecvBuffer */
char *gss_ResultBuffer; /* Decryption of data in gss_RecvBuffer */