Skip to content

rsa: hold the key mutex on every shared-key wolfCrypt call #487

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.

Sign up for GitHub

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

Open
yosuke-wolfssl wants to merge 1 commit into
base: master
Choose a base branch
from
Open

rsa: hold the key mutex on every shared-key wolfCrypt call #487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 21 additions & 11 deletions src/wp_rsa_asym.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,12 +311,22 @@ static int wp_rsaa_encrypt(wp_RsaAsymCtx* ctx, unsigned char* out,
}
else {
int rc = 0;
int locked = 0;

if (outSize == (size_t)-1) {
outSize = *outLen;
}
if ((ctx->padMode == RSA_PKCS1_PADDING) ||
(ctx->padMode == RSA_PKCS1_WITH_TLS_PADDING)) {
if (wp_lock(wp_rsa_get_mutex(ctx->rsa)) != 1) {
ok = 0;
}
else {
locked = 1;
}
if (!ok) {
/* Mutex not held - skip rather than run the operation unlocked. */
}
else if ((ctx->padMode == RSA_PKCS1_PADDING) ||
(ctx->padMode == RSA_PKCS1_WITH_TLS_PADDING)) {
rc = wc_RsaPublicEncrypt(in, (word32)inLen, out, (word32)outSize,
wp_rsa_get_key(ctx->rsa), &ctx->rng);
if (rc < 0) {
Expand Down Expand Up @@ -354,6 +364,9 @@ static int wp_rsaa_encrypt(wp_RsaAsymCtx* ctx, unsigned char* out,
else {
ok = 0;
}
if (locked) {
wp_unlock(wp_rsa_get_mutex(ctx->rsa));
}
if (ok) {
*outLen = rc;
}
Expand Down Expand Up @@ -433,31 +446,28 @@ static int wp_rsaa_decrypt(wp_RsaAsymCtx* ctx, unsigned char* out,
}
else {
int rc = 0;
#ifdef WC_RSA_BLINDING
int locked = 0;
#endif

if (outSize == (size_t)-1) {
outSize = *outLen;
}
#ifdef WC_RSA_BLINDING
/* Fail closed if the key mutex can't be held for the shared-key RNG. */
if (wp_lock(wp_rsa_get_mutex(ctx->rsa)) != 1) {
ok = 0;
}
else {
locked = 1;
#ifdef WC_RSA_BLINDING
rc = wc_RsaSetRNG(wp_rsa_get_key(ctx->rsa), &ctx->rng);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaSetRNG", rc);
ok = 0;
}
#endif /* WC_RSA_BLINDING */
}
if (!ok) {
/* Lock or RNG setup failed - skip the operation. */
}
else
#endif /* WC_RSA_BLINDING */
if (ctx->padMode == RSA_PKCS1_PADDING) {
else if (ctx->padMode == RSA_PKCS1_PADDING) {
PRIVATE_KEY_UNLOCK();
rc = wc_RsaPrivateDecrypt(in, (word32)inLen, out, (word32)outSize,
wp_rsa_get_key(ctx->rsa));
Expand Down Expand Up @@ -546,12 +556,12 @@ static int wp_rsaa_decrypt(wp_RsaAsymCtx* ctx, unsigned char* out,
else {
ok = 0;
}
#ifdef WC_RSA_BLINDING
if (locked) {
#ifdef WC_RSA_BLINDING
wc_RsaSetRNG(wp_rsa_get_key(ctx->rsa), NULL);
#endif
wp_unlock(wp_rsa_get_mutex(ctx->rsa));
}
#endif
if (ok) {
*outLen = rc;
}
Expand Down
41 changes: 28 additions & 13 deletions src/wp_rsa_kem.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,16 +344,24 @@ static int wp_rsasve_generate(wp_RsaKemCtx* ctx, unsigned char* out,
}
if (ok && (out != NULL)) {
/* Step 3: out = RSAEP((n,e), z) */
int rc;
int rc = 0;

oLen = nLen;
rc = wc_RsaDirect(secret, nLen, out, &oLen, rsa, RSA_PUBLIC_ENCRYPT,
&ctx->rng);
if (rc < 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaDirect", rc);
if (wp_lock(wp_rsa_get_mutex(ctx->rsa)) != 1) {
OPENSSL_cleanse(secret, nLen);
ok = 0;
}
if (ok) {
rc = wc_RsaDirect(secret, nLen, out, &oLen, rsa,
RSA_PUBLIC_ENCRYPT, &ctx->rng);
wp_unlock(wp_rsa_get_mutex(ctx->rsa));
if (rc < 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaDirect",
rc);
OPENSSL_cleanse(secret, nLen);
ok = 0;
}
}
/* Front pad output with zeros if required. */
if (ok && (oLen < nLen)) {
word32 padLen = nLen - oLen;
Expand Down Expand Up @@ -450,16 +458,23 @@ static int wp_rsasve_recover(wp_RsaKemCtx* ctx, unsigned char* out,
/* Step 3: out = RSADP((n,d), in) */
if (ok && (out != NULL)) {
word32 oLen = nLen;
int rc;

PRIVATE_KEY_UNLOCK();
rc = wc_RsaDirect((byte*)in, (word32)inLen, out, &oLen, rsa,
RSA_PRIVATE_DECRYPT, &ctx->rng);
PRIVATE_KEY_LOCK();
if (rc < 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaDirect decrypt", rc);
int rc = 0;

if (wp_lock(wp_rsa_get_mutex(ctx->rsa)) != 1) {
ok = 0;
}
if (ok) {
PRIVATE_KEY_UNLOCK();
rc = wc_RsaDirect((byte*)in, (word32)inLen, out, &oLen, rsa,
RSA_PRIVATE_DECRYPT, &ctx->rng);
PRIVATE_KEY_LOCK();
wp_unlock(wp_rsa_get_mutex(ctx->rsa));
if (rc < 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG,
"wc_RsaDirect decrypt", rc);
ok = 0;
}
}
/* Front pad output with zeros if required. */
if (ok && (oLen < nLen)) {
word32 padLen = nLen - oLen;
Expand Down
17 changes: 13 additions & 4 deletions src/wp_rsa_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ struct wp_Rsa {
RsaKey key;

#ifndef WP_SINGLE_THREADED
/** Mutex for reference count updating. */
/** Held while refCnt changes and while a wolfCrypt call uses key. */
wolfSSL_Mutex mutex;
#endif
/** Count of references to this object. */
Expand Down Expand Up @@ -1280,11 +1280,20 @@ static int wp_rsa_validate(const wp_Rsa* rsa, int selection, int checkType)

#ifdef WOLFSSL_RSA_KEY_CHECK
if (checkPub && checkPriv) {
int rc = wc_CheckRsaKey((RsaKey*)&rsa->key);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_CheckRsaKey", rc);
int rc = 0;

if (wp_lock(wp_rsa_get_mutex((wp_Rsa*)rsa)) != 1) {
ok = 0;
}
if (ok) {
rc = wc_CheckRsaKey((RsaKey*)&rsa->key);
wp_unlock(wp_rsa_get_mutex((wp_Rsa*)rsa));
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_CheckRsaKey",
rc);
ok = 0;
}
}

/* wc_CheckRsaKey runs the private key operation and only bounds-checks
* d (d < n). Check d*e = 1 mod lcm(p-1,q-1) to match OSSL */
Expand Down
71 changes: 53 additions & 18 deletions src/wp_rsa_sig.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,7 @@ static int wp_rsa_verify_pkcs1(wp_RsaSigCtx* ctx, const unsigned char* sig,
unsigned char* decryptedSig)
{
int ok = 1;
int rc;
int rc = 0;
unsigned char* encodedDigest = NULL;
int encodedDigestLen = 0;

Expand All @@ -1094,12 +1094,19 @@ static int wp_rsa_verify_pkcs1(wp_RsaSigCtx* ctx, const unsigned char* sig,
return 0;
}

rc = wc_RsaSSL_Verify(sig, (word32)sigLen, decryptedSig, (word32)sigLen,
wp_rsa_get_key(ctx->rsa));
if (rc < 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaSSL_Verify", rc);
if (wp_lock(wp_rsa_get_mutex(ctx->rsa)) != 1) {
ok = 0;
}
if (ok) {
rc = wc_RsaSSL_Verify(sig, (word32)sigLen, decryptedSig, (word32)sigLen,
wp_rsa_get_key(ctx->rsa));
wp_unlock(wp_rsa_get_mutex(ctx->rsa));
if (rc < 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaSSL_Verify",
rc);
ok = 0;
}
}

if (ok && ((size_t)rc > tbsLen)) {
encodedDigest = (unsigned char*)OPENSSL_malloc(MAX_DER_DIGEST_SZ);
Expand Down Expand Up @@ -1152,8 +1159,9 @@ static int wp_rsa_verify_pss(wp_RsaSigCtx* ctx, const unsigned char* sig,
unsigned char* decryptedSig)
{
int ok = 1;
int rc;
int saltLen;
int rc = 0;
int saltLen = 0;
int locked = 0;

WOLFPROV_ENTER(WP_LOG_COMP_RSA, "wp_rsa_verify_pss");

Expand All @@ -1178,6 +1186,14 @@ static int wp_rsa_verify_pss(wp_RsaSigCtx* ctx, const unsigned char* sig,
#endif
wp_rsa_get_key(ctx->rsa), EVP_PKEY_OP_VERIFY);

if (wp_lock(wp_rsa_get_mutex(ctx->rsa)) != 1) {
ok = 0;
}
else {
locked = 1;
}
}
if (ok) {
rc = wc_RsaPSS_Verify_ex((byte*)sig, (word32)sigLen, decryptedSig,
(word32)sigLen,
#if LIBWOLFSSL_VERSION_HEX >= 0x05007004
Expand All @@ -1192,6 +1208,9 @@ static int wp_rsa_verify_pss(wp_RsaSigCtx* ctx, const unsigned char* sig,
ok = 0;
}
}
if (locked) {
wp_unlock(wp_rsa_get_mutex(ctx->rsa));
}
if (ok) {
rc = wc_RsaPSS_CheckPadding_ex(tbs, (word32)tbsLen, decryptedSig, rc,
#if LIBWOLFSSL_VERSION_HEX >= 0x05007004
Expand Down Expand Up @@ -1227,17 +1246,23 @@ static int wp_rsa_verify_no_pad(wp_RsaSigCtx* ctx, const unsigned char* sig,
unsigned char* decryptedSig)
{
int ok = 1;
int rc;
int rc = 0;
word32 len = (word32)sigLen;

WOLFPROV_ENTER(WP_LOG_COMP_RSA, "wp_rsa_verify_no_pad");

rc = wc_RsaDirect((byte*)sig, (word32)sigLen, decryptedSig, &len,
wp_rsa_get_key(ctx->rsa), RSA_PUBLIC_DECRYPT, &ctx->rng);
if (rc < 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaDirect", rc);
if (wp_lock(wp_rsa_get_mutex(ctx->rsa)) != 1) {
ok = 0;
}
if (ok) {
rc = wc_RsaDirect((byte*)sig, (word32)sigLen, decryptedSig, &len,
wp_rsa_get_key(ctx->rsa), RSA_PUBLIC_DECRYPT, &ctx->rng);
wp_unlock(wp_rsa_get_mutex(ctx->rsa));
if (rc < 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaDirect", rc);
ok = 0;
}
}
if (ok && (((size_t)rc != tbsLen) || ((XMEMCMP(tbs, decryptedSig,
tbsLen) != 0)))) {
ok = 0;
Expand Down Expand Up @@ -1329,20 +1354,26 @@ static int wp_rsa_verify_x931(wp_RsaSigCtx* ctx, const unsigned char* sig,
unsigned char* decryptedSig)
{
int ok = 1;
int rc;
int rc = 0;
word32 len = (word32)sigLen;
unsigned char* unpadded = NULL;
mp_int toMp;
mp_int nMinusTo;

WOLFPROV_ENTER(WP_LOG_COMP_RSA, "wp_rsa_verify_x931");

rc = wc_RsaDirect((byte*)sig, (word32)sigLen, decryptedSig, &len,
wp_rsa_get_key(ctx->rsa), RSA_PUBLIC_DECRYPT, &ctx->rng);
if (rc < 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaDirect", rc);
if (wp_lock(wp_rsa_get_mutex(ctx->rsa)) != 1) {
ok = 0;
}
if (ok) {
rc = wc_RsaDirect((byte*)sig, (word32)sigLen, decryptedSig, &len,
wp_rsa_get_key(ctx->rsa), RSA_PUBLIC_DECRYPT, &ctx->rng);
wp_unlock(wp_rsa_get_mutex(ctx->rsa));
if (rc < 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaDirect", rc);
ok = 0;
}
}
if (ok) {
/*
* X9.31 specifies, "If e is odd, then
Expand Down Expand Up @@ -1516,7 +1547,7 @@ static int wp_rsa_verify_recover_init(wp_RsaSigCtx* ctx, wp_Rsa* rsa,
static int wp_rsa_verify_recover(wp_RsaSigCtx* ctx, unsigned char* rout,
size_t* routlen, size_t routsize, const unsigned char* sig, size_t sigLen)
{
int rc;
int rc = 0;
int ok = 1;

WOLFPROV_ENTER(WP_LOG_COMP_RSA, "wp_rsa_verify_recover");
Expand All @@ -1535,9 +1566,13 @@ static int wp_rsa_verify_recover(wp_RsaSigCtx* ctx, unsigned char* rout,
if (ok && ((!WP_FITS_WORD32(sigLen)) || (!WP_FITS_WORD32(routsize)))) {
ok = 0;
}
if (ok && (wp_lock(wp_rsa_get_mutex(ctx->rsa)) != 1)) {
ok = 0;
}
if (ok) {
rc = wc_RsaSSL_Verify(sig, (word32)sigLen, rout, (word32)routsize,
wp_rsa_get_key(ctx->rsa));
wp_unlock(wp_rsa_get_mutex(ctx->rsa));
if (rc < 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_RsaSSL_Verify", rc);
ok = 0;
Expand Down
Loading
Loading