Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove derived Debug and Serialize from private key types #696

Merged
merged 7 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions signature/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.2.0]((https://github.com/EspressoSystems/jellyfish/compare/jf-signatures-v0.1.1...jf-signatures-v0.2.0)) (2024-10-29)

### Changed

- [#696](https://github.com/EspressoSystems/jellyfish/pull/696): removed derived `Debug` and `Serialize` for private keys.
- Manually implemented `Debug` for private signing keys to not print the actual value.
- Remove derived (de)serialization. Implemented `to/from_bytes()` and the conversions to/from `TaggedBase64`.

## [0.1.1]((https://github.com/EspressoSystems/jellyfish/compare/0.4.5...jf-signatures-v0.1.1)) (2024-07-25)

### Changed
Expand Down
2 changes: 1 addition & 1 deletion signature/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "jf-signature"
version = "0.1.1"
version = "0.2.0"
description = "Implementation of signature schemes, including BLS and Schnorr."
authors = { workspace = true }
edition = { workspace = true }
Expand Down
99 changes: 53 additions & 46 deletions signature/src/bls_over_bls12381.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,66 +89,66 @@ use ark_std::{
};
use blst::{min_sig::*, BLST_ERROR};
use derivative::Derivative;
use tagged_base64::tagged;
use tagged_base64::{tagged, TaggedBase64, Tb64Error};
use zeroize::{Zeroize, Zeroizing};

#[tagged(tag::BLS_SIGNING_KEY)]
#[derive(Clone, Derivative, Zeroize)]
#[zeroize(drop)]
#[derivative(Debug)]
/// A BLS Secret Key (Signing Key).
pub struct BLSSignKey(#[derivative(Debug = "ignore")] SecretKey);

impl Deref for BLSSignKey {
type Target = SecretKey;
fn deref(&self) -> &Self::Target {
&self.0
/// A BLS Secret Key (Signing Key). We intentionally omit the implementation of
/// `serde::Serializable` so that it won't be unnoticably serialized or printed
/// out through outer structs. However, users could manually serialize it into
/// bytes by calling `to_bytes()` or `to_tagged_base64()` and exercise with
/// self-cautions.
pub struct BLSSignKey(SecretKey);

// This content-hiding `Debug` implementation makes sure that the auto-derived
// `Debug` for user's outer struct won't undesirably print out this secret key.
impl core::fmt::Debug for BLSSignKey {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("<BLSSignKey>").finish()
}
}

impl CanonicalSerialize for BLSSignKey {
/// Secret key can only be serialized in compressed mode.
fn serialize_with_mode<W: Write>(
&self,
writer: W,
_compress: Compress,
) -> Result<(), SerializationError> {
// TODO (tessico): should we fail if compress is `Compress::No`?
CanonicalSerialize::serialize_compressed(&self.to_bytes()[..], writer)
impl BLSSignKey {
/// Explicit calls to serialize a `SignKey` into bytes.
pub fn to_bytes(&self) -> [u8; 32] {
self.0.to_bytes()
}

fn serialized_size(&self, _compress: Compress) -> usize {
BLS_SIG_SK_SIZE
/// Deserialize `SignKey` from bytes.
pub fn from_bytes(bytes: &[u8; 32]) -> Result<BLSSignKey, BLST_ERROR> {
SecretKey::from_bytes(bytes).map(|sk| BLSSignKey(sk))
}

/// Explicit calls to serialize a `SignKey` into `TaggedBase64`.
pub fn to_tagged_base64(&self) -> Result<TaggedBase64, Tb64Error> {
TaggedBase64::new(tag::BLS_SIGNING_KEY, &self.to_bytes())
}
}

impl CanonicalDeserialize for BLSSignKey {
fn deserialize_with_mode<R: Read>(
mut reader: R,
compress: Compress,
validate: Validate,
) -> Result<Self, SerializationError> {
let len = <usize as ark_serialize::CanonicalDeserialize>::deserialize_with_mode(
&mut reader,
compress,
validate,
)?;
if len != BLS_SIG_SK_SIZE {
return Err(SerializationError::InvalidData);
}
impl<'a> TryFrom<&'a TaggedBase64> for BLSSignKey {
type Error = Tb64Error;

let mut sk_bytes = [0u8; BLS_SIG_SK_SIZE];
reader.read_exact(&mut sk_bytes)?;
SecretKey::deserialize(&sk_bytes)
.map(Self)
.map_err(|_| SerializationError::InvalidData)
fn try_from(tb: &'a TaggedBase64) -> Result<Self, Self::Error> {
if tb.tag() != tag::BLS_SIGNING_KEY {
return Err(Tb64Error::InvalidTag);
}
SecretKey::from_bytes(tb.as_ref())
.map(|sk| BLSSignKey(sk))
.map_err(|err| Tb64Error::InvalidData)
}
}

impl Valid for BLSSignKey {
fn check(&self) -> Result<(), ark_serialize::SerializationError> {
// TODO no `validate()` method in `blst` on `SecretKey`
Ok(())
impl TryFrom<TaggedBase64> for BLSSignKey {
type Error = Tb64Error;

fn try_from(tb: TaggedBase64) -> Result<Self, Self::Error> {
if tb.tag() != tag::BLS_SIGNING_KEY {
return Err(Tb64Error::InvalidTag);
}
SecretKey::from_bytes(tb.as_ref())
.map(|sk| BLSSignKey(sk))
.map_err(|err| Tb64Error::InvalidData)
}
}

Expand Down Expand Up @@ -374,7 +374,7 @@ impl SignatureScheme for BLSSignatureScheme {
msg: M,
_prng: &mut R,
) -> Result<Self::Signature, SignatureError> {
Ok(BLSSignature(sk.sign(
Ok(BLSSignature(sk.0.sign(
msg.as_ref(),
Self::CS_ID.as_bytes(),
&[],
Expand Down Expand Up @@ -448,7 +448,14 @@ mod test {
let msg = "The quick brown fox jumps over the lazy dog";
let sig = BLSSignatureScheme::sign(&(), &sk, msg, &mut rng).unwrap();

test_canonical_serde_helper(sk);
let bytes = sk.to_bytes();
let de = BLSSignKey::from_bytes(&bytes).unwrap();
assert_eq!(sk, de);

let tagged_blob = sk.to_tagged_base64().unwrap();
let de: BLSSignKey = tagged_blob.try_into().unwrap();
assert_eq!(sk, de);

test_canonical_serde_helper(pk);
test_canonical_serde_helper(sig);
}
Expand Down
146 changes: 112 additions & 34 deletions signature/src/bls_over_bn254.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ use ark_std::{
hash::{Hash, Hasher},
rand::{CryptoRng, RngCore},
string::ToString,
vec,
vec::Vec,
One, UniformRand,
};
Expand All @@ -67,7 +68,7 @@ use sha3::Keccak256;

use crate::SignatureError::{ParameterError, VerificationError};

use tagged_base64::tagged;
use tagged_base64::{tagged, TaggedBase64, Tb64Error};
use zeroize::Zeroize;

/// BLS signature scheme.
Expand Down Expand Up @@ -224,24 +225,66 @@ impl AggregateableSignatureSchemes for BLSOverBN254CurveSignatureScheme {
// =====================================================
// Signing key
// =====================================================
#[tagged(tag::BLS_SIGNING_KEY)]
#[derive(
Clone,
Hash,
Default,
Zeroize,
Eq,
PartialEq,
CanonicalSerialize,
CanonicalDeserialize,
Derivative,
Ord,
PartialOrd,
)]
#[derive(Clone, Hash, Default, Zeroize, Eq, PartialEq)]
#[zeroize(drop)]
#[derivative(Debug)]
/// Signing key for BLS signature.
pub struct SignKey(#[derivative(Debug = "ignore")] pub(crate) ScalarField);
/// Signing key for BLS signature. We intentionally omit the implementation of
/// `serde::Serializable` so that it won't be unnoticably serialized or printed
/// out through outer structs. However, users could manually serialize it into
/// bytes by calling `to_bytes()` or `to_tagged_base64()` and exercise with
/// self-cautions.
pub struct SignKey(pub(crate) ScalarField);

// This content-hiding `Debug` implementation makes sure that the auto-derived
// `Debug` for user's outer struct won't undesirably print out this secret key.
impl core::fmt::Debug for SignKey {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("<BLSSignKey>").finish()
}
}

impl SignKey {
/// Signature Key generation function
pub fn generate<R: CryptoRng + RngCore>(prng: &mut R) -> SignKey {
SignKey(ScalarField::rand(prng))
}

/// Explicit calls to serialize a `SignKey` into bytes.
pub fn to_bytes(&self) -> Vec<u8> {
self.0.into_bigint().to_bytes_le()
}

/// Deserialize `SignKey` from bytes.
pub fn from_bytes(bytes: &[u8]) -> Self {
Self(ScalarField::from_le_bytes_mod_order(bytes))
}

/// Explicit calls to serialize a `SignKey` into `TaggedBase64`.
pub fn to_tagged_base64(&self) -> Result<TaggedBase64, Tb64Error> {
TaggedBase64::new(tag::BLS_SIGNING_KEY, &self.to_bytes())
}
Comment on lines +262 to +264
Copy link
Contributor

@alxiong alxiong Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. should we explicit about tag::BLS_OVER_BN254_SIGN_KEY?
  2. add code doc on SignKey that if one wants to implement serde on outer struct that contains this key (which should be rare), need to exercise self-caution about the implication of that, (since we removed it), and manually implement them by calling one of to_bytes or to_tagged_base64()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly hesitant on the first one. It's making a long tag.
Doc add in 5f85f10

}

impl<'a> TryFrom<&'a TaggedBase64> for SignKey {
type Error = Tb64Error;

fn try_from(tb: &'a TaggedBase64) -> Result<Self, Self::Error> {
if tb.tag() != tag::BLS_SIGNING_KEY {
return Err(Tb64Error::InvalidTag);
}
Ok(SignKey::from_bytes(tb.as_ref()))
}
}

impl TryFrom<TaggedBase64> for SignKey {
type Error = Tb64Error;

fn try_from(tb: TaggedBase64) -> Result<Self, Self::Error> {
if tb.tag() != tag::BLS_SIGNING_KEY {
return Err(Tb64Error::InvalidTag);
}
Ok(SignKey::from_bytes(tb.as_ref()))
}
}

// =====================================================
// Verification key
Expand Down Expand Up @@ -279,7 +322,7 @@ impl VerKey {
// =====================================================

/// Signature secret key pair used to sign messages
#[derive(CanonicalSerialize, CanonicalDeserialize, Clone, Zeroize)]
#[derive(Clone, Debug, Zeroize, Eq, PartialEq)]
#[zeroize(drop)]
pub struct KeyPair {
sk: SignKey,
Expand Down Expand Up @@ -402,6 +445,43 @@ impl KeyPair {
let sigma = hash_value * self.sk.0;
Signature { sigma }
}

/// Explicit calls to serialize a `KeyPair` into bytes.
pub fn to_bytes(&self) -> Vec<u8> {
self.sk.to_bytes()
}

/// Deserialize `KeyPair` from bytes.
pub fn from_bytes(bytes: &[u8]) -> Self {
Self::generate_with_sign_key(ScalarField::from_le_bytes_mod_order(bytes))
}

/// Explicit calls to serialize a `KeyPair` into `TaggedBase64`.
pub fn to_tagged_base64(&self) -> Result<TaggedBase64, Tb64Error> {
TaggedBase64::new(tag::BLS_KEY_PAIR, &self.to_bytes())
}
}

impl<'a> TryFrom<&'a TaggedBase64> for KeyPair {
type Error = Tb64Error;

fn try_from(tb: &'a TaggedBase64) -> Result<Self, Self::Error> {
if tb.tag() != tag::BLS_KEY_PAIR {
return Err(Tb64Error::InvalidTag);
}
Ok(KeyPair::from_bytes(tb.as_ref()))
}
}

impl TryFrom<TaggedBase64> for KeyPair {
type Error = Tb64Error;

fn try_from(tb: TaggedBase64) -> Result<Self, Self::Error> {
if tb.tag() != tag::BLS_KEY_PAIR {
return Err(Tb64Error::InvalidTag);
}
Ok(KeyPair::from_bytes(tb.as_ref()))
}
}

impl From<SignKey> for KeyPair {
Expand All @@ -411,13 +491,6 @@ impl From<SignKey> for KeyPair {
}
}

impl SignKey {
/// Signature Key generation function
pub fn generate<R: CryptoRng + RngCore>(prng: &mut R) -> SignKey {
SignKey(ScalarField::rand(prng))
}
}

impl From<&SignKey> for VerKey {
fn from(sk: &SignKey) -> Self {
VerKey(G2Projective::generator() * sk.0)
Expand Down Expand Up @@ -532,17 +605,22 @@ mod tests {
let msg = vec![87u8];
let sig = keypair.sign(&msg, CS_ID_BLS_BN254);

let mut ser_bytes: Vec<u8> = Vec::new();
keypair.serialize_compressed(&mut ser_bytes).unwrap();
let de: KeyPair = KeyPair::deserialize_compressed(&ser_bytes[..]).unwrap();
assert_eq!(de.ver_key_ref(), keypair.ver_key_ref());
assert_eq!(de.ver_key_ref(), &VerKey::from(&de.sk));
let mut ser_bytes: Vec<u8> = keypair.to_bytes();
let de = KeyPair::from_bytes(&ser_bytes);
assert_eq!(de, keypair);

let mut ser_bytes: Vec<u8> = Vec::new();
sk.serialize_compressed(&mut ser_bytes).unwrap();
let de: SignKey = SignKey::deserialize_compressed(&ser_bytes[..]).unwrap();
let tagged_blob = keypair.to_tagged_base64().unwrap();
let de: KeyPair = tagged_blob.try_into().unwrap();
assert_eq!(de, keypair);

let mut ser_bytes: Vec<u8> = sk.to_bytes();
let de = SignKey::from_bytes(&ser_bytes);
assert_eq!(VerKey::from(&de), VerKey::from(&sk));

let tagged_blob = sk.to_tagged_base64().unwrap();
let de: SignKey = tagged_blob.try_into().unwrap();
assert_eq!(de, sk);

let mut ser_bytes: Vec<u8> = Vec::new();
vk.serialize_compressed(&mut ser_bytes).unwrap();
let de: VerKey = VerKey::deserialize_compressed(&ser_bytes[..]).unwrap();
Expand Down
2 changes: 2 additions & 0 deletions signature/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

/// Tags
pub mod tag {
/// Tag for BLS key pair
pub const BLS_KEY_PAIR: &str = "BLS_KEY_PAIR";
/// Tag for BLS verification key
pub const BLS_VER_KEY: &str = "BLS_VER_KEY";
/// Tag for BLS signing key
Expand Down
10 changes: 1 addition & 9 deletions signature/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,7 @@ pub trait SignatureScheme: Clone + Send + Sync + 'static {
const CS_ID: &'static str;

/// Signing key.
type SigningKey: Debug
+ Clone
+ Send
+ Sync
+ Zeroize
+ for<'a> Deserialize<'a>
+ Serialize
+ PartialEq
+ Eq;
type SigningKey: Debug + Clone + Send + Sync + Zeroize + PartialEq + Eq;

/// Verification key
type VerificationKey: Debug
Expand Down
Loading
Loading