For a long time SSL_SESSION has had both a cipher ID and a pointer to
an SSL_CIPHER (and not both are guaranteed to be populated). There is also
a pointer to an SSL_CIPHER in the SSL_HANDSHAKE that denotes the cipher
being used for this connection. Some code has been using the cipher from
SSL_SESSION and some code has been using the cipher from SSL_HANDSHAKE.
Remove cipher from SSL_SESSION and use the version in SSL_HANDSHAKE
everywhere. If resuming from a session then we need to use the SSL_SESSION
cipher ID to set the SSL_HANDSHAKE cipher. And we still need to ensure that
we update the cipher ID in the SSL_SESSION whenever the SSL_HANDSHAKE
cipher changes (this only occurs in a few places).
ok tb@
Instead of attempting to allocate a few times and only then check all the
returned pointers for NULL, allocate and check one after the othre. This
is easier on the eyes and what we usually do.
Prompted by a report by Ilya Shipitsin
ok beck
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
This converts the legacy TLS stack to tls_content - records are now
opened into a tls_content structure, rather than being written back into
the same buffer that the sealed record was read into.
This will allow for further clean up of the legacy record layer.
ok tb@
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@
Now that {dtls1,ssl3}_read_bytes() have been refactored, do a clean up
pass - this cleans up various parts of the code and reduces differences
between these two functions.
ok = 1; *(&(ok)) tb@
ok inoguchi@
The code that handles each record type already has appropriate length
checks. Furthermore, the handling of application data here is likely
incorrect and bypasses the normal state checks at the end of this function.
ok inoguchi@ tb@
Rewrite the code that handles unexpected handshake messages in the legacy
DTLS stack. Parse the DTLS message header up front, then process it based
on the message type. Overall the code should be more strict and we should
reject various invalid messages that would have previously been accepted.
ok inoguchi@ tb@
The TLS record layer has to be able to handle unexpected handshake messages
that result when it has been asked to read application data. The way that
this is currently done in the legacy stack is a layering violation - the
record layer knows about DTLS/TLS handshake messages, parsing them and then
deciding what action to take. This is further complicated by the need to
handle handshake message fragments.
For now, factor this code out with minimal changes - since it is a layering
violation we have to retain separate code for DTLS and TLS.
ok beck@ inoguchi@ tb@
Factor out the code that handles the processing of a change cipher spec
message that has been read in the legacy stack, deduplicating code in the
DTLS stack.
ok inoguchi@ tb@
Pull out the code that processes incoming alerts - a chunk of the
complexity is due to the fact that in TLSv1.2 and earlier, alerts can be
fragmented across multiple records or multiple alerts can be delivered
in a single record.
In DTLS there is no way that we can reassemble fragmented alerts (although
the RFC is silent on this), however we could have multiple alerts in the
same record. This change means that we will handle this situation more
appropriately and if we encounter a fragmented alert we will now treat this
as a decode error (instead of silently ignoring it).
ok beck@ tb@
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@
This is effectively the same record processing limit that was previously
added to the legacy TLS stack - without this a single session can be made
to spin on a stream of alerts or other similar records.
ok beck@ tb@
Rather than manually checking multiple bytes, actually parse the DTLS
handshake message header, then check the values against what we parsed.
ok inoguchi@ tb@
Rather than pulling out the epoch and then six bytes of sequence number,
pull out SSL3_SEQUENCE_SIZE for the sequence number, then pull the epoch
off the start of the sequence number.
ok inoguchi@ tb@
In normal TLS, it is possible for record fragments to be sent that contain
one byte of alert or handshake message payload. In this case we have to
read and collate multiple message fragments before we can decide what to
do with the record.
However, in the case of DTLS, one record is effectively one packet and
while it is possible to send handshake messages across multiple
records/packets, the minimum payload is the DTLS handshake message header
(plus one byte of data if the handshake message has a payload) - without
this, there is insufficient information available to be able to reassemble
the handshake message. Likewise, splitting an alert across multiple DTLS
records simply does not work, as we have no way of knowing if we're
collating the same alert or two different alerts that we lost half of each
from (unfortunately, these details are not really specified in the DTLS
RFC).
This means that for DTLS we can expect to receive a full alert message
(a whole two bytes) or a handshake record with at least the handshake
message header (12 bytes). If we receive messages with less than these
lengths we discard them and carry on (which is what the DTLS code already
does).
Remove all of the pointless fragment handling code from DTLS, while also
fixing an issue where one case used rr->data instead of the handshake
fragment.
ok inoguchi@ tb@
The info and msg callbacks result in duplication - both for code that
refers to the function pointers and for the call sites. Avoid this by
providing typedefs for the function pointers and pulling the calling
sequences into their own functions.
ok inoguchi@ tb@
The code for dtls1_dispatch_alert() and ssl3_dispatch_alert() is largely
identical - with a bit of reshuffling we can use ssl3_dispatch_alert() for
both protocols and remove the ssl_dispatch_alert function pointer.
ok inoguchi@ tb@
When DTLS handshake records are received from the next epoch, we will
potentially queue them on the unprocessed_rcds queue - this is usually
a Finished message that has been received without the ChangeCipherSuite
(CCS) message (which may have been dropped or reordered).
After the epoch increments (due to the CCS being received), the current
code processes all records on the unprocessed queue and immediate queues
them on the processed queue, which dtls1_get_record() then pulls from.
This form of processing only adds more complexity and another queue.
Instead, once the epoch increments, pull a single record from the
unprocessed queue and process it, allowing the contents to be consumed
by the caller. We repeat this process until the unprocessed queue is
empty, at which point we go back to consuming messages from the wire.
ok inoguchi@ tb@
Per RFC 6347 section 4.1.2.1, DTLS should silently discard invalid records,
including those that have a bad MAC. When converting to the new record
layer, we inadvertantly switched to standard TLS behaviour, where an
invalid record is fatal. This restores the previous behaviour.
Issue noted by inoguchi@
ok inoguchi@
All this code does is read one byte from memory with an unknown length,
potentially being a one byte overread... and then nothing is actually done
with the value.
ok tb@
Due to a type bug that has been present in DTLS since the code was first
committed in 2005, dtls1_get_bitmap() fails to handle next epoch correctly
when the epoch is currently 0xffff (and wraps to zero).
For various reasons unknown, the epoch field in the SSL3_RECORD_INTERNAL
(formerly SSL3_RECORD) was added as unsigned long (even though the value
is an unsigned 16 bit value on the wire, hence cannot exceed 0xffff),
however was added to other code as unsigned short.
Due to integer promotion, the r_epoch value is incremented by one to
become 0x10000, before being cast to an unsigned long and compared to
the value pulled from the DTLS record header (which is zero). Strangely
0x10000 != 0, meaning that we drop the DTLS record, instead of queueing
it for the next epoch.
Fix this issue by using more appropriate types and pulling up the
calculation of the next epoch value for improved readability.
ok inoguchi@ tb@
The original DTLS code had some strange alert handling code (basically one
type of alert included extra data) - a few years later this was "fixed",
however the rest of the code was left as is.
This means that rather than sending the alert data from send_alert
(like ssl3_dispatch_alert() does), we have a local buffer on the stack,
which we memset, copy the send_alert bytes into, then send from.
ok inoguchi@ tb@
Now that the DTLS structs are opaque, add a dtls_locl.h header and move
internal-only structs from dtls1.h, along with prototypes from ssl_locl.h.
Only pull this header in where DTLS code actually exists.
ok inoguchi@ tb@
Rather than doing flag gymnastics, split dtls1_reset_seq_numbers() into
separate read and write functions. Move the calls of these functions into
tls1_change_cipher_state() so they directly follow the change of cipher
state in the record layer, which avoids having to duplicate the calls in
the client and server.
ok inoguchi@ tb@
This provides the basic framework for handling change of cipher state in
the new TLSv1.2 record layer, creating new record protection. In the DTLS
case we retain the previous write record protection and can switch back to
it when retransmitting. This will allow the record layer to start owning
sequence numbers and encryption/decryption state.
ok inoguchi@ tb@
Call these functions from code that needs to know if we've changed cipher
state and enabled record protection, rather than inconsistently checking
various pointers from other places in the code base. This also fixes a
minor bug where the wrong pointers are checked if we're operating with
AEAD.
ok inoguchi@ tb@
Pass the explicit DTLS read sequence number to dtls1_record_bitmap_update()
and dtls1_record_replay_check(), rather than expecting it to be in
S3I(s)->read_sequence. Also, store the read sequence number into
S3I(s)->rrec.seq_num when we're processing the record header, rather than
having dtls1_record_replay_check() be responsible for copying it.
ok inoguchi@ tb@
Handle protocol specific (DTLS vs TLS) sequence number differences in the
open/seal record functions and propagate the sequence number through to
the called functions. This means that DTLS specific knowledge is limited
to two functions and also avoids building sequence numbers multiple times
over. As a result, the DTLS explicit sequence number is now extracted from
the record header and passed through for processing, which makes the read
epoch handling redundant.
ok inoguchi@ 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@
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@
This takes the same design/approach used in TLSv1.3 and provides an
opaque struct that is self contained and cannot reach back into other
layers. For now this just implements/replaces the writing of records
for DTLSv1/TLSv1.0/TLSv1.1/TLSv1.2. In doing so we stop copying the
plaintext into the same buffer that is used to transmit to the wire.
ok inoguchi@ tb@
