-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Implement and use MBEDTLS_STATIC_ASSERT() #7229
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ | |
#include "mbedtls/build_info.h" | ||
#include "alignment.h" | ||
|
||
#include <assert.h> | ||
#include <stddef.h> | ||
#include <stdint.h> | ||
#include <stddef.h> | ||
|
@@ -149,4 +150,18 @@ inline void mbedtls_xor(unsigned char *r, const unsigned char *a, const unsigned | |
#endif | ||
/* *INDENT-ON* */ | ||
|
||
/* Always provide a static assert macro, so it can be used unconditionally. | ||
* It will expand to nothing on some systems. | ||
* Can be used outside functions (but don't add a trailing ';' in that case: | ||
* the semicolon is included here to avoid triggering -Wextra-semi when | ||
* MBEDTLS_STATIC_ASSERT() expands to nothing). | ||
* Can't use the C11-style `defined(static_assert)` on FreeBSD, since it | ||
* defines static_assert even with -std=c99, but then complains about it. | ||
*/ | ||
#if defined(static_assert) && !defined(__FreeBSD__) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In C23, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We've already decided not to pre-add stuff for C23 on other PRs, so let's not add the |
||
#define MBEDTLS_STATIC_ASSERT(expr, msg) static_assert(expr, msg); | ||
#else | ||
#define MBEDTLS_STATIC_ASSERT(expr, msg) | ||
#endif | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we also use this in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, should be possible, but I haven't tested it. That would make an excellent follow-up PR |
||
#endif /* MBEDTLS_LIBRARY_COMMON_H */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,7 +48,6 @@ | |
|
||
#include "psa_crypto_random_impl.h" | ||
|
||
#include <assert.h> | ||
#include <stdlib.h> | ||
#include <string.h> | ||
#include "mbedtls/platform.h" | ||
|
@@ -1471,14 +1470,15 @@ psa_status_t psa_export_public_key(mbedtls_svc_key_id_t key, | |
return (status == PSA_SUCCESS) ? unlock_status : status; | ||
} | ||
|
||
#if defined(static_assert) | ||
static_assert((MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE) == 0, | ||
"One or more key attribute flag is listed as both external-only and dual-use"); | ||
static_assert((PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE) == 0, | ||
"One or more key attribute flag is listed as both internal-only and dual-use"); | ||
static_assert((PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY) == 0, | ||
"One or more key attribute flag is listed as both internal-only and external-only"); | ||
#endif | ||
MBEDTLS_STATIC_ASSERT( | ||
(MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE) == 0, | ||
"One or more key attribute flag is listed as both external-only and dual-use") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Annoyingly I couldn't find a way to have the macro work outside functions where it could have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it work to make it expand to a function declaration? |
||
MBEDTLS_STATIC_ASSERT( | ||
(PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE) == 0, | ||
"One or more key attribute flag is listed as both internal-only and dual-use") | ||
MBEDTLS_STATIC_ASSERT( | ||
(PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY) == 0, | ||
"One or more key attribute flag is listed as both internal-only and external-only") | ||
|
||
/** Validate that a key policy is internally well-formed. | ||
* | ||
|
@@ -1742,11 +1742,10 @@ static psa_status_t psa_finish_key_creation( | |
psa_key_slot_number_t slot_number = | ||
psa_key_slot_get_slot_number(slot); | ||
|
||
#if defined(static_assert) | ||
static_assert(sizeof(slot_number) == | ||
sizeof(data.slot_number), | ||
"Slot number size does not match psa_se_key_data_storage_t"); | ||
#endif | ||
MBEDTLS_STATIC_ASSERT(sizeof(slot_number) == | ||
sizeof(data.slot_number), | ||
"Slot number size does not match psa_se_key_data_storage_t"); | ||
|
||
memcpy(&data.slot_number, &slot_number, sizeof(slot_number)); | ||
status = psa_save_persistent_key(&slot->attr, | ||
(uint8_t *) &data, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1510,10 +1510,9 @@ static int ssl_parse_client_hello(mbedtls_ssl_context *ssl) | |
MBEDTLS_TLS_SIG_NONE | ||
}; | ||
|
||
#if defined(static_assert) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NOTE: since there was no |
||
static_assert(sizeof(default_sig_algs) / sizeof(default_sig_algs[0]) <= | ||
MBEDTLS_RECEIVED_SIG_ALGS_SIZE, "default_sig_algs is too big"); | ||
#endif | ||
MBEDTLS_STATIC_ASSERT(sizeof(default_sig_algs) / sizeof(default_sig_algs[0]) | ||
<= MBEDTLS_RECEIVED_SIG_ALGS_SIZE, | ||
"default_sig_algs is too big"); | ||
|
||
memcpy(received_sig_algs, default_sig_algs, sizeof(default_sig_algs)); | ||
} | ||
|
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.
Oh, you didn't find a way to bypass than warning 😞 . Maybe we could detect FreeBSD and use
_Static_assert
there?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.
_Static_assert()
is only available depending what you're building for. But yes, that could be added in a follow-up PR