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 68886be7e2cd395a759fcd41d2cede461b68843d)
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 1e4a355dcabe2f75df5bb8b41b394d37037169d2)
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 c56353071d9849220714d8a556806703771b9269)
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 133dce447b259e0bb93076bf5fd0ce475d16910a)
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 591b7aef05b22cba09b179e5787a9bf40dfc9508)
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 0baaff1a76d4c91b9c1121059be63175454d7b03)
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 fc52ac9028b9492fb086ba35a3352ea46e03ecfc)
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 69567687b0b7ed67831c17a010f4d46dfd059aef)
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 579734ced696125c0768a2f4e56e97939b0e6e49)
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 0f539dc1a2f45580435c39dada44dd276e79cb88)
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 7cc18d8158b5fc2676393d99b51c30c135502107)
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 ac38115c1a4fb61c66c2a8cd2a9800751828d328)
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 2b8dc08b74fc3c6d4c2fc855cc23bac691d985be)
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 344c271eb339fc2982e9a3584a94e51112d84584)
Remove dependency on ssl_locl.h from v3_scts.c, and incidentally fix a build problem with
kerberos (the dependency meant v3_scts.c was trying to include krb5.h, but without having been
passed the relevanant -I flags to the compiler)
Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
(cherry picked from commit d13bd6130bd856e9f512c43499e5fc70943bf133)
Conflicts:
crypto/x509v3/v3_scts.c
Use sizeof *foo parameter, to avoid these errors.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(cherry picked from commit 53ba0a9e91ad203de2943edaf1090ab17ec435fa)
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 34166d41892643a36ad2d1f53cc0025e2edc2a39)
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 3deeeeb61b0c5b9b5f0993a67b7967d2f85186da)
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 9d9e37744cd5119f9921315864d1cd28717173cd)
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 b86d7dca69f5c80abd60896c8ed3039fc56210cc)
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 c8269881093324b881b81472be037055571f73f3)
Conflicts:
ssl/record/ssl3_record.c
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 873fb39f20b6763daba226b74e83fb194924c7bf)
This reverts commit 47daa155a31b0a54ce09ad2ed4d55fad74096dab.
The above commit was backported to the 1.0.2 branch as part of backporting
the alternative chain verify algorithm changes. However it has been pointed
out (credit to Shigeki Ohtsu) that this is unnecessary in 1.0.2 as this
commit is a work around for loop checking that only exists in master.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Thanks to Brian Smith for reporting these issues.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 53dd4ddf71ad79a64be934ca19445b1cf560adab)
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)
This addresses
- request for improvement for faster key setup in RT#3576;
- clearing registers and stack in RT#3554 (this is more of a gesture to
see if there will be some traction from compiler side);
- more commentary around input parameters handling and stack layout
(desired when RT#3553 was reviewed);
- minor size and single block performance optimization (was lying around);
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 23f6eec71dbd472044db7dc854599f1de14a1f48)
Disable loop checking when we retry verification with an alternative path.
This fixes the case where an intermediate CA is explicitly trusted and part
of the untrusted certificate list. By disabling loop checking for this case
the untrusted CA can be replaced by the explicitly trusted case and
verification will succeed.
Signed-off-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit e5991ec528b1c339062440811e2641f5ea2b328b)
Reviewed-by: Rich Salz <rsalz@openssl.org>
