Skip to content

Commit

Permalink
[crypto] Move P-384 point-validity checks into OTBN.
Browse files Browse the repository at this point in the history 
Now that we have the IMEM space for it, it makes sense to push point-validity
checks closer to the use of the point in ECDSA verify and ECDH. This removes a
lot of places where the validity check and point use are decoupled, which could
lead to security holes. It also simplifies the code quite a bit and saves a
chunk of code size.

Signed-off-by: Jade Philipoom <[email protected]>
(cherry picked from commit 810945a)
  • Loading branch information
@jadephilipoom @moidx
jadephilipoom authored and moidx committed Dec 12, 2024
1 parent 334831f commit a08dcef
Show file tree
Hide file tree
Showing 13 changed files with 82 additions and 209 deletions.
16 changes: 0 additions & 16 deletions sw/device/lib/crypto/impl/ecc/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ cc_library(
target_compatible_with = [OPENTITAN_CPU],
deps = [
":p384_common",
":p384_curve_point_valid",
"//sw/device/lib/base:hardened",
"//sw/device/lib/base:hardened_memory",
"//sw/device/lib/crypto/drivers:otbn",
Expand Down Expand Up @@ -81,7 +80,6 @@ cc_library(
target_compatible_with = [OPENTITAN_CPU],
deps = [
":p384_common",
":p384_curve_point_valid",
"//sw/device/lib/base:hardened",
"//sw/device/lib/base:hardened_memory",
"//sw/device/lib/crypto/drivers:otbn",
Expand All @@ -98,17 +96,3 @@ cc_library(
"//sw/device/lib/crypto/impl:status",
],
)

cc_library(
name = "p384_curve_point_valid",
srcs = ["p384_curve_point_valid.c"],
hdrs = ["p384_curve_point_valid.h"],
target_compatible_with = [OPENTITAN_CPU],
deps = [
":p384_common",
"//sw/device/lib/base:hardened",
"//sw/device/lib/base:hardened_memory",
"//sw/device/lib/crypto/drivers:otbn",
"//sw/otbn/crypto:p384_curve_point_valid",
],
)
22 changes: 12 additions & 10 deletions sw/device/lib/crypto/impl/ecc/ecdh_p384.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include "sw/device/lib/base/hardened.h"
#include "sw/device/lib/base/hardened_memory.h"
#include "sw/device/lib/crypto/drivers/otbn.h"
#include "sw/device/lib/crypto/impl/ecc/p384_curve_point_valid.h"

#include "hw/top_earlgrey/sw/autogen/top_earlgrey.h"

Expand All @@ -21,14 +20,16 @@ OTBN_DECLARE_SYMBOL_ADDR(p384_ecdh, y); // The public key y-coordinate.
OTBN_DECLARE_SYMBOL_ADDR(p384_ecdh,
d0); // The private key scalar d (share 0).
OTBN_DECLARE_SYMBOL_ADDR(p384_ecdh,
d1); // The private key scalar d (share 1).
d1); // The private key scalar d (share 1).
OTBN_DECLARE_SYMBOL_ADDR(p384_ecdh, ok); // Status code.

static const otbn_app_t kOtbnAppEcdh = OTBN_APP_T_INIT(p384_ecdh);
static const otbn_addr_t kOtbnVarEcdhMode = OTBN_ADDR_T_INIT(p384_ecdh, mode);
static const otbn_addr_t kOtbnVarEcdhX = OTBN_ADDR_T_INIT(p384_ecdh, x);
static const otbn_addr_t kOtbnVarEcdhY = OTBN_ADDR_T_INIT(p384_ecdh, y);
static const otbn_addr_t kOtbnVarEcdhD0 = OTBN_ADDR_T_INIT(p384_ecdh, d0);
static const otbn_addr_t kOtbnVarEcdhD1 = OTBN_ADDR_T_INIT(p384_ecdh, d1);
static const otbn_addr_t kOtbnVarEcdhOk = OTBN_ADDR_T_INIT(p384_ecdh, ok);

enum {
/*
Expand Down Expand Up @@ -88,10 +89,6 @@ status_t ecdh_p384_keypair_finalize(p384_masked_scalar_t *private_key,

status_t ecdh_p384_shared_key_start(const p384_masked_scalar_t *private_key,
const p384_point_t *public_key) {
// Check if public key is valid
HARDENED_TRY(p384_curve_point_validate_start(public_key));
HARDENED_TRY(p384_curve_point_validate_finalize());

// Load the ECDH/P-384 app. Fails if OTBN is non-idle.
HARDENED_TRY(otbn_load_app(kOtbnAppEcdh));

Expand All @@ -117,6 +114,15 @@ status_t ecdh_p384_shared_key_finalize(ecdh_p384_shared_key_t *shared_key) {
// Spin here waiting for OTBN to complete.
HARDENED_TRY(otbn_busy_wait_for_done());

// Read the status code out of DMEM (false if basic checks on the validity of
// the public key failed).
uint32_t ok;
HARDENED_TRY(otbn_dmem_read(1, kOtbnVarEcdhOk, &ok));
if (launder32(ok) != kHardenedBoolTrue) {
return OTCRYPTO_BAD_ARGS;
}
HARDENED_CHECK_EQ(ok, kHardenedBoolTrue);

// Read the shares of the key from OTBN dmem (at vars x and y).
HARDENED_TRY(
otbn_dmem_read(kP384CoordWords, kOtbnVarEcdhX, shared_key->share0));
Expand Down Expand Up @@ -156,10 +162,6 @@ status_t ecdh_p384_sideload_keypair_finalize(p384_point_t *public_key) {
}

status_t ecdh_p384_sideload_shared_key_start(const p384_point_t *public_key) {
// Check if public key is valid
HARDENED_TRY(p384_curve_point_validate_start(public_key));
HARDENED_TRY(p384_curve_point_validate_finalize());

// Load the ECDH/P-384 app. Fails if OTBN is non-idle.
HARDENED_TRY(otbn_load_app(kOtbnAppEcdh));

Expand Down
23 changes: 15 additions & 8 deletions sw/device/lib/crypto/impl/ecc/ecdsa_p384_verify.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include "sw/device/lib/base/hardened.h"
#include "sw/device/lib/base/hardened_memory.h"
#include "sw/device/lib/crypto/drivers/otbn.h"
#include "sw/device/lib/crypto/impl/ecc/p384_curve_point_valid.h"

#include "hw/top_earlgrey/sw/autogen/top_earlgrey.h"

Expand All @@ -20,9 +19,10 @@ OTBN_DECLARE_SYMBOL_ADDR(p384_ecdsa_verify, y); // Public key y-coordinate.
OTBN_DECLARE_SYMBOL_ADDR(p384_ecdsa_verify,
x_r); // result of verify (x1 coordinate)
OTBN_DECLARE_SYMBOL_ADDR(p384_ecdsa_verify,
msg); // hash message to sign/verify
OTBN_DECLARE_SYMBOL_ADDR(p384_ecdsa_verify, r); // r part of signature
OTBN_DECLARE_SYMBOL_ADDR(p384_ecdsa_verify, s); // s part of signature
msg); // hash message to sign/verify
OTBN_DECLARE_SYMBOL_ADDR(p384_ecdsa_verify, r); // r part of signature
OTBN_DECLARE_SYMBOL_ADDR(p384_ecdsa_verify, s); // s part of signature
OTBN_DECLARE_SYMBOL_ADDR(p384_ecdsa_verify, ok); // Status code.

static const otbn_app_t kOtbnAppEcdsaVerify =
OTBN_APP_T_INIT(p384_ecdsa_verify);
Expand All @@ -38,14 +38,12 @@ static const otbn_addr_t kOtbnVarEcdsaS =
OTBN_ADDR_T_INIT(p384_ecdsa_verify, s);
static const otbn_addr_t kOtbnVarEcdsaRnd =
OTBN_ADDR_T_INIT(p384_ecdsa_verify, x_r);
static const otbn_addr_t kOtbnVarEcdsaOk =
OTBN_ADDR_T_INIT(p384_ecdsa_verify, ok);

status_t ecdsa_p384_verify_start(const ecdsa_p384_signature_t *signature,
const uint32_t digest[kP384ScalarWords],
const p384_point_t *public_key) {
// Check if public key is valid
HARDENED_TRY(p384_curve_point_validate_start(public_key));
HARDENED_TRY(p384_curve_point_validate_finalize());

// Load the ECDSA/P-384 app
HARDENED_TRY(otbn_load_app(kOtbnAppEcdsaVerify));

Expand Down Expand Up @@ -73,6 +71,15 @@ status_t ecdsa_p384_verify_finalize(const ecdsa_p384_signature_t *signature,
// Spin here waiting for OTBN to complete.
HARDENED_TRY(otbn_busy_wait_for_done());

// Read the status code out of DMEM (false if basic checks on the validity of
// the signature and public key failed).
uint32_t ok;
HARDENED_TRY(otbn_dmem_read(1, kOtbnVarEcdsaOk, &ok));
if (launder32(ok) != kHardenedBoolTrue) {
return OTCRYPTO_BAD_ARGS;
}
HARDENED_CHECK_EQ(ok, kHardenedBoolTrue);

// Read x_r (recovered R) out of OTBN dmem.
uint32_t x_r[kP384ScalarWords];
HARDENED_TRY(otbn_dmem_read(kP384ScalarWords, kOtbnVarEcdsaRnd, x_r));
Expand Down
1 change: 0 additions & 1 deletion sw/device/lib/crypto/impl/ecc/ecdsa_p384_verify.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "sw/device/lib/base/hardened.h"
#include "sw/device/lib/crypto/drivers/otbn.h"
#include "sw/device/lib/crypto/impl/ecc/p384_common.h"
#include "sw/device/lib/crypto/impl/ecc/p384_curve_point_valid.h"

#ifdef __cplusplus
extern "C" {
Expand Down
51 changes: 0 additions & 51 deletions sw/device/lib/crypto/impl/ecc/p384_curve_point_valid.c
View file

This file was deleted.

45 changes: 0 additions & 45 deletions sw/device/lib/crypto/impl/ecc/p384_curve_point_valid.h
View file

This file was deleted.

13 changes: 2 additions & 11 deletions sw/otbn/crypto/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,17 +364,6 @@ otbn_binary(
],
)

otbn_binary(
name = "p384_curve_point_valid",
srcs = [
"p384_curve_point_valid.s",
],
deps = [
":p384_base",
":p384_isoncurve",
],
)

otbn_binary(
name = "p384_ecdh",
srcs = [
Expand All @@ -386,6 +375,7 @@ otbn_binary(
":p384_base",
":p384_base_mult",
":p384_internal_mult",
":p384_isoncurve",
":p384_keygen",
":p384_keygen_from_seed",
":p384_scalar_mult",
Expand Down Expand Up @@ -433,6 +423,7 @@ otbn_binary(
":p384_base",
":p384_base_mult",
":p384_internal_mult",
":p384_isoncurve",
":p384_modinv",
":p384_verify",
],
Expand Down
46 changes: 0 additions & 46 deletions sw/otbn/crypto/p384_curve_point_valid.s
View file

This file was deleted.

Loading

0 comments on commit a08dcef

Please sign in to comment.