diff options
author | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-05-04 12:18:09 +0000 |
---|---|---|
committer | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-05-04 12:18:09 +0000 |
commit | ebe124eacd7c3faa36ed358e7cc1d7c5b419e5f6 (patch) | |
tree | 48fe9c3270176faa25c7d9fb47326f689be82fe4 /src/backend/libpq | |
parent | Releasing progress-linux version 15.5-0+deb12u1~progress6.99u1. (diff) | |
download | postgresql-15-ebe124eacd7c3faa36ed358e7cc1d7c5b419e5f6.tar.xz postgresql-15-ebe124eacd7c3faa36ed358e7cc1d7c5b419e5f6.zip |
Merging upstream version 15.6.
Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>
Diffstat (limited to '')
-rw-r--r-- | src/backend/libpq/auth.c | 12 | ||||
-rw-r--r-- | src/backend/libpq/be-secure-gssapi.c | 60 | ||||
-rw-r--r-- | src/backend/libpq/be-secure-openssl.c | 26 | ||||
-rw-r--r-- | src/backend/libpq/pqcomm.c | 22 |
4 files changed, 65 insertions, 55 deletions
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index efc53f3..235152e 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -850,15 +850,13 @@ CheckPWChallengeAuth(Port *port, const char **logdetail) if (shadow_pass) pfree(shadow_pass); - - /* - * If get_role_password() returned error, return error, even if the - * authentication succeeded. - */ - if (!shadow_pass) + else { + /* + * If get_role_password() returned error, authentication better not + * have succeeded. + */ Assert(auth_result != STATUS_OK); - return STATUS_ERROR; } if (auth_result == STATUS_OK) diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c index 2844c5a..d3337a3 100644 --- a/src/backend/libpq/be-secure-gssapi.c +++ b/src/backend/libpq/be-secure-gssapi.c @@ -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); + + /* We're reporting all the data as sent, so reset PqGSSSendConsumed. */ + PqGSSSendConsumed = 0; - return bytes_sent; + return bytes_encrypted; } /* diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index f5c5ed2..ed13e8b 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -457,6 +457,7 @@ aloop: * per-thread error queue following another call to an OpenSSL I/O * routine. */ + errno = 0; ERR_clear_error(); r = SSL_accept(port->ssl); if (r <= 0) @@ -493,7 +494,7 @@ aloop: WAIT_EVENT_SSL_OPEN_SERVER); goto aloop; case SSL_ERROR_SYSCALL: - if (r < 0) + if (r < 0 && errno != 0) ereport(COMMERROR, (errcode_for_socket_access(), errmsg("could not accept SSL connection: %m"))); @@ -727,7 +728,7 @@ be_tls_read(Port *port, void *ptr, size_t len, int *waitfor) break; case SSL_ERROR_SYSCALL: /* leave it to caller to ereport the value of errno */ - if (n != -1) + if (n != -1 || errno == 0) { errno = ECONNRESET; n = -1; @@ -785,8 +786,14 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor) n = -1; break; case SSL_ERROR_SYSCALL: - /* leave it to caller to ereport the value of errno */ - if (n != -1) + + /* + * Leave it to caller to ereport the value of errno. However, if + * errno is still zero then assume it's a read EOF situation, and + * report ECONNRESET. (This seems possible because SSL_write can + * also do reads.) + */ + if (n != -1 || errno == 0) { errno = ECONNRESET; n = -1; @@ -839,11 +846,6 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor) * to retry; do we need to adopt their logic for that? */ -#ifndef HAVE_BIO_GET_DATA -#define BIO_get_data(bio) (bio->ptr) -#define BIO_set_data(bio, data) (bio->ptr = data) -#endif - static BIO_METHOD *my_bio_methods = NULL; static int @@ -853,7 +855,7 @@ my_sock_read(BIO *h, char *buf, int size) if (buf != NULL) { - res = secure_raw_read(((Port *) BIO_get_data(h)), buf, size); + res = secure_raw_read(((Port *) BIO_get_app_data(h)), buf, size); BIO_clear_retry_flags(h); if (res <= 0) { @@ -873,7 +875,7 @@ my_sock_write(BIO *h, const char *buf, int size) { int res = 0; - res = secure_raw_write(((Port *) BIO_get_data(h)), buf, size); + res = secure_raw_write(((Port *) BIO_get_app_data(h)), buf, size); BIO_clear_retry_flags(h); if (res <= 0) { @@ -949,7 +951,7 @@ my_SSL_set_fd(Port *port, int fd) SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB); goto err; } - BIO_set_data(bio, port); + BIO_set_app_data(bio, port); BIO_set_fd(bio, fd, BIO_NOCLOSE); SSL_set_bio(port->ssl, bio, bio); diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c index 75392a8..bb9fa77 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -954,6 +954,8 @@ pq_recvbuf(void) { int r; + errno = 0; + r = secure_read(MyProcPort, PqRecvBuffer + PqRecvLength, PQ_RECV_BUFFER_SIZE - PqRecvLength); @@ -966,10 +968,13 @@ pq_recvbuf(void) * Careful: an ereport() that tries to write to the client would * cause recursion to here, leading to stack overflow and core * dump! This message must go *only* to the postmaster log. + * + * If errno is zero, assume it's EOF and let the caller complain. */ - ereport(COMMERROR, - (errcode_for_socket_access(), - errmsg("could not receive data from client: %m"))); + if (errno != 0) + ereport(COMMERROR, + (errcode_for_socket_access(), + errmsg("could not receive data from client: %m"))); return EOF; } if (r == 0) @@ -1046,6 +1051,8 @@ pq_getbyte_if_available(unsigned char *c) /* Put the socket into non-blocking mode */ socket_set_nonblocking(true); + errno = 0; + r = secure_read(MyProcPort, c, 1); if (r < 0) { @@ -1062,10 +1069,13 @@ pq_getbyte_if_available(unsigned char *c) * Careful: an ereport() that tries to write to the client would * cause recursion to here, leading to stack overflow and core * dump! This message must go *only* to the postmaster log. + * + * If errno is zero, assume it's EOF and let the caller complain. */ - ereport(COMMERROR, - (errcode_for_socket_access(), - errmsg("could not receive data from client: %m"))); + if (errno != 0) + ereport(COMMERROR, + (errcode_for_socket_access(), + errmsg("could not receive data from client: %m"))); r = EOF; } } |