Modified version of the commit ffaef3f15 in the master branch by Stephen
Henson. This makes the SSL_OP_SINGLE_DH_USE option a no-op and always
generates a new DH key for every handshake regardless.
This is a follow on from CVE-2016-0701. This branch is not impacted by
that CVE because it does not support X9.42 style parameters. It is still
possible to generate parameters based on primes that are not "safe",
although by default OpenSSL does not do this. The documentation does
sign post that using such parameters is unsafe if the private DH key is
reused. However to avoid accidental problems or future attacks this commit
has been backported to this branch.
Issue reported by Antonio Sanso
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
For BSD systems, Configure adds a shared_ldflags including a reference
to the Makefile variable LIBRPATH, but since it must be passed down to
Makefile.shared, care must be taken so the value of LIBRPATH doesn't
get expanded too early, or it ends up giving an empty string.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(cherry picked from commit c64879d3f3)
Describe the usage of the OCSP callback functions on both the client and
the server side.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(cherry picked from commit c52c3b5e11)
It makes no sense to call the OCSP status callback if we are resuming a
session because no certificates will be sent.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(cherry picked from commit 0ac6239955)
If a server sends the status_request extension then it may choose
to send the CertificateStatus message. However this is optional.
We were treating it as mandatory and the connection was failing.
Thanks to BoringSSL for reporting this issue.
RT#4120
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(cherry picked from commit 905943af3b)
BIO_int_ctrl isn't made for the purpose BIO_get_conn_int_port used it
for.
This also changes BIO_C_GET_CONNECT to actually return the port
instead of assigning it to a pointer that was never returned back to
the caller.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 2a60fccdd9)
Fix some more URLs mangled by indent in the reformat. These ones don't exist
in master so we have a separate commit. Based on a patch supplied by Arnaud
Lacombe <al@aerilon.ca>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Some URLs in the source code ended up getting mangled by indent. This fixes
it. Based on a patch supplied by Arnaud Lacombe <al@aerilon.ca>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Commit 2b0180c37f attempted to do this but
only hit one of many BN_mod_exp codepaths. Fix remaining variants and add
a test for each method.
Thanks to Hanno Boeck for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
(cherry picked from commit d911097d7c)
(cherry picked from commit 44e4f5b04b)
A BIO_flush call in the DTLS code was not correctly setting the |rwstate|
variable to SSL_WRITING. This means that SSL_get_error() will not return
SSL_ERROR_WANT_WRITE in the event of an IO retry.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 67f60be8c9)
If using DTLS and NBIO then if a second or subsequent handshake message
fragment hits a retry, then the retry attempt uses the wrong fragment
offset value. This commit restores the fragment offset from the last
attempt.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 2ad226e88b)
As part of this, move release creation to a script to be called from
.travis.yml. That makes it much easier to test outside of travis.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 382af61f62)
Introducing DISTTARVARS to propagate changed variables down to the
tar-making target.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 4d3c30a179)
It seems like some tar versions don't like the name:id form for
--owner and --group. The closest known anonymous user being 0 (root),
that seems to be the most appropriate user/group to assign ownership
to. It matters very little when unpacking either way.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit b91dd150d2)
Make TARFILE include ../ instead of having that hard coded all over the place.
When transforming file names in TAR_COMMAND, use $(NAME) instead of openssl-$(VERSION)
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 4a544810f0)
Avoid seg fault by checking mgf1 parameter is not NULL. This can be
triggered during certificate verification so could be a DoS attack
against a client or a server enabling client authentication.
Thanks to Loïc Jonas Etienne (Qnective AG) for discovering this bug.
CVE-2015-3194
Reviewed-by: Matt Caswell <matt@openssl.org>
When parsing a combined structure pass a flag to the decode routine
so on error a pointer to the parent structure is not zeroed as
this will leak any additional components in the parent.
This can leak memory in any application parsing PKCS#7 or CMS structures.
CVE-2015-3195.
Thanks to Adam Langley (Google/BoringSSL) for discovering this bug using
libFuzzer.
PR#4131
Reviewed-by: Richard Levitte <levitte@openssl.org>
The feature_test_macros(7) manual tells us that _BSD_SOURCE is
deprecated since glibc 2.20 and that the compiler will warn about it
being used, unless _DEFAULT_SOURCE is defined as well.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit f9fd35248c)
In the DTLS ClientHello processing the return value is stored in |ret| which
by default is -1. We wish to return 1 on success or 2 on success *and* we
have validated the DTLS cookie. Previously on successful validation of the
cookie we were setting |ret| to 2. Unfortunately if we later encounter an
error then we can end up returning a successful (positive) return code from
the function because we already set |ret| to a positive value.
This does not appear to have a security consequence because the handshake
just fails at a later point.
Reviewed-by: Andy Polyakov <appro@openssl.org>
If somewhere in SSL_new() there is a memory allocation failure, ssl3_free() can
get called with s->s3 still being NULL.
Patch also provided by Willy Tarreau <wtarreau@haproxy.com>
Signed-off-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Viktor Dukhovni <openssl-users@dukhovni.org>
(cherry picked from commit 3e7bd2ce0b16f8611298175d6dc7cb35ee06ea6d)
Thanks to Guido Vranken <guidovranken@gmail.com> for reporting this issue.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 158e5207a7)
Conflicts:
crypto/asn1/asn1_par.c
There are lots of calls to EVP functions from within libssl There were
various places where we should probably check the return value but don't.
This adds these checks.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 56d9134675)
Conflicts:
ssl/s3_enc.c
ssl/s3_srvr.c
If a DTLS client that does not support secure renegotiation connects to an
OpenSSL DTLS server then, by default, renegotiation is disabled. If a
server application attempts to initiate a renegotiation then OpenSSL is
supposed to prevent this. However due to a discrepancy between the TLS and
DTLS code, the server sends a HelloRequest anyway in DTLS.
This is not a security concern because the handshake will still fail later
in the process when the client responds with a ClientHello.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit d40ec4ab8e)
In DTLS if an IO retry occurs during writing of a fragmented ClientHello
then we can end up reseting the finish mac variables on the retry, which
causes a handshake failure. We should only reset on the first attempt not
on retries.
Thanks to BoringSSL for reporting this issue.
RT#4119
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 15a7164eb7)
During work on a larger change in master a number of locations were
identified where return value checks were missing. This backports the
relevant fixes.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 903738ac63)
Conflicts:
crypto/cms/cms_sd.c
./Configure [target] --strict-warnings -Wno-pedantic-ms-format
would not add '-pedantic' because it matches '-Wno-pedantic-ms-format',
which was added first.
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 6703c4ea87)
EVP_SignInit_ex was missing from the NAME section of its man page so
typing "man EVP_SignInit_ex" failed to load the page.
Reviewed-by: Stephen Henson <steve@openssl.org>
(cherry picked from commit 3d866ea67e)
Clarify that git format-patch output is preferred for creating patch files.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit f89ee71bc8)
Close GH Issue 69
Close GH PR 457/RT4113
Some other updates
By Rich Salz, Alessandro Ghedini, Steve Marquess, Collin Anderson
(manual cherry-pick of a2aaf8be7e and
b06935f439)
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Previous language was unclear. New language isn't pretty but I believe
it is more accurate.
Signed-off-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Laurie <ben@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 8cbb048c3e)
In X509_cmp, if cert digest is equal, look at DER of the
signed part. This is what master and 1.0.2 already do.
Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
The function int_rsa_verify is an internal function used for verifying an
RSA signature. It takes an argument |dtype| which indicates the digest type
that was used. Dependant on that digest type the processing of the
signature data will vary. In particular if |dtype == NID_mdc2| and the
signature data is a bare OCTETSTRING then it is treated differently to the
default case where the signature data is treated as a DigestInfo (X509_SIG).
Due to a missing "else" keyword the logic actually correctly processes the
OCTETSTRING format signature first, and then attempts to continue and
process it as DigestInfo. This will invariably fail because we already know
that it is a bare OCTETSTRING.
This failure doesn't actualy make a real difference because it ends up at
the |err| label regardless and still returns a "success" result. This patch
just cleans things up to make it look a bit more sane.
RT#4076
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit dffe51091f)
BN_with_flags() will read the dest->flags to keep the BN_FLG_MALLOCED but
overwrites everything else.
Signed-off-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Rich Salz <rsalz@openssl.org>
MR #1231
(cherry picked from commit f92768e6f5)
Some extension handling functions were passing in a pointer to the start
of the data, plus the length in order to calculate the end, rather than
just passing in the end to start with. This change makes things a little
more readable.
Reviewed-by: Emilia Käsper <emilia@openssl.org>
Conflicts:
ssl/s3_srvr.c
ssl/ssl_locl.h
ssl/t1_lib.c
the alias supported by OpenSSL 1.0.1 is "EECDH" not "EECDHE"
(GH PR 405)
Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Andy Polyakov <appro@openssl.org>
RFC 5077 section 3.3 says: If the server determines that it does not
want to include a ticket after it has included the SessionTicket
extension in the ServerHello, then it sends a zero-length ticket in the
NewSessionTicket handshake message.
Previously the client would fail upon attempting to allocate a
zero-length buffer. Now, we have the client ignore the empty ticket and
keep the existing session.
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 21b538d616)
BUF_strndup was calling strlen through BUF_strlcpy, and ended up reading
past the input if the input was not a C string.
Make it explicitly part of BUF_strndup's contract to never read more
than |siz| input bytes. This augments the standard strndup contract to
be safer.
The commit also adds a check for siz overflow and some brief documentation
for BUF_strndup().
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 110f7b37de)
(cherry picked from commit f61216ba9d)
For all release branches. It adds travis build support. If you don't
have a config file it uses the default (because we enabled travis for the
project), which uses ruby/rake/rakefiles, and you get confusing "build
still failing" messages.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit db9defdfe3)
If we use BIO_new_file(), on Windows it'll jump through hoops to work
around their unusual charset/Unicode handling. it'll convert a UTF-8
filename to UCS-16LE and attempt to use _wfopen().
If you use BIO_read_filename(), it doesn't do this. Shouldn't it be
consistent?
It would certainly be nice if SSL_use_certificate_chain_file() worked.
Also made BIO_C_SET_FILENAME work (rsalz)
Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit ff03599a2f)
There are a couple of minor fixes here:
1) Handle the case when RegisterEventSource() fails (which it may for
various reasons) and do the work of logging the event only if it succeeds.
2) Handle the case when ReportEvent() fails and do our best in debug builds
to at least attempt somehow indicate that something has gone wrong. The
typical situation would be someone running tools like DbMon, DBWin32,
DebugView or just having the debugger attached. The intent is to make sure
that at least some data will be captured so that we can save hours and days
of debugging time.
3) Minor fix to change the MessageBox() flag to MB_ICONERROR. Though the
value of MB_ICONERROR is the same value as MB_ICONSTOP, the intent is
better conveyed by using MB_ICONERROR.
Testing performed:
1) Clean compilation for debug-VC-WIN32 and VC-WIN32.
2) Good test results (nmake -f ms\ntdll.mak test) for debug-VC-WIN32 and
VC-WIN32.
3) Stepped through relevant changes using WinDBG and exercised the impacted
code paths.
Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 4cd94416a4)
There were some memory leaks in the creation of an SRP verifier (both on
successful completion and also on some error paths).
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit bf95cde287)
Was only approved for master, to avoid compatibility issues on
previous releases.
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit 6be18a2219)
In master we have the function OPENSSL_clear_free(x,y), which immediately
returns if x == NULL. In <=1.0.2 this function does not exist so we have to
do:
OPENSSL_cleanse(x, y);
OPENSSL_free(x);
However, previously, OPENSSL_cleanse did not check that if x == NULL, so
the real equivalent check would have to be:
if (x != NULL)
OPENSSL_cleanse(x, y);
OPENSSL_free(x);
It would be easy to get this wrong during cherry-picking to other branches
and therefore, for safety, it is best to just ensure OPENSSL_cleanse also
checks for NULL.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 020d8fc83f)
Previously, the conversion would silently coerce to ASCII. Now, we error
out.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit b785504a10)
(cherry picked from commit cb71f17dc7)
Rewrite EVP_DecodeUpdate.
In particular: reject extra trailing padding, and padding in the middle
of the content. Don't limit line length. Add tests.
Previously, the behaviour was ill-defined, and depended on the position
of the padding within the input.
In addition, this appears to fix a possible two-byte oob read.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Dr Stephen Henson <steve@openssl.org>
(cherry picked from commit 3cdd1e94b1)
(cherry picked from commit 37faf11796)
If the field separator isn't specified through -nameopt then use
XN_FLAG_SEP_CPLUS_SPC instead of printing nothing and returing an error.
PR#2397
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 03706afa30)
Fix both the caller to error out on malloc failure, as well as the
eventual callee to handle a NULL gracefully.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Commit f0348c842e introduced a problem with session resumption. The
version for the session is fixed when the session is created. By moving
the creation of the session earlier in the process the version is fixed
*before* version negotiation has completed when processing the ServerHello
on the client side. This fix updates the session version after version neg
has completed.
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit dc0c888811)
Builds using no-tlsext in 1.0.0 and 0.9.8 are broken. This commit fixes the
issue. The same commit is applied to 1.0.1 and 1.0.2 branches for code
consistency. However this commit will not fix no-tlsext in those branches
which have always been broken for other reasons. The commit is not applied
to master at all, because no-tlsext has been completely removed from that
branch.
Based on a patch by Marc Branchaud <marcnarc@xiplink.com>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit 9a931208d7)
The NULL cipher case can't actually happen because we have no
EVP_PBE_CTL combinations where cipher_nid is -1 and keygen is
PKCS12_PBE_keyivgen. But make the code more obviously correct.
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 394f7b6fcc)
- Pass in the right ciphertext length to ensure we're indeed testing
ciphertext corruption (and not truncation).
- Only test one mutation per byte to not make the test too slow.
- Add a separate test for truncated ciphertexts.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 25d6b3401c)
Because we recently encourage people to have a .dir-locals.el, it's a good
idea to ignore it on a git level.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit d7c02691a5)
Don't dereference |d| when |top| is zero. Also test that various BIGNUM methods behave correctly on zero/even inputs.
Follow-up to b11980d79a
Reviewed-by: Rich Salz <rsalz@openssl.org>
This file, when copied to .dir-locals.el in the OpenSSL source top,
will make sure that the CC mode style "OpenSSL-II" will be used for
all C files.
Additionally, I makes sure that tabs are never used as indentation
character, regardless of the emacs mode, and that the fill column is
78.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 0927f0d822)
If the seed value for dsa key generation is too short (< qsize),
return an error. Also update the documentation.
Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit f00a10b897)
A DTLS client will abort a handshake if the server attempts to renew the
session ticket. This is caused by a state machine discrepancy between DTLS
and TLS discovered during the state machine rewrite work.
The bug can be demonstrated as follows:
Start a DTLS s_server instance:
openssl s_server -dtls
Start a client and obtain a session but no ticket:
openssl s_client -dtls -sess_out session.pem -no_ticket
Now start a client reusing the session, but allow a ticket:
openssl s_client -dtls -sess_in session.pem
The client will abort the handshake.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit ee4ffd6fcc)
Conflicts:
ssl/d1_clnt.c
Clarify and update documention for extra chain certificates.
PR#3878.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 2fd7fb99db)
When config'd with "sctp" running "make test" causes a seg fault. This is
actually due to the way ssltest works - it dives under the covers and frees
up BIOs manually and so some BIOs are NULL when the SCTP code does not
expect it. The simplest fix is just to add some sanity checks to make sure
the BIOs aren't NULL before we use them.
This problem occurs in master and 1.0.2. The fix has also been applied to
1.0.1 to keep the code in sync.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit f75d5171be)
There are some missing return value checks in the SCTP code. In master this
was causing a compilation failure when config'd with
"--strict-warnings sctp".
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit d8e8590ed9)
The function BN_MONT_CTX_set was assuming that the modulus was non-zero
and therefore that |mod->top| > 0. In an error situation that may not be
the case and could cause a seg fault.
This is a follow on from CVE-2015-1794.
Reviewed-by: Richard Levitte <levitte@openssl.org>
If a client receives a ServerKeyExchange for an anon DH ciphersuite with the
value of p set to 0 then a seg fault can occur. This commits adds a test to
reject p, g and pub key parameters that have a 0 value (in accordance with
RFC 5246)
The security vulnerability only affects master and 1.0.2, but the fix is
additionally applied to 1.0.1 for additional confidence.
CVE-2015-1794
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
EC_KEY_set_public_key_affine_coordinates was using some variables that only
apply if OPENSSL_NO_EC2M is not defined.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(cherry picked from commit 8d11b7c7ee)
The function SSL_set_session_ticket_ext can be used to set custom session
ticket data passed in the initial ClientHello. This can be particularly
useful for EAP-FAST. However, when using SSLv23_method, the session does
not get created until the ServerHello has been received. The extension code
will only add the SessionTicket data to the ClientHello if a session already
exists. Therefore SSL_set_session_ticket_ext has no impact when used in
conjunction with SSLv23_method. The solution is to simply create the session
during creation of the ClientHello instead of waiting for the ServerHello.
This commit fixes the test failure introduced by the previous commit.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
The function SSL_set_session_ticket_ext sets the ticket data to be sent in
the ClientHello. This is useful for EAP-FAST. This commit adds a test to
ensure that when this function is called the expected ticket data actually
appears in the ClientHello.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
This leaves behind files with names ending with '.iso-8859-1'. These
should be safe to remove. If something went wrong when re-encoding,
there will be some files with names ending with '.utf8' left behind.
Reviewed-by: Rich Salz <rsalz@openssl.org>
This requires 'iconv' and that 'file' can take the options '-b' and '-i'.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit f608b4064d)
Instead of piping through tardy, and possibly suffering from bugs in certain
versions, use --transform, --owner and --group directly with GNU tar (we
already expect that tar variant).
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 27f98436b9)
Conflicts:
Makefile.org
This is a follow up to the alternate chains certificate forgery issue
(CVE-2015-1793). That issue is exacerbated in 1.0.1 by a related bug which
means that we *always* check for an alternative chain, even if we have
already found a chain. The code is supposed to stop as soon as it has found
one (and does do in master and 1.0.2).
Reviewed-by: Stephen Henson <steve@openssl.org>
The function X509_verify_cert checks the value of |ctx->chain| at the
beginning, and if it is NULL then it initialises it, along with the value
of ctx->untrusted. The normal way to use X509_verify_cert() is to first
call X509_STORE_CTX_init(); then set up various parameters etc; then call
X509_verify_cert(); then check the results; and finally call
X509_STORE_CTX_cleanup(). The initial call to X509_STORE_CTX_init() sets
|ctx->chain| to NULL. The only place in the OpenSSL codebase where
|ctx->chain| is set to anything other than a non NULL value is in
X509_verify_cert itself. Therefore the only ways that |ctx->chain| could be
non NULL on entry to X509_verify_cert is if one of the following occurs:
1) An application calls X509_verify_cert() twice without re-initialising
in between.
2) An application reaches inside the X509_STORE_CTX structure and changes
the value of |ctx->chain| directly.
With regards to the second of these, we should discount this - it should
not be supported to allow this.
With regards to the first of these, the documentation is not exactly
crystal clear, but the implication is that you must call
X509_STORE_CTX_init() before each call to X509_verify_cert(). If you fail
to do this then, at best, the results would be undefined.
Calling X509_verify_cert() with |ctx->chain| set to a non NULL value is
likely to have unexpected results, and could be dangerous. This commit
changes the behaviour of X509_verify_cert() so that it causes an error if
|ctx->chain| is anything other than NULL (because this indicates that we
have not been initialised properly). It also clarifies the associated
documentation. This is a follow up commit to CVE-2015-1793.
Reviewed-by: Stephen Henson <steve@openssl.org>
This adds a test for CVE-2015-1793. This adds a new test file
verify_extra_test.c, which could form the basis for additional
verification tests.
Reviewed-by: Stephen Henson <steve@openssl.org>
Conflicts:
test/Makefile
During certificate verfification, OpenSSL will attempt to find an
alternative certificate chain if the first attempt to build such a chain
fails. An error in the implementation of this logic can mean that an
attacker could cause certain checks on untrusted certificates to be
bypassed, such as the CA flag, enabling them to use a valid leaf
certificate to act as a CA and "issue" an invalid certificate.
This occurs where at least one cert is added to the first chain from the
trust store, but that chain still ends up being untrusted. In that case
ctx->last_untrusted is decremented in error.
Patch provided by the BoringSSL project.
CVE-2015-1793
Reviewed-by: Stephen Henson <steve@openssl.org>
In CCM mode don't require a tag before initialising decrypt: this allows
the tag length to be set without requiring the tag.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 9cca7be11d)
The PSK identity hint should be stored in the SSL_SESSION structure
and not in the parent context (which will overwrite values used
by other SSL structures with the same SSL_CTX).
Use BUF_strndup when copying identity as it may not be null terminated.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 3c66a669df)
Construct bio_err and bio_stdout from file handles instead of FILE
pointers, since the latter might not be implemented (when OPENSSL_NO_STDIO
is defined).
Convert all output to use BIO_printf.
Change lh_foo to lh_SSL_SESSION_foo.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit bb8abd6735)
Conflicts:
crypto/threads/mttest.c
It is valid for an extension block to be present in a ClientHello, but to
be of zero length.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Recent HMAC changes broke ABI compatibility due to a new field in HMAC_CTX.
This backs that change out, and does it a different way.
Thanks to Timo Teras for the concept.
Conflicts:
crypto/hmac/hmac.c
Reviewed-by: Richard Levitte <levitte@openssl.org>
Also tighten X509_cmp_time to reject more than three fractional
seconds in the time; and to reject trailing garbage after the offset.
CVE-2015-1789
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Fix loop in do_free_upto if cmsbio is NULL: this will happen when attempting
to verify and a digest is not recognised. Reported by Johannes Bauer.
CVE-2015-1792
Reviewed-by: Matt Caswell <matt@openssl.org>
Fix error handling in ssl_session_dup, as well as incorrect setting up of
the session ticket. Follow on from CVE-2015-1791.
Thanks to LibreSSL project for reporting these issues.
Conflicts:
ssl/ssl_sess.c
Reviewed-by: Tim Hudson <tjh@openssl.org>
It should not be possible for DTLS message fragments to span multiple
packets. However previously if the message header fitted exactly into one
packet, and the fragment body was in the next packet then this would work.
Obviously this would fail if packets get re-ordered mid-flight.
Reviewed-by: Tim Hudson <tjh@openssl.org>
In the event of an error in the HMAC function, leaks can occur because the
HMAC_CTX does not get cleaned up.
Thanks to the BoringSSL project for reporting this issue.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit e43a13c807)
The function EC_POINT_is_on_curve does not return a boolean value.
It returns 1 if the point is on the curve, 0 if it is not, and -1
on error. Many usages within OpenSSL were incorrectly using this
function and therefore not correctly handling error conditions.
With thanks to the Open Crypto Audit Project for reporting this issue.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
(cherry picked from commit 68886be7e2)
This adds additional checks to the processing of extensions in a ClientHello
to ensure that either no extensions are present, or if they are then they
take up the exact amount of space expected.
With thanks to the Open Crypto Audit Project for reporting this issue.
Reviewed-by: Stephen Henson <steve@openssl.org>
Conflicts:
ssl/t1_lib.c
This fixes a memory leak that can occur whilst duplicating a BIO chain if
the call to CRYPTO_dup_ex_data() fails. It also fixes a second memory leak
where if a failure occurs after successfully creating the first BIO in the
chain, then the beginning of the new chain was not freed.
With thanks to the Open Crypto Audit Project for reporting this issue.
Reviewed-by: Stephen Henson <steve@openssl.org>
Conflicts:
crypto/bio/bio_lib.c
BUF_MEM_free() attempts to cleanse memory using memset immediately prior
to a free. This is at risk of being optimised away by the compiler, so
replace with a call to OPENSSL_cleanse() instead.
With thanks to the Open Crypto Audit Project for reporting this issue.
Reviewed-by: Stephen Henson <steve@openssl.org>
The fix for CVE-2015-1791 introduced an error in ssl_session_dup for
Kerberos.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit dcad51bc13)
For librypto to be complete, the stuff in both crypto/ and engines/
have to be built. Doing 'make test' or 'make apps' from a clean
source tree failed to do so.
Corrected by using the new 'build_libcrypto' in the top Makefile.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit acaff3b797)
There's a need for a target that will build all of libcrypto, so let's
add 'build_libcrypto' that does this. For ortogonality, let's also
add 'build_libssl'. Have both also depend on 'libcrypto.pc' and
'libssl.pc' so those get built together with the libraries.
This makes 'all' depend on fewer things directly.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 177b5f9c82)
Conflicts:
Makefile.org
Remove a comment that suggested further clean up was required.
DH_free() performs the necessary cleanup.
With thanks to the Open Crypto Audit Project for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit f3d889523e)
Ensure OPENSSL_cleanse() is called on the premaster secret value calculated for GOST.
With thanks to the Open Crypto Audit Project for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit b7ee4815f2)
Conflicts:
ssl/s3_srvr.c
Ensure the Kerberos pre-master secret has OPENSSL_cleanse called on it.
With thanks to the Open Crypto Audit Project for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 4e3dbe37ca)
A BIGNUM can have the value of -0. The function BN_bn2hex fails to account
for this and can allocate a buffer one byte too short in the event of -0
being used, leading to a one byte buffer overrun. All usage within the
OpenSSL library is considered safe. Any security risk is considered
negligible.
With thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke and
Filip Palian for discovering and reporting this issue.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit c56353071d)
Conflicts:
crypto/bn/bn_print.c
objects.pl only looked for a space to see if the name could be
used as a C identifier. Improve the test to match the real C
rules.
Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 591b7aef05)
This is for consistency.
Additionally, have its presence define OPENSSL_SYS_WINDOWS as well.
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 3f131556d6)
Conflicts:
e_os2.h
If a NewSessionTicket is received by a multi-threaded client when
attempting to reuse a previous ticket then a race condition can occur
potentially leading to a double free of the ticket data.
CVE-2015-1791
This also fixes RT#3808 where a session ID is changed for a session already
in the client session cache. Since the session ID is the key to the cache
this breaks the cache access.
Parts of this patch were inspired by this Akamai change:
c0bf69a791
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 27c76b9b80)
Conflicts:
ssl/ssl.h
ssl/ssl_err.c
This is a backport of commit e83ee04bb7 from
the master branch (and this has also been applied to 1.0.2). In 1.0.2 this
was CVE-2015-0207. For other branches there is no known security issue, but
this is being backported as a precautionary measure.
The DTLSv1_listen function is intended to be stateless and processes
the initial ClientHello from many peers. It is common for user code to
loop over the call to DTLSv1_listen until a valid ClientHello is received
with an associated cookie. A defect in the implementation of DTLSv1_listen
means that state is preserved in the SSL object from one invokation to the
next.
Reviewed-by: Richard Levitte <levitte@openssl.org>
dtls1_get_message has an |mt| variable which is the type of the message that
is being requested. If it is negative then any message type is allowed.
However the value of |mt| is not checked in one of the main code paths, so a
peer can send a message of a completely different type and it will be
processed as if it was the message type that we were expecting. This has
very little practical consequences because the current behaviour will still
fail when the format of the message isn't as expected.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 8c2b1d872b)
The size of the SRP extension can never be negative (the variable
|size| is unsigned). Therefore don't check if it is less than zero.
RT#3862
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 9c89d29083)
The return value of i2d functions can be negative if an error occurs.
Therefore don't assign the return value to an unsigned type and *then*
check if it is negative.
RT#3862
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 90e7cdff3a)
The members of struct timeval on OpenVMS are unsigned. The logic for
calculating timeouts needs adjusting to deal with this.
RT#3862
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit fc52ac9028)
If the record received is for a version that we don't support, previously we
were sending an alert back. However if the incoming record already looks
like an alert then probably we shouldn't do that. So suppress an outgoing
alert if it looks like we've got one incoming.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
The update: target in engines/ didn't recurse into engines/ccgost.
The update: and depend: targets in engines/ccgost needed a fixup.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 8b822d2566)
We had updates of certain header files in both Makefile.org and the
Makefile in the directory the header file lived in. This is error
prone and also sometimes generates slightly different results (usually
just a comment that differs) depending on which way the update was
done.
This removes the file update targets from the top level Makefile, adds
an update: target in all Makefiles and has it depend on the depend: or
local_depend: targets, whichever is appropriate, so we don't get a
double run through the whole file tree.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 0f539dc1a2)
Conflicts:
Makefile.org
apps/Makefile
test/Makefile
If BN_rand is called with |bits| set to 1 and |top| set to 1 then a 1 byte
buffer overflow can occur. There are no such instances within the OpenSSL at
the moment.
Thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke, Filip Palian for
discovering and reporting this issue.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
The functions BN_rshift and BN_lshift shift their arguments to the right or
left by a specified number of bits. Unpredicatable results (including
crashes) can occur if a negative number is supplied for the shift value.
Thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke and Filip Palian
for discovering and reporting this issue.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
(cherry picked from commit 7cc18d8158)
Conflicts:
crypto/bn/bn.h
crypto/bn/bn_err.c
If a client receives a bad hello request in DTLS then the alert is not
sent correctly.
RT#2801
Signed-off-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
(cherry picked from commit 4dc1aa0436)
When building a trust chain if the first attempt fails, then try to see if
alternate chains could be constructed that are trusted.
RT3637
RT3621
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
40 bit ciphers are limited to 512 bit RSA, 56 bit ciphers to 1024 bit.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit ac38115c1a)
Since the client has no way of communicating her supported parameter
range to the server, connections to servers that choose weak DH will
simply fail.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
- Do not advise generation of DH parameters with dsaparam to save
computation time.
- Promote use of custom parameters more, and explicitly forbid use of
built-in parameters weaker than 2048 bits.
- Advise the callback to ignore <keylength> - it is currently called
with 1024 bits, but this value can and should be safely ignored by
servers.
Reviewed-by: Rich Salz <rsalz@openssl.org>
The default bitlength is now 2048. Also clarify that either the number
of bits or the generator must be present:
$ openssl dhparam -2
and
$ openssl dhparam 2048
generate parameters but
$ openssl dhparam
does not.
Reviewed-by: Matt Caswell <matt@openssl.org>
Backport old patch to make it work in mixture of perls for Windows.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Cherry-picked from 7bb98eee3c
(cherry picked from commit 051b41df41)
The big "don't check for NULL" cleanup requires backporting some
of the lowest-level functions to actually do nothing if NULL is
given. This will make it easier to backport fixes to release
branches, where master assumes those lower-level functions are "safe"
This commit addresses those tickets: 3798 3799 3801.
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit f34b095fab)
The function obj_cmp() (file crypto/objects/obj_dat.c) can in some
situations call memcmp() with a null pointer and a zero length.
This is invalid behaviour. When compiling openssl with undefined
behaviour sanitizer (add -fsanitize=undefined to compile flags) this
can be seen. One example that triggers this behaviour is the pkcs7
command (but there are others, e.g. I've seen it with the timestamp
function):
apps/openssl pkcs7 -in test/testp7.pem
What happens is that obj_cmp takes objects of the type ASN1_OBJECT and
passes their ->data pointer to memcmp. Zero-sized ASN1_OBJECT
structures can have a null pointer as data.
RT#3816
Signed-off-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 2b8dc08b74)
Currently we set change_cipher_spec_ok to 1 before calling
ssl3_get_cert_verify(). This is because this message is optional and if it
is not sent then the next thing we would expect to get is the CCS. However,
although it is optional, we do actually know whether we should be receiving
one in advance. If we have received a client cert then we should expect
a CertificateVerify message. By the time we get to this point we will
already have bombed out if we didn't get a Certificate when we should have
done, so it is safe just to check whether |peer| is NULL or not. If it is
we won't get a CertificateVerify, otherwise we will. Therefore we should
change the logic so that we only attempt to get the CertificateVerify if
we are expecting one, and not allow a CCS in this scenario.
Whilst this is good practice for TLS it is even more important for DTLS.
In DTLS messages can be lost. Therefore we may be in a situation where a
CertificateVerify message does not arrive even though one was sent. In that
case the next message the server will receive will be the CCS. This could
also happen if messages get re-ordered in-flight. In DTLS if
|change_cipher_spec_ok| is not set and a CCS is received it is ignored.
However if |change_cipher_spec_ok| *is* set then a CCS arrival will
immediately move the server into the next epoch. Any messages arriving for
the previous epoch will be ignored. This means that, in this scenario, the
handshake can never complete. The client will attempt to retransmit
missing messages, but the server will ignore them because they are the wrong
epoch. The server meanwhile will still be waiting for the CertificateVerify
which is never going to arrive.
RT#2958
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit a0bd649336)
Matt's note: I added a call to X509V3err to Kurt's original patch.
RT#3840
Signed-off-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 344c271eb3)
If sk_SSL_CIPHER_new_null() returns NULL then ssl_bytes_to_cipher_list()
should also return NULL.
Based on an original patch by mrpre <mrpre@163.com>.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 14def5f537)
Ensure all fatal errors transition into the new error state for DTLS.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit cefc93910c)
Conflicts:
ssl/d1_srvr.c
Conflicts:
ssl/d1_srvr.c
Ensure all fatal errors transition into the new error state on the client
side.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit cc273a9361)
Conflicts:
ssl/s3_clnt.c
Conflicts:
ssl/s3_clnt.c
Ensure all fatal errors transition into the new error state on the server
side.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit cf9b0b6fb2)
Conflicts:
ssl/s3_srvr.c
Reusing an SSL object when it has encountered a fatal error can
have bad consequences. This is a bug in application code not libssl
but libssl should be more forgiving and not crash.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit a89db885e0)
Conflicts:
ssl/s3_srvr.c
ssl/ssl_stat.c
RT2943 only complains about the incorrect check of -K argument size,
we might as well do the same thing with the -iv argument.
Before this, we only checked that the given argument wouldn't give a
bitstring larger than EVP_MAX_KEY_LENGTH. we can be more precise and
check against the size of the actual cipher used.
(cherry picked from commit 8920a7cd04)
Reviewed-by: Rich Salz <rsalz@openssl.org>
The problem occurs in EVP_PKEY_sign() when using RSA with X931 padding.
It is only triggered if the RSA key size is smaller than the digest length.
So with SHA512 you can trigger the overflow with anything less than an RSA
512 bit key. I managed to trigger a 62 byte overflow when using a 16 bit RSA
key. This wasn't sufficient to cause a crash, although your mileage may
vary.
In practice RSA keys of this length are never used and X931 padding is very
rare. Even if someone did use an excessively short RSA key, the chances of
them combining that with a longer digest and X931 padding is very
small. For these reasons I do not believe there is a security implication to
this. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 34166d4189)
Add a sanity check to the print_bin function to ensure that the |off|
argument is positive. Thanks to Kevin Wojtysiak (Int3 Solutions) and
Paramjot Oberoi (Int3 Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 3deeeeb61b)
Sanity check the |len| parameter to ensure it is positive. Thanks to Kevin
Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for
reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit cb0f400b0c)
The return value is checked for 0. This is currently safe but we should
really check for <= 0 since -1 is frequently used for error conditions.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit c427570e50)
Conflicts:
ssl/ssl_locl.h
Conflicts:
ssl/ssl_locl.h
For SSLv3 the code assumes that |header_length| > |md_block_size|. Whilst
this is true for all SSLv3 ciphersuites, this fact is far from obvious by
looking at the code. If this were not the case then an integer overflow
would occur, leading to a subsequent buffer overflow. Therefore I have
added an explicit sanity check to ensure header_length is always valid.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 29b0a15a48)
The static function dynamically allocates an output buffer if the output
grows larger than the static buffer that is normally used. The original
logic implied that |currlen| could be greater than |maxlen| which is
incorrect (and if so would cause a buffer overrun). Also the original
logic would call OPENSSL_malloc to create a dynamic buffer equal to the
size of the static buffer, and then immediately call OPENSSL_realloc to
make it bigger, rather than just creating a buffer than was big enough in
the first place. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot
Oberoi (Int3 Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 9d9e37744c)
There was already a sanity check to ensure the passed buffer length is not
zero. Extend this to ensure that it also not negative. Thanks to Kevin
Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for
reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit b86d7dca69)
The various implementations of EVP_CTRL_AEAD_TLS_AAD expect a buffer of at
least 13 bytes long. Add sanity checks to ensure that the length is at
least that. Also add a new constant (EVP_AEAD_TLS1_AAD_LEN) to evp.h to
represent this length. Thanks to Kevin Wojtysiak (Int3 Solutions) and
Paramjot Oberoi (Int3 Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit c826988109)
Conflicts:
ssl/record/ssl3_record.c
Conflicts:
apps/speed.c
crypto/evp/e_aes_cbc_hmac_sha256.c
crypto/evp/evp.h
Add a sanity check to DES_enc_write to ensure the buffer length provided
is not negative. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot
Oberoi (Int3 Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 873fb39f20)
Fortify flagged up a problem in n_do_ssl_write() in SSLv2. Analysing the
code I do not believe there is a real problem here. However the logic flows
are complicated enough that a sanity check of |len| is probably worthwhile.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit c5f8cd7bc6)
The function CRYPTO_strdup (aka OPENSSL_strdup) fails to check the return
value from CRYPTO_malloc to see if it is NULL before attempting to use it.
This patch adds a NULL check.
RT3786
Signed-off-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 37b0cf936744d9edb99b5dd82cae78a7eac6ad60)
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 20d21389c8b6f5b754573ffb6a4dc4f3986f2ca4)
EAP-FAST session resumption relies on handshake message lookahead
to determine server intentions. Commits
980bc1ec61
and
7b3ba508af
removed the lookahead so broke session resumption.
This change partially reverts the commits and brings the lookahead back
in reduced capacity for TLS + EAP-FAST only. Since EAP-FAST does not
support regular session tickets, the lookahead now only checks for a
Finished message.
Regular handshakes are unaffected by this change.
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 6e3d015363)
The logic with how 'ok' was calculated didn't quite convey what's "ok",
so the logic is slightly redone to make it less confusing.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 06affe3dac)
Filled in lots of return value checks that were missing the GOST engine, and
added appropriate error handling.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 8817e2e0c9)
Fix miscellaneous NULL pointer derefs in the sureware engine.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 7b611e5fe8)
Fix bug where i2c_ASN1_INTEGER mishandles zero if it is marked as
negative.
Thanks to Huzaifa Sidhpurwala <huzaifas@redhat.com> and
Hanno Böck <hanno@hboeck.de> for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit a0eed48d37)
A 0-length ciphers list is never permitted. The old code only used to
reject an empty ciphers list for connections with a session ID. It
would later error out on a NULL structure, so this change just moves
the alert closer to the problem source.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 3ae91cfb32)
If OpenSSL is configured with no-tlsext then ssl_get_prev_session can read
past the end of the ClientHello message if the session_id length in the
ClientHello is invalid. This should not cause any security issues since the
underlying buffer is 16k in size. It should never be possible to overrun by
that many bytes.
This is probably made redundant by the previous commit - but you can never be
too careful.
With thanks to Qinghao Tang for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 5e0a80c1c9)
The ClientHello processing is insufficiently rigorous in its checks to make
sure that we don't read past the end of the message. This does not have
security implications due to the size of the underlying buffer - but still
needs to be fixed.
With thanks to Qinghao Tang for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit c9642eb1ff79a30e2c7632ef8267cc34cc2b0d79)
It would set gen->d.dirn to a freed pointer in case X509V3_NAME_from_section
failed.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 8ec5c5dd36)
While *pval is usually a pointer in rare circumstances it can be a long
value. One some platforms (e.g. WIN64) where
sizeof(long) < sizeof(ASN1_VALUE *) this will write past the field.
*pval is initialised correctly in the rest of ASN1_item_ex_new so setting it
to NULL is unecessary anyway.
Thanks to Julien Kauffmann for reporting this issue.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit f617b4969a)
Conflicts:
crypto/asn1/tasn_new.c
Since source reformat, we ended up with some error reason string
definitions that spanned two lines. That in itself is fine, but we
sometimes edited them to provide better strings than what could be
automatically determined from the reason macro, for example:
{ERR_REASON(SSL_R_NO_GOST_CERTIFICATE_SENT_BY_PEER),
"Peer haven't sent GOST certificate, required for selected ciphersuite"},
However, mkerr.pl didn't treat those two-line definitions right, and
they ended up being retranslated to whatever the macro name would
indicate, for example:
{ERR_REASON(SSL_R_NO_GOST_CERTIFICATE_SENT_BY_PEER),
"No gost certificate sent by peer"},
Clearly not what we wanted. This change fixes this problem.
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 2cfdfe0918)
There is no indication that the timing differences are exploitable in
OpenSSL, and indeed there is some indication (Usenix '14) that they
are too small to be exploitable. Nevertheless, be careful and apply
the same countermeasures as in s3_srvr.c
Thanks to Nimrod Aviram, Sebastian Schinzel and Yuval Shavitt for
reporting this issue.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit ae50d82700)
Resolve a problem when using s_server with ECDHE cipher
suites in OpenSSL_1_0_1-stable. Due to an uninitialized variable,
SSL_CTX_set_tmp_ecdh() is not always invoked within s_server. This bug
appears to have been introduced by
059907771b.
Reviewed-by: Tim Hudson <tjh@openssl.org>
RFC5915 requires the use of the I2OSP primitive as defined in RFC3447
for storing an EC Private Key. This converts the private key into an
OCTETSTRING and retains any leading zeros. This commit ensures that those
leading zeros are present if required.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 30cd4ff294)
Conflicts:
crypto/ec/ec_asn1.c
While a true positive, it's almost harmless because EVP_DecryptInit_ex would have to fail and that doesn't happen under normal operation.
Reviewed-by: Richard Levitte <levitte@openssl.org>
if CAfile or CApath were also supplied and successfully loaded first.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 70e5fd8778)
create an HMAC
Inspired by BoringSSL commit 2fe7f2d0d9a6fcc75b4e594eeec306cc55acd594
Reviewed-by: Richard Levitte <levitte@openssl.org>
Conflicts:
crypto/hmac/hmac.c
Ensure all calls to RAND_bytes and RAND_pseudo_bytes have their return
value checked correctly
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 8f8e4e4f52)
Conflicts:
crypto/evp/e_des3.c
The certificate already contains the DH parameters in that case.
ssl3_send_server_key_exchange() would fail in that case anyway.
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 93f1c13619)
If a set of certificates is supplied to OCSP_basic_verify use those in
addition to any present in the OCSP response as untrusted CAs when
verifying a certificate chain.
PR#3668
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 4ca5efc287)
Fix some unsigned/signed warnings introduced as part of the fix
for CVE-2015-0293
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Remove entries from CHANGES and NEWS from letter releases that occur *after*
the next point release. Without this we get duplicate entries for the same
issue appearing multiple times.
Reviewed-by: Richard Levitte <levitte@openssl.org>
This assert is reachable for servers that support SSLv2 and export ciphers.
Therefore, such servers can be DoSed by sending a specially crafted
SSLv2 CLIENT-MASTER-KEY.
Also fix s2_srvr.c to error out early if the key lengths are malformed.
These lengths are sent unencrypted, so this does not introduce an oracle.
CVE-2015-0293
This issue was discovered by Sean Burford (Google) and Emilia Käsper of
the OpenSSL development team.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
In PKCS#7, the ASN.1 content component is optional.
This typically applies to inner content (detached signatures),
however we must also handle unexpected missing outer content
correctly.
This patch only addresses functions reachable from parsing,
decryption and verification, and functions otherwise associated
with reading potentially untrusted data.
Correcting all low-level API calls requires further work.
CVE-2015-0289
Thanks to Michal Zalewski (Google) for reporting this issue.
Reviewed-by: Steve Henson <steve@openssl.org>
Fix segmentation violation when ASN1_TYPE_cmp is passed a boolean type. This
can be triggered during certificate verification so could be a DoS attack
against a client or a server enabling client authentication.
CVE-2015-0286
Reviewed-by: Richard Levitte <levitte@openssl.org>
Some miscellaneous removal of dead code from apps. Also fix an issue with
error handling with pkcs7.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 11abf92259)
Passing a negative value for the "-time" option to s_time results in a seg
fault. This commit fixes it so that time has to be greater than 0.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit dfef52f6f2)
The function tls1_PRF counts the number of digests in use and partitions
security evenly between them. There always needs to be at least one digest
in use, otherwise this is an internal error. Add a sanity check for this.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 668f6f08c6)
The function sk_zero is supposed to zero the elements held within a stack.
It uses memset to do this. However it calculates the size of each element
as being sizeof(char **) instead of sizeof(char *). This probably doesn't
make much practical difference in most cases, but isn't a portable
assumption.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 7132ac830f)
Move memory allocation failure checks closer to the site of the malloc in
dgst app. Only a problem if the debug flag is set...but still should be
fixed.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit be1477adc9)
Td4 and Te4 are arrays of u8. A u8 << int promotes the u8 to an int first then shifts.
If the mathematical result of a shift (as modelled by lhs * 2^{rhs}) is not representable
in an integer, behaviour is undefined. In other words, you can't shift into the sign bit
of a signed integer. Fix this by casting to u32 whenever we're shifting left by 24.
(For consistency, cast other shifts, too.)
Caught by -fsanitize=shift
Submitted by Nick Lewycky (Google)
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 8b37e5c14f)
When printing out an ASN.1 structure if the type is an item template don't
fall thru and attempt to interpret as a primitive type.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 5dc1247a74)
The return value from ASN1_STRING_new() was not being checked which could
lead to a NULL deref in the event of a malloc failure. Also fixed a mem
leak in the error path.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 0c7ca4033d)
The return value from ASN1_STRING_new() was not being checked which could
lead to a NULL deref in the event of a malloc failure. Also fixed a mem
leak in the error path.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 6aa8dab2bb)
Conflicts:
crypto/dh/dh_ameth.c
The call to asn1_do_adb can return NULL on error, so we should check the
return value before attempting to use it.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 34a7ed0c39)
ASN1_primitive_new takes an ASN1_ITEM * param |it|. There are a couple
of conditional code paths that check whether |it| is NULL or not - but
later |it| is deref'd unconditionally. If |it| was ever really NULL then
this would seg fault. In practice ASN1_primitive_new is marked as an
internal function in the public header file. The only places it is ever
used internally always pass a non NULL parameter for |it|. Therefore, change
the code to sanity check that |it| is not NULL, and remove the conditional
checking.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 9e488fd6ab)
Calling EVP_DigestInit_ex which has already had the digest set up for it
should be possible. You are supposed to be able to pass NULL for the type.
However currently this seg faults.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit a01087027b)
In the event of an error |rr| could be NULL. Therefore don't assume you can
use |rr| in the error handling code.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 8c5a7b33c6)
Cleanse various intermediate buffers used by the PRF (backported version
from master).
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 35fafc4dbc)
Conflicts:
ssl/s3_enc.c
Ensure all malloc failures return -1.
Reported by Adam Langley (Google).
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 06c6a2b4a3)
BIO_debug_callback() no longer assumes the hexadecimal representation of
a pointer fits in 8 characters.
Signed-off-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 460e920d8a)
New function ASN1_STRING_clear_free which cleanses an ASN1_STRING
structure before freeing it.
Call ASN1_STRING_clear_free on PKCS#8 private key components.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit a8ae0891d4)
They are moved to the COMPLEMENTOFDEFAULT instead.
This also fixes SSLv2 to be part of COMPLEMENTOFDEFAULT.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit f417997a32)
Conflicts:
ssl/ssl_ciph.c
This patch uses warning/fatal constants instead of numbers with comments for
warning/alerts in d1_pkt.c and s3_pkt.c
RT#3725
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit fd865cadcb)
Miscellaneous unchecked malloc fixes. Also fixed some mem leaks on error
paths as I spotted them along the way.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 918bb86529)
Conflicts:
crypto/bio/bss_dgram.c
Conflicts:
apps/cms.c
apps/s_cb.c
apps/s_server.c
apps/speed.c
crypto/dh/dh_pmeth.c
ssl/s3_pkt.c
The format script didn't correctly recognise some ASN.1 macros and
didn't reformat some files as a result. Fix script and reformat
affected files.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 437b14b533)
Some Cisco appliances use a pre-standard version number for DTLS. We support
this as DTLS1_BAD_VER within the code.
This change fixes d2i_SSL_SESSION for that DTLS version.
Based on an original patch by David Woodhouse <dwmw2@infradead.org>
RT#3704
Reviewed-by: Tim Hudson <tjh@openssl.org>
Conflicts:
ssl/ssl_asn1.c
Conflicts:
ssl/dtls1.h
Fixed various missing return value checks in ssl3_send_newsession_ticket.
Also a mem leak on error.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Conflicts:
ssl/s3_srvr.c
Conflicts:
ssl/s3_srvr.c
When OpenSSL is configured with no-ec, then the new evp_extra_test fails to
pass. This change adds appropriate OPENSSL_NO_EC guards around the code.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit a988036259)
called evp_test.c, so I have called this one evp_extra_test.c
Reviewed-by: Emilia Käsper <emilia@openssl.org>
Conflicts:
crypto/evp/Makefile
test/Makefile
Conflicts:
test/Makefile
crypto/evp/evp_extra_test.c
The previous defaulting to TERMIOS took away -DTERMIOS / -DTERMIO a
bit too enthusiastically. Windows/DOSish platforms of all sorts get
identified as OPENSSL_SYS_MSDOS, and they get a different treatment
altogether UNLESS -DTERMIO or -DTERMIOS is explicitely given with the
configuration. The answer is to restore those macro definitions for
the affected configuration targets.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit ba4bdee718)
Conflicts:
Configure
The rationale for this move is that TERMIOS is default, supported by
POSIX-1.2001, and most definitely on Linux. For a few other systems,
TERMIO may still be the termnial interface of preference, so we keep
-DTERMIO on those in Configure.
crypto/ui/ui_openssl.c is simplified in this regard, and will define
TERMIOS for all systems except a select few exceptions.
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 64e6bf64b3)
Conflicts:
Configure
crypto/ui/ui_openssl.c
Free up bio_err after memory leak data has been printed to it.
In int_free_ex_data if ex_data is NULL there is nothing to free up
so return immediately and don't reallocate it.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 9c7a780bbe)
use read_ahead with DTLS because it doesn't work. Therefore read_ahead needs
to be the default.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit f400241251)
Conflicts:
apps/s_client.c
apps/s_server.c
cherry-picked from db7cb7ab9a
This wasn't cleanly cherry-picked, since the build
process changed a bit for 1.0.2.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Per discussion: should not exit. Should not print to stderr.
Errors are ignored. Updated doc to reflect that, and the fact
that this function is to be avoided.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(cherry picked from commit abdd677125)
// On Itanium2ittakes~190ticks.Thisisbecauseofstallson
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.