From 1851825b05d266509c4157d1413c0d853a9add28 Mon Sep 17 00:00:00 2001 From: Joel Kitching Date: Tue, 25 Jun 2019 16:21:09 +0800 Subject: vboot: remove VerifyMemberInside function Use vboot2-style vb2_verify_member_inside instead. Correct some strings in vboot2 tests to refer to new vboot2 functions instead. BUG=b:124141368 TEST=make clean && make runtests BRANCH=none Change-Id: Idb3bcf1657c9d955acc6f93983c7b0c7f06427e3 Signed-off-by: Joel Kitching Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1675870 Tested-by: Joel Kitching Commit-Queue: Joel Kitching Reviewed-by: Julius Werner --- firmware/lib/include/vboot_common.h | 5 ----- firmware/lib/vboot_common.c | 43 ++++++----------------------------- firmware/linktest/main.c | 1 - tests/vb2_common_tests.c | 32 +++++++++++++------------- tests/vboot_common_tests.c | 45 +++++++------------------------------ 5 files changed, 31 insertions(+), 95 deletions(-) diff --git a/firmware/lib/include/vboot_common.h b/firmware/lib/include/vboot_common.h index b01ed3b2..9bf9f0e5 100644 --- a/firmware/lib/include/vboot_common.h +++ b/firmware/lib/include/vboot_common.h @@ -57,11 +57,6 @@ const uint8_t *GetSignatureDataC(const VbSignature *sig); * parent data. Returns 0 if inside, 1 if error. */ -int VerifyMemberInside(const void *parent, uint64_t parent_size, - const void *member, uint64_t member_size, - uint64_t member_data_offset, - uint64_t member_data_size); - int VerifyPublicKeyInside(const void *parent, uint64_t parent_size, const VbPublicKey *key); diff --git a/firmware/lib/vboot_common.c b/firmware/lib/vboot_common.c index ea44588c..3238fd5c 100644 --- a/firmware/lib/vboot_common.c +++ b/firmware/lib/vboot_common.c @@ -52,52 +52,23 @@ const uint8_t *GetSignatureDataC(const VbSignature *sig) /* * Helper functions to verify the data pointed to by a subfield is inside - * the parent data. Returns 0 if inside, 1 if error. + * the parent data. */ -int VerifyMemberInside(const void *parent, uint64_t parent_size, - const void *member, uint64_t member_size, - uint64_t member_data_offset, - uint64_t member_data_size) -{ - uint64_t end = vb2_offset_of(parent, member); - - if (end > parent_size) - return 1; - - if (UINT64_MAX - end < member_size) - return 1; /* Detect wraparound in integer math */ - if (end + member_size > parent_size) - return 1; - - if (UINT64_MAX - end < member_data_offset) - return 1; - end += member_data_offset; - if (end > parent_size) - return 1; - - if (UINT64_MAX - end < member_data_size) - return 1; - if (end + member_data_size > parent_size) - return 1; - - return 0; -} - int VerifyPublicKeyInside(const void *parent, uint64_t parent_size, const VbPublicKey *key) { - return VerifyMemberInside(parent, parent_size, - key, sizeof(VbPublicKey), - key->key_offset, key->key_size); + return vb2_verify_member_inside(parent, parent_size, + key, sizeof(VbPublicKey), + key->key_offset, key->key_size); } int VerifySignatureInside(const void *parent, uint64_t parent_size, const VbSignature *sig) { - return VerifyMemberInside(parent, parent_size, - sig, sizeof(VbSignature), - sig->sig_offset, sig->sig_size); + return vb2_verify_member_inside(parent, parent_size, + sig, sizeof(VbSignature), + sig->sig_offset, sig->sig_size); } void PublicKeyInit(VbPublicKey *key, uint8_t *key_data, uint64_t key_size) diff --git a/firmware/linktest/main.c b/firmware/linktest/main.c index 6877bfad..03195ed2 100644 --- a/firmware/linktest/main.c +++ b/firmware/linktest/main.c @@ -62,7 +62,6 @@ int main(void) GetPublicKeyDataC(0); GetSignatureData(0); GetSignatureDataC(0); - VerifyMemberInside(0, 0, 0, 0, 0, 0); VerifyPublicKeyInside(0, 0, 0); VerifySignatureInside(0, 0, 0); PublicKeyInit(0, 0, 0); diff --git a/tests/vb2_common_tests.c b/tests/vb2_common_tests.c index acc904e9..2b44233b 100644 --- a/tests/vb2_common_tests.c +++ b/tests/vb2_common_tests.c @@ -173,52 +173,52 @@ static void test_helper_functions(void) { uint8_t *p = (uint8_t *)test_helper_functions; TEST_SUCC(vb2_verify_member_inside(p, 20, p, 6, 11, 3), - "MemberInside ok 1"); + "vb2_verify_member_inside() ok 1"); TEST_SUCC(vb2_verify_member_inside(p, 20, p+4, 4, 8, 4), - "MemberInside ok 2"); + "vb2_verify_member_inside() ok 2"); TEST_EQ(vb2_verify_member_inside(p, 20, p-4, 4, 8, 4), VB2_ERROR_INSIDE_MEMBER_OUTSIDE, - "MemberInside member before parent"); + "vb2_verify_member_inside() member before parent"); TEST_EQ(vb2_verify_member_inside(p, 20, p+20, 4, 8, 4), VB2_ERROR_INSIDE_MEMBER_OUTSIDE, - "MemberInside member after parent"); + "vb2_verify_member_inside() member after parent"); TEST_EQ(vb2_verify_member_inside(p, 20, p, 21, 0, 0), VB2_ERROR_INSIDE_MEMBER_OUTSIDE, - "MemberInside member too big"); + "vb2_verify_member_inside() member too big"); TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, 21, 0), VB2_ERROR_INSIDE_DATA_OUTSIDE, - "MemberInside data after parent"); + "vb2_verify_member_inside() data after parent"); TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, SIZE_MAX, 0), VB2_ERROR_INSIDE_DATA_OUTSIDE, - "MemberInside data before parent"); + "vb2_verify_member_inside() data before parent"); TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, 4, 17), VB2_ERROR_INSIDE_DATA_OUTSIDE, - "MemberInside data too big"); + "vb2_verify_member_inside() data too big"); TEST_EQ(vb2_verify_member_inside(p, 20, p, 8, 4, 8), VB2_ERROR_INSIDE_DATA_OVERLAP, - "MemberInside data overlaps member"); + "vb2_verify_member_inside() data overlaps member"); TEST_EQ(vb2_verify_member_inside(p, -8, p, 12, 0, 0), VB2_ERROR_INSIDE_PARENT_WRAPS, - "MemberInside wraparound 1"); + "vb2_verify_member_inside() wraparound 1"); TEST_EQ(vb2_verify_member_inside(p, 20, p, -8, 0, 0), VB2_ERROR_INSIDE_MEMBER_WRAPS, - "MemberInside wraparound 2"); + "vb2_verify_member_inside() wraparound 2"); TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, 4, -12), VB2_ERROR_INSIDE_DATA_WRAPS, - "MemberInside wraparound 3"); + "vb2_verify_member_inside() wraparound 3"); } { struct vb2_packed_key k = {.key_offset = sizeof(k), .key_size = 128}; TEST_SUCC(vb2_verify_packed_key_inside(&k, sizeof(k)+128, &k), - "PublicKeyInside ok 1"); + "vb2_packed_key_inside() ok 1"); TEST_SUCC(vb2_verify_packed_key_inside(&k - 1, 2*sizeof(k)+128, &k), - "PublicKeyInside ok 2"); + "vb2_packed_key_inside() ok 2"); TEST_EQ(vb2_verify_packed_key_inside(&k, 128, &k), VB2_ERROR_INSIDE_DATA_OUTSIDE, - "PublicKeyInside key too big"); + "vb2_packed_key_inside() key too big"); } { @@ -226,7 +226,7 @@ static void test_helper_functions(void) .key_size = 4}; TEST_EQ(vb2_verify_packed_key_inside(&k, 99, &k), VB2_ERROR_INSIDE_DATA_OUTSIDE, - "PublicKeyInside offset too big"); + "vb2_packed_key_inside() offset too big"); } } diff --git a/tests/vboot_common_tests.c b/tests/vboot_common_tests.c index 277a44a0..9bbfba69 100644 --- a/tests/vboot_common_tests.c +++ b/tests/vboot_common_tests.c @@ -59,49 +59,20 @@ static void VerifyHelperFunctions(void) "GetPublicKeyDataC() spaced"); } - { - uint8_t *p = (uint8_t *)VerifyHelperFunctions; - TEST_EQ(VerifyMemberInside(p, 20, p, 6, 11, 3), 0, - "MemberInside ok 1"); - TEST_EQ(VerifyMemberInside(p, 20, p+4, 4, 8, 4), 0, - "MemberInside ok 2"); - TEST_EQ(VerifyMemberInside(p, 20, p-4, 4, 8, 4), 1, - "MemberInside member before parent"); - TEST_EQ(VerifyMemberInside(p, 20, p+20, 4, 8, 4), 1, - "MemberInside member after parent"); - TEST_EQ(VerifyMemberInside(p, 20, p, 21, 0, 0), 1, - "MemberInside member too big"); - TEST_EQ(VerifyMemberInside(p, 20, p, 4, 21, 0), 1, - "MemberInside data after parent"); - TEST_EQ(VerifyMemberInside(p, 20, p, 4, (uint64_t)-1, 0), 1, - "MemberInside data before parent"); - TEST_EQ(VerifyMemberInside(p, 20, p, 4, 4, 17), 1, - "MemberInside data too big"); - TEST_EQ(VerifyMemberInside(p, (uint64_t)-1, - p+(uint64_t)-10, 12, 5, 0), 1, - "MemberInside wraparound 1"); - TEST_EQ(VerifyMemberInside(p, (uint64_t)-1, - p+(uint64_t)-10, 5, 12, 0), 1, - "MemberInside wraparound 2"); - TEST_EQ(VerifyMemberInside(p, (uint64_t)-1, - p+(uint64_t)-10, 5, 0, 12), 1, - "MemberInside wraparound 3"); - } - { VbPublicKey k = {sizeof(k), 128, 0, 0}; TEST_EQ(VerifyPublicKeyInside(&k, sizeof(k)+128, &k), 0, "PublicKeyInside ok 1"); TEST_EQ(VerifyPublicKeyInside(&k - 1, 2*sizeof(k)+128, &k), 0, "PublicKeyInside ok 2"); - TEST_EQ(VerifyPublicKeyInside(&k, 128, &k), 1, - "PublicKeyInside key too big"); + TEST_NEQ(VerifyPublicKeyInside(&k, 128, &k), 0, + "PublicKeyInside key too big"); } { VbPublicKey k = {100, 4, 0, 0}; - TEST_EQ(VerifyPublicKeyInside(&k, 99, &k), 1, - "PublicKeyInside offset too big"); + TEST_NEQ(VerifyPublicKeyInside(&k, 99, &k), 0, + "PublicKeyInside offset too big"); } { @@ -110,14 +81,14 @@ static void VerifyHelperFunctions(void) "SignatureInside ok 1"); TEST_EQ(VerifySignatureInside(&s - 1, 2*sizeof(s)+128, &s), 0, "SignatureInside ok 2"); - TEST_EQ(VerifySignatureInside(&s, 128, &s), 1, - "SignatureInside sig too big"); + TEST_NEQ(VerifySignatureInside(&s, 128, &s), 0, + "SignatureInside sig too big"); } { VbSignature s = {100, 4, 0}; - TEST_EQ(VerifySignatureInside(&s, 99, &s), 1, - "SignatureInside offset too big"); + TEST_NEQ(VerifySignatureInside(&s, 99, &s), 0, + "SignatureInside offset too big"); } } -- cgit v1.2.1