| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
| |
ok jsing
|
|
|
|
| |
ok jsing
|
|
|
|
| |
ok jsing
|
|
|
|
| |
ok jsing
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
|
|
| |
These are two functions that will help streamlining various functions
in the TLSv1.3 code that do not need to know about the interna of this
struct.
input/ok jsing
|
| |
|
|
|
|
|
|
|
|
|
| |
In tls13_{client,server}_finished_recv() we use verify_data_len, which
makes more sense than hmac_len. Use the same name in
tls13_{client,server}_finished_send(), keeping things consistent between
functions.
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The new verifier builds all chains, starting with the shortest possible
path. It also does not currently return partial chains. Both of these
things conflict with auto chain, where we want to build the longest
possible chain (to include all intermediates, and probably the root
unnecessarily), as well as using an incomplete chain when a trusted chain
is not known.
Depending on software configuration, we can end up building a chain
consisting only of a leaf certificate, rather than a longer chain. This
results in auto chain not including intermediates, which is undesireable.
For now, switch auto chain building to use the legacy verifier.
This should resolve the issues encountered by ajacoutot@ with sendmail.
ok tb@
|
|
|
|
|
|
| |
Yet another mostly meaningless error value...
Noted by and ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When a certificate (namely a root) is specified as both a trusted and
untrusted certificate, the new verifier will find multiple chains - the
first being back to the trusted root certificate and a second via the root
that is untrusted, followed by the trusted root certificate. This situation
can be triggered by a server that (unnecessarily) includes the root
certificate in its certificate list.
While this validates correctly (using the first chain), it means that we
encounter a failure while building the second chain due to the root
certificate already being in the chain. When this occurs we call the verify
callback indicating a bad certificate. Some sensitive software (including
bacula and icinga), treat this single bad chain callback as terminal, even
though we successfully verify the certificate.
Avoid this problem by simply dumping the chain if we encounter a situation
where the certificate is already in the chain and also a trusted root -
we'll have already picked up the trusted root as a shorter path.
Issue with icinga2 initially reported by Theodore Wynnychenko.
Fix tested by sthen@ for both bacula and icinga2.
ok tb@
|
|
|
|
|
|
| |
fix in libcrypto/asn1/a_time_tm.c r1.16.
Suggested by jsing
|
| |
|
| |
|
|
|
|
|
|
| |
order of the struct members for reviewability.
ok jsing
|
|
|
|
|
|
|
| |
* Do not abuse .Bl -tag for lists without bodies, use .Bl -item instead.
* In tagged lists, put bodies into bodies, not into heads.
* Add a few missing macros.
* Drop some useless quoting.
|
|
|
|
|
|
|
| |
Follow the previous commit and complete the manual page for consistency;
better readable and tags for free.
OK tb
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
httpd(8)'s incorrect tls_close() after closing the underlying socket
led to a leak: tls_close()'s attempt to send out the close_notify won't
work very well over a closed pipe. This resulted in alert_data still
hanging off the TLSv1.3 context's record layer struct. The tls_free()
call should have cleaned this up but failed to do so.
The record layer's phh_data potentially has the same issue, so free it
as well. This diff makes -current httpd(8) run in constant memory over
hundreds of thousands TLS connections with a static site.
ok inoguchi jsing
|
|
|
|
| |
From miod@, OK tb@
|
|
|
|
|
|
|
|
|
|
| |
Manuals like httpd.conf(5) refer to this for valid protocol strings, but
elements inlined into sentences are hard find to spot.
Use a list as already done elsewhere in this manual.
OK jmc on earlier version
Feeback OK tb
|
|
|
|
| |
ok inoguchi jmc kn
|
|
|
|
|
| |
an OOR2 operator. Also includes a regress test for the issue.
From FreeBSD via miod@
|
| |
|
|
|
|
| |
Ensure that it works with obj directory and link regress to build.
|
|
|
|
|
| |
This makes CFLAGS pick up -O2, which shaves a few seconds runtime
off these very slow tests.
|
|
|
|
|
|
| |
Add a stub for pthread_mutex_destroy() for installers.
ok tb@
|
|
|
|
| |
noted by deraadt@
|
|
|
|
| |
ok inoguchi@
|
| |
|
| |
|
|
|
|
| |
pointed out by jsing
|
|
|
|
|
|
|
|
|
|
|
| |
When parsing an UTCTime into a struct tm that wasn't cleared by the caller,
the years would be added to the already present value, which could give an
incorrect result. This is an issue in ASN1_UTCTIME_cmp_time_t(), which is
practically unused. Fix this by always zeroing the passed struct tm.
Issue reported by Olivier Taïbi, thanks!
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
It is a historical artifact that cert_verify_md[], finish_md[] and
peer_finish_md[] are twice as large as they need to be. This is
confusing, especially for finish_md[] and peer_finish_md[] which are
copied to to previous_client_finished[] and previous_server_finished[]
which are only half as large. It is easy to check that they will never
get more than EVP_MAX_MD_SIZE data written to them.
In 1998, EVP_MAX_MD_SIZE was 20 bytes long (for SHA-1). This got bumped to
16+20 for the SSLv3-specific md5+sha1. Apparently under the impression
that EVP_MAX_MD_SIZE was still 20 bytes, someone else doubled finish_md[]'s
size to EVP_MAX_MD_SIZE*2 and added /* actually only needs to be 16+20 */.
A bit later finish_md[] was split up, and still a bit later the comment was
amended for TLSv1. Shortly thereafter SHA-512 required a bump of
EVP_MAX_MD_SIZE to 64 by a third person and we have been carrying 192 bytes
of untouched memory in each of our SSLs ever since.
ok inoguchi jsing (jsing had the same diff)
|
|
|
|
|
|
|
| |
ERR_peek_error() returns unsigned long.
Reported by github issue by @rozhuk-im.
ok bcook@ jsing@
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
| |
|
| |
|
|
|
|
|
| |
behavior of SSL_is_server(). This would have caught the regression
introduced in the method unification.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|