Skip to content
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

She extension update #27

Merged
merged 20 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
321 changes: 309 additions & 12 deletions src/wh_client_she.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@
#include "wolfhsm/wh_packet.h"
#include "wolfhsm/wh_client.h"

int wh_Client_ShePreProgramKey(whClientContext* c, whNvmId keyId,
whNvmFlags flags, uint8_t* key, whNvmSize keySz)
{
int ret;
int outRc;
ret = wh_Client_NvmAddObject(c, MAKE_WOLFHSM_KEYID(WOLFHSM_KEYTYPE_SHE,
c->comm->client_id, keyId), 0, flags, 0, NULL, keySz, key, &outRc);
if (ret == 0)
ret = outRc;
return ret;
}

int wh_Client_SheSetUidRequest(whClientContext* c, uint8_t* uid, uint32_t uidSz)
{
int ret;
Expand All @@ -41,8 +53,7 @@ int wh_Client_SheSetUidResponse(whClientContext* c)
if (c == NULL)
return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz,
(uint8_t*)packet);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)packet);
if (ret == 0)
ret = packet->rc;
return ret;
Expand Down Expand Up @@ -153,8 +164,7 @@ int wh_Client_SheGetStatusResponse(whClientContext* c, uint8_t* sreg)
if (c == NULL || sreg == NULL)
return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz,
(uint8_t*)packet);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)packet);
/* return error or set sreg */
if (ret == 0) {
if (packet->rc != WOLFHSM_SHE_ERC_NO_ERROR)
Expand Down Expand Up @@ -211,8 +221,7 @@ int wh_Client_SheLoadKeyResponse(whClientContext* c, uint8_t* messageFour,
if (c == NULL || messageFour == NULL || messageFive == NULL)
return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz,
(uint8_t*)packet);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)packet);
if (ret == 0) {
if (packet->rc != WOLFHSM_SHE_ERC_NO_ERROR)
ret = packet->rc;
Expand Down Expand Up @@ -241,6 +250,51 @@ int wh_Client_SheLoadKey(whClientContext* c, uint8_t* messageOne,
return ret;
}

int wh_Client_SheLoadPlainKeyRequest(whClientContext* c, uint8_t* key,
uint32_t keySz)
{
int ret;
whPacket* packet;
if (c == NULL || key == NULL || keySz < WOLFHSM_SHE_KEY_SZ)
billphipps marked this conversation as resolved.
Show resolved Hide resolved
return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
/* copy the key */
memcpy(packet->sheLoadPlainKeyReq.key, key, WOLFHSM_SHE_KEY_SZ);
/* send load key req */
ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_SHE,
WH_SHE_LOAD_PLAIN_KEY, WOLFHSM_PACKET_STUB_SIZE +
sizeof(packet->sheLoadPlainKeyReq), (uint8_t*)packet);
return ret;
}

int wh_Client_SheLoadPlainKeyResponse(whClientContext* c)
{
int ret;
uint16_t group;
uint16_t action;
uint16_t dataSz;
whPacket* packet;
if (c == NULL)
return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)packet);
if (ret == 0)
ret = packet->rc;
return ret;
}

int wh_Client_SheLoadPlainKey(whClientContext* c, uint8_t* key, uint32_t keySz)
{
int ret;
ret = wh_Client_SheLoadPlainKeyRequest(c, key, keySz);
if (ret == 0) {
do {
ret = wh_Client_SheLoadPlainKeyResponse(c);
} while (ret == WH_ERROR_NOTREADY);
}
return ret;
}

int wh_Client_SheExportRamKeyRequest(whClientContext* c)
{
int ret;
Expand All @@ -267,8 +321,7 @@ int wh_Client_SheExportRamKeyResponse(whClientContext* c, uint8_t* messageOne,
messageThree == NULL || messageFour == NULL || messageFive == NULL)
return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz,
(uint8_t*)packet);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)packet);
if (ret == 0) {
if (packet->rc != WOLFHSM_SHE_ERC_NO_ERROR)
ret = packet->rc;
Expand Down Expand Up @@ -368,8 +421,7 @@ int wh_Client_SheRndResponse(whClientContext* c, uint8_t* out, uint32_t* outSz)
if (c == NULL || out == NULL || outSz == NULL ||*outSz < WOLFHSM_SHE_KEY_SZ)
return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz,
(uint8_t*)packet);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)packet);
if (ret == 0) {
if (packet->rc != WOLFHSM_SHE_ERC_NO_ERROR)
ret = packet->rc;
Expand Down Expand Up @@ -420,8 +472,7 @@ int wh_Client_SheExtendSeedResponse(whClientContext* c)
if (c == NULL)
return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz,
(uint8_t*)packet);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)packet);
if (ret == 0)
ret = packet->rc;
return ret;
Expand All @@ -439,3 +490,249 @@ int wh_Client_SheExtendSeed(whClientContext* c, uint8_t* entropy,
}
return ret;
}

int wh_Client_SheEncEcbRequest(whClientContext* c, uint8_t keyId, uint8_t* in,
uint32_t sz)
{
int ret;
uint8_t* packIn;
whPacket* packet;
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to check that it is a multiple of the block/key size here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think there are cases here where we need to validate the size. The size the user passes is directly passed to memcpy without any upper bound checking, so while it might be considered abuse of the function, it is still a potential buffer overflow. Yes it is on the client side and so not any real security implications, but still, I don't think we should allow the client to overflow a buffer in one of our public APIs if we can avoid it. We should look at all the size arguments passed to these APIs and ensure they aren't used directly without validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not how it's done in any of our crypto for aes, the ecb encrypt and decrypt functions don't even check minimum length:

        for (i = 0; i < sz; i += AES_BLOCK_SIZE) {
            ret = wc_AesEncryptDirect(aes, out, in);
            if (ret != 0)
                break;
            in += AES_BLOCK_SIZE;
            out += AES_BLOCK_SIZE;
        }

when the data is longer than the packet buffer, assuming we're not using shared memory, we'll need a different scheme that lets the client send multiple packets for a single command, and that's not in the scope of this change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better error out if it can't be placed in a single packet.

Suggested change
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ)
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ || sz > (WH_COMM_DATA_LEN - WOLFHSM_PACKET_STUB_SIZE - sizeof(packet->sheEncEcbReq))

return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
/* in is after fixed sized fields */
packIn = (uint8_t*)(&packet->sheEncEcbReq + 1);
packet->sheEncEcbReq.keyId = keyId;
packet->sheEncEcbReq.sz = sz;
/* set in */
memcpy(packIn, in, sz);
/* send enc ecb */
ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_SHE, WH_SHE_ENC_ECB,
WOLFHSM_PACKET_STUB_SIZE + sizeof(packet->sheEncEcbReq) + sz,
(uint8_t*)packet);
return ret;
}

int wh_Client_SheEncEcbResponse(whClientContext* c, uint8_t* out, uint32_t sz)
{
int ret;
uint16_t group;
uint16_t action;
uint16_t dataSz;
uint8_t* packOut;
whPacket* packet;
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ)
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ|| sz > (WH_COMM_DATA_LEN - WOLFHSM_PACKET_STUB_SIZE - sizeof(packet->sheEncEcbRes))

return WH_ERROR_BADARGS;
Comment on lines +544 to +553
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as above re: size checking against WOLFHSM_SHE_KEY_SZ

packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
/* out is after fixed sized fields */
packOut = (uint8_t*)(&packet->sheEncEcbRes + 1);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)packet);
if (ret == 0) {
if (packet->rc != WOLFHSM_SHE_ERC_NO_ERROR)
ret = packet->rc;
else if (sz < packet->sheEncEcbRes.sz)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

equality check? If we are copying out fewer bytes than expected to the user (e.g. if sz > packet->sheEncEcbRes.sz) then we need to indicate that via output argument. But this is probably an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just based it on how we do aes in wolfCrypt, see https://www.wolfssl.com/doxygen/group__AES.html#gaed1e38cd30d917165183fc68dd4b218b, usually the same buffer is used and overwritten for the output anyways.

ret = WH_ERROR_BADARGS;
else
memcpy(out, packOut, packet->sheEncEcbRes.sz);
}
return ret;
}

int wh_Client_SheEncEcb(whClientContext* c, uint8_t keyId, uint8_t* in,
uint8_t* out, uint32_t sz)
{
int ret;
ret = wh_Client_SheEncEcbRequest(c, keyId, in, sz);
if (ret == 0) {
do {
ret = wh_Client_SheEncEcbResponse(c, out, sz);
} while (ret == WH_ERROR_NOTREADY);
}
return ret;
}

int wh_Client_SheEncCbcRequest(whClientContext* c, uint8_t keyId, uint8_t* iv,
uint32_t ivSz, uint8_t* in, uint32_t sz)
{
int ret;
uint8_t* packIn;
whPacket* packet;
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ || iv == NULL ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as above re: size checking against WOLFHSM_SHE_KEY_SZ

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ || iv == NULL ||
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ || || sz > (WH_COMM_DATA_LEN - WOLFHSM_PACKET_STUB_SIZE - sizeof(packet->sheEncCbcReq || iv == NULL ||

ivSz < WOLFHSM_SHE_KEY_SZ)
return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
/* in is after fixed sized fields */
packIn = (uint8_t*)(&packet->sheEncCbcReq + 1);
packet->sheEncCbcReq.keyId = keyId;
packet->sheEncCbcReq.sz = sz;
/* set iv */
memcpy(packet->sheEncCbcReq.iv, iv, ivSz);
/* set in */
memcpy(packIn, in, sz);
/* send enc ecb */
ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_SHE, WH_SHE_ENC_CBC,
WOLFHSM_PACKET_STUB_SIZE + sizeof(packet->sheEncCbcReq) + sz,
(uint8_t*)packet);
return ret;
}

int wh_Client_SheEncCbcResponse(whClientContext* c, uint8_t* out, uint32_t sz)
{
int ret;
uint16_t group;
uint16_t action;
uint16_t dataSz;
uint8_t* packOut;
whPacket* packet;
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as above re: size checking against WOLFHSM_SHE_KEY_SZ

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ)
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ || sz > (WH_COMM_DATA_LEN - WOLFHSM_PACKET_STUB_SIZE - sizeof(packet->sheEncCbcRes))

return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
/* out is after fixed sized fields */
packOut = (uint8_t*)(&packet->sheEncCbcRes + 1);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)packet);
if (ret == 0) {
if (packet->rc != WOLFHSM_SHE_ERC_NO_ERROR)
ret = packet->rc;
else if (sz < packet->sheEncCbcRes.sz)
ret = WH_ERROR_BADARGS;
else
memcpy(out, packOut, packet->sheEncCbcRes.sz);
}
return ret;
}

int wh_Client_SheEncCbc(whClientContext* c, uint8_t keyId, uint8_t* iv,
uint32_t ivSz, uint8_t* in, uint8_t* out, uint32_t sz)
{
int ret;
ret = wh_Client_SheEncCbcRequest(c, keyId, iv, ivSz, in, sz);
if (ret == 0) {
do {
ret = wh_Client_SheEncCbcResponse(c, out, sz);
} while (ret == WH_ERROR_NOTREADY);
}
return ret;
}

int wh_Client_SheDecEcbRequest(whClientContext* c, uint8_t keyId, uint8_t* in,
uint32_t sz)
{
int ret;
uint8_t* packIn;
whPacket* packet;
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ)
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ || sz > (WH_COMM_DATA_LEN - WOLFHSM_PACKET_STUB_SIZE - sizeof(packet->sheDecEcbReq))

return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
/* in is after fixed sized fields */
packIn = (uint8_t*)(&packet->sheDecEcbReq + 1);
packet->sheDecEcbReq.keyId = keyId;
packet->sheDecEcbReq.sz = sz;
/* set in */
memcpy(packIn, in, sz);
/* send enc ecb */
ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_SHE, WH_SHE_DEC_ECB,
WOLFHSM_PACKET_STUB_SIZE + sizeof(packet->sheDecEcbReq) + sz,
(uint8_t*)packet);
return ret;
}

int wh_Client_SheDecEcbResponse(whClientContext* c, uint8_t* out, uint32_t sz)
{
int ret;
uint16_t group;
uint16_t action;
uint16_t dataSz;
uint8_t* packOut;
whPacket* packet;
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ)
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ || sz > (WH_COMM_DATA_LEN - WOLFHSM_PACKET_STUB_SIZE - sizeof(packet->sheDecEcbRes))

return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
/* out is after fixed sized fields */
packOut = (uint8_t*)(&packet->sheDecEcbRes + 1);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)packet);
if (ret == 0) {
if (packet->rc != WOLFHSM_SHE_ERC_NO_ERROR)
ret = packet->rc;
else if (sz < packet->sheDecEcbRes.sz)
ret = WH_ERROR_BADARGS;
else
memcpy(out, packOut, packet->sheDecEcbRes.sz);
}
return ret;
}

int wh_Client_SheDecEcb(whClientContext* c, uint8_t keyId, uint8_t* in,
uint8_t* out, uint32_t sz)
{
int ret;
ret = wh_Client_SheDecEcbRequest(c, keyId, in, sz);
if (ret == 0) {
do {
ret = wh_Client_SheDecEcbResponse(c, out, sz);
} while (ret == WH_ERROR_NOTREADY);
}
return ret;
}

int wh_Client_SheDecCbcRequest(whClientContext* c, uint8_t keyId, uint8_t* iv,
uint32_t ivSz, uint8_t* in, uint32_t sz)
{
int ret;
uint8_t* packIn;
whPacket* packet;
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ || iv == NULL ||
billphipps marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ || iv == NULL ||
if (c == NULL || in == NULL || sz < WOLFHSM_SHE_KEY_SZ || || sz > (WH_COMM_DATA_LEN - WOLFHSM_PACKET_STUB_SIZE - sizeof(packet->sheDecCbcReq) || iv == NULL ||

ivSz < WOLFHSM_SHE_KEY_SZ)
return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
/* in is after fixed sized fields */
packIn = (uint8_t*)(&packet->sheDecCbcReq + 1);
packet->sheDecCbcReq.keyId = keyId;
packet->sheDecCbcReq.sz = sz;
/* set iv */
memcpy(packet->sheDecCbcReq.iv, iv, ivSz);
/* set in */
memcpy(packIn, in, sz);
/* send enc ecb */
ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_SHE, WH_SHE_DEC_CBC,
WOLFHSM_PACKET_STUB_SIZE + sizeof(packet->sheDecCbcReq) + sz,
(uint8_t*)packet);
return ret;
}

int wh_Client_SheDecCbcResponse(whClientContext* c, uint8_t* out, uint32_t sz)
{
int ret;
uint16_t group;
uint16_t action;
uint16_t dataSz;
uint8_t* packOut;
whPacket* packet;
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ)
if (c == NULL || out == NULL || sz < WOLFHSM_SHE_KEY_SZ || sz > (WH_COMM_DATA_LEN - WOLFHSM_PACKET_STUB_SIZE - sizeof(packet->sheDecCbcRes))

return WH_ERROR_BADARGS;
packet = (whPacket*)wh_CommClient_GetDataPtr(c->comm);
/* out is after fixed sized fields */
packOut = (uint8_t*)(&packet->sheDecCbcRes + 1);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)packet);
if (ret == 0) {
if (packet->rc != WOLFHSM_SHE_ERC_NO_ERROR)
ret = packet->rc;
else if (sz < packet->sheDecCbcRes.sz)
ret = WH_ERROR_BADARGS;
else
memcpy(out, packOut, packet->sheDecCbcRes.sz);
}
return ret;
}

int wh_Client_SheDecCbc(whClientContext* c, uint8_t keyId, uint8_t* iv,
uint32_t ivSz, uint8_t* in, uint8_t* out, uint32_t sz)
{
int ret;
ret = wh_Client_SheDecCbcRequest(c, keyId, iv, ivSz, in, sz);
if (ret == 0) {
do {
ret = wh_Client_SheDecCbcResponse(c, out, sz);
} while (ret == WH_ERROR_NOTREADY);
}
return ret;
}
Loading