Commit Graph

335 Commits

Author SHA1 Message Date
Michael Paquier d0071f9228 pgcrypto: Fix incorrect argument vs PG_GETARG*() mappings
The following functions use a mix of bytea and text arguments, but their
C internals always used PG_GETARG_BYTEA_PP(), creating an incorrect mix
with the argument types expected by encrypt_internal():
- pgp_sym_encrypt_bytea(bytea,text[,text])
- pgp_sym_encrypt(text,text[,text])
- pgp_sym_decrypt_bytea(bytea,text[,text])
- pgp_sym_decrypt(bytea,text[,text])
- pgp_pub_encrypt_bytea(bytea,bytea[,text])
- pgp_pub_encrypt(text,bytea[,text])
- pgp_pub_decrypt_bytea(bytea, bytea[,text[,text]])
- pgp_pub_decrypt(bytea,bytea[,text[,text]])

This commit fixes the inconsistencies between the PG_GETARG*() macros
and the argument types of each function.

Both BYTEA_PP() and TEXT_PP() rely on PG_DETOAST_DATUM_PACKED(), that
returns an unaligned pointer, so this was not leading to an actual
problem as far as I know, but let's be consistent.

Author: Shihao Zhong
Discussion: https://postgr.es/m/CAGRkXqRfiWT--DzVPx_UGpNHTt0YT5Jo8eV2CtT56jNP=QpXSQ@mail.gmail.com
2024-02-14 08:59:05 +09:00
Daniel Gustafsson b527ebc1d3 pgcrypto: Fix check for buffer size
The code copying the PGP block into the temp buffer failed to
account for the extra 2 bytes in the buffer which are needed
for the prefix. If the block was oversized, subsequent checks
of the prefix would have exceeded the buffer size.  Since the
block sizes are hardcoded in the list of supported ciphers it
can be verified that there is no live bug here. Backpatch all
the way for consistency though, as this bug is old.

Author: Mikhail Gribkov <youzhick@gmail.com>
Discussion: https://postgr.es/m/CAMEv5_uWvcMCMdRFDsJLz2Q8g16HEa9xWyfrkr+FYMMFJhawOw@mail.gmail.com
Backpatch-through: v12
2024-01-30 11:15:46 +01:00
Bruce Momjian 29275b1d17 Update copyright for 2024
Reported-by: Michael Paquier

Discussion: https://postgr.es/m/ZZKTDPxBBMt3C0J9@paquier.xyz

Backpatch-through: 12
2024-01-03 20:49:05 -05:00
Robert Haas 0d9937d118 Fix typos in comments and in one isolation test.
Dagfinn Ilmari Mannsåker, reviewed by Shubham Khanna. Some subtractions
by me.

Discussion: http://postgr.es/m/87le9fmi01.fsf@wibble.ilmari.org
2024-01-02 12:05:41 -05:00
Peter Eisentraut 795592865c pgcrypto: Allow tests to pass in OpenSSL FIPS mode
This adds several alternative expected files for when MD5 and 3DES are
not available.  This is similar to the alternative expected files for
when the legacy provider is disabled.  In fact, running the pgcrypto
tests in FIPS mode makes use of some of these existing alternative
expected files as well (e.g., for blowfish).

These new expected files currently cover the FIPS mode provided by
OpenSSL 3.x as well as the modified OpenSSL 3.x from Red Hat (e.g.,
Fedora 38), but not the modified OpenSSL 1.x from Red Hat (e.g.,
Fedora 35).  (The latter will have some error message wording
differences.)

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/dbbd927f-ef1f-c9a1-4ec6-c759778ac852%40enterprisedb.com
2023-11-17 14:55:51 +01:00
Peter Eisentraut 3af0d17ace pgcrypto: Split off pgp-encrypt-md5 test
In FIPS mode, these tests will fail.  By having them in a separate
file, it would make it easier to have an alternative output file or
selectively disable these tests.  This isn't done here; this is just
some preparation.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/2766054.1700080156@sss.pgh.pa.us
2023-11-16 16:16:07 +01:00
Heikki Linnakangas cd694f60dc Change pgcrypto to use the new ResourceOwner mechanism.
This is a nice example of how extensions can now use ResourceOwners to
track extension-specific resource kinds

Reviewed-by: Peter Eisentraut, Andres Freund
Discussion: https://www.postgresql.org/message-id/d746cead-a1ef-7efe-fb47-933311e876a3%40iki.fi
2023-11-08 13:30:55 +02:00
Peter Eisentraut 611806cd72 Add trailing commas to enum definitions
Since C99, there can be a trailing comma after the last value in an
enum definition.  A lot of new code has been introducing this style on
the fly.  Some new patches are now taking an inconsistent approach to
this.  Some add the last comma on the fly if they add a new last
value, some are trying to preserve the existing style in each place,
some are even dropping the last comma if there was one.  We could
nudge this all in a consistent direction if we just add the trailing
commas everywhere once.

I omitted a few places where there was a fixed "last" value that will
always stay last.  I also skipped the header files of libpq and ecpg,
in case people want to use those with older compilers.  There were
also a small number of cases where the enum type wasn't used anywhere
(but the enum values were), which ended up confusing pgindent a bit,
so I left those alone.

Discussion: https://www.postgresql.org/message-id/flat/386f8c45-c8ac-4681-8add-e3b0852c1620%40eisentraut.org
2023-10-26 09:20:54 +02:00
Peter Eisentraut 64b787656d Add some const qualifiers
There was a mismatch between the const qualifiers for
excludeDirContents in src/backend/backup/basebackup.c and
src/bin/pg_rewind/filemap.c, which led to a quick search for similar
cases.  We should make excludeDirContents match, but the rest of the
changes seem like a good idea as well.

Author: David Steele <david@pgmasters.net>
Discussion: https://www.postgresql.org/message-id/flat/669a035c-d23d-2f38-7ff0-0cb93e01d610@pgmasters.net
2023-09-26 11:28:57 +01:00
Peter Eisentraut 67c0ef9752 Improve const use in zlib-using code
If we define ZLIB_CONST before including zlib.h, zlib augments some
interfaces with const decorations.  By doing that we can keep our own
interfaces cleaner and can remove some unconstify calls.

ZLIB_CONST was introduced in zlib 1.2.5.2 (17 Dec 2011).  When
compiling with older zlib releases, you might now get compiler
warnings about discarding qualifiers.

CentOS 6 has zlib 1.2.3, but in 8e278b6576, we removed support for the
OpenSSL release in CentOS 6, so it seems ok to de-support the zlib
release in CentOS 6 as well.

Reviewed-by: Tristan Partin <tristan@neon.tech>
Discussion: https://www.postgresql.org/message-id/flat/33462926-bb1e-7cc9-8d92-d86318e8ed1d%40eisentraut.org
2023-08-07 09:34:38 +02:00
Tom Lane d98ed080bb Fix small overestimation of base64 encoding output length.
pg_base64_enc_len() and its clones overestimated the output
length by up to 2 bytes, as a result of sloppy thinking about
where to divide.  No callers require a precise estimate, so
this has no consequences worse than palloc'ing a byte or two
more than necessary.  We might as well get it right though.

This bug is very ancient, dating to commit 79d78bb26 which
added encode.c.  (The other instances were presumably copied
from there.)  Still, it doesn't quite seem worth back-patching.

Oleg Tselebrovskiy

Discussion: https://postgr.es/m/f94da55286a63022150bc266afdab754@postgrespro.ru
2023-06-08 11:24:31 -04:00
Peter Eisentraut d952373a98 New header varatt.h split off from postgres.h
This new header contains all the variable-length data types support
(TOAST support) from postgres.h, which isn't needed by large parts of
the backend code.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/ddcce239-0f29-6e62-4b47-1f8ca742addf%40enterprisedb.com
2023-01-10 05:54:36 +01:00
Bruce Momjian c8e1ba736b Update copyright for 2023
Backpatch-through: 11
2023-01-02 15:00:37 -05:00
Andrew Dunstan 8284cf5f74 Add copyright notices to meson files
Discussion: https://postgr.es/m/222b43a5-2fb3-2c1b-9cd0-375d376c8246@dunslane.net
2022-12-20 07:54:39 -05:00
Daniel Gustafsson ba185d318d Remove redundant memset call following palloc0
This is a follow-up commit to ca7f8e2 which removed the allocation
abstraction from pgcrypto and replaced px_alloc + memset calls with
palloc0 calls. The particular memset in this commit was missed in
that work though.

Author: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Bruce Momjian <bruce@momjian.us>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/CALNJ-vT5qRucrFMPSzQyAWods1b4MnNPG-M=_ZUzh1SoTh0vNw@mail.gmail.com
2022-10-13 23:18:00 +02:00
Andres Freund e5555657ba meson: Add support for building with precompiled headers
This substantially speeds up building for windows, due to the vast amount of
headers included via windows.h. A cross build from linux targetting mingw goes
from

994.11user 136.43system 0:31.58elapsed 3579%CPU
to
422.41user 89.05system 0:14.35elapsed 3562%CPU

The wins on windows are similar-ish (but I don't have a system at hand just
now for actual numbers). Targetting other operating systems the wins are far
smaller (tested linux, macOS, FreeBSD).

For now precompiled headers are disabled by default, it's not clear how well
they work on all platforms. E.g. on FreeBSD gcc doesn't seem to have working
support, but clang does.

When doing a full build precompiled headers are only beneficial for targets
with multiple .c files, as meson builds a separate precompiled header for each
target (so that different compilation options take effect). This commit
therefore only changes target with at least two .c files to use precompiled
headers.

Because this commit adds b_pch=false to the default_options new build
directories will have precompiled headers disabled by default, however
existing build directories will continue use the default value of b_pch, which
is true.

Note that using precompiled headers with ccache requires setting
CCACHE_SLOPPINESS=pch_defines,time_macros to get hits.

Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/CA+hUKG+50eOUbN++ocDc0Qnp9Pvmou23DSXu=ZA6fepOcftKqA@mail.gmail.com
Discussion: https://postgr.es/m/c5736f70-bb6d-8d25-e35c-e3d886e4e905@enterprisedb.com
Discussion: https://postgr.es/m/20190826054000.GE7005%40paquier.xyz
2022-10-06 17:19:30 -07:00
Andres Freund 902ab2fcef meson: Add windows resource files
The generated resource files aren't exactly the same ones as the old
buildsystems generate. Previously "InternalName" and "OriginalFileName" were
mostly wrong / not set (despite being required), but that was hard to fix in
at least the make build. Additionally, the meson build falls back to a
"auto-generated" description when not set, and doesn't set it in a few cases -
unlikely that anybody looks at these descriptions in detail.

Author: Andres Freund <andres@anarazel.de>
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2022-10-05 09:56:05 -07:00
Peter Geoghegan 0faf7d933f Harmonize parameter names in contrib code.
Make sure that function declarations use names that exactly match the
corresponding names from function definitions in contrib code.

Like other recent commits that cleaned up function parameter names, this
commit was written with help from clang-tidy.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-WznJt9CMM9KJTMjJh_zbL5hD9oX44qdJ4aqZtjFi-zA3Tg@mail.gmail.com
2022-09-22 13:59:20 -07:00
Andres Freund e6927270cd meson: Add initial version of meson based build system
Autoconf is showing its age, fewer and fewer contributors know how to wrangle
it. Recursive make has a lot of hard to resolve dependency issues and slow
incremental rebuilds. Our home-grown MSVC build system is hard to maintain for
developers not using Windows and runs tests serially. While these and other
issues could individually be addressed with incremental improvements, together
they seem best addressed by moving to a more modern build system.

After evaluating different build system choices, we chose to use meson, to a
good degree based on the adoption by other open source projects.

We decided that it's more realistic to commit a relatively early version of
the new build system and mature it in tree.

This commit adds an initial version of a meson based build system. It supports
building postgres on at least AIX, FreeBSD, Linux, macOS, NetBSD, OpenBSD,
Solaris and Windows (however only gcc is supported on aix, solaris). For
Windows/MSVC postgres can now be built with ninja (faster, particularly for
incremental builds) and msbuild (supporting the visual studio GUI, but
building slower).

Several aspects (e.g. Windows rc file generation, PGXS compatibility, LLVM
bitcode generation, documentation adjustments) are done in subsequent commits
requiring further review. Other aspects (e.g. not installing test-only
extensions) are not yet addressed.

When building on Windows with msbuild, builds are slower when using a visual
studio version older than 2019, because those versions do not support
MultiToolTask, required by meson for intra-target parallelism.

The plan is to remove the MSVC specific build system in src/tools/msvc soon
after reaching feature parity. However, we're not planning to remove the
autoconf/make build system in the near future. Likely we're going to keep at
least the parts required for PGXS to keep working around until all supported
versions build with meson.

Some initial help for postgres developers is at
https://wiki.postgresql.org/wiki/Meson

With contributions from Thomas Munro, John Naylor, Stone Tickle and others.

Author: Andres Freund <andres@anarazel.de>
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Author: Peter Eisentraut <peter@eisentraut.org>
Reviewed-By: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/20211012083721.hvixq4pnh2pixr3j@alap3.anarazel.de
2022-09-21 22:37:17 -07:00
Daniel Gustafsson 1d3566f1b2 pgcrypto: Remove unused code
The mbuf_tell, mbuf_rewind and pgp_get_cipher_name functions were
introduced in commit e94dd6ab91, but were never used, so remove.

Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://postgr.es/m/FCF3F14E-17D5-41F2-AC58-0A97B341193A@yesql.se
2022-09-14 21:58:30 +02:00
Thomas Munro 92d70b77eb Tidy up claimed supported CPUs and OSes.
* Remove arbitrary mention of certain endianness and bitness variants;
   it's enough to say that applicable variants are expected to work.
 * List RISC-V (known to work, being tested).
 * List SuperH and M88K (code exists, unknown status, like M68K).
 * De-list VAX and remove code (known not to work).
 * Remove stray trace of Alpha (support was removed years ago).
 * List illumos, DragonFlyBSD (known to work, being tested).
 * No need to single Windows out by listing a specific version, when we
   don't do that for other OSes; it's enough to say that we support
   current versions of the listed OSes (when 16 ships, that'll be
   Windows 10+).

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Greg Stark <stark@mit.edu>
Discussion: https://postgr.es/m/CA%2BhUKGKk7NZO1UnJM0PyixcZPpCGqjBXW_0bzFZpJBGAf84XKg%40mail.gmail.com
2022-07-11 11:50:41 +12:00
Peter Eisentraut d746021de1 Add construct_array_builtin, deconstruct_array_builtin
There were many calls to construct_array() and deconstruct_array() for
built-in types, for example, when dealing with system catalog columns.
These all hardcoded the type attributes necessary to pass to these
functions.

To simplify this a bit, add construct_array_builtin(),
deconstruct_array_builtin() as wrappers that centralize this hardcoded
knowledge.  This simplifies many call sites and reduces the amount of
hardcoded stuff that is spread around.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/2914356f-9e5f-8c59-2995-5997fc48bcba%40enterprisedb.com
2022-07-01 11:23:15 +02:00
Tom Lane 23e7b38bfe Pre-beta mechanical code beautification.
Run pgindent, pgperltidy, and reformat-dat-files.
I manually fixed a couple of comments that pgindent uglified.
2022-05-12 15:17:30 -04:00
Daniel Gustafsson ee97d46cdb pgcrypto: remove questionmark from error message
The PXE_CIPHER_INIT error is used to report initialization errors, so
appending a questionmark to the error isn't entirely accurate (using a
space before the questionmark doubly so).

Discussion: https://postgr.es/m/C89D932C-501E-4473-9750-638CFCD9095E@yesql.se
2022-05-06 14:41:36 +02:00
Daniel Gustafsson 0250a167a0 pgcrypto: report init errors as PXE_CIPHER_INIT
Report OpenSSL errors during initialization as PXE_CIPHER_INIT since
that's just what they were, and not generic unknown errors. This also
removes the last users of the generic error, and thus it can be removed.

Discussion: http://postgr.es/m/C89D932C-501E-4473-9750-638CFCD9095E@yesql.se
2022-05-06 14:41:33 +02:00
Peter Eisentraut f5576a21b0 pgcrypto: Remove internal padding implementation
Use the padding provided by OpenSSL instead of doing it ourselves.
The internal implementation was once applicable to the non-OpenSSL
code paths, but those have since been removed.  The padding algorithm
is still the same.

The OpenSSL padding implementation is stricter than the previous
internal one: Bad padding during decryption is now an error, and
encryption without padding now requires the input size to be a
multiple of the block size, otherwise it is also an error.
Previously, these cases silently proceeded, in spite of the
documentation saying otherwise.

Add some test cases about this, too.  (The test cases are in
rijndael.sql, but they apply to all encryption algorithms.)

Reviewed-by: Jacob Champion <pchampion@vmware.com>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/ba94c26b-0c58-c97e-7a44-f44e08b4cca2%40enterprisedb.com
2022-03-22 08:58:44 +01:00
Tom Lane 46ab07ffda Clean up assorted failures under clang's -fsanitize=undefined checks.
Most of these are cases where we could call memcpy() or other libc
functions with a NULL pointer and a zero count, which is forbidden
by POSIX even though every production version of libc allows it.
We've fixed such things before in a piecemeal way, but apparently
never made an effort to try to get them all.  I don't claim that
this patch does so either, but it gets every failure I observe in
check-world, using clang 12.0.1 on current RHEL8.

numeric.c has a different issue that the sanitizer doesn't like:
"ln(-1.0)" will compute log10(0) and then try to assign the
resulting -Inf to an integer variable.  We don't actually use the
result in such a case, so there's no live bug.

Back-patch to all supported branches, with the idea that we might
start running a buildfarm member that tests this case.  This includes
back-patching c1132aae3 (Check the size in COPY_POINTER_FIELD),
which previously silenced some of these issues in copyfuncs.c.

Discussion: https://postgr.es/m/CALNJ-vT9r0DSsAOw9OXVJFxLENoVS_68kJ5x0p44atoYH+H4dg@mail.gmail.com
2022-03-03 18:13:24 -05:00
Peter Eisentraut 3f649663a4 pgcrypto: Remove unused error code
PXE_DEV_READ_ERROR hasn't been used since random device support was
removed from pgcrypto (fe0a0b5993).
2022-02-21 10:55:03 +01:00
Peter Eisentraut abe81ee084 pgcrypto: Remove unused error code
PXE_MCRYPT_INTERNAL was apparently never used even when it was added.
2022-02-21 10:28:43 +01:00
Peter Eisentraut 814e1d9ff7 pgcrypto: Remove explicit hex encoding/decoding from tests
This was from before the hex format was available in bytea.  Now we
can remove the extra explicit encoding/decoding calls and rely on the
default output format.

Discussion: https://www.postgresql.org/message-id/flat/17dcb4f7-7ac1-e2b6-d5f7-2dfba06cd9ee%40enterprisedb.com
2021-12-08 06:06:22 +01:00
Peter Eisentraut db7d1a7b05 pgcrypto: Remove non-OpenSSL support
pgcrypto had internal implementations of some encryption algorithms,
as an alternative to calling out to OpenSSL.  These were rarely used,
since most production installations are built with OpenSSL.  Moreover,
maintaining parallel code paths makes the code more complex and
difficult to maintain.

This patch removes these internal implementations.  Now, pgcrypto is
only built if OpenSSL support is configured.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/0b42f1df-8cba-6a30-77d7-acc241cc88c1%40enterprisedb.com
2021-11-05 14:06:59 +01:00
Daniel Gustafsson 72bbff4cd6 Add alternative output for OpenSSL 3 without legacy loaded
OpenSSL 3 introduced the concept of providers to support modularization,
and moved the outdated ciphers to the new legacy provider. In case it's
not loaded in the users openssl.cnf file there will be a lot of regress
test failures, so add alternative outputs covering those.

Also document the need to load the legacy provider in order to use older
ciphers with OpenSSL-enabled pgcrypto.

This will be backpatched to all supported version once there is sufficient
testing in the buildfarm of OpenSSL 3.

Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/FEF81714-D479-4512-839B-C769D2605F8A@yesql.se
2021-08-10 15:08:46 +02:00
Daniel Gustafsson 318df80235 Disable OpenSSL EVP digest padding in pgcrypto
The PX layer in pgcrypto is handling digest padding on its own uniformly
for all backend implementations. Starting with OpenSSL 3.0.0, DecryptUpdate
doesn't flush the last block in case padding is enabled so explicitly
disable it as we don't use it.

This will be backpatched to all supported version once there is sufficient
testing in the buildfarm of OpenSSL 3.

Reviewed-by: Peter Eisentraut, Michael Paquier
Discussion: https://postgr.es/m/FEF81714-D479-4512-839B-C769D2605F8A@yesql.se
2021-08-10 15:01:52 +02:00
Peter Eisentraut 22e1943f13 pgcrypto: Check for error return of px_cipher_decrypt()
This has previously not been a problem (that anyone ever reported),
but in future OpenSSL versions (3.0.0), where legacy ciphers are/can
be disabled, this is the place where this is reported.  So we need to
catch the error here, otherwise the higher-level functions would
return garbage.  The nearby encryption code already handled errors
similarly.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/9e9c431c-0adc-7a6d-9b1a-915de1ba3fe7@enterprisedb.com
2021-03-23 11:48:37 +01:00
Michael Paquier b83dcf7928 Add result size as argument of pg_cryptohash_final() for overflow checks
With its current design, a careless use of pg_cryptohash_final() could
would result in an out-of-bound write in memory as the size of the
destination buffer to store the result digest is not known to the
cryptohash internals, without the caller knowing about that.  This
commit adds a new argument to pg_cryptohash_final() to allow such sanity
checks, and implements such defenses.

The internals of SCRAM for HMAC could be tightened a bit more, but as
everything is based on SCRAM_KEY_LEN with uses particular to this code
there is no need to complicate its interface more than necessary, and
this comes back to the refactoring of HMAC in core.  Except that, this
minimizes the uses of the existing DIGEST_LENGTH variables, relying
instead on sizeof() for the result sizes.  In ossp-uuid, this also makes
the code more defensive, as it already relied on dce_uuid_t being at
least the size of a MD5 digest.

This is in philosophy similar to cfc40d3 for base64.c and aef8948 for
hex.c.

Reported-by: Ranier Vilela
Author: Michael Paquier, Ranier Vilela
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CAEudQAoqEGmcff3J4sTSV-R_16Monuz-UpJFbf_dnVH=APr02Q@mail.gmail.com
2021-02-15 10:18:34 +09:00
Michael Paquier fe61df7f82 Introduce --with-ssl={openssl} as a configure option
This is a replacement for the existing --with-openssl, extending the
logic to make easier the addition of new SSL libraries.  The grammar is
chosen to be similar to --with-uuid, where multiple values can be
chosen, with "openssl" as the only supported value for now.

The original switch, --with-openssl, is kept for compatibility.

Author: Daniel Gustafsson, Michael Paquier
Reviewed-by: Jacob Champion
Discussion: https://postgr.es/m/FAB21FC8-0F62-434F-AA78-6BD9336D630A@yesql.se
2021-02-01 19:19:44 +09:00
Michael Paquier a8ed6bb8f4 Introduce SHA1 implementations in the cryptohash infrastructure
With this commit, SHA1 goes through the implementation provided by
OpenSSL via EVP when building the backend with it, and uses as fallback
implementation KAME which was located in pgcrypto and already shaped for
an integration with a set of init, update and final routines.
Structures and routines have been renamed to make things consistent with
the fallback implementations of MD5 and SHA2.

uuid-ossp has used for ages a shortcut with pgcrypto to fetch a copy of
SHA1 if needed.  This was built depending on the build options within
./configure, so this cleans up some code and removes the build
dependency between pgcrypto and uuid-ossp.

Note that this will help with the refactoring of HMAC, as pgcrypto
offers the option to use MD5, SHA1 or SHA2, so only the second option
was missing to make that possible.

Author: Michael Paquier
Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/X9HXKTgrvJvYO7Oh@paquier.xyz
2021-01-23 11:33:04 +09:00
Michael Paquier 15b824da97 Fix and simplify some code related to cryptohashes
This commit addresses two issues:
- In pgcrypto, MD5 computation called pg_cryptohash_{init,update,final}
without checking for the result status.
- Simplify pg_checksum_raw_context to use only one variable for all the
SHA2 options available in checksum manifests.

Reported-by: Heikki Linnakangas
Discussion: https://postgr.es/m/f62f26bb-47a5-8411-46e5-4350823e06a5@iki.fi
2021-01-08 10:37:03 +09:00
Bruce Momjian ca3b37487b Update copyright for 2021
Backpatch-through: 9.5
2021-01-02 13:06:25 -05:00
Michael Paquier 93e8ff8701 Refactor logic to check for ASCII-only characters in string
The same logic was present for collation commands, SASLprep and
pgcrypto, so this removes some code.

Author: Michael Paquier
Reviewed-by: Stephen Frost, Heikki Linnakangas
Discussion: https://postgr.es/m/X9womIn6rne6Gud2@paquier.xyz
2020-12-21 09:37:11 +09:00
Michael Paquier 9b584953e7 Improve some code around cryptohash functions
This adjusts some code related to recent changes for cryptohash
functions:
- Add a variable in md5.h to track down the size of a computed result,
moved from pgcrypto.  Note that pg_md5_hash() assumed a result of this
size already.
- Call explicit_bzero() on the hashed data when freeing the context for
fallback implementations.  For MD5, particularly, it would be annoying
to leave some non-zeroed data around.
- Clean up some code related to recent changes of uuid-ossp.  .gitignore
still included md5.c and a comment was incorrect.

Discussion: https://postgr.es/m/X9HXKTgrvJvYO7Oh@paquier.xyz
2020-12-14 12:38:13 +09:00
Michael Paquier b67b57a966 Refactor MD5 implementations according to new cryptohash infrastructure
This commit heavily reorganizes the MD5 implementations that exist in
the tree in various aspects.

First, MD5 is added to the list of options available in cryptohash.c and
cryptohash_openssl.c.  This means that if building with OpenSSL, EVP is
used for MD5 instead of the fallback implementation that Postgres had
for ages.  With the recent refactoring work for cryptohash functions,
this change is straight-forward.  If not building with OpenSSL, a
fallback implementation internal to src/common/ is used.

Second, this reduces the number of MD5 implementations present in the
tree from two to one, by moving the KAME implementation from pgcrypto to
src/common/, and by removing the implementation that existed in
src/common/.  KAME was already structured with an init/update/final set
of routines by pgcrypto (see original pgcrypto/md5.h) for compatibility
with OpenSSL, so moving it to src/common/ has proved to be a
straight-forward move, requiring no actual manipulation of the internals
of each routine.  Some benchmarking has not shown any performance gap
between both implementations.

Similarly to the fallback implementation used for SHA2, the fallback
implementation of MD5 is moved to src/common/md5.c with an internal
header called md5_int.h for the init, update and final routines.  This
gets then consumed by cryptohash.c.

The original routines used for MD5-hashed passwords are moved to a
separate file called md5_common.c, also in src/common/, aimed at being
shared between all MD5 implementations as utility routines to keep
compatibility with any code relying on them.

Like the SHA2 changes, this commit had its round of tests on both Linux
and Windows, across all versions of OpenSSL supported on HEAD, with and
even without OpenSSL.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20201106073434.GA4961@paquier.xyz
2020-12-10 11:59:10 +09:00
Michael Paquier 28d1601ad9 pgcrypto: Detect errors with EVP calls from OpenSSL
The following routines are called within pgcrypto when handling digests
but there were no checks for failures:
- EVP_MD_CTX_size (can fail with -1 as of 3.0.0)
- EVP_MD_CTX_block_size (can fail with -1 as of 3.0.0)
- EVP_DigestInit_ex
- EVP_DigestUpdate
- EVP_DigestFinal_ex

A set of elog(ERROR) is added by this commit to detect such failures,
that should never happen except in the event of a processing failure
internal to OpenSSL.

Note that it would be possible to use ERR_reason_error_string() to get
more context about such errors, but these refer mainly to the internals
of OpenSSL, so it is not really obvious how useful that would be.  This
is left out for simplicity.

Per report from Coverity.  Thanks to Tom Lane for the discussion.

Backpatch-through: 9.5
2020-12-08 15:22:12 +09:00
Michael Paquier 87ae9691d2 Move SHA2 routines to a new generic API layer for crypto hashes
Two new routines to allocate a hash context and to free it are created,
as these become necessary for the goal behind this refactoring: switch
the all cryptohash implementations for OpenSSL to use EVP (for FIPS and
also because upstream does not recommend the use of low-level cryptohash
functions for 20 years).  Note that OpenSSL hides the internals of
cryptohash contexts since 1.1.0, so it is necessary to leave the
allocation to OpenSSL itself, explaining the need for those two new
routines.  This part is going to require more work to properly track
hash contexts with resource owners, but this not introduced here.
Still, this refactoring makes the move possible.

This reduces the number of routines for all SHA2 implementations from
twelve (SHA{224,256,386,512} with init, update and final calls) to five
(create, free, init, update and final calls) by incorporating the hash
type directly into the hash context data.

The new cryptohash routines are moved to a new file, called cryptohash.c
for the fallback implementations, with SHA2 specifics becoming a part
internal to src/common/.  OpenSSL specifics are part of
cryptohash_openssl.c.  This infrastructure is usable for more hash
types, like MD5 or HMAC.

Any code paths using the internal SHA2 routines are adapted to report
correctly errors, which are most of the changes of this commit.  The
zones mostly impacted are checksum manifests, libpq and SCRAM.

Note that e21cbb4 was a first attempt to switch SHA2 to EVP, but it
lacked the refactoring needed for libpq, as done here.

This patch has been tested on Linux and Windows, with and without
OpenSSL, and down to 1.0.1, the oldest version supported on HEAD.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20200924025314.GE7405@paquier.xyz
2020-12-02 10:37:20 +09:00
Michael Paquier 8a15e735be Fix some grammar and typos in comments and docs
The documentation fixes are backpatched down to where they apply.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20201031020801.GD3080@telsasoft.com
Backpatch-through: 9.6
2020-11-02 15:14:41 +09:00
Michael Paquier aecaa04418 Add error code for encryption failure in pgcrypto
PXE_DECRYPT_FAILED exists already for decryption errors, and an
equivalent for encryption did not exist.  There is one code path that
deals with such failures for OpenSSL but it used PXE_ERR_GENERIC, which
was inconsistent.  This switches this code path to use the new error
PXE_ENCRYPT_FAILED instead of PXE_ERR_GENERIC, making the code used for
encryption more consistent with the decryption.

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/03049139-CB7A-436E-B71B-42696D3E2EF7@yesql.se
2020-11-01 19:22:59 +09:00
Michael Paquier ca2a12c935 Fix potential memory leak in pgcrypto
When allocating a EVP context, it would have been possible to leak some
memory allocated directly by OpenSSL, that PostgreSQL lost track of if
the initialization of the context allocated failed.  The cleanup can be
done with EVP_MD_CTX_destroy().

Note that EVP APIs exist since OpenSSL 0.9.7 and we have in the tree
equivalent implementations for older versions since ce9b75d (code
removed with 9b7cd59a as of 10~).  However, in 9.5 and 9.6, the existing
code makes use of EVP_MD_CTX_destroy() and EVP_MD_CTX_create() without
an equivalent implementation when building the tree with OpenSSL 0.9.6
or older, meaning that this code is in reality broken with such versions
since it got introduced in e2838c5.  As we have heard no complains about
that, it does not seem worth bothering with in 9.5 and 9.6, so I have
left that out for simplicity.

Author: Michael Paquier
Discussion: https://postgr.es/m/20201015072212.GC2305@paquier.xyz
Backpatch-through: 9.5
2020-10-19 09:36:56 +09:00
Tom Lane 02a75f8369 Add missing error check in pgcrypto/crypt-md5.c.
In theory, the second px_find_digest call in px_crypt_md5 could fail
even though the first one succeeded, since resource allocation is
required.  Don't skip testing for a failure.  (If one did happen,
the likely result would be a crash rather than clean recovery from
an OOM failure.)

The code's been like this all along, so back-patch to all supported
branches.

Daniel Gustafsson

Discussion: https://postgr.es/m/AA8D6FE9-4AB2-41B4-98CB-AE64BA668C03@yesql.se
2020-10-16 11:59:13 -04:00
Michael Paquier ca7f8e2b86 Remove custom memory allocation layer in pgcrypto
PX_OWN_ALLOC was intended as a way to disable the use of palloc(), and
over the time new palloc() or equivalent calls have been added like in
32984d8, making this extra layer losing its original purpose.  This
simplifies on the way some code paths to use palloc0() rather than
palloc() followed by memset(0).

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/A5BFAA1A-B2E8-4CBC-895E-7B1B9475A527@yesql.se
2020-09-25 10:25:55 +09:00
Peter Eisentraut 1784f278a6 Replace remaining StrNCpy() by strlcpy()
They are equivalent, except that StrNCpy() zero-fills the entire
destination buffer instead of providing just one trailing zero.  For
all but a tiny number of callers, that's just overhead rather than
being desirable.

Remove StrNCpy() as it is now unused.

In some cases, namestrcpy() is the more appropriate function to use.
While we're here, simplify the API of namestrcpy(): Remove the return
value, don't check for NULL input.  Nothing was using that anyway.
Also, remove a few unused name-related functions.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/44f5e198-36f6-6cdb-7fa9-60e34784daae%402ndquadrant.com
2020-08-10 23:20:37 +02:00