Skip to content

Commit

Permalink
peer review for #8187 and unrelated bug fixes:
Browse files Browse the repository at this point in the history
return error code from wolfSSL_RefWithMutexUnlock() to expose result to caller;

fix endianness bug in src/x509.c:wolfSSL_X509_add_ext() (fixes failing test_wolfSSL_X509_add_ext on BE targets);

fix possible file handle leak in tests/api.c:test_wolfSSL_d2i_X509_REQ() (reported by clang-tidy);

in wolfssl/ssl.h, define CONST_NUM_ERR_WOLFSSL_SUCCESS, so that WOLFSSL_SUCCESS can be benignly miswrapped in WC_NO_ERR_TRACE().
  • Loading branch information
douzzer committed Nov 15, 2024
1 parent 595f55e commit a95b759
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 15 deletions.
3 changes: 2 additions & 1 deletion src/ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -11124,7 +11124,8 @@ int wolfSSL_set_compression(WOLFSSL* ssl)
ret = wolfSSL_CertManagerUnloadIntermediateCerts(ctx->cm);
}

wolfSSL_RefWithMutexUnlock(&ctx->ref);
if (wolfSSL_RefWithMutexUnlock(&ctx->ref) != 0)
WOLFSSL_MSG("Failed to unlock mutex!");

return ret;
}
Expand Down
5 changes: 4 additions & 1 deletion src/x509.c
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,9 @@ int wolfSSL_X509_add_ext(WOLFSSL_X509 *x509, WOLFSSL_X509_EXTENSION *ext,
if (ext->value.length == sizeof(word16)) {
/* if ext->value is already word16, set directly */
x509->keyUsage = *(word16*)ext->value.data;
#ifdef BIG_ENDIAN_ORDER
x509->keyUsage = rotlFixed16(x509->keyUsage, 8U);
#endif
x509->keyUsageCrit = (byte)ext->crit;
x509->keyUsageSet = 1;
}
Expand All @@ -1406,7 +1409,7 @@ int wolfSSL_X509_add_ext(WOLFSSL_X509 *x509, WOLFSSL_X509_EXTENSION *ext,
case WC_NID_ext_key_usage:
if (ext && ext->value.data) {
if (ext->value.length == sizeof(byte)) {
/* if ext->value is already word16, set directly */
/* if ext->value is already 1 byte, set directly */
x509->extKeyUsage = *(byte*)ext->value.data;
x509->extKeyUsageCrit = (byte)ext->crit;
}
Expand Down
5 changes: 4 additions & 1 deletion tests/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -83073,7 +83073,10 @@ static int test_wolfSSL_d2i_X509_REQ(void)
* (PEM_read_X509_REQ)*/
ExpectTrue((f = XFOPEN(csrDsaFile, "rb")) != XBADFILE);
ExpectNull(PEM_read_X509_REQ(XBADFILE, &req, NULL, NULL));
ExpectNotNull(PEM_read_X509_REQ(f, &req, NULL, NULL));
if (EXPECT_SUCCESS())
ExpectNotNull(PEM_read_X509_REQ(f, &req, NULL, NULL));
else if (f != XBADFILE)
XFCLOSE(f);
ExpectIntEQ(X509_REQ_verify(req, pub_key), 1);

X509_free(req);
Expand Down
4 changes: 0 additions & 4 deletions wolfcrypt/src/misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,6 @@ masking and clearing memory logic.

#endif

#ifdef WC_RC2

/* This routine performs a left circular arithmetic shift of <x> by <y> value */
WC_MISC_STATIC WC_INLINE word16 rotlFixed16(word16 x, word16 y)
{
Expand All @@ -130,8 +128,6 @@ WC_MISC_STATIC WC_INLINE word16 rotrFixed16(word16 x, word16 y)
return (x >> y) | (x << (sizeof(x) * 8 - y));
}

#endif /* WC_RC2 */

/* This routine performs a byte swap of 32-bit word value. */
#if defined(__CCRX__) && !defined(NO_INLINE) /* shortest version for CC-RX */
#define ByteReverseWord32(value) _builtin_revl(value)
Expand Down
4 changes: 2 additions & 2 deletions wolfcrypt/src/wc_port.c
Original file line number Diff line number Diff line change
Expand Up @@ -1330,9 +1330,9 @@ int wolfSSL_RefWithMutexLock(wolfSSL_RefWithMutex* ref)
return wc_LockMutex(&ref->mutex);
}

void wolfSSL_RefWithMutexUnlock(wolfSSL_RefWithMutex* ref)
int wolfSSL_RefWithMutexUnlock(wolfSSL_RefWithMutex* ref)
{
wc_UnLockMutex(&ref->mutex);
return wc_UnLockMutex(&ref->mutex);
}

void wolfSSL_RefWithMutexDec(wolfSSL_RefWithMutex* ref, int* isZero, int* err)
Expand Down
8 changes: 6 additions & 2 deletions wolfssl/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2759,16 +2759,20 @@ WOLFSSL_API void wolfSSL_ERR_print_errors(WOLFSSL_BIO *bio);
enum { /* ssl Constants */
WOLFSSL_ERROR_NONE = 0, /* for most functions */
WOLFSSL_FAILURE = 0, /* for some functions */
WOLFSSL_SUCCESS = 1,

#if defined(WOLFSSL_DEBUG_TRACE_ERROR_CODES) && \
(defined(BUILDING_WOLFSSL) || \
defined(WOLFSSL_DEBUG_TRACE_ERROR_CODES_ALWAYS))
#define WOLFSSL_FAILURE WC_ERR_TRACE(WOLFSSL_FAILURE)
#define CONST_NUM_ERR_WOLFSSL_FAILURE 0
/* include CONST_NUM_ERR_ variants of the success codes, so that they
* can be harmlessly wrapped in WC_NO_ERR_TRACE().
*/
#define CONST_NUM_ERR_WOLFSSL_ERROR_NONE 0
#define CONST_NUM_ERR_WOLFSSL_SUCCESS 1
#endif

WOLFSSL_SUCCESS = 1,

/* WOLFSSL_SHUTDOWN_NOT_DONE is returned by wolfSSL_shutdown and
* wolfSSL_SendUserCanceled when the other end
* of the connection has yet to send its close notify alert as part of the
Expand Down
2 changes: 0 additions & 2 deletions wolfssl/wolfcrypt/misc.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,10 @@ word32 rotlFixed(word32 x, word32 y);
WOLFSSL_LOCAL
word32 rotrFixed(word32 x, word32 y);

#ifdef WC_RC2
WOLFSSL_LOCAL
word16 rotlFixed16(word16 x, word16 y);
WOLFSSL_LOCAL
word16 rotrFixed16(word16 x, word16 y);
#endif

WOLFSSL_LOCAL
word32 ByteReverseWord32(word32 value);
Expand Down
4 changes: 2 additions & 2 deletions wolfssl/wolfcrypt/wc_port.h
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ typedef struct wolfSSL_RefWithMutex wolfSSL_Ref;
#define wolfSSL_RefWithMutexFree wolfSSL_RefFree
#define wolfSSL_RefWithMutexInc wolfSSL_RefInc
#define wolfSSL_RefWithMutexLock(ref) 0
#define wolfSSL_RefWithMutexUnlock(ref) WC_DO_NOTHING
#define wolfSSL_RefWithMutexUnlock(ref) 0
#define wolfSSL_RefWithMutexDec wolfSSL_RefDec

#else
Expand All @@ -517,7 +517,7 @@ WOLFSSL_LOCAL void wolfSSL_RefWithMutexFree(wolfSSL_RefWithMutex* ref);
WOLFSSL_LOCAL void wolfSSL_RefWithMutexInc(wolfSSL_RefWithMutex* ref,
int* err);
WOLFSSL_LOCAL int wolfSSL_RefWithMutexLock(wolfSSL_RefWithMutex* ref);
WOLFSSL_LOCAL void wolfSSL_RefWithMutexUnlock(wolfSSL_RefWithMutex* ref);
WOLFSSL_LOCAL int wolfSSL_RefWithMutexUnlock(wolfSSL_RefWithMutex* ref);
WOLFSSL_LOCAL void wolfSSL_RefWithMutexDec(wolfSSL_RefWithMutex* ref,
int* isZero, int* err);

Expand Down

0 comments on commit a95b759

Please sign in to comment.