Commit graph

1975 commits

Author SHA1 Message Date
Tom Lane
a644f5fc66 Don't put library-supplied -L/-I switches before user-supplied ones.
For many optional libraries, we extract the -L and -l switches needed
to link the library from a helper program such as llvm-config.  In
some cases we put the resulting -L switches into LDFLAGS ahead of
-L switches specified via --with-libraries.  That risks breaking
the user's intention for --with-libraries.

It's not such a problem if the library's -L switch points to a
directory containing only that library, but on some platforms a
library helper may "helpfully" offer a switch such as -L/usr/lib
that points to a directory holding all standard libraries.  If the
user specified --with-libraries in hopes of overriding the standard
build of some library, the -L/usr/lib switch prevents that from
happening since it will come before the user-specified directory.

To fix, avoid inserting these switches directly into LDFLAGS during
configure, instead adding them to LIBDIRS or SHLIB_LINK.  They will
still eventually get added to LDFLAGS, but only after the switches
coming from --with-libraries.

The same problem exists for -I switches: those coming from
--with-includes should appear before any coming from helper programs
such as llvm-config.  We have not heard field complaints about this
case, but it seems certain that a user attempting to override a
standard library could have issues.

The changes for this go well beyond configure itself, however,
because many Makefiles have occasion to manipulate CPPFLAGS to
insert locally-desirable -I switches, and some of them got it wrong.
The correct ordering is any -I switches pointing at within-the-
source-tree-or-build-tree directories, then those from the tree-wide
CPPFLAGS, then those from helper programs.  There were several places
that risked pulling in a system-supplied copy of libpq headers, for
example, instead of the in-tree files.  (Commit cb36f8ec2 fixed one
instance of that a few months ago, but this exercise found more.)

The Meson build scripts may or may not have any comparable problems,
but I'll leave it to someone else to investigate that.

Reported-by: Charles Samborski <demurgos@demurgos.net>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/70f2155f-27ca-4534-b33d-7750e20633d7@demurgos.net
Backpatch-through: 13
2025-07-29 15:17:41 -04:00
Tom Lane
3f10d2b665 Fix PQport to never return NULL unless the connection is NULL.
This is the documented behavior, and it worked that way before
v10.  However, addition of the connhost[] array created cases
where conn->connhost[conn->whichhost].port is NULL.  The rest
of libpq is careful to substitute DEF_PGPORT[_STR] for a null
or empty port string, but we failed to do so here, leading to
possibly returning NULL.  As of v18 that causes psql's \conninfo
command to segfault.  Older psql versions avoid that, but it's
pretty likely that other clients have trouble with this,
so we'd better back-patch the fix.

In stable branches, just revert to our historical behavior of
returning an empty string when there was no user-given port
specification.  However, it seems substantially more useful and
indeed more correct to hand back DEF_PGPORT_STR in such cases,
so let's make v18 and master do that.

Author: Daniele Varrazzo <daniele.varrazzo@gmail.com>
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA+mi_8YTS8WPZPO0PAb2aaGLwHuQ0DEQRF0ZMnvWss4y9FwDYQ@mail.gmail.com
Backpatch-through: 13
2025-07-17 12:46:58 -04:00
Tom Lane
445bd37b19 Correctly copy the target host identification in PQcancelCreate.
PQcancelCreate failed to copy struct pg_conn_host's "type" field,
instead leaving it zero (a/k/a CHT_HOST_NAME).  This seemingly
has no great ill effects if it should have been CHT_UNIX_SOCKET
instead, but if it should have been CHT_HOST_ADDRESS then a
null-pointer dereference will occur when the cancelConn is used.

Bug: #18974
Reported-by: Maxim Boguk <maxim.boguk@gmail.com>
Author: Sergei Kornilov <sk@zsrv.org>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/18974-575f02b2168b36b3@postgresql.org
Backpatch-through: 17
2025-07-02 15:47:59 -04:00
Tom Lane
30e0d9ee90 Don't reduce output request size on non-Unix-socket connections.
Traditionally, libpq's pqPutMsgEnd has rounded down the amount-to-send
to be a multiple of 8K when it is eagerly writing some data.  This
still seems like a good idea when sending through a Unix socket, as
pipes typically have a buffer size of 8K or some fraction/multiple of
that.  But there's not much argument for it on a TCP connection, since
(a) standard MTU values are not commensurate with that, and (b) the
kernel typically applies its own packet splitting/merging logic.

Worse, our SSL and GSSAPI code paths both have API stipulations that
if they fail to send all the data that was offered in the previous
write attempt, we mustn't offer less data in the next attempt; else
we may get "SSL error: bad length" or "GSSAPI caller failed to
retransmit all data needing to be retried".  The previous write
attempt might've been pqFlush attempting to send everything in the
buffer, so pqPutMsgEnd can't safely write less than the full buffer
contents.  (Well, we could add some more state to track exactly how
much the previous write attempt was, but there's little value evident
in such extra complication.)  Hence, apply the round-down only on
AF_UNIX sockets, where we never use SSL or GSSAPI.

Interestingly, we had a very closely related bug report before,
which I attempted to fix in commit d053a879b.  But the test case
we had then seemingly didn't trigger this pqFlush-then-pqPutMsgEnd
scenario, or at least we failed to recognize this variant of the bug.

Bug: #18907
Reported-by: Dorjpalam Batbaatar <htgn.dbat.95@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/18907-d41b9bcf6f29edda@postgresql.org
Backpatch-through: 13
2025-06-10 18:39:34 -04:00
Tom Lane
8b0aa7a6b7 Allow larger packets during GSSAPI authentication exchange.
Our GSSAPI code only allows packet sizes up to 16kB.  However it
emerges that during authentication, larger packets might be needed;
various authorities suggest 48kB or 64kB as the maximum packet size.
This limitation caused login failure for AD users who belong to many
AD groups.  To add insult to injury, we gave an unintelligible error
message, typically "GSSAPI context establishment error: The routine
must be called again to complete its function: Unknown error".

As noted in code comments, the 16kB packet limit is effectively a
protocol constant once we are doing normal data transmission: the
GSSAPI code splits the data stream at those points, and if we change
the limit then we will have cross-version compatibility problems
due to the receiver's buffer being too small in some combinations.
However, during the authentication exchange the packet sizes are
not determined by us, but by the underlying GSSAPI library.  So we
might as well just try to send what the library tells us to.
An unpatched recipient will fail on a packet larger than 16kB,
but that's not worse than the sender failing without even trying.
So this doesn't introduce any meaningful compatibility problem.

We still need a buffer size limit, but we can easily make it be
64kB rather than 16kB until transport negotiation is complete.
(Larger values were discussed, but don't seem likely to add
anything.)

Reported-by: Chris Gooch <cgooch@bamfunds.com>
Fix-suggested-by: Jacob Champion <jacob.champion@enterprisedb.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://postgr.es/m/DS0PR22MB5971A9C8A3F44BCC6293C4DABE99A@DS0PR22MB5971.namprd22.prod.outlook.com
Backpatch-through: 13
2025-05-30 12:55:15 -04:00
Noah Misch
ec5f89e8a2 With GB18030, prevent SIGSEGV from reading past end of allocation.
With GB18030 as source encoding, applications could crash the server via
SQL functions convert() or convert_from().  Applications themselves
could crash after passing unterminated GB18030 input to libpq functions
PQescapeLiteral(), PQescapeIdentifier(), PQescapeStringConn(), or
PQescapeString().  Extension code could crash by passing unterminated
GB18030 input to jsonapi.h functions.  All those functions have been
intended to handle untrusted, unterminated input safely.

A crash required allocating the input such that the last byte of the
allocation was the last byte of a virtual memory page.  Some malloc()
implementations take measures against that, making the SIGSEGV hard to
reach.  Back-patch to v13 (all supported versions).

Author: Noah Misch <noah@leadboat.com>
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Backpatch-through: 13
Security: CVE-2025-4207
2025-05-05 04:52:07 -07:00
Peter Eisentraut
954aacaee3 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: ff466b83eb6fbf9434ff087426546e3dc988135d
2025-05-05 12:14:36 +02:00
Álvaro Herrera
d0ed7d2a5a
Translation updates
Source-Git-URL: ssh://git@git.postgresql.org/pgtranslation/messages.git
Source-Git-Hash: 4b3f97f98ced3e9b03a6b24a16ac06eec2eab330
2025-02-17 17:51:30 +01:00
Tom Lane
3abe6e04cc Make escaping functions retain trailing bytes of an invalid character.
Instead of dropping the trailing byte(s) of an invalid or incomplete
multibyte character, replace only the first byte with a known-invalid
sequence, and process the rest normally.  This seems less likely to
confuse incautious callers than the behavior adopted in 5dc1e42b4.

While we're at it, adjust PQescapeStringInternal to produce at most
one bleat about invalid multibyte characters per string.  This
matches the behavior of PQescapeInternal, and avoids the risk of
producing tons of repetitive junk if a long string is simply given
in the wrong encoding.

This is a followup to the fixes for CVE-2025-1094, and should be
included if cherry-picking those fixes.

Author: Andres Freund <andres@anarazel.de>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Reported-by: Jeff Davis <pgsql@j-davis.com>
Discussion: https://postgr.es/m/20250215012712.45@rfd.leadboat.com
Backpatch-through: 13
2025-02-15 16:20:21 -05:00
Andres Freund
a92db3d02d Fix PQescapeLiteral()/PQescapeIdentifier() length handling
In 5dc1e42b4f I fixed bugs in various escape functions, unfortunately as part
of that I introduced a new bug in PQescapeLiteral()/PQescapeIdentifier(). The
bug is that I made PQescapeInternal() just use strlen(), rather than taking
the specified input length into account.

That's bad, because it can lead to including input that wasn't intended to be
included (in case len is shorter than null termination of the string) and
because it can lead to reading invalid memory if the input string is not null
terminated.

Expand test_escape to this kind of bug:

a) for escape functions with length support, append data that should not be
   escaped and check that it is not

b) add valgrind requests to detect access of bytes that should not be touched

Author: Tom Lane <tgl@sss.pgh.pa.us>
Author: Andres Freund <andres@anarazel.de
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/Z64jD3u46gObCo1p@pryzbyj2023
Backpatch: 13
2025-02-14 18:09:21 -05:00
Andres Freund
43a77239d4 Fix handling of invalidly encoded data in escaping functions
Previously invalidly encoded input to various escaping functions could lead to
the escaped string getting incorrectly parsed by psql.  To be safe, escaping
functions need to ensure that neither invalid nor incomplete multi-byte
characters can be used to "escape" from being quoted.

Functions which can report errors now return an error in more cases than
before. Functions that cannot report errors now replace invalid input bytes
with a byte sequence that cannot be used to escape the quotes and that is
guaranteed to error out when a query is sent to the server.

The following functions are fixed by this commit:
- PQescapeLiteral()
- PQescapeIdentifier()
- PQescapeString()
- PQescapeStringConn()
- fmtId()
- appendStringLiteral()

Reported-by: Stephen Fewer <stephen_fewer@rapid7.com>
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Backpatch-through: 13
Security: CVE-2025-1094
2025-02-10 10:03:38 -05:00
Peter Eisentraut
439776ba64 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: d20e57fd51cef4a7c57d9802f8719ef6f442c9c3
2025-02-10 15:00:55 +01:00
Andres Freund
73ed502ea6 meson: Add missing dependencies for libpq tests
The missing dependency was, e.g., visible when doing
  ninja clean && ninja meson-test-prereq && meson test --no-rebuild --suite setup --suite libpq

This is a bit more complicated than other related fixes, because until now
libpq's tests depended on 'frontend_code', which includes a dependency on
fe_utils, which in turns on libpq. That in turn required
src/interfaces/libpq/test to be entered from the top-level, not from
libpq/meson.build.  Because of that the test definitions in libpq/meson.build
could not declare a dependency on the binaries defined in
libpq/test/meson.build.

To fix this, this commit creates frontend_no_fe_utils_code, which allows us to
recurse into libpq/test from withing libpq/meson.build.

Apply this to all branches with meson support, as part of an effort to fix
incorrect test dependencies that can lead to test failures.

Discussion: https://postgr.es/m/CAGECzQSvM3iSDmjF+=Kof5an6jN8UbkP_4cKKT9w6GZavmb5yQ@mail.gmail.com
Discussion: https://postgr.es/m/bdba588f-69a9-4f3e-9b95-62d07210a32e@eisentraut.org
Backpatch: 16-, where meson support was added
2025-02-04 17:56:20 -05:00
Tom Lane
a0dfeae0dc Avoid symbol collisions between pqsignal.c and legacy-pqsignal.c.
In the name of ABI stability (that is, to avoid a library major
version bump for libpq), libpq still exports a version of pqsignal()
that we no longer want to use ourselves.  However, since that has
the same link name as the function exported by src/port/pqsignal.c,
there is a link ordering dependency determining which version will
actually get used by code that uses libpq as well as libpgport.a.

It now emerges that the wrong version has been used by pgbench and
psql since commit 06843df4a rearranged their link commands.  This
can result in odd failures in pgbench with the -T switch, since its
SIGALRM handler will now not be marked SA_RESTART.  psql may have
some edge-case problems in \watch, too.

Since we don't want to depend on link ordering effects anymore,
let's fix this in the same spirit as b6c7cfac8: use macros to change
the actual link names of the competing functions.  We cannot change
legacy-pqsignal.c's exported name of course, so the victim has to be
src/port/pqsignal.c.

In master, rename its exported name to be pqsignal_fe in frontend or
pqsignal_be in backend.  (We could perhaps have gotten away with using
the same symbol in both cases, but since the FE and BE versions now
work a little differently, it seems advisable to use different names.)

In back branches, rename to pqsignal_fe in frontend but keep it as
pqsignal in backend.  The frontend change could affect third-party
code that is calling pqsignal from libpgport.a or libpgport_shlib.a,
but only if the code is compiled against port.h from a different minor
release than libpgport.  Since we don't support using libpgport as a
shared library, it seems unlikely that there will be such a problem.
I left the backend symbol unchanged to avoid an ABI break for
extensions.  This means that the link ordering hazard still exists
for any extension that links against libpq.  However, none of our own
extensions use both pqsignal() and libpq, and we're not making things
any worse for third-party extensions that do.

Report from Andy Fan, diagnosis by Fujii Masao, patch by me.
Back-patch to all supported branches, as 06843df4a was.

Discussion: https://postgr.es/m/87msfz5qv2.fsf@163.com
2025-01-14 18:50:24 -05:00
Peter Eisentraut
6bf5bf11c3 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 2592030f456910263c8972668576f954fce10595
2024-11-11 13:52:24 +01:00
Michael Paquier
a5cc4c6671 libpq: Bail out during SSL/GSS negotiation errors
This commit changes libpq so that errors reported by the backend during
the protocol negotiation for SSL and GSS are discarded by the client, as
these may include bytes that could be consumed by the client and write
arbitrary bytes to a client's terminal.

A failure with the SSL negotiation now leads to an error immediately
reported, without a retry on any other methods allowed, like a fallback
to a plaintext connection.

A failure with GSS discards the error message received, and we allow a
fallback as it may be possible that the error is caused by a connection
attempt with a pre-11 server, GSS encryption having been introduced in
v12.  This was a problem only with v17 and newer versions; older
versions discard the error message already in this case, assuming a
failure caused by a lack of support for GSS encryption.

Author: Jacob Champion
Reviewed-by: Peter Eisentraut, Heikki Linnakangas, Michael Paquier
Security: CVE-2024-10977
Backpatch-through: 12
2024-11-11 10:19:56 +09:00
Tom Lane
c7a201053e Parse libpq's "keepalives" option more like other integer options.
Use pqParseIntParam (nee parse_int_param) instead of using strtol
directly.  This allows trailing whitespace, which the previous coding
didn't, and makes the spelling of the error message consistent with
other similar cases.

This seems to be an oversight in commit e7a221797, which introduced
parse_int_param.  That fixed places that were using atoi(), but missed
this place which was randomly using strtol() instead.

Ordinarily I'd consider this minor cleanup not worth back-patching.
However, it seems that ecpg assumes it can add trailing whitespace
to URL parameters, so that use of the keepalives option fails in
that context.  Perhaps that's worth improving as a separate matter.
In the meantime, back-patch this to all supported branches.

Yuto Sasaki (some further cleanup by me)

Discussion: https://postgr.es/m/TY2PR01MB36286A7B97B9A15793335D18C1772@TY2PR01MB3628.jpnprd01.prod.outlook.com
2024-10-02 17:30:36 -04:00
Peter Eisentraut
29d483fb33 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 4b069f67b5be4227eb620a74c9900f079f2e59f4
2024-09-23 12:06:47 +02:00
Tom Lane
79c3012dc2 Provide feature-test macros for libpq features added in v17.
As per the policy established in commit 6991e774e, invent macros
that can be tested at compile time to detect presence of new libpq
features.  This should make calling code more readable and less
error-prone than checking the libpq version would be (especially
since we don't expose that at compile time; the server version is
an unreliable substitute).

Discussion: https://postgr.es/m/2042418.1724346970@sss.pgh.pa.us
2024-08-23 10:12:56 -04:00
Peter Eisentraut
91099bb287 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: f1fa38f3bf3e0a5d3a95304dcf6a11acf304577c
2024-08-05 12:12:32 +02:00
Peter Eisentraut
821fbd63ea libpq: Use strerror_r instead of strerror
Commit 453c468737 introduced a use of strerror() into libpq, but that
is not thread-safe.  Fix by using strerror_r() instead.

In passing, update some of the code comments added by 453c468737, as
we have learned more about the reason for the change in OpenSSL that
started this.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: Discussion: https://postgr.es/m/b6fb018b-f05c-4afd-abd3-318c649faf18@highgo.ca
2024-07-28 10:19:57 +02:00
Heikki Linnakangas
f06a632a77 Fix fallback behavior when server sends an ERROR early at startup
With sslmode=prefer, the desired behavior is to completely fail the
connection attempt, *not* fall back to a plaintext connection, if the
server responds to the SSLRequest with an error ('E') response instead
of rejecting SSL with an 'N' response. This was broken in commit
05fd30c0e7.

Reported-by: Jacob Champion
Reviewed-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com
Backpatch-through: 17
2024-07-26 15:02:29 +03:00
Heikki Linnakangas
5afebbe529 Fix outdated comment after removal of direct SSL fallback
The option to fall back from direct SSL to negotiated SSL or a
plaintext connection was removed in commit fb5718f35f.

Discussion: https://www.postgresql.org/message-id/c82ad227-e049-4e18-8898-475a748b5a5a@iki.fi
2024-07-08 12:44:56 +03:00
Alvaro Herrera
e72f841cbe
Fix copy/paste mistake in comment
Backpatch to 17

Author: Yugo NAGATA <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/20240704134638.355ad44a445fa1e764a220cd@sranhm.sraoss.co.jp
2024-07-04 13:57:47 +02:00
Alvaro Herrera
6d2ac55491
Fix copy-paste mistake in PQcancelCreate
When an OOM occurred, this function was incorrectly setting a status of
CONNECTION_BAD on the passed in PGconn instead of on the newly created
PGcancelConn.

Mistake introduced with 61461a300c.  Backpatch to 17.

Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20240630190040.26.nmisch@google.com
2024-07-01 13:58:22 +02:00
Nathan Bossart
32f07991b7 Use PqMsg_* macros in fe-auth.c.
Commit f4b54e1ed9, which introduced macros for protocol characters,
missed updating a few places in fe-auth.c.

Author: Jelte Fennema-Nio
Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS%2BYWXBhOGo%2BY1YecLgknF3g%40mail.gmail.com
2024-06-26 11:25:38 -05:00
Peter Eisentraut
f7f4e7e6fa Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 4409d73e450606ff15b428303d706f1d15c1f597
2024-06-24 13:11:27 +02:00
Tom Lane
105024a472 Improve the granularity of PQsocketPoll's timeout parameter.
Commit f5e4dedfa exposed libpq's internal function PQsocketPoll
without a lot of thought about whether that was an API we really
wanted to chisel in stone.  The main problem with it is the use of
time_t to specify the timeout.  While we do want an absolute time
so that a loop around PQsocketPoll doesn't have problems with
timeout slippage, time_t has only 1-second resolution.  That's
already problematic for libpq's own internal usage --- for example,
pqConnectDBComplete has long had a kluge to treat "connect_timeout=1"
as 2 seconds so that it doesn't accidentally round to nearly zero.
And it's even less likely to be satisfactory for external callers.
Hence, let's change this while we still can.

The best idea seems to be to use an int64 count of microseconds since
the epoch --- basically the same thing as the backend's TimestampTz,
but let's use the standard Unix epoch (1970-01-01) since that's more
likely for clients to be easy to calculate.  Millisecond resolution
would be plenty for foreseeable uses, but maybe the day will come that
we're glad we used microseconds.

Also, since time(2) isn't especially helpful for computing timeouts
defined this way, introduce a new function PQgetCurrentTimeUSec
to get the current time in this form.

Remove the hack in pqConnectDBComplete, so that "connect_timeout=1"
now means what you'd expect.

We can also remove the "#include <time.h>" that f5e4dedfa added to
libpq-fe.h, since there's no longer a need for time_t in that header.
It seems better for v17 not to enlarge libpq-fe.h's include footprint
from what it's historically been, anyway.

I also failed to resist the temptation to do some wordsmithing
on PQsocketPoll's documentation.

Patch by me, per complaint from Dominique Devienne.

Discussion: https://postgr.es/m/913559.1718055575@sss.pgh.pa.us
2024-06-13 15:14:32 -04:00
Peter Eisentraut
6ac5600a36 libpq: Some message style normalization 2024-06-13 07:10:35 +02:00
Peter Eisentraut
a0fe90efef libpq: Add missing gettext markers
Follow-up to 87d2801d4b: That commit restored some lost error
messages, but they ended up in a place where xgettext wouldn't find
them.  Rather than elevating ENCRYPTION_NEGOTIATION_FAILED() to a
gettext trigger, it's easiest for now to put in some explicit
libpq_gettext() calls in the couple of call sites.
2024-06-12 15:31:31 +02:00
Peter Eisentraut
d112ea4681 libpq: Remove a gettext marker
This one error message is just a workaround for a missing OpenSSL
error string.  But OpenSSL does not have gettext support, so we don't
need to provide it in our workaround either.  That way, the
user-facing behavior is consistent whether the user has a fixed
OpenSSL or not.
2024-06-12 08:43:43 +02:00
Peter Eisentraut
f376996bb7 Fix typo in error message 2024-06-12 04:48:39 +02:00
Daniel Gustafsson
4013a5ada2 Fix check for memory allocation
Commit 61461a300c accidentally checked memory allocation success
using the wrong variable.

Author: Ranier Vilela <ranier.vf@gmail.com>
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAEudQAqQFTH7xCB-+K6zEKjfqbhqCxcr_w4DuJTxVT6h3vzu2w@mail.gmail.com
2024-05-27 19:37:17 +02:00
Peter Eisentraut
18cbed13d5 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 647792ce18e56f51614f7559106ad15362c5d1cc
2024-05-20 12:04:11 +02:00
Peter Eisentraut
17974ec259 Revise GUC names quoting in messages again
After further review, we want to move in the direction of always
quoting GUC names in error messages, rather than the previous (PG16)
wildly mixed practice or the intermittent (mid-PG17) idea of doing
this depending on how possibly confusing the GUC name is.

This commit applies appropriate quotes to (almost?) all mentions of
GUC names in error messages.  It partially supersedes a243569bf6 and
8d9978a717, which had moved things a bit in the opposite direction
but which then were abandoned in a partial state.

Author: Peter Smith <smithpb2250@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAHut%2BPv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w%40mail.gmail.com
2024-05-17 11:44:26 +02:00
Heikki Linnakangas
fb5718f35f Remove option to fall back from direct to postgres SSL negotiation
There were three problems with the sslnegotiation options:

1. The sslmode=prefer and sslnegotiation=requiredirect combination was
somewhat dangerous, as you might unintentionally fall back to
plaintext authentication when connecting to a pre-v17 server.

2. There was an asymmetry between 'postgres' and 'direct'
options. 'postgres' meant "try only traditional negotiation", while
'direct' meant "try direct first, and fall back to traditional
negotiation if it fails". That was apparent only if you knew that the
'requiredirect' mode also exists.

3. The "require" word in 'requiredirect' suggests that it's somehow
more strict or more secure, similar to sslmode. However, I don't
consider direct SSL connections to be a security feature.

To address these problems:

- Only allow sslnegotiation='direct' if sslmode='require' or
stronger. And for the record, Jacob and Robert felt that we should do
that (or have sslnegotiation='direct' imply sslmode='require') anyway,
regardless of the first issue.

- Remove the 'direct' mode that falls back to traditional negotiation,
and rename what was called 'requiredirect' to 'direct' instead. In
other words, there is no "try both methods" option anymore, 'postgres'
now means the traditional negotiation and 'direct' means a direct SSL
connection.

Reviewed-by: Jelte Fennema-Nio, Robert Haas, Jacob Champion
Discussion: https://www.postgresql.org/message-id/d3b1608a-a1b6-4eda-9ec5-ddb3e4375808%40iki.fi
2024-05-16 17:17:37 +03:00
Tom Lane
da256a4a7f Pre-beta mechanical code beautification.
Run pgindent, pgperltidy, and reformat-dat-files.

The pgindent part of this is pretty small, consisting mainly of
fixing up self-inflicted formatting damage from patches that
hadn't bothered to add their new typedefs to typedefs.list.
In order to keep it from making anything worse, I manually added
a dozen or so typedefs that appeared in the existing typedefs.list
but not in the buildfarm's list.  Perhaps we should formalize that,
or better find a way to get those typedefs into the automatic list.

pgperltidy is as opinionated as always, and reformat-dat-files too.
2024-05-14 16:34:50 -04:00
Peter Eisentraut
7a31eb2aaa Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: be182cc55e6f72c66215fd9b38851969e3ce5480
2024-05-06 12:06:31 +02:00
Daniel Gustafsson
c34d7df6ad Fix comment regarding LibreSSL availability
SSL_AD_NO_APPLICATION_PROTOCOL is indeed available in LibreSSL, but only
in 3.4.3 and later (shipped in OpenBSD 7.0).

Discussion: https://postgr.es/m/E1s1g0Z-000jeC-OR@gemulon.postgresql.org
2024-05-05 09:47:35 +02:00
David Rowley
a42fc1c903 Fix an assortment of typos
Author: Alexander Lakhin
Discussion: https://postgr.es/m/ae9f2fcb-4b24-5bb0-4240-efbbbd944ca1@gmail.com
2024-05-04 02:33:25 +12:00
Peter Eisentraut
42510c031b Rename libpq trace internal functions
libpq's pqTraceOutputMessage() used to look like this:

    case 'Z':               /* Ready For Query */
        pqTraceOutputZ(conn->Pfdebug, message, &logCursor);
        break;

Commit f4b54e1ed9 introduced macros for protocol characters, so now
it looks like this:

    case PqMsg_ReadyForQuery:
        pqTraceOutputZ(conn->Pfdebug, message, &logCursor);
        break;

But this introduced a disconnect between the symbol in the switch case
and the function name to be called, so this made the manageability of
this file a bit worse.

This patch changes the function names to match, so now it looks like
this:

    case PqMsg_ReadyForQuery:
        pqTraceOutput_ReadyForQuery(conn->Pfdebug, message, &logCursor);
        break;

(This also improves the readability of the file in general, since some
function names like "pqTraceOutputt" were a little hard to read
accurately.)

Some protocol characters have different meanings to and from the
server.  The old code structure had a common function for both, for
example, pqTraceOutputD().  The new structure splits this up into
separate ones to match the protocol message name, like
pqTraceOutput_Describe() and pqTraceOutput_DataRow().

Reviewed-by: Yugo NAGATA <nagata@sraoss.co.jp>
Discussion: https://www.postgresql.org/message-id/flat/575e4f9d-acfe-45e3-b7f1-7e32c579090e%40eisentraut.org
2024-05-02 16:11:26 +02:00
Heikki Linnakangas
5bcbe9813b Fix compilation on OpenSSL 1.0.2 and LibreSSL
SSL_AD_NO_APPLICATION_PROTOCOL was introduced in OpenSSL 1.1.0.

While we're at it, add a link to the related OpenSSL github issue to
the comment.

Per buildfarm and Tom Lane.

Discussion: https://www.postgresql.org/message-id/1452995.1714433552@sss.pgh.pa.us
2024-04-30 08:22:24 +03:00
Heikki Linnakangas
17a834a04d Reject SSL connection if ALPN is used but there's no common protocol
If the client supports ALPN but tries to use some other protocol, like
HTTPS, reject the connection in the server. That is surely a confusion
of some sort. Furthermore, the ALPN RFC 7301 says:

> In the event that the server supports no protocols that the client
> advertises, then the server SHALL respond with a fatal
> "no_application_protocol" alert.

This commit makes the server follow that advice.

In the client, specifically check for the OpenSSL error code for the
"no_application_protocol" alert. Otherwise you got a cryptic "SSL
error: SSL error code 167773280" error if you tried to connect to a
non-PostgreSQL server that rejects the connection with
"no_application_protocol". ERR_reason_error_string() returns NULL for
that code, which frankly seems like an OpenSSL bug to me, but we can
easily print a better message ourselves.

Reported-by: Jacob Champion
Discussion: https://www.postgresql.org/message-id/6aedcaa5-60f3-49af-a857-2c76ba55a1f3@iki.fi
2024-04-29 18:12:26 +03:00
Heikki Linnakangas
03a0e0d4bb libpq: Enforce ALPN in direct SSL connections
ALPN is mandatory with direct SSL connections. That is documented, and
the server checks it, but libpq was missing the check.

Reported-by: Jacob Champion
Reviewed-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/CAOYmi+=sj+1uydS0NR4nYzw-LRWp3Q-s5speBug5UCLSPMbvGA@mail.gmail.com
2024-04-29 18:12:24 +03:00
Heikki Linnakangas
87d2801d4b libpq: Fix error messages when server rejects SSL or GSS
These messages were lost in commit 05fd30c0e7. Put them back.

This makes one change in the error message behavior compared to v16,
in the case that the server responds to GSSRequest with an error
instead of rejecting it with 'N'. Previously, libpq would hide the
error that the server sent, assuming that you got the error because
the server is an old pre-v12 version that doesn't understand the
GSSRequest message. A v11 server sends a "FATAL: unsupported frontend
protocol 1234.5680: server supports 2.0 to 3.0" error if you try to
connect to it with GSS. That was a reasonable assumption when the
feature was introduced, but v12 was released a long time ago and I
don't think it's the most probable cause anymore. The attached patch
changes things so that libpq prints the error message that the server
sent in that case, making the "server responds with error to
GSSRequest" case behave the same as the "server responds with error to
SSLRequest" case.

Reported-by: Peter Eisentraut
Discussion: https://www.postgresql.org/message-id/bb3b94da-afc7-438d-8940-cb946e553d9d@eisentraut.org
2024-04-29 18:12:21 +03:00
Heikki Linnakangas
3c18409265 libpq: If ALPN is not used, make PQsslAttribute(conn, "alpn") == ""
The documentation says that PQsslAttribute(conn, "alpn") returns an
empty string if ALPN is not used, but the code actually returned
NULL. Fix the code to match the documentation.

Reported-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/ZideNHji0G4gxmc3@paquier.xyz
2024-04-29 12:26:46 +03:00
Heikki Linnakangas
5c9f35fc48 Fix documentation and comments on what happens after GSS rejection
The paragraph in the docs and the comment applied to
sslnegotiaton=direct, but not sslnegotiation=requiredirect. In
'requiredirect' mode, negotiated SSL is never used. Move the paragraph
in the docs under the description of 'direct' mode, and rephrase it.

Also the comment's reference to reusing a plaintext connection was
bogus. Authentication failure in plaintext mode only happens after
sending the startup packet, so the connection cannot be reused.

Reported-by: Jacob Champion
Discussion: https://www.postgresql.org/message-id/CAOYmi+=sj+1uydS0NR4nYzw-LRWp3Q-s5speBug5UCLSPMbvGA@mail.gmail.com
2024-04-28 22:39:35 +03:00
Daniel Gustafsson
9c58bf1507 Fix incorrect parameter name in prototype
The function declaration for select_next_encryption_method use the
variable name have_valid_connection, so fix the prototype in the
header to match that.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/3F577953-A29E-4722-98AD-2DA9EFF2CBB8@yesql.se
2024-04-19 09:58:00 +02:00
Daniel Gustafsson
950d4a2cb1 Fix typos and duplicate words
This fixes various typos, duplicated words, and tiny bits of whitespace
mainly in code comments but also in docs.

Author: Daniel Gustafsson <daniel@yesql.se>
Author: Heikki Linnakangas <hlinnaka@iki.fi>
Author: Alexander Lakhin <exclusion@gmail.com>
Author: David Rowley <dgrowleyml@gmail.com>
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/3F577953-A29E-4722-98AD-2DA9EFF2CBB8@yesql.se
2024-04-18 21:28:07 +02:00
Peter Eisentraut
2e75492b3c Add missing source file to libpq/nls.mk 2024-04-17 09:11:02 +02:00