-
Notifications
You must be signed in to change notification settings - Fork 96
Test a build without any asymmetric cryptography #298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test a build without any asymmetric cryptography #298
Conversation
13f2acc
to
d47e792
Compare
Add a reference configuration with most symmetric cryptographic algorithms enabled, but without any asymmetric cryptography. This checks that we don't have spurious unexpected dependencies on asymmetric-only modules such as bignum. Keep HAVE_ASM disabled because it's platform-specific. Keep HAVEGE disabled because it's untested and not recommended. Keep MEMORY_BUFFER_ALLOC out because it isn't related to cryptography at all and it makes memory sanitizers ineffective. Keep THREADING disabled because it requires special build options.
config-symmetric-only.h enables MBEDTLS_ENTROPY_NV_SEED so it needs a seedfile. Create it in test-ref-configs.pl so that the script works on its own, even if it is not invoked by all.sh.
d47e792
to
581bfcf
Compare
#define MBEDTLS_CONFIG_H | ||
|
||
/* System support */ | ||
//#define MBEDTLS_HAVE_ASM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this line left in, even though it's commented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a couple of options commented out because I thought that they were options that should not be enabled in our test configuration, but could be useful to users who want to make a symmetric-only configuration tuned to their use case, and use this file as a starting point.
psa_storage_uid_t uid = file_uid_for_lifetime( lifetime ); | ||
struct psa_storage_info_t info; | ||
uint8_t *loaded = NULL; | ||
int ok = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In such cases I'd rather use "is_ok" to show that it's a boolean, but that's an opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int ok
is what I've used in similar support functions before, so I used the same style here for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although the submitted fixes aren't directly related to this PR, I'm fine with them going in as-is, since they're small.
Enabling memory_buffer_alloc is slow and makes ASan ineffective. We have a patch pending to remove it from the full config. In the meantime, disable it explicitly.
New CI run now that the problems with the Cypress boards are fixed: https://jenkins-internal.mbed.com/job/mbed-crypto-pr/job/PR-298-merge/15/ → PASS |
* ARMmbed#292: Make psa_close_key(0) and psa_destroy_key(0) succeed * ARMmbed#299: Allow xxx_drbg_set_entropy_len before xxx_drbg_seed * ARMmbed#259: Check `len` against buffers size upper bound in PSA tests * ARMmbed#288: Add ECDSA tests with hash and key of different lengths * ARMmbed#305: CTR_DRBG: grab a nonce from the entropy source if needed * ARMmbed#316: Stop transactions from being reentrant * ARMmbed#317: getting_started: Make it clear that keys are passed in * ARMmbed#314: Fix pk_write with EC key to use a constant size for the private value * ARMmbed#298: Test a build without any asymmetric cryptography * ARMmbed#284: Fix some possibly-undefined variable warnings * ARMmbed#315: Define MBEDTLS_PK_SIGNATURE_MAX_SIZE * ARMmbed#318: Finish side-porting commits from mbedtls-restricted that missed the split
Fix #296.
Also fix two bugs that the tests revealed:
component_test_se_full
wasn't using the full config, and there was a memory leak in an SE HAL test (fix duplicated in #304).