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.
Previously, a call to mbedtls_x509_crt_xxx_release() would return
MBEDTLS_ERR_THREADING_MUTEX_ERROR if usage counter for the frame/PK
was 0. Now that resource counting can also be used outside of
threading support, this is no longer adequate, and this commit
changes the return code to MBEDTLS_ERR_X509_FATAL_ERROR; while
generic, this at least matches the top-level module.
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.
The previous tests used 100 parallel workers which for EC certificates
leads to a memory usage of more than 1Mb, hence leading to an out of memory
condition in tests using the memory buffer allocator which has a pool of 1Mb.
Use 25 workers but an increased number of iterations per worker instead.
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 allows to build the library + tests via `make` without
specifying `PTHREAD=1`, in which case the X.509 threading
test suite will be silently dropped.
This is analogous to the pre-existing handling of the example
application `ssl_pthread_server`, which is only build if `PTHREAD=1`
and silently dropped otherwise.
The pre-existing LINK_WITH_PTHREAD CMake option controls whether
`pthread` should be linked into the library, but didn't apply
to the test suites so far.
This commit also links test suites to `pthread` in CMake-based
builds which have LINK_WITH_PTHREAD set.
This commit enhances the X.509 parsing test suite by a test
which exercises multiple threads concurrently verifying the
same certificate with the same set of trusted roots.
In contrast to mbedtls_x509_crt_frame_acquire(), the public key context
returned by mbedtls_x509_crt_pk_acquire() cannot be marked `const` because
the caller must be able to use it e.g. for mbedtls_pk_sign() and
mbedtls_pk_verify(), which don't have `const` input parameters.
Instead, return a non-`const` context, but explicitly state that callers
must use that context in a thread-safe way.
We cannot move it to x509_crt.c because there are some static inline
function definitions in x509_crt.h which access members of
mbedtls_x509_crt_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.
