Context: There are two public key writing functions in Mbed TLS. First,
mbedtls_pk_write_pubkey(), which exports a public key in the form of a
SubjectPublicKey structure containing the raw keying material
(for example, EC point coordinates for an EC public key, without
reference to the underlying curve). Secondly, mbedtls_pk_write_pubkey_der(),
which exports a public key in the form of a SubjectPublicKeyInfo structure,
wrapping the SubjectPublicKey structure by additional information
identifying the type of public key (and for ECC, e.g., it'd also contain
the ECC group identifier). The implementation of mbedtls_pk_write_pubkey_der()
calls mbedtls_pk_write_pubkey() first and then adds the corresponding
algorithm identifier wrapper.
Both of these functions need to be provided for PSA-based opaque PK contexts,
based on PSA's public key export function.
Previously, PSA used the SubjectPublicKeyInfo structure as its export format,
so mbedtls_pk_write_pubkey_der() could be easily implemented, while
mbedtls_pk_write_pubkey() would need to trim the output of the PSA export.
The previous implementation of mbedtls_pk_write_pubkey() is not quite right
because it calls PSA export doesn't do any trimming, hence exporting the large
SubjectPublicKeyInfo structure instead of the small SubjectPublicKey.
mbedtls_pk_write_pubkey_der(), in turn, immediately returns after calling
mbedtls_pk_write_pubkey(), hence also returning the SubjectPublicKeyInfo
structure, which is correct.
By now, the PSA public key export format has changed to the smaller
SubjectPublicKey structure. This means that, now, mbedtls_pk_write_pubkey()
can be implemented by just calling the PSA export, and that
mbedtls_pk_write_pubkey_der() needs to add the algorithm information around
it, just as in the other types of PK contexts. While not correct for the
old format, the existing code for mbedtls_pk_write_pubkey() is therefore
correct for the new PSA public key format, and needs no change apart from
the missing pointer shift in the last commit.
The implementation of mbedtls_pk_write_pubkey_der() needs a special code
path for PSA-based opaque PK contexts, as the PK context only contains
the PSA key handle, and the PSA API needs to be used to extract the
underlying EC curve to be able to write the AlgorithmParameter structure
that's part of the SubjectPublicKeyInfo structure.
That's what this commit does, (hopefully) making both
mbedtls_pk_write_pubkey() and mbedtls_pk_write_pubkey_der() export
the correctly formatted public key based on the new PSA public key format.
In mbedtls_mpi_exp_mod(), the limit check on wsize is never true when
MBEDTLS_MPI_WINDOW_SIZE is at least 6. Wrap in a preprocessor guard
to remove the dead code and resolve a Coverity finding from the
DEADCODE checker.
Change-Id: Ice7739031a9e8249283a04de11150565b613ae89
Additional changes to temporarily enable running tests:
ssl_srv.c and test_suite_ecdh use mbedtls_ecp_group_load instead of
mbedtls_ecdh_setup
test_suite_ctr_drbg uses mbedtls_ctr_drbg_update instead of
mbedtls_ctr_drbg_update_ret
Previously, PSA used SubjectPublicKeyInfo structures to serialize EC public keys.
This has recently been changed to using ECPoint structures instead, but the wrapper
making PSA ECDSA verification available through Mbed TLS' PK API hasn't yet been
adapted accordingly - which is what this commit does.
Luckily, Mbed TLS' PK API offers two functions mbedtls_pk_write_pubkey()
and mbedtls_pk_write_pubkey_der(), the latter exporting a SubjectPublicKeyInfo
structure and the former exporting an ECPoint structure in case of EC public
keys. For the adaptation of the ECDSA wrapper ecdsa_verify_wrap() it is therefore
sufficient to use mbedtls_pk_write_pubkey() instead of mbedtls_pk_write_pubkey_der().
The file oid.c had conditional inclusion of functions based on a config.h
define that belongs to X.509, which is backwards. For now, just include those
functions unconditionally and rely on the linker to garbage-collect them if
not used.
In the longer term X.509-specific functions are likely to be removed from
libmbedcrypto, but at this step the goal is to preserve the API (and even ABI)
of libmbedcrypto for as long as possible while separating the source trees of
Mbed Crypto and Mbed TLS.
As agreed during the workshop, temporarily move definitions to oid.h even if
they might not semantically belong here, as a short-term measure allowing to
build libmbecrypto on its own (without X.509 files present in the source tree)
but still provide all the things Mbed TLS currently expects, and more
specifically preserve the API and ABI exposed by libmbedtls.
Return the error code if failed, instead of returning value `1`.
If not failed, return the call of the underlying function,
in `mbedtls_ecdsa_genkey()`.
Use `cmake -D CMAKE_BUILD_TYPE=Asan` rather than manually setting
`-fsanitize=address`. This lets cmake determine the necessary compiler
and linker flags.
With UNSAFE_BUILD on, force -Wno-error. This is necessary to build
with MBEDTLS_TEST_NULL_ENTROPY.
mbedtls_mpi_read_binary() calls memcpy() with the source pointer being
the source pointer passed to mbedtls_mpi_read_binary(), the latter may
be NULL if the buffer length is 0 (and this happens e.g. in the ECJPAKE
test suite). The behavior of memcpy(), in contrast, is undefined when
called with NULL source buffer, even if the length of the copy operation
is 0.
This commit fixes this by explicitly checking that the source pointer is
not NULL before calling memcpy(), and skipping the call otherwise.
Context: The function `mbedtls_mpi_fill_random()` uses a temporary stack
buffer to hold the random data before reading it into the target MPI.
Problem: This is inefficient both computationally and memory-wise.
Memory-wise, it may lead to a stack overflow on constrained devices with
limited stack.
Fix: This commit introduces the following changes to get rid of the
temporary stack buffer entirely:
1. It modifies the call to the PRNG to output the random data directly
into the target MPI's data buffer.
This alone, however, constitutes a change of observable behaviour:
The previous implementation guaranteed to interpret the bytes emitted by
the PRNG in a big-endian fashion, while rerouting the PRNG output into the
target MPI's limb array leads to an interpretation that depends on the
endianness of the host machine.
As a remedy, the following change is applied, too:
2. Reorder the bytes emitted from the PRNG within the target MPI's
data buffer to ensure big-endian semantics.
Luckily, the byte reordering was already implemented as part of
`mbedtls_mpi_read_binary()`, so:
3. Extract bigendian-to-host byte reordering from
`mbedtls_mpi_read_binary()` to a separate internal function
`mpi_bigendian_to_host()` to be used by `mbedtls_mpi_read_binary()`
and `mbedtls_mpi_fill_random()`.
The calls to cipher_finish didn't actually do anything:
- the cipher mode is always ECB
- in that case cipher_finish() only sets *olen to zero, and returns either 0
or an error depending on whether there was pending data
- olen is a local variable in the caller, so setting it to zero right before
returning is not essential
- the return value of cipher_finis() was not checked by the caller so that's
not useful either
- the cipher layer does not have ALT implementations so the behaviour
described above is unconditional on ALT implementations (in particular,
cipher_finish() can't be useful to hardware as (with ECB) it doesn't call any
functions from lower-level modules that could release resources for example)
Since the calls are causing issues with parameter validation, and were no
serving any functional purpose, it's simpler to just remove them.
Somehow, mbedtls_sha256_ret() is defined even if MBEDTLS_SHA256_ALT
is set, and it is using SHA256_VALIDATE_RET. The documentation should
be enhanced to indicate that MBEDTLS_SHA256_ALT does _not_ replace
the entire module, but only the core SHA-256 functions.
Somehow, mbedtls_sha512_ret() is defined even if MBEDTLS_SHA512_ALT
is set, and it is using SHA512_VALIDATE_RET. The documentation should
be enhanced to indicate that MBEDTLS_SHA512_ALT does _not_ replace
the entire module, but only the core SHA-512 functions.
Somehow, mbedtls_sha1_ret() is defined even if MBEDTLS_SHA1_ALT
is set, and it is using SHA1_VALIDATE_RET. The documentation should
be enhanced to indicate that MBEDTLS_SHA1_ALT does _not_ replace
the entire module, but only the core SHA-1 functions.
Document when a context must be initialized or not, when it must be
set up or not, and whether it needs a private key or a public key will
do.
The implementation is sometimes more liberal than the documentation,
accepting a non-set-up context as a context that can't perform the
requested information. This preserves backward compatibility.
The MPI_VALIDATE_RET() macro cannot be used for parameter
validation of mbedtls_mpi_lsb() because this function returns
a size_t.
Use the underlying MBEDTLS_INTERNAL_VALIDATE_RET() insteaed,
returning 0 on failure.
Also, add a test for this behaviour.
A 0-length buffer for the key is a legitimate edge case. Ensure that
it works, even with buf=NULL. Document the key and keylen parameters.
There are already test cases for parsing an empty buffer. A subsequent
commit will add tests for writing to an empty buffer.
Add checks for null pointers under MBEDTLS_CHECK_PARAMS.
In functions that perform operations with a context, only check if the
context pointer is non-null under MBEDTLS_CHECK_PARAMS. In the default
configuration, unconditionally dereference the context pointer.
In functions that query a context, support NULL as a
pointer-to-context argument, and return the same value as for a
context which has been initialized but not set up.
- The validity of the input and output parameters is checked by
parameter validation.
- A PRNG is required in public mode only (even though it's also
recommended in private mode), so move the check to the
corresponding branch.
The check was already done later when calling ECB, (as evidenced by the tests
passing, which have a call with data_unit set to NULL), but it's more readable
to have it here too, and more helpful when debugging.
The function called through the macro MBEDTLS_PARAM_FAILED() must be supplied by
users and makes no sense as a library function, apart from debug and test.
Function calls to alternative implementations have to follow certain
rules in order to preserve correct functionality. To avoid accidentally
breaking these rules we state them explicitly in the ECP module for
ourselves and every contributor to see.
We initialized the ECC hardware before calling
mbedtls_ecp_mul_shortcuts(). This in turn calls
mbedtls_ecp_mul_restartable(), which initializes and frees the hardware
too. This issue has been introduced by recent changes and caused some
accelerators to hang.
We move the initialization after the mbedtle_ecp_mul_shortcuts() calls
to avoid double initialization.
The SSL module accesses ECDH context members directly. This can't work
with the new context, where we can't make any assumption about the
implementation of the context.
This commit makes use of the new functions to avoid accessing ECDH
members directly. The only members that are still accessed directly are
the group ID and the point format and they are independent from the
implementation.
The SSL module accesses ECDH context members directly to print debug
information. This can't work with the new context, where we can't make
assumptions about the implementation of the context. This commit adds
new debug functions to complete the encapsulation of the ECDH context
and work around the problem.
The functionality from public API functions are moved to
`xxx_internal()` functions. The public API functions are modified to do
basic parameter validation and dispatch the call to the right
implementation.
There is no intended change in behaviour when
`MBEDTLS_ECDH_LEGACY_CONTEXT` is enabled.
In the future we want to support alternative ECDH implementations. We
can't make assumptions about the structure of the context they might
use, and therefore shouldn't access the members of
`mbedtls_ecdh_context`.
Currently the lifecycle of the context can't be done without direct
manipulation. This commit adds `mbedtls_ecdh_setup()` to complete
covering the context lifecycle with functions.
`mbedtls_ecp_tls_read_group()` both parses the group ID and loads the
group into the structure provided. We want to support alternative
implementations of ECDH in the future and for that we need to parse the
group ID without populating an `mbedtls_ecp_group` structure (because
alternative implementations might not use that).
This commit moves the part that parses the group ID to a new function.
There is no need to test the new function directly, because the tests
for `mbedtls_ecp_tls_read_group()` are already implicitly testing it.
There is no intended change in behaviour in this commit.
Refactor `mpi_write_hlp()` to not be recursive, to fix stack overflows.
Iterate over the `mbedtls_mpi` division of the radix requested,
until it is zero. Each iteration, put the residue in the next LSB
of the output buffer. Fixes#2190
Refactor mbedtls_ctr_drbg_update_seed_file and
mbedtls_hmac_drbg_update_seed_file to make the error logic clearer.
The new code does not use fseek, so it works with non-seekable files.
Return early from mbedtls_pk_write_pubkey_der - public opaque key
exporting is expected to contain all of the needed data, therefore it shouldn't
be written again.
There is a probability that r will be encoded as 31 or less bytes in DER,
so additional padding is added in such case.
Added a signature-part extraction function to tidy up the code further.
Reasons:
- For the first release, we attempt to support TLS-1.2 only,
- At least TLS-1.0 is known to not work at the moment, as
for CBC ciphersuites the code in mbedtls_ssl_decrypt_buf()
and mbedtls_ssl_encrypt_buf() assumes that mbedtls_cipher_crypt()
updates the structure field for the IV in the cipher context,
which the PSA-based implementation currently doesn't.
This commit modifies the default SSL ticket implementation
from `library/ssl_ticket.c` to use PSA-based cipher context
for ticket creation and parsing.
As in mbedtls_ssl_derive_keys() adapted in an earlier commit,
we allow fallback to the ordinary mbedtls_cipher_setup()
if the provided cipher is not known. We do this even though
we always call mbedtls_ssl_ticket_setup() with AES-GCM
in our own code since this function is public and might
be used with other ciphers by users.
This commit changes the code path in mbedtls_ssl_derive_keys()
responsible for setting up record protection cipher contexts
to attempt to use the new API mbedtls_cipher_setup_psa() in
case MBEDTLS_USE_PSA_CRYPTO is set.
For that, the AEAD tag length must be provided, which is already
computed earlier in mbedtls_ssl_derive_keys() and only needs to be
stored a function scope to be available for mbedtls_cipher_setup_psa().
If mbedtls_cipher_setup_psa() fails cleanly indicating that the
requested cipher is not supported in PSA, we fall through to
the default setup using mbedtls_cipher_setup(). However, we print
a debug message in this case, to allow catching the fallthrough in
tests where we know we're using a cipher which should be supported
by PSA.
mbedtls_cipher_setup_psa() should return
MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE when the requested
cipher is not supported by PSA, so that the caller can
try the original mbedtls_cipher_setup() instead.
The previous version of mbedtls_cipher_setup_psa(), however,
only attempted to translate the cipher mode (GCM, CCM, CBC,
ChaChaPoly, Stream), but didn't consider the underlying
cipher primitive. Hence, it wouldn't fail when attempting
to setup a cipher context for, say, 3DES-CBC, where CBC
is currently supported by PSA but 3DES isn't.
This commit adds a check to mbedtls_cipher_setup_psa()
for whether the requested cipher primitive is available
in the underlying PSA Crypto implementation, and fails
cleanly with MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE if
it is isn't.
For AEAD ciphers, the information contained in mbedtls_cipher_info
is not enough to deduce a PSA algorithm value of type psa_algorithm_t.
This is because mbedtls_cipher_info doesn't contain the AEAD tag
length, while values of type psa_algorithm_t do.
This commit adds the AEAD tag length as a separate parameter
to mbedtls_cipher_setup_psa(). For Non-AEAD ciphers, the value
must be 0.
This approach is preferred over passing psa_algorithm_t directly
in order to keep the changes in existing code using the cipher layer
small.
Mbed TLS cipher layer allows usage of keys for other purposes
than indicated in the `operation` parameter of `mbedtls_cipher_setkey()`.
The semantics of the PSA Crypto API, in contrast, checks key
usage against the key policy.
As a remedy, this commit modifies the PSA key slot setup to
always allow both encryption and decryption.
This commit implements the internal key slot management performed
by PSA-based cipher contexts. Specifically, `mbedtls_cipher_setkey()`
wraps the provided raw key material into a key slot, and
`mbedtls_cipher_free()` destroys that key slot.
This field determines whether a cipher context should
use an external implementation of the PSA Crypto API for
cryptographic operations, or Mbed TLS' own crypto library.
The commit also adds dummy implementations for the cipher API.
It's better for names in the API to describe the "what" (opaque keys) rather
than the "how" (using PSA), at least since we don't intend to have multiple
function doing the same "what" in different ways in the foreseeable future.
Unfortunately the can_do wrapper does not receive the key context as an
argument, so it cannot check psa_get_key_information(). Later we might want to
change our internal structures to fix this, but for now we'll just restrict
opaque PSA keys to be ECDSA keypairs, as this is the only thing we need for
now. It also simplifies testing a bit (no need to test each key type).
The code maintains the invariant that raw and opaque PSKs are never
configured simultaneously, so strictly speaking `ssl_conf_remove_psk()`
need not consider clearing the raw PSK if it has already cleared an
opaque one - and previously, it didn't. However, it doesn't come at
any cost to keep this check as a safe-guard to future unforeseen
situations where opaque and raw PSKs _are_ both present.
In multiple places, it occurrs as the fixed length of
the master secret, so use a constant with a descriptive
name instead. This is reinforced by the fact the some
further occurrences of '48' are semantically different.
Using finer grained control over include directories will allow differnt
targets to use different include files. This will be useful when the
`crypto` subcomponent wants to use its own include files instead of or in
addition to the top level ones.
In mbedtls_mpi_write_binary, avoid leaking the size of the number
through timing or branches, if possible. More precisely, if the number
fits in the output buffer based on its allocated size, the new code's
trace doesn't depend on the value of the number.
Deprecate the module-specific XXX_HW_ACCEL_FAILED and
XXX_FEATURE_UNAVAILABLE errors, as alternative implementations should now
return `MBEDTLS_ERR_PLATFORM_HW_FAILED` and
`MBEDTLS_ERR_PLATFORM_FEATURE_UNSUPPORTED`.
ssl_write_handshake_msg() includes the assertion that
`ssl->handshake != NULL` when handling a record which is
(a) a handshake message, and NOT
(b) a HelloRequest.
However, it later calls `ssl_append_flight()` for any
record different from a HelloRequest handshake record,
that is, records satisfying !(a) || !(b), instead of
(a) && !(b) as covered by the assertion (specifically,
CCS or Alert records).
Since `ssl_append_flight()` assumes that `ssl->handshake != NULL`,
this rightfully triggers static analyzer warnings.
This commit expands the scope of the assertion to check
that `ssl->handshake != NULL` for any record which is not
a HelloRequest.
Revert changes for checking whether `MBEDTLS_ECP_RESTARTABLE`
is defined, since it broke the CI. The context is used whether the
restartable feature is defined or not.
1. Checge to check for `MBEDTLS_ECP_RESTARTABLE` for all definitions
of `rs_ctx`.
2. Remove checks for `_ALT` when using `rs_ctx` as they cannot coexist
with the Restartable configuration.
Previously, when checking whether a CRT was revoked through
one of the configured CRLs, the library would only consider
those CRLs whose `issuer` field binary-matches the `subject`
field of the CA that has issued the CRT in question. If those
fields were not binary equivalent, the corresponding CRL was
discarded.
This is not in line with RFC 5280, which demands that the
comparison should be format- and case-insensitive. For example:
- If the same string is once encoded as a `PrintableString` and
another time as a `UTF8String`, they should compare equal.
- If two strings differ only in their choice of upper and lower case
letters, they should compare equal.
This commit fixes this by using the dedicated x509_name_cmp()
function to compare the CRL issuer with the CA subject.
Fixes#1784.
library/certs.c provides some hardcoded certificates that
are used e.g. by the test applications ssl_server2, ssl_client2
in case no certificates are provided on the command line.
The certificates used are from the tests/data_files folder
and have been updated in the latest commits. This commit
updates their copies in certs.c. It also adds comments
indicating the files from which the data is taken, in
order to ease update in the future.
Previous commits have added or modified build instructions for
server1*, server2*, server5*, test-ca*, cli-rsa* in the Makefile
tests/data_files/Makefile, or the apps they invoke have been changed.
This commit regenerates those files to make sure they are in match with
the build instructions.
Previously, when checking whether a CRT was revoked through
one of the configured CRLs, the library would only consider
those CRLs whose `issuer` field binary-matches the `subject`
field of the CA that has issued the CRT in question. If those
fields were not binary equivalent, the corresponding CRL was
discarded.
This is not in line with RFC 5280, which demands that the
comparison should be format- and case-insensitive. For example:
- If the same string is once encoded as a `PrintableString` and
another time as a `UTF8String`, they should compare equal.
- If two strings differ only in their choice of upper and lower case
letters, they should compare equal.
This commit fixes this by using the dedicated x509_name_cmp()
function to compare the CRL issuer with the CA subject.
Fixes#1784.
Return the condition compilation flags surrounding
`mbedtls_ecdh_compute_shared()`, `mbedtls_ecdh_gen_public()`,
`mbedtls_ecdsa_sign()` and `mbedtls_ecdsa_verify()` that were accidentally
removed in a previous merge.
Resolves#2163
This commit modifies a bounds check in `mbedtls_ecp_check_budget()` to
be correct even if the requested number of ECC operations would overflow
the operation counter.
Use `( x >> y ) & z` instead of `x >> y & z`. Both are equivalent
by operator precedence, but the former is more readable and the
commonly used idiom in the library.
Correct a typo in an AES XTS implementation comment where the relevant
NIST standard was incorrectly referred to as NIST 80-38E instead of NIST
800-38E.
It is inaccurate to call a data unit a "sector". A disk sector is a
common use case for the data unit, but there exist other types of data
units that are not sectors.
It turns out that in some environments MinGW esposes a non-conforming
(v)snprintf behavior despite the tests against the MSVC runtime pass.
Therefore it has been included in the "non-conforming" scope alongide
older MSVC rungime environments.
This commit fixes issue #1212 related to platform-specific entropy
polling in an syscall-emulated environment.
Previously, the implementation of the entropy gathering function
`mbedtls_platform_entropy_poll()` for linux machines used the
following logic to determine how to obtain entropy from the kernel:
1. If the getrandom() system call identifier SYS_getrandom is present and
the kernel version is 3.17 or higher, use syscall( SYS_getrandom, ... )
2. Otherwise, fall back to reading from /dev/random.
There are two issues with this:
1. Portability:
When cross-compiling the code for a different
architecture and running it through system call
emulation in qemu, qemu reports the host kernel
version through uname but, as of v.2.5.0,
doesn't support emulating the getrandom() syscall.
This leads to `mbedtls_platform_entropy_poll()`
failing even though reading from /dev/random would
have worked.
2. Style:
Extracting the linux kernel version from
the output of `uname` is slightly tedious.
This commit fixes both by implementing the suggestion in #1212:
- It removes the kernel-version detection through uname().
- Instead, it checks whether `syscall( SYS_getrandom, ... )`
fails with errno set to ENOSYS indicating an unknown system call.
If so, it falls through to trying to read from /dev/random.
Fixes#1212.
This commit changes the behavior of the record decryption routine
`ssl_decrypt_buf()` in the following situation:
1. A CBC ciphersuite with Encrypt-then-MAC is used.
2. A record with valid MAC but invalid CBC padding is received.
In this situation, the previous code would not raise and error but
instead forward the decrypted packet, including the wrong padding,
to the user.
This commit changes this behavior to return the error
MBEDTLS_ERR_SSL_INVALID_MAC instead.
While erroneous, the previous behavior does not constitute a
security flaw since it can only happen for properly authenticated
records, that is, if the peer makes a mistake while preparing the
padded plaintext.
This commit duplicates the public function mbedtls_asn1_find_named_data()
defined in library/asn1parse.c within library/asn1write.c in order to
avoid a dependency of the ASN.1 writing module on the ASN.1 parsing module.
The duplication is unproblematic from a semantic and an efficiency
perspective becasue it is just a short list traversal that doesn't
actually do any ASN.1 parsing.
Previously, mbedtls_pkcs5_pbes2() was unconditionally declared
in `pkcs5.h` but defined as a stub returning
`MBEDTLS_ERR_PKCS5_FEATURE_UNAVAILABLE` in case
MBEDTLS_ASN1_PARSE_C was not defined.
In line with the previous commits, this commit removes declaration
and definition from both `pkcs5.h` and `pkcs5.c` in case
MBEDTLS_ASN1_PARSE_C is not defined.
Our API makes no guarantee that you can use a context after free()ing it
without re-init()ing it first, so better not give the wrong impression that we
do, while it's not policy and the rest of the code might not allow it.
Rename the PLATFORM HW error, to avoid ABI breakage with Mbed OS.
The value changed as well, as previous value was not in the range of
Mbed TLS low level error codes.
* development:
ssl-opt.sh: change expected output for large srv packet test with SSLv3
Adapt ChangeLog
Fix bug in SSL ticket implementation removing keys of age < 1s
ssl-opt.sh: Add DTLS session resumption tests
Add ChangeLog entry
Fix typo
Fix hmac_drbg failure in benchmark, with threading
Remove trailing whitespace
Remove trailing whitespace
ssl_server2: add buffer overhead for a termination character
Add missing large and small packet tests for ssl_server2
Added buffer_size and response_size options for ssl-server2. Added appropriate tests.
Solving a conflict in tests/ssl-opt.sh: two set of tests were added at the
same place (just after large packets):
- restartable ECC tests (in this branch)
- server-side large packets (in development)
Resolution was to move the ECC tests after the newly added server large packet
ones.
The code assumed that `int x = - (unsigned) u` with 0 <= u < INT_MAX
sets `x` to the negative of u, but actually this calculates
(UINT_MAX - u) and then converts this value to int, which overflows.
Cast to int before applying the unary minus operator to guarantee the
desired behavior.
The code was making two unsequenced reads from volatile locations.
This is undefined behavior. It was probably harmless because we didn't
care in what order the reads happened and the reads were from ordinary
memory, but UB is UB and IAR8 complained.
This commit ensures that buffers holding fragmented or
future handshake messages get zeroized before they are
freed when the respective handshake message is no longer
needed. Previously, the handshake message content would
leak on the heap.
Context: This commit makes a change to mbedtls_pk_parse_key() which
is responsible for parsing of private keys. The function doesn't know
the key format in advance (PEM vs. DER, encrypted vs. unencrypted) and
tries them one by one, resetting the PK context in between.
Issue: The previous code resets the PK context through a call to
mbedtls_pk_free() along, lacking the accompanying mbedtls_pk_init()
call. Practically, this is not an issue because functionally
mbedtls_pk_free() + mbedtls_pk_init() is equivalent to mbedtls_pk_free()
with the current implementation of these functions, but strictly
speaking it's nonetheless a violation of the API semantics according
to which xxx_free() functions leave a context in uninitialized state.
(yet not entirely random, because xxx_free() functions must be idempotent,
so they cannot just fill the context they operate on with garbage).
Change: The commit adds calls to mbedtls_pk_init() after those calls
to mbedtls_pk_free() within mbedtls_pk_parse_key() after which the
PK context might still be used.
This commit removes the definition of the API function
`mbedtls_platform_set_calloc_free()`
from `library/platform.c` in case the macros
`MBEDTLS_PLATFORM_CALLOC_MACRO`
`MBEDTLS_PLATFORM_FREE_MACRO`
for compile time configuration of calloc/free are set.
This is in line with the corresponding header `mbedtls/platform.h`
which declares `mbedtls_platform_set_calloc_free()` only if
`MBEDTLS_PLATFORM_{CALLOC/FREE}_MACRO` are not defined.
Fixes#1642.
The previous code triggered a compiler warning because of a comparison
of a signed and an unsigned integer.
The conversion is safe because `len` is representable by 16-bits,
hence smaller than the maximum integer.
When a random number is generated for the Miller-Rabin primality test,
if the bit length of the random number is larger than the number being
tested, the random number is shifted right to have the same bit length.
This introduces bias, as the random number is now guaranteed to be
larger than 2^(bit length-1).
Changing this to instead zero all bits higher than the tested numbers
bit length will remove this bias and keep the random number being
uniformly generated.
When using a primality testing function the tolerable error rate depends
on the scheme in question, the required security strength and wether it
is used for key generation or parameter validation. To support all use
cases we need more flexibility than what the old API provides.
The input distribution to primality testing functions is completely
different when used for generating primes and when for validating
primes. The constants used in the library are geared towards the prime
generation use case and are weak when used for validation. (Maliciously
constructed composite numbers can pass the test with high probability)
The mbedtls_mpi_is_prime() function is in the public API and although it
is not documented, it is reasonable to assume that the primary use case
is validating primes. The RSA module too uses it for validating key
material.
The FIPS 186-4 RSA key generation prescribes lower failure probability
in primality testing and this makes key generation slower. We enable the
caller to decide between compliance/security and performance.
This python script calculates the base two logarithm of the formulas in
HAC Fact 4.48 and was used to determine the breakpoints and number of
rounds:
def mrpkt_log_2(k, t):
if t <= k/9.0:
return 3*math.log(k,2)/2+t-math.log(t,2)/2+4-2*math.sqrt(t*k)
elif t <= k/4.0:
c1 = math.log(7.0*k/20,2)-5*t
c2 = math.log(1/7.0,2)+15*math.log(k,2)/4.0-k/2.0-2*t
c3 = math.log(12*k,2)-k/4.0-3*t
return max(c1, c2, c3)
else:
return math.log(1/7.0)+15*math.log(k,2)/4.0-k/2.0-2*t
In the previous bounds check `(*p) > end - len`, the computation
of `end - len` might underflow if `end` is within the first 64KB
of the address space (note that the length `len` is controlled by
the peer). In this case, the bounds check will be bypassed, leading
to `*p` exceed the message bounds by up to 64KB when leaving
`ssl_parse_server_psk_hint()`. In a pure PSK-based handshake,
this doesn't seem to have any consequences, as `*p*` is not accessed
afterwards. In a PSK-(EC)DHE handshake, however, `*p` is read from
in `ssl_parse_server_ecdh_params()` and `ssl_parse_server_dh_params()`
which might lead to an application crash of information leakage.
Change the secondary X509 CSR parsing call for the alternative MS header to only
occur if the first call fails due to the header being unfound, instead of any
call.
Add support for RFC7468, and the alternative Microsoft footer/headers for CSR's
that contain the text 'BEGIN NEW CERTIFICATE REQUEST' instead of
'BEGIN CERTIFICATE REQUEST'.
Get rid of the variable p. This makes it more apparent where the code
accesses the buffer at an offset whose value is sensitive.
No intended behavior change in this commit.
Rather than doing the quadratic-time constant-memory-trace on the
whole working buffer, do it on the section of the buffer where the
data to copy has to lie, which can be significantly smaller if the
output buffer is significantly smaller than the working buffer, e.g.
for TLS RSA ciphersuites (48 bytes vs MBEDTLS_MPI_MAX_SIZE).
In mbedtls_rsa_rsaes_pkcs1_v15_decrypt, use size_greater_than (which
is based on bitwise operations) instead of the < operator to compare
sizes when the values being compared must not leak. Some compilers
compile < to a branch at least under some circumstances (observed with
gcc 5.4 for arm-gnueabi -O9 on a toy program).
Replace memmove(to, to + offset, length) by a functionally equivalent
function that strives to make the same memory access patterns
regardless of the value of length. This fixes an information leak
through timing (especially timing of memory accesses via cache probes)
that leads to a Bleichenbacher-style attack on PKCS#1 v1.5 decryption
using the plaintext length as the observable.
mbedtls_rsa_rsaes_pkcs1_v15_decrypt takes care not to reveal whether
the padding is valid or not, even through timing or memory access
patterns. This is a defense against an attack published by
Bleichenbacher. The attacker can also obtain the same information by
observing the length of the plaintext. The current implementation
leaks the length of the plaintext through timing and memory access
patterns.
This commit is a first step towards fixing this leak. It reduces the
leak to a single memmove call inside the working buffer.
Make the function more robust by taking an arbitrary zero/nonzero
argument instead of insisting on zero/all-bits-one. Update and fix its
documentation.
stdio.h was being included both conditionally if MBEDTLS_FS_IO was
defined, and also unconditionally, which made at least one of them
redundant.
This change removes the unconditional inclusion of stdio.h and makes it
conditional on MBEDTLS_PLATFORM_C.
mbedtls_rsa_rsaes_pkcs1_v15_decrypt took care of calculating the
padding length without leaking the amount of padding or the validity
of the padding. However it then skipped the copying of the data if the
padding was invalid, which could allow an adversary to find out
whether the padding was valid through precise timing measurements,
especially if for a local attacker who could observe memory access via
cache timings.
Avoid this leak by always copying from the decryption buffer to the
output buffer, even when the padding is invalid. With invalid padding,
copy the same amount of data as what is expected on valid padding: the
minimum valid padding size if this fits in the output buffer,
otherwise the output buffer size. To avoid leaking payload data from
an unsuccessful decryption, zero the decryption buffer before copying
if the padding was invalid.
It should be valid to RSASSA-PSS sign a SHA-512 hash with a 1024-bit or
1032-bit RSA key, but with the salt size being always equal to the hash
size, this isn't possible: the key is too small.
To enable use of hashes that are relatively large compared to the key
size, allow reducing the salt size to no less than the hash size minus 2
bytes. We don't allow salt sizes smaller than the hash size minus 2
bytes because that too significantly changes the security guarantees the
library provides compared to the previous implementation which always
used a salt size equal to the hash size. The new calculated salt size
remains compliant with FIPS 186-4.
We also need to update the "hash too large" test, since we now reduce
the salt size when certain key sizes are used. We used to not support
1024-bit keys with SHA-512, but now we support this by reducing the salt
size to 62. Update the "hash too large" test to use a 1016-bit RSA key
with SHA-512, which still has too large of a hash because we will not
reduce the salt size further than 2 bytes shorter than the hash size.
The RSA private key used for the test was generated using "openssl
genrsa 1016" using OpenSSL 1.1.1-pre8.
$ openssl genrsa 1016
Generating RSA private key, 1016 bit long modulus (2 primes)
..............++++++
....++++++
e is 65537 (0x010001)
-----BEGIN RSA PRIVATE KEY-----
MIICVwIBAAKBgACu54dKTbLxUQBEQF2ynxTfDze7z2H8vMmUo9McqvhYp0zI8qQK
yanOeqmgaA9iz52NS4JxFFM/2/hvFvyd/ly/hX2GE1UZpGEf/FnLdHOGFhmnjj7D
FHFegEz/gtbzLp9X3fOQVjYpiDvTT0Do20EyCbFRzul9gXpdZcfaVHNLAgMBAAEC
gYAAiWht2ksmnP01B2nF8tGV1RQghhUL90Hd4D/AWFJdX1C4O1qc07jRBd1KLDH0
fH19WocLCImeSZooGCZn+jveTuaEH14w6I0EfnpKDcpWVAoIP6I8eSdAttrnTyTn
Y7VgPrcobyq4WkCVCD/jLUbn97CneF7EHNspXGMTvorMeQJADjy2hF5SginhnPsk
YR5oWawc6n01mStuLnloI8Uq/6A0AOQoMPkGl/CESZw+NYfe/BnnSeckM917cMKL
DIKAtwJADEj55Frjj9tKUUO+N9eaEM1PH5eC7yakhIpESccs/XEsaDUIGHNjhctK
mrbbWu+OlsVRA5z8yJFYIa7gae1mDQJABjtQ8JOQreTDGkFbZR84MbgCWClCIq89
5R3DFZUiAw4OdS1o4ja+Shc+8DFxkWDNm6+C63g/Amy5sVuWHX2p9QI/a69Cxmns
TxHoXm1w9Azublk7N7DgB26yqxlTfWJo+ysOFmLEk47g0ekoCwLPxkwXlYIEoad2
JqPh418DwYExAkACcqrd9+rfxtrbCbTXHEizW7aHR+fVOr9lpXXDEZTlDJ57sRkS
SpjXbAmylqQuKLqH8h/72RbiP36kEm5ptmw2
-----END RSA PRIVATE KEY-----
Setting the dh_flag to 1 used to indicate that the caller requests safe
primes from mbedtls_mpi_gen_prime. We generalize the functionality to
make room for more flags in that parameter.
* development-restricted: (578 commits)
Update library version number to 2.13.1
Don't define _POSIX_C_SOURCE in header file
Don't declare and define gmtime()-mutex on Windows platforms
Correct preprocessor guards determining use of gmtime()
Correct documentation of mbedtls_platform_gmtime_r()
Correct typo in documentation of mbedtls_platform_gmtime_r()
Correct POSIX version check to determine presence of gmtime_r()
Improve documentation of mbedtls_platform_gmtime_r()
platform_utils.{c/h} -> platform_util.{c/h}
Don't include platform_time.h if !MBEDTLS_HAVE_TIME
Improve wording of documentation of MBEDTLS_PLATFORM_GMTIME_R_ALT
Fix typo in documentation of MBEDTLS_PLATFORM_GMTIME_R_ALT
Replace 'thread safe' by 'thread-safe' in the documentation
Improve documentation of MBEDTLS_HAVE_TIME_DATE
ChangeLog: Add missing renamings gmtime -> gmtime_r
Improve documentation of MBEDTLS_HAVE_TIME_DATE
Minor documentation improvements
Style: Add missing period in documentation in threading.h
Rename mbedtls_platform_gmtime() to mbedtls_platform_gmtime_r()
Guard decl and use of gmtime mutex by HAVE_TIME_DATE and !GMTIME_ALT
...
When writing a private EC key, use a constant size for the private
value, as specified in RFC 5915. Previously, the value was written
as an ASN.1 INTEGER, which caused the size of the key to leak
about 1 bit of information on average, and could cause the value to be
1 byte too large for the output buffer.
Previous commits attempted to use `gmtime_s()` for IAR systems; however,
this attempt depends on the use of C11 extensions which lead to incompatibility
with other pieces of the library, such as the use of `memset()` which is
being deprecated in favor of `memset_s()` in C11.
By the standard (RFC 6066, Sect. 4), the Maximum Fragment Length (MFL)
extension limits the maximum record payload size, but not the maximum
datagram size. However, not inferring any limitations on the MTU when
setting the MFL means that a party has no means to dynamically inform
the peer about MTU limitations.
This commit changes the function ssl_get_remaining_payload_in_datagram()
to never return more than
MFL - { Total size of all records within the current datagram }
thereby limiting the MTU to MFL + { Maximum Record Expansion }.
The function ssl_free_buffered_record() frees a future epoch record, if
such is present. Previously, it was called in mbedtls_handshake_free(),
i.e. an unused buffered record would be cleared at the end of the handshake.
This commit moves the call to the function ssl_buffering_free() responsible
for freeing all buffering-related data, and which is called not only at
the end of the handshake, but at the end of every flight. In particular,
future record epochs won't be buffered across flight boundaries anymore,
and they shouldn't.
The previous code appended messages to flights only if their handshake type,
as derived from the first byte in the message, was different from
MBEDTLS_SSL_HS_HELLO_REQUEST. This check should only be performed
for handshake records, while CCS records should immediately be appended.
In SSLv3, the client sends a NoCertificate alert in response to
a CertificateRequest if it doesn't have a CRT. This previously
lead to failure in ssl_write_handshake_msg() which only accepted
handshake or CCS records.
The debugging functions
- mbedtls_debug_print_ret,
- mbedtls_debug_print_buf,
- mbedtls_debug_print_mpi, and
- mbedtls_debug_print_crt
return immediately if the SSL configuration bound to the
passed SSL context is NULL, has no debugging functions
configured, or if the debug threshold is below the debugging
level.
However, they do not check whether the provided SSL context
is not NULL before accessing the SSL configuration bound to it,
therefore leading to a segmentation fault if it is.
In contrast, the debugging function
- mbedtls_debug_print_msg
does check for ssl != NULL before accessing ssl->conf.
This commit unifies the checks by always returning immediately
if ssl == NULL.
The previous code appended messages to flights only if their handshake type,
as derived from the first byte in the message, was different from
MBEDTLS_SSL_HS_HELLO_REQUEST. This check should only be performed
for handshake records, while CCS records should immediately be appended.
In SSLv3, the client sends a NoCertificate alert in response to
a CertificateRequest if it doesn't have a CRT. This previously
lead to failure in ssl_write_handshake_msg() which only accepted
handshake or CCS records.
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.
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.
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.
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.
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.
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().
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.
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.
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.
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).
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.
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).
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 !
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().
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().
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.
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.
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().
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.
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().
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.
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.
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().
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().
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().
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().
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".
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.
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.
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.
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.
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.
This commit finally enables datagram packing by modifying the
record preparation function ssl_write_record() to not always
calling mbedtls_ssl_flush_output().
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.
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().
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.
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.
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.
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
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
- 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
`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.
