Commit 42d7d7dd6 turned this function from returning void to
returning an int error code. This instance of calling it was
missed.
Found by Coverity.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5338)
This is purported to save a few cycles, but makes the code less
obvious and more brittle, and in fact breaks on platforms where for
ABI continuity reasons there is a SHA2 implementation in libc, and
so EVP needs to call those to avoid conflicts.
A sufficiently good optimizer could simply generate the same entry
points for:
foo(...) { ... }
and
bar(...) { return foo(...); }
but, even without that, the different is negligible, with the
"winner" varying from run to run (openssl speed -evp sha384):
Old:
type 16 bytes 64 bytes 256 bytes 1024 bytes 8192 bytes 16384 bytes
sha384 28864.28k 117362.62k 266469.21k 483258.03k 635144.87k 649123.16k
New:
type 16 bytes 64 bytes 256 bytes 1024 bytes 8192 bytes 16384 bytes
sha384 30055.18k 120725.98k 272057.26k 482847.40k 634585.09k 650308.27k
Reviewed-by: Rich Salz <rsalz@openssl.org>
Without that, output comes one character per line. It's the same
issue as has been observed before, this happens when using write()
on a record oriented stream (possibly unbuffered too).
This also uncovered a bug in BIO_f_linebuffer, where this would cause
an error:
BIO_write(bio, "1\n", 1);
I.e. there's a \n just after the part of the string that we currently
ask to get written.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5352)
.. if avalable. STCK has an artificial delay to ensure uniqueness
which can result in a performance penalty if used heavily
concurrently.
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com>
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5284)
Output copyright year depends on any input file(s) and the script.
This is not perfect, but better than what we had.
Also run 'make update'
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5350)
If the global DRBGs are allocated on the secure heap, then calling
CRYPTO_secure_malloc_done() inside main() will have no effect, unless
OPENSSL_cleanup() has been called explicitely before that, because
otherwise the DRBGs will still be allocated. So it is better to cleanup
the secure heap automatically at the end of OPENSSL_cleanup().
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5294)
The NIST standard presents two alternative ways for seeding the
CTR DRBG, depending on whether a derivation function is used or not.
In Section 10.2.1 of NIST SP800-90Ar1 the following is assessed:
The use of the derivation function is optional if either an
approved RBG or an entropy source provides full entropy output
when entropy input is requested by the DRBG mechanism.
Otherwise, the derivation function shall be used.
Since the OpenSSL DRBG supports being reseeded from low entropy random
sources (using RAND_POOL), the use of a derivation function is mandatory.
For that reason we change the default and replace the opt-in flag
RAND_DRBG_FLAG_CTR_USE_DF with an opt-out flag RAND_DRBG_FLAG_CTR_NO_DF.
This change simplifies the RAND_DRBG_new() calls.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5294)
The functions drbg_setup() and drbg_cleanup() used to duplicate a lot of
code from RAND_DRBG_new() and RAND_DRBG_free(). This duplication has been
removed, which simplifies drbg_setup() and makes drbg_cleanup() obsolete.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5294)
This commit adds three new accessors to the internal DRBG lock
int RAND_DRBG_lock(RAND_DRBG *drbg)
int RAND_DRBG_unlock(RAND_DRBG *drbg)
int RAND_DRBG_enable_locking(RAND_DRBG *drbg)
The three shared DRBGs are intended to be used concurrently, so they
have locking enabled by default. It is the callers responsibility to
guard access to the shared DRBGs by calls to RAND_DRBG_lock() and
RAND_DRBG_unlock().
All other DRBG instances don't have locking enabled by default, because
they are intendended to be used by a single thread. If it is desired,
locking can be enabled by using RAND_DRBG_enable_locking().
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5294)
when the data block ends with SPACEs or NULs.
The problem is, you can't see if the data ends
with SPACE or NUL or a combination of both.
This can happen for instance with
openssl rsautl -decrypt -hexdump
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5328)
Remove the timer and TSC additional input code and instead provide a single
routine that attempts to use the "best" timer/counter available on the
system. It attempts to use TSC, then various OS dependent resources and
finally several tries to obtain the date. If any of these timer/counters
is successful, the rest are skipped.
No randomness is credited for this.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/5231)
If such a timer/counter register is not available, the return value is always
zero. This matches the assembly implementations' behaviour.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/5231)
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com>
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5230)
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com>
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5230)
The functions RAND_bytes() and RAND_priv_bytes() are now both based
on a common implementation using RAND_DRBG_bytes() (if the default
OpenSSL rand method is active). This not only simplifies the code
but also has the advantage that additional input from a high precision
timer is added on every generate call if the timer is available.
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/5251)
When comparing the implementations of drbg_bytes() and RAND_DRBG_bytes(),
it was noticed that the former split the buffer into chunks when calling
RAND_DRBG_generate() to circumvent the size limitation of the buffer
to outlen <= drb->max_request. This loop was missing in RAND_DRBG_bytes(),
so it was adopted from drbg_bytes().
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/5251)
This check not only prevented the automatic reinstantiation of the
DRBG, which is implemented in RAND_DRBG_generate(), but also prevented
an error message from being generated in the case of failure.
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/5251)
BN_from_montgomery_word doesn't have a constant memory access pattern.
Replace the pointer trick with a constant-time select. There is, of
course, still the bn_correct_top leak pervasive in BIGNUM itself.
See also https://boringssl-review.googlesource.com/22904 from BoringSSL.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/5228)
The exponent here is one of d, dmp1, or dmq1 for RSA. This value and its
bit length are both secret. The only public upper bound is the bit width
of the corresponding modulus (RSA n, p, and q, respectively).
Although BN_num_bits is constant-time (sort of; see bn_correct_top notes
in preceding patch), this does not fix the root problem, which is that
the windows are based on the minimal bit width, not the upper bound. We
could use BN_num_bits(m), but BN_mod_exp_mont_consttime is public API
and may be called with larger exponents. Instead, use all top*BN_BITS2
bits in the BIGNUM. This is still sensitive to the long-standing
bn_correct_top leak, but we need to fix that regardless.
This may cause us to do a handful of extra multiplications for RSA keys
which are just above a whole number of words, but that is not a standard
RSA key size.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5154)
(This patch was written by Andy Polyakov. I only wrote the commit
message. Mistakes in the analysis are my fault.)
BN_num_bits, by way of BN_num_bits_word, currently leaks the
most-significant word of its argument via branching and memory access
pattern.
BN_num_bits is called on RSA prime factors in various places. These have
public bit lengths, but all bits beyond the high bit are secret. This
fully resolves those cases.
There are a few places where BN_num_bits is called on an input where the
bit length is also secret. This does *not* fully resolve those cases as
we still only look at the top word. Today, that is guaranteed to be
non-zero, but only because of the long-standing bn_correct_top timing
leak. Once that is fixed, a constant-time BN_num_bits on such inputs
must count bits on each word.
Instead, those cases should not call BN_num_bits at all. In particular,
BN_mod_exp_mont_consttime uses the exponent bit width to pick windows,
but it should be using the maximum bit width. The next patch will fix
this.
Thanks to Dinghao Wu, Danfeng Zhang, Shuai Wang, Pei Wang, and Xiao Liu
for reporting this issue.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5154)
Add SSL_verify_client_post_handshake() for servers to initiate PHA
Add SSL_force_post_handshake_auth() for clients that don't have certificates
initially configured, but use a certificate callback.
Update SSL_CTX_set_verify()/SSL_set_verify() mode:
* Add SSL_VERIFY_POST_HANDSHAKE to postpone client authentication until after
the initial handshake.
* Update SSL_VERIFY_CLIENT_ONCE now only sends out one CertRequest regardless
of when the certificate authentication takes place; either initial handshake,
re-negotiation, or post-handshake authentication.
Add 'RequestPostHandshake' and 'RequirePostHandshake' SSL_CONF options that
add the SSL_VERIFY_POST_HANDSHAKE to the 'Request' and 'Require' options
Add support to s_client:
* Enabled automatically when cert is configured
* Can be forced enabled via -force_pha
Add support to s_server:
* Use 'c' to invoke PHA in s_server
* Remove some dead code
Update documentation
Update unit tests:
* Illegal use of PHA extension
* TLSv1.3 certificate tests
DTLS and TLS behave ever-so-slightly differently. So, when DTLS1.3 is
implemented, it's PHA support state machine may need to be different.
Add a TODO and a #error
Update handshake context to deal with PHA.
The handshake context for TLSv1.3 post-handshake auth is up through the
ClientFinish message, plus the CertificateRequest message. Subsequent
Certificate, CertificateVerify, and Finish messages are based on this
handshake context (not the Certificate message per se, but it's included
after the hash). KeyUpdate, NewSessionTicket, and prior Certificate
Request messages are not included in post-handshake authentication.
After the ClientFinished message is processed, save off the digest state
for future post-handshake authentication. When post-handshake auth occurs,
copy over the saved handshake context into the "main" handshake digest.
This effectively discards the any KeyUpdate or NewSessionTicket messages
and any prior post-handshake authentication.
This, of course, assumes that the ID-22 did not mean to include any
previous post-handshake authentication into the new handshake transcript.
This is implied by section 4.4.1 that lists messages only up to the
first ClientFinished.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4964)
The behavior of resetting the init_lock value to NULL after
freeing it during OPENSSL_cleanup() was added as part of the
global lock commits that were just reverted, but there is desire
to retain this behavior for clarity.
It is unclear that the library would actually remain usable in
any form after OPENSSL_cleanup(), since the required re-initialization
occurs under a CRYPTO_ONCE check that cannot be reset at cleanup time.
That said, a NULL dereference is probably more friendly behavior
in these treacherous waters than using freed memory would be.
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/5089)
Conceptually, this is a squashed version of:
Revert "Address feedback"
This reverts commit 75551e07bd.
and
Revert "Add CRYPTO_thread_glock_new"
This reverts commit ed6b2c7938.
But there were some intervening commits that made neither revert apply
cleanly, so instead do it all as one shot.
The crypto global locks were an attempt to cope with the awkward
POSIX semantics for pthread_atfork(); its documentation (the "RATIONALE"
section) indicates that the expected usage is to have the prefork handler
lock all "global" locks, and the parent and child handlers release those
locks, to ensure that forking happens with a consistent (lock) state.
However, the set of functions available in the child process is limited
to async-signal-safe functions, and pthread_mutex_unlock() is not on
the list of async-signal-safe functions! The only synchronization
primitives that are async-signal-safe are the semaphore primitives,
which are not really appropriate for general-purpose usage.
However, the state consistency problem that the global locks were
attempting to solve is not actually a serious problem, particularly for
OpenSSL. That is, we can consider four cases of forking application
that might use OpenSSL:
(1) Single-threaded, does not call into OpenSSL in the child (e.g.,
the child calls exec() immediately)
For this class of process, no locking is needed at all, since there is
only ever a single thread of execution and the only reentrancy is due to
signal handlers (which are themselves limited to async-signal-safe
operation and should not be doing much work at all).
(2) Single-threaded, calls into OpenSSL after fork()
The application must ensure that it does not fork() with an unexpected
lock held (that is, one that would get unlocked in the parent but
accidentally remain locked in the child and cause deadlock). Since
OpenSSL does not expose any of its internal locks to the application
and the application is single-threaded, the OpenSSL internal locks
will be unlocked for the fork(), and the state will be consistent.
(OpenSSL will need to reseed its PRNG in the child, but that is
an orthogonal issue.) If the application makes use of locks from
libcrypto, proper handling for those locks is the responsibility of
the application, as for any other locking primitive that is available
for application programming.
(3) Multi-threaded, does not call into OpenSSL after fork()
As for (1), the OpenSSL state is only relevant in the parent, so
no particular fork()-related handling is needed. The internal locks
are relevant, but there is no interaction with the child to consider.
(4) Multi-threaded, calls into OpenSSL after fork()
This is the case where the pthread_atfork() hooks to ensure that all
global locks are in a known state across fork() would come into play,
per the above discussion. However, these "calls into OpenSSL after
fork()" are still subject to the restriction to async-signal-safe
functions. Since OpenSSL uses all sorts of locking and libc functions
that are not on the list of safe functions (e.g., malloc()), this
case is not currently usable and is unlikely to ever be usable,
independently of the locking situation. So, there is no need to
go through contortions to attempt to support this case in the one small
area of locking interaction with fork().
In light of the above analysis (thanks @davidben and @achernya), go
back to the simpler implementation that does not need to distinguish
"library-global" locks or to have complicated atfork handling for locks.
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/5089)
They aren't needed if all they do is set bio->init = 1 and zero other
fields that are already zeroed
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5223)
Without this, every BIO implementation is forced to have a create
method, just to set bio->init = 1.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5223)
Some older glibc versions require the `-lrt` linker option for
resolving the reference to `clock_gettime'. Since it is not desired
to add new library dependencies in version 1.1.1, the call to
clock_gettime() is replaced by a call to gettimeofday() for the
moment. It will be added back in version 1.2.
Signed-off-by: Dr. Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/5199)
C preprocessor flags get separated from C flags, which has the
advantage that we don't get loads of macro definitions and inclusion
directory specs when linking shared libraries, DSOs and programs.
This is a step to add support for "make variables" when configuring.
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5177)
The new extension is like signature_algorithms, but only for the
signature *on* the certificate we will present to the peer (the
old signature_algorithms extension is still used for signatures that
we *generate*, i.e., those over TLS data structures).
We do not need to generate this extension, since we are the same
implementation as our X.509 stack and can handle the same types
of signatures, but we need to be prepared to receive it, and use the received
information when selecting what certificate to present.
There is a lot of interplay between signature_algorithms_cert and
signature_algorithms, since both affect what certificate we can
use, and thus the resulting signature algorithm used for TLS messages.
So, apply signature_algorithms_cert (if present) as a filter on what
certificates we can consider when choosing a certificate+sigalg
pair.
As part of this addition, we also remove the fallback code that let
keys of type EVP_PKEY_RSA be used to generate RSA-PSS signatures -- the
new rsa_pss_pss_* and rsa_pss_rsae_* signature schemes have pulled
the key type into what is covered by the signature algorithm, so
we should not apply this sort of compatibility workaround.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5068)
Correct error return value in OCSP_basic_sign().
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4190)
Add a -rsigopt option to the ocsp command that allows signature parameters to be provided for the signing of OCSP responses. The parameters that may be provided to -rsigopt are the same as may be provided to -sigopt in the ca, req, and x509 commands.
This PR also defines a OCSP_basic_sign_ctx() function, which functions in the same way as OCSP_basic_sign(), except that it accepts a EVP_MD_CTX rather than a key and digest. The OCSP_basic_sign_ctx() function is used to implement the -rsigopt option in the ocsp command.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4190)
This just adds the various extension functions. More changes will be
required to actually use them.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4435)
Support added for these two digests, available only via the EVP interface.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5093)
