Skip to content

Commit

Permalink
Fix buffer overflow issue with SHA3 and ECX.
Browse files Browse the repository at this point in the history 
- Fix memcpy with correct bufferlen in case of SHA-3
- Fix ECX reverse_bytes to reverse based on input sizes.

Signed-off-by: Yogaraj Alamenda <[email protected]>
  • Loading branch information
@Yogaraj-Alamenda
Yogaraj-Alamenda committed Oct 10, 2022
1 parent 98ccbe8 commit cab65f3
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 58 deletions.
4 changes: 2 additions & 2 deletions qat_hw_asym_common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ int qat_mod_exp(BIGNUM *r, const BIGNUM *a, const BIGNUM *p, const BIGNUM *m,
void qat_ecx_cb(void *pCallbackTag, CpaStatus status,
void *pOpData, CpaBoolean multiplyStatus,
CpaFlatBuffer *pXk, CpaFlatBuffer *pYk);
int reverse_bytes(unsigned char *tobuffer,
unsigned char *frombuffer, unsigned int size);
int reverse_bytes(unsigned char *tobuffer, unsigned char *frombuffer,
unsigned int tosize, unsigned int fromsize);
# endif
#endif /* QAT_HW_ASYM_COMMON_H */
68 changes: 38 additions & 30 deletions qat_hw_ecx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,20 +92,25 @@ typedef struct {
unsigned char *privkey;
} ECX_KEY;

int reverse_bytes(unsigned char *tobuffer,
unsigned char *frombuffer, unsigned int size)
int reverse_bytes(unsigned char *tobuffer, unsigned char *frombuffer,
unsigned int tosize, unsigned int fromsize)
{
int i = 0;
int tobuffer_frombuffer_length_diff = 0;
if (tobuffer == NULL || frombuffer == NULL ) {
WARN("Either tobuffer or frombuffer is NULL %d\n", size);
if (tobuffer == NULL || frombuffer == NULL) {
WARN("Either tobuffer or frombuffer is NULL \n");
return 0;
}
if (X448_KEYLEN == size)
tobuffer_frombuffer_length_diff = X448_DATA_KEY_DIFF;
for (i = 0; i < size; i++) {
tobuffer[i] = frombuffer[size - 1 - i + tobuffer_frombuffer_length_diff];

if (fromsize == X448_KEYLEN)
i = 8; /* Adds zeros at the begining for 64 byte alignment */

/* Reverse bytes and copy to dest buffer */
for (; i < tosize; i++) {
tobuffer[i] = frombuffer[--fromsize];
if (fromsize <=0)
break;
}

return 1;
}

Expand Down Expand Up @@ -240,7 +245,7 @@ int qat_pkey_ecx_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
goto err;
}
pubkey = key->pubkey;
privkey = key->privkey = OPENSSL_secure_zalloc(qat_keylen);
privkey = key->privkey = OPENSSL_secure_zalloc(keylen);
if (privkey == NULL) {
WARN("Cannot allocate privkey.\n");
QATerr(QAT_F_QAT_PKEY_ECX_KEYGEN, ERR_R_MALLOC_FAILURE);
Expand Down Expand Up @@ -273,7 +278,7 @@ int qat_pkey_ecx_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
qat_ecx_op_data->curveType =
is_ecx_448 ? CPA_CY_EC_MONTEDWDS_CURVE448_TYPE : CPA_CY_EC_MONTEDWDS_CURVE25519_TYPE;

if (0 == reverse_bytes(qat_ecx_op_data->k.pData, privkey, qat_keylen)) {
if (0 == reverse_bytes(qat_ecx_op_data->k.pData, privkey, qat_keylen, keylen)) {
WARN("Failed to reverse bytes for submission of data to QAT driver\n");
QATerr(QAT_F_QAT_PKEY_ECX_KEYGEN, ERR_R_INTERNAL_ERROR);
goto err;
Expand Down Expand Up @@ -429,7 +434,7 @@ int qat_pkey_ecx_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)

qat_cleanup_op_done(&op_done);

if (0 == reverse_bytes(pubkey, pXk->pData, keylen)) {
if (0 == reverse_bytes(pubkey, pXk->pData, keylen, qat_keylen)) {
WARN("Failed to reverse bytes for data received from QAT driver\n");
QATerr(QAT_F_QAT_PKEY_ECX_KEYGEN, ERR_R_INTERNAL_ERROR);
goto err;
Expand Down Expand Up @@ -525,7 +530,6 @@ int qat_pkey_ecx_derive25519(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keyl
CpaBoolean multiplyStatus = CPA_TRUE;
CpaFlatBuffer *pXk = NULL;
const unsigned char *privkey, *pubkey;
Cpa8U dataLenInBytes = X25519_KEYLEN;
op_done_t op_done;
int qatPerformOpRetries = 0;
int iMsgRetry = getQatMsgRetryCount();
Expand Down Expand Up @@ -557,7 +561,7 @@ int qat_pkey_ecx_derive25519(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keyl
return 0;

if (key == NULL) {
*keylen = dataLenInBytes;
*keylen = X25519_KEYLEN;
return 1;
}

Expand All @@ -571,46 +575,48 @@ int qat_pkey_ecx_derive25519(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keyl
}
memset(qat_ecx_op_data, 0, sizeof(CpaCyEcMontEdwdsPointMultiplyOpData));

qat_ecx_op_data->k.pData = (Cpa8U *)qaeCryptoMemAlloc(dataLenInBytes, __FILE__, __LINE__);
qat_ecx_op_data->k.pData = (Cpa8U *)qaeCryptoMemAlloc(X25519_KEYLEN, __FILE__, __LINE__);
if (qat_ecx_op_data->k.pData == NULL) {
WARN("Failure to allocate k.pData.\n");
QATerr(QAT_F_QAT_PKEY_ECX_DERIVE25519, ERR_R_MALLOC_FAILURE);
goto err;
}
qat_ecx_op_data->k.dataLenInBytes = (Cpa32U)dataLenInBytes;
qat_ecx_op_data->k.dataLenInBytes = X25519_KEYLEN;

qat_ecx_op_data->x.pData = (Cpa8U *)qaeCryptoMemAlloc(dataLenInBytes, __FILE__, __LINE__);
qat_ecx_op_data->x.pData = (Cpa8U *)qaeCryptoMemAlloc(X25519_KEYLEN, __FILE__, __LINE__);
if (qat_ecx_op_data->x.pData == NULL) {
WARN("Failure to allocate x.pData.\n");
QATerr(QAT_F_QAT_PKEY_ECX_DERIVE25519, ERR_R_MALLOC_FAILURE);
goto err;
}
qat_ecx_op_data->x.dataLenInBytes = (Cpa32U)dataLenInBytes;
qat_ecx_op_data->x.dataLenInBytes = X25519_KEYLEN;

pXk = (CpaFlatBuffer *)OPENSSL_zalloc(sizeof(CpaFlatBuffer));
if (NULL == pXk) {
WARN("Failed to allocate memory for pXk\n");
QATerr(QAT_F_QAT_PKEY_ECX_DERIVE25519, ERR_R_MALLOC_FAILURE);
goto err;
}
pXk->pData = (Cpa8U *)qaeCryptoMemAlloc(dataLenInBytes, __FILE__, __LINE__);
pXk->pData = (Cpa8U *)qaeCryptoMemAlloc(X25519_KEYLEN, __FILE__, __LINE__);
if (NULL == pXk->pData) {
WARN("Failed to allocate memory for pXk data\n");
QATerr(QAT_F_QAT_PKEY_ECX_DERIVE25519, ERR_R_MALLOC_FAILURE);
goto err;
}
pXk->dataLenInBytes = dataLenInBytes;
pXk->dataLenInBytes = X25519_KEYLEN;

qat_ecx_op_data->generator = CPA_FALSE;
qat_ecx_op_data->curveType = CPA_CY_EC_MONTEDWDS_CURVE25519_TYPE;

if (0 == reverse_bytes(qat_ecx_op_data->k.pData, (unsigned char *)privkey, dataLenInBytes)) {
if (0 == reverse_bytes(qat_ecx_op_data->k.pData, (unsigned char *)privkey,
X25519_KEYLEN, X25519_KEYLEN)) {
WARN("Failed to reverse bytes for submission of data to QAT driver\n");
QATerr(QAT_F_QAT_PKEY_ECX_DERIVE25519, ERR_R_INTERNAL_ERROR);
goto err;
}

if (0 == reverse_bytes(qat_ecx_op_data->x.pData,(unsigned char *)pubkey, dataLenInBytes)) {
if (0 == reverse_bytes(qat_ecx_op_data->x.pData,(unsigned char *)pubkey,
X25519_KEYLEN, X25519_KEYLEN)) {
WARN("Failed to reverse bytes for submission of data to QAT driver\n");
QATerr(QAT_F_QAT_PKEY_ECX_DERIVE25519, ERR_R_INTERNAL_ERROR);
goto err;
Expand Down Expand Up @@ -766,32 +772,32 @@ int qat_pkey_ecx_derive25519(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keyl

qat_cleanup_op_done(&op_done);

if (0 == reverse_bytes(key, pXk->pData, dataLenInBytes)) {
if (0 == reverse_bytes(key, pXk->pData, X25519_KEYLEN, X25519_KEYLEN)) {
WARN("Failed to reverse bytes for data received from QAT driver\n");
QATerr(QAT_F_QAT_PKEY_ECX_DERIVE25519, ERR_R_INTERNAL_ERROR);
goto err;
}

*keylen = (size_t)dataLenInBytes;
*keylen = X25519_KEYLEN;
ret = 1;

err:
/* Clean the memory. */
if (pXk != NULL) {
if (pXk->pData != NULL) {
OPENSSL_cleanse(pXk->pData, dataLenInBytes);
OPENSSL_cleanse(pXk->pData, X25519_KEYLEN);
qaeCryptoMemFreeNonZero(pXk->pData);
}
OPENSSL_free(pXk);
}

if (NULL != qat_ecx_op_data) {
if (qat_ecx_op_data->k.pData != NULL) {
OPENSSL_cleanse(qat_ecx_op_data->k.pData, dataLenInBytes);
OPENSSL_cleanse(qat_ecx_op_data->k.pData, X25519_KEYLEN);
qaeCryptoMemFreeNonZero(qat_ecx_op_data->k.pData);
}
if (qat_ecx_op_data->x.pData != NULL) {
OPENSSL_cleanse(qat_ecx_op_data->x.pData, dataLenInBytes);
OPENSSL_cleanse(qat_ecx_op_data->x.pData, X25519_KEYLEN);
qaeCryptoMemFreeNonZero(qat_ecx_op_data->x.pData);
}
qaeCryptoMemFreeNonZero(qat_ecx_op_data);
Expand Down Expand Up @@ -896,13 +902,15 @@ int qat_pkey_ecx_derive448(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keylen
qat_ecx_op_data->generator = CPA_FALSE;
qat_ecx_op_data->curveType = CPA_CY_EC_MONTEDWDS_CURVE448_TYPE;

if (0 == reverse_bytes(qat_ecx_op_data->k.pData, (unsigned char *)privkey, QAT_X448_DATALEN)) {
if (0 == reverse_bytes(qat_ecx_op_data->k.pData, (unsigned char *)privkey,
QAT_X448_DATALEN, X448_KEYLEN)) {
WARN("Failed to reverse bytes for submission of data to QAT driver\n");
QATerr(QAT_F_QAT_PKEY_ECX_DERIVE448, ERR_R_INTERNAL_ERROR);
goto err;
}

if (0 == reverse_bytes(qat_ecx_op_data->x.pData, (unsigned char *)pubkey, QAT_X448_DATALEN)) {
if (0 == reverse_bytes(qat_ecx_op_data->x.pData, (unsigned char *)pubkey,
QAT_X448_DATALEN, X448_KEYLEN)) {
WARN("Failed to reverse bytes for submission of data to QAT driver\n");
QATerr(QAT_F_QAT_PKEY_ECX_DERIVE448, ERR_R_INTERNAL_ERROR);
goto err;
Expand Down Expand Up @@ -1058,7 +1066,7 @@ int qat_pkey_ecx_derive448(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keylen

qat_cleanup_op_done(&op_done);

if (0 == reverse_bytes(key, pXk->pData, X448_KEYLEN)) {
if (0 == reverse_bytes(key, pXk->pData, X448_KEYLEN, QAT_X448_DATALEN)) {
WARN("Failed to reverse bytes for data received from QAT driver\n");
QATerr(QAT_F_QAT_PKEY_ECX_DERIVE448, ERR_R_INTERNAL_ERROR);
goto err;
Expand Down
12 changes: 7 additions & 5 deletions qat_hw_sha3.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -844,17 +844,19 @@ static int qat_sha3_update(EVP_MD_CTX *ctx, const void *in, size_t len)
#endif
/* Allocate buffer for HASH operation. */
buffer_len = len + sha3_ctx->digest_size ;
sha3_ctx->src_buffer[0].pData = qaeCryptoMemAlloc( buffer_len , __FILE__, __LINE__);
sha3_ctx->src_buffer[0].pData = qaeCryptoMemAlloc(buffer_len, __FILE__, __LINE__);

if ((sha3_ctx->src_buffer[0].pData) == NULL) {
WARN("Unable to allocate memory for buffer for sha3 hash.\n");
QATerr(QAT_F_QAT_SHA3_UPDATE, ERR_R_MALLOC_FAILURE);
goto err;
WARN("Unable to allocate memory for buffer for sha3 hash.\n");
QATerr(QAT_F_QAT_SHA3_UPDATE, ERR_R_MALLOC_FAILURE);
goto err;
}

memset(sha3_ctx->src_buffer[0].pData, 0, buffer_len);

sha3_ctx->dst_buffer[0].pData = sha3_ctx->src_buffer[0].pData;

memcpy(sha3_ctx->src_buffer[0].pData, in, buffer_len);
memcpy(sha3_ctx->src_buffer[0].pData, in, len);

tlv = qat_check_create_local_variables();
if (NULL == tlv) {
Expand Down
Loading

0 comments on commit cab65f3

Please sign in to comment.