Skip to content

Commit

Permalink
Change to better distinguish signature schemes
Browse files Browse the repository at this point in the history
It is currently possible to call `APK::verify` or `PublicKey::verify` on
the same signature type, leading to situations where even if a user is
technically *allowed* to call the function, verification is in principle
impossible, since the signature schemes used to produce a signature are
different. Furthermore, the `sign_vulnerable` function is arguably
misnamed, since it is only actually vulnerable to a rogue key attack
*when used in a multi-signature context*.

In the interest of fixing this, this commit introduces a couple of changes
to the API of the crate, designed to better distinguish and clarify the
usage of the two different signature schemes it provides.

First and foremost, a new signature type - `MultisigSignature` is added,
representing a signature produced using the multi-signature scheme.
Conversely, the already existing `Signature` type is taken to mean the a
signature produced using the single-key scheme.
The functions under `SecretKey`, `sign_vulnerable` and `sign`, are
respectively renamed into `sign` and `sign_multisig`, and the latter is
changed to return the new `MultisigSignature` type.
The "aggregated public key" type is renamed from `APK` into
`MultisigPublicKey`, to better denote its use and to be in line with the
kind signature it can now verify using the `verify` function - which is
changed to take a `MultisigSignature`.

On a more subtle note, the `From<&PublicKey>` and `From<&SecretKey>`
implementations for `MultisigPublicKey`, formerly `APK` are removed and
the API of `MultisigPublicKey::aggregate` is changed to *not* take
`&self`, and only take a slice of `PublicKey`. This is designed to
promote the intended usage of the struct - aggregating a collection of
public keys, as opposed to just "aggregating" one.

Resolves: #18
  • Loading branch information
Eduardo Leegwater Simões committed Aug 1, 2024
1 parent fcecfad commit d428201
Show file tree
Hide file tree
Showing 12 changed files with 303 additions and 275 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Add `MultisigSignature` struct [#18]
- Add `Error::NoKeysProvided` variant [#18]

### Changed

- Rename `SecretKey::sign` to `sign_multisig` and change return to `MultisigSignature` [#18]
- Rename `SecretKey::sign_vulnerable` to `sign` [#18]
- Rename `APK` to `MultisigPublicKey` [#18]
- Change `APK::verify` to take a `MultisigSignature` [#18]

### Removed

- Remove `impl From<&PublicKey> for APK` [#18]
- Remove `impl From<&SecretKey> for APK` [#18]

## [0.3.1] - 2024-06-27

### Changed
Expand Down Expand Up @@ -48,6 +65,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add initial commit, this package continues the development of [dusk-bls12_381-sign](https://github.com/dusk-network/bls12_381-sign/) at version `0.6.0` under the new name: `bls12_381-bls` and without the go related code.

<!-- ISSUES -->
[#18]: https://github.com/dusk-network/bls12_381-bls/issues/18
[#8]: https://github.com/dusk-network/bls12_381-bls/issues/8
[#7]: https://github.com/dusk-network/bls12_381-bls/issues/7
[#5]: https://github.com/dusk-network/bls12_381-bls/issues/5
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ bytecheck = { version = "0.6", optional = true, default-features = false }
rayon = { version = "1.8", optional = true }

[dev-dependencies]
rand = { version = "0.8", default-features = false, features = ["std_rng"] }
rand = "0.8"

[features]
rkyv-impl = [
Expand Down
48 changes: 15 additions & 33 deletions benches/signature_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,79 +9,61 @@
extern crate test;

mod benches {
use bls12_381_bls::{PublicKey, SecretKey, APK};
use bls12_381_bls::{MultisigPublicKey, PublicKey, SecretKey};
use dusk_bytes::Serializable;
use rand_core::{OsRng, RngCore};
use rand::rngs::OsRng;
use rand::RngCore;
use test::Bencher;

#[bench]
fn bench_sign_vulnerable(b: &mut Bencher) {
fn bench_sign(b: &mut Bencher) {
let sk = SecretKey::random(&mut OsRng);
let msg = random_message();
b.iter(|| sk.sign_vulnerable(&msg));
b.iter(|| sk.sign(&msg));
}

#[bench]
fn bench_sign(b: &mut Bencher) {
fn bench_multisig_sign(b: &mut Bencher) {
let sk = SecretKey::random(&mut OsRng);
let pk = PublicKey::from(&sk);
let msg = random_message();
b.iter(|| sk.sign(&pk, &msg));
b.iter(|| sk.sign_multisig(&pk, &msg));
}

#[bench]
fn bench_verify(b: &mut Bencher) {
let sk = SecretKey::random(&mut OsRng);
let pk = PublicKey::from(&sk);
let msg = random_message();
let sig = sk.sign_vulnerable(&msg);
let sig = sk.sign(&msg);
b.iter(|| pk.verify(&sig, &msg));
}

#[bench]
fn bench_aggregate_sig(b: &mut Bencher) {
fn bench_multisig_aggregate_sig(b: &mut Bencher) {
let sk = SecretKey::random(&mut OsRng);
let pk = PublicKey::from(&sk);
let msg = random_message();
let sig = sk.sign_vulnerable(&msg);
let sig = sk.sign_multisig(&pk, &msg);
b.iter(|| sig.aggregate(&[sig]));
}

#[bench]
fn bench_aggregate_pk(b: &mut Bencher) {
let sk = SecretKey::random(&mut OsRng);
let pk = PublicKey::from(&sk);
let mut apk = APK::from(&pk);
b.iter(|| apk.aggregate(&[pk]));
}

#[bench]
fn bench_aggregate_pk_64_bulk(b: &mut Bencher) {
fn bench_multisig_aggregate_pk(b: &mut Bencher) {
let sk = SecretKey::random(&mut OsRng);
let pk = PublicKey::from(&sk);
let mut apk = APK::from(&pk);
let mut pks = vec![];
for _ in 0..64 {
let sk = SecretKey::random(&mut OsRng);
let pk = PublicKey::from(&sk);
pks.push(pk)
}
let pks = &pks[..];
b.iter(|| apk.aggregate(pks));
b.iter(|| MultisigPublicKey::aggregate(&[pk]));
}

#[bench]
fn bench_aggregate_pk_64_each(b: &mut Bencher) {
let sk = SecretKey::random(&mut OsRng);
let pk = PublicKey::from(&sk);
let mut apk = APK::from(&pk);
fn bench_multisig_aggregate_pk_64_bulk(b: &mut Bencher) {
let mut pks = vec![];
for _ in 0..64 {
let sk = SecretKey::random(&mut OsRng);
let pk = PublicKey::from(&sk);
pks.push(pk)
}
let pks = &pks[..];
b.iter(|| pks.iter().for_each(|&p| apk.aggregate(&[p])));
b.iter(|| MultisigPublicKey::aggregate(&pks[..]));
}

fn random_message() -> [u8; 100] {
Expand Down
5 changes: 5 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ pub enum Error {
InvalidSignature,
/// Invalid Point
InvalidPoint,
/// Tried to aggregate an empty list of public keys
NoKeysProvided,
}

impl From<DuskBytesError> for Error {
Expand All @@ -35,6 +37,9 @@ impl fmt::Display for Error {
Self::InvalidPoint => {
write!(f, "Invalid Point")
}
Self::NoKeysProvided => {
write!(f, "No keys provided")
}
}
}
}
1 change: 0 additions & 1 deletion src/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,5 @@
//
// Copyright (c) DUSK NETWORK. All rights reserved.

pub mod apk;
pub mod public;
pub mod secret;
127 changes: 0 additions & 127 deletions src/keys/apk.rs

This file was deleted.

Loading

0 comments on commit d428201

Please sign in to comment.