Skip to content

Commit b22d178

Browse files
padelsbachpadelsbach
committed
Address PR comments
1 parent 4fb8d91 commit b22d178

1 file changed

Lines changed: 102 additions & 147 deletions

File tree

‎src/wh_client_she.c‎

Lines changed: 102 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,8 @@ int wh_Client_SheSetUidResponse(whClientContext* c)
9696

9797
resp = (whMessageShe_SetUidResponse*)wh_CommClient_GetDataPtr(c->comm);
9898
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
99-
if (ret == WH_ERROR_OK) {
100-
if (group != WH_MESSAGE_GROUP_SHE ||
101-
action != WH_SHE_SET_UID ||
102-
dataSz < sizeof(*resp)) {
103-
ret = WH_ERROR_ABORTED;
104-
}
99+
if (ret == WH_ERROR_OK && dataSz < sizeof(*resp)) {
100+
ret = WH_ERROR_ABORTED;
105101
}
106102
if (ret == WH_ERROR_OK) {
107103
ret = resp->rc;
@@ -157,12 +153,8 @@ int wh_Client_SheSecureBoot(whClientContext* c, uint8_t* bootloader,
157153
initResp = (whMessageShe_SecureBootInitResponse*)respBuf;
158154
} while (ret == WH_ERROR_NOTREADY);
159155
}
160-
if (ret == 0) {
161-
if (group != WH_MESSAGE_GROUP_SHE ||
162-
action != WH_SHE_SECURE_BOOT_INIT ||
163-
dataSz < sizeof(*initResp)) {
164-
ret = WH_ERROR_ABORTED;
165-
}
156+
if (ret == 0 && dataSz < sizeof(*initResp)) {
157+
ret = WH_ERROR_ABORTED;
166158
}
167159

168160
/* send update sub command until we've sent the entire bootloader */
@@ -203,9 +195,7 @@ int wh_Client_SheSecureBoot(whClientContext* c, uint8_t* bootloader,
203195
if (ret == 0) {
204196
whMessageShe_SecureBootUpdateResponse* updateResp =
205197
(whMessageShe_SecureBootUpdateResponse*)respBuf;
206-
if (group != WH_MESSAGE_GROUP_SHE ||
207-
action != WH_SHE_SECURE_BOOT_UPDATE ||
208-
dataSz < sizeof(*updateResp)) {
198+
if (dataSz < sizeof(*updateResp)) {
209199
ret = WH_ERROR_ABORTED;
210200
}
211201
}
@@ -228,12 +218,8 @@ int wh_Client_SheSecureBoot(whClientContext* c, uint8_t* bootloader,
228218
finishResp = (whMessageShe_SecureBootFinishResponse*)respBuf;
229219
} while (ret == WH_ERROR_NOTREADY);
230220
}
231-
if (ret == 0) {
232-
if (group != WH_MESSAGE_GROUP_SHE ||
233-
action != WH_SHE_SECURE_BOOT_FINISH ||
234-
dataSz < sizeof(*finishResp)) {
235-
ret = WH_ERROR_ABORTED;
236-
}
221+
if (ret == 0 && dataSz < sizeof(*finishResp)) {
222+
ret = WH_ERROR_ABORTED;
237223
}
238224

239225
if (ret == 0) {
@@ -272,18 +258,15 @@ int wh_Client_SheGetStatusResponse(whClientContext* c, uint8_t* sreg)
272258
resp = (whMessageShe_GetStatusResponse*)wh_CommClient_GetDataPtr(c->comm);
273259

274260
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
261+
262+
/* return error or set sreg */
275263
if (ret == 0) {
276-
if (group != WH_MESSAGE_GROUP_SHE ||
277-
action != WH_SHE_GET_STATUS ||
278-
dataSz < sizeof(*resp)) {
264+
if (dataSz < sizeof(*resp))
279265
ret = WH_ERROR_ABORTED;
280-
}
281-
}
282-
if (ret == 0) {
283-
ret = resp->rc;
284-
}
285-
if (ret == 0) {
286-
*sreg = resp->sreg;
266+
else if (resp->rc != WH_SHE_ERC_NO_ERROR)
267+
ret = resp->rc;
268+
else
269+
*sreg = resp->sreg;
287270
}
288271
return ret;
289272
}
@@ -342,19 +325,17 @@ int wh_Client_SheLoadKeyResponse(whClientContext* c, uint8_t* messageFour,
342325

343326
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
344327
if (ret == 0) {
345-
if (group != WH_MESSAGE_GROUP_SHE ||
346-
action != WH_SHE_LOAD_KEY ||
347-
dataSz < sizeof(*resp)) {
328+
if (dataSz < sizeof(*resp)) {
348329
ret = WH_ERROR_ABORTED;
349330
}
350-
}
351-
if (ret == 0) {
352-
ret = resp->rc;
353-
}
354-
if (ret == 0) {
355-
/* copy out message 4 and 5 */
356-
memcpy(messageFour, resp->messageFour, sizeof(resp->messageFour));
357-
memcpy(messageFive, resp->messageFive, sizeof(resp->messageFive));
331+
else if (resp->rc != WH_SHE_ERC_NO_ERROR) {
332+
ret = resp->rc;
333+
}
334+
else {
335+
/* copy out message 4 and 5 */
336+
memcpy(messageFour, resp->messageFour, sizeof(resp->messageFour));
337+
memcpy(messageFive, resp->messageFive, sizeof(resp->messageFive));
338+
}
358339
}
359340
return ret;
360341
}
@@ -406,12 +387,8 @@ int wh_Client_SheLoadPlainKeyResponse(whClientContext* c)
406387
(whMessageShe_LoadPlainKeyResponse*)wh_CommClient_GetDataPtr(c->comm);
407388

408389
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
409-
if (ret == 0) {
410-
if (group != WH_MESSAGE_GROUP_SHE ||
411-
action != WH_SHE_LOAD_PLAIN_KEY ||
412-
dataSz < sizeof(*resp)) {
413-
ret = WH_ERROR_ABORTED;
414-
}
390+
if (ret == 0 && dataSz < sizeof(*resp)) {
391+
ret = WH_ERROR_ABORTED;
415392
}
416393
if (ret == 0) {
417394
ret = resp->rc;
@@ -464,22 +441,20 @@ int wh_Client_SheExportRamKeyResponse(whClientContext* c, uint8_t* messageOne,
464441

465442
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
466443
if (ret == 0) {
467-
if (group != WH_MESSAGE_GROUP_SHE ||
468-
action != WH_SHE_EXPORT_RAM_KEY ||
469-
dataSz < sizeof(*resp)) {
444+
if (dataSz < sizeof(*resp)) {
470445
ret = WH_ERROR_ABORTED;
471446
}
472-
}
473-
if (ret == 0) {
474-
ret = resp->rc;
475-
}
476-
if (ret == 0) {
477-
memcpy(messageOne, resp->messageOne, sizeof(resp->messageOne));
478-
memcpy(messageTwo, resp->messageTwo, sizeof(resp->messageTwo));
479-
memcpy(messageThree, resp->messageThree,
480-
sizeof(resp->messageThree));
481-
memcpy(messageFour, resp->messageFour, sizeof(resp->messageFour));
482-
memcpy(messageFive, resp->messageFive, sizeof(resp->messageFive));
447+
else if (resp->rc != WH_SHE_ERC_NO_ERROR) {
448+
ret = resp->rc;
449+
}
450+
else {
451+
memcpy(messageOne, resp->messageOne, sizeof(resp->messageOne));
452+
memcpy(messageTwo, resp->messageTwo, sizeof(resp->messageTwo));
453+
memcpy(messageThree, resp->messageThree,
454+
sizeof(resp->messageThree));
455+
memcpy(messageFour, resp->messageFour, sizeof(resp->messageFour));
456+
memcpy(messageFive, resp->messageFive, sizeof(resp->messageFive));
457+
}
483458
}
484459

485460
return ret;
@@ -525,12 +500,8 @@ int wh_Client_SheInitRndResponse(whClientContext* c)
525500

526501
resp = (whMessageShe_InitRngResponse*)wh_CommClient_GetDataPtr(c->comm);
527502
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
528-
if (ret == 0) {
529-
if (group != WH_MESSAGE_GROUP_SHE ||
530-
action != WH_SHE_INIT_RND ||
531-
dataSz < sizeof(*resp)) {
532-
ret = WH_ERROR_ABORTED;
533-
}
503+
if (ret == 0 && dataSz < sizeof(*resp)) {
504+
ret = WH_ERROR_ABORTED;
534505
}
535506
if (ret == 0) {
536507
ret = resp->rc;
@@ -575,21 +546,17 @@ int wh_Client_SheRndResponse(whClientContext* c, uint8_t* out, uint32_t* outSz)
575546
resp = (whMessageShe_RndResponse*)wh_CommClient_GetDataPtr(c->comm);
576547

577548
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
549+
578550
if (ret == 0) {
579-
if (group != WH_MESSAGE_GROUP_SHE ||
580-
action != WH_SHE_RND ||
581-
dataSz < sizeof(*resp)) {
551+
if (dataSz < sizeof(*resp))
582552
ret = WH_ERROR_ABORTED;
553+
else if (resp->rc != WH_SHE_ERC_NO_ERROR)
554+
ret = resp->rc;
555+
else {
556+
memcpy(out, resp->rnd, sizeof(resp->rnd));
557+
*outSz = sizeof(resp->rnd);
583558
}
584559
}
585-
if (ret == 0) {
586-
ret = resp->rc;
587-
}
588-
if (ret == 0) {
589-
memcpy(out, resp->rnd, sizeof(resp->rnd));
590-
*outSz = sizeof(resp->rnd);
591-
}
592-
593560
return ret;
594561
}
595562

@@ -642,12 +609,8 @@ int wh_Client_SheExtendSeedResponse(whClientContext* c)
642609
resp = (whMessageShe_ExtendSeedResponse*)wh_CommClient_GetDataPtr(c->comm);
643610
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
644611

645-
if (ret == 0) {
646-
if (group != WH_MESSAGE_GROUP_SHE ||
647-
action != WH_SHE_EXTEND_SEED ||
648-
dataSz < sizeof(*resp)) {
649-
ret = WH_ERROR_ABORTED;
650-
}
612+
if (ret == 0 && dataSz < sizeof(*resp)) {
613+
ret = WH_ERROR_ABORTED;
651614
}
652615
if (ret == 0) {
653616
ret = resp->rc;
@@ -712,19 +675,18 @@ int wh_Client_SheEncEcbResponse(whClientContext* c, uint8_t* out, uint32_t sz)
712675

713676
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
714677
if (ret == 0) {
715-
if (group != WH_MESSAGE_GROUP_SHE ||
716-
action != WH_SHE_ENC_ECB ||
717-
dataSz < sizeof(*resp) ||
718-
dataSz < sizeof(*resp) + resp->sz) {
678+
if (dataSz < sizeof(*resp)) {
719679
ret = WH_ERROR_ABORTED;
720680
}
721-
}
722-
if (ret == 0) {
723-
ret = resp->rc;
724-
}
725-
if (ret == 0) {
726-
if (sz < resp->sz) {
727-
ret = WH_ERROR_BUFFER_SIZE;
681+
else if (resp->rc != WH_SHE_ERC_NO_ERROR) {
682+
ret = resp->rc;
683+
}
684+
/* payload is only present on success, so validate its size before copy */
685+
else if (dataSz < sizeof(*resp) + resp->sz) {
686+
ret = WH_ERROR_ABORTED;
687+
}
688+
else if (sz < resp->sz) {
689+
ret = WH_ERROR_BADARGS;
728690
}
729691
else {
730692
memcpy(out, packOut, resp->sz);
@@ -792,19 +754,18 @@ int wh_Client_SheEncCbcResponse(whClientContext* c, uint8_t* out, uint32_t sz)
792754

793755
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
794756
if (ret == 0) {
795-
if (group != WH_MESSAGE_GROUP_SHE ||
796-
action != WH_SHE_ENC_CBC ||
797-
dataSz < sizeof(*resp) ||
798-
dataSz < sizeof(*resp) + resp->sz) {
757+
if (dataSz < sizeof(*resp)) {
799758
ret = WH_ERROR_ABORTED;
800759
}
801-
}
802-
if (ret == 0) {
803-
ret = resp->rc;
804-
}
805-
if (ret == 0) {
806-
if (sz < resp->sz) {
807-
ret = WH_ERROR_BUFFER_SIZE;
760+
else if (resp->rc != WH_SHE_ERC_NO_ERROR) {
761+
ret = resp->rc;
762+
}
763+
/* payload is only present on success, so validate its size before copy */
764+
else if (dataSz < sizeof(*resp) + resp->sz) {
765+
ret = WH_ERROR_ABORTED;
766+
}
767+
else if (sz < resp->sz) {
768+
ret = WH_ERROR_BADARGS;
808769
}
809770
else {
810771
memcpy(out, packOut, resp->sz);
@@ -868,19 +829,18 @@ int wh_Client_SheDecEcbResponse(whClientContext* c, uint8_t* out, uint32_t sz)
868829

869830
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
870831
if (ret == 0) {
871-
if (group != WH_MESSAGE_GROUP_SHE ||
872-
action != WH_SHE_DEC_ECB ||
873-
dataSz < sizeof(*resp) ||
874-
dataSz < sizeof(*resp) + resp->sz) {
832+
if (dataSz < sizeof(*resp)) {
875833
ret = WH_ERROR_ABORTED;
876834
}
877-
}
878-
if (ret == 0) {
879-
ret = resp->rc;
880-
}
881-
if (ret == 0) {
882-
if (sz < resp->sz) {
883-
ret = WH_ERROR_BUFFER_SIZE;
835+
else if (resp->rc != WH_SHE_ERC_NO_ERROR) {
836+
ret = resp->rc;
837+
}
838+
/* payload is only present on success, so validate its size before copy */
839+
else if (dataSz < sizeof(*resp) + resp->sz) {
840+
ret = WH_ERROR_ABORTED;
841+
}
842+
else if (sz < resp->sz) {
843+
ret = WH_ERROR_BADARGS;
884844
}
885845
else {
886846
memcpy(out, packOut, resp->sz);
@@ -948,19 +908,18 @@ int wh_Client_SheDecCbcResponse(whClientContext* c, uint8_t* out, uint32_t sz)
948908

949909
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
950910
if (ret == 0) {
951-
if (group != WH_MESSAGE_GROUP_SHE ||
952-
action != WH_SHE_DEC_CBC ||
953-
dataSz < sizeof(*resp) ||
954-
dataSz < sizeof(*resp) + resp->sz) {
911+
if (dataSz < sizeof(*resp)) {
955912
ret = WH_ERROR_ABORTED;
956913
}
957-
}
958-
if (ret == 0) {
959-
ret = resp->rc;
960-
}
961-
if (ret == 0) {
962-
if (sz < resp->sz) {
963-
ret = WH_ERROR_BUFFER_SIZE;
914+
else if (resp->rc != WH_SHE_ERC_NO_ERROR) {
915+
ret = resp->rc;
916+
}
917+
/* payload is only present on success, so validate its size before copy */
918+
else if (dataSz < sizeof(*resp) + resp->sz) {
919+
ret = WH_ERROR_ABORTED;
920+
}
921+
else if (sz < resp->sz) {
922+
ret = WH_ERROR_BADARGS;
964923
}
965924
else {
966925
memcpy(out, packOut, resp->sz);
@@ -1023,17 +982,15 @@ int wh_Client_SheGenerateMacResponse(whClientContext* c, uint8_t* out,
1023982

1024983
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
1025984
if (ret == 0) {
1026-
if (group != WH_MESSAGE_GROUP_SHE ||
1027-
action != WH_SHE_GEN_MAC ||
1028-
dataSz < sizeof(*resp)) {
985+
if (dataSz < sizeof(*resp)) {
1029986
ret = WH_ERROR_ABORTED;
1030987
}
1031-
}
1032-
if (ret == 0) {
1033-
ret = resp->rc;
1034-
}
1035-
if (ret == 0) {
1036-
memcpy(out, resp->mac, WH_SHE_KEY_SZ);
988+
else if (resp->rc != WH_SHE_ERC_NO_ERROR) {
989+
ret = resp->rc;
990+
}
991+
else {
992+
memcpy(out, resp->mac, WH_SHE_KEY_SZ);
993+
}
1037994
}
1038995
return ret;
1039996
}
@@ -1100,17 +1057,15 @@ int wh_Client_SheVerifyMacResponse(whClientContext* c, uint8_t* outStatus)
11001057
resp = (whMessageShe_VerifyMacResponse*)wh_CommClient_GetDataPtr(c->comm);
11011058
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
11021059
if (ret == 0) {
1103-
if (group != WH_MESSAGE_GROUP_SHE ||
1104-
action != WH_SHE_VERIFY_MAC ||
1105-
dataSz < sizeof(*resp)) {
1060+
if (dataSz < sizeof(*resp)) {
11061061
ret = WH_ERROR_ABORTED;
11071062
}
1108-
}
1109-
if (ret == 0) {
1110-
ret = resp->rc;
1111-
}
1112-
if (ret == 0) {
1113-
*outStatus = resp->status;
1063+
else if (resp->rc != WH_SHE_ERC_NO_ERROR) {
1064+
ret = resp->rc;
1065+
}
1066+
else {
1067+
*outStatus = resp->status;
1068+
}
11141069
}
11151070
return ret;
11161071
}

0 commit comments

Comments
 (0)