From d9edb778ae1dfeaa408865e9fd21f27a58b12b7d Mon Sep 17 00:00:00 2001 From: Tim Trippel Date: Mon, 22 Jan 2024 14:22:07 -0800 Subject: [PATCH 1/3] [provisioning] fix kmac init bug in FT personalize KMAC must be configured properly for the keymgr to advance. Additionally, this erases the flash info pages that store certificitates before they are written to (to ensure we always start from a clean slate). Signed-off-by: Tim Trippel --- .../manuf/skus/earlgrey_a0/sival_bringup/BUILD | 2 ++ .../skus/earlgrey_a0/sival_bringup/ft_personalize_3.c | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/sw/device/silicon_creator/manuf/skus/earlgrey_a0/sival_bringup/BUILD b/sw/device/silicon_creator/manuf/skus/earlgrey_a0/sival_bringup/BUILD index d23293a382b79..7ce5b1d9baec3 100644 --- a/sw/device/silicon_creator/manuf/skus/earlgrey_a0/sival_bringup/BUILD +++ b/sw/device/silicon_creator/manuf/skus/earlgrey_a0/sival_bringup/BUILD @@ -278,7 +278,9 @@ opentitan_binary( "//sw/device/silicon_creator/lib:error", "//sw/device/silicon_creator/lib:otbn_boot_services", "//sw/device/silicon_creator/lib/drivers:flash_ctrl", + "//sw/device/silicon_creator/lib/drivers:hmac", "//sw/device/silicon_creator/lib/drivers:keymgr", + "//sw/device/silicon_creator/lib/drivers:kmac", "//sw/device/silicon_creator/manuf/lib:flash_info_fields", "//sw/otbn/crypto:boot", ], diff --git a/sw/device/silicon_creator/manuf/skus/earlgrey_a0/sival_bringup/ft_personalize_3.c b/sw/device/silicon_creator/manuf/skus/earlgrey_a0/sival_bringup/ft_personalize_3.c index 0080164af0bb1..8acc173614b46 100644 --- a/sw/device/silicon_creator/manuf/skus/earlgrey_a0/sival_bringup/ft_personalize_3.c +++ b/sw/device/silicon_creator/manuf/skus/earlgrey_a0/sival_bringup/ft_personalize_3.c @@ -15,6 +15,7 @@ #include "sw/device/silicon_creator/lib/drivers/flash_ctrl.h" #include "sw/device/silicon_creator/lib/drivers/hmac.h" #include "sw/device/silicon_creator/lib/drivers/keymgr.h" +#include "sw/device/silicon_creator/lib/drivers/kmac.h" #include "sw/device/silicon_creator/lib/error.h" #include "sw/device/silicon_creator/lib/keymgr_binding_value.h" #include "sw/device/silicon_creator/lib/otbn_boot_services.h" @@ -74,6 +75,8 @@ static status_t personalize(ujson_t *uj) { // Advance keymgr to Initialized state. TRY(entropy_complex_init()); + // Initialize KMAC for key manager operations. + TRY(kmac_keymgr_configure()); keymgr_advance_state(); TRY(keymgr_state_check(kKeymgrStateInit)); @@ -100,6 +103,8 @@ static status_t personalize(ujson_t *uj) { kAttestationPublicKeyCoordBytes); memcpy(out_data.uds_certificate.y, curr_pubkey.y, kAttestationPublicKeyCoordBytes); + TRY(flash_ctrl_info_erase(&kFlashCtrlInfoPageUdsCertificate, + kFlashCtrlEraseTypePage)); TRY(flash_ctrl_info_write(&kFlashCtrlInfoPageUdsCertificate, kFlashInfoFieldUdsCertificate.byte_offset, sizeof(attestation_public_key_t) / sizeof(uint32_t), @@ -136,6 +141,8 @@ static status_t personalize(ujson_t *uj) { kAttestationPublicKeyCoordBytes); memcpy(out_data.cdi_0_certificate.y, curr_pubkey.y, kAttestationPublicKeyCoordBytes); + TRY(flash_ctrl_info_erase(&kFlashCtrlInfoPageCdi0Certificate, + kFlashCtrlEraseTypePage)); TRY(flash_ctrl_info_write(&kFlashCtrlInfoPageCdi0Certificate, kFlashInfoFieldCdi0Certificate.byte_offset, sizeof(attestation_public_key_t) / sizeof(uint32_t), @@ -165,6 +172,8 @@ static status_t personalize(ujson_t *uj) { kAttestationPublicKeyCoordBytes); memcpy(out_data.cdi_1_certificate.y, curr_pubkey.y, kAttestationPublicKeyCoordBytes); + TRY(flash_ctrl_info_erase(&kFlashCtrlInfoPageCdi1Certificate, + kFlashCtrlEraseTypePage)); TRY(flash_ctrl_info_write(&kFlashCtrlInfoPageCdi1Certificate, kFlashInfoFieldCdi1Certificate.byte_offset, sizeof(attestation_public_key_t) / sizeof(uint32_t), From a02ee9c19871ddef1b7f3bd151244cace22f2c75 Mon Sep 17 00:00:00 2001 From: Tim Trippel Date: Mon, 22 Jan 2024 16:45:06 -0800 Subject: [PATCH 2/3] [bazel] fix bug in cert gen bazel rule The bazel rule must add the basename of the autogenerated files, not the pathname. Signed-off-by: Tim Trippel --- rules/certificates.bzl | 3 ++- sw/device/silicon_creator/lib/cert/BUILD | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/rules/certificates.bzl b/rules/certificates.bzl index db234a96eed3e..5cee588d5060d 100644 --- a/rules/certificates.bzl +++ b/rules/certificates.bzl @@ -1,6 +1,7 @@ # Copyright lowRISC contributors. # Licensed under the Apache License, Version 2.0, see LICENSE for details. # SPDX-License-Identifier: Apache-2.0 + load("@bazel_skylib//lib:paths.bzl", "paths") load("//rules:rv.bzl", "rv_rule") load("//rules/opentitan:toolchain.bzl", "LOCALTOOLS_TOOLCHAIN") @@ -8,7 +9,7 @@ load("//rules/opentitan:toolchain.bzl", "LOCALTOOLS_TOOLCHAIN") def _certificate_codegen_impl(ctx): tc = ctx.toolchains[LOCALTOOLS_TOOLCHAIN] - basename = paths.replace_extension(ctx.file.template.path, "") + basename = paths.replace_extension(ctx.file.template.basename, "") # Output files before formatting. pre_c = ctx.actions.declare_file("{}.pre.c".format(basename)) diff --git a/sw/device/silicon_creator/lib/cert/BUILD b/sw/device/silicon_creator/lib/cert/BUILD index 7f3ab984011d8..03eac6f9c490d 100644 --- a/sw/device/silicon_creator/lib/cert/BUILD +++ b/sw/device/silicon_creator/lib/cert/BUILD @@ -7,6 +7,8 @@ load( "certificate_template", ) +package(default_visibility = ["//visibility:public"]) + certificate_template( name = "generic_template", template = "generic.hjson", From 8c1f607b5ec9918d526842390a7c28fbd4a7852e Mon Sep 17 00:00:00 2001 From: Tim Trippel Date: Mon, 22 Jan 2024 16:49:01 -0800 Subject: [PATCH 3/3] [cert] correct hash sizes in cert templates A SHA256 hash is 32 bytes, not 20. Signed-off-by: Tim Trippel --- sw/device/silicon_creator/lib/cert/cdi_0.hjson | 8 ++++---- sw/device/silicon_creator/lib/cert/cdi_1.hjson | 4 ++-- sw/device/silicon_creator/lib/cert/generic.hjson | 4 ++-- sw/device/silicon_creator/lib/cert/uds.hjson | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/sw/device/silicon_creator/lib/cert/cdi_0.hjson b/sw/device/silicon_creator/lib/cert/cdi_0.hjson index c9fd731437902..3d82ce95e1628 100644 --- a/sw/device/silicon_creator/lib/cert/cdi_0.hjson +++ b/sw/device/silicon_creator/lib/cert/cdi_0.hjson @@ -28,15 +28,15 @@ type: "byte-array", size: 20, }, - // Hash of the ROM_EXT. + // Hash of the ROM_EXT (SHA256). rom_ext_hash: { type: "byte-array", - size: 20, + size: 32, }, - // Hash of the ownership manifest. + // Hash of the ownership manifest (SHA256). ownership_manifest_hash: { type: "byte-array", - size: 20, + size: 32, }, // ROM_EXT security version, used to prevent rollback. rom_ext_security_version: { diff --git a/sw/device/silicon_creator/lib/cert/cdi_1.hjson b/sw/device/silicon_creator/lib/cert/cdi_1.hjson index 137d8c966c7e3..1aa6c149f269c 100644 --- a/sw/device/silicon_creator/lib/cert/cdi_1.hjson +++ b/sw/device/silicon_creator/lib/cert/cdi_1.hjson @@ -28,10 +28,10 @@ type: "byte-array", size: 20, }, - // Hash of the owner stage firmware. + // Hash of the owner stage firmware (SHA256). owner_firmware_hash: { type: "byte-array", - size: 20, + size: 32, }, // Owner security version, used to prevent rollback. owner_security_version: { diff --git a/sw/device/silicon_creator/lib/cert/generic.hjson b/sw/device/silicon_creator/lib/cert/generic.hjson index a6e3ade02ef9e..ee6d3cd0ca49e 100644 --- a/sw/device/silicon_creator/lib/cert/generic.hjson +++ b/sw/device/silicon_creator/lib/cert/generic.hjson @@ -51,11 +51,11 @@ }, hash_1: { type: "byte-array", - size: 20, + size: 32, }, hash_2: { type: "byte-array", - size: 20, + size: 32, }, security_version: { type: "integer", diff --git a/sw/device/silicon_creator/lib/cert/uds.hjson b/sw/device/silicon_creator/lib/cert/uds.hjson index bf58096f13efa..3d2a152d0df91 100644 --- a/sw/device/silicon_creator/lib/cert/uds.hjson +++ b/sw/device/silicon_creator/lib/cert/uds.hjson @@ -32,17 +32,17 @@ // Hash of the creator_sw_cfg OTP partition (SHA256). otp_creator_sw_cfg_hash: { type: "byte-array", - size: 20, + size: 32, }, // Hash of the owner_sw_cfg OTP partition (SHA256). otp_owner_sw_cfg_hash: { type: "byte-array", - size: 20, + size: 32, }, // Hash of the hw_cfg0 OTP partition (SHA256). otp_hw_cfg0_hash: { type: "byte-array", - size: 20, + size: 32, }, // Debug? debug_flag: {