| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
|
|
|
| |
order of the struct members for reviewability.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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)
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
| |
ok jsing kn
|
|
|
|
| |
ok jsing kn
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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 implements the key material exporter for TLSv1.3, as defined in
RFC8446 section 7.5.
Issue reported by nmathewson on github.
ok inoguchi@ 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@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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@
|
|
|
|
|
|
|
|
|
|
| |
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@
|
|
|
|
| |
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@
|
|
|
|
| |
ok tb@ jsing@
|
|
|
|
|
|
| |
.data.rel.ro and .rodata respectively.
ok tb@ jsing@
|
|
|
|
|
| |
documenting that SSL_set_bio(3) cannot fail. A similar commit was
made by schwarze a while ago for a few functions in libcrypto.
|
| |
|
| |
|
|
|
|
| |
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When switching to the legacy TLS stack we previously copied any remaining
handshake messages into the receive buffer, but do not include any TLS
record header (largely due to the fact that we've already processed part
of the TLS record that we actually received - that part is placed into the
init_buf). This worked fine with the old record layer implementation,
however the new record layer expects to find the TLS record header.
This means that if we switch from the new stack to the legacy stack (i.e.
the remote side does not support TLSv1.3) and there is more than one
handshake message in the TLS plaintext record (which Microsoft's TLS
stack is known to do), we now read a TLS record of zero bytes instead of
getting the correct length.
Fix this by generating a pseudo-TLS record header when switching from the
new TLS stack to the legacy stack.
Found the hard way by guenther@.
Thanks to tb@ for coming up with a reproducible test case and doing much
of the debugging.
ok inoguchi@ tb@
|
|
|
|
|
|
|
| |
The d1_{clnt,srvr}.c contain a single function each - merge these into the
ssl_{clnt,srvr}.c, renaming them with an ssl3_ prefix for consistency.
ok beck@ tb@
|
|
|
|
|
|
|
|
| |
DTLSv1 is TLSv1.1 over datagrams - there is no need for a separate
SSL3_ENC_METHOD struct, just use TLSv1_1_enc_data and remove
DTLSv1_enc_data entirely.
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is the next step in replacing the TLSv1.2 record layer.
The existing record handling code does decryption and processing in
place, which is not ideal for various reasons, however it is retained
for now as other code depends on this behaviour. Additionally, CBC
requires special handling to avoid timing oracles - for now the
existing timing safe code is largely retained.
ok beck@ inoguchi@ tb@
|
|
|
|
|
|
|
| |
This avoids naming confusion with an upcoming TLSv1.2 record layer change
and is more descriptive of this function.
Requested by tb@
|
|
|
|
|
|
| |
This removes the need for extra variables and casts.
ok inoguchi@ tb@
|
|
|
|
|
|
|
| |
This improves readability - while here also add a missing return value
check (although it cannot currently fail).
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
| |
Currently dtls1_drain_fragments() has a incomplete handrolled version of
dtls1_hm_fragment_free(), which has the potential to leak memory. Replace
the handrolled free with a call to dtls1_hm_fragment_free().
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
| |
Allocate into the appropriate structures and call dtls1_free() on failure,
rather than allocating into local variables and then remembering to free
various things on failure.
ok tb@
|
|
|
|
|
|
|
|
|
|
| |
Rather than using local variables and having to remember which things need
to be freed upon a failure at a certain point, simply allocate into the
hm_fragment struct and call dtls1_hm_fragment_free() on failure.
Also use calloc() to ensure memory is appropriately zeroed/initialised.
ok tb@
|
|
|
|
|
|
|
|
|
|
| |
An upcoming cleanup diff by jsing needs dtls1_clear_queues() to be
able to handle NULL pqueues. While one can easily add a NULL check
to pqueue_pop(), this does not really fit in with the rest of the
code. There are two kinds of while loops in dtls1_clear_queues that
drain pqueues, so add two helper functions with a NULL check each.
ok jsing
|
|
|
|
| |
ok beck@ inoguchi@ tb@
|
|
|
|
|
|
|
|
|
| |
Provide a ssl3_release_buffer() function that correctly frees a buffer
and call it from the appropriate locations. While here also change
ssl3_release_{read,write}_buffer() to void since they cannot fail and
no callers check the return value currently.
ok beck@ inoguchi@ tb@
|
|
|
|
|
| |
because tb@ decided to not enable it before the release.
OK tb@
|
|
|
|
| |
issue noticed by and patch OK by jsing@
|