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)
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)
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)
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>
Avoid using cnid = 0, use NID_undef instead, and return early instead
of trying to find an instance of that in the subject DN.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit fffc2faeb2)
Initialize pointers in param id by the book (explicit NULL assignment,
rather than just memset 0).
In x509_verify_param_zero() set peername to NULL after freeing it.
In x509_vfy.c's internal check_hosts(), avoid potential leak of
possibly already non-NULL peername. This is only set when a check
succeeds, so don't need to do this repeatedly in the loop.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit a0724ef1c9)
The |z| value should be 0x04 not 0x02
RT#3838
Signed-off-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 91d2728b38)
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)
Slightly modified from the original PR.
Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit a7e974c7be)
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>
Fix more potential leaks in X509_verify_cert()
Fix memory leak in ClientHello test
Fix memory leak in gost2814789 test
Fix potential memory leak in PKCS7_verify()
Fix potential memory leaks in X509_add1_reject_object()
Refactor to use "goto err" in cleanup.
Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit 55500ea7c4)
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)
Part of RT 3997
Per Ben, just jump to common exit code.
Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit cc2829e664)
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>
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)
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>
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>
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)
A small rearrangement so the inclusion of rsaz_exp.h would be
unconditional, but what that header defines becomes conditional.
This solves the weirdness where rsaz_exp.h gets in and out of the
dependency list for bn_exp.c, depending on the present architecture.
Reviewed-by: Rich Salz <rsalz@openssl.org>
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
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>
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 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>
Pointed out by Victor Vasiliev (vasilvv@mit.edu) via Adam Langley
(Google).
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 1e4a355dca)
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
Some tool chains (e.g. android) do not define IP_PMTUDISC_PROBE, and so
this build breaks.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 133dce447b)
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)
The code in bss_dgram.c checks if IP_MTUDISCOVER is defined, where it
should test for IP_MTU_DISCOVER
RT#3888
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 0baaff1a76)
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)
bn_get_bits5 was overstepping array boundary by 1 byte. It was exclusively
read overstep and data could not have been used. The only potential problem
would be if array happens to end on the very edge of last accesible page.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 69567687b0)
BLKINIT optimization worked on T4, but for some reason appears "too
aggressive" for T3 triggering intermiitent EC failures. It's not clear
why only EC is affected...
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 579734ced6)
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
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)
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>
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)
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)
