summaryrefslogtreecommitdiff
path: root/src/lib/libssl/tls13_legacy.c (follow)
Commit message (Collapse)AuthorAgeFilesLines
* Restore SSL_shutdown() two step sequence.jsing2024-01-301-1/+3
| | | | | | | | | | | Change SSL_shutdown() such that it will return 0 after sending a close-notify, before potentially returning 1 (indicating that a close-notify has been sent and received) on a subsequent call. Some software depends on this behaviour, even though there are cases where the first call could immediately return 1 (for example, when the peer has already sent a close-notify prior to SSL_shutdown() being called). ok tb@
* Rework tls13_legacy_shutdown() to match the legacy stack behaviour.jsing2024-01-271-18/+19
| | | | | | | | Respect the ssl->shutdown flags rather than what has actually happened, return -1 for all EOF errors and completely ignore the return value when attempting to read a close-notify from the wire. ok tb@
* Make tls13_legacy_return_code() static.jsing2024-01-271-2/+2
|
* Switch to legacy method late in tls13_use_legacy_stack()tb2023-11-281-3/+7
| | | | | | | | | | | | | | If memory allocation of s->init_buf fails in ssl3_setup_init_buffer() during downgrade to the legacy stack, the legacy state machine would resume with an incorrectly set up SSL, resulting in a NULL dereference. The fix is to switch to the legacy method only after the SSL is fully set up. There is a second part to this fix, which will be committed once we manage to agree on the color of the bikeshed. Detailed analysis and patch from Masaru Masuda, many thanks! https://github.com/libressl/openbsd/issues/146 ok jsing
* Make internal header file names consistenttb2022-11-261-2/+2
| | | | | | | | | | | | | | | | Libcrypto currently has a mess of *_lcl.h, *_locl.h, and *_local.h names used for internal headers. Move all these headers we inherited from OpenSSL to *_local.h, reserving the name *_internal.h for our own code. Similarly, move dtls_locl.h and ssl_locl.h to dtls_local and ssl_local.h. constant_time_locl.h is moved to constant_time.h since it's special. Adjust all .c files in libcrypto, libssl and regress. The diff is mechanical with the exception of tls13_quic.c, where #include <ssl_locl.h> was fixed manually. discussed with jsing, no objection bcook
* Get rid of SSL_CTX_INTERNAL and SSL_INTERNAL.jsing2022-10-021-36/+36
| | | | | | | | These are no longer necessary due to SSL_CTX and SSL now being fully opaque. Merge SSL_CTX_INTERNAL back into SSL_CTX and SSL_INTERNAL back into SSL. Prompted by tb@
* Pass SSL pointer to tls13_ctx_new().jsing2022-07-171-11/+3
| | | | | | | | struct tls13_ctx already knows about SSL's and this way tls13_ctx_new() can set up various pointers, rather than duplicating this in tls13_legacy_accept() and tls13_legacy_connect(). ok tb@
* Handle zero byte reads/writes that trigger handshakes in the TLSv1.3 stack.jsing2022-02-061-1/+5
| | | | | | | | | | | | | | | With the legaacy stack, it is possible to do a zero byte SSL_read() or SSL_write() that triggers the handshake, but then returns zero without SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE being flagged. This currently works in the TLSv1.3 stack by returning TLS_IO_WANT_POLLIN or TLS_IO_WANT_POLLOUT, which is then hidden by SSL_get_error(). However, due to upcoming changes to SSL_get_error() this will no longer be the case. In order to maintain the existing legacy behaviour, explicitly handle zero byte reads and writes in the TLSv1.3 stack, following completion of a handshake. ok inoguchi@ tb@
* Bye bye S3I.jsing2022-02-051-17/+17
| | | | | | | | S3I has served us well, however now that libssl is fully opaque it is time to say goodbye. Aside from removing the calloc/free/memset, the rest is mechanical sed. ok inoguchi@ tb@
* Fix another return 0 bug in SSL_shutdown()tb2022-01-251-6/+8
| | | | | | | | | | | If tls13_recod_layer_send_pending() returns TLS13_IO_EOF, we will bubble this up to the caller via tls13_legacy_return_code(), which translates TLS13_IO_EOF to 0. This can happen if we have pending post handshake-handshake data and the peer closes the pipe. Presumably tls13_legacy_shutdown() should be rewritten yet again. ok jsing
* Avoid an infinite loop in SSL_shutdown()tb2022-01-251-2/+4
| | | | | | | | | | | | | | | | | | | | | If the peer closed the write side of the connection and we have not yet received the close_notify, SSL_shutdown() makes an extra read to try and read the peer's close_notify from the pipe. In that situation, we receive EOF. The legacy stack will return -1 while the TLSv1.3 stack will end up returning 0. Since the documentation is not super explicit about what should be done if SSL_shutdown() returns 0, some applications will enter an infinite loop. The code and documentation indicate that SSL_shutdown() should only be called once more if it returned 0. Newer versions of the OpenSSL documentation explicitly say that one should call SSL_read() if SSL_shutdown() returns 0 in order to retrieve the close_notify. Doing this would also have avoided this infinite loop. Reported by Carsten Arzig and bluhm with a test case extracted from the syslogd tests using IO::Socket::SSL, which has such an infinite loop. ok bluhm jsing
* unifdef TLS13_USE_LEGACY_CLIENT_AUTHtb2021-12-161-9/+1
| | | | | | | | | | | | Before the TLSv1.3 stack grew client certificate support, it fell back to the legacy stack. Proper client certificate support was added in a2k20 with a TLS13_USE_LEGACY_CLIENT_AUTH knob to provide an easy fallback in case the new code should have a problem. This was never needed. As ifdefed code is wont to do, this bitrotted a few months later when the client and server methods were merged. discussed with jsing
* Provide a way to determine our maximum legacy version.jsing2021-10-231-3/+3
| | | | | | | | | | | | | | With the introduction of TLSv1.3, we need the ability to determine our maximum legacy version and to track our peer's maximum legacy version. This is needed for both the TLS record layer when using TLSv1.3, plus it is needed for RSA key exhange in TLS prior to TLSv1.3, where the maximum legacy version is incorporated in the pre-master secret to avoid downgrade attacks. This unbreaks RSA KEX for the TLS client when the non-version specific method is used with TLSv1.0 or TLSv1.1 (clearly no one does this). ok tb@
* Implement flushing for TLSv1.3 handshakes.jsing2021-09-161-1/+25
| | | | | | | | | | | | | | | When we finish sending a flight of records, flush the record layer output. This effectively means calling BIO_flush() on the wbio. Some things (such as apache2) have custom BIOs that perform buffering and do not actually send on BIO_write(). Without BIO_flush() the server thinks it has sent data and starts receiving records, however the client never sends records since it never received those that the server should have sent. Joint work with tb@ ok tb@
* Call the info cb on connect/accept exit in TLSv1.3tb2021-09-141-3/+13
| | | | | | | | | The p5-Net-SSLeay test expects the info callback to be called on connect exit. This is the behavior in the legacy stack but wasn't implemented in the TLSv1.3 stack. With this commit, p5-Net-SSLeay tests are happy again after the bump. ok bluhm inoguchi jsing
* Factor out the TLSv1.3 code that handles content from TLS records.jsing2021-09-041-2/+2
| | | | | | | | | | | | Currently, the plaintext content from opened TLS records is handled via the rbuf code in the TLSv1.3 record layer. Factor this out and provide a separate struct tls_content, which knows how to track and manipulate the content. This makes the TLSv1.3 code cleaner, however it will also soon also be used to untangle parts of the legacy record layer. ok beck@ tb@
* Set message_size correctly when switching to the legacy stack.jsing2021-09-031-2/+2
| | | | | | | | The message_size variable is not actually the handshake message size, rather the number of bytes contained within the handshake message, hence we have to subtract the length of the handshake message header. ok beck@
* Ignore warning alert returns from servername callback in TLSv1.3tb2021-08-301-3/+7
| | | | | | | | | | | | | | | | If a servername callback returns SSL_TLSEXT_ERR_ALERT_WARNING, this results in a fatal error in TLSv1.3 since alert levels are implicit in the alert type and neither close_notify nor user_canceled make sense in this context. OpenSSL chose to ignore this, so we need to follow suit. Found via a broken servername callback in p5-IO-Socket-SSL which returns a Boolean instead of SSL_TLSEXT_ERR_*. This happened to have worked before TLSv1.3 since warning alerts are often ignored. This "fixes" sni.t and sni-verify.t in p5-IO-Socket-SSL. ok beck jsing
* Merge SSL_METHOD_INTERNAL into SSL_METHOD.jsing2021-07-011-8/+8
| | | | | | | Now that SSL_METHOD is opaque and in internal headers, we can remove SSL_METHOD_INTERNAL by merging it back into SSL_METHOD. ok tb@
* The state machine now takes care of setting the legacy state,tb2021-06-281-11/+1
| | | | | | | so it is no longer necessary in to do this by hand in various places of the code interfacing with the legacy stack. ok jsing
* Move reuse_message, message_type, message_size and cert_verify into thejsing2021-04-191-4/+4
| | | | | | TLSv1.2 handshake struct. ok inoguchi@ tb@
* Move the TLSv1.3 handshake struct inside the shared handshake struct.jsing2021-03-211-9/+9
| | | | | | | | | | | | | | | | There are currently three different handshake structs that are in use - the SSL_HANDSHAKE struct (as S3I(s)->hs), the SSL_HANDSHAKE_TLS13 struct (as S3I(s)->hs_tls13 or ctx->hs in the TLSv1.3 code) and the infamous 'tmp' embedded in SSL3_STATE_INTERNAL (as S3I(s)->tmp)). This is the first step towards cleaning up the handshake structs so that shared data is in the SSL_HANDSHAKE struct, with sub-structs for TLSv1.2 and TLSv1.3 specific information. Place SSL_HANDSHAKE_TLS13 inside SSL_HANDSHAKE and change ctx->hs to refer to the SSL_HANDSHAKE struct instead of the SSL_HANDSHAKE_TLS13 struct. This allows the TLSv1.3 code to access the shared handshake data without needing the SSL struct. ok inoguchi@ tb@
* Only use TLS versions internally (rather than both TLS and DTLS versions).jsing2021-02-251-3/+3
| | | | | | | | | | | | | | DTLS protocol version numbers are the 1's compliment of human readable TLS version numbers, which means that newer versions decrease in value and there is no direct mapping between TLS protocol version numbers and DTLS protocol version numbers. Rather than having to deal with this internally, only use TLS versions internally and map between DTLS and TLS protocol versions when necessary. Rename functions and variables to use 'tls_version' when they contain a TLS version (and never a DTLS version). ok tb@
* Rename two local variables ssl to s for consistencytb2021-01-071-4/+4
| | | | | | | | | | | | | In our tls13_* files, we use SSL *s for local variables and SSL *ssl for function arguments. This is odd, but probably the result of finger memory. We intended to use ssl everywhere. Be that as it may, all local variables except in two functions ended up being called s, so align the two outliers with that. As noted by jsing, this is not ideal either as in tls13_legacy_servername_process() the ssl_ctx is now inconsistent. Renaming all s to ssl is a substantial amount of unnecessary churn at a moment that isn't ideal, so we have to live with that. ok bcook inoguchi jsing
* whitespacetb2021-01-041-5/+5
|
* Only check BIO_should_read() on read and BIO_should_write() on write.jsing2020-11-031-5/+1
| | | | | | | | | | | | | | | | | | 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@
* SSL3_ENC_METHOD is just a flag word; merge it into SSL_METHOD_INTERNALguenther2020-10-111-5/+1
| | | | | | | with #defines for the per-version initializers instead of extern globals. Add SSL_USE_SHA256_PRF() to complete the abstraction. ok tb@ jsing@
* Grow init_buf before stashing a handshake message for the legacy stack.jsing2020-10-111-1/+3
| | | | | | | | | | | | | 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@
* Condense and simplify TLS methods.jsing2020-10-111-9/+9
| | | | | | | | | | | | | | | 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@
* fix line wrappingtb2020-10-071-3/+2
|
* Include a TLS record header when switching to the legacy stack.jsing2020-10-071-9/+27
| | | | | | | | | | | | | | | | | | | | | | | | | 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@
* Improve handling of BIO_read()/BIO_write() failures in the TLSv1.3 stack.jsing2020-09-131-1/+9
| | | | | | | | | | | | | | | | When BIO returns a failure, it does not always add an error to the error stack. In the case of the legacy stack, this was generally handled by the guesswork performed by SSL_get_error(). However, in the case of the new stack we push an 'unknown' error onto the stack. Improve this situation by specifically checking errno in the case of a BIO_read() or BIO_write() failure. If the error stack is empty then push a SYSerror() with the errno which is preferable to the 'unknown' error later. Noted by bluhm@ via syslogd regress. ok beck@ tb@
* Have ssl_init_wbio_buffer() push the buffering BIO rather than doing itjsing2020-07-301-5/+2
| | | | | | ourselves. Spotted by tb@ during a previous review.
* Handle SSL_MODE_AUTO_RETRY being changed during a TLSv1.3 session.jsing2020-07-251-1/+4
| | | | | | | | | | | | | | | Both Perl's HTTP::Tiny and IO::Socket::SSL know about SSL_MODE_AUTO_RETRY and try to work around the fact that OpenSSL enabled it by default. However, this can lead to the mode being disabled prior to the TLSv1.3 handshake and then enabled after the handshake has completed. In order to handle this correctly we have to check the mode and inform the record layer prior to every read. Issue reported and test case provided by Nathanael Rensen <nathanael@polymorpheus.com>. ok inoguchi@ tb@
* Dedup the use legacy stack code.jsing2020-07-141-56/+25
| | | | ok inoguchi@ tb@
* Make tls13_legacy_shutdown() match ssl3_shutdown() semantics.jsing2020-06-241-21/+22
| | | | | | | | | | | | | When first called, queue and send a close notify, before returning 0 or 1 to indicate if a close notify has already been received from the peer. If called again only attempt to read a close notify if there is no pending application data and only read one record from the wire. In particular, this avoids continuing to read application data where the peer continues to send application data. Issue noted by naddy@ with ftp(1). ok jca@ tb@
* Wire up the servername callback in the TLSv1.3 server.jsing2020-05-291-1/+27
| | | | | | | | This makes SNI work correctly with TLSv1.3. Found the hard way by danj@, gonzalo@ and others. ok beck@ inoguchi@ tb@
* Add TLS13_ERR_NO_CERTIFICATE.jsing2020-05-161-1/+4
| | | | | | This was missed in previous tls13_server.c commit. ok inoguchi@ tb@
* Provide an alert sent record layer callback.jsing2020-05-111-3/+3
| | | | | | | | Use this to push an error on to the SSL error stack so that we report the details of the alert that we sent, rather than failing with an unknown error. ok tb@
* Honour SSL_VERIFY_FAIL_IF_NO_PEER_CERT in the TLSv1.3 server.jsing2020-05-101-1/+4
| | | | ok beck@
* Provide alert defines for TLSv1.3 and use in the TLSv1.3 code.jsing2020-05-101-2/+2
| | | | | | | | Rather than using a mess of SSL_AL_*, SSL_AD_*, SSL3_AD_* and TLS1_AD_* defines, provide our own TLS13_ALERT_* defines and use those. This also provides the alerts that are new to TLSv1.3. ok beck@
* Move legacy stack interfacing functions into tls13_legacy.c.jsing2020-04-281-1/+190
| | | | | | No functional change. ok inoguchi@ tb@
* Remove the enc function pointers.jsing2020-03-101-2/+1
| | | | | | | The enc function pointers do not serve any purpose these days - remove a layer of indirection and call dtls1_enc()/tls1_enc() directly. ok inoguchi@ tb@
* Move the TLSv1.3 code that interfaces with the legacy APIs/stack into ajsing2020-02-151-0/+327
separate file. Discussed with beck@ and tb@