| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
|
|
|
|
|
|
|
| |
As reported by Steffen Ullrich and bluhm, the Finished tests in
p5-Net-SSLeay's t/local/43_misc_functions.t broke with with TLSv1.3.
The reason for this is that we don't copy the MDs over to the SSL, so
the API functions can't retrieve them. This commit fixes this part of
the test (one unrelated test still fails).
ok inoguchi jsing
|
|
|
|
|
|
|
| |
This is the natural type for these and it simplifies an upcoming commit.
The few consumers have been carefully checked to be fine with this.
ok inoguchi jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Comparing two GENERAL_NAME structures containing an EDIPARTYNAME can lead
to a crash. This enables a denial of service attack for an attacker who can
control both sides of the comparison.
Issue reported to OpenSSL on Nov 9 by David Benjamin.
OpenSSL shared the information with us on Dec 1st.
Fix from Matt Caswell (OpenSSL) with a few small tweaks.
ok jsing
|
|
|
|
| |
ok jsing kn
|
|
|
|
| |
ok jsing kn
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Bad API design makes it possible to set an EC_KEY public key to
a point not on the curve. As a consequence, it was possible to
have bogus ECDSA signatures validated. In practice, all software
uses either EC_POINT_oct2point*() to unmarshal public keys or
issues a call to EC_KEY_check_key() after setting it. This way,
a point on curve check is performed and the problem is mitigated.
In OpenSSL commit 1e2012b7ff4a5f12273446b281775faa5c8a1858, Emilia
Kasper moved the point-on-curve check from EC_POINT_oct2point to
EC_POINT_set_affine_coordinates_*, which results in more checking.
In addition to this commit, we also check in the currently unused
codepath of a user set callback for setting compressed coordinates,
just in case this will be used at some point in the future.
The documentation of EC_KEY_check_key() is very vague on what it
checks and when checks are needed. It could certainly be improved
a lot. It's also strange that EC_KEY_set_key() performs no checks,
while EC_KEY_set_public_key_affine_coordinates() implicitly calls
EC_KEY_check_key().
It's a mess.
Issue found and reported by Guido Vranken who also tested an earlier
version of this fix.
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The method unification broke an API promise of SSL_is_server(). According
to the documentation, calling SSL_is_server() on SSL objects constructed
from generic and server methods would result in 1 even before any call to
SSL_set_accept_state(). This means the information needs to be available
when SSL_new() is called, so must come from the method itself.
Prior to the method unification, s->server would be set to 0 or 1 in
SSL_new() depending on whether the accept method was undefined or not.
Instead, introduce a flag to the internal structs to distinguish client
methods from server and generic methods and copy that flag to s->server in
SSL_new().
This problem was reported to otto due to breakage of DoH in net/dnsdist.
The reason for this is that www/h2o relies on SSL_is_server() to decide
whether to call SSL_accept() or SSL_connect(). Thus, the h2o server would
end up responding to a ClientHello with another ClientHello, which results
in a handshake failure. The bandaid applied to www/h2o can be removed once
this fix has made it into snaps. No other breakage is known.
This commit brings back only about half of the duplication removed in the
method unification, so is preferable to a full revert.
ok jsing
|
|
|
|
|
|
|
|
|
| |
This happens if name->der_len == 0. Since we already have a length
check, we can malloc and memcpy inside the conditional. This also
makes the code easier to read.
agreement from millert
ok jsing
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
x509_verify_chain_new() allocates a few members of a certificate chain:
an empty stack of certificates, a list of errors encountered while
validating the chain, and a list of name constraints. The function to
copy a chain would allocate a new chain using x509_verify_chain_new()
and then clobber its members by copies of the old chain. Fix this by
replacing x509_verify_chain_new() with calloc().
Found by review while investigating the report by Hanno Zysik who
found the same leak using valgrind. This is a cleaner version of
my initial fix from jsing.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The legacy validator would only call x509_vfy_check_policy() once at
the very end after cobbling together a chain. Therefore it didn't
matter that X509_policy_check() always allocates a new tree on top of
the one that might have been passed in. This is in stark contrast to
other, similar APIs in this code base. The new validator calls this
function several times over while building its chains. This adds up
to a sizable leak in the new validator.
Reported with a reproducer by Hanno Zysik on github, who also bisected
this to the commit enabling the new validator.
Narrowed down to x509_vfy_check_policy() by jsing.
We simultaenously came up with a functionally identical fix.
ok jsing
|
| |
|
|
|
|
|
|
| |
a few lines after.
stylistic nit from jsing
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
| |
This implements the key material exporter for TLSv1.3, as defined in
RFC8446 section 7.5.
Issue reported by nmathewson on github.
ok inoguchi@ tb@
|
|
|
|
| |
ok beck@ tb@
|
|
|
|
|
|
|
|
| |
This was inadvertently removed in r1.19.
Spotted by tb@
ok beck@ tb@
|
|
|
|
|
|
|
|
|
| |
in order to be compatible with the openssl error craziness in the legacy
verifier case.
This will fix a regress problem noticed by znc
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
With the old verifier, the verify callback can always return 1 instructing
the verifier to simply continue regardless of a certificate verification
failure (e.g. the certificate is expired or revoked). This would result
in a chain being built, however the first error encountered would be
persisted, which allows the caller to build the chain, have the
verification process succeed, yet upon inspecting the error code note
that the chain is not valid for some reason.
Mimic this behaviour by keeping track of certificate errors while building
chains - when we finish verification, find the certificate error closest
to the leaf certificate and expose that via the X509_STORE_CTX. There are
various corner cases that we also have to handle, like the fact that we
keep an certificate error until we find the issuer, at which point we have
to clear it.
Issue reported by Ilya Shipitcin due to failing haproxy regression tests.
With much discussion and input from beck@ and tb@!
ok beck@ tb@
|
|
|
|
|
|
|
|
|
| |
Apparently OpenLDAP relies on this craziness to provide intermediates,
rather than specifying the chain directly like a normal TLS server would.
Issue noted by sthen@ and Bernard Spil, who both also tested this diff.
ok tb@
|
|
|
|
|
|
|
| |
This allows us to remove a check and will make future changes simpler. Use
suitable names for tls1_generate_key_block() arguments while here.
ok inoguchi@ tb@
|
|
|
|
|
| |
wincrypt is deprecated and no longer works with newer Windows environments,
such as in Windows Store apps.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The TLSv1.3 code that drives a BIO currently checks BIO_should_read()
after BIO_write() and BIO_should_write() after BIO_read(), which was
modelled on SSL_get_error(). However, there are certain cases where
this can confuse the caller - primarily where the same BIO is being
used for both read and write and the caller is manipulating the retry
flags. SSL_get_error() tends avoids this issue by relying on another
layer of state tracking.
Unfortunately haproxy hits this situation - it has its own BIO_METHOD,
the same BIO is used for both read and write and it manipulates the
retry flags - resulting in it stalling.
Issued noted by Thorsten Lockert <tholo@tzecmaun.org>
ok beck@ tb@
|
|
|
|
|
|
|
|
|
|
|
| |
If we fail to find a parent certificate from either the supplied roots or
intermediates and we have a X509_STORE_CTX, call its get_issuer() callback
to see if it can supply a suitable certificate. This makes things like
certificates by directory (aka by_dir) work correctly.
Issue noted by Uwe Werler <uwe@werler.is>
ok beck@ tb@
|
| |
|
|
|
|
| |
Suggested by and discussed with beck
|
|
|
|
|
|
|
| |
context. This is what is returned in SSL_get_verify_result().
Spotted and initial diff from jeremy; discussed with jsing
ok beck
|
|
|
|
|
|
| |
ctx->xsc->error. Will be needed in an upcoming diff.
from beck
|
|
|
|
|
|
|
| |
In x509.h r1.70 (2018/08/24) I turned some macros into actual functions
to follow what OpenSSL is doing since 1.1.0. The documentation still
claims that they are implemented as macros. Update a doc sync commit
hash while there.
|
|
|
|
|
|
|
|
|
|
| |
When retransmitting a flight that includes a CCS, the record protection
from the previous epoch has to be used to send the messages up to and
including the CCS, with messages after the CCS using record protection
from the current epoch. The code that restores the record protection state
failed to work correctly with the new TLSv1.2 record layer.
ok tb@
|
| |
|
|
|
|
|
|
| |
Garbage collect the now unused SSL_IS_DTLS macro.
ok tb@
|
|
|
|
|
|
|
| |
For now this is #ifdef LIBRESSL_INTERNAL and will be exposed during the
next library bump.
ok tb@
|
|
|
|
|
|
|
|
| |
Rather than inferring DTLS from the method version, add a field that marks
a method as specifically being DTLS. Have SSL_IS_DTLS condition on this
rather than on version.
ok tb@
|
|
|
|
| |
ok guenther tb millert
|
|
|
|
| |
suggested by tb@
|
|
|
|
|
|
| |
deprecated methods to a separate table. Simplify and shorten the
surrounding verbiage.
Joint work with tb@.
|
|
|
|
|
|
|
| |
and *_client_method(3). Adjust the documentation.
While here, delete most of the verbiage regarding the deprecated
functions SSLv23_*(3) and add the missing entry to RETURN VALUES.
OK tb@
|
|
|
|
|
|
|
| |
with #defines for the per-version initializers instead of extern
globals. Add SSL_USE_SHA256_PRF() to complete the abstraction.
ok tb@ jsing@
|
|
|
|
|
|
|
|
| |
This condition previously existed for DTLS BAD_VER, which has long been
removed. Furthermore, conditioning on DTLS1_VERSION means this is broken
for any newer DTLS version. While here roll up two assertions into one.
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When transitioning from the TLSv1.3 stack to the legacy stack, grow
init_buf before stashing the handshake message. The TLSv1.3 stack has
already received the handshake message (potentially from multiple TLS
records) and validated its size, however the default allocation is only
for a single plaintext record, which can result in the handshake message
failing to fit in certain cases.
Issue noted by tb@ via tlsfuzzer.
ok tb@
|
|
|
|
|
|
|
|
| |
There is no reason (and there never was any) for profile_name to be
non-const, it was always just passed to strncmp(). Changing this
allows removing an ugly instance of casting away const.
ok guenther jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Historically, OpenSSL has had client and server specific methods - the only
difference between these is that the .ssl_connect or .ssl_accept function
pointer is set to ssl_undefined_function, with the intention of reducing
code size for a statically linked binary that was only a client or server.
These days the difference is minimal or non-existant in many cases and
we can reduce the amount of code and complexity by having single method.
Internally remove all of the client and server specific methods,
simplifying code in the process. The external client/server specific API
remain, however these now return the same thing as TLS_method() does.
ok tb@
|