mbedtls/ecp.h defines constants
MBEDTLS_ECP_PF_UNCOMPRESSED
MBEDTLS_ECP_PF_COMPRESSED
MBEDTLS_ECP_TLS_NAMED_CURVE
which regard the encoding of elliptic curves and curve point formats in TLS.
As such, they should be defined in the SSL namespace. Asides, this will help
replacing the legacy ECC crypto by alternative ECC implementations.
PEM-encoded keys with PEM header
-----BEGIN EC PRIVATE KEY-----
...
-----END EC PRIVATE KEY-----
were previously not parsed in configurations using TinyCrypt
instead of legacy ECC crypto.
The PK type MBEDTLS_PK_ECDSA is never returned from
`mbedtls_pk_info_from_type()`. Instead, EC keys either
are identified as MBEDTLS_PK_ECKEY_DH (in case they
must only be used for ECDHE) or MBEDTLS_PK_ECKEY (in
case they can be used for any algorithm).
With TinyCrypt and legacy ECC mutually exclusive, we don't have
to use #if TINYCRYPT #else #if LEGACY #endif #endif anymore, but
can add the TC and legacy based ECC implementations independently.
- TinyCrypt uses `0` for errors.
- The first argument to uECC_verify() should be the public key,
but the previous code passed the beginning of the entire
private-public key structure.
The PK-type MBEDTLS_PK_ECDSA isn't really used by the library.
Especially, when parsing a generic EC key, a PK context of type
MBEDTLS_PK_ECKEY will be requested. Hence, to drop in TinyCrypt
for the legacy-ECC implementation, the PK type that TinyCrypt
implements must be MBEDTLS_PK_ECKEY.
The SSL context maintains a set of 'out pointers' indicating the
address at which to write the header fields of the next outgoing
record. Some of these addresses have a static offset from the
beginning of the record header, while other offsets can vary
depending on the active record encryption mechanism: For example,
if an explicit IV is in use, there's an offset between the end
of the record header and the beginning of the encrypted data to
allow the explicit IV to be placed in between; also, if the DTLS
Connection ID (CID) feature is in use, the CID is part of the
record header, shifting all subsequent information (length, IV, data)
to the back.
When setting up an SSL context, the out pointers are initialized
according to the identity transform + no CID, and it is important
to keep them up to date whenever the record encryption mechanism
changes, which is done by the helper function ssl_update_out_pointers().
During context deserialization, updating the out pointers according
to the deserialized record transform went missing, leaving the out
pointers the initial state. When attemping to encrypt a record in
this state, this lead to failure if either a CID or an explicit IV
was in use. This wasn't caught in the tests by the bad luck that
they didn't use CID, _and_ used the default ciphersuite based on
ChaChaPoly, which doesn't have an explicit IV. Changing either of
this would have made the existing tests fail.
This commit fixes the bug by adding a call to ssl_update_out_pointers()
to ssl_context_load() implementing context deserialization.
Extending test coverage is left for a separate commit.
This patch fixes an issue we encountered with more stringent compiler
warnings. The signature_is_good variable has a possibility of being
used uninitialized. This patch moves the use of the variable to a
place where it cannot be used while uninitialized.
Signed-off-by: Andy Gross <andy.gross@linaro.org>
According to SP800-90A, the DRBG seeding process should use a nonce
of length `security_strength / 2` bits as part of the DRBG seed. It
further notes that this nonce may be drawn from the same source of
entropy that is used for the first `security_strength` bits of the
DRBG seed. The present HMAC DRBG implementation does that, requesting
`security_strength * 3 / 2` bits of entropy from the configured entropy
source in total to form the initial part of the DRBG seed.
However, some entropy sources may have thresholds in terms of how much
entropy they can provide in a single call to their entropy gathering
function which may be exceeded by the present HMAC DRBG implementation
even if the threshold is not smaller than `security_strength` bits.
Specifically, this is the case for our own entropy module implementation
which only allows requesting at most 32 Bytes of entropy at a time
in configurations disabling SHA-512, and this leads to runtime failure
of HMAC DRBG when used with Mbed Crypto' own entropy callbacks in such
configurations.
This commit fixes this by splitting the seed entropy acquisition into
two calls, one requesting `security_strength` bits first, and another
one requesting `security_strength / 2` bits for the nonce.
Fixes#237.
* mbedtls-2.16: (21 commits)
Exclude DTLS 1.2 only with older OpenSSL
Document the rationale for the armel build
Switch armel build to -Os
Add a build on ARMv5TE in ARM mode
Add changelog entry for ARM assembly fix
bn_mul.h: require at least ARMv6 to enable the ARM DSP code
Changelog entry for test certificates update
Change worktree_rev to HEAD for rev-parse
Add ChangeLog entry for entropy_nv_seed test case fix
entropy_nv_seed: cope with SHA-256
entropy_nv_seed: clean up properly
Add ChangeLog entry for undefined behavior fix in test_suite_nist_kw
Don't call memset after calloc
Adapt ChangeLog
ECP restart: Don't calculate address of sub ctx if ctx is NULL
Update certificates to expire in 2029
Update soon to be expired crl
Test that a shared library build produces a dynamically linked executable
Test that the shared library build with CMake works
Add a test of MBEDTLS_CONFIG_FILE
...
The NO_INLINE annotation of tls_prf_sha256() and tls_prf_sha384() from
the last commit surprisingly had an influence on ARMC5 compilation in
that tls_prf_generic() was no longer automatically inlined into
tls_prf_sha256() if only the latter was enabled (and is the point
where tls_prf_generic() is called). This commit forces inlining
of tls_prf_generic() in this case.
Usually, compilers are clever enough to pick the best inlining
strategy, but in this instance, it appears that compiling on ARMC6,
the compilers inlines xxx_prf_yyy() and xxx_calc_finished_yyy()
even though it really shouldn't. Forbid inlining through the use
of __attribute__((noinline)).
Somehow, at least ARMC5 isn't able to recognize this automatically.
Since some of the arguments to ssl_populate_transform() are compile-
time constants in reduced configurations, inlining leads to slightly
shorter code.
This saves a few bytes in configurations where only one hash
is enabled, and configurations allowing multiple hashes probably
don't care about code-size anyway.
This function is called on client-only once the ciphersuite has
been chosen and it it is known which digest the client will need
for the handshake transcript throughout the handshake, and causes
all other unneeded handshake transcripts to be discontinued.
(On the server, we cannot call this function because we don't know
which hash the client will those in its CertificateVerify message).
However, the benefit of this call is marginal, since transcript hash
computation is negligible compared to asymmetric crypto, and moreover
the handshake transcript contexts for the unused digests are still
stored in the SSL handshake parameter structure and not freed until
the end of the handshake.
Finally, if we're running on a _really_ constrained client, there
will be only one hash function enabled anyway, and in this case
the checksum optimization has no effect.
This commit therefore removes checksum optimization altogether,
saving some code on constrained systems.
- a comment regarding the implementation of hmac_drbg_reseed_core()
was misplaced.
- add more references to the standard, and add details on how the
comments in the code refer to various parts of the standard.
Now function mbedtls_ssl_set_hostname is compile-time configurable
in config.h with define MBEDTLS_X509_REMOVE_HOSTNAME_VERIFICATION.
This affects to many x509 API's. See config.h for details.
According to SP800-90A, the DRBG seeding process should use a nonce
of length `security_strength / 2` bits as part of the DRBG seed. It
further notes that this nonce may be drawn from the same source of
entropy that is used for the first `security_strength` bits of the
DRBG seed. The present HMAC DRBG implementation does that, requesting
`security_strength * 3 / 2` bits of entropy from the configured entropy
source in total to form the initial part of the DRBG seed.
However, some entropy sources may have thresholds in terms of how much
entropy they can provide in a single call to their entropy gathering
function which may be exceeded by the present HMAC DRBG implementation
even if the threshold is not smaller than `security_strength` bits.
Specifically, this is the case for our own entropy module implementation
which only allows requesting at most 32 Bytes of entropy at a time
in configurations disabling SHA-512, and this leads to runtime failure
of HMAC DRBG when used with Mbed TLS' own entropy callbacks in such
configurations.
This commit fixes this by splitting the seed entropy acquisition into
two calls, one requesting `security_strength` bits first, and another
one requesting `security_strength / 2` bits for the nonce.
The use of tinyCrypt is restricted Secp256r1-only, and a check in
ssl_ciphersuite_is_match() ensures that an EC ciphersuite is chosen
only if the client advertised support for Secp256r1, too.
In a way inconsistent with the rest of the library restricting the
use of tinyCrypt to pure-ECDHE, the previous ServerKeyExchange writing
routine would use tinyCrypt also for ECDHE-PSK-based ciphersuites.
This commit fixes this.
Previously, MBEDTLS_KEY_EXCHANGE_ECDH[E]_XXX_ENABLED would imply
that MBEDTLS_ECDH_C is set, but with the introduction of tinyCrypt
as an alternative ECDH implementation, this is no longer the case.
Eventually, all HS parsing/writing functions should take an arbitrary buffer +
length pair as their argument, and return MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL if
the provided buffer is too short. So far, we've only made a first step by
allowing to pass an arbitrary buffer, but don't yet add bounds checks
throughout. While deliberate for now, this must be clearly documented.
This makes grepping the functions more difficult, and also leads to compilation failures
when trying to build the library from a single source file (which might be useful for
code-size reasons).
IAR doesn't like `((void) var);` as a means to indicate an unused
variable if that variable hasn't been initialized before. Make it
happy by initializing the variable before.
ssl_server_key_exchange_parse() is compiled even if there's no ciphersuite
enabled which uses it (for example, that's the case in RSA-only builds).
The rationale for that is to avoid cluttering the code with numerous
compile-time guards. A consequence, however, is the top of
ssl_server_key_exchange_parse() contains declarations for variables
which are never put to use, and rightfully leading to compiler warnings.
This commit silences these warnings by putting `((void) VAR);` statements
in the branch which detects if we ever happen to call the function in an
unexpected ciphersuite.
In the PSK and RSA-PSK ciphersuites, the ServerKeyExchange message
MAY be skipped. This commit moves the code-path peeking at the
incoming message to decide whether it's probably a ServerKeyExchange
to the new coordination function ssl_server_key_exchange_coordinate().
This commit moves the code checking whether a SrvKeyExchange message
is expected or not to the new function ssl_srv_key_exchange_coordinate().
Note that the potential static DH extraction is done prior to the
coordination step.
This code moves the code-path that extracts static DH parameters
from the server's CRT (if applicable) to the new function
ssl_server_key_exchange_prepare().
This commit adds declarations and dummy implementations for
the restructured incoming server key exchange handling that
will replace the previous ssl_parse_server_key_exchange().
The entry point for the SrvKeyExchange handling that is called
from the handshake state machine is
`ssl_process_server_key_exchange()`,
splitting the processing into the following steps:
- Preparation: For a static DH key exchange, extract
DH parameters from the server's CRT.
- Coordination: Check if a SrvKeyExchange message is expected
(e.g., it isn't for a RSA-based key exchange)
- Reading: Fetch and check content and handshake type
of incoming message.
- Parsing: Parse and store the ServerKeyExchange message.
- Postprocessing: Update handstate state machine.
The subsequent commits will scatter the code from the previous
monolithic function ssl_parse_server_key_exchange() among those
dedicated functions, commenting out each part of
ssl_parse_server_key_exchange() that has already been dealt with.
This gradual progression is meant to ease reviewing. Once all
code has been moved and all changes explained,
ssl_parse_server_key_exchange() will be removed.
The postprocessing code for the server-side incoming client key
exchange and the client-side outgoing client key exchange both
contain the same code-paths for building the premaster secret
depending on the chosen ciphersuite (e.g., for ECDHE-PSK,
concatenating the ECDHE secret with the chosen PSK).
This commit moves this common code to ssl_tls.c, allowing
client- and server-side to share it.
The code from the previous function ssl_parse_client_key_exchange()
has been entirely moved to one of the newly introduced subroutines
and is no longer needed. This commit removes it.
After parsing and performing key generation operations,
the server-side incoming ClientKeyExchange handling includes
code-paths to assembly the PreMasterSecret (PMS) from the
available keying material, the exact assembly procedure
depending on which ciphersuite is in use. E.g., in an
(EC)DHE-PSK ciphersuite, the (EC)DHE secret would be concatenated
with the PSK to form the PMS.
This assembly of the PMS logically comes done after the ClientKeyExchange
has been parsed and the respective keying material has been generated,
and this commit moves it to the new postprocessing function
ssl_client_key_exchange_postprocess().
This commit moves the generation of the master secret and session keys
from the premaster secret (done in mbedtlsssl_derive_keys()) from the
previous ClientKeyExchange parsing function ssl_parse_client_key_exchange()
to the new postprocessing function ssl_client_key_exchange_postprocess().
This commit adds declarations and dummy implementations for
the restructured incoming client key exchange handling that
will replace the previous ssl_parse_client_key_exchange().
The entry point for the CliKeyExchange handling that is called
from the handshake state machine is
`ssl_process_client_key_exchange()`,
splitting the processing into the following steps:
- Fetching: Read next message from the messaging layer
and check that it has the correct type.
The ClientKeyExchange message is never
omitted, so there is no ambiguity in what
to expect, and hence no dedicated preparation
step as for other handshake states.
- Parsing: Parse the ClientKeyExchange message and
use the information in it to derive keying
material such as the shared (EC)DHE secret.
- Postprocessing:
Compute the session keys from the available
keying material. This splits in two steps:
(1) Build the PreMasterSecret (PMS) from the
available keying material, e.g. concatenate
the (EC)DHE secret with a PSK, if used.
(2) Extract the MasterSecret and Session Keys
from the PreMasterSecret.
The subsequent commits will scatter the code from the previous
monolithic function ssl_parse_client_key_exchange() among those
dedicated functions, commenting out each part of
ssl_parse_client_key_exchange() that has already been dealt with.
This gradual progression is meant to ease reviewing. Once all
code has been moved and all changes explained,
ssl_parse_client_key_exchange() will be removed.
The code from the previous function ssl_write_client_key_exchange()
has been entirely moved to one of the newly introduced subroutines
and is no longer needed. This commit removes it.
This commit moves the code responsible for
(a) generating the client's private and public (EC)DHE keys
(b) writing it to the message buffer
to the new writing function ssl_client_key_exchange_write().
As mentioned in the previous commit message, (a) and (b) are
currently inseparable at the (EC)DHE API level, which is why
(a) can't be moved to the preparation step.
For RSA or RSA-PSK exchanges, the PMS contains 46 random bytes
picked by the client. These bytes are generated prior to the
writing of the ClientKeyExchange message.
This commit splits the previous function ssl_write_encrypted_pms() into
PPMS-GEN: ssl_rsa_generate_partial_pms()
PPMS-ENC: ssl_rsa_encrypt_partial_pms().
The prefix 'partial' is meant to emphasize that the generation of the PMS
is not always entirely done by these functions: For RSA-PSK e.g., the
PSK still needs to be added.
The two calls of ssl_write_encrypted_pms() in
ssl_write_client_key_exchange() will split in calls of the functions
PPMS-GEN and PPMS-ENC each, with PPMS-GEN being moved to the new
preparation function ssl_client_key_exchange_prepare() in this commit,
and PPMS-ENC being moved to ssl_client_key_exchange_write() in the
next commit.
After and performing key generation operations,
the client-side outgoing ClientKeyExchange handling includes
code-paths to assembly the PreMasterSecret (PMS) from the
available keying material, the exact assembly procedure
depending on which ciphersuite is in use. E.g., in an
(EC)DHE-PSK ciphersuite, the (EC)DHE secret would be concatenated
with the PSK to form the PMS.
This assembly of the PMS logically can be done after the ClientKeyExchange
has been written and the respective keying material has been generated,
and this commit moves it to the new postprocessing function
ssl_client_key_exchange_postprocess().
Ideally, the PMS assembly could be done prior to writing the
ClientKeyExchange message, but the (EC)DHE API does currently
not allow splitting secret-generation and secret-export; as
long as that's the case, we to generation and exporting in the
message writing function, forcing PMS assembly to be done in
the postprocessing.
This commit adds declarations and dummy implementations for
the restructured outgoing client key exchange handling that
will replace the previous ssl_write_client_key_exchange().
The entry point for the CliKeyExchange handling that is called
from the handshake state machine is
`ssl_process_client_key_exchange()`,
splitting the processing into the following steps:
- Preparation
Compute the keying material to be sent.
* For (EC)DH: Pick parameters and compute PMS.
* For ECJPAKE: Run round 2
* For RSA: Encrypt PMS
- Writing: Prepare the writing of a new messae.
- Postprocessing: Update handstate state machine.
The subsequent commits will scatter the code from the previous
monolithic function ssl_write_client_key_exchange() among those
dedicated functions, commenting out each part of
ssl_write_client_key_exchange() that has already been dealt with.
This gradual progression is meant to ease reviewing. Once all
code has been moved and all changes explained,
ssl_write_client_key_exchange() will be removed.
This commit implements the record checking API
mbedtls_ssl_check_record()
on top of the restructured incoming record stack.
Specifically, it makes use of the fact that the core processing routines
ssl_parse_record_header()
mbedtls_ssl_decrypt_buf()
now operate on instances of the SSL record structure mbedtls_record
instead of the previous mbedtls_ssl_context::in_xxx fields.
After the rewrite of incoming record processing to use the internal
SSL record structure mbedtls_record (which contains the data_offset
field to indicate where the IV resides), this field is no longer
necessary.
Note: This is an API break.
ssl_get_next_record() updates the legacy in_xxx fields in two places,
once before record decryption and once after. Now that record decryption
doesn't use or affect the in_xxx fields anymore, setting up the these
legacy fields can entirely be moved to the end of ssl_get_next_record(),
which is what this comit does.
This commit solely moves existing code, but doesn't yet simplify the
now partially redundant settings of the in_xxx fields. This will be
done in a separate commit.
Multiple record attributes such as content type and payload length
may change during record decryption, and the legacy in_xxx fields
in the SSL context therefore need to be updated after the record
decryption routine ssl_decrypt_buf() has been called.
After the previous commit has made ssl_prepare_record_content()
independent of the in_xxx fields, setting them can be moved
outside of ssl_prepare_record_content(), which is what this
commit does.
Previously, ssl_update_in_pointers() ensured that the in_xxx pointers
in the SSL context are set to their default state so that the record
header parsing function ssl_parse_record_header() could make use of them.
By now, the latter is independent of these pointers, so they don't need
to be setup before calling ssl_parse_record_header() anymore.
However, other parts of the messaging stack might still depend on it
(to be studied), and hence this commit does not yet reomve
ssl_update_in_pointers() entirely.
The stack maintains pointers mbedtls_ssl_context::in_xxx pointing to
various parts of the [D]TLS record header. Originally, these fields
were determined and set in ssl_parse_record_header(). By now,
ssl_parse_record_header() has been modularized to setup an instance
of the internal SSL record structure mbedtls_record, and to derive
the old in_xxx fields from that.
This commit takes a further step towards removing the in_xxx fields
by deriving them from the established record structure _outside_ of
ssl_parse_record_header() after the latter has succeeded.
One exception is the handling of possible client reconnects,
which happens in the case then ssl_parse_record_header() returns
MBEDTLS_ERR_SSL_UNEXPECTED_RECORD; since ssl_check_client_reconnect()
so far uses the in_xxx fields, they need to be derived from the
record structure beforehand.
This commit makes a first step towards modularizing the incoming record
processing by having it operate on instances of the structure mbedtls_record
representing SSL records.
So far, only record encryption/decryption operate in terms of record
instances, but the rest of the parsing doesn't. In particular,
ssl_parse_record_header() operates directly on the fixed input buffer,
setting the various ssl->in_xxx pointers and fields, and only directly
before/after calling ssl_decrypt_buf() these fields a converted to/from
mbedtls_record instances.
This commit does not yet remove the ssl->in_xxx fields, but makes a step
towards extending the lifetime of mbedtls_record structure representing
incoming records, by modifying ssl_parse_record_header() to setup an
instance of mbedtls_record, and setting the ssl->in_xxx fields from that
instance. The instance so-constructed isn't used further so far, and in
particular it is not yet consolidated with the instance set up for use
in ssl_decrypt_record(). That's for a later commit.
Previously, ssl_parse_record_header() did not check whether the current
datagram is large enough to hold a record of the advertised size. This
could lead to records being silently skipped over or backed up on the
basis of an invalid record length. Concretely, the following would happen:
1) In the case of a record from an old epoch, the record would be
'skipped over' by setting next_record_offset according to the advertised
but non-validated length, and only in the subsequent mbedtls_ssl_fetch_input()
it would be noticed in an assertion failure if the record length is too
large for the current incoming datagram.
While not critical, this is fragile, and also contrary to the intend
that MBEDTLS_ERR_SSL_INTERNAL_ERROR should never be trigger-able by
external input.
2) In the case of a future record being buffered, it might be that we
backup a record before we have validated its length, hence copying
parts of the input buffer that don't belong to the current record.
This is a bug, and it's by luck that it doesn't seem to have critical
consequences.
This commit fixes this by modifying ssl_parse_record_header() to check that
the current incoming datagram is large enough to hold a record of the
advertised length, returning MBEDTLS_ERR_SSL_INVALID_RECORD otherwise.
We don't send alerts on other instances of ill-formed records,
so why should we do it here? If we want to keep it, the alerts
should rather be sent ssl_get_next_record().
As explained in the previous commit, if mbedtls_ssl_fetch_input()
is called multiple times, all but the first call are equivalent to
bounds checks in the incoming datagram.
In DTLS, if mbedtls_ssl_fetch_input() is called multiple times without
resetting the input buffer in between, the non-initial calls are functionally
equivalent to mere bounds checks ensuring that the incoming datagram is
large enough to hold the requested data. In the interest of code-size
and modularity (removing a call to a non-const function which is logically
const in this instance), this commit replaces such a call to
mbedtls_ssl_fetch_input() by an explicit bounds check in
ssl_parse_record_header().
Previously, `ssl_handle_possible_reconnect()` was part of
`ssl_parse_record_header()`, which was required to return a non-zero error
code to indicate a record which should not be further processed because it
was invalid, unexpected, duplicate, .... In this case, some error codes
would lead to some actions to be taken, e.g. `MBEDTLS_ERR_SSL_EARLY_MESSAGE`
to potential buffering of the record, but eventually, the record would be
dropped regardless of the precise value of the error code. The error code
`MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED` returned from
`ssl_handle_possible_reconnect()` did not receive any special treatment and
lead to silent dopping of the record - in particular, it was never returned
to the user.
In the new logic this commit introduces, `ssl_handle_possible_reconnect()` is
part of `ssl_check_client_reconnect()` which is triggered _after_
`ssl_parse_record_header()` found an unexpected record, which is already in
the code-path eventually dropping the record; we want to leave this code-path
only if a valid cookie has been found and we want to reset, but do nothing
otherwise. That's why `ssl_handle_possible_reconnect()` now returns `0` unless
a valid cookie has been found or a fatal error occurred.
Availability of sufficient incoming data should be checked when
it is needed, which is in mbedtls_ssl_fetch_input(), and this
function has the necessary bounds checks in place.
mbedtls_ssl_decrypt_buf() asserts that the passed transform is not NULL,
but the function is only invoked in a single place, and this invocation
is clearly visible to be within a branch ensuring that the incoming
transform isn't NULL. Remove the assertion for the benefit of code-size.
The previous code performed architectural maximum record length checks
both before and after record decryption. Since MBEDTLS_SSL_IN_CONTENT_LEN
bounds the maximum length of the record plaintext, it suffices to check
only once after (potential) decryption.
This must not be confused with the internal check that the record
length is small enough to make the record fit into the internal input
buffer; this is done in mbedtls_ssl_fetch_input().
The check is in terms of the internal input buffer length and is
hence likely to be originally intended to protect against overflow
of the input buffer when fetching data from the underlying
transport in mbedtls_ssl_fetch_input(). For locality of reasoning,
it's better to perform such a check close to where it's needed,
and in fact, mbedtls_ssl_fetch_input() _does_ contain an equivalent
bounds check, too, rendering the bounds check in question redundant.
When looking for a parent, all candidates were considered time-invalid due to
the #ifdef incorrectly including the `parent_valid = 1` line.
When MBEDTLS_HAVE_TIME_DATE is unset the time-validity of certificates is
never checked and always treated as valid. This is usually achieved by proper
usage of mbedtls_x509_time_is_past() and mbedtls_x509_time_is_future() (and
their definition when we don't HAVE_TIME_DATE).
Here the calls to these functions needs to be guarded by
MBEDTLS_X509_CRT_REMOVE_TIME as they access struct members whose presence is
controlled by this option. But the "valid" branch should still always be taken.
(Note: MBEDTLS_X509_CRT_REMOVE_TIME being set forces MBEDTLS_HAVE_TIME_DATE to
be unset, as enforce by check_config.h.)
This bug was found by `all.sh test_baremetal` - no need for a new test.
Asserting `*p == end` right after setting `end = *p + len` will always fail
unless `len == 0`, which is never the case with properly-formed certificates.
The function x509_skip_dates() is modelled after x509_get_dates() which between
setting `end` and comparing it to `*p` calls mbedtls_x509_get_time() which
advances `*p` to the expected value, which is why this test works in
get_dates().
Since `skip_dates()` has `skip`, not `validate` in its name, and the entire
point of `MBEDTLS_X509_CRT_REMOVE_TIME` is to save code, we don't want to
call the relatively large functions needed to properly parse (and validate)
dates before throwing the parsed dates away, we can just fast-forward to the
end of the sequence.
This makes updating `end` and comparing it to `*p` after the fast-forward
redundant, as the comparison will always be true (unlike the case where we
actually parse the contents of the sequence).
This bug was found by `all.sh test_baremetal` - no need for a new test.
Breaking into a series of statements makes things easier when stepping through
the code in a debugger.
Previous comments we stating the opposite or what the code tested for (what we
want vs what we're erroring out on) which was confusing.
Also expand a bit on the reasons for these restrictions.
ssl_get_next_record() may pend fatal alerts in response to receiving
invalid records. Previously, however, those were never actually sent
because there was no code-path checking for pending alerts.
This commit adds a call to ssl_send_pending_fatal_alert() after
the invocation of ssl_get_next_record() to fix this.
Modelled after the config-checking header from session s11n.
The list of relevant config flags was established by manually checking the
fields serialized in the format, and which config.h flags they depend on.
This probably deserves double-checking by reviewers.
Since the type of cid_len is unsigned but shorter than int, it gets
"promoted" to int (which is also the type of the result), unless we make the
other operand an unsigned int which then forces the expression to unsigned int
as well.
The number of meaning of the flags will be determined later, when handling the
relevant struct members. For now three bytes are reserved as an example, but
this number may change later.
This mainly follows the design document (saving all fields marked "saved" in
the main structure and the transform sub-structure) with two exceptions:
- things related to renegotiation are excluded here (there weren't quite in
the design document as the possibility of allowing renegotiation was still
on the table, which is no longer is) - also, ssl.secure_renegotiation (which
is not guarded by MBEDTLS_SSL_RENEGOTIATION because it's used in initial
handshakes even with renegotiation disabled) is still excluded, as we don't
need it after the handshake.
- things related to Connection ID are added, as they weren't present at the
time the design document was written.
The exact format of the header (value of the bitflag indicating compile-time
options, whether and how to merge it with the serialized session header) will
be determined later.
Enforce restrictions indicated in the documentation.
This allows to make some simplifying assumptions (no need to worry about
saving IVs for CBC in TLS < 1.1, nor about saving handshake data) and
guarantees that all values marked as "forced" in the design document have the
intended values and can be skipped when serialising.
Some of the "forced" values are not checked because their value is a
consequence of other checks (for example, session_negotiated == NULL outside
handshakes). We do however check that session and transform are not NULL (even
if that's also a consequence of the initial handshake being over) as we're
going to dereference them and static analyzers may appreciate the info.
At that point, the timer might not yet be configured.
The timer is reset at the following occasions:
- when it is initially configured through
mbedtls_ssl_set_timer_cb() or
mbedtls_ssl_set_timer_cb_cx()
- when a session is reset in mbedtls_ssl_session_reset()
- when a handshake finishes via mbedtls_ssl_handshake_wrap()
All modules using restartable ECC operations support passing `NULL`
as the restart context as a means to not use the feature.
The restart contexts for ECDSA and ECP are nested, and when calling
restartable ECP operations from restartable ECDSA operations, the
address of the ECP restart context to use is calculated by adding
the to the address of the ECDSA restart context the offset the of
the ECP restart context.
If the ECP restart context happens to not reside at offset `0`, this
leads to a non-`NULL` pointer being passed to restartable ECP
operations from restartable ECDSA-operations; those ECP operations
will hence assume that the pointer points to a valid ECP restart
address and likely run into a segmentation fault when trying to
dereference the non-NULL but close-to-NULL address.
The problem doesn't arise currently because luckily the ECP restart
context has offset 0 within the ECDSA restart context, but we should
not rely on it.
This commit fixes the passage from restartable ECDSA to restartable ECP
operations by propagating NULL as the restart context pointer.
Apart from being fragile, the previous version could also lead to
NULL pointer dereference failures in ASanDbg builds which dereferenced
the ECDSA restart context even though it's not needed to calculate the
address of the offset'ed ECP restart context.
All modules using restartable ECC operations support passing `NULL`
as the restart context as a means to not use the feature.
The restart contexts for ECDSA and ECP are nested, and when calling
restartable ECP operations from restartable ECDSA operations, the
address of the ECP restart context to use is calculated by adding
the to the address of the ECDSA restart context the offset the of
the ECP restart context.
If the ECP restart context happens to not reside at offset `0`, this
leads to a non-`NULL` pointer being passed to restartable ECP
operations from restartable ECDSA-operations; those ECP operations
will hence assume that the pointer points to a valid ECP restart
address and likely run into a segmentation fault when trying to
dereference the non-NULL but close-to-NULL address.
The problem doesn't arise currently because luckily the ECP restart
context has offset 0 within the ECDSA restart context, but we should
not rely on it.
This commit fixes the passage from restartable ECDSA to restartable ECP
operations by propagating NULL as the restart context pointer.
Apart from being fragile, the previous version could also lead to
NULL pointer dereference failures in ASanDbg builds which dereferenced
the ECDSA restart context even though it's not needed to calculate the
address of the offset'ed ECP restart context.
This commit introduces the option MBEDTLS_SSL_CONF_SINGLE_HASH
which can be used to register a single supported signature hash
algorithm at compile time. It replaces the runtime configuration
API mbedtls_ssl_conf_sig_hashes() which allows to register a _list_
of supported signature hash algorithms.
In contrast to other options used to hardcode configuration options,
MBEDTLS_SSL_CONF_SINGLE_HASH isn't a numeric option, but instead it's
only relevant if it's defined or not. To actually set the single
supported hash algorithm that should be supported, numeric options
MBEDTLS_SSL_CONF_SINGLE_HASH_TLS_ID
MBEDTLS_SSL_CONF_SINGLE_HASH_MD_ID
must both be defined and provide the TLS ID and the Mbed TLS internal
ID and the chosen hash algorithm, respectively.
mbedtls_ssl_set_calc_verify_md() serves two purposes:
(a) It checks whether a hash algorithm is suitable to be used
in the CertificateVerify message.
(b) It updates the function callback pointing to the function that
computes handshake transcript for the CertificateVerify message
w.r.t. the chosen hash function.
Step (b) is only necessary when receiving the CertificateVerify
message, while writing the CertificateRequest only involves (a).
This commit modifies the writing code for the CertificateRequest
message to inline the check (a) and thereby avoiding the call to
mbedtls_ssl_calc_verify_md().
mbedtls_ssL_set_calc_verify_md() is used to select valid hashes when
writing the server's CertificateRequest message, as well as to verify
and act on the client's choice when reading its CertificateVerify
message.
If enabled at compile-time and configured via mbedtls_ssl_conf_sig_hashes()
the current code also offers SHA-1 in TLS 1.2. However, the SHA-1-based
handshake transcript in TLS 1.2 is different from the SHA-1 handshake
transcript used in TLS < 1.2, and we only maintain the latter
(through ssl_update_checksum_md5sha1()), but not the former.
Concretely, this will lead to CertificateVerify verification failure
if the client picks SHA-1 for the CertificateVerify message in a TLS 1.2
handshake.
This commit removes SHA-1 from the list of supported hashes in
the CertificateRequest message, and adapts two tests in ssl-opt.sh
which expect SHA-1 to be listed in the CertificateRequest message.
mbedtls_ssl_set_calc_verify_md() is only called from places
where it has been checked that TLS 1.2 is being used. The
corresponding compile-time and runtime guards checking the
version in mbedtls_ssl_set_calc_verify_md() are therefore
redundant and can be removed.
The previous code writes the content (the EC curve list) of the extension
before writing the extension length field at the beginning, which is common
in the library in places where we don't know the length upfront. Here,
however, we do traverse the EC curve list upfront to infer its length
and do the bounds check, so we can reorder the code to write the extension
linearly and hence improve readability.
This commit introduces the option MBEDTLS_SSL_CONF_SINGLE_EC
which can be used to register a single supported elliptic curve
at compile time. It replaces the runtime configuration API
mbedtls_ssl_conf_curves() which allows to register a _list_
of supported elliptic curves.
In contrast to other options used to hardcode configuration options,
MBEDTLS_SSL_CONF_SINGLE_EC isn't a numeric option, but instead it's
only relevant if it's defined or not. To actually set the single
elliptic curve that should be supported, numeric options
MBEDTLS_SSL_CONF_SINGLE_EC_TLS_ID
MBEDTLS_SSL_CONF_SINGLE_EC_GRP_ID
must both be defined and provide the TLS ID and the Mbed TLS internal
ID and the chosen curve, respectively.
For both client/server the EC curve list is assumed not to be NULL:
- On the client-side, it's assumed when writing the
supported elliptic curve extension:
c54ee936d7/library/ssl_cli.c (L316)
- On the server, it is assumed when searching for a
suitable curve for the ECDHE exchange:
c54ee936d7/library/ssl_srv.c (L3200)
It is therefore not necessary to check this in mbedtls_ssl_check_curve().
ssl_write_supported_elliptic_curves_ext() is guarded by
```
#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \
defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED)
```
each of which implies (by check_config.h) that MBEDTLS_ECP_C
is enabled.
The fields
- mbedtls_ssl_handshake_params::max_major_ver,
- mbedtls_ssl_handshake_params::max_minor_ver
are used only for server-side RSA-based key exchanges
can be removed otherwise.
Reasons:
- If the transport type is fixed at compile-time,
mbedtls_ssl_read_version() and mbedtls_ssl_write_version()
are called with a compile-time determined `transport`
parameter, so the transport-type branch in their body
can be eliminated at compile-time.
- mbedtls_ssl_read_version() is called with addresses of
local variables, which so far need to be put on the stack
to be addressable. Inlining the call allows to read directly
into the registers holding these local variables.
This saves 60 bytes w.r.t. the measurement performed by
> ./scripts/baremetal.sh --rom --gcc
If the minor/major version is enforced at compile-time, the `major_ver`
and `minor_ver` fields in `mbedtls_ssl_context` are redundant and can
be removed.
This commit introduces the numeric compile-time constants
- MBEDTLS_SSL_CONF_MIN_MINOR_VER
- MBEDTLS_SSL_CONF_MAX_MINOR_VER
- MBEDTLS_SSL_CONF_MIN_MAJOR_VER
- MBEDTLS_SSL_CONF_MAX_MAJOR_VER
which, when defined, overwrite the runtime configurable fields
mbedtls_ssl_config::min_major_ver etc. in the SSL configuration.
As for the preceding case of the ExtendedMasterSecret configuration,
it also introduces and puts to use getter functions for these variables
which evaluate to either a field access or the macro value, maintaining
readability of the code.
The runtime configuration API mbedtls_ssl_conf_{min|max}_version()
is kept for now but has no effect if MBEDTLS_SSL_CONF_XXX are set.
This is likely to be changed in a later commit but deliberately omitted
for now, in order to be able to study code-size benefits earlier in the
process.
* origin/mbedtls-2.16:
Changelog entry for HAVEGE fix
Prevent building the HAVEGE module on platforms where it doesn't work
Fix misuse of signed ints in the HAVEGE module
The failure of mbedtls_md was not checked in one place. This could have led
to an incorrect computation if a hardware accelerator failed. In most cases
this would have led to the key exchange failing, so the impact would have been
a hard-to-diagnose error reported in the wrong place. If the two sides of the
key exchange failed in the same way with an output from mbedtls_md that was
independent of the input, this could have led to an apparently successful key
exchange with a predictable key, thus a glitching md accelerator could have
caused a security vulnerability.
This commit restructures ssl_ciphersuites.h and ssl_ciphersuites.c to
define all ciphersuite helper functions static inline in ssl_ciphersuites.h
if MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE is set, and to otherwise put their
definitions in ssl_ciphersuites.c.
If MBEDTLS_SSL_SINGLE_CIPHERSUITE is enabled, the type
mbedtls_ssl_ciphersuite_handle_t
is logically a boolean (concretely realized as `unsigned char`),
containing the invalid handle and the unique valid handle, which
represents the single enabled ciphersuite.
The SSL session structure mbedtls_ssl_session contains an instance
of mbedtls_ssl_ciphersuite_handle_t which is guaranteed to be valid,
and which is hence redundant in any two-valued implementation of
mbedtls_ssl_ciphersuite_handle_t.
This commit replaces read-uses of
mbedtls_ssl_session::ciphersuite_info
by a getter functions which, and defines this getter function
either by just reading the field from the session structure
(in case MBEDTLS_SSL_SINGLE_CIPHERSUITE is disabled), or by
returning the single valid ciphersuite handle (in case
MBEDTLS_SSL_SINGLE_CIPHERSUITE is enabled) and removing the
field from mbedtls_ssl_session in this case.
If MBEDTLS_SSL_SINGLE_CIPHERSUITE is enabled, it overwrites
the runtime configuration of supported ciphersuites, which
includes both the configuration API and the fields which are
used to store the configuration. Both are therefore no longer
needed and should be removed for the benefit of code-size,
memory usage, and API clarity (no accidental hiccup of runtime
vs. compile-time configuration possible).
The configuration API mbedtls_ssl_conf_ciphersuites() has
already been removed in case MBEDTLS_SSL_SINGLE_CIPHERSUITE,
and this commit removes the field
mbedtls_ssl_config::ciphersuite_list
which it updates.
If MBEDTLS_SSL_SINGLE_CIPHERSUITE is enabled, the type
mbedtls_ssl_ciphersuite_handle_t
is logically a boolean (concretely realized as `unsigned char`),
containing the invalid handle and the unique valid handle, which
represents the single enabled ciphersuite.
The SSL handshake structure mbedtls_ssl_handshake_params contains
an instance of mbedtls_ssl_ciphersuite_handle_t which is guaranteed
to be valid, and which is hence redundant in any two-valued
implementation of mbedtls_ssl_ciphersuite_handle_t.
This commit replaces read-uses of
mbedtls_ssl_handshake_params::ciphersuite_info
by a getter functions which, and defines this getter function
either by just reading the field from the handshake structure
(in case MBEDTLS_SSL_SINGLE_CIPHERSUITE is disabled), or by
returning the single valid ciphersuite handle (in case
MBEDTLS_SSL_SINGLE_CIPHERSUITE is enabled) and removing the
field from mbedtls_ssl_handshake_params in this case.
This commit adapts the ClientHello parsing routines in ssl_srv.c
to use the ciphersuite traversal macros
MBEDTLS_SSL_BEGIN_FOR_EACH_CIPHERSUITE
MBEDTLS_SSL_END_FOR_EACH_CIPHERSUITE
introduced in the last commit, thereby making them work
both with and without MBEDTLS_SSL_SINGLE_CIPHERSUITE.
Another notable change concerns the ssl_ciphersuite_match:
Previous, this function would take a ciphersuite ID and a
pointer to a destination ciphersuite info structure as input
and write eithe NULL or a valid ciphersuite info structure
to that destination address, depending on whether the suite
corresponding to the given ID was suitable or not. The
function would always return 0 outside of a fatal error.
This commit changes this to ssl_ciphersuite_is_match() which
instead already takes a ciphersuite handle (which outside
of a hardcoded ciphersuite is the same as the ptr to a
ciphersuite info structure) and returns 0 or 1 (or a
negative error code in case of a fatal error) indicating
whether the suite corresponding to the handle was acceptable
or not. The conversion of the ciphersuite ID to the ciphersuite
info structure is done prior to calling ssl_ciphersuite_is_match().
This commit modifies the ClientHello writing routine ssl_write_client_hello
in ssl_cli.c to switch between
(a) listing all runtime configured ciphersuites
(in case MBEDTLS_SSL_SINGLE_CIPHERSUITE is not defined)
(b) listing just the single hardcoded ciphersuite
(in case MBEDTLS_SSL_SINGLE_CIPHERSUITE is defined)
The approach taken is to introduce a pair of helper macros
MBEDTLS_SSL_BEGIN_FOR_EACH_CIPHERSUITE( ssl, ver, info )
MBEDTLS_SSL_END_FOR_EACH_CIPHERSUITE
which when delimiting a block of code lead to that block of
code being run once for each ciphersuite that's enabled in the
context `ssl` and version `ver`, referenced through the (fresh)
`info` variable. Internally, this is implemented either through
a plain `for` loop traversing the runtime configured ciphersuite
list (if MBEDTLS_SSL_SINGLE_CIPHERSUITE is disabled) or by just
hardcoding `info` to the single enabled ciphersuite (if
MBEDTLS_SSL_SINGLE_CIPHERSUITE is enabled).
These helper macros will prove useful whereever previous code
traversed the runtime configured ciphersuite list, but adaptations
of those occasions outside ClientHello writing are left for later
commits.
This commit is a step towards the goal of allowing to hardcode the choice
of a single ciphersuite at compile-time. The hoped for benefit of this is
that whereever a ciphersuite attribute is queried and checked against a
compile-time constant, the check can be recognized as either true or false
at compile-time, hence leading to a code-size reduction.
For this to work, the ciphersuite attribute getter functions
mbedtls_ssl_suite_get_xxx() will be modified to return something
the compiler can recognize as a compile-time constant. In particular,
in order to avoid relying on constant propagation abilities of the
compiler, these functions should ideally return constant symbols
(instead of, say, fields in a globally const structure instance).
This puts us in the following situation: On the one hand, there's the
array of ciphersuite information structures defining the attribute of
those ciphersuites the stack knows about. On the other hand, we need
direct access to those fields through constant symbols in the getter
functions.
In order to avoid any duplication of information, this commit exemplifies
how ciphersuites can be conveniently defined on the basis of macro
definitions, and how the corresponding instances of the ciphersuite
information structure can be auto-generated from this.
In the approach, to add support for a ciphersuite with official name
NAME (such as TLS_ECDHE_ECDSA_WITH_AES_128_CCM_8), the following macro
constants need to be defined in ssl_ciphersuites.h:
MBEDTLS_SUITE__ NAME __ID
MBEDTLS_SUITE__ NAME __NAME
MBEDTLS_SUITE__ NAME __CIPHER
MBEDTLS_SUITE__ NAME __MAC
...
To make check-names.sh happy, one also needs a dummy macro
MBEDTLS_SUITE__ NAME()
These ciphersuite attribute values can then be queried via
MBEDTLS_SSL_SUITE_ID( NAME_MACRO )
...
where NAME_MACRO can be any macro expanding to a defined NAME.
Further, a convenience macro
MBEDTLS_SSL_SUITE_INFO( NAME_MACRO )
is provided that again takes a macro NAME_MACRO expanding to a
defined NAME, and itself expands to an instance of
mbedtls_ssl_ciphersuite_info_t using the macro attributes
defined for NAME. This macro is then used in ssl_ciphersuites.c
when defining the array of known ciphersuite information structures,
(a) without duplicating the information, and (b) with increased
readability, because there's only one line for each ciphersuite.
This commit introduces an internal zero-cost abstraction layer for
SSL ciphersuites: Instead of addressing ciphersuites via pointers
to instances of mbedtls_ssl_ciphersuite_t and accessing their fields
directly, this commit introduces an opaque type
mbedtls_ssl_ciphersuite_handle_t,
and getter functions
mbedtls_ssl_suite_get_xxx()
operating on ciphersuite handles.
The role of NULL is played by a new macro constant
MBEDTLS_SSL_CIPHERSUITE_INVALID_HANDLE
which results of functions returning handles can be checked against.
(For example, when doing a lookup of a ciphersuite from a peer-provided
ciphersuite ID in the per's Hello message).
The getter functions have the validity of the handle as a precondition
and are undefined if the handle is invalid.
So far, there's only one implementation of this abstraction layer, namely
mbedtls_ssl_ciphersuite_handle_t being mbedtls_ssl_ciphersuite_t const *
and
getter functions being field accesses.
In subsequent commits, however, the abstraction layer will be useful
to save code in the situation where only a single ciphersuite is enabled.
* origin/pr/2700:
Changelog entry for HAVEGE fix
Prevent building the HAVEGE module on platforms where it doesn't work
Fix misuse of signed ints in the HAVEGE module
So far, the client-proposed list of elliptic curves was stored for the
duration of the entire handshake in a heap-allocated buffer referenced
from mbedtls_ssl_handshake_params::curves. It is used in the following
places:
1) When the server chooses a suitable ciphersuite, it checks that
it has a certificate matching the ciphersuite; in particular, if
the ciphersuite involves ECDHE, the server needs an EC certificate
with a curve suitable for the client.
2) When performing the ECDHE key exchange, the server choose one
curve among those proposed by the client which matches the server's
own supported curve configuration.
This commit removes the hold back the entire client-side curve list
during the handshake, by performing (1) and (2) on during ClientHello
parsing, and in case of (2) only remembering the curve chosen for ECDHE
within mbedtls_ssl_handshake_params.
Fix an "unused variable" warning that happened in some configurations
(without EC, found by depend-pkalg.pl) and was not present in any parent PR
but only in the result of merging them: one of the PRs clarified the
distinction between `ret` and `verify_ret` and the other removed one
occurrence of using `ret`, and the conjunction of the two made `ret` unused in
some cases. Resolving by reducing the scope of that variable.
* restricted/pr/608:
programs: Make `make clean` clean all programs always
ssl_tls: Enable Suite B with subset of ECP curves
windows: Fix Release x64 configuration
timing: Remove redundant include file
net_sockets: Fix typo in net_would_block()
Add all.sh component that exercises invalid_param checks
Remove mbedtls_param_failed from programs
Make it easier to define MBEDTLS_PARAM_FAILED as assert
Make test suites compatible with #include <assert.h>
Pass -m32 to the linker as well
Update library to 2.16.2
Use 'config.pl baremetal' in all.sh
Clarify ChangeLog entry for fix to #1628Fix#2370, minor typos and spelling mistakes
Add Changelog entry for clang test-ref-configs.pl fix
Enable more compiler warnings in tests/Makefile
Change file scoping of test helpers.function
* restricted/pr/594:
Adapt baremetal.h and baremetal.sh
Don't incl. CAs in CertReq message in baremetal build
Allow config'n of incl of CertificateReq CA list Y/N at compile-time
Allow configuration of endpoint (cli/srv) at compile-time
Allow configuration of read timeouts at compile-time
Allow configuration of ConnectionID at compile-time
Allow compile-time configuration of legacy renegotiation
Allow compile-time configuration of authentication mode
Allow compile-time configuration of DTLS badmac limit
Allow compile-time configuration of DTLS anti replay
* restricted/pr/601: (27 commits)
Fix compile-time guard for optional field in struct
Move code to reduce probability of conflicts
Fix typos caught by check-names.sh
Clarify conditions related to resumption in client
Introduce getter function for renego_status
Add getter function for handshake->resume
Remove now-redundant code
Remove cache callbacks from config on client
Fix a few style issues
Expand documentation of new options a bit
Fix renaming oversight in documentation
Remove backticks in doxygen in config.h
Declare dependency on tickets for two ssl-opt.sh tests
Exclude new negative options from config.pl full
Restore config.h defaults
Address review comments
Fix ssl_cli resumption guards
Fix check-files, check-names and check-generated-features
Add test to all.sh
Add changelog entry
...
* restricted/pr/584: (140 commits)
Remove superfluous new line in x509.c
Add comment about X.509 name comparison of buffer with itself
[Fixup] Add missing PK release call in Cert Verify parsing
Fix guard controlling whether nested acquire calls are allowed
Add X.509 CRT test for nested calls for CRT frame / PK acquire
Don't return threading error on release()-without-acquire() calls
Don't allow nested CRT acquire()-calls if MBEDTLS_X509_ALWAYS_FLUSH
Make X.509 CRT cache reference counting unconditional
Remove memory buffer alloc from i386 test in all.sh
Don't mention pk_sign() in the context of public-key contexts
Don't use assertion for failures of mbedtls_x509_crt_x_acquire()
Fix copy pasta in x509_crt.h
Reference copy-less versions of X.509 CRT frame/PK getters
x509_crt.c: Add blank line to increase readability
[FIXUP] Fix bug in ASN.1 traversal of silently ignored tag
[FIXUP] Fix typo in declaration of mbedtls_x509_memcasecmp()
Move signature-info extraction out of MBEDTLS_X509_REMOVE_INFO
Fix certificate validity checking logic to work with !TIME_DATE
Simplify X.509 CRT version check in UID parsing
Remove unused variable warning in on-demand X.509 parsing
...
Introduces MBEDTLS_SSL_CONF_CERT_REQ_CA_LIST which allows to configure
at compile-time whether a CA list should be included in the
CertificateRequest message sent by the server.
Impact on code-size:
| | GCC 8.2.1 | ARMC5 5.06 | ARMC6 6.12 |
| --- | --- | --- | --- |
| `libmbedtls.a` before | 23131 | 23805 | 26673 |
| `libmbedtls.a` after | 23099 | 23781 | 26639 |
| gain in Bytes | 32 | 24 | 34 |
Introduces MBEDTLS_SSL_CONF_BADMAC_LIMIT to fix the maximum
number of records with bad MAC tolerated in DTLS at compile-time.
Impact on code-size:
| | GCC | ARMC5 | ARMC6 |
| --- | --- | --- | --- |
| `libmbedtls.a` before | 23511 | 24049 | 27903 |
| `libmbedtls.a` after | 23487 | 24025 | 27885 |
| gain in Bytes | 24 | 24 | 18 |
mbedtls_ssl_read() can fail non-fatally, in which case
ssl_parse_certificate_verify() returned immediately without
calling mbedtls_x509_crt_pk_release(), which in turn lead
to a fatal error because of nested acquire calls in the
next call to the function.
While not strictly related to this PR, this change improves readability in
some resumption-related runtime conditions that previously had rather ugly
preprocessor directives in the middle of already complex predicates.
Due to previous change of conditions, this is now in the 'else' branch of 'if
resume == 1' and the only allowed values are 0 or 1, so setting to 0 is
redundant.
Add a new configuration option MBEDTLS_SSL_SESSION_RESUMPTION
to enable/disable the session resumption feature including
ticket and cache based session resumption.
Resource counting as a safe-guard against nested acquire calls
is implemented if and only if MBEDTLS_X509_ALWAYS_FLUSH is disabled
_or_ MBEDTLS_THREADING_C is enabled.
Forbidding nested calls to acquire() allows to remove the reference
counting logic and hence saving some bytes of code. This is valuable
because MBEDTLS_X509_ALWAYS_FLUSH is likely to be used on constrained
systems where code-size is limited.
Previously, reference counting for the CRT frames and PK contexts
handed out by mbedtls_x509_crt_{frame|pk}_acquire() was implemented
only in case threading support was enabled, which leaves the door
open for a potential use-after-free should a single-threaded application
use nested calls to mbedtls_x509_crt_acquire().
Since Mbed TLS itself does not use such nested calls, it might be
preferred long-term to forbid nesting of acquire calls on the API
level, and hence get rid of reference counting in the interest of
code-size benefits. However, this can be considered as an optimization
of X.509 on demand parsing, and for now this commit introduces
reference counting unconditionally to have a safe version of
on demand parsing to build further optimizations upon.
During rebase, the definition of ::mbedtls_x509_crt_sig_info
as well as x509_crt_free_sig_info() and x509_crt_get_sig_info()
were accidentally guarded by !MBEDTLS_X509_REMOVE_INFO.
This commit moves their definition outside of that guard.
If MBEDTLS_HAVE_TIME_DATE is undefined, the functions
`mbedtls_x509_time_is_past()` and `mbedtls_x509_time_is_future()`
are still defined but return `0` (that is, no time is seen to in
the past or future). To maintain functional correctness, this
means that these functions have to be called in a way where
the condition being checked for is the erroneous one: Concretely,
one shouldn't check that a CRT's `validFrom` is in the past,
or that its `validTo` is in the future, because that would
fail if !MBEDTLS_HAVE_TIME_DATE. Instead, one should check
that `validFrom` is NOT in the future, and `validTo` is NOT
in the past. That was the logic previously, but an uncautious
change during transition to X.509 on-demand parsing has
changed it. This commit fixes this.
WHen parsing the CRT version, we already check that
version is either 1, 2, or 3, so checking whether
version == 2 or version == 3 is equivalent to
version != 1.
Previously, only one thread could access the parsing cache of an X.509 CRT
at a time. Firstly, this leads to significant performance penalties on
systems running many concurrent threads which share CRT structures --
for example, server threads sharing an SSL configuration containing the
server CRT. Secondly, the locking should be logically unnecessary, because
the threads are supposed to access the CRT frame and PK in a read-only,
or at least thread-safe manner.
This commit modifies the X.509 CRT cache implementation by allowing an
arbitrary number of concurrent readers, locking only the path of setting
up and clearing the cache.
This commit modifies the implementation of x509_get_ext_key_usage()
to not rely on mbedtls_asn1_get_sequence_of() but to instead use
mbedtls_asn1_traverse_sequence_of() with the same sequence-building
callback that also x509_get_subject_alt_name() uses, and which agrees
with the callback used by mbedtls_asn1_get_sequence_of().
The reason for this is that with this change, Mbed TLS itself isn't
using mbedtls_asn1_get_sequence_of() anymore, but only the more powerful
mbedtls_asn1_traverse_sequence_of(), so that unless application code
makes use of mbedtls_asn1_get_sequence_of(), its implementation
-- including the underlying sequence building callback -- will be
removed by link time garbage collection.
This commit introduces two static helpers
- `x509_buf_to_buf_raw()`
- `x509_buf_raw_to_buf()`
which convert to/from the old `mbedtls_x509_buf` and
the new `mbedtls_x509_buf_raw` (the latter omitting the
ASN.1 tag field).
So far, the CRT frame structure `mbedtls_x509_crt_frame` used
as `issuer_raw` and `subject_raw` the _content_ of the ASN.1
name structure for issuer resp. subject. This was in contrast
to the fields `issuer_raw` and `subject_raw` from the legacy
`mbedtls_x509_crt` structure, and caused some information
duplication by having both variants `xxx_no_hdr` and `xxx_with_hdr`
in `mbedtls_x509_crt` and `mbedtls_x509_crt_frame`.
This commit removes this mismatch by solely using the legacy
form of `issuer_raw` and `subject_raw`, i.e. those _including_
the ASN.1 name header.
Previously, `mbedtls_x509_crt_cache_provide_frame()` provided the requested
CRT frame by always parsing the raw data underlying the CRT. That's inefficient
in legacy mode, where the CRTs fields are permanently accessible through the
legacy `mbedtls_x509_crt` structure.
This commit modifies `mbedtls_x509_crt_cache_provide_frame()` in legacy mode
(that is, !MBEDTLS_X509_ON_DEMAND_PARSING) to setup the CRT frame by copying
fields from the legacy CRT structure.
This commit modifies the CRT parsing routine to flush
the CRT cache after parsing. More specifically, the
frame cache is flushed before the PK is parsed, to
avoid storing the PK and frame in RAM at the same time.
With the introduction of `mbedtls_x509_crt_get_{issuer|name}()`,
users need an easy way of freeing the dynamic name structures these
functions return.
To that end, this commit renames `x509_{sequence|name}_free()`
to `mbedtls_x509_{sequence|name}_free()` and gives them external linkage.
The legacy `mbedtls_x509_crt` contains fields `issuer/subject`
which are dynamically allocated linked list presentations of the
CRTs issuer and subject names, respectively.
The new CRT frame structure `mbedtls_x509_crt_frame`, however,
only provides pointers to the raw ASN.1 buffers for the issuer
and subject, for reasons of memory usage.
For convenience to users that previously used the `issuer`/`subject`
fields of `mbedtls_x509_crt`, this commit adds two public API functions
`mbedtls_x509_crt_get_subject()` and `mbedtls_x509_crt_get_issuer()`
which allow to request the legacy linked list presentation of the
CRTs subject / issuer names.
Similar to `mbedtls_x509_crt_get_pk()`, the returned names are owned
by the user, and must be freed through a call to `mbedtls_x509_name_free()`.
This commit unconditionally adds two convenience API functions:
- mbedtls_x509_crt_get_frame()
- mbedtls_x509_crt_get_pk()
which allow users to extract a CRT frame or PK context
from a certificate.
The difference with the existing acquire/release API for frame and PK
contexts is that in contrast to the latter, the structures returned by
the new API are owned by the user (and, in case of the PK context, need
to be freed by him). This makes the API easier to use, but comes at the
cost of additional memory overhead.
This commit replaces the dummy implementation of the CRT acquire/release
framework by a cache-based implementation which remembers frame and PK
associated to a CRT across multiple `acquire/release` pairs.
Access the peer's PK through the PK acquire/release API only.
Care has to be taken not to accidentally overwrite the return
value `ret` from the CRT chain verification.
This commit modifies the static function `x509_crt_verify_name()` to
use the acquire/release API to access the given CRTs `subject` field.
This function is solely called from the beginning of the CRT chain
verification routine, which also needs to access the child's CRT frame.
It should therefore be considered - for a later commit - to collapse
the two acquire/release pairs to one, thereby saving some code.
Previously, `mbedtls_x509_crt_der_internal()` used the `version` field
(which is `0` after initialization but strictly greater than 0 once a
CRT has successfully been parsed) to determine whether an
`mbedtls_x509_crt` instance had already been setup.
Preparating for the removal of `version` from the structure, this
commit modifies the code to instead peek at the raw data pointer,
which is NULL as long as the CRT structure hasn't been setup with a CRT,
and will be kept in the new CRT structure.
This commit adapts `mbedtls_x509_crt_info()` to no longer access
structure fields from `mbedtls_x509_crt` directly, but to instead
query for a `mbedtls_x509_crt_frame` and `mbedtls_pk_context` and
use these to extract the required CRT information.
This commit continues rewriting the CRT chain verification to use
the new acquire/release framework for CRTs. Specifically, it replaces
all member accesses of the current _parent_ CRT by accesses to the
respective frame.
This commit introduces an internal structure `mbedtls_x509_crt_sig_info`
containing all information that has to be kept from a child CRT when searching
for a potential parent:
- The issuer name
- The signature type
- The signature
- The hash of the CRT
The structure can be obtained from a CRT frame via `x509_crt_get_sig_info()`
and freed via `x509_crt_free_sig_info()`.
The purpose of this is to reduce the amount of RAM used during CRT
chain verification; once we've extracted the signature info structure
from the current child CRT, we can free all cached data for that CRT
(frame and PK) before searching for a suitable parent. This way, there
will ultimately not be more than one frame needed at a single point
during the verification.
The function `x509_crt_find_parent_in()` traverses a list of CRTs
to find a potential parent to a given CRT. So far, the logic was
the following: For each candidate,
- check basic parenting skills (mostly name match)
- verify signature
- verify validity
This order is insuitable for the new acquire/release layer of
indirection when dealing with CRTs, because we either have to
query the candidate's CRT frame twice, or query frame and PK
simultaneously.
This commit moves the validity check to the beginning of the
routine to allow querying for the frame and then for the PK.
The entry point for restartable ECC needs to be moved for that
to not forget the validity-flag while pausing ECC computations.
During CRT verification, `x509_crt_check_signature()` checks whether a
candidate parent CRT correctly signs the current child CRT.
This commit rewrites this function to use the new acquire/release
framework for using CRTs.
The goal of the subsequent commits is to remove all direct uses
of the existing `mbedtls_x509_crt` apart from the `raw` buffer
and the linked list `next` pointer.
The approach is the following: Whenever a code-path needs to inspect
a CRT, it can request a frame for the CRT through the API
`x509_crt_frame_acquire()`. On success, this function returns a pointer
to a frame structure for the CRT (the origin of which is flexible and
need not concern the caller) that can be used to inspect the desired
fields. Once done, the caller hands back the frame through an explicit
call to `x509_crt_frame_release()`.
This commit also adds an inefficient dummy implementation for
`x509_crt_frame_acquire()` which always allocates a new
`mbedtls_x509_crt_frame` structure on the heap and parses it
from the raw data underlying the CRT. This will change in subsequent
commits, but it constitutes a valid implementation to test against.
Ultimately, `x509_crt_frame_acquire()` is to compute a frame for the
given CRT only once, and cache it for subsequent calls.
The need for `x509_crt_frame_release()` is the following: When
implementing `x509_crt_frame_acquire()` through a flushable cache
as indicated above, it must be ensured that no thread destroys
a cached frame structure for the time it is needed by another
thread. The `acquire/release` pair allows to explicitly delimit
the lifetime requirements for the returned frame structure.
The frame pointer must not be used after the `release` call anymore;
and in fact, the dummy implementation shows that it would
immediately lead to a memory failure.
Analogously to `x509_crt_frame_{acquire|release}()`, there's also
`x509_crt_pk_{acquire|release}()` which allows to acquire/release
a PK context setup from the public key contained within the CRT.
This commit restructures the parsing of X.509 CRTs in the following way:
First, it introduces a 'frame' structure `mbedtls_x509_crt_frame`, which
contains pointers to some structured fields of a CRT as well as copies of
primitive fields. For example, there's a pointer-length pair delimiting the raw
public key data in the CRT, but there's a C-uint8 to store the CRT version
(not a pointer-length pair delimiting the ASN.1 structure holding the version).
Setting up a frame from a raw CRT buffer does not require any memory outside
of the frame structure itself; it's just attaches a 'template' to the buffer
that allows to inspect the structured parts of the CRT afterwards.
Note that the frame structure does not correspond to a particular ASN.1
structure; for example, it contains pointers to delimit the three parts
of a CRT (TBS, SignatureAlgorithm, Signature), but also pointers to the
fields of the TBS, and pointers into the Extensions substructure of the TBS.
Further, the commit introduces an internal function `x509_crt_parse_frame()`
which sets up a frame from a raw CRT buffer, as well as several small helper
functions which help setting up the more complex structures (Subject, Issuer, PK)
from the frame.
These functions are then put to use to rewrite the existing parsing function
`mbedtls_x509_crt_parse_der_core()` by setting up a CRT frame from the input
buffer, residing on the stack, and afterwards copying the respective fields
to the actual `mbedtls_x509_crt` structure and performing the deeper parsing
through the various helper functions.
At every occasion where we're using `mbedtls_x509_memcasecmp()` we're
checking that the two buffer lengths coincide before making the call.
This commit saves a few bytes of code by moving this length check
to `mbedtls_x509_memcasecmp()`.
This commit adds a new function `mbedtls_asn1_traverse_sequence_of()`
which traverses an ASN.1 SEQUENCE and calls a user-provided callback
for each entry.
It allows to put the following constraints on the tags allowed
in the SEQUENCE:
- A tag mask and mandatory tag value w.r.t. that mask.
A non-matching tag leads to an MBEDTLS_ERR_ASN1_UNEXPECTED_TAG error.
For example, it the mask if 0xFF, this means that only
a single tag will be allowed in the SEQUENCE.
- A tag mask and optional tag value w.r.t. that mask.
A non-matching tag is silently ignored.
The main use for this flexibility is the traversal of the
`SubjectAlternativeNames` extension, where some parts of the
tag are fixed but some are flexible to indicate which type
of name the entry describes.
This commit adds a callback for use with `x509_subject_alt_name_traverse()`
which builds the legacy dynamically allocated linked list presentation
of the `SubjectAlternativeNames` extension while traversing the raw data.
The current CN name verification x509_crt_verify_name() traverses
the dynamically allocated linked list presentation of the subject
alternative name extension, searching for an alternative name that
matches the desired hostname configured by the application.
Eventually, we want to remove this dynamically allocated linked list
for the benefit of reduced code size and RAM usage, and hence need to
rewrite x509_crt_verify_name() in a way that builds on the raw ASN.1
buffer holding the SubjectAlternativeNames extension.
This commit does this by using the existing SubjectAlternativeNames
traversal routine x509_subject_alt_name_traverse(), passing to it a
callback which compares the current alternative name component to the
desired hostname configured by the application.
This commit adds a new function `x509_subject_alt_name_traverse()`
which allows to traverse the raw ASN.1 data of a `SubjectAlternativeNames`
extension.
The `SubjectAlternativeNames` extension needs to be traversed
in the following situations:
1 Initial traversal to check well-formedness of ASN.1 data
2 Traversal to check for a particular name component
3 Building the legacy linked list presentation
Analogously to how multiple tasks related to X.509 name comparison
are implemented through the workhorse `mbedtlS_x509_name_cmp_raw()`,
the new function `x509_subject_alt_name_traverse()` allows to pass
an arbitrary callback which is called on any component of the
`SubjectAlternativeNames` extension found. This way, the above
three tasks can be implemented by passing
1 a NULL callback,
2 a name comparison callback
3 a linked list building callback.
In preparation for rewriting the `SubjectAlternativeName` search routine
to use raw ASN.1 data, this commit changes `x509_check_wildcard()` and
`x509_check_cn()`, responsible for checking whether a name matches a
wildcard pattern, to take a raw buffer pointer and length as parameters
instead of an `mbedtls_x509_buf` instance.
This is analogous to a previous commit for the `ExtendedKeyUsage`
extension: We aim at not using dynamically allocated linked lists
to represent the components of the `SubjectAlternativeName` extension,
but to traverse the raw ASN.1 data when needed.
This commit adds a field to `mbedtls_x509_crt` containing the raw
ASN.1 buffer bounds of the `SubjectAlternativeNames` extension.
This commit re-implements `mbedtls_x509_crt_check_extended_key_usage()`
to not use the dynamically allocated linked list presentation of the
`ExtendedKeyUsage` but to search for the required usage by traversing
the raw ASN.1 data.
The previous commits replace the use of dynamically allocated linked lists
for X.509 name inspection. This commit is the first in a series which attempts
the same for the `ExtendedKeyUsage` extension. So far, when a CRT is parsed,
the extension is traversed and converted into a dynamically allocated linked
list, which is then search through whenever the usage of a CRT needs to be
checked through `mbedtls_x509_check_extended_key_usage()`.
As a first step, this commit introduces a raw buffer holding the bounds
of the `ExtendedKeyUsage` extension to the `mbedtls_x509_crt` structure.
The previous CN name comparison function x509_crt_verify_name()
traversed the dynamically allocated linked list presentation of
the CRT's subject, comparing each entry to the desired hostname
configured by the application code.
Eventually, we want to get rid of the linked list presentation of
the CRT's subject to save both code and RAM usage, and hence need
to rewrite the CN verification routine in a way that builds on the
raw ASN.1 subject data only.
In order to avoid duplicating the code for the parsing of the nested
ASN.1 name structure, this commit performs the name search by using
the existing name traversal function mbedtls_x509_name_cmp_raw(),
passing to it a callback which checks whether the current name
component matches the desired hostname.
There are three operations that need to be performed on an X.509 name:
1 Initial traversal to check well-formedness of the ASN.1 structure.
2 Comparison between two X.509 name sequences.
3 Checking whether an X.509 name matches a client's ServerName request.
Each of these tasks involves traversing the nested ASN.1 structure,
In the interest of saving code, we aim to provide a single function
which can perform all of the above tasks.
The existing comparison function is already suitable not only for task 2,
but also for 1: One can simply pass two equal ASN.1 name buffers, in which
case the function will succeed if and only if that buffer is a well-formed
ASN.1 name.
This commit further adds a callback to `mbedtls_x509_name_cmp_raw()` which
is called after each successful step in the simultaneous name traversal and
comparison; it may perform any operation on the current name and potentially
signal that the comparison should be aborted.
With that, task 3 can be implemented by passing equal names and a callback
which aborts as soon as it finds the desired name component.
This commit replaces the previous calls to `mbedtls_x509_name_cmp()`
during CRT verification (to match child and parent, to check whether
a CRT is self-issued, and to match CRLs and CAs) by calls to the new
`mbedtls_x509_name_cmp_raw()` using the raw ASN.1 data; it passes the
raw buffers introduced in the last commits.
The previous name comparison function mbedtls_x509_name_cmp() is now
both unused and unneeded, and is removed.
To make use of the X.509 name comparison function based on raw
ASN.1 data that was introduced in the previous commit, this commit
adds an ASN.1 buffer field `issuer_raw_no_hdr` to `mbedtls_x509_crl`
which delimits the raw contents of the CRLs `Issuer` field.
The previous field `issuer_raw` isn't suitable for that because
it includes the ASN.1 header.
This commit provides a new function `mbedtls_x509_name_cmp_raw()`
to x509.c for comparing to X.509 names by traversing the raw ASN.1
data (as opposed to using the dynamically allocated linked list
of `mbedtls_x509_name` structures). It has external linkage because
it will be needed in `x509_crt` and `x509_crl`, but is marked
internal and hence not part of the public API.
The function `mbedtls_x509_sig_alg_gets()` previously needed the
raw ASN.1 OID string even though it is implicit in the PK and MD
parameters.
This commit modifies `mbedtls_x509_sig_alg_gets()` to infer the OID
and remove it from the parameters.
This will be needed for the new X.509 CRT structure which will
likely not store the signature OID.
Care has to be taken to handle the case of RSASSA-PSS correctly,
where the hash algorithm in the OID list is set to MBEDTLS_MD_NONE
because it's only determined by the algorithm parameters.
The previous code
- checked that at least 1 byte of ASN.1 tag data is available,
- read and stored that ASN.1 tag,
- called the ASN.1 parsing function, part of which is checking
that enough space is available and that the ASN.1 tag matches
the expected value MBEDTLS_ASN1_OID.
Since the ASN.1 parsing function includes bounds checks,
this can be streamlined to:
- call the ASN.1 parsing function directly,
- on success, store MBEDTLS_ASN1_OID in the tag field.
This commit applies this simplification to mbedtls_asn1_get_alg().
Consider the following code-template:
int beef();
static int foo()
{
/* ... */
ret = beef();
if( ret != 0 )
return( ret + HIGH_LEVEL );
/* ... */
}
int bar()
{
/* ... */
ret = foo();
if( ret != 0 )
...
/* ... */
}
This leads to slightly larger code than expected, because when the
compiler inlines foo() into bar(), the sequence of return sequences
cannot be squashed, because compiler might not have knowledge that
the wrapping `ret + HIGH_LEVEL` of the return value of beef() doesn't
lead to foo() returning 0.
This can be avoided by performing error code wrapping in nested
functions calls at the top of the call chain.
This commit applies this slight optimization to mbedtls_x509_get_name().
It also moves various return statements into a single exit section,
again with the intend to save code.
X.509 names in ASN.1 are encoded as ASN.1 SEQUENCEs of ASN.1 SETs
of Attribute-Value pairs, one for each component in the name. (For
example, there could be an Attribute-Value pair for "DN=www.mbedtls.org").
So far, `mbedtls_x509_get_name()` parsed such names by two nested
loops, the outer one traversing the outer ASN.1 SEQUENCE and the
inner one the ASN.1 SETs.
This commit introduces a helper function `x509_set_sequence_iterate()`
which implements an iterator through an ASN.1 name buffer; the state
of the iterator is a triple consisting of
- the current read pointer
- the end of the current SET
- the end of the name buffer
The iteration step reads a new SET if the current read pointer has
reached the end of the current SET, and afterwards reads the next
AttributeValue pair.
This way, iteration through the X.509 name data can be implemented
in a single loop, which increases readability and slightly reduces
the code-size.
This commit introduces a macro `MBEDTLS_ASN1_IS_STRING_TAG`
that can be used to check if an ASN.1 tag is among the list
of string tags:
- MBEDTLS_ASN1_BMP_STRING
- MBEDTLS_ASN1_UTF8_STRING
- MBEDTLS_ASN1_T61_STRING
- MBEDTLS_ASN1_IA5_STRING
- MBEDTLS_ASN1_UNIVERSAL_STRING
- MBEDTLS_ASN1_PRINTABLE_STRING
- MBEDTLS_ASN1_BIT_STRING
`x509_get_attr_type_value()` checks for the presence of a tag byte
and reads and stores it before calling `mbedtls_asn1_get_tag()` which
fails if either the tag byte is not present or not as expected. Therefore,
the manual check can be removed and left to `mbedtls_asn1_get_tag()`, and
the tag can be hardcoded after the call succeeded. This saves a few bytes
of code.
The server-side routine `ssl_pick_cert()` is responsible for
picking a suitable CRT from the list of CRTs configured on the
server. For that, it previously used the public key context
from the certificate to check whether its type (including the
curve type for ECC keys) suits the ciphersuite and the client's
preferences.
This commit changes the code to instead use the PK context
holding the corresponding private key. For inferring the type
of the key, this makes no difference, and it removes a PK-from-CRT
extraction step which, if CRTs are stored raw, is costly in terms
of computation and memory: CRTs need to be parsed, and memory needs
to be allocated for the PK context.
The server-side routine `ssl_decrypt_encrypted_pms()` is
responsible for decrypting the RSA-encrypted PMS in case of
an RSA-based ciphersuite.
Previously, the code checked that the length of the PMS sent
by the client matches the bit length of the RSA key. This commit
removes this check -- thereby removing the need to access the
server's own CRT -- because the RSA decryption routine performs
this check itself, too.
`mbedtls_x509_name` and `mbedtls_x509_sequence` are dynamically allocated
linked lists that need a loop to free properly. Introduce a static helper
function to do that and use it in `mbedtls_x509_crt_free()`, where the
CRT's issuer and subject names (of type `mbedtls_x509_name`) and the
SubjectAlternativeName and ExtendedKeyUsage extensions (of type
`mbedtls_x509_sequence`) need freeing. Increases code-clarity and saves
a few bytes of flash.
If the ExtendedMasterSecret extension is configured at compile-time
by setting MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET and/or
MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET, the runtime
configuration APIs mbedtls_ssl_conf_extended_master_secret()
and mbedtls_ssl_conf_extended_master_secret_enforce() must
either be removed or modified to take no effect (or at most
check that the runtime value matches the hardcoded one, but
that would undermine the code-size benefits the hardcoding
is supposed to bring in the first place).
Previously, the API was kept but modified to have no effect.
While convenient for us because we don't have to adapt example
applications, this comes at the danger of users calling the runtime
configuration API, forgetting that the respective fields are
potentially already hardcoded at compile-time - and hence silently
using a configuration they don't intend to use.
This commit changes the approach to removing the configuration
API in case the respective field is hardcoded at compile-time,
and exemplifies it in the only case implemented so far, namely
the configuration of the ExtendedMasterSecret extension.
It adapts ssl_client2 and ssl_server2 by omitting the call to
the corresponding API if MBEDTLS_SSL_CONF_XXX are defined and
removing the command line parameters for the runtime configuration
of the ExtendedMasterSecret extension.
`mbedtls_ssl_handshake_params::extended_ms` holds the state of the
ExtendedMasterSecret extension in the current handshake. Initially
set to 'disabled' for both client and server,
- the client sets it to 'enabled' as soon as it finds the ExtendedMS
extension in the `ServerHello` and it has advertised that extension
in its ClientHello,
- the server sets it to 'enabled' as soon as it finds the ExtendedMS
extension in the `ClientHello` and is willing to advertise is in its
`ServerHello`.
This commit slightly restructures this logic in prepraration for the
removal of `mbedtls_ssl_handshake_params::extended_ms` in case both
the use and the enforcement of the ExtendedMasterSecret extension have
been fixed at compile-time. Namely, in this case there is no need for
the `extended_ms` field in the handshake structure, as the ExtendedMS
must be in use if the handshake progresses beyond the Hello stage.
Paving the way for the removal of mbedtls_ssl_handshake_params::extended_ms
this commit introduces a temporary variable tracking the presence of the
ExtendedMS extension in the ClientHello/ServerHello messages, leaving
the derivation of `extended_ms` (and potential failure) to the end of
the parsing routine.
This commit is the first in a series demonstrating how code-size
can be reduced by hardcoding parts of the SSL configuration at
compile-time, focusing on the example of the configuration of
the ExtendedMasterSecret extension.
The flexibility of an SSL configuration defined a runtime vs.
compile-time is necessary for the use of Mbed TLS as a
dynamically linked library, but is undesirable in constrained
environments because it introduces the following overhead:
- Definition of SSL configuration API (code-size overhead)
(and on the application-side: The API needs to be called)
- Additional fields in the SSL configuration (RAM overhead,
and potentially code-size overhead if structures grow
beyond immediate-offset bounds).
- Dereferencing is needed to obtain configuration settings.
- Code contains branches and potentially additional structure
fields to distinguish between different configurations.
Considering the example of the ExtendedMasterSecret extension,
this instantiates as follows:
- mbedtls_ssl_conf_extended_master_secret() and
mbedtls_ssl_conf_extended_master_secret_enforced()
are introduced to configure the ExtendedMasterSecret extension.
- mbedtls_ssl_config contains bitflags `extended_ms` and
`enforce_extended_master_secret` reflecting the runtime
configuration of the ExtendedMasterSecret extension.
- Whenever we need to access these fields, we need a chain
of dereferences `ssl->conf->extended_ms`.
- Determining whether Client/Server should write the
ExtendedMasterSecret extension needs a branch
depending on `extended_ms`, and the state of the
ExtendedMasterSecret negotiation needs to be stored in a new
handshake-local variable mbedtls_ssl_handshake_params::extended_ms.
Finally (that's the point of ExtendedMasterSecret) key derivation
depends on this handshake-local state of ExtendedMasterSecret.
All this is unnecessary if it is known at compile-time that the
ExtendedMasterSecret extension is used and enforced:
- No API calls are necessary because the configuration is fixed
at compile-time.
- No SSL config fields are necessary because there are corresponding
compile-time constants instead.
- Accordingly, no dereferences for field accesses are necessary,
and these accesses can instead be replaced by the corresponding
compile-time constants.
- Branches can be eliminated at compile-time because the compiler
knows the configuration. Also, specifically for the ExtendedMasterSecret
extension, the field `extended_ms` in the handshake structure
is unnecessary, because we can fail immediately during the Hello-
stage of the handshake if the ExtendedMasterSecret extension
is not negotiated; accordingly, the non-ExtendedMS code-path
can be eliminated from the key derivation logic.
A way needs to be found to allow fixing parts of the SSL configuration
at compile-time which removes this overhead in case it is used,
while at the same time maintaining readability and backwards
compatibility.
This commit proposes the following approach:
From the user perspective, for aspect of the SSL configuration
mbedtls_ssl_config that should be configurable at compile-time,
introduce a compile-time option MBEDTLS_SSL_CONF_FIELD_NAME.
If this option is not defined, the field is kept and configurable
at runtime as usual. If the option is defined, the field is logically
forced to the value of the option at compile time.
Internally, read-access to fields in the SSL configuration which are
configurable at compile-time gets replaced by new `static inline` getter
functions which evaluate to the corresponding field access or to the
constant MBEDTLS_SSL_CONF_FIELD_NAME, depending on whether the latter
is defined or not.
Write-access to fields which are configurable at compile-time needs
to be removed: Specifically, the corresponding API itself either
needs to be removed or replaced by a stub function without effect.
This commit takes the latter approach, which has the benefit of
not requiring any change on the example applications, but introducing
the risk of mismatching API calls and compile-time configuration,
in case a user doesn't correctly keep track of which parts of the
configuration have been fixed at compile-time, and which haven't.
Write-access for the purpose of setting defaults is simply omitted.
* restricted/pr/582:
Add a test for signing content with a long ECDSA key
Add documentation notes about the required size of the signature buffers
Add missing MBEDTLS_ECP_C dependencies in check_config.h
Change size of preallocated buffer for pk_sign() calls
* origin/pr/2714:
programs: Make `make clean` clean all programs always
ssl_tls: Enable Suite B with subset of ECP curves
windows: Fix Release x64 configuration
timing: Remove redundant include file
net_sockets: Fix typo in net_would_block()
* origin/pr/2701:
Add all.sh component that exercises invalid_param checks
Remove mbedtls_param_failed from programs
Make it easier to define MBEDTLS_PARAM_FAILED as assert
Make test suites compatible with #include <assert.h>
Pass -m32 to the linker as well
If `MBEDTLS_SSL_KEEP_PEER_CERTIFICATE` is not set, `mbedtls_ssl_session`
contains the digest of the peer's certificate for the sole purpose of
detecting a CRT change on renegotiation. Hence, it is not needed if
renegotiation is disabled.
This commit removes the `peer_cert_digest` fields (and friends) from
`mbedtls_ssl_session` if
`!MBEDTLS_SSL_KEEP_PEER_CERTIFICATE + !MBEDTLS_SSL_RENEGOTIATION`,
which is a sensible configuration for constrained devices.
Apart from straightforward replacements of
`if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)`
by
`if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) && \
defined(MBEDTLS_SSL_RENEGOTIATION)`,
there's one notable change: On the server-side, the CertificateVerify
parsing function is a no-op if the client hasn't sent a certificate.
So far, this was determined by either looking at the peer CRT or the
peer CRT digest in the SSL session structure (depending on the setting
of `MBEDTLS_SSL_KEEP_PEER_CERTIFICATE`), which now no longer works if
`MBEDTLS_SSL_KEEP_PEER_CERTIFICATE` is unset. Instead, this function
now checks whether the temporary copy of the peer's public key within
the handshake structure is initialized or not (which is also a
beneficial simplification in its own right, because the pubkey is
all the function needs anyway).
The previous placing of the return statement made it look like there
are configurations for which no return statement is emitted; while
that's not true (if this function is used, at least some version of
TLS must be enabled), it's still clearer to move the failing return
statement to outside of all preprocessor guards.
If we don't need to store the peer's CRT chain permanently, we may
free it immediately after verifying it. Moreover, since we parse the
CRT chain in-place from the input buffer in this case, pointers from
the CRT structure remain valid after freeing the structure, and we
use that to extract the digest and pubkey from the CRT after freeing
the structure.
It is used in `mbedtls_ssl_session_free()` under
`MBEDTLS_X509_CRT_PARSE_C`, but defined only if
`MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED`.
Issue #2422 tracks the use of
`MBEDTLS_KEY_EXCHANGE__WITH_CERT_ENABLED` instead of
`MBEDTLS_X509_CRT_PARSE_C` for code and fields
related to CRT-based ciphersuites.
The server expects a CertificateVerify message only if it has
previously received a Certificate from the client.
So far, this was detected by looking at the `peer_cert` field
in the current session. Preparing to remove the latter, this
commit changes this to instead determine the presence of a peer
certificate by checking the new `peer_cert_digest` pointer.
We must dispatch between the peer's public key stored as part of
the peer's CRT in the current session structure (situation until
now, and future behaviour if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is
enabled), and the sole public key stored in the handshake structure
(new, if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is disabled).
We must dispatch between the peer's public key stored as part of
the peer's CRT in the current session structure (situation until
now, and future behaviour if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is
enabled), and the sole public key stored in the handshake structure
(new, if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is disabled).
We must dispatch between the peer's public key stored as part of
the peer's CRT in the current session structure (situation until
now, and future behaviour if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is
enabled), and the sole public key stored in the handshake structure
(new, if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is disabled).
We must dispatch between the peer's public key stored as part of
the peer's CRT in the current session structure (situation until
now, and future behaviour if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is
enabled), and the sole public key stored in the handshake structure
(new, if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is disabled).
This commit modifies `mbedtls_ssl_parse_certificate()` to store a
copy of the peer's public key after parsing and verifying the peer's
CRT chain.
So far, this leads to heavy memory duplication: We have the CRT chain
in the I/O buffer, then parse (and, thereby, copy) it to a
`mbedtls_x509_crt` structure, and then make another copy of the
peer's public key, plus the overhead from the MPI and ECP structures.
This inefficiency will soon go away to a significant extend, because:
- Another PR adds functionality to parse CRTs without taking
ownership of the input buffers. Applying this here will allow
parsing and verifying the peer's chain without making an additional
raw copy. The overhead reduces to the size of `mbedtls_x509_crt`,
the public key, and the DN structures referenced in the CRT.
- Once copyless parsing is in place and the removal of the peer CRT
is fully implemented, we can extract the public key bounds from
the parsed certificate and then free the entire chain before
parsing the public key again. This means that we never store
the parsed public key twice at the same time.
When removing the (session-local) copy of the peer's CRT chain, we must
keep a handshake-local copy of the peer's public key, as (naturally) every
key exchange will make use of that public key at some point to verify that
the peer actually owns the corresponding private key (e.g., verify signatures
from ServerKeyExchange or CertificateVerify, or encrypt a PMS in a RSA-based
exchange, or extract static (EC)DH parameters).
This commit adds a PK context field `peer_pubkey` to the handshake parameter
structure `mbedtls_handshake_params_init()` and adapts the init and free
functions accordingly. It does not yet make actual use of the new field.
This commit adds an ASN.1 buffer field `pk_raw` to `mbedtls_x509_crt`
which stores the bounds of the raw public key data within an X.509 CRT.
This will be useful in subsequent commits to extract the peer's public
key from its certificate chain.
This commit changes the format of session tickets to include
the digest of the peer's CRT if MBEDTLS_SSL_KEEP_PEER_CERTIFICATE
is disabled.
This commit does not yet remove the peer CRT itself.
`mbedtls_ssl_parse_certificate()` parses the peer's certificate chain
directly into the `peer_cert` field of the `mbedtls_ssl_session`
structure being established. To allow to optionally remove this field
from the session structure, this commit changes this to parse the peer's
chain into a local variable instead first, which can then either be freed
after CRT verification - in case the chain should not be stored - or
mapped to the `peer_cert` if it should be kept. For now, only the latter
is implemented.
