From 28e87731c4b2c76b855328821e070b52c535e229 Mon Sep 17 00:00:00 2001 From: Norman Ashley Date: Wed, 25 Oct 2023 09:16:29 -0400 Subject: [PATCH] =?UTF-8?q?Remove=20use=20of=20mutex=20status=20bool.=20Us?= =?UTF-8?q?e=20recursive=20mutex=E2=80=9D=20src/sig=5Fstfl/lms/sig=5Fstfl?= =?UTF-8?q?=5Flms.c=20src/sig=5Fstfl/xmss/sig=5Fstfl=5Fxmss=5Fsecret=5Fkey?= =?UTF-8?q?=5Ffunctions.c=20tests/test=5Fsig=5Fstfl.c?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/sig_stfl/lms/sig_stfl_lms.c | 60 ----------------- .../xmss/sig_stfl_xmss_secret_key_functions.c | 21 ++---- tests/test_sig_stfl.c | 65 +++++++++++-------- 3 files changed, 43 insertions(+), 103 deletions(-) diff --git a/src/sig_stfl/lms/sig_stfl_lms.c b/src/sig_stfl/lms/sig_stfl_lms.c index d5a978bfa1..2ea68ab135 100644 --- a/src/sig_stfl/lms/sig_stfl_lms.c +++ b/src/sig_stfl/lms/sig_stfl_lms.c @@ -82,9 +82,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H5_W1_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -172,9 +169,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H5_W2_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -262,9 +256,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H5_W4_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -352,9 +343,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H5_W8_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -442,9 +430,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H10_W1_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -532,9 +517,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H10_W2_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -622,9 +604,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H10_W4_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -712,9 +691,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H10_W8_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -802,9 +778,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H15_W1_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -892,9 +865,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H15_W2_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -982,9 +952,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H15_W4_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -1072,9 +1039,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H15_W8_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -1162,9 +1126,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H20_W1_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -1252,9 +1213,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H20_W2_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -1342,9 +1300,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H20_W4_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -1432,9 +1387,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H20_W8_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -1522,9 +1474,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H25_W1_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -1612,9 +1561,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H25_W2_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -1702,9 +1648,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H25_W4_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ @@ -1792,9 +1735,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_LMS_SHA256_H25_W8_new(void) { */ sk->lock_key = NULL; - /* Boolean if the secret key is locked */ - sk->is_locked = false; - /* * Set Secret Key Unlocking / Releasing Function */ diff --git a/src/sig_stfl/xmss/sig_stfl_xmss_secret_key_functions.c b/src/sig_stfl/xmss/sig_stfl_xmss_secret_key_functions.c index 5e083d3550..351695efda 100644 --- a/src/sig_stfl/xmss/sig_stfl_xmss_secret_key_functions.c +++ b/src/sig_stfl/xmss/sig_stfl_xmss_secret_key_functions.c @@ -52,9 +52,6 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_XMSS_new(size_t length_secret_key) { // Set Secret Key unlocking / releasing function sk->unlock_key = NULL; - // Boolean if the secret key is locked - sk->is_locked = false; - // Set Secret Key saving function sk->secure_store_scrt_key = NULL; @@ -127,12 +124,9 @@ void OQS_SECRET_KEY_XMSS_activate_lock(OQS_SIG_STFL_SECRET_KEY *sk) { return; } - if (sk->is_locked == false) { - /* Lock the key if possible */ - if ((sk->lock_key != NULL) && (sk->mutex != NULL)) { - sk->lock_key(sk->mutex); - sk->is_locked = true; - } + /* Lock the key if possible */ + if ((sk->lock_key != NULL) && (sk->mutex != NULL)) { + sk->lock_key(sk->mutex); } } @@ -141,11 +135,8 @@ void OQS_SECRET_KEY_XMSS_activate_unlock(OQS_SIG_STFL_SECRET_KEY *sk) { return; } - if (sk->is_locked == true) { - /* Unlock the key if possible */ - if ((sk->unlock_key != NULL) && (sk->mutex != NULL)) { - sk->unlock_key(sk->mutex); - sk->is_locked = false; - } + /* Unlock the key if possible */ + if ((sk->unlock_key != NULL) && (sk->mutex != NULL)) { + sk->unlock_key(sk->mutex); } } diff --git a/tests/test_sig_stfl.c b/tests/test_sig_stfl.c index 428bf35697..530f67383d 100644 --- a/tests/test_sig_stfl.c +++ b/tests/test_sig_stfl.c @@ -19,6 +19,7 @@ #include static pthread_mutex_t *test_sk_lock = NULL; +static pthread_mutex_t *sk_lock = NULL; #endif #ifdef OQS_ENABLE_TEST_CONSTANT_TIME @@ -247,7 +248,7 @@ OQS_STATUS sig_stfl_keypair_from_KATs(OQS_SIG_STFL *sig, uint8_t *public_key, OQ * XMSSMT-SHAKE_60/3_256 */ OQS_STATUS sig_stfl_KATs_keygen(OQS_SIG_STFL *sig, uint8_t *public_key, OQS_SIG_STFL_SECRET_KEY *secret_key, const char *katfile) { - if (sig == NULL || public_key == NULL || secret_key == NULL || katfile == NULL) { + if (sig == NULL || public_key == NULL || secret_key == NULL ) { return OQS_ERROR; } @@ -386,10 +387,6 @@ static OQS_STATUS sig_stfl_test_correctness(const char *method_name, const char magic_t magic; -#if OQS_USE_PTHREADS_IN_TESTS - pthread_mutex_t *sk_lock = NULL; -#endif - OQS_STATUS rc, ret = OQS_ERROR; //The magic numbers are random values. @@ -423,14 +420,6 @@ static OQS_STATUS sig_stfl_test_correctness(const char *method_name, const char OQS_SIG_STFL_SECRET_KEY_SET_store_cb(secret_key, save_secret_key, (void *)context); #if OQS_USE_PTHREADS_IN_TESTS - sk_lock = (pthread_mutex_t *)malloc(sizeof(pthread_mutex_t)); - if (sk_lock == NULL) { - goto err; - } - - if (0 != pthread_mutex_init(sk_lock, 0)) { - goto err; - } OQS_SIG_STFL_SECRET_KEY_SET_mutex(secret_key, sk_lock); #endif public_key = malloc(sig->length_public_key + 2 * sizeof(magic_t)); @@ -559,12 +548,6 @@ static OQS_STATUS sig_stfl_test_correctness(const char *method_name, const char OQS_MEM_insecure_free(context); OQS_MEM_insecure_free(file_store); -#if OQS_USE_PTHREADS_IN_TESTS - if (sk_lock) { - pthread_mutex_destroy(sk_lock); - OQS_MEM_insecure_free(sk_lock); - } -#endif return ret; } @@ -902,15 +885,6 @@ static OQS_STATUS sig_stfl_test_secret_key_lock(const char *method_name, const c OQS_SIG_STFL_SECRET_KEY_SET_unlock(lock_test_sk, unlock_sk_key); #if OQS_USE_PTHREADS_IN_TESTS - - test_sk_lock = (pthread_mutex_t *)malloc(sizeof(pthread_mutex_t)); - if (test_sk_lock == NULL) { - goto err; - } - - if (0 != pthread_mutex_init(test_sk_lock, 0)) { - goto err; - } OQS_SIG_STFL_SECRET_KEY_SET_mutex(lock_test_sk, test_sk_lock); #endif @@ -1063,6 +1037,33 @@ int main(int argc, char **argv) { td_create.katfile = katfile; td_sign.katfile = katfile; td_query.katfile = katfile; + pthread_mutexattr_t attr1, attr2; + + test_sk_lock = (pthread_mutex_t *)malloc(sizeof(pthread_mutex_t)); + if (test_sk_lock == NULL) { + goto err; + } + sk_lock = (pthread_mutex_t *)malloc(sizeof(pthread_mutex_t)); + if (sk_lock == NULL) { + goto err; + } + + if (0 != pthread_mutexattr_init(&attr1)) { + goto err; + } + if (0 != pthread_mutexattr_init(&attr2)) { + goto err; + } + + pthread_mutexattr_settype(&attr1, PTHREAD_MUTEX_RECURSIVE); + pthread_mutexattr_settype(&attr2, PTHREAD_MUTEX_RECURSIVE); + + if (0 != pthread_mutex_init(test_sk_lock, &attr1)) { + goto err; + } + if (0 != pthread_mutex_init(test_sk_lock, &attr2)) { + goto err; + } int trc = pthread_create(&thread, NULL, test_wrapper, &td); if (trc) { @@ -1100,6 +1101,14 @@ int main(int argc, char **argv) { } pthread_join(query_key_thread, NULL); rc_qry = td_query.rc; + +err: + if (test_sk_lock) { + pthread_mutex_destroy(test_sk_lock); + } + if (sk_lock) { + pthread_mutex_destroy(sk_lock); + } #else rc = sig_stfl_test_correctness(alg_name, katfile); rc1 = sig_stfl_test_secret_key(alg_name, katfile);