Commit graph

211 commits

Author SHA1 Message Date
Viktor Dukhovni
d33def6624 Deprecate the -issuer_checks debugging option
This was a developer debugging feature and was never a useful public
interface.

Added all missing X509 error codes to the verify(1) manpage, but
many still need a description beyond the associated text string.

Sorted the errors in x509_txt.c by error number.

Reviewed-by: Stephen Henson <steve@openssl.org>
2016-02-10 12:34:06 -05:00
Viktor Dukhovni
c0a445a9f2 Suppress DANE TLSA reflection when verification fails
As documented both SSL_get0_dane_authority() and SSL_get0_dane_tlsa()
are expected to return a negative match depth and nothing else when
verification fails.  However, this only happened when verification
failed during chain construction.  Errors in verification of the
constructed chain did not have the intended effect on these functions.

This commit updates the functions to check for verify_result ==
X509_V_OK, and no longer erases any accumulated match information
when chain construction fails.  Sophisticated developers can, with
care, use SSL_set_verify_result(ssl, X509_V_OK) to "peek" at TLSA
info even when verification fail.  They must of course first check
and save the real error, and restore the original error as quickly
as possible.  Hiding by default seems to be the safer interface.

Introduced X509_V_ERR_DANE_NO_MATCH code to signal failure to find
matching TLSA records.  Previously reported via X509_V_ERR_CERT_UNTRUSTED.

This also changes the "-brief" output from s_client to include
verification results and TLSA match information.

Mentioned session resumption in code example in SSL_CTX_dane_enable(3).
Also mentioned that depths returned are relative to the verified chain
which is now available via SSL_get0_verified_chain(3).

Added a few more test-cases to danetest, that exercise the new
code.

Resolved thread safety issue in use of static buffer in
X509_verify_cert_error_string().

Fixed long-stating issue in apps/s_cb.c which always sets verify_error
to either X509_V_OK or "chain to long", code elsewhere (e.g.
s_time.c), seems to expect the actual error.  [ The new chain
construction code is expected to correctly generate "chain
too long" errors, so at some point we need to drop the
work-arounds, once SSL_set_verify_depth() is also fixed to
propagate the depth to X509_STORE_CTX reliably. ]

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-02-08 14:46:09 -05:00
FdaSilvaYY
0d4fb84390 GH601: Various spelling fixes.
Signed-off-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-02-05 15:25:50 -05:00
Viktor Dukhovni
3921ded79a Ensure correct chain depth for policy checks with DANE bare key TA
Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
2016-02-05 11:13:11 -05:00
Viktor Dukhovni
895c2f84a6 Long overdue cleanup of X509 policy tree verification
Replace all magic numbers with #defined constants except in boolean
functions that return 0 for failure and 1 for success.  Avoid a
couple memory leaks in error recovery code paths.  Code style
improvements.

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
2016-02-05 11:13:11 -05:00
Viktor Dukhovni
33cc5dde47 Compat self-signed trust with reject-only aux data
When auxiliary data contains only reject entries, continue to trust
self-signed objects just as when no auxiliary data is present.

This makes it possible to reject specific uses without changing
what's accepted (and thus overring the underlying EKU).

Added new supported certs and doubled test count from 38 to 76.

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
2016-01-31 21:24:12 -05:00
Viktor Dukhovni
0daccd4dc1 Check chain extensions also for trusted certificates
This includes basic constraints, key usages, issuer EKUs and auxiliary
trust OIDs (given a trust suitably related to the intended purpose).

Added tests and updated documentation.

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
2016-01-31 21:23:23 -05:00
Rich Salz
349807608f Remove /* foo.c */ comments
This was done by the following
        find . -name '*.[ch]' | /tmp/pl
where /tmp/pl is the following three-line script:
        print unless $. == 1 && m@/\* .*\.[ch] \*/@;
        close ARGV if eof; # Close file to reset $.

And then some hand-editing of other files.

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
2016-01-26 16:40:43 -05:00
Viktor Dukhovni
6e32825631 Check Suite-B constraints with EE DANE records
When DANE-EE(3) matches or either of DANE-EE/PKIX-EE fails, we don't
build a chain at all, but rather succeed or fail with just the leaf
certificate.  In either case also check for Suite-B violations.

As unlikely as it may seem that anyone would enable both DANE and
Suite-B, we should do what the application asks.

Took the opportunity to eliminate the "cb" variables in x509_vfy.c,
just call ctx->verify_cb(ok, ctx)

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
2016-01-20 18:59:46 -05:00
Viktor Dukhovni
0e76014e58 Drop cached certificate signature validity flag
It seems risky in the context of cross-signed certificates when the
same certificate might have multiple potential issuers.  Also rarely
used, since chains in OpenSSL typically only employ self-signed
trust-anchors, whose self-signatures are not checked, while untrusted
certificates are generally ephemeral.

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
2016-01-18 13:20:48 -05:00
Dr. Stephen Henson
c864e7611f Add lookup_certs for a trusted stack.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
2016-01-15 22:37:18 +00:00
Viktor Dukhovni
497ecc0d7d Cosmetic polish for last-resort depth 0 check
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-01-14 15:12:24 -05:00
Viktor Dukhovni
bdcadca24e Fix last-resort depth 0 check when the chain has multiple certificates
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-01-14 14:40:50 -05:00
Viktor Dukhovni
311f27852a Always initialize X509_STORE_CTX get_crl pointer
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-01-14 12:29:16 -05:00
Viktor Dukhovni
170b735820 DANE support for X509_verify_cert()
Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-01-07 13:48:59 -05:00
Viktor Dukhovni
919ba00942 DANE support structures, constructructors and accessors
Also tweak some of the code in demos/bio, to enable interactive
testing of BIO_s_accept's use of SSL_dup.  Changed the sconnect
client to authenticate the server, which now exercises the new
SSL_set1_host() function.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-01-05 19:31:49 -05:00
Viktor Dukhovni
e29c73c93b Fix X509_STORE_CTX_cleanup()
Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
2016-01-03 18:45:05 -05:00
Viktor Dukhovni
d9b8b89bec X509_verify_cert() cleanup
Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-01-03 17:21:16 -05:00
Dr. Stephen Henson
c01ff880d4 New function X509_get0_pubkey
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
2015-12-14 23:06:14 +00:00
Rich Salz
e6390acac9 ex_data part 2: doc fixes and CRYPTO_free_ex_index.
Add CRYPTO_free_ex_index (for shared libraries)
Unify and complete the documentation for all "ex_data" API's and objects.
Replace xxx_get_ex_new_index functions with a macro.
Added an exdata test.
Renamed the ex_data internal datatypes.

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-12-01 11:48:37 -05:00
Dr. Stephen Henson
9689a6aeed Remove X509_VERIFY_PARAM_ID
Now that X509_VERIFY_PARAM is opaque X509_VERIFY_PARAM_ID is no longer
needed.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-11-26 16:07:14 +00:00
Matt Caswell
90945fa31a Continue standardising malloc style for libcrypto
Continuing from previous commit ensure our style is consistent for malloc
return checks.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2015-11-09 22:48:41 +00:00
Dr. Stephen Henson
34a42e1489 embed CRL serial number and signature fields
Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-10-15 15:36:58 +01:00
mrpre
ecdaa1aefd In X509_STORE_CTX_init, cleanup on failure
Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-09-05 17:25:10 -04:00
David Woodhouse
d35ff2c0ad RT3951: Add X509_V_FLAG_NO_CHECK_TIME to suppress time check
In some environments, such as firmware, the current system time is entirely
meaningless. Provide a clean mechanism to suppress the checks against it.

Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-09-05 16:17:15 -04:00
David Woodhouse
47bbaa5b60 Revert "OPENSSL_NO_xxx cleanup: RFC3779"
This reverts the non-cleanup parts of commit c73ad69017. We do actually
have a reasonable use case for OPENSSL_NO_RFC3779 in the EDK2 UEFI
build, since we don't have a strspn() function in our runtime environment
and we don't want the RFC3779 functionality anyway.

In addition, it changes the default behaviour of the Configure script so
that RFC3779 support isn't disabled by default. It was always disabled
from when it was first added in 2006, right up until the point where
OPENSSL_NO_RFC3779 was turned into a no-op, and the code in the
Configure script was left *trying* to disable it, but not actually
working.

Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-09-03 16:31:09 -04:00
Rich Salz
b51bce9420 Add and use OPENSSL_zalloc
There are many places (nearly 50) where we malloc and then memset.
Add an OPENSSL_zalloc routine to encapsulate that.
(Missed one conversion; thanks Richard)
Also fixes GH328

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-09-02 22:05:37 -04:00
Dr. Stephen Henson
e3e571925c make X509_CRL opaque
Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-09-02 21:26:17 +01:00
Viktor Dukhovni
a0724ef1c9 Better handling of verify param id peername field
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>
2015-09-02 09:53:29 -04:00
Dr. Stephen Henson
05f0fb9f6a Add X509_up_ref function.
Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-08-31 23:18:55 +01:00
Dr. Stephen Henson
65cbf983ca Add X509_CRL_up_ref function
Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-08-31 23:18:54 +01:00
Alessandro Ghedini
55500ea7c4 GH354: Memory leak fixes
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>
2015-08-28 11:18:04 -04:00
Ismo Puustinen
cc2829e664 GH364: Free memory on an error path
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>
2015-08-13 12:13:49 -04:00
Rich Salz
fbfcb22439 RT3999: Remove sub-component version strings
Especially since after the #ifdef cleanups this is not useful.

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-08-10 12:13:32 -04:00
Matt Caswell
7f3f41d816 Extend -show_chain option to verify to show more info
The -show_chain flag to the verify command line app shows information about
the chain that has been built. This commit adds the text "untrusted" against
those certificates that have been used from the untrusted list.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-07-07 21:57:12 +01:00
Matt Caswell
aae41f8c54 Reject calls to X509_verify_cert that have not been reinitialised
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>
2015-07-07 21:57:11 +01:00
Matt Caswell
2aacec8f4a Fix alternate chains certificate forgery issue
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>
2015-07-07 21:57:11 +01:00
Emilia Kasper
f48b83b4fb Fix length checks in X509_cmp_time to avoid out-of-bounds reads.
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>
2015-06-11 11:06:30 +01:00
Richard Levitte
b39fc56061 Identify and move common internal libcrypto header files
There are header files in crypto/ that are used by a number of crypto/
submodules.  Move those to crypto/include/internal and adapt the
affected source code and Makefiles.

The header files that got moved are:

crypto/cryptolib.h
crypto/md32_common.h

Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-05-14 17:21:40 +02:00
Gunnar Kudrjavets
4c9b0a0314 Initialize potentially uninitialized local variables
Compiling OpenSSL code with MSVC and /W4 results in a number of warnings.
One category of warnings is particularly interesting - C4701 (potentially
uninitialized local variable 'name' used). This warning pretty much means
that there's a code path which results in uninitialized variables being used
or returned. Depending on compiler, its options, OS, values in registers
and/or stack, the results can be nondeterministic. Cases like this are very
hard to debug so it's rational to fix these issues.

This patch contains a set of trivial fixes for all the C4701 warnings (just
initializing variables to 0 or NULL or appropriate error code) to make sure
that deterministic values will be returned from all the execution paths.

RT#3835

Signed-off-by: Matt Caswell <matt@openssl.org>

Matt's note: All of these appear to be bogus warnings, i.e. there isn't
actually a code path where an unitialised variable could be used - its just
that the compiler hasn't been able to figure that out from the logic. So
this commit is just about silencing spurious warnings.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2015-05-06 13:06:46 +01:00
Rich Salz
16f8d4ebf0 memset, memcpy, sizeof consistency fixes
Just as with the OPENSSL_malloc calls, consistently use sizeof(*ptr)
for memset and memcpy.  Remove needless casts for those functions.
For memset, replace alternative forms of zero with 0.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-05-05 22:18:59 -04:00
Rich Salz
b4faea50c3 Use safer sizeof variant in malloc
For a local variable:
        TYPE *p;
Allocations like this are "risky":
        p = OPENSSL_malloc(sizeof(TYPE));
if the type of p changes, and the malloc call isn't updated, you
could get memory corruption.  Instead do this:
        p = OPENSSL_malloc(sizeof(*p));
Also fixed a few memset() calls that I noticed while doing this.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-05-04 15:00:13 -04:00
Rich Salz
25aaa98aa2 free NULL cleanup -- coda
After the finale, the "real" final part. :)  Do a recursive grep with
"-B1 -w [a-zA-Z0-9_]*_free" to see if any of the preceeding lines are
an "if NULL" check that can be removed.

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-05-01 14:37:16 -04:00
Rich Salz
666964780a Remove goto inside an if(0) block
There were a dozen-plus instances of this construct:
   if (0) { label: ..... }

Reviewed-by: Tim Hudson <tjh@openssl.org>
2015-05-01 14:29:48 -04:00
Rich Salz
222561fe8e free NULL cleanup 5a
Don't check for NULL before calling a free routine.  This gets X509_.*free:
    x509_name_ex_free X509_policy_tree_free X509_VERIFY_PARAM_free
    X509_STORE_free X509_STORE_CTX_free X509_PKEY_free
    X509_OBJECT_free_contents X509_LOOKUP_free X509_INFO_free

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-04-30 17:33:59 -04:00
Rich Salz
b196e7d936 remove malloc casts
Following ANSI C rules, remove the casts from calls to
OPENSSL_malloc and OPENSSL_realloc.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2015-04-28 15:28:14 -04:00
Rich Salz
c5ba2d9904 free NULL cleanup
EVP_.*free; this gets:
        EVP_CIPHER_CTX_free EVP_PKEY_CTX_free EVP_PKEY_asn1_free
        EVP_PKEY_asn1_set_free EVP_PKEY_free EVP_PKEY_free_it
        EVP_PKEY_meth_free; and also EVP_CIPHER_CTX_cleanup

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2015-03-28 10:54:15 -04:00
Dr. Stephen Henson
e5991ec528 Fix verify algorithm.
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.

Reviewed-by: Matt Caswell <matt@openssl.org>
2015-03-24 17:35:59 +00:00
Matt Caswell
15dba5be6a Add flag to inhibit checking for alternate certificate chains. Setting this
behaviour will force behaviour as per previous versions of OpenSSL

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
2015-02-25 09:14:54 +00:00
Matt Caswell
da084a5ec6 In certain situations the server provided certificate chain may no longer be
valid. However the issuer of the leaf, or some intermediate cert is in fact
in the trust store.

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: Dr. Stephen Henson <steve@openssl.org>
2015-02-25 09:14:13 +00:00