[mask_rom, sig_verify] Use 32-bit arithmetic in subtract_modulus
Signed-off-by: Alphan Ulusoy <alphan@google.com>
diff --git a/sw/device/silicon_creator/lib/drivers/hmac.c b/sw/device/silicon_creator/lib/drivers/hmac.c
index 949256b..b60c994 100644
--- a/sw/device/silicon_creator/lib/drivers/hmac.c
+++ b/sw/device/silicon_creator/lib/drivers/hmac.c
@@ -7,13 +7,13 @@
#include "sw/device/lib/base/bitfield.h"
#include "sw/device/lib/base/memory.h"
#include "sw/device/lib/base/mmio.h"
+#include "sw/device/silicon_creator/lib/error.h"
#include "hmac_regs.h" // Generated.
-// TODO(#34): Use the unified error space
-int hmac_sha256_init(const hmac_t *hmac) {
+rom_error_t hmac_sha256_init(const hmac_t *hmac) {
if (hmac == NULL) {
- return -1;
+ return kErrorHmacInvalidArgument;
}
// Clear the config, stopping the SHA engine.
@@ -34,12 +34,13 @@
reg = bitfield_bit32_write(reg, HMAC_CMD_HASH_START_BIT, true);
mmio_region_write32(hmac->base_addr, HMAC_CMD_REG_OFFSET, reg);
- return 0;
+ return kErrorOk;
}
-int hmac_sha256_update(const hmac_t *hmac, const void *data, size_t len) {
+rom_error_t hmac_sha256_update(const hmac_t *hmac, const void *data,
+ size_t len) {
if (hmac == NULL || data == NULL) {
- return -1;
+ return kErrorHmacInvalidArgument;
}
const uint8_t *data_sent = (const uint8_t *)data;
@@ -60,12 +61,12 @@
for (; len != 0; --len) {
mmio_region_write8(hmac->base_addr, HMAC_MSG_FIFO_REG_OFFSET, *data_sent++);
}
- return 0;
+ return kErrorOk;
}
-int hmac_sha256_final(const hmac_t *hmac, hmac_digest_t *digest) {
+rom_error_t hmac_sha256_final(const hmac_t *hmac, hmac_digest_t *digest) {
if (hmac == NULL || digest == NULL) {
- return -1;
+ return kErrorHmacInvalidArgument;
}
uint32_t reg = 0;
@@ -82,5 +83,5 @@
mmio_region_memcpy_from_mmio32(hmac->base_addr, HMAC_DIGEST_0_REG_OFFSET,
digest->digest,
HMAC_PARAM_NUMWORDS * sizeof(uint32_t));
- return 0;
+ return kErrorOk;
}
diff --git a/sw/device/silicon_creator/lib/drivers/hmac.h b/sw/device/silicon_creator/lib/drivers/hmac.h
index 819e79b..9eed615 100644
--- a/sw/device/silicon_creator/lib/drivers/hmac.h
+++ b/sw/device/silicon_creator/lib/drivers/hmac.h
@@ -8,6 +8,7 @@
#include <stdint.h>
#include "sw/device/lib/base/mmio.h"
+#include "sw/device/silicon_creator/lib/error.h"
#ifdef __cplusplus
extern "C" {
@@ -40,7 +41,7 @@
* @param hmac A HMAC handle.
* @return The result of the operation.
*/
-int hmac_sha256_init(const hmac_t *hmac);
+rom_error_t hmac_sha256_init(const hmac_t *hmac);
/**
* Sends `len` bytes from `data` to the SHA2-256 function.
@@ -54,7 +55,8 @@
* @param len size of the `data` buffer.
* @return The result of the operation.
*/
-int hmac_sha256_update(const hmac_t *hmac, const void *data, size_t len);
+rom_error_t hmac_sha256_update(const hmac_t *hmac, const void *data,
+ size_t len);
/**
* Finalizes SHA256 operation and writes `digest` buffer.
@@ -63,7 +65,7 @@
* @param[out] digest Buffer to copy digest to.
* @return The result of the operation.
*/
-int hmac_sha256_final(const hmac_t *hmac, hmac_digest_t *digest);
+rom_error_t hmac_sha256_final(const hmac_t *hmac, hmac_digest_t *digest);
#ifdef __cplusplus
}
diff --git a/sw/device/silicon_creator/lib/drivers/hmac_unittest.cc b/sw/device/silicon_creator/lib/drivers/hmac_unittest.cc
index 7d5b434..91b4a7b 100644
--- a/sw/device/silicon_creator/lib/drivers/hmac_unittest.cc
+++ b/sw/device/silicon_creator/lib/drivers/hmac_unittest.cc
@@ -10,6 +10,7 @@
#include "gtest/gtest.h"
#include "sw/device/lib/base/mmio.h"
#include "sw/device/lib/base/testing/mock_mmio.h"
+#include "sw/device/silicon_creator/lib/error.h"
#include "hmac_regs.h" // Generated.
@@ -30,8 +31,7 @@
class Sha256InitTest : public HmacTest {};
TEST_F(Sha256InitTest, NullArgs) {
- // FIXME: unified error space.
- EXPECT_EQ(hmac_sha256_init(nullptr), -1);
+ EXPECT_EQ(hmac_sha256_init(nullptr), kErrorHmacInvalidArgument);
}
TEST_F(Sha256InitTest, Initialize) {
@@ -45,16 +45,17 @@
{HMAC_CFG_HMAC_EN_BIT, false},
});
EXPECT_WRITE32(HMAC_CMD_REG_OFFSET, {{HMAC_CMD_HASH_START_BIT, true}});
- EXPECT_EQ(hmac_sha256_init(&hmac_), 0);
+ EXPECT_EQ(hmac_sha256_init(&hmac_), kErrorOk);
}
class Sha256UpdateTest : public HmacTest {};
TEST_F(Sha256UpdateTest, NullArgs) {
- EXPECT_EQ(hmac_sha256_update(&hmac_, nullptr, 0), -1);
+ EXPECT_EQ(hmac_sha256_update(&hmac_, nullptr, 0), kErrorHmacInvalidArgument);
uint32_t data;
- EXPECT_EQ(hmac_sha256_update(nullptr, &data, sizeof(uint32_t)), -1);
+ EXPECT_EQ(hmac_sha256_update(nullptr, &data, sizeof(uint32_t)),
+ kErrorHmacInvalidArgument);
}
TEST_F(Sha256UpdateTest, SendData) {
@@ -66,11 +67,11 @@
// Trigger 8bit aligned writes.
EXPECT_WRITE8(HMAC_MSG_FIFO_REG_OFFSET, 0x01);
EXPECT_WRITE8(HMAC_MSG_FIFO_REG_OFFSET, 0x02);
- EXPECT_EQ(hmac_sha256_update(&hmac_, &kData[1], 2), 0);
+ EXPECT_EQ(hmac_sha256_update(&hmac_, &kData[1], 2), kErrorOk);
// Trigger a single 32bit aligned write.
EXPECT_WRITE32(HMAC_MSG_FIFO_REG_OFFSET, 0x03020100);
- EXPECT_EQ(hmac_sha256_update(&hmac_, &kData[0], 4), 0);
+ EXPECT_EQ(hmac_sha256_update(&hmac_, &kData[0], 4), kErrorOk);
// Trigger 8bit/32bit/8bit sequence.
EXPECT_WRITE8(HMAC_MSG_FIFO_REG_OFFSET, 0x02);
@@ -78,16 +79,16 @@
EXPECT_WRITE32(HMAC_MSG_FIFO_REG_OFFSET, 0x07060504);
EXPECT_WRITE8(HMAC_MSG_FIFO_REG_OFFSET, 0x08);
EXPECT_WRITE8(HMAC_MSG_FIFO_REG_OFFSET, 0x09);
- EXPECT_EQ(hmac_sha256_update(&hmac_, &kData[2], 8), 0);
+ EXPECT_EQ(hmac_sha256_update(&hmac_, &kData[2], 8), kErrorOk);
}
class Sha256FinalTest : public HmacTest {};
TEST_F(Sha256FinalTest, NullArgs) {
- EXPECT_EQ(hmac_sha256_final(&hmac_, nullptr), -1);
+ EXPECT_EQ(hmac_sha256_final(&hmac_, nullptr), kErrorHmacInvalidArgument);
hmac_digest_t digest;
- EXPECT_EQ(hmac_sha256_final(nullptr, &digest), -1);
+ EXPECT_EQ(hmac_sha256_final(nullptr, &digest), kErrorHmacInvalidArgument);
}
TEST_F(Sha256FinalTest, GetDigest) {
@@ -125,7 +126,7 @@
EXPECT_READ32(HMAC_DIGEST_7_REG_OFFSET, kExpectedDigest[7]);
hmac_digest_t got_digest;
- EXPECT_EQ(hmac_sha256_final(&hmac_, &got_digest), 0);
+ EXPECT_EQ(hmac_sha256_final(&hmac_, &got_digest), kErrorOk);
EXPECT_THAT(got_digest.digest, ElementsAreArray(kExpectedDigest));
}
diff --git a/sw/device/silicon_creator/lib/drivers/meson.build b/sw/device/silicon_creator/lib/drivers/meson.build
index afbb645..ecd73c6 100644
--- a/sw/device/silicon_creator/lib/drivers/meson.build
+++ b/sw/device/silicon_creator/lib/drivers/meson.build
@@ -17,20 +17,22 @@
)
test('sw_silicon_creator_lib_driver_hmac_unittest', executable(
- 'sw_silicon_creator_lib_driver_hmac_unittest',
- sources: [
- 'hmac_unittest.cc',
- hw_ip_hmac_reg_h,
- 'hmac.c',
- ],
- dependencies: [
- sw_vendor_gtest,
- sw_lib_base_testing_mock_mmio,
- ],
- native: true,
- c_args: ['-DMOCK_MMIO'],
- cpp_args: ['-DMOCK_MMIO'],
-))
+ 'sw_silicon_creator_lib_driver_hmac_unittest',
+ sources: [
+ 'hmac_unittest.cc',
+ hw_ip_hmac_reg_h,
+ 'hmac.c',
+ ],
+ dependencies: [
+ sw_vendor_gtest,
+ sw_lib_base_testing_mock_mmio,
+ ],
+ native: true,
+ c_args: ['-DMOCK_MMIO'],
+ cpp_args: ['-DMOCK_MMIO'],
+ ),
+ suite: 'mask_rom',
+)
# Mask ROM uart driver
sw_silicon_creator_lib_driver_uart = declare_dependency(
@@ -47,17 +49,19 @@
)
test('sw_silicon_creator_lib_driver_uart_unittest', executable(
- 'sw_silicon_creator_lib_driver_uart_unittest',
- sources: [
- 'uart_unittest.cc',
- hw_ip_uart_reg_h,
- 'uart.c',
- ],
- dependencies: [
- sw_vendor_gtest,
- sw_lib_base_testing_mock_mmio,
- ],
- native: true,
- c_args: ['-DMOCK_MMIO'],
- cpp_args: ['-DMOCK_MMIO'],
-))
+ 'sw_silicon_creator_lib_driver_uart_unittest',
+ sources: [
+ 'uart_unittest.cc',
+ hw_ip_uart_reg_h,
+ 'uart.c',
+ ],
+ dependencies: [
+ sw_vendor_gtest,
+ sw_lib_base_testing_mock_mmio,
+ ],
+ native: true,
+ c_args: ['-DMOCK_MMIO'],
+ cpp_args: ['-DMOCK_MMIO'],
+ ),
+ suite: 'mask_rom',
+)
diff --git a/sw/device/silicon_creator/lib/error.h b/sw/device/silicon_creator/lib/error.h
index 445b85a..36989bd 100644
--- a/sw/device/silicon_creator/lib/error.h
+++ b/sw/device/silicon_creator/lib/error.h
@@ -44,6 +44,7 @@
X(kErrorOk, 0x739), \
X(kErrorUartInvalidArgument, ERROR_(1, kModuleUart, kInvalidArgument)), \
X(kErrorUartBadBaudRate, ERROR_(2, kModuleUart, kInvalidArgument)), \
+ X(kErrorHmacInvalidArgument, ERROR_(1, kModuleHmac, kInvalidArgument)), \
X(kErrorUnknown, 0xFFFFFFFF)
// clang-format on
diff --git a/sw/device/silicon_creator/lib/meson.build b/sw/device/silicon_creator/lib/meson.build
index 79750aa..7eba9ca 100644
--- a/sw/device/silicon_creator/lib/meson.build
+++ b/sw/device/silicon_creator/lib/meson.build
@@ -17,13 +17,15 @@
)
test('sw_silicon_creator_lib_error_unittest', executable(
- 'sw_silicon_creator_lib_error_unittest',
- sources: [
- 'error_unittest.cc',
- ],
- dependencies: [
- sw_vendor_gtest,
- ],
- native: true,
-))
+ 'sw_silicon_creator_lib_error_unittest',
+ sources: [
+ 'error_unittest.cc',
+ ],
+ dependencies: [
+ sw_vendor_gtest,
+ ],
+ native: true,
+ ),
+ suite: 'mask_rom',
+)
diff --git a/sw/device/silicon_creator/mask_rom/mask_rom.c b/sw/device/silicon_creator/mask_rom/mask_rom.c
index 41301ca..77d7b42 100644
--- a/sw/device/silicon_creator/mask_rom/mask_rom.c
+++ b/sw/device/silicon_creator/mask_rom/mask_rom.c
@@ -42,24 +42,20 @@
uint32_t rom_ext_identifier = rom_ext_get_identifier(rom_ext);
if (rom_ext_identifier != kRomExtIdentifierExpected) {
- return -1;
+ return kErrorUnknown;
}
- if (hmac_sha256_init(&hmac) != 0) {
- return -1;
- }
-
- if (hmac_sha256_update(&hmac, &rom_ext_identifier, sizeof(uint32_t)) != 0) {
- return -1;
- }
+ RETURN_IF_ERROR(hmac_sha256_init(&hmac));
+ RETURN_IF_ERROR(
+ hmac_sha256_update(&hmac, &rom_ext_identifier, sizeof(uint32_t)));
hmac_digest_t digest;
- if (hmac_sha256_final(&hmac, &digest) != 0) {
- return -1;
- }
+ RETURN_IF_ERROR(hmac_sha256_final(&hmac, &digest));
return memcmp(digest.digest, kROMExtIdentifierExpectedDigest,
- sizeof(digest.digest));
+ sizeof(digest.digest)) == 0
+ ? kErrorOk
+ : kErrorUnknown;
}
void mask_rom_boot(void) {
@@ -161,7 +157,7 @@
// break
//}
// Temporary implementation for the two above `if` blocks.
- if (verify_rom_ext_identifier(rom_ext) != 0) {
+ if (verify_rom_ext_identifier(rom_ext) != kErrorOk) {
break;
}
diff --git a/sw/device/silicon_creator/mask_rom/rsa_verify.c b/sw/device/silicon_creator/mask_rom/rsa_verify.c
index f01c343..e4b6872 100644
--- a/sw/device/silicon_creator/mask_rom/rsa_verify.c
+++ b/sw/device/silicon_creator/mask_rom/rsa_verify.c
@@ -23,9 +23,11 @@
sigverify_rsa_buffer_t *a) {
uint32_t borrow = 0;
for (size_t i = 0; i < ARRAYSIZE(a->data); ++i) {
- uint64_t diff = (uint64_t)a->data[i] - key->n.data[i] - borrow;
- borrow = diff > a->data[i];
- a->data[i] = (uint32_t)diff;
+ uint32_t temp = a->data[i] - borrow;
+ // We borrow if either the above or the below subtraction wraps around.
+ // Note: borrow can only be 0 or 1.
+ borrow = (a->data[i] < borrow) + (temp < key->n.data[i]);
+ a->data[i] = temp - key->n.data[i];
}
return borrow;
}
diff --git a/sw/device/tests/rom_ext/meson.build b/sw/device/tests/rom_ext/meson.build
index b4f51a5..38a8061 100644
--- a/sw/device/tests/rom_ext/meson.build
+++ b/sw/device/tests/rom_ext/meson.build
@@ -11,18 +11,20 @@
}
test('rom_ext_parser_unittest', executable(
- 'rom_ext_parser_unittest',
- sources: [
- 'rom_ext_parser_unittest.cc',
- meson.source_root() / 'sw/device/silicon_creator/rom_exts/rom_ext_manifest_parser.c',
- rom_exts_manifest_offsets_header,
- ],
- dependencies: [
- sw_vendor_gtest,
- sw_lib_base_testing_mock_mmio,
- sw_lib_testing_bitfield,
- ],
- native: true,
- c_args: ['-DMOCK_MMIO'],
- cpp_args: ['-DMOCK_MMIO'],
-))
+ 'rom_ext_parser_unittest',
+ sources: [
+ 'rom_ext_parser_unittest.cc',
+ meson.source_root() / 'sw/device/silicon_creator/rom_exts/rom_ext_manifest_parser.c',
+ rom_exts_manifest_offsets_header,
+ ],
+ dependencies: [
+ sw_vendor_gtest,
+ sw_lib_base_testing_mock_mmio,
+ sw_lib_testing_bitfield,
+ ],
+ native: true,
+ c_args: ['-DMOCK_MMIO'],
+ cpp_args: ['-DMOCK_MMIO'],
+ ),
+ suite: 'mask_rom',
+)
