Commit Graph

147 Commits

Author SHA1 Message Date
Tom Lane
077711c2e3 Remove arbitrary limitation on length of common name in SSL certificates.
Both libpq and the backend would truncate a common name extracted from a
certificate at 32 bytes.  Replace that fixed-size buffer with dynamically
allocated string so that there is no hard limit.  While at it, remove the
code for extracting peer_dn, which we weren't using for anything; and
don't bother to store peer_cn longer than we need it in libpq.

This limit was not so terribly unreasonable when the code was written,
because we weren't using the result for anything critical, just logging it.
But now that there are options for checking the common name against the
server host name (in libpq) or using it as the user's name (in the server),
this could result in undesirable failures.  In the worst case it even seems
possible to spoof a server name or user name, if the correct name is
exactly 32 bytes and the attacker can persuade a trusted CA to issue a
certificate in which that string is a prefix of the certificate's common
name.  (To exploit this for a server name, he'd also have to send the
connection astray via phony DNS data or some such.)  The case that this is
a realistic security threat is a bit thin, but nonetheless we'll treat it
as one.

Back-patch to 8.4.  Older releases contain the faulty code, but it's not
a security problem because the common name wasn't used for anything
interesting.

Reported and patched by Heikki Linnakangas

Security: CVE-2012-0867
2012-02-23 15:48:04 -05:00
Bruce Momjian
e126958c2e Update copyright notices for year 2012. 2012-01-01 18:01:58 -05:00
Magnus Hagander
5b40677986 Treat ENOTDIR as ENOENT when looking for client certificate file
This makes it possible to use a libpq app with home directory set
to /dev/null, for example - treating it the same as if the file
doesn't exist (which it doesn't).

Per bug #6302, reported by Diego Elio Petteno
2011-12-03 15:05:24 +01:00
Magnus Hagander
64aea1ebc7 Add libpq connection option to disable SSL compression
This can be used to remove the overhead of SSL compression on
fast networks.

Laurenz Albe
2011-11-28 13:13:42 +01:00
Tom Lane
bcf23ba4bf Fix previous patch so it also works if not USE_SSL (mea culpa).
On balance, the need to cover this case changes my mind in favor of pushing
all error-message generation duties into the two fe-secure.c routines.
So do it that way.
2011-07-24 23:29:03 -04:00
Tom Lane
fee476da95 Improve libpq's error reporting for SSL failures.
In many cases, pqsecure_read/pqsecure_write set up useful error messages,
which were then overwritten with useless ones by their callers.  Fix this
by defining the responsibility to set an error message to be entirely that
of the lower-level function when using SSL.

Back-patch to 8.3; the code is too different in 8.2 to be worth the
trouble.
2011-07-24 16:29:07 -04:00
Tom Lane
d0c23026b2 Use OpenSSL's SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER flag.
This disables an entirely unnecessary "sanity check" that causes failures
in nonblocking mode, because OpenSSL complains if we move or compact the
write buffer.  The only actual requirement is that we not modify pending
data once we've attempted to send it, which we don't.  Per testing and
research by Martin Pihlak, though this fix is a lot simpler than his patch.

I put the same change into the backend, although it's less clear whether
it's necessary there.  We do use nonblock mode in some situations in
streaming replication, so seems best to keep the same behavior in the
backend as in libpq.

Back-patch to all supported releases.
2011-07-24 15:17:51 -04:00
Bruce Momjian
bf50caf105 pgindent run before PG 9.1 beta 1. 2011-04-10 11:42:00 -04:00
Tom Lane
bd58d9d883 In initialize_SSL, don't fail unnecessarily when home dir is unavailable.
Instead, just act as though the certificate file(s) are not present.
There is only one case where this need be a hard failure condition: when
sslmode is verify-ca or verify-full, not having a root cert file is an
error.  Change the logic so that we complain only in that case, and
otherwise fall through cleanly.  This is how it used to behave pre-9.0,
but my patch 4ed4b6c54e of 2010-05-26 broke
the case.  Per report from Christian Kastner.
2011-03-04 11:38:45 -05:00
Bruce Momjian
5d950e3b0c Stamp copyrights for year 2011. 2011-01-01 13:18:15 -05:00
Magnus Hagander
9f2e211386 Remove cvs keywords from all files. 2010-09-20 22:08:53 +02:00
Tom Lane
d494e685c5 Allow full SSL certificate verification (wherein libpq checks its host name
parameter against server cert's CN field) to succeed in the case where
both host and hostaddr are specified.  As with the existing precedents
for Kerberos, GSSAPI, SSPI, it is the calling application's responsibility
that host and hostaddr match up --- we just use the host name as given.
Per bug #5559 from Christopher Head.

In passing, make the error handling and messages for the no-host-name-given
failure more consistent among these four cases, and correct a lie in the
documentation: we don't attempt to reverse-lookup host from hostaddr
if host is missing.

Back-patch to 8.4 where SSL cert verification was introduced.
2010-07-14 17:09:45 +00:00
Bruce Momjian
239d769e7e pgindent run for 9.0, second run 2010-07-06 19:19:02 +00:00
Tom Lane
4ed4b6c54e Rearrange libpq's SSL initialization to simplify it and make it handle some
additional cases correctly.  The original coding failed to load additional
(chain) certificates from the client cert file, meaning that indirectly signed
client certificates didn't work unless one hacked the server's root.crt file
to include intermediate CAs (not the desired approach).  Another problem was
that everything got loaded into the shared SSL_context object, which meant
that concurrent connections trying to use different sslcert settings could
well fail due to conflicting over the single available slot for a keyed
certificate.

To fix, get rid of the use of SSL_CTX_set_client_cert_cb(), which is
deprecated anyway in the OpenSSL documentation, and instead just
unconditionally load the client cert and private key during connection
initialization.  This lets us use SSL_CTX_use_certificate_chain_file(),
which does the right thing with additional certs, and is lots simpler than
the previous hacking about with BIO-level access.  A small disadvantage is
that we have to load the primary client cert a second time with
SSL_use_certificate_file, so that that one ends up in the correct slot
within the connection's SSL object where it can get paired with the key.
Given the other overhead of making an SSL connection, that doesn't seem
worth worrying about.

Per discussion ensuing from bug #5468.
2010-05-26 21:39:27 +00:00
Tom Lane
20d629320b Add missing newlines to some SSL-related error messages. Noted while testing. 2010-05-25 22:03:27 +00:00
Bruce Momjian
65e806cba1 pgindent run for 9.0 2010-02-26 02:01:40 +00:00
Bruce Momjian
0239800893 Update copyright for the year 2010. 2010-01-02 16:58:17 +00:00
Tom Lane
4847d5956c Set errno to zero before invoking SSL_read or SSL_write. It appears that
at least in some Windows versions, these functions are capable of returning
a failure indication without setting errno.  That puts us into an infinite
loop if the previous value happened to be EINTR.  Per report from Brendan
Hill.

Back-patch to 8.2.  We could take it further back, but since this is only
known to be an issue on Windows and we don't support Windows before 8.2,
it does not seem worth the trouble.
2009-12-30 03:45:46 +00:00
Magnus Hagander
abf23ee86d Reject certificates with embedded NULLs in the commonName field. This stops
attacks where an attacker would put <attack>\0<propername> in the field and
trick the validation code that the certificate was for <attack>.

This is a very low risk attack since it reuqires the attacker to trick the
CA into issuing a certificate with an incorrect field, and the common
PostgreSQL deployments are with private CAs, and not external ones. Also,
default mode in 8.4 does not do any name validation, and is thus also not
vulnerable - but the higher security modes are.

Backpatch all the way. Even though versions 8.3.x and before didn't have
certificate name validation support, they still exposed this field for
the user to perform the validation in the application code, and there
is no way to detect this problem through that API.

Security: CVE-2009-4034
2009-12-09 06:37:06 +00:00
Tom Lane
cea80e726e Avoid extra system calls to block SIGPIPE if the platform provides either
sockopt(SO_NOSIGPIPE) or the MSG_NOSIGNAL flag to send().

We assume these features are available if (1) the symbol is defined at
compile time and (2) the kernel doesn't reject the call at runtime.
It might turn out that there are some platforms where (1) and (2) are
true and yet the signal isn't really blocked, in which case applications
would die on server crash.  If that sort of thing gets reported, then
we'll have to add additional defenses of some kind.

Jeremy Kerr
2009-07-24 17:58:31 +00:00
Magnus Hagander
a93e7432cf Properly initialize SSL engines when used from libpq. This is required for
most external engines.

Per report and initial code from Lars Kanis
2009-06-23 18:13:23 +00:00
Bruce Momjian
d747140279 8.4 pgindent run, with new combined Linux/FreeBSD/MinGW typedef list
provided by Andrew.
2009-06-11 14:49:15 +00:00
Tom Lane
ac73ced725 Fix already-obsolete hint message ... sslverify parameter is no more. 2009-05-03 17:16:58 +00:00
Magnus Hagander
e883d0b551 Remove sslverify parameter again, replacing it with two new sslmode values:
"verify-ca" and "verify-full".

Since "prefer" remains the default, this will make certificate validation
off by default, which should lead to less upgrade issues.
2009-04-24 09:43:10 +00:00
Bruce Momjian
418fd59663 Add libpq error message text on how to handle missing root.crt file. 2009-04-14 17:30:16 +00:00
Tom Lane
97503a5200 Add PQinitOpenSSL() function to support applications that use libcrypto
but not OpenSSL (or perhaps vice versa, if that's possible).

Andrew Chernow, with minor editorialization by me.
2009-03-31 01:41:27 +00:00
Bruce Momjian
b86a71c8f4 Clarify variable naming: pq_initssllib -> pq_init_ssl_lib 2009-03-28 18:48:55 +00:00
Bruce Momjian
3ab95c2ab0 Better document PQinitSSL(0) behavior in regards to libcrypto. 2009-03-28 01:36:11 +00:00
Magnus Hagander
16c46d5d7a Go over all OpenSSL return values and make sure we compare them
to the documented API value. The previous code got it right as
it's implemented, but accepted too much/too little compared to
the API documentation.

Per comment from Zdenek Kotala.
2009-01-28 15:06:47 +00:00
Tom Lane
cc1d292d78 Fix accidental (I suppose) introduction of non-ASCII quote marks. 2009-01-19 17:17:50 +00:00
Peter Eisentraut
0d8c613180 Message wordsmithing 2009-01-19 08:59:13 +00:00
Magnus Hagander
75eafe965e Don't require pqGetHomeDirectory to succeed if the user has specified
hardcoded paths for SSL rootcert/crl/clientcert/key.

As noted by Andrew Chernow
2009-01-07 12:02:46 +00:00
Bruce Momjian
511db38ace Update copyright for 2009. 2009-01-01 17:24:05 +00:00
Magnus Hagander
5f3724dd7c Support specifying filename for SSL certificate, key, root certificate store
and certificate revokation list by using connection parameters or environment
variables.

Original patch by Mark Woodward, heavily reworked by Alvaro Herrera and
Magnus Hagander.
2008-12-15 10:28:22 +00:00
Magnus Hagander
30c52532d2 Comment said we don't free the lockarray, and why. The proper fix is to
make the code do what the comment says...
2008-12-04 14:07:42 +00:00
Bruce Momjian
98be3b49be Fix typo in recent SSL unload patch.
Kris Jurka
2008-12-04 02:52:31 +00:00
Magnus Hagander
4e81628653 Properly unregister OpenSSL callbacks when libpq is done with
it's connection. This is required for applications that unload
the libpq library (such as PHP) in which case we'd otherwise
have pointers to these functions when they no longer exist.

This needs a bit more testing before we can consider a backpatch,
so not doing that yet.

In passing, remove unused functions in backend/libpq.

Bruce Momjian and Magnus Hagander, per report and analysis
by Russell Smith.
2008-12-03 20:04:26 +00:00
Magnus Hagander
2c69fa0c38 Change wildcard cerfificate mapping to be much simler - we now only match
the * character at the beginning of a pattern, and it does not match
subdomains.

Since this means we no longer need fnmatch, remove the imported implementation
from port, along with the autoconf check for it.
2008-12-02 10:39:31 +00:00
Magnus Hagander
47ed197c37 Disable FNM_CASEFOLD. Need a proper solution later, but just comment
it out for now so the buildfarm recovers.
2008-11-24 19:19:46 +00:00
Magnus Hagander
cb10467d30 Add support for matching wildcard server certificates to the new SSL code.
This uses the function fnmatch() which is not available on all platforms
(notably Windows), so import the implementation from NetBSD into src/port.
2008-11-24 09:15:16 +00:00
Magnus Hagander
c89404edf3 Fix libpq certificate validation for SSL connections.
Add config parameter "sslverify" to control the verification. Default
is to do full verification.

Clean up some old SSL code that never really worked.
2008-11-13 09:45:25 +00:00
Magnus Hagander
bb8c822dbf Remove notes from the frontend SSL source that are incorrect or
end-user documentation that lives in the actual documentation.
2008-10-24 12:29:11 +00:00
Magnus Hagander
1d89026788 Implement error checking for pthreads calls in thread-safe mode. They really
should always succeed, but in the likely event of a failure we would
previously fall through *without locking* - the new code will exit(1).

Printing the error message on stderr will not work for all applications, but
it's better than nothing at all - and our API doesn't provide a way to return
the error to the caller.
2008-05-16 18:30:53 +00:00
Tom Lane
3405f2b925 Use error message wordings for permissions checks on .pgpass and SSL private
key files that are similar to the one for the postmaster's data directory
permissions check.  (I chose to standardize on that one since it's the most
heavily used and presumably best-wordsmithed by now.)  Also eliminate explicit
tests on file ownership in these places, since the ensuing read attempt must
fail anyway if it's wrong, and there seems no value in issuing the same error
message for distinct problems.  (But I left in the explicit ownership test in
postmaster.c, since it had its own error message anyway.)  Also be more
specific in the documentation's descriptions of these checks.  Per a gripe
from Kevin Hunter.
2008-03-31 02:43:14 +00:00
Bruce Momjian
d4fa0b4e0a Rename a libpq NOT_USED SSL function to
verify_peer_name_matches_certificate(), clarify some of the function's
variables and logic, and update a comment.  This should make SSL
improvements easier in the future.
2008-02-16 21:03:30 +00:00
Tom Lane
fad2b99523 Arrange to ignore SIGPIPE during SSL_read() and SSL_shutdown(), as these
are known to write on the socket sometimes and thus we are vulnerable to
being killed by the signal if the server happens to go away unexpectedly.
Noticed while trying (futilely) to reproduce bug #3902.

This bug has been there all along, but since the situation is usually
only of interest to developers, I chose not to back-patch the changes.
2008-01-29 02:03:39 +00:00
Bruce Momjian
9098ab9e32 Update copyrights in source tree to 2008. 2008-01-01 19:46:01 +00:00
Bruce Momjian
fdf5a5efb7 pgindent run for 8.3. 2007-11-15 21:14:46 +00:00
Tom Lane
fdbce93b85 Minor improvements to hack for old OpenSSL libraries: avoid unused
variable warning on Windows, improve comment.
2007-10-03 15:12:45 +00:00
Magnus Hagander
76a6ddfa47 Attempt to open certificate file "manually" using fopen before
trying BIO functions.
Helps problem with older versions of OpenSSL that lacks error
stack functions and would show an incorrect error message for
file-not-found-or-not-openable. The problem may still exist for
other errors, but file open error is by far the most common one.
2007-10-03 13:57:52 +00:00