Skip to content

Commit

Permalink
Review SecretBox usage
Browse files Browse the repository at this point in the history
  • Loading branch information
fjarri committed Jan 8, 2022
1 parent 3860a78 commit 4d11efc
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 36 deletions.
13 changes: 7 additions & 6 deletions umbral-pre/src/capsule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,18 @@ impl Capsule {
) -> (Capsule, SecretBox<KeySeed>) {
let g = CurvePoint::generator();

let priv_r = NonZeroCurveScalar::random(rng);
let pub_r = &g * &priv_r;
let priv_r = SecretBox::new(NonZeroCurveScalar::random(rng));
let pub_r = &g * priv_r.as_secret();

let priv_u = NonZeroCurveScalar::random(rng);
let pub_u = &g * &priv_u;
let priv_u = SecretBox::new(NonZeroCurveScalar::random(rng));
let pub_u = &g * priv_u.as_secret();

let h = hash_capsule_points(&pub_r, &pub_u);

let s = &priv_u + &(&priv_r * &h);
let s = priv_u.as_secret() + &(priv_r.as_secret() * &h);

let shared_key = SecretBox::new(&delegating_pk.to_point() * &(&priv_r + &priv_u));
let shared_key =
SecretBox::new(&delegating_pk.to_point() * &(priv_r.as_secret() + priv_u.as_secret()));

let capsule = Self::new(pub_r, pub_u, s);

Expand Down
11 changes: 6 additions & 5 deletions umbral-pre/src/capsule_frag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::curve::{CurvePoint, CurveScalar, NonZeroCurveScalar};
use crate::hashing_ds::{hash_to_cfrag_verification, kfrag_signature_message};
use crate::key_frag::{KeyFrag, KeyFragID};
use crate::keys::{PublicKey, Signature};
use crate::secret_box::SecretBox;
use crate::traits::{
fmt_public, ConstructionError, DeserializableFromArray, DeserializationError, HasTypeName,
RepresentableAsArray, SerializableToArray,
Expand Down Expand Up @@ -84,7 +85,7 @@ impl CapsuleFragProof {
let params = capsule.params;

let rk = kfrag.key;
let t = NonZeroCurveScalar::random(rng);
let t = SecretBox::new(NonZeroCurveScalar::random(rng));

// Here are the formulaic constituents shared with `CapsuleFrag::verify()`.

Expand All @@ -97,15 +98,15 @@ impl CapsuleFragProof {
let u = params.u;
let u1 = kfrag.proof.commitment;

let e2 = &e * &t;
let v2 = &v * &t;
let u2 = &u * &t;
let e2 = &e * t.as_secret();
let v2 = &v * t.as_secret();
let u2 = &u * t.as_secret();

let h = hash_to_cfrag_verification(&[e, *e1, e2, v, *v1, v2, u, u1, u2]);

////////

let z3 = &(&rk * &h) + &t;
let z3 = &(&rk * &h) + t.as_secret();

Self {
point_e2: e2,
Expand Down
8 changes: 5 additions & 3 deletions umbral-pre/src/key_frag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::curve::{CurvePoint, CurveScalar, NonZeroCurveScalar};
use crate::hashing_ds::{hash_to_polynomial_arg, hash_to_shared_secret, kfrag_signature_message};
use crate::keys::{PublicKey, SecretKey, Signature, Signer};
use crate::params::Parameters;
use crate::secret_box::SecretBox;
use crate::traits::{
fmt_public, ConstructionError, DeserializableFromArray, DeserializationError, HasTypeName,
RepresentableAsArray, SerializableToArray,
Expand Down Expand Up @@ -460,10 +461,10 @@ impl KeyFragBase {

// The precursor point is used as an ephemeral public key in a DH key exchange,
// and the resulting shared secret 'dh_point' is used to derive other secret values
let private_precursor = NonZeroCurveScalar::random(rng);
let precursor = &g * &private_precursor;
let private_precursor = SecretBox::new(NonZeroCurveScalar::random(rng));
let precursor = &g * private_precursor.as_secret();

let dh_point = &receiving_pk_point * &private_precursor;
let dh_point = &receiving_pk_point * private_precursor.as_secret();

// Secret value 'd' allows to make Umbral non-interactive
let d = hash_to_shared_secret(&precursor, &receiving_pk_point, &dh_point);
Expand All @@ -474,6 +475,7 @@ impl KeyFragBase {
let mut coefficients = Vec::<NonZeroCurveScalar>::with_capacity(threshold);
coefficients.push(coefficient0);
for _i in 1..threshold {
// Assuming these are not secret, otherwise they should be put in SecretBox.
coefficients.push(NonZeroCurveScalar::random(rng));
}

Expand Down
36 changes: 14 additions & 22 deletions umbral-pre/src/keys.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use alloc::boxed::Box;
use alloc::vec::Vec;
use core::cmp::Ordering;
use core::fmt;
Expand All @@ -20,7 +19,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer};
use crate::curve::{CurvePoint, CurveScalar, CurveType, NonZeroCurveScalar};
use crate::dem::kdf;
use crate::hashing::{BackendDigest, Hash, ScalarDigest};
use crate::secret_box::{CanBeZeroizedOnDrop, SecretBox};
use crate::secret_box::SecretBox;
use crate::traits::{
fmt_public, fmt_secret, ConstructionError, DeserializableFromArray, HasTypeName,
RepresentableAsArray, SerializableToArray, SerializableToSecretArray, SizeMismatchError,
Expand Down Expand Up @@ -95,19 +94,16 @@ impl fmt::Display for Signature {
}
}

impl CanBeZeroizedOnDrop for BackendSecretKey<CurveType> {
fn ensure_zeroized_on_drop(&mut self) {
// BackendSecretKey is zeroized on drop, nothing to do
}
}

// TODO (#89): derive `ZeroizeOnDrop` for `SecretKey` when it's available.
// For now we know that `BackendSecretKey` is zeroized on drop (as of elliptic-curve=0.11),
// but cannot check that at compile-time.
/// A secret key.
#[derive(Clone)]
pub struct SecretKey(SecretBox<BackendSecretKey<CurveType>>);
pub struct SecretKey(BackendSecretKey<CurveType>);

impl SecretKey {
fn new(sk: BackendSecretKey<CurveType>) -> Self {
Self(SecretBox::new(sk))
Self(sk)
}

/// Creates a secret key using the given RNG.
Expand All @@ -124,20 +120,17 @@ impl SecretKey {

/// Returns a public key corresponding to this secret key.
pub fn public_key(&self) -> PublicKey {
PublicKey(self.0.as_secret().public_key())
PublicKey(self.0.public_key())
}

fn from_nonzero_scalar(scalar: SecretBox<NonZeroCurveScalar>) -> Self {
// TODO: SecretBox it
let backend_sk =
BackendSecretKey::<CurveType>::from(scalar.as_secret().as_backend_scalar());

Self::new(backend_sk)
let backend_scalar_ref = scalar.as_secret().as_backend_scalar();
Self::new(BackendSecretKey::<CurveType>::from(backend_scalar_ref))
}

/// Returns a reference to the underlying scalar of the secret key.
pub(crate) fn to_secret_scalar(&self) -> SecretBox<NonZeroCurveScalar> {
let backend_scalar = SecretBox::new(self.0.as_secret().to_nonzero_scalar());
let backend_scalar = SecretBox::new(self.0.to_nonzero_scalar());
SecretBox::new(NonZeroCurveScalar::from_backend_scalar(
*backend_scalar.as_secret(),
))
Expand All @@ -150,7 +143,7 @@ impl RepresentableAsArray for SecretKey {

impl SerializableToSecretArray for SecretKey {
fn to_secret_array(&self) -> SecretBox<GenericArray<u8, Self::Size>> {
SecretBox::new(self.0.as_secret().to_be_bytes())
SecretBox::new(self.0.to_be_bytes())
}
}

Expand Down Expand Up @@ -193,10 +186,9 @@ impl Signer {
pub fn sign_with_rng(&self, rng: &mut (impl CryptoRng + RngCore), message: &[u8]) -> Signature {
let digest = digest_for_signing(message);
let secret_key = self.0.clone();
// We could use SecretBox here, but SigningKey does not implement Clone.
// Box is good enough, seeing as how `signing_key` does not leave this method.
let signing_key = Box::new(SigningKey::<CurveType>::from(secret_key.0.as_secret()));
Signature(signing_key.as_ref().sign_digest_with_rng(rng, digest))
// `k256::SigningKey` is zeroized on `Drop` as of `k256=0.10`.
let signing_key = SigningKey::<CurveType>::from(secret_key.0);
Signature(signing_key.sign_digest_with_rng(rng, digest))
}

/// Signs the given message using the default RNG.
Expand Down

0 comments on commit 4d11efc

Please sign in to comment.