Commit Graph

962 Commits

Author SHA1 Message Date
Hanno Becker
01315ea03a Account for future epoch records in the total buffering size
Previous commits introduced the field `total_bytes_buffered`
which is supposed to keep track of the cumulative size of
all heap allocated buffers used for the purpose of reassembly
and/or buffering of future messages.

However, the buffering of future epoch records were not reflected
in this field so far. This commit changes this, adding the length
of a future epoch record to `total_bytes_buffered` when it's buffered,
and subtracting it when it's freed.
2018-08-21 17:22:17 +01:00
Hanno Becker
a02b0b462d Add function making space for current message reassembly
This commit adds a static function ssl_buffer_make_space() which
takes a buffer size as an argument and attempts to free as many
future message bufffers as necessary to ensure that the desired
amount of buffering space is available without violating the
total buffering limit set by MBEDTLS_SSL_DTLS_MAX_BUFFERING.
2018-08-21 17:20:27 +01:00
Hanno Becker
e1801399a9 Add another debug message to ssl_buffer_message()
Report if there's not enough buffering space available to reassemble
the next expected incoming message.
2018-08-21 16:51:05 +01:00
Hanno Becker
55e9e2aa6b Free future buffers if next handshake messages can't be reassembled
If the next expected handshake message can't be reassembled because
buffered future messages have already used up too much of the available
space for buffering, free those future message buffers in order to
make space for the reassembly, starting with the handshake message
that's farthest in the future.
2018-08-21 16:24:30 +01:00
Hanno Becker
e605b19631 Add function to free a particular buffering slot
This commit adds a static function ssl_buffering_free_slot()
which allows to free a particular structure used to buffer
and/or reassembly some handshake message.
2018-08-21 16:24:27 +01:00
Hanno Becker
96a6c69d0c Correct bounds check in ssl_buffer_message()
The previous bounds check omitted the DTLS handshake header.
2018-08-21 16:11:09 +01:00
Hanno Becker
e0b150f96b Allow limiting the total amount of heap allocations for buffering
This commit introduces a compile time constant MBEDTLS_SSL_DTLS_MAX_BUFFERING
to mbedtls/config.h which allows the user to control the cumulative size of
all heap buffer allocated for the purpose of reassembling and buffering
handshake messages.

It is put to use by introducing a new field `total_bytes_buffered` to
the buffering substructure of `mbedtls_ssl_handshake_params` that keeps
track of the total size of heap allocated buffers for the purpose of
reassembly and buffering at any time. It is increased whenever a handshake
message is buffered or prepared for reassembly, and decreased when a
buffered or fully reassembled message is copied into the input buffer
and passed to the handshake logic layer.

This commit does not yet include future epoch record buffering into
account; this will be done in a subsequent commit.

Also, it is now conceivable that the reassembly of the next expected
handshake message fails because too much buffering space has already
been used up for future messages. This case currently leads to an
error, but instead, the stack should get rid of buffered messages
to be able to buffer the next one. This will need to be implemented
in one of the next commits.
2018-08-21 16:11:04 +01:00
Hanno Becker
2a97b0e7a3 Introduce function to return size of buffer needed for reassembly
A previous commit introduced the function ssl_prepare_reassembly_buffer()
which took a message length and a boolean flag indicating if a reassembly
bit map was needed, and attempted to heap-allocate a buffer of sufficient
size to hold both the message, its header, and potentially the reassembly
bitmap.

A subsequent commit is going to introduce a limit on the amount of heap
allocations allowed for the purpose of buffering, and this change will
need to know the reassembly buffer size before attempting the allocation.

To this end, this commit changes ssl_prepare_reassembly_buffer() into
ssl_get_reassembly_buffer_size() which solely computes the reassembly
buffer size, and performing the heap allocation manually in
ssl_buffer_message().
2018-08-21 15:47:49 +01:00
Hanno Becker
e678eaa93e Reject invalid CCS records early
This commit moves the length and content check for CCS messages to
the function mbedtls_ssl_handle_message_type() which is called after
a record has been deprotected.

Previously, these checks were performed in the function
mbedtls_ssl_parse_change_cipher_spec(); however, now that
the arrival of out-of-order CCS messages is remembered
as a boolean flag, the check also has to happen when this
flag is set. Moving the length and content check to
mbedtls_ssl_handle_message_type() allows to treat both
checks uniformly.
2018-08-21 14:57:46 +01:00
Hanno Becker
47db877039 ssl_write_record: Consider setting flush variable only if unset 2018-08-21 13:32:13 +01:00
Hanno Becker
1f5a15d86d Check retval of remaining_payload_in_datagram in ssl_write_record() 2018-08-21 13:31:31 +01:00
Hanno Becker
ecff205548 Remove stray bracket if MBEDTLS_ZLIB_SUPPORT is defined 2018-08-21 13:20:00 +01:00
Manuel Pégourié-Gonnard
000281e07d Fix "unused parameter" warning in small configs 2018-08-21 11:20:58 +02:00
Hanno Becker
6aeaa05a95 Merge branch 'iotssl-165-dtls-hs-fragmentation-new' into datagram_packing 2018-08-20 12:53:37 +01:00
Hanno Becker
513815a38d Fix typo in debugging output 2018-08-20 11:56:09 +01:00
Hanno Becker
4cb782d2f6 Return from ssl_load_buffered_record early if no record is buffered 2018-08-20 11:19:05 +01:00
Manuel Pégourié-Gonnard
a1071a58a3 Compute record expansion at the right time
Depends on the current transform, which might change when retransmitting a
flight containing a Finished message, so compute it only after the transform
is swapped.
2018-08-20 11:56:14 +02:00
Manuel Pégourié-Gonnard
065a2a3472 Fix some typos and links in comments and doc 2018-08-20 11:09:26 +02:00
Hanno Becker
3a0aad1c9d Rename update_digest to update_hs_digest 2018-08-20 09:44:02 +01:00
Hanno Becker
4422bbb096 Whitespace fixes 2018-08-20 09:40:19 +01:00
Hanno Becker
e00ae375d3 Omit debug output in ssl_load_buffered_message outside a handshake 2018-08-20 09:39:42 +01:00
Manuel Pégourié-Gonnard
6e7aaca146 Move MTU setting to SSL context, not config
This setting belongs to the individual connection, not to a configuration
shared by many connections. (If a default value is desired, that can be handled
by the application code that calls mbedtls_ssl_set_mtu().)

There are at least two ways in which this matters:
- per-connection settings can be adjusted if MTU estimates become available
  during the lifetime of the connection
- it is at least conceivable that a server might recognize restricted clients
  based on range of IPs and immediately set a lower MTU for them. This is much
easier to do with a per-connection setting than by maintaining multiple
near-duplicated ssl_config objects that differ only by the MTU setting.
2018-08-20 10:37:23 +02:00
Hanno Becker
0d4b376ddf Return through cleanup section in ssl_load_buffered_message() 2018-08-20 09:36:59 +01:00
Hanno Becker
56d5eaa96c Mark SSL ctx unused in ssl_prepare_reassembly_buffer() if !DEBUG
The SSL context is passed to the reassembly preparation function
ssl_prepare_reassembly_buffer() solely for the purpose of allowing
debugging output. This commit marks the context as unused if
debugging is disabled (through !MBEDTLS_DEBUG_C).
2018-08-17 16:52:08 +01:00
Hanno Becker
5f066e7aac Implement future record buffering
This commit implements the buffering of a record from the next epoch.

- The buffering substructure of mbedtls_ssl_handshake_params
  gets another field to hold a raw record (incl. header) from
  a future epoch.
- If ssl_parse_record_header() sees a record from the next epoch,
  it signals that it might be suitable for buffering by returning
  MBEDTLS_ERR_SSL_EARLY_MESSAGE.
- If ssl_get_next_record() finds this error code, it passes control
  to ssl_buffer_future_record() which may or may not decide to buffer
  the record; it does so if
  - a handshake is in progress,
  - the record is a handshake record
  - no record has already been buffered.
  If these conditions are met, the record is backed up in the
  aforementioned buffering substructure.
- If the current datagram is fully processed, ssl_load_buffered_record()
  is called to check if a record has been buffered, and if yes,
  if by now the its epoch is the current one; if yes, it copies
  the record into the (empty! otherwise, ssl_load_buffered_record()
  wouldn't have been called) input buffer.
2018-08-17 16:52:08 +01:00
Hanno Becker
37f9532081 Implement future message buffering and loading
This commit implements future handshake message buffering
and loading by implementing ssl_load_buffered_message()
and ssl_buffer_message().

Whenever a handshake message is received which is
- a future handshake message (i.e., the sequence number
  is larger than the next expected one), or which is
- a proper fragment of the next expected handshake message,
ssl_buffer_message() is called, which does the following:
- Ignore message if its sequence number is too far ahead
  of the next expected sequence number, as controlled by
  the macro constant MBEDTLS_SSL_MAX_BUFFERED_HS.
- Otherwise, check if buffering for the message with the
  respective sequence number has already commenced.
  - If not, allocate space to back up the message within
    the buffering substructure of mbedtls_ssl_handshake_params.
    If the message is a proper fragment, allocate additional
    space for a reassembly bitmap; if it is a full message,
    omit the bitmap. In any case, fall throuh to the next case.
  - If the message has already been buffered, check that
    the header is the same, and add the current fragment
    if the message is not yet complete (this excludes the
    case where a future message has been received in a single
    fragment, hence omitting the bitmap, and is afterwards
    also received as a series of proper fragments; in this
    case, the proper fragments will be ignored).

For loading buffered messages in ssl_load_buffered_message(),
the approach is the following:
- Check the first entry in the buffering window (the window
  is always based at the next expected handshake message).
  If buffering hasn't started or if reassembly is still
  in progress, ignore. If the next expected message has been
  fully received, copy it to the input buffer (which is empty,
  as ssl_load_buffered_message() is only called in this case).
2018-08-17 16:52:08 +01:00
Hanno Becker
6d97ef5a03 Use uniform treatment for future messages and proper HS fragments
This commit returns the error code MBEDTLS_ERR_SSL_EARLY_MESSAGE
for proper handshake fragments, forwarding their treatment to
the buffering function ssl_buffer_message(); currently, though,
this function does not yet buffer or reassembly HS messages, so:

! This commit temporarily disables support for handshake reassembly !
2018-08-17 16:52:08 +01:00
Hanno Becker
44650b7a74 Introduce function checking sanity of the DTLS HS header
This commit introduces helper functions
- ssl_get_hs_frag_len()
- ssl_get_hs_frag_off()
to parse the fragment length resp. fragment offset fields
in the handshake header.

Moreover, building on these helper functions, it adds a
function ssl_check_hs_header() checking the validity of
a DTLS handshake header with respect to the specification,
i.e. the indicated fragment must be a subrange of the total
handshake message, and the total handshake fragment length
(including header) must not exceed the record content size.

These checks were previously performed at a later stage during
ssl_reassemble_dtls_handshake().
2018-08-17 16:52:08 +01:00
Hanno Becker
12555c61d3 Introduce function to parse total handshake length
This commit introduces a static helper function ssl_get_hs_total_len()
parsing the total message length field in the handshake header, and
puts it to use in mbedtls_ssl_prepare_handshake_record().
2018-08-17 16:52:08 +01:00
Hanno Becker
0271f967d6 Introduce buffering structure for handshake messages
This commit introduces, but does not yet put to use, a sub-structure
of mbedtls_ssl_handshake_params::buffering that will be used for the
buffering and/or reassembly of handshake messages with handshake
sequence numbers that are greater or equal to the next expected
sequence number.
2018-08-17 16:52:08 +01:00
Hanno Becker
d7f8ae2508 Introduce sub-structure of ssl_handshake_params for buffering
This commit introduces a sub-structure `buffering` within
mbedtls_ssl_handshake_params that shall contain all data
related to the reassembly and/or buffering of handshake
messages.

Currently, only buffering of CCS messages is implemented,
so the only member of this struct is the previously introduced
`seen_ccs` field.
2018-08-17 16:52:08 +01:00
Hanno Becker
e25e3b7d96 Add function to check is HS msg is a proper fragment
This commit introduces a static function ssl_hs_is_proper_fragment()
to check if the current incoming handshake message is a proper fragment.
It is used within mbedtls_ssl_prepare_handshake_record() to decide whether
handshake reassembly through ssl_reassemble_dtls_handshake() is needed.

The commit changes the behavior of the library in the (unnatural)
situation where proper fragments for a handshake message are followed
by a non-fragmented version of the same message. In this case,
the previous code invoked the handshake reassembly routine
ssl_reassemble_dtls_handshake(), while with this commit, the full
handshake message is directly forwarded to the user, no altering
the handshake reassembly state -- in particular, not freeing it.
As a remedy, freeing of a potential handshake reassembly structure
is now done as part of the handshake update function
mbedtls_ssl_update_handshake_status().
2018-08-17 16:52:08 +01:00
Hanno Becker
d07df86871 Make allocation of reassembly bitmap optional
This commit adds a parameter to ssl_prepare_reassembly_buffer()
allowing to disable the allocation of space for a reassembly bitmap.
This will allow this function to be used for the allocation of buffers
for future handshake messages in case these need no fragmentation.
2018-08-17 16:52:08 +01:00
Hanno Becker
56e205e2c9 Prepare handshake reassembly in separate function
This commit moves the code-path preparing the handshake
reassembly buffer, consisting of header, message content,
and reassembly bitmap, to a separate function
ssl_prepare_reassembly_buffer().
2018-08-17 16:52:08 +01:00
Hanno Becker
9e1ec22c36 Return MBEDTLS_ERR_SSL_EARLY_MESSAGE for future HS messages
This leads future HS messages to traverse the buffering
function ssl_buffer_message(), which however doesn't do
anything at the moment for HS messages. Since the error
code MBEDTLS_ERR_SSL_EARLY_MESSAGE is afterwards remapped
to MBEDTLS_ERR_SSL_CONTINUE_PROCESSING -- which is what
was returned prior to this commit when receiving a future
handshake message -- this commit therefore does not yet
introduce any change in observable behavior.
2018-08-17 16:52:08 +01:00
Hanno Becker
2ed6bcc793 Implement support for remembering CCS messages
This commit implements support for remembering out-of-order
CCS messages. Specifically, a flag is set whenever a CCS message
is read which remains until the end of a flight, and when a
CCS message is expected and a CCS message has been seen in the
current flight, a synthesized CCS record is created.
2018-08-17 16:52:08 +01:00
Hanno Becker
40f50848fa Add frame for loading and storing buffered messages
This commit introduces the frame for saving and loading
buffered messages within message reading function
mbedtls_ssl_read_record().
2018-08-17 16:52:08 +01:00
Hanno Becker
e74d556b43 Introduce function to indicate if record is fully processed
This commit introduces a function ssl_record_is_in_progress()
to indicate if there is there is more data within the current
record to be processed. Further, it moves the corresponding
call from ssl_read_record_layer() to the parent function
mbedtls_ssl_read_record(). With this change, ssl_read_record_layer()
has the sole purpose of fetching and decoding a new record,
and hence this commit also renames it to ssl_get_next_record().
2018-08-17 16:52:08 +01:00
Hanno Becker
2699459529 Move call to ssl_consume_current_message()
Subsequent commits will potentially inject buffered
messages after the last incoming message has been
consumed, but before a new one is fetched. As a
preparatory step to this, this commit moves the call
to ssl_consume_current_message() from ssl_read_record_layer()
to the calling function mbedtls_ssl_read_record().
2018-08-17 16:52:08 +01:00
Hanno Becker
1097b34022 Extract message-consuming code-path to separate function
The first part of the function ssl_read_record_layer() was
to mark the previous message as consumed. This commit moves
the corresponding code-path to a separate static function
ssl_consume_current_message().
2018-08-17 16:52:08 +01:00
Hanno Becker
4162b11eb4 Make mbedtls_ssl_read_record_layer() static
This function was previously global because it was
used directly within ssl_parse_certificate_verify()
in library/ssl_srv.c. The previous commit removed
this dependency, replacing the call by a call to
the global parent function mbedtls_ssl_read_record().
This renders mbedtls_ssl_read_record_layer() internal
and therefore allows to make it static, and accordingly
rename it as ssl_read_record_layer().
2018-08-17 16:52:08 +01:00
Hanno Becker
a4b143a57c Remove nested loop in mbedtls_ssl_read_record() 2018-08-17 16:52:08 +01:00
Hanno Becker
02f5907499 Correct misleading debugging output
Usually, debug messages beginning with "=> and "<="
match up and indicate entering of and returning from
functions, respectively. This commit fixes one exception
to this rule in mbedtls_ssl_read_record(), which sometimes
printed two messages of the form "<= XXX".
2018-08-17 16:52:08 +01:00
Hanno Becker
327c93b182 Add parameter to ssl_read_record() controlling checksum update
Previously, mbedtls_ssl_read_record() always updated the handshake
checksum in case a handshake record was received. While desirable
most of the time, for the CertificateVerify message the checksum
update must only happen after the message has been fully processed,
because the validation requires the handshake digest up to but
excluding the CertificateVerify itself. As a remedy, the bulk
of mbedtls_ssl_read_record() was previously duplicated within
ssl_parse_certificate_verify(), hardening maintenance in case
mbedtls_ssl_read_record() is subject to changes.

This commit adds a boolean parameter to mbedtls_ssl_read_record()
indicating whether the checksum should be updated in case of a
handshake message or not. This allows using it also for
ssl_parse_certificate_verify(), manually updating the checksum
after the message has been processed.
2018-08-17 16:52:08 +01:00
Hanno Becker
e1dcb03557 Don't send empty fragments of nonempty handshake messages
This for example lead to the following corner case bug:
The code attempted to piggy-back a Finished message at
the end of a datagram where precisely 12 bytes of payload
were still available. This lead to an empty Finished fragment
being sent, and when  mbedtls_ssl_flight_transmit() was called
again, it believed that it was just starting to send the
Finished message, thereby calling ssl_swap_epochs() which
had already happened in the call sending the empty fragment.
Therefore, the second call would send the 'rest' of the
Finished message with wrong epoch.
2018-08-17 16:47:58 +01:00
Hanno Becker
04da189225 Make datagram packing dynamically configurable
This commit adds a public function

   `mbedtls_ssl_conf_datagram_packing()`

that allows to allow / forbid the packing of multiple
records within a single datagram.
2018-08-17 15:45:25 +01:00
Hanno Becker
7e7721350b Fix unused variable warning in ssl_session_reset_int()
The `partial` argument is only used when DTLS and same port
client reconnect are enabled. This commit marks the variable
as unused if that's not the case.
2018-08-17 15:45:10 +01:00
Hanno Becker
0defedb488 Fix unused variable warning in mbedtls_ssl_get_max_record_payload
If neither the maximum fragment length extension nor DTLS
are used, the SSL context argument is unnecessary as the
maximum payload length is hardcoded as MBEDTLS_SSL_MAX_CONTENT_LEN.
2018-08-17 15:45:05 +01:00
Hanno Becker
f29d4702f7 Reset in/out pointers on SSL session reset
If a previous session was interrupted during flushing, the out
pointers might point arbitrarily into the output buffer.
2018-08-17 15:44:57 +01:00
Hanno Becker
4ccbf064ed Minor improvements in ssl_session_reset_int() 2018-08-17 15:44:53 +01:00
Hanno Becker
2a43f6f539 Introduce function to reset in/out pointers 2018-08-17 15:44:43 +01:00
Hanno Becker
b50a253a87 Move size check for records 2018-08-17 15:44:26 +01:00
Hanno Becker
67bc7c3a38 Don't immediately flush datagram after preparing a record
This commit finally enables datagram packing by modifying the
record preparation function ssl_write_record() to not always
calling mbedtls_ssl_flush_output().
2018-08-17 15:44:09 +01:00
Hanno Becker
2b1e354754 Increase record buffer pointer after preparing a record
The packing of multiple records within a single datagram works
by increasing the pointer `out_hdr` (pointing to the beginning
of the next outgoing record) within the datagram buffer, as
long as space is available and no flush was mandatory.

This commit does not yet change the code's behavior of always
flushing after preparing a record, but it introduces the logic
of increasing `out_hdr` after preparing the record, and resetting
it after the flush has been completed.
2018-08-17 15:41:02 +01:00
Hanno Becker
3b235902b8 Log calls to ssl_flight_append() in debugging output 2018-08-17 15:40:55 +01:00
Hanno Becker
04484621d0 Increment record sequence number in ssl_write_record()
Previously, the record sequence number was incremented at the
end of each successful call to mbedtls_ssl_flush_output(),
which works as long as there is precisely one such call for
each outgoing record.

When packing multiple records into a single datagram, this
property is no longer true, and instead the increment of the
record sequence number must happen after the record has been
prepared, and not after it has been dispatched.

This commit moves the code for incrementing the record sequence
number from mbedtls_ssl_flush_output() to ssl_write_record().
2018-08-17 15:40:52 +01:00
Hanno Becker
198594709b Store outgoing record sequence number outside record buffer
This commit is another step towards supporting the packing of
multiple records within a single datagram.

Previously, the incremental outgoing record sequence number was
statically stored within the record buffer, at its final place
within the record header. This slightly increased efficiency
as it was not necessary to copy the sequence number when writing
outgoing records.

When allowing multiple records within a single datagram, it is
necessary to allow the position of the current record within the
datagram buffer to be flexible; in particular, there is no static
address for the record sequence number field within the record header.

This commit introduces an additional field `cur_out_ctr` within
the main SSL context structure `mbedtls_ssl_context` to keep track
of the outgoing record sequence number independent of the buffer used
for the current record / datagram. Whenever a new record is written,
this sequence number is copied to the the address `out_ctr` of the
sequence number header field within the current outgoing record.
2018-08-17 15:40:35 +01:00
Hanno Becker
5aa4e2cedd Move deduction of internal record buffer pointers to function
The SSL/TLS module maintains a number of internally used pointers
`out_hdr`, `out_len`, `out_iv`, ..., indicating where to write the
various parts of the record header.

These pointers have to be kept in sync and sometimes need update:
Most notably, the `out_msg` pointer should always point to the
beginning of the record payload, and its offset from the pointer
`out_iv` pointing to the end of the record header is determined
by the length of the explicit IV used in the current record
protection mechanism.

This commit introduces functions deducing these pointers from
the pointers `out_hdr` / `in_hdr` to the beginning of the header
of the current outgoing / incoming record.

The flexibility gained by these functions will subsequently
be used to allow shifting of `out_hdr` for the purpose of
packing multiple records into a single datagram.
2018-08-17 15:40:24 +01:00
Hanno Becker
3136ede0e8 Compute record expansion in steps to ease readability 2018-08-17 15:28:19 +01:00
Manuel Pégourié-Gonnard
19c62f90e4 Add test for session resumption 2018-08-16 10:50:39 +02:00
Manuel Pégourié-Gonnard
7e89c17788 Fix two typos in comments 2018-08-16 10:01:47 +02:00
Manuel Pégourié-Gonnard
b747c6cf9b Add basic first tests for MTU setting
For now, just check that it causes us to fragment. More tests are coming in
follow-up commits to ensure we respect the exact value set, including when
renegotiating.
2018-08-16 10:01:47 +02:00
Manuel Pégourié-Gonnard
637e234d9f Merge remote-tracking branch 'public/pr/1915' into iotssl-165-dtls-hs-fragmentation-new
* public/pr/1915:
  Adapt ChangeLog
  Fix mbedtls_ssl_get_record_expansion() for ChaChaPoly and CBC
2018-08-16 10:01:21 +02:00
Manuel Pégourié-Gonnard
9468ff1966 Implement support for MTU setting 2018-08-16 10:01:10 +02:00
Manuel Pégourié-Gonnard
0b1d9b2c75 Declare ssl_conf_mtu() 2018-08-16 10:01:10 +02:00
Manuel Pégourié-Gonnard
2cb17e201b Make handshake fragmentation follow max_frag_len
Note: no interop tests in ssl-opt.sh for now, as some of them make us run into
bugs in (the CI's default versions of) OpenSSL and GnuTLS, so interop tests
will be added later once the situation is clarified. <- TODO
2018-08-16 10:01:10 +02:00
Manuel Pégourié-Gonnard
28f4beab1c Start implementing fragmentation 2018-08-16 10:01:10 +02:00
Manuel Pégourié-Gonnard
87a346f64e Always save flight first, (re)send later
This will allow fragmentation to always happen in the same place, always from
a buffer distinct from ssl->out_msg, and with the same way of resuming after
returning WANT_WRITE
2018-08-16 10:01:10 +02:00
Manuel Pégourié-Gonnard
9c3a8caa92 Clarify code a bit in write_handshake_msg()
- take advantage of the fact that we're only called for first send
- put all sanity checks at the top
- rename and constify shortcut variables
- improve comments
2018-08-16 10:00:35 +02:00
Manuel Pégourié-Gonnard
31c1586893 Start separating handshake from record writing 2018-08-16 10:00:27 +02:00
Hanno Becker
7864090ec1 Reset session_in/out pointers in ssl_session_reset_int()
Fixes #1941.
2018-08-13 16:35:15 +01:00
Hanno Becker
5b559ac7ab Fix mbedtls_ssl_get_record_expansion() for ChaChaPoly and CBC
`mbedtls_ssl_get_record_expansion()` is supposed to return the maximum
difference between the size of a protected record and the size of the
encapsulated plaintext.

It had the following two bugs:
(1) It did not consider the new ChaChaPoly ciphersuites, returning
    the error code #MBEDTLS_ERR_SSL_INTERNAL_ERROR in this case.
(2) It did not correctly estimate the maximum record expansion in case
    of CBC ciphersuites in (D)TLS versions 1.1 and higher, in which
    case the ciphertext is prefixed by an explicit IV.

This commit fixes both bugs.
2018-08-03 10:07:35 +01:00
k-stachowiak
9f7798ed3f Revert change of a return variable name 2018-07-31 16:52:32 +02:00
Simon Butcher
37b9fd5df6 Merge remote-tracking branch 'restricted/pr/490' into development 2018-07-24 23:40:37 +01:00
k-stachowiak
c9a5f02eab Move comment to a separate line 2018-07-24 13:53:31 +02:00
Angus Gratton
1a7a17e548 Check for invalid short Alert messages
(Short Change Cipher Spec & Handshake messages are already checked for.)
2018-07-20 23:09:29 +01:00
Angus Gratton
34817929ea TLSv1.2: Treat zero-length fragments as invalid, unless they are application data
TLS v1.2 explicitly disallows other kinds of zero length fragments (earlier standards
don't mention zero-length fragments at all).
2018-07-20 23:09:29 +01:00
Angus Gratton
b512bc1d29 CBC mode: Allow zero-length message fragments (100% padding)
Fixes https://github.com/ARMmbed/mbedtls/issues/1632
2018-07-20 23:09:29 +01:00
Simon Butcher
922bd1efb2 Merge remote-tracking branch 'public/pr/1752' into development 2018-07-20 14:33:18 +01:00
Simon Butcher
df15356259 Merge remote-tracking branch 'public/pr/1663' into development 2018-07-19 19:48:10 +01:00
k-stachowiak
21feae58cb Update change log 2018-07-11 17:34:55 +02:00
k-stachowiak
a47911cb70 Fix memory leak in ssl_setup 2018-07-11 17:26:07 +02:00
Manuel Pégourié-Gonnard
8744a02357 Clarify a few comments
The "+" sign could be misinterpreted as addition.
2018-07-11 12:30:40 +02:00
Simon Butcher
e7aeef09ee Merge remote-tracking branch 'public/pr/536' into development 2018-07-10 15:24:26 +01:00
Manuel Pégourié-Gonnard
6a25cfae2a Avoid debug message that might leak length
The length to the debug message could conceivably leak through the time it
takes to print it, and that length would in turn reveal whether padding was
correct or not.
2018-07-10 11:15:36 +02:00
Manuel Pégourié-Gonnard
7b42030b5d Add counter-measure to cache-based Lucky 13
The basis for the Lucky 13 family of attacks is for an attacker to be able to
distinguish between (long) valid TLS-CBC padding and invalid TLS-CBC padding.
Since our code sets padlen = 0 for invalid padding, the length of the input to
the HMAC function, and the location where we read the MAC, give information
about that.

A local attacker could gain information about that by observing via a
cache attack whether the bytes at the end of the record (at the location of
would-be padding) have been read during MAC verification (computation +
comparison).

Let's make sure they're always read.
2018-07-05 14:44:49 +02:00
Simon Butcher
4b6b08e7d2 Merge remote-tracking branch 'public/pr/1006' into development 2018-06-28 12:08:59 +01:00
niisato
8ee2422ef8 about a issue Replace "new" variable #1782 2018-06-25 19:05:48 +09:00
Andres Amaya Garcia
5b92352374 Document ssl_write_real() behaviour in detail 2018-06-21 19:23:21 +01:00
Manuel Pégourié-Gonnard
79d9b50421 Merge branch 'development' into iotssl-1260-non-blocking-ecc-restricted
* development: (180 commits)
  Change the library version to 2.11.0
  Fix version in ChangeLog for fix for #552
  Add ChangeLog entry for clang version fix. Issue #1072
  Compilation warning fixes on 32b platfrom with IAR
  Revert "Turn on MBEDTLS_SSL_ASYNC_PRIVATE by default"
  Fix for missing len var when XTS config'd and CTR not
  ssl_server2: handle mbedtls_x509_dn_gets failure
  Fix harmless use of uninitialized memory in ssl_parse_encrypted_pms
  SSL async tests: add a few test cases for error in decrypt
  Fix memory leak in ssl_server2 with SNI + async callback
  SNI + SSL async callback: make all keys async
  ssl_async_resume: free the operation context on error
  ssl_server2: get op_name from context in ssl_async_resume as well
  Clarify "as directed here" in SSL async callback documentation
  SSL async callbacks documentation: clarify resource cleanup
  Async callback: use mbedtls_pk_check_pair to compare keys
  Rename mbedtls_ssl_async_{get,set}_data for clarity
  Fix copypasta in the async callback documentation
  SSL async callback: cert is not always from mbedtls_ssl_conf_own_cert
  ssl_async_set_key: detect if ctx->slots overflows
  ...
2018-06-20 09:46:17 +02:00
Manuel Pégourié-Gonnard
2e58e8ee34 Implement ChachaPoly mode in TLS 2018-06-19 12:12:47 +02:00
Angus Gratton
d8213d00db Let MBEDTLS_SSL_MAX_CONTENT_LEN to be split into outward & inward sizes
For the situation where the mbedTLS device has limited RAM, but the
other end of the connection doesn't support the max_fragment_length
extension. To be spec-compliant, mbedTLS has to keep a 16384 byte
incoming buffer. However the outgoing buffer can be made smaller without
breaking spec compliance, and we save some RAM.

See comments in include/mbedtls/config.h for some more details.

(The lower limit of outgoing buffer size is the buffer size used during
handshake/cert negotiation. As the handshake is half-duplex it might
even be possible to store this data in the "incoming" buffer during the
handshake, which would save even more RAM - but it would also be a lot
hackier and error-prone. I didn't really explore this possibility, but
thought I'd mention it here in case someone sees this later on a mission
to jam mbedTLS into an even tinier RAM footprint.)
2018-06-18 20:51:51 +10:00
Simon Butcher
5f57f1e3cc Merge remote-tracking branch 'public/pr/1270' into development 2018-06-15 14:17:31 +01:00
Manuel Pégourié-Gonnard
558da9c3fe Make SSL error code more generic
It's undesirable to have users of the SSL layer check for an error code
specific to a lower-level layer, both out of general layering principles, and
also because if we later make another crypto module gain resume capabilities,
we would need to change the contract again (checking for a new module-specific
error code).
2018-06-13 12:02:12 +02:00
Manuel Pégourié-Gonnard
da19f4c79f Merge branch 'development' into iotssl-1260-non-blocking-ecc-restricted
Summary of merge conflicts:

include/mbedtls/ecdh.h -> documentation style
include/mbedtls/ecdsa.h -> documentation style
include/mbedtls/ecp.h -> alt style, new error codes, documentation style
include/mbedtls/error.h -> new error codes
library/error.c -> new error codes (generated anyway)
library/ecp.c:
    - code of an extracted function was changed
library/ssl_cli.c:
    - code addition on one side near code change on the other side
      (ciphersuite validation)
library/x509_crt.c -> various things
    - top fo file: helper structure added near old zeroize removed
    - documentation of find_parent_in()'s signature: improved on one side,
      added arguments on the other side
    - documentation of find_parent()'s signature: same as above
    - verify_chain(): variables initialised later to give compiler an
      opportunity to warn us if not initialised on a code path
    - find_parent(): funcion structure completely changed, for some reason git
      tried to insert a paragraph of the old structure...
    - merge_flags_with_cb(): data structure changed, one line was fixed with a
      cast to keep MSVC happy, this cast is already in the new version
    - in verify_restratable(): adjacent independent changes (function
      signature on one line, variable type on the next)
programs/ssl/ssl_client2.c:
    - testing for IN_PROGRESS return code near idle() (event-driven):
      don't wait for data in the the socket if ECP_IN_PROGRESS
tests/data_files/Makefile: adjacent independent additions
tests/suites/test_suite_ecdsa.data: adjacent independent additions
tests/suites/test_suite_x509parse.data: adjacent independent additions

* development: (1059 commits)
  Change symlink to hardlink to avoid permission issues
  Fix out-of-tree testing symlinks on Windows
  Updated version number to 2.10.0 for release
  Add a disabled CMAC define in the no-entropy configuration
  Adapt the ARIA test cases for new ECB function
  Fix file permissions for ssl.h
  Add ChangeLog entry for PR#1651
  Fix MicroBlaze register typo.
  Fix typo in doc and copy missing warning
  Fix edit mistake in cipher_wrap.c
  Update CTR doc for the 64-bit block cipher
  Update CTR doc for other 128-bit block ciphers
  Slightly tune ARIA CTR documentation
  Remove double declaration of mbedtls_ssl_list_ciphersuites
  Update CTR documentation
  Use zeroize function from new platform_util
  Move to new header style for ALT implementations
  Add ifdef for selftest in header file
  Fix typo in comments
  Use more appropriate type for local variable
  ...
2018-06-13 09:52:54 +02:00
Philippe Antoine
747fd53938 Fixes different off by ones 2018-06-05 16:13:10 +02:00
Gilles Peskine
d0e55a4657 ssl_decrypt_buf: remove code for hashes that aren't used in TLS 2018-06-04 14:41:19 +02:00
Gilles Peskine
5c38984fa7 Use our habitual INTERNAL_ERROR debug message 2018-06-04 12:02:43 +02:00
Gilles Peskine
a7fe25d5a5 Remove tests of #define's that don't exist 2018-06-04 12:01:18 +02:00
Gilles Peskine
1bd9d58b21 Clarify comment about integer division by a variable 2018-06-04 11:58:44 +02:00
Gilles Peskine
20b4408fbd Fix Lucky13 attack protection when using HMAC-SHA-384
As a protection against the Lucky Thirteen attack, the TLS code for
CBC decryption in encrypt-then-MAC mode performs extra MAC
calculations to compensate for variations in message size due to
padding. The amount of extra MAC calculation to perform was based on
the assumption that the bulk of the time is spent in processing
64-byte blocks, which is correct for most supported hashes but not for
SHA-384. Correct the amount of extra work for SHA-384 (and SHA-512
which is currently not used in TLS, and MD2 although no one should
care about that).
2018-05-29 14:06:49 +02:00
Manuel Pégourié-Gonnard
a3712beb9b Merge branch 'development' into iotssl-1941-aria-ciphersuites
* development: (504 commits)
  Fix minor code style issues
  Add the uodate to the soversion to the ChangeLog
  Fix the ChangeLog for clarity, english and credit
  Update version to 2.9.0
  ecp: Fix binary compatibility with group ID
  Changelog entry
  Change accepted ciphersuite versions when parsing server hello
  Remove preprocessor directives around platform_util.h include
  Fix style for mbedtls_mpi_zeroize()
  Improve mbedtls_platform_zeroize() docs
  mbedtls_zeroize -> mbedtls_platform_zeroize in docs
  Reword config.h docs for MBEDTLS_PLATFORM_ZEROIZE_ALT
  Organize CMakeLists targets in alphabetical order
  Organize output objs in alfabetical order in Makefile
  Regenerate errors after ecp.h updates
  Update ecp.h
  Change variable bytes_written to header_bytes in record decompression
  Update ecp.h
  Update ecp.h
  Update ecp.h
  ...
2018-05-22 15:58:50 +02:00
Darryl Green
11999bb72e Fix minor code style issues 2018-05-15 09:21:57 +01:00
Jaeden Amero
a331e0f0af Merge remote-tracking branch 'upstream-restricted/pr/421' into development-proposed 2018-05-04 14:39:24 +01:00
Gilles Peskine
1febfef561 Rename mbedtls_ssl_async_{get,set}_data for clarity
Rename to mbedtls_ssl_get_async_operation_data and
mbedtls_ssl_set_async_operation_data so that they're about
"async operation data" and not about some not-obvious "data".
2018-04-30 11:54:39 +02:00
Gilles Peskine
8f97af7ea3 Don't pass the async config data to async callbacks
The config data is in the SSL config, so callbacks can retrieve it
from there, with the new function mbedtls_ssl_conf_get_async_config_data.
2018-04-26 11:46:10 +02:00
Gilles Peskine
df13d5c7a6 Pass the SSL context to async callbacks
When a handshake step starts an asynchronous operation, the
application needs to know which SSL connection the operation is for,
so that when the operation completes, the application can wake that
connection up. Therefore the async start callbacks need to take the
SSL context as an argument. It isn't enough to let them set a cookie
in the SSL connection, the application needs to be able to find the
right SSL connection later.

Also pass the SSL context to the other callbacks for consistency. Add
a new field to the handshake that the application can use to store a
per-connection context. This new field replaces the former
context (operation_ctx) that was created by the start function and
passed to the resume function.

Add a boolean flag to the handshake structure to track whether an
asynchronous operation is in progress. This is more robust than
relying on the application to set a non-null application context.
2018-04-26 10:00:40 +02:00
Gilles Peskine
9b562d5c36 mbedtls_ssl_handshake_free: take the SSL context as argument
Change the signature of mbedtls_ssl_handshake_free again. Now take the
whole SSL context as argument and not just the configuration and the
handshake substructure.

This is in preparation for changing the asynchronous cancel callback
to take the SSL context as an argument.
2018-04-26 10:00:40 +02:00
Gilles Peskine
b74a1c73b1 Rename MBEDTLS_SSL_ASYNC_PRIVATE_C to MBEDTLS_SSL_ASYNC_PRIVATE
This is an optional feature, not a module of its own, so don't call it
MBEDTLS_xxx_C and put it in the appropriate section of config.h.
2018-04-26 10:00:39 +02:00
Gilles Peskine
f112725487 Style and grammar fixes 2018-04-26 10:00:39 +02:00
Gilles Peskine
b44692f126 Merge branch 'mbedtls_ssl_get_key_exchange_md_ssl_tls-return_hashlen' into tls_async_server-2.9
Conflict resolution:
* ChangeLog: put the new entry from my branch in the proper place.
* include/mbedtls/error.h: counted high-level module error codes again.
* include/mbedtls/ssl.h: picked different numeric codes for the
  concurrently added errors; made the new error a full sentence per
  current standards.
* library/error.c: ran scripts/generate_errors.pl.
* library/ssl_srv.c:
    * ssl_prepare_server_key_exchange "DHE key exchanges": the conflict
      was due to style corrections in development
      (4cb1f4d49c) which I merged with
      my refactoring.
    * ssl_prepare_server_key_exchange "For key exchanges involving the
      server signing", first case, variable declarations: merged line
      by line:
        * dig_signed_len: added in async
        * signature_len: removed in async
        * hashlen: type changed to size_t in development
        * hash: size changed to MBEDTLS_MD_MAX_SIZE in async
        * ret: added in async
    * ssl_prepare_server_key_exchange "For key exchanges involving the
      server signing", first cae comment: the conflict was due to style
      corrections in development (4cb1f4d49c)
      which I merged with my comment changes made as part of refactoring
      the function.
    * ssl_prepare_server_key_exchange "Compute the hash to be signed" if
      `md_alg != MBEDTLS_MD_NONE`: conflict between
      ebd652fe2d
      "ssl_write_server_key_exchange: calculate hashlen explicitly" and
      46f5a3e9b4 "Check return codes from
      MD in ssl code". I took the code from commit
      ca1d742904 made on top of development
      which makes mbedtls_ssl_get_key_exchange_md_ssl_tls return the
      hash length.
* programs/ssl/ssl_server2.c: multiple conflicts between the introduction
  of MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS and new auxiliary functions and
  definitions for async support, and the introduction of idle().
    * definitions before main: concurrent additions, kept both.
    * main, just after `handshake:`: in the loop around
      mbedtls_ssl_handshake(), merge the addition of support for
      MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS and SSL_ASYNC_INJECT_ERROR_CANCEL
      with the addition of the idle() call.
    * main, if `opt.transport == MBEDTLS_SSL_TRANSPORT_STREAM`: take the
      code from development and add a check for
      MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS.
    * main, loop around mbedtls_ssl_read() in the datagram case:
      take the code from development and add a check for
      MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS; revert to a do...while loop.
    * main, loop around mbedtls_ssl_write() in the datagram case:
      take the code from development and add a check for
      MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS; revert to a do...while loop.
2018-04-26 10:00:27 +02:00
Andrzej Kurek
a9ceef8e03 Change variable bytes_written to header_bytes in record decompression
The name is changed to better reflect the input, decompression case
2018-04-24 06:34:17 -04:00
Gilles Peskine
ca1d742904 mbedtls_ssl_get_key_exchange_md_tls1_2: return hashlen
In mbedtls_ssl_get_key_exchange_md_tls1_2, add an output parameter for
the hash length. The code that calls this function can currently do
without it, but it will need the hash length in the future, when
adding support for a third-party callback to calculate the signature
of the hash.
2018-04-24 11:53:22 +02:00
Gilles Peskine
8bf79f6dc6 SSL asynchronous private key operation callbacks: interface
New compile-time option MBEDTLS_SSL_ASYNC_PRIVATE_C, enabling
callbacks to replace private key operations. These callbacks allow the
SSL stack to make an asynchronous call to an external cryptographic
module instead of calling the cryptography layer inside the library.
The call is asynchronous in that it may return the new status code
MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS, in which case the SSL stack returns
and can be later called where it left off.

This commit introduces the configuration option. Later commits will
implement the feature proper.
2018-04-24 09:32:28 +02:00
Gilles Peskine
59e83d96db Add conf parameter to mbedtls_ssl_handshake_free
This function is declared in ssl_internal.h, so this is not a public
API change.

This is in preparation for mbedtls_ssl_handshake_free needing to call
methods from the config structure.
2018-04-24 09:32:28 +02:00
Andrzej Kurek
5462e02874 ssl_tls: Fix invalid buffer sizes during compression / decompression
Adjust information passed to zlib to include already written data.
2018-04-20 07:58:53 -04:00
Andres Amaya Garcia
1f6301b3c8 Rename mbedtls_zeroize to mbedtls_platform_zeroize 2018-04-17 10:00:21 -05:00
Andres Amaya Garcia
e32df087fb Remove individual copies of mbedtls_zeroize()
This commit removes all the static occurrencies of the function
mbedtls_zeroize() in each of the individual .c modules. Instead the
function has been moved to utils.h that is included in each of the
modules.
2018-04-17 09:19:05 -05:00
Gilles Peskine
80aa3b8d65 Merge branch 'pr_946' into development-proposed 2018-04-04 10:33:45 +02:00
Gilles Peskine
73db8380ca Merge remote-tracking branch 'upstream-public/pr/1547' into development-proposed 2018-04-04 09:19:12 +02:00
Gilles Peskine
a09453f495 Merge branch 'pr_1395' into development-proposed 2018-04-04 09:14:12 +02:00
mohammad1603
19d392b258 Fix compatibility problem in the printed message
Replace %zu with %lu and add cast for the printed value.
2018-04-02 07:35:50 -07:00
Gilles Peskine
039fd12834 Robustness fix in mbedtls_ssl_derive_keys
In mbedtls_ssl_derive_keys, don't call mbedtls_md_hmac_starts in
ciphersuites that don't use HMAC. This doesn't change the behavior of
the code, but avoids relying on an uncaught error when attempting to
start an HMAC operation that hadn't been initialized.
2018-03-31 22:53:49 +02:00
mohammad1603
52aecb9a7f Check whether INT_MAX larger than SIZE_MAX scenario
Check whether INT_MAX larger than SIZE_MAX scenario
2018-03-28 23:41:40 -07:00
Hanno Becker
2bd57578af Merge branch 'development' into iotssl-1204 2018-03-28 14:52:35 +01:00
mohammad1603
b878805919 Verify that f_send and f_recv send and receive the expected length
Verify that f_send and f_recv send and receive the expected length
2018-03-22 02:58:23 -07:00
Azim Khan
27e8a120b2 Assign NULL after freeing psk and psk_identity 2018-03-21 14:24:11 +00:00
junyeonLEE
316b162ac3 Separate psk and psk_identity buffers free
Sometimes, psk_identity buffer can't released because psk buffer is NULL.
So, separate it.
2018-03-21 14:15:28 +00:00
Gilles Peskine
b4c571e603 Merge remote-tracking branch 'upstream-public/pr/1296' into HEAD 2018-03-11 00:44:14 +01:00
mohammad1603
5bd15cbfa0 Avoid wraparound for ssl->in_left
Add check to avoid wraparound for ssl->in_left
2018-02-28 04:30:59 -08:00
Markku-Juhani O. Saarinen
c06e1014e1 ARIA ciphersuites for TLS 1.2 2018-02-27 12:39:12 +01:00
Gilles Peskine
b7f6086ba3 Merge branch 'prr_424' into development-proposed 2018-02-22 16:15:01 +01:00
mohammad1603
4bbaeb4ffa Add guard to out_left to avoid negative values
return error when f_send return a value greater than out_left
2018-02-22 05:04:48 -08:00
Jaeden Amero
784de59ccd Merge remote-tracking branch 'upstream-restricted/pr/410' into development-restricted
- Resolve ChangeLog conflicts
- Update Doxygen warning block in dhm.h to render correctly
- Prefix the exported identifier deprecated_constant_t with mbedtls_
2018-01-26 18:43:04 +00:00
Jaeden Amero
66954e1c1f Merge branch 'development' into development-restricted 2018-01-25 17:28:31 +00:00
Ron Eldor
5e9f14d4d9 Set correct minimal versions in default conf
Set `MBEDTLS_SSL_MIN_MAJOR_VERSION` and `MBEDTLS_SSL_MIN_MINOR_VERSION`
instead of `MBEDTLS_SSL_MAJOR_VERSION_3` and `MBEDTLS_SSL_MINOR_VERSION_1`
2018-01-22 22:06:44 +01:00
Gilles Peskine
9e4f77c606 New MD API: rename functions from _ext to _ret
The _ext suffix suggests "new arguments", but the new functions have
the same arguments. Use _ret instead, to convey that the difference is
that the new functions return a value.
2018-01-22 11:54:42 +01:00
Gilles Peskine
d91f2a26cb Merge branch 'development' into iotssl-1251-2.7
Conflict resolution:

* ChangeLog: put the new entries in their rightful place.
* library/x509write_crt.c: the change in development was whitespace
  only, so use the one from the iotssl-1251 feature branch.
2018-01-19 11:25:10 +01:00
Hanno Becker
3d8c90711b Compute outgoing MAC in temporary buffer for MAC-then-Encrypt
A previous commit changed the record encryption function
`ssl_encrypt_buf` to compute the MAC in a temporary buffer
and copying the relevant part of it (which is strictly smaller
if the truncated HMAC extension is used) to the outgoing message
buffer. However, the change was only made in case Encrypt-Then-MAC
was enabled, but not in case of MAC-Then-Encrypt. While this
doesn't constitute a problem, for the sake of uniformity this
commit changes `ssl_encrypt_buf` to compute the MAC in a temporary
buffer in this case, too.
2018-01-05 16:24:22 +00:00
Gilles Peskine
82d607eb9e Merge remote-tracking branch 'upstream-restricted/pr/433' into development-restricted 2017-12-19 19:20:27 +01:00
Manuel Pégourié-Gonnard
d04c623ed6 Merge remote-tracking branch 'restricted/pr/403' into development-restricted
* restricted/pr/403:
  Correct record header size in case of TLS
  Don't allocate space for DTLS header if DTLS is disabled
  Improve debugging output
  Adapt ChangeLog
  Add run-time check for handshake message size in ssl_write_record
  Add run-time check for record content size in ssl_encrypt_buf
  Add compile-time checks for size of record content and payload
2017-12-19 11:31:20 +01:00
Manuel Pégourié-Gonnard
b053efb295 Fix magic constant in previous commit 2017-12-19 10:03:46 +01:00
Manuel Pégourié-Gonnard
464147cadc Fix SSLv3 MAC computation
In a previous PR (Fix heap corruption in implementation of truncated HMAC
extension #425) the place where MAC is computed was changed from the end of
the SSL I/O buffer to a local buffer (then (part of) the content of the local
buffer is either copied to the output buffer of compare to the input buffer).

Unfortunately, this change was made only for TLS 1.0 and later, leaving SSL
3.0 in an inconsistent state due to ssl_mac() still writing to the old,
hard-coded location, which, for MAC verification, resulted in later comparing
the end of the input buffer (containing the computed MAC) to the local buffer
(uninitialised), most likely resulting in MAC verification failure, hence no
interop (even with ourselves).

This commit completes the move to using a local buffer by using this strategy
for SSL 3.0 too. Fortunately ssl_mac() was static so it's not a problem to
change its signature.
2017-12-18 18:04:59 +01:00
Gilles Peskine
02e28fe0fd Merge remote-tracking branch 'upstream-restricted/pr/425' into development-restricted 2017-12-01 17:58:12 +01:00
Gilles Peskine
0960f0663e Merge branch 'development' into development-restricted 2017-11-29 21:07:55 +01:00
Gilles Peskine
0884f4811b Merge remote-tracking branch 'upstream-public/pr/1141' into development 2017-11-29 20:50:59 +01:00
Hanno Becker
1df4923eb1 Remove compile-time deprecation warning for TRUNCATED_HMAC_COMPAT 2017-11-29 16:55:56 +00:00
Gilles Peskine
c753f5daf4 Merge remote-tracking branch 'upstream-restricted/pr/369' into development-restricted 2017-11-28 14:16:47 +01:00
Gilles Peskine
68306ed31f Merge remote-tracking branch 'upstream-public/pr/1094' into development 2017-11-23 20:02:46 +01:00
Hanno Becker
4c2ac7ef58 Deprecate MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT 2017-11-21 18:28:35 +00:00
Hanno Becker
563423fb21 Improve documentation of MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT option
Explain more clearly when this option should be used and which versions of Mbed
TLS build on the non-compliant implementation.
2017-11-21 17:20:17 +00:00
Hanno Becker
e89353a6b4 Add fallback to non-compliant truncated HMAC for compatibiltiy
In case truncated HMAC must be used but the Mbed TLS peer hasn't been updated
yet, one can use the compile-time option MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT to
temporarily fall back to the old, non-compliant implementation of the truncated
HMAC extension.
2017-11-20 16:50:43 +00:00
Hanno Becker
81c7b18351 Don't truncate MAC key when truncated HMAC is negotiated
The truncated HMAC extension as described in
https://tools.ietf.org/html/rfc6066.html#section-7 specifies that when truncated
HMAC is used, only the HMAC output should be truncated, while the HMAC key
generation stays unmodified. This commit fixes Mbed TLS's behavior of also
truncating the key, potentially leading to compatibility issues with peers
running other stacks than Mbed TLS.

Details:
The keys for the MAC are pieces of the keyblock that's generated from the
master secret in `mbedtls_ssl_derive_keys` through the PRF, their size being
specified as the size of the digest used for the MAC, regardless of whether
truncated HMAC is enabled or not.

             /----- MD size ------\ /------- MD size ----\
Keyblock    +----------------------+----------------------+------------------+---
now         |     MAC enc key      |      MAC dec key     |     Enc key      |  ...
(correct)   +----------------------+----------------------+------------------+---

In the previous code, when truncated HMAC was enabled, the HMAC keys
were truncated to 10 bytes:

             /-10 bytes-\  /-10 bytes-\
Keyblock    +-------------+-------------+------------------+---
previously  | MAC enc key | MAC dec key |     Enc key      |  ...
(wrong)     +-------------+-------------+------------------+---

The reason for this was that a single variable `transform->maclen` was used for
both the keysize and the size of the final MAC, and its value was reduced from
the MD size to 10 bytes in case truncated HMAC was negotiated.

This commit fixes this by introducing a temporary variable `mac_key_len` which
permanently holds the MD size irrespective of the presence of truncated HMAC,
and using this temporary to obtain the MAC key chunks from the keyblock.
2017-11-20 16:25:50 +00:00
Hanno Becker
992b6872f3 Fix heap corruption in ssl_decrypt_buf
Previously, MAC validation for an incoming record proceeded as follows:

1) Make a copy of the MAC contained in the record;
2) Compute the expected MAC in place, overwriting the presented one;
3) Compare both.

This resulted in a record buffer overflow if truncated MAC was used, as in this
case the record buffer only reserved 10 bytes for the MAC, but the MAC
computation routine in 2) always wrote a full digest.

For specially crafted records, this could be used to perform a controlled write of
up to 6 bytes past the boundary of the heap buffer holding the record, thereby
corrupting the heap structures and potentially leading to a crash or remote code
execution.

This commit fixes this by making the following change:
1) Compute the expected MAC in a temporary buffer that has the size of the
   underlying message digest.
2) Compare to this to the MAC contained in the record, potentially
   restricting to the first 10 bytes if truncated HMAC is used.

A similar fix is applied to the encryption routine `ssl_encrypt_buf`.
2017-11-20 08:52:25 +00:00
Manuel Pégourié-Gonnard
888fedea06 Merge branch 'development' into development-restricted
* development: (30 commits)
  update README file (#1144)
  Fix typo in asn1.h
  Improve leap year test names in x509parse.data
  Correctly handle leap year in x509_date_is_valid()
  Renegotiation: Add tests for SigAlg ext parsing
  Parse Signature Algorithm ext when renegotiating
  Minor style fix
  config.pl get: be better behaved
  config.pl get: don't rewrite config.h; detect write errors
  Fixed "config.pl get" for options with no value
  Fix typo and bracketing in macro args
  Ensure failed test_suite output is sent to stdout
  Remove use of GNU sed features from ssl-opt.sh
  Fix typos in ssl-opt.sh comments
  Add ssl-opt.sh test to check gmt_unix_time is good
  Extend ssl-opt.h so that run_test takes function
  Always print gmt_unix_time in TLS client
  Restored note about using minimum functionality in makefiles
  Note in README that GNU make is required
  Fix changelog for ssl_server2.c usage fix
  ...
2017-11-14 08:24:22 +01:00
Hanno Becker
05c4fc8608 Correct typo in debugging message 2017-11-09 14:34:06 +00:00
Hanno Becker
e41158ba10 Add comment on the meaning of ssl->in_offt == NULL 2017-10-23 13:30:32 +01:00
Hanno Becker
e72489de11 Remove internal references and use milder wording for some comments 2017-10-23 13:23:50 +01:00
Hanno Becker
a6fb089efc Don't split debug messages 2017-10-23 13:17:48 +01:00
Hanno Becker
21df7f90d2 Fix handling of HS msgs in mbedtls_ssl_read if renegotiation unused
Previously, if `MBEDTLS_SSL_RENEGOTIATION` was disabled, incoming handshake
messages in `mbedtls_ssl_read` (expecting application data) lead to the
connection being closed. This commit fixes this, restricting the
`MBEDTLS_SSL_RENEGOTIATION`-guard to the code-paths responsible for accepting
renegotiation requests and aborting renegotiation attempts after too many
unexpected records have been received.
2017-10-17 11:03:26 +01:00
Hanno Becker
b4ff0aafd9 Swap branches accepting/refusing renegotiation in in ssl_read 2017-10-17 11:03:04 +01:00
Hanno Becker
c76c619dd0 Reconcile resending of previous flights
This commit reconciles the code path responsible for resending the
final DTLS handshake flight with the path for handling resending of
the other flights.
2017-10-10 16:04:49 +01:00
Hanno Becker
90333dab85 Replace wrong usage of WANT_READ by CONTINUE_PROCESSING 2017-10-10 16:04:48 +01:00
Hanno Becker
52c6dc64c6 Correct length check for DTLS records from old epochs.
DTLS records from previous epochs were incorrectly checked against the
current epoch transform's minimal content length, leading to the
rejection of entire datagrams. This commit fixed that and adapts two
test cases accordingly.

Internal reference: IOTSSL-1417
2017-10-10 16:04:32 +01:00
Hanno Becker
8b170a0a0b Enhance and extend checking of message processing state
-  Enhances the documentation of mbedtls_ssl_get_bytes_avail (return
   the number of bytes left in the current application data record, if
   there is any).
-  Introduces a new public function mbedtls_ssl_check_pending for
   checking whether any data in the internal buffers still needs to be
   processed. This is necessary for users implementing event-driven IO
   to decide when they can safely idle until they receive further
   events from the underlying transport.
2017-10-10 16:04:32 +01:00
Hanno Becker
e65ce7862a Enhance debugging output in ssl_tls.c
Give a note on the debugging output on the following occasions:
(1) The timer expires in mbedtls_ssl_fetch_input
(2) There's more than one records within a single datagram
2017-10-10 16:02:36 +01:00
Hanno Becker
1a9a51c7cf Enhance documentation of ssl_write_hostname_ext, adapt ChangeLog.
Add a reference to the relevant RFC, adapt ChangeLog.
2017-10-06 11:58:50 +01:00
Hanno Becker
947194e7cf Make mbedtls_ssl_set_hostname safe to be called multiple times
Zeroize and free previously set hostnames before overwriting
them. Also, allow clearance of hostname by providing NULL parameter.
2017-10-06 11:58:50 +01:00
Hanno Becker
a90658f248 Add ssl_conf_dh_param_bin superseding ssl_conf_dh_param 2017-10-04 15:29:08 +01:00
Hanno Becker
470a8c4d87 Deprecate mbedtls_ssl_conf_dh_param 2017-10-04 15:28:46 +01:00
Hanno Becker
184f675256 Improve debugging output 2017-10-04 13:47:33 +01:00
Hanno Becker
00d0a6834a Adapt code setting default DHM parameters 2017-10-04 13:17:49 +01:00
Hanno Becker
2f38a43d3a Enhance documentation of ssl_write_hostname_ext, adapt ChangeLog.
Add a reference to the relevant RFC, adapt ChangeLog.
2017-09-30 23:35:21 +01:00
Hanno Becker
39f5d359f5 Make mbedtls_ssl_set_hostname safe to be called multiple times
Zeroize and free previously set hostnames before overwriting
them. Also, allow clearance of hostname by providing NULL parameter.
2017-09-30 23:35:02 +01:00
Hanno Becker
8c8b0ab877 Change default Diffie-Hellman parameters from RFC 5114 to RFC 7919
The origin of the primes in RFC 5114 is undocumented and their use therefore
constitutes a security risk.
2017-09-27 12:43:57 +01:00
Florin
0b7b83fd91 Fixed SIGSEGV problem when writing with ssl_write_real a buffer that is over MBEDTLS_SSL_MAX_CONTENT_LEN bytes
Signed-off-by: Florin <petriuc.florin@gmail.com>
2017-09-18 16:11:42 +01:00
Hanno Becker
2b187c4d5f Correct typo 2017-09-18 16:11:42 +01:00
Hanno Becker
9648f8b59c Add run-time check for handshake message size in ssl_write_record 2017-09-18 10:56:15 +01:00
Hanno Becker
d33f1ca34c Add run-time check for record content size in ssl_encrypt_buf 2017-09-18 10:56:14 +01:00
Andres Amaya Garcia
01692531c6 Document code silently discarding invalid records 2017-09-14 20:20:31 +01:00
Andres Amaya Garcia
2fad94b193 Dont send alert on invalid DTLS record type
Do not send fatal alerts when receiving a record with an invalid header
while running DTLS as this is not compliant behaviour.
2017-09-14 20:18:37 +01:00
Manuel Pégourié-Gonnard
0b23f167ba SSL: rework restart state handling
As done by previous commits for ECC and ECDSA:
- use explicit state assignments rather than increment
- always place the state update right before the operation label

This will make it easier to add restart support for other operations later if
desired.

SSL-specific changes:
- remove useless states: when the last restartable operation on a message is
  complete, ssl->state is incremented already, so we don't need any additional
state update: ecrs_state is only meant to complement ssl->state
- rename remaining states consistently as <message>_<operation>
- move some labels closer to the actual operation when possible (no assignment
  to variables used after the label between its previous and current position)
2017-08-24 12:08:33 +02:00
Manuel Pégourié-Gonnard
3bf49c4552 Enable restart for certificate verify 2017-08-15 14:12:47 +02:00
Manuel Pégourié-Gonnard
fed37ed039 Extract some code to separate function
Goals include:
- reducing the number of local variables in the main function (so that we
  don't have to worry about saving/restoring them)
- reducing the number exit points in the main function, making it easier to
  update ssl->state only right before we return
2017-08-15 13:35:42 +02:00
Manuel Pégourié-Gonnard
39eda87382 Make more auto variables const
That way we know we don't have to worry about saving and restoring their
value.
2017-08-15 13:00:33 +02:00
Manuel Pégourié-Gonnard
6b7301c872 Change restart context type.
No need to have both x509 and ecdsa, as the former contains the later.
2017-08-15 12:08:45 +02:00
Manuel Pégourié-Gonnard
d27d1a5a82 Clean up existing SSL restartable ECC code
- more consistent naming with ecrs prefix for everything
- always check it enabled before touching the rest
- rm duplicated code in parse_server_hello()
2017-08-15 11:49:08 +02:00
Manuel Pégourié-Gonnard
862cde5b8e Add restart support for ECDSA client auth 2017-08-09 11:44:53 +02:00
Andres Amaya Garcia
3395250f5f Fix use of uninitialised ret ssl_tls.c 2017-07-20 16:29:16 +01:00
Andres Amaya Garcia
46f5a3e9b4 Check return codes from MD in ssl code 2017-07-20 16:17:51 +01:00
Andres Amaya Garcia
bbafd34ebb Set len var to 0 when buf is freed in ssl_tls.c 2017-07-05 14:25:21 +01:00
Andres Amaya Garcia
1a607a1b9a Change ssl_tls to use new MD API and check ret code 2017-06-29 17:09:42 +01:00
Andres Amaya Garcia
a00498819f Zeroize old psk buf when changing value in ssl_tls 2017-06-26 11:35:17 +01:00
Hanno Becker
bdf3905fff Ensure application data records are not kept when fully processed
This commit fixes the following case: If a client is both expecting a
SERVER_HELLO and has an application data record that's partially
processed in flight (that's the situation the client gets into after
receiving a ServerHelloRequest followed by ApplicationData), a
subsequent call to mbedtls_ssl_read will set keep_current_message = 1
when seeing the unexpected application data, but not reset it to 0
after the application data has been processed. This commit fixes this.

It also documents and suggests how the problem might be solved in a
more structural way on the long run.
2017-06-09 10:42:03 +01:00
Hanno Becker
bb9dd0c044 Add hard assertion to mbedtls_ssl_read_record_layer
This commit adds a hard assertion to mbedtls_ssl_read_record_layer
triggering if both ssl->in_hslen and ssl->in_offt are not 0. This
should never happen, and if it does, there's no sensible way of
telling whether the previous message was a handshake or an application
data message.
2017-06-08 11:55:34 +01:00
Hanno Becker
4a810fba69 Fix mbedtls_ssl_read
Don't fetch a new record in mbedtls_ssl_read_record_layer as long as an application data record is being processed.
2017-06-08 10:12:16 +01:00
Hanno Becker
af0665d8b0 Simplify retaining of messages for future processing
There are situations in which it is not clear what message to expect
next. For example, the message following the ServerHello might be
either a Certificate, a ServerKeyExchange or a CertificateRequest. We
deal with this situation in the following way: Initially, the message
processing function for one of the allowed message types is called,
which fetches and decodes a new message. If that message is not the
expected one, the function returns successfully (instead of throwing
an error as usual for unexpected messages), and the handshake
continues to the processing function for the next possible message. To
not have this function fetch a new message, a flag in the SSL context
structure is used to indicate that the last message was retained for
further processing, and if that's set, the following processing
function will not fetch a new record.

This commit simplifies the usage of this message-retaining parameter
by doing the check within the record-fetching routine instead of the
specific message-processing routines. The code gets cleaner this way
and allows retaining messages to be used in other situations as well
without much effort. This will be used in the next commits.
2017-06-08 10:12:16 +01:00
Hanno Becker
e6706e62d8 Add tests for missing CA chains and bad curves.
This commit adds four tests to tests/ssl-opt.sh:
(1) & (2): Check behaviour of optional/required verification when the
trusted CA chain is empty.
(3) & (4): Check behaviour of optional/required verification when the
client receives a server certificate with an unsupported curve.
2017-06-07 11:26:59 +01:00
Hanno Becker
39ae8cd207 Fix implementation of VERIFY_OPTIONAL verification mode
This commit changes the behaviour of mbedtls_ssl_parse_certificate
to make the two authentication modes MBEDTLS_SSL_VERIFY_REQUIRED and
MBEDTLS_SSL_VERIFY_OPTIONAL be in the following relationship:

    Mode == MBEDTLS_SSL_VERIFY_REQUIRED
<=> Mode == MBEDTLS_SSL_VERIFY_OPTIONAL + check verify result

Also, it changes the behaviour to perform the certificate chain
verification even if the trusted CA chain is empty. Previously, the
function failed in this case, even when using optional verification,
which was brought up in #864.
2017-06-07 11:13:19 +01:00
Manuel Pégourié-Gonnard
383a118338 Merge remote-tracking branch 'gilles/IOTSSL-1330/development' into development
* gilles/IOTSSL-1330/development:
  Changelog entry for the bug fixes
  SSLv3: when refusing renegotiation, stop processing
  Ignore failures when sending fatal alerts
  Cleaned up double variable declaration
  Code portability fix
  Added changelog entry
  Send TLS alerts in many more cases
  Skip all non-executables in run-test-suites.pl
  SSL tests: server requires auth, client has no certificate
  Balanced braces across preprocessor conditionals
  Support setting the ports on the command line
2017-06-06 19:22:41 +02:00