Richard Levitte [Mon, 7 Dec 2015 15:50:15 +0000 (16:50 +0100)]
Change tar owner and group to just 0
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.
Richard Levitte [Mon, 7 Dec 2015 14:45:50 +0000 (15:45 +0100)]
Small changes to creating dists
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)
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: Richard Levitte <levitte@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>
Richard Levitte [Wed, 2 Dec 2015 17:18:03 +0000 (18:18 +0100)]
_BSD_SOURCE is deprecated, use _DEFAULT_SOURCE instead
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.
Matt Caswell [Mon, 30 Nov 2015 10:38:54 +0000 (10:38 +0000)]
Remove cookie validation return value trick
In the DTLS ClientHello processing the return value is stored in |ret| which
by default is -1. |ret| is only updated to a positive value once we are past
all points where we could hit an error. 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, and then once we were
past all error points we set |ret = -ret|. This is non-obvious behaviour and
could be error prone. This commit tries to make this a bit more intuitive.
Pascal Cuoq [Sun, 22 Nov 2015 23:13:15 +0000 (00:13 +0100)]
ssl3_free(): Return if it wasn't created
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)
Viktor Dukhovni [Sat, 21 Nov 2015 05:59:07 +0000 (00:59 -0500)]
Good hygiene with size_t output argument.
Though the callers check the function return value and ignore the
size_t output argument on failure, it is still often not ideal to
store -1 in a size_t on error. That might signal an unduly large
buffer. Instead set the size_t to 0, to indicate no space.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Fri, 6 Nov 2015 16:31:21 +0000 (16:31 +0000)]
Ensure all EVP calls have their returns checked where appropriate
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>
Andy Polyakov [Fri, 13 Nov 2015 22:44:23 +0000 (23:44 +0100)]
bn/asm/ppc64-mont.pl: adapt for little-endian.
The problem remained unnoticed so far, because it's never called by default.
You have to craft OPENSSL_ppccap environment variable to trigger the problem.
Andy Polyakov [Tue, 10 Nov 2015 20:11:24 +0000 (21:11 +0100)]
bn/asm/s390x.S: improve performance on z196 and z13 by up to 26%. [even z10 is couple percent faster]. Triggered by RT#4128, but solves the problem by real modulo-scheduling.
Matt Caswell [Tue, 10 Nov 2015 15:17:42 +0000 (15:17 +0000)]
Stop DTLS servers asking for unsafe legacy renegotiation
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.
Matt Caswell [Tue, 3 Nov 2015 15:49:08 +0000 (15:49 +0000)]
Only call ssl3_init_finished_mac once for DTLS
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.
Matt Caswell [Mon, 9 Nov 2015 16:37:33 +0000 (16:37 +0000)]
Remove redundant check from tls1_get_curvelist
The function tls1_get_curvelist() has an explicit check to see if s->cert
is NULL or not. However the check appears *after* calling the tls1_suiteb
macro which derefs s->cert. In reality s->cert can never be NULL because
it is created in SSL_new(). If the malloc fails then the SSL_new call fails
and no SSL object is created.
Matt Caswell [Wed, 4 Nov 2015 15:51:02 +0000 (15:51 +0000)]
Fix missing malloc return value checks
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>
Richard Levitte [Mon, 9 Nov 2015 08:50:56 +0000 (09:50 +0100)]
Make the match for previous cflags a bit more strict
./Configure [target] --strict-warnings -Wno-pedantic-ms-format
would not add '-pedantic' because it matches '-Wno-pedantic-ms-format',
which was added first.
RFC5753 requires that we omit parameters for AES key wrap and set them
to NULL for 3DES wrap. OpenSSL decrypt uses the received algorithm
parameters so can transparently handle either form.
Matt Caswell [Thu, 8 Oct 2015 12:36:10 +0000 (13:36 +0100)]
Don't treat a bare OCTETSTRING as DigestInfo in int_rsa_verify
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.
Disable -Wshadow error when building with GCC
Add support for linu-x86_64-clang debug; this is needed for Travis CI.
Disable linux-clang and mingw debug builds on Travis CI; not supported.
Fix Travis email notifications config
Signed-off-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Andy Polyakov <appro@openssl.org>
Matt Caswell [Mon, 5 Oct 2015 13:12:05 +0000 (14:12 +0100)]
Change functions to pass in a limit rather than calculate it
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.
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.
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.
David Woodhouse [Wed, 9 Sep 2015 19:49:01 +0000 (15:49 -0400)]
RT3479: Add UTF8 support to BIO_read_filename()
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.
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.
Matt Caswell [Wed, 16 Sep 2015 09:47:15 +0000 (10:47 +0100)]
Make sure OPENSSL_cleanse checks for NULL
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.
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 3cdd1e94b1d71f2ce3002738f9506da91fe2af45)