Side-port from crypto: programs/ #3038
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes it easier to add or remove programs as well as see which programs were added or removed in diffs. Side port of 30fae8e in mbed-crypto.
There is some commented out X.509 certificate writing code present in rsa_genkey. It looks like it has been commented out since the beginning of time. Let's remove it, since commented out code is not in good style.
As the SSL programs, like ssl_client2 and ssl_server2, are dependent on SSL and therefore about to be removed, the only consumer of query_config is the query_compile_time_config test. As such, it makes sense to move query_config to be next to what uses it.
When building with CMake, for sample programs that only use functionality in libmbedcrypto (i.e. crypto and platform), link with libmbedcrypto, not with libmbedtls. This doesn't change the result, because the linker skips libraries in which no symbol is used, but it changes the build dependencies, and it has the advantage of bringing programs/*/CMakeLists.txt closer to the corresponding files under crypto/. The programs concerned are crypto sample and test programs, and programs that only use (potential) platform functions such as mbedtls_printf. dh_client and dh_server keep linking with mbedtls because they use functions from the net_sockets module.
When building with CMake, for sample programs that only use functionality in libmbedcrypto and libmbedx509, link with libmbedx509, not with libmbedtls. cert_app makes a TLS connection, so do link it with libmbedtls.
You can't reuse a CTR_DRBG context without free()ing it and re-init()ing. This generally happened to work, but was never guaranteed. It could have failed with alternative implementations of the AES module because mbedtls_ctr_drbg_seed() calls mbedtls_aes_init() on a context which is already initialized if mbedtls_ctr_drbg_seed() hasn't been called before, plausibly causing a memory leak. Calling free() and seed() with no intervening init fails when MBEDTLS_THREADING_C is enabled and all-bits-zero is not a valid mutex representation. So add the missing free() and init().
Add a very basic test of calloc to the selftest program. The selftest program acts in its capacity as a platform compatibility checker rather than in its capacity as a test of the library. The main objective is to report whether calloc returns NULL for a size of 0. Also observe whether a free/alloc sequence returns the address that was just freed and whether a size overflow is properly detected.
Exercise the library functions with calloc returning NULL for a size
of 0. Make this a separate job with UBSan (and ASan) to detect
places where we try to dereference the result of calloc(0) or to do
things like
buf = calloc(size, 1);
if (buf == NULL && size != 0) return INSUFFICIENT_MEMORY;
memcpy(buf, source, size);
which has undefined behavior when buf is NULL at the memcpy call even
if size is 0.
This is needed because other test components jobs either use the system
malloc which returns non-NULL on Linux and FreeBSD, or the
memory_buffer_alloc malloc which returns NULL but does not give as
useful feedback with ASan (because the whole heap is a single C
object).
gilles-peskine-arm
added
enhancement
mbed TLS team
component-crypto
Contributor
I think this figure makes things look less encouraging that they are, by counting context lines. Using git diff stat makes things look brighter: |
gilles-peskine-arm
added
needs-review
yanesca
added
approved
This was referenced Mar 4, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Side-port changes made in mbed-crypto under
programs/. The goal of this pull request is to bring in bug fixes and enhancements made in mbed-crypto to files that exist both in mbed-crypto and in mbedtls. The goal of this pull request is not to be exhaustive, though it is close apart from the build scripts, generated files, and references to files that only exist on one side.programs/Makefilereproduced from 30fae8e from Break non-crypto dependencies ARMmbed/mbed-crypto#74.CMakeLists.txt, don't link x509-only programs withlibmbedtls. The linker would optimize it away anyway, but this is analogous to linking crypto-only programs with onlylibmbedcryptowhich mbed-crypto had to do.query_configtoprograms/test. Cherry-picked from Break non-crypto dependencies ARMmbed/mbed-crypto#74.all.sh, also run part ofssl-opt.sh.(excluding a very long automatically generated file) is now 576 lines long, which approaches something manageable.