- Feb 02, 2018
-
-
Tatsuhiro Tsujikawa authored
Reviewed-by:
Rich Salz <rsalz@openssl.org> Reviewed-by:
Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/4944)
-
Tatsuhiro Tsujikawa authored
Reviewed-by:
-
Tatsuhiro Tsujikawa authored
Reviewed-by:
-
Richard Levitte authored
Reviewed-by:
Andy Polyakov <appro@openssl.org> (Merged from https://github.com/openssl/openssl/pull/5243)
-
Matt Caswell authored
Reviewed-by:
-
- Feb 01, 2018
-
-
Richard Levitte authored
The assumption that the received buffer has to be NUL-terminated was faulty. Fault found in #5224 Reviewed-by:
-
David Benjamin authored
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:
Kurt Roeckx <kurt@roeckx.be> (Merged from https://github.com/openssl/openssl/pull/5228)
-
David Benjamin authored
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:
-
David Benjamin authored
(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:
-
Michael Richardson authored
added macro to create version number use the macro to build OPENSSL_VERSION_AT_LEAST(maj,min,fix) so that customers of libssl (such as ruby-openssl) do not need to be so aware of openssl version numbers. includes updates to ssl(7) and OPENSSL_VERSION_NUMBER(3) man page Reviewed-by:
Richard Levitte <levitte@openssl.org> Reviewed-by:
-
Rich Salz authored
Reviewed-by:
-
Richard Levitte authored
So as not to be mixed up with a device specification... Reviewed-by:
Tim Hudson <tjh@openssl.org> (Merged from https://github.com/openssl/openssl/pull/5234)
-
Todd Short authored
Reviewed-by:
Ben Kaduk <kaduk@mit.edu> Reviewed-by:
-
Todd Short authored
Reviewed-by:
-
Todd Short authored
Reviewed-by:
-
Todd Short authored
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:
-
Todd Short authored
Reviewed-by:
-
Matt Caswell authored
In a few places we sent an internal_error alert instead of a decode_error. Fixes #5213 Fixes #5214 Reviewed-by:
-
Richard Levitte authored
Reviewed-by:
-
Richard Levitte authored
The reason to do this is that some output might start with an 'ok', which TAP catches and takes for TAP output. The TAP compatible way is to make all output it shouldn't catch look like comments. We do this by setting the environment variable HARNESS_OSSL_PREFIX during tests. When that is set, apps/openssl uses BIO_f_linebuffer and sets its prefix to the content of that environment variable. Reviewed-by:
-
Richard Levitte authored
Reviewed-by:
-
- Jan 31, 2018
-
-
Richard Levitte authored
Reviewed-by:
-
Richard Levitte authored
This avoids having to enumerate specific modules in apps, or to have to include them in libtestutil.a. Reviewed-by:
-
Richard Levitte authored
Most modules are direct implementations of openssl application sub-commands, but some constitute a support library, which can be used by more than one program (and is, incidently, by test/uitest). For practical purposes, we place the support library modules in a private, static library. Finally, there are some modules that don't have direct references in the rest of the apps code, but are still crucial. See them as some kind of extra crt0 or similar for your platform. Inspiration from David von Oheimb Reviewed-by:
-
Richard Levitte authored
Everything in apps includes apps.h, because that one declares apps internal library routines. However, progs.h doesn't declare library routines, but rather the main commands and their options, and there's no reason why the library modules should include it. So, remove the inclusion of progs.h from apps.h and add that inclusion in all command source files. Reviewed-by:
-
Andy Polyakov authored
Reviewed-by:
-
Andy Polyakov authored
Reviewed-by: -
Benjamin Kaduk authored
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:
Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> (Merged from https://github.com/openssl/openssl/pull/5089)
-
Benjamin Kaduk authored
Conceptually, this is a squashed version of: Revert "Address feedback" This reverts commit 75551e07. and Revert "Add CRYPTO_thread_glock_new" This reverts commit ed6b2c79 . 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: -
Richard Levitte authored
They aren't needed if all they do is set bio->init = 1 and zero other fields that are already zeroed Reviewed-by:
-
Richard Levitte authored
Without this, every BIO implementation is forced to have a create method, just to set bio->init = 1. Reviewed-by:
-
Dr. Matthias St. Pierre authored
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:
-
Peter Meerwald-Stadler authored
INSTALL: Mention 'aria' algorithm for no-<alg> Signed-off-by:
Peter Meerwald-Stadler <pmeerw@pmeerw.net> Reviewed-by:
-
- Jan 30, 2018
-
-
Richard Levitte authored
Fixes #5207 (another PR) Reviewed-by:
-
Richard Levitte authored
Reviewed-by:
-
Richard Levitte authored
Fixes #5203 Reviewed-by:
-
Matt Caswell authored
If a server receives an unexpected ClientHello then we may or may not accept it. Make sure all such decisions are made in the state machine and not in the record layer. This also removes a disparity between the TLS and the DTLS code. The TLS code was making this decision in the record layer, while the DTLS code was making it later. Finally it also solves a problem where a warning alert was being sent during tls_setup_handshake() and the function was returning a failure return code. This is problematic because it can be called from a transition function - which we only allow fatal errors to occur in. Reviewed-by:
-
Richard Levitte authored
Reviewed-by:
-
- Jan 29, 2018
-
-
Richard Levitte authored
For proper escaping, we need the direct perl variable values, not a make variable reference. Reviewed-by:
-
Richard Levitte authored
Reviewed-by:
-
