Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd053a87

Browse files
committed
Fix timing-dependent failure in GSSAPI data transmission.
When using GSSAPI encryption in non-blocking mode, libpq sometimesfailed with "GSSAPI caller failed to retransmit all data needingto be retried". The cause is that pqPutMsgEnd rounds its transmitrequest down to an even multiple of 8K, and sometimes that can leadto not requesting a write of data that was requested to be written(but reported as not written) earlier. That can upset pg_GSS_write'slogic for dealing with not-yet-written data, since it's possiblethe data in question had already been incorporated into an encryptedpacket 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'sround-down behavior, but that seems like making the caller work arounda behavior that pg_GSS_write shouldn't expose in this way. Instead,adjust pg_GSS_write to never report a partial write: it eitherreports a complete write, or reflects the failure of the lower-levelpqsecure_raw_write call. The requirement still exists for the callerto present at least as much data as on the previous call, but withthe caller-visible write start point not moving there is no temptationfor it to present less. We lose some ability to reclaim buffer spaceearly, but I doubt that that will make much difference in practice.This also gets rid of a rather dubious assumption that "anyinteresting failure condition (from pqsecure_raw_write) will recuron 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 routinebe_gssapi_write(). It is probable that there's no bug on the backendside, since we don't have a notion of nonblock mode there; but weshould 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
1 parent50c67c2 commitd053a87

File tree

3 files changed

+56
-60
lines changed

3 files changed

+56
-60
lines changed

‎src/backend/libpq/be-secure-gssapi.c

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ static char *PqGSSSendBuffer;/* Encrypted data waiting to be sent */
6060
staticintPqGSSSendLength;/* End of data available in PqGSSSendBuffer */
6161
staticintPqGSSSendNext;/* Next index to send a byte from
6262
* PqGSSSendBuffer */
63-
staticintPqGSSSendConsumed;/* Number of*unencrypted* bytesconsumed for
64-
*current contents of PqGSSSendBuffer */
63+
staticintPqGSSSendConsumed;/* Number ofsource bytesencrypted but not
64+
*yet reported as sent */
6565

6666
staticchar*PqGSSRecvBuffer;/* Received, encrypted data */
6767
staticintPqGSSRecvLength;/* End of data available in PqGSSRecvBuffer */
@@ -83,8 +83,8 @@ static uint32 PqGSSMaxPktSize;/* Maximum size we can encrypt and fit the
8383
*
8484
* On success, returns the number of data bytes consumed (possibly less than
8585
* len). On failure, returns -1 with errno set appropriately. For retryable
86-
* errors, caller should call again (passing the same data) once the socket
87-
* is ready.
86+
* errors, caller should call again (passing the sameor moredata) once the
87+
*socketis ready.
8888
*
8989
* Dealing with fatal errors here is a bit tricky: we can't invoke elog(FATAL)
9090
* since it would try to write to the client, probably resulting in infinite
@@ -98,26 +98,33 @@ be_gssapi_write(Port *port, void *ptr, size_t len)
9898
minor;
9999
gss_buffer_descinput,
100100
output;
101-
size_tbytes_sent=0;
102101
size_tbytes_to_encrypt;
103102
size_tbytes_encrypted;
104103
gss_ctx_id_tgctx=port->gss->ctx;
105104

106105
/*
107-
* When we get a failure, we must not tell the caller we have successfully
108-
* transmitted everything, else it won't retry. Hence a "success"
109-
* (positive) return value must only count source bytes corresponding to
110-
* fully-transmitted encrypted packets. The amount of source data
111-
* corresponding to the current partly-transmitted packet is remembered in
106+
* When we get a retryable failure, we must not tell the caller we have
107+
* successfully transmitted everything, else it won't retry. For
108+
* simplicity, we claim we haven't transmitted anything until we have
109+
* successfully transmitted all "len" bytes. Between calls, the amount of
110+
* the current input data that's already been encrypted and placed into
111+
* PqGSSSendBuffer (and perhaps transmitted) is remembered in
112112
* PqGSSSendConsumed. On a retry, the caller *must* be sending that data
113113
* again, so if it offers a len less than that, something is wrong.
114+
*
115+
* Note: it may seem attractive to report partial write completion once
116+
* we've successfully sent any encrypted packets. However, that can cause
117+
* problems for callers; notably, pqPutMsgEnd's heuristic to send only
118+
* full 8K blocks interacts badly with such a hack. We won't save much,
119+
* typically, by letting callers discard data early, so don't risk it.
114120
*/
115121
if (len<PqGSSSendConsumed)
116122
{
117123
elog(COMMERROR,"GSSAPI caller failed to retransmit all data needing to be retried");
118124
errno=ECONNRESET;
119125
return-1;
120126
}
127+
121128
/* Discount whatever source data we already encrypted. */
122129
bytes_to_encrypt=len-PqGSSSendConsumed;
123130
bytes_encrypted=PqGSSSendConsumed;
@@ -146,33 +153,20 @@ be_gssapi_write(Port *port, void *ptr, size_t len)
146153

147154
ret=secure_raw_write(port,PqGSSSendBuffer+PqGSSSendNext,amount);
148155
if (ret <=0)
149-
{
150-
/*
151-
* Report any previously-sent data; if there was none, reflect
152-
* the secure_raw_write result up to our caller. When there
153-
* was some, we're effectively assuming that any interesting
154-
* failure condition will recur on the next try.
155-
*/
156-
if (bytes_sent)
157-
returnbytes_sent;
158156
returnret;
159-
}
160157

161158
/*
162159
* Check if this was a partial write, and if so, move forward that
163160
* far in our buffer and try again.
164161
*/
165-
if (ret!=amount)
162+
if (ret<amount)
166163
{
167164
PqGSSSendNext+=ret;
168165
continue;
169166
}
170167

171-
/* We've successfully sent whatever data was in that packet. */
172-
bytes_sent+=PqGSSSendConsumed;
173-
174-
/* All encrypted data was sent, our buffer is empty now. */
175-
PqGSSSendLength=PqGSSSendNext=PqGSSSendConsumed=0;
168+
/* We've successfully sent whatever data was in the buffer. */
169+
PqGSSSendLength=PqGSSSendNext=0;
176170
}
177171

178172
/*
@@ -196,7 +190,10 @@ be_gssapi_write(Port *port, void *ptr, size_t len)
196190
output.value=NULL;
197191
output.length=0;
198192

199-
/* Create the next encrypted packet */
193+
/*
194+
* Create the next encrypted packet. Any failure here is considered a
195+
* hard failure, so we return -1 even if some data has been sent.
196+
*/
200197
major=gss_wrap(&minor,gctx,1,GSS_C_QOP_DEFAULT,
201198
&input,&conf_state,&output);
202199
if (major!=GSS_S_COMPLETE)
@@ -239,10 +236,13 @@ be_gssapi_write(Port *port, void *ptr, size_t len)
239236
}
240237

241238
/* If we get here, our counters should all match up. */
242-
Assert(bytes_sent==len);
243-
Assert(bytes_sent==bytes_encrypted);
239+
Assert(len==PqGSSSendConsumed);
240+
Assert(len==bytes_encrypted);
241+
242+
/* We're reporting all the data as sent, so reset PqGSSSendConsumed. */
243+
PqGSSSendConsumed=0;
244244

245-
returnbytes_sent;
245+
returnbytes_encrypted;
246246
}
247247

248248
/*

‎src/interfaces/libpq/fe-secure-gssapi.c

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@
7979
* On success, returns the number of data bytes consumed (possibly less than
8080
* len). On failure, returns -1 with errno set appropriately. If the errno
8181
* indicates a non-retryable error, a message is added to conn->errorMessage.
82-
* For retryable errors, caller should call again (passing the samedata)
83-
* once the socket is ready.
82+
* For retryable errors, caller should call again (passing the sameor more
83+
*data)once the socket is ready.
8484
*/
8585
ssize_t
8686
pg_GSS_write(PGconn*conn,constvoid*ptr,size_tlen)
@@ -90,19 +90,25 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
9090
gss_buffer_descinput,
9191
output=GSS_C_EMPTY_BUFFER;
9292
ssize_tret=-1;
93-
size_tbytes_sent=0;
9493
size_tbytes_to_encrypt;
9594
size_tbytes_encrypted;
9695
gss_ctx_id_tgctx=conn->gctx;
9796

9897
/*
99-
* When we get a failure, we must not tell the caller we have successfully
100-
* transmitted everything, else it won't retry. Hence a "success"
101-
* (positive) return value must only count source bytes corresponding to
102-
* fully-transmitted encrypted packets. The amount of source data
103-
* corresponding to the current partly-transmitted packet is remembered in
98+
* When we get a retryable failure, we must not tell the caller we have
99+
* successfully transmitted everything, else it won't retry. For
100+
* simplicity, we claim we haven't transmitted anything until we have
101+
* successfully transmitted all "len" bytes. Between calls, the amount of
102+
* the current input data that's already been encrypted and placed into
103+
* PqGSSSendBuffer (and perhaps transmitted) is remembered in
104104
* PqGSSSendConsumed. On a retry, the caller *must* be sending that data
105105
* again, so if it offers a len less than that, something is wrong.
106+
*
107+
* Note: it may seem attractive to report partial write completion once
108+
* we've successfully sent any encrypted packets. However, that can cause
109+
* problems for callers; notably, pqPutMsgEnd's heuristic to send only
110+
* full 8K blocks interacts badly with such a hack. We won't save much,
111+
* typically, by letting callers discard data early, so don't risk it.
106112
*/
107113
if (len<PqGSSSendConsumed)
108114
{
@@ -140,33 +146,20 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
140146

141147
retval=pqsecure_raw_write(conn,PqGSSSendBuffer+PqGSSSendNext,amount);
142148
if (retval <=0)
143-
{
144-
/*
145-
* Report any previously-sent data; if there was none, reflect
146-
* the pqsecure_raw_write result up to our caller. When there
147-
* was some, we're effectively assuming that any interesting
148-
* failure condition will recur on the next try.
149-
*/
150-
if (bytes_sent)
151-
returnbytes_sent;
152149
returnretval;
153-
}
154150

155151
/*
156152
* Check if this was a partial write, and if so, move forward that
157153
* far in our buffer and try again.
158154
*/
159-
if (retval!=amount)
155+
if (retval<amount)
160156
{
161157
PqGSSSendNext+=retval;
162158
continue;
163159
}
164160

165-
/* We've successfully sent whatever data was in that packet. */
166-
bytes_sent+=PqGSSSendConsumed;
167-
168-
/* All encrypted data was sent, our buffer is empty now. */
169-
PqGSSSendLength=PqGSSSendNext=PqGSSSendConsumed=0;
161+
/* We've successfully sent whatever data was in the buffer. */
162+
PqGSSSendLength=PqGSSSendNext=0;
170163
}
171164

172165
/*
@@ -192,7 +185,7 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
192185

193186
/*
194187
* Create the next encrypted packet. Any failure here is considered a
195-
* hard failure, so we return -1 even ifbytes_sent > 0.
188+
* hard failure, so we return -1 even ifsome data has been sent.
196189
*/
197190
major=gss_wrap(&minor,gctx,1,GSS_C_QOP_DEFAULT,
198191
&input,&conf_state,&output);
@@ -236,10 +229,13 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
236229
}
237230

238231
/* If we get here, our counters should all match up. */
239-
Assert(bytes_sent==len);
240-
Assert(bytes_sent==bytes_encrypted);
232+
Assert(len==PqGSSSendConsumed);
233+
Assert(len==bytes_encrypted);
234+
235+
/* We're reporting all the data as sent, so reset PqGSSSendConsumed. */
236+
PqGSSSendConsumed=0;
241237

242-
ret=bytes_sent;
238+
ret=bytes_encrypted;
243239

244240
cleanup:
245241
/* Release GSSAPI buffer storage, if we didn't already */

‎src/interfaces/libpq/libpq-int.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -583,8 +583,8 @@ struct pg_conn
583583
intgss_SendLength;/* End of data available in gss_SendBuffer */
584584
intgss_SendNext;/* Next index to send a byte from
585585
* gss_SendBuffer */
586-
intgss_SendConsumed;/* Number of*unencrypted* bytesconsumed
587-
*for current contents of gss_SendBuffer */
586+
intgss_SendConsumed;/* Number ofsource bytesencrypted but
587+
*not yet reported as sent */
588588
char*gss_RecvBuffer;/* Received, encrypted data */
589589
intgss_RecvLength;/* End of data available in gss_RecvBuffer */
590590
char*gss_ResultBuffer;/* Decryption of data in gss_RecvBuffer */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp