Hongren Zheng [Thu, 16 May 2024 08:41:25 +0000 (16:41 +0800)]
fips provider: explicitly setup cpuid when initializing
Fixes: #23979
Previously fips module relied on OPENSSL_cpuid_setup
being used as constructor by the linker to correctly
setup the capability vector, either via .section .init
(for x86_64) or via __attribute__((constructor)).
This would make ld.so call OPENSSL_cpuid_setup before
the init function for fips module. However, this early
constructing behavior has several disadvantages:
1. Not all platform/toolchain supports such behavior
2. Initialisation sequence is not well defined, and
some function might not be initialized when cpuid_setup
is called
3. Implicit path is hard to maintain and debug
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24419)
Tomas Mraz [Wed, 8 May 2024 13:23:45 +0000 (15:23 +0200)]
Check DSA parameters for excessive sizes before validating
This avoids overly long computation of various validation
checks.
Fixes CVE-2024-4603
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/24346)
Liu-Ermeng [Mon, 8 Jan 2024 04:01:29 +0000 (20:01 -0800)]
fix sm2 encryption implementation bug.
According to the "GB/T 32918.4-2016"
section 6.1 encryption, step A5:
If result of the "KDF" is all zeros, we should go back to
the begin(step A1).
section 7.1 decryption, step B4:
If result of the "KDF" is all zeros, we should raise error and exit.
Signed-off-by: Liu-Ermeng <liuermeng2@huawei.com> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23210)
Jacob Champion [Mon, 6 May 2024 16:50:11 +0000 (09:50 -0700)]
Add reason codes with the correct offset for two alerts
Fixes #24300. The current values of SSL_R_NO_APPLICATION_PROTOCOL and
SSL_R_PSK_IDENTITY_NOT_FOUND don't allow for a correct lookup of the
corresponding reason strings.
CLA: trivial
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24351)
Georgi Valkov [Fri, 3 May 2024 04:51:08 +0000 (07:51 +0300)]
threads_win: fix build error with VS2010
VC 2010 or earlier compilers do not support static inline.
To work around this problem, we can use the ossl_inline macro.
Fixes:
crypto\threads_win.c(171) : error C2054: expected '(' to follow 'inline'
crypto\threads_win.c(172) : error C2085: 'get_hold_current_qp' : not in formal parameter list
crypto\threads_win.c(172) : error C2143: syntax error : missing ';' before '{'
crypto\threads_win.c(228) : warning C4013: 'get_hold_current_qp' undefined; assuming extern returning int
crypto\threads_win.c(228) : warning C4047: '=' : 'rcu_qp *' differs in levels of indirection from 'int'
Signed-off-by: Georgi Valkov <gvalkov@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24370)
Daiki Ueno [Mon, 13 May 2024 00:07:57 +0000 (09:07 +0900)]
doc: Fix description of EVP_CIPHER_CTX_dup
This fixes a couple of copy and paste error from EVP_MD_CTX_dup,
where: EVP_CIPHER_CTX_dup is useful to avoid multiple
EVP_CIPHER_fetch (instead of EVP_MD_fetch) and returns
EVP_CIPHER_CTX (instead of EVP_MD_CTX).
Signed-off-by: Daiki Ueno <dueno@redhat.com> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/24376)
Georgi Valkov [Sat, 4 May 2024 08:24:08 +0000 (11:24 +0300)]
quic_multistream_test: fix undefined symbol snprintf with VS2010
As snprintf is not available everywhere, use BIO_snprintf instead.
Fixes:
IF EXIST test\quic_multistream_test.exe.manifest DEL /F /Q test\quic_multistream_test.exe.manifest
"link" /nologo /debug setargv.obj /subsystem:console /opt:ref /nologo /debug @V:\_tmp\nm4.tmp
quic_multistream_test-bin-quic_multistream_test.obj : error LNK2019: unresolved external symbol _snprintf referenced in function _helper_init
test\quic_multistream_test.exe : fatal error LNK1120: 1 unresolved externals
NMAKE : fatal error U1077: '"E:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\BIN\link.EXE"' : return code '0x460'
Signed-off-by: Georgi Valkov <gvalkov@gmail.com> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24369)
Signed-off-by: dependabot[bot] <support@github.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24350)
Huiyue Xu [Thu, 9 Nov 2023 02:54:02 +0000 (10:54 +0800)]
Add linux-arm64ilp32-clang target
While clang 15 config target by '--target', not cannot support
'-mabi=ilp32', so add the linux-arm64ilp32-clang target.
Signed-off-by: Huiyue Xu <xuhuiyue@huawei.com> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22666)
all of which are simmilar, in that they catch potential divide by zero
in double values. It can't actually happen since the the threads which
increment these counters don't exit until they reach non-zero values,
but its easy to add the checks, so lets do that to ensure that we don't
change something in the future that causes it.
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/24337)
which pertain to memory leaks in the conf_mod code
If an error is encountered after the module STACK_OF is duplicated or
created in the new_modules variable, we need to remember to free it in
the error path
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/24337)
Matt Caswell [Wed, 1 May 2024 10:23:57 +0000 (11:23 +0100)]
Fix undefined behaviour in the event of a zero length session id
Don't attempt to memcpy a NULL pointer if the length is 0.
Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24309)
Matt Caswell [Tue, 30 Apr 2024 14:35:42 +0000 (15:35 +0100)]
Document the SSL_set_session_secret_cb() function
This function is only useful for EAP-FAST, but was previously undocumented.
Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24309)
Matt Caswell [Tue, 30 Apr 2024 13:31:26 +0000 (14:31 +0100)]
Set the server sig algs before calling the session_secret_cb
Setting the server sig algs sets up the certificate "s3->tmp.valid_flags".
These are needed when calling ssl3_choose_cipher() which can happen
immediately after calling the session_secret_cb
Fixes #24213
Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24309)
Matt Caswell [Mon, 29 Apr 2024 15:58:24 +0000 (16:58 +0100)]
Add a test for the session_secret_cb
Ensure that if a session_secret_cb is being used that a connection can
be successfully made
Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24309)
That caused several memory leaks in case of error.
Also when the CMS object that is created by CMS_EncryptedData_encrypt
is not used in the normal way, but instead just deleted
by CMS_ContentInfo_free some memory was lost.
Fixes #21985
Reviewed-by: Todd Short <todd.short@me.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22031)
Tomas Mraz [Thu, 11 Apr 2024 11:10:09 +0000 (13:10 +0200)]
Make BN_generate_dsa_nonce() constant time and non-biased
Co-authored-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24265)
Matt Caswell [Mon, 25 Mar 2024 12:32:17 +0000 (12:32 +0000)]
Fix intermittent sslapitest early data related failures
Early data is time sensitive. We have an approx 8 second allowance between
writing the early data and reading it. If we exceed that time tests will
fail. This can sometimes (rarely) occur in normal CI operation. We can try
and detect this and just ignore the result of such test failures if the test
has taken too long. We assume anything over 7 seconds is too long.
This is a partial fix for #22605
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23966)
Robert Schulze [Mon, 29 Apr 2024 11:27:07 +0000 (11:27 +0000)]
Fix race for X509 store found by thread sanitizer
The following issue was found in automatic tests with thread sanitizer
builds in ClickHouse (which uses OpenSSL 3.2.1) [0].
The first stack [1] does proper locking (function 'x509_store_add',
x509_lu.c) but in the second stack [2], function 'get_cert_by_subject_ex'
(by_dir.b) forgets to lock when calling 'sk_X509_OBJECT_is_sorted'.
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24290)
Daniel McCarney [Thu, 21 Mar 2024 19:41:11 +0000 (15:41 -0400)]
doc: clarify SSL_CIPHER_description allocation
Previously the documentation for `SSL_CIPHER_description` said:
> If buf is provided, it must be at least 128 bytes, otherwise a buffer
> will be allocated using OPENSSL_malloc().
In reality, `OPENSSL_malloc` is only invoked if the provided `buf`
argument is `NULL`. If the `buf` arg is not `NULL`, but smaller than
128 bytes, the function returns `NULL` without attempting to allocate
a new buffer for the description.
This commit adjusts the documentation to better describe the implemented
behaviour.
CLA: trivial
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23921)
Some CI jobs produce a significant amount artifacts and it takes a lot
of time to upload them into GitHub artifacts storage. It will be much
faster to upload only one archive with artifacts.
Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24264)
Tomas Mraz [Fri, 22 Mar 2024 15:11:42 +0000 (16:11 +0100)]
82-test_ocsp_cert_chain.t: kill -HUP the server after client quits
This ensures even if the connection for some reason
fails, the server will terminate and the test won't get
stuck.
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from https://github.com/openssl/openssl/pull/23857)
Tomas Mraz [Fri, 15 Mar 2024 14:33:01 +0000 (15:33 +0100)]
82-test_ocsp_cert_chain.t: Terminate the server after 1 connection
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from https://github.com/openssl/openssl/pull/23857)
Tomas Mraz [Fri, 16 Feb 2024 15:24:49 +0000 (16:24 +0100)]
Update perl-actions/install-with-cpanm version in CI
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/23613)
Remove all references to FLOSS for NonStop Builds.
FLOSS is no longer a dependency for NonStop as of the deprecation of the SPT
thread model builds.
Fixes: #24214 Signed-off-by: Randall S. Becker <randall.becker@nexbridge.ca> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24217)
Invoke tear_down when exiting test_encode_tls_sct() prematurely
Fixes #24121
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24222)
Neil Horman [Fri, 19 Apr 2024 14:17:54 +0000 (10:17 -0400)]
Fix missing NULL check in prov_config_test
coverity-1596500 caught a missing null check. We should never hit it as
the test harness always sets the environment variable, but lets add the
check for safety
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24208)
Rajeev Ranjan [Mon, 25 Mar 2024 13:00:58 +0000 (14:00 +0100)]
fix sending error when no root CA cert update available
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24169)
Fix migration guide mappings for i2o/o2i_ECPublicKey
Fixes #23854
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24041)
Richard Levitte [Wed, 17 Apr 2024 09:31:31 +0000 (11:31 +0200)]
OSSL_STORE: Add reference docs for the built-in Windows store implementation
Fixes openssl/project#422
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24170)
Neil Horman [Fri, 5 Apr 2024 13:06:10 +0000 (09:06 -0400)]
Fix up path generation to use OPENSSL_MODULES
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24025)
Neil Horman [Thu, 4 Apr 2024 19:39:17 +0000 (15:39 -0400)]
Update modulepath test for provider config to skip if not present
If the p_test.so library isn't present, don't run the test
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24025)
Neil Horman [Wed, 3 Apr 2024 19:18:33 +0000 (15:18 -0400)]
Add test for OSSL_PROVIDER_load with module path set
Ensure that, with the modulepath setting set in a config field, that we
are able to load a provider from the path relative to OPENSSL_MODULES
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24025)
Neil Horman [Tue, 2 Apr 2024 19:02:51 +0000 (15:02 -0400)]
set module path from template
Modules that aren't activated at conf load time don't seem to set the
module path from the template leading to load failures. Make sure to
set that
Fixes #24020
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24025)
Richard Levitte [Tue, 16 Apr 2024 09:48:52 +0000 (11:48 +0200)]
.ctags.d is previous, include it in our tarballs
This is a simple change of .gitattributes, so our tarballs continue to
be a reproducible output of a util/mktar.sh (i.e. git archive with no
other funny business).
Fixes #24090
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24156)
Jerry Shih [Sat, 9 Mar 2024 07:03:56 +0000 (15:03 +0800)]
Use scalar ALU and vector ALU together for chacha20 stream cipher
Fixes #24070
Use scalar ALU for 1 chacha block with rvv ALU simultaneously.
The tail elements(non-multiple of block length) will be handled by
the scalar logic.
Use rvv path if the input length > chacha_block_size.
And we have about 1.2x improvement comparing with the original code.
Reviewed-by: Hongren Zheng <i@zenithal.me> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24097)
Tomas Mraz [Fri, 12 Apr 2024 13:37:58 +0000 (15:37 +0200)]
fuzz/decoder.c: Limit the EVP_PKEY_param_check on DHX keys as well
Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24126)
OpenSSL 3.2.0, QUIC, macOS, error 56 on connected UDP socket
current `translate_msg()` function attempts to set `->msg_name`
(and `->msg_namelen`) with `BIO`'s peer name (connection destination)
regardless if underlying socket is connected or not. Such implementation
uncovers differences in socket implementation between various OSes.
As we have learned hard way `sendmsg()` and `sendmmsg()` on `OpenBSD`
and (`MacOS` too) fail to send messages with `->msg_name` being
set on connected socket. In such case the caller receives
`EISCON` errro.
I think `translate_msg()` caller should provide a hint to indicate
whether we deal with connected (or un-connected) socket. For
connected sockets the peer's name should not be set/filled
by `translate_msg()`. On the other hand if socket is un-connected,
then `translate_msg()` must populate `->msg_name` and `->msg_namelen`
members.
The caller can use `getpeername(2)` to see if socket is
connected. If `getpeername()` succeeds then we must be dealing
with connected socket and `translate_msg()` must not set
`->msg_name` and `->msg_namelen` members. If `getpeername(2)`
fails, then `translate_msg()` must provide peer's name (destination
address) in `->msg_name` and set `->msg_namelen` accordingly.
The propposed fix introduces `is_connected()` function,
which applies `getpeername()` to socket bound to `BIO` instance.
The `dgram_sendmmsg()` uses `is_connected()` as a hint
for `translate_msg()` function, so msghdr gets initialized
with respect to socket state.
The change also modifies existing `test/quic_client_test.c`
so it also covers the case of connected socket. To keep
things simple we can introduce optional argument `connect_first`
to `./quic_client_test` function. Without `connect_first`
the test run as usual. With `connect_first` the test creates
and connects socket first. Then it passes such socket to
`BIO` sub-system to perform `QUIC` connect test as usual.
Fixes #23251
Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23396)
Richard Levitte [Fri, 12 Apr 2024 08:03:21 +0000 (10:03 +0200)]
crypto/threads_pthread.c: refactor all atomics fallbacks for type safety
The atomics fallbacks were using 'void *' as a generic transport for all
possible scalar and pointer types, with the hypothesis that a pointer is
as large as the largest possible scalar type that we would use.
Then enters the use of uint64_t, which is larger than a pointer on any
32-bit system (or any system that has 32-bit pointer configurations).
We could of course choose a larger type as a generic transport. However,
that only pushes the problem forward in time... and it's still a hack.
It's therefore safer to reimplement the fallbacks per type that atomics
are used for, and deal with missing per type fallbacks when the need
arrises in the future.
For test build purposes, the macro USE_ATOMIC_FALLBACKS is introduced.
If OpenSSL is configured with '-DUSE_ATOMIC_FALLBACKS', the fallbacks
will be used, unconditionally.
Fixes #24096
Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24123)
afshinpir [Wed, 28 Feb 2024 03:58:03 +0000 (16:58 +1300)]
Adding missing NULL pointer check
CLA: trivial
In the provider store API, it is not necessary to provide both open and
attach method at the same time and providing at least one of them is
enough. Adding some null pointer checks to prevent exceptions in case
of not providing both methods at the same time.
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23703)
Tomas Mraz [Thu, 11 Apr 2024 07:27:47 +0000 (09:27 +0200)]
ossl_provider_new(): Fix memory leak on error
Fixes #24095
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24100)
Tomas Mraz [Thu, 11 Apr 2024 07:40:18 +0000 (09:40 +0200)]
make_addressPrefix(): Fix a memory leak in error case
Fixes #24098
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24102)
Neil Horman [Wed, 10 Apr 2024 12:28:43 +0000 (08:28 -0400)]
Fix duplicate mutex allocation in threads_win.c
Creating an rcu lock does a double allocation of the underlying mutex.
Not sure how asan didn't catch this, but we clearly have a duplicate
line here
Fixes #24085
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24086)
Richard Levitte [Wed, 10 Apr 2024 08:18:46 +0000 (10:18 +0200)]
VMS: Move defining _XOPEN_SOURCE and _XOPEN_SOURCE_EXTENDED to config target
For all other platforms that need these macros defined, that's how it's
done, so we have VMS follow suit. That avoids a crash between in source
definitions and command line definitions on some other platforms.
Fixes #24075
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24083)
Neil Horman [Fri, 29 Mar 2024 15:39:12 +0000 (11:39 -0400)]
Add locking to atomic operations in rw/rcu tests
I neglected to add locks to the calls to CRYPTO_atomic_add in these
test, which on newer compilers is fine, as atomic operations are
defined. However on older compilers the __ATOMIC_ACQ_REL definition is
missing causing these function to be implemented using an rwlock, which
when NULL causes the locks to fail.
Fix this my creating the lock and using them appropriately
Fixes #24000
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/24001)
CRYPTO_atomic_add has a lock as a parameter, which is often ignored, but in
some cases (for example, when BROKEN_CLANG_ATOMICS is defined) it is required.
There is no easy way to determine if the lock is needed or not. The current
logic looks like this:
if defined(OPENSSL_THREADS) && !defined(CRYPTO_TDEBUG) && !defined(OPENSSL_SYS_WINDOWS)
if defined(__GNUC__) && defined(__ATOMIC_ACQ_REL) && !defined(BROKEN_CLANG_ATOMICS)
- It works without the lock, but in general the need for the
lock depends on __atomic_is_lock_free results
elif defined(__sun) && (defined(__SunOS_5_10) || defined(__SunOS_5_11))
- The lock is not needed (unless ret is NULL, which should never
happen?)
else
- The lock is required
endif
else
- The lock is not needed
endif
Adding such conditions outside of crypto.h is error-prone, so it is better to
always allocate the lock, otherwise CRYPTO_atomic_add may silently fail.
Fixes #23376.
CLA: trivial Fixes: fc570b2605 ("Avoid taking a write lock in ossl_provider_doall_activated()") Signed-off-by: Oleg Bulatov <oleg@bulatov.me> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24081)
Neil Horman [Tue, 26 Mar 2024 13:59:14 +0000 (09:59 -0400)]
Ensure proper memory barriers around ossl_rcu_deref/ossl_rcu_assign_ptr
Since the addition of macos14 M1 runners in our CI jobs we've been
seeing periodic random failures in the test_threads CI job.
Specifically we've seen instances in which the shared pointer in the
test (which points to a monotonically incrementing uint64_t went
backwards.
From taking a look at the disassembled code in the failing case, we see
that __atomic_load_n when emitted in clang 15 looks like this 0000000100120488 <_ossl_rcu_uptr_deref>: 100120488: f8bfc000 ldapr x0, [x0] 10012048c: d65f03c0 ret
Checking the arm docs for the difference between ldar and ldapr:
https://developer.arm.com/documentation/ddi0602/2023-09/Base-Instructions/LDAPR--Load-Acquire-RCpc-Register-
https://developer.arm.com/documentation/dui0802/b/A64-Data-Transfer-Instructions/LDAR
It seems that the ldar instruction provides a global cpu fence, not
completing until all writes in a given cpus writeback queue have
completed
Conversely, the ldapr instruction attmpts to achieve performance
improvements by honoring the Local Ordering register available in the
system coprocessor, only flushing writes in the same address region as
other cpus on the system.
I believe that on M1 virtualized cpus the ldapr is not properly ordering
writes, leading to an out of order read, despite the needed fencing.
I've opened an issue with apple on this here:
https://developer.apple.com/forums/thread/749530
I believe that it is not safe to issue an ldapr instruction unless the
programmer knows that the Local order registers are properly configured
for use on the system.
So to fix it I'm proposing with this patch that we, in the event that:
1) __APPLE__ is defined
AND
2) __clang__ is defined
AND
3) __aarch64__ is defined
during the build, that we override the ATOMIC_LOAD_N macro in the rcu
code such that it uses a custom function with inline assembly to emit
the ldar instruction rather than the ldapr instruction. The above
conditions should get us to where this is only used on more recent MAC
cpus, and only in the case where the affected clang compiler emits the
offending instruction.
I've run this patch 10 times in our CI and failed to reproduce the
issue, whereas previously I could trigger it within 5 runs routinely.
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23974)
Tomas Mraz [Mon, 8 Apr 2024 15:29:51 +0000 (17:29 +0200)]
Downgrade also the download-artifact action
It has to have the same version as upload-artifact.
Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Shane Lontis <shane.lontis@oracle.com> Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/24065)
Neil Horman [Mon, 18 Mar 2024 18:59:32 +0000 (14:59 -0400)]
Add check for public key presence on sm2 signing
SM2 requires that the public EC_POINT be present in a key when signing.
If its not there we crash on a NULL pointer. Add a check to ensure that
its present, and raise an error if its not
Reviewed-by: Paul Yang <kaishen.yy@antfin.com> Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23887)
Jiasheng Jiang [Sat, 16 Mar 2024 21:27:14 +0000 (21:27 +0000)]
APPS: Add missing OPENSSL_free() and combine the error handler
Add the OPENSSL_free() in the error handler to release the "*md_value"
allocated by app_malloc(). To make the code clear and avoid possible
future errors, combine the error handler in the "err" tag.
Then, we only need to use "goto err" instead of releasing the memory
separately.
Since the EVP_MD_get_size() may return negative numbers when an error occurs,
create_query() may fail to catch the error since it only considers 0 as an
error code.
Therefore, unifying the error codes of create_digest() from non-positive
numbers to 0 is better, which also benefits future programming.
Fixes: c7235be ("RFC 3161 compliant time stamp request creation, response generation and response verification.") Signed-off-by: Jiasheng Jiang <jiasheng@purdue.edu> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/23873)
When using CCM, openssl speed uses the loop function EVP_Update_loop_ccm() which
sets a (fake) tag when decrypting. When using -aead (which benchmarks a different
sequence than normal, to be comparable to TLS operation), the loop function
EVP_Update_loop_aead() is used, which also sets a tag when decrypting.
However, when using defaults, the loop function EVP_Update_loop() is used, which
does not set a tag on decryption, leading to "Error finalizing cipher loop".
To fix this, set a fake tag value if we're doing decryption on an AEAD cipher in
EVP_Update_loop(). We don't check the return value: this shouldn't really be able
to fail, and if it does, the following EVP_DecryptUpdate() is almost certain to
fail, so that can catch it.
The decryption is certain to fail (well, almost certain, but with a very low
probability of success), but this is no worse than at present. This minimal
change means that future benchmarking data should be comparable to previous
benchmarking data.
(This is benchmarking code: don't write real apps like this!)