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

feat(common) Allow adding signed data #141

Merged
merged 8 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
feat: Added data op is always signed with key from chain
When solely relying on an external signature, this leaves prism vulnerable to replay attacks. As a result, ops to add data are always signed with a valid chain key and can optionally have an additional signature for the data to be added.
  • Loading branch information
jns-ps committed Oct 22, 2024
commit abfc65ee3e396fd6e2be8f88f8f861866789676f
40 changes: 26 additions & 14 deletions crates/common/src/hashchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::{
use crate::{
keys::VerifyingKey,
operation::{
AddSignedDataArgs, CreateAccountArgs, KeyOperationArgs, Operation, RegisterServiceArgs,
AddDataArgs, CreateAccountArgs, KeyOperationArgs, Operation, RegisterServiceArgs,
ServiceChallenge, ServiceChallengeInput,
},
tree::{Digest, Hasher},
Expand Down Expand Up @@ -131,7 +131,7 @@ impl Hashchain {
Operation::RevokeKey(args) => {
valid_keys.remove(&args.value);
}
Operation::AddSignedData(_) => {}
Operation::AddData(_) => {}
}
}

Expand Down Expand Up @@ -163,22 +163,29 @@ impl Hashchain {
)?;
}
Operation::AddKey(args) | Operation::RevokeKey(args) => {
let message = bincode::serialize(&last_entry.operation.without_signature())?;

self.verify_signature_at_key_idx(
&message,
&last_entry.operation,
&args.signature.signature,
args.signature.key_idx,
&valid_keys,
)?;
}
Operation::AddSignedData(args) => {
Operation::AddData(args) => {
self.verify_signature_at_key_idx(
&args.value,
&args.signature.signature,
args.signature.key_idx,
&last_entry.operation,
&args.op_signature.signature,
args.op_signature.key_idx,
&valid_keys,
)?;

let Some(value_signature) = &args.value_signature else {
return Ok(());
};

// If data to be added is signed, also validate its signature
value_signature
.verifying_key
.verify_signature(&args.value, &value_signature.signature)?;
}
}

Expand All @@ -203,7 +210,7 @@ impl Hashchain {

for entry in self.entries.clone() {
match &entry.operation {
Operation::RegisterService(_) | Operation::AddSignedData(_) => {}
Operation::RegisterService(_) | Operation::AddData(_) => {}
Operation::CreateAccount(args) => {
valid_keys.insert(args.value.clone());
}
Expand Down Expand Up @@ -232,7 +239,7 @@ impl Hashchain {

fn verify_signature_at_key_idx(
&self,
value: &[u8],
operation: &Operation,
signature: &[u8],
idx: usize,
valid_keys: &HashSet<VerifyingKey>,
Expand All @@ -245,7 +252,9 @@ impl Hashchain {
valid_keys
);
}
verifying_key.verify_signature(value, signature)

let message = bincode::serialize(&operation.without_signature())?;
verifying_key.verify_signature(&message, signature)
}

pub fn iter(&self) -> std::slice::Iter<'_, HashchainEntry> {
Expand Down Expand Up @@ -281,7 +290,7 @@ impl Hashchain {
self.push(operation)
}

/// Verifies the structure and signature of a new operation without checking if the key is revoked.
/// Verifies the structure and signature of a new operation
fn validate_new_operation(&self, operation: &Operation) -> Result<()> {
match operation {
Operation::RegisterService(_) => {
Expand All @@ -292,7 +301,10 @@ impl Hashchain {
}
Operation::AddKey(KeyOperationArgs { signature, .. })
| Operation::RevokeKey(KeyOperationArgs { signature, .. })
| Operation::AddSignedData(AddSignedDataArgs { signature, .. }) => {
| Operation::AddData(AddDataArgs {
op_signature: signature,
..
}) => {
let signing_key = self.get_key_at_index(signature.key_idx)?;

if self.is_key_revoked(signing_key.clone()) {
Expand Down
106 changes: 71 additions & 35 deletions crates/common/src/operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub enum Operation {
/// Adds a key to an existing account.
AddKey(KeyOperationArgs),
/// Adds arbitrary signed data to an existing account.
AddSignedData(AddSignedDataArgs),
AddData(AddDataArgs),
/// Revokes a key from an existing account.
RevokeKey(KeyOperationArgs),
/// Registers a new service with the given id.
Expand All @@ -26,22 +26,31 @@ pub enum Operation {
#[derive(Clone, Serialize, Deserialize, Default, Debug, PartialEq)]
/// Represents a signature bundle, which includes the index of the key
/// in the user's hashchain and the associated signature.
pub struct SignatureBundle {
pub struct HashchainSignatureBundle {
/// Index of the key in the hashchain
pub key_idx: usize,
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
/// The actual signature
pub signature: Vec<u8>,
}

impl SignatureBundle {
impl HashchainSignatureBundle {
pub fn empty_with_idx(idx: usize) -> Self {
SignatureBundle {
HashchainSignatureBundle {
key_idx: idx,
signature: vec![],
}
}
}

#[derive(Clone, Serialize, Deserialize, Debug, PartialEq)]
/// Represents a signature including its.
pub struct SignatureBundle {
/// The key that can be used to verify the signature
pub verifying_key: VerifyingKey,
/// The actual signature
pub signature: Vec<u8>,
}

#[derive(Clone, Serialize, Deserialize, Debug, PartialEq)]
/// Input required to complete a challenge for account creation.
pub enum ServiceChallengeInput {
Expand Down Expand Up @@ -84,14 +93,16 @@ impl From<SigningKey> for ServiceChallenge {
}

#[derive(Clone, Serialize, Deserialize, Debug, PartialEq)]
/// Structure for adding signed data.
pub struct AddSignedDataArgs {
/// Structure for adding data.
pub struct AddDataArgs {
/// Account ID
pub id: String,
/// Signed data to be added
/// Data to be added
pub value: Vec<u8>,
/// Optional external signature used to sign the data to be added
pub value_signature: Option<SignatureBundle>,
/// Signature to authorize the action
pub signature: SignatureBundle,
pub op_signature: HashchainSignatureBundle,
}

#[derive(Clone, Serialize, Deserialize, Debug, PartialEq)]
Expand All @@ -102,7 +113,7 @@ pub struct KeyOperationArgs {
/// Public key being added or revoked
pub value: VerifyingKey,
/// Signature to authorize the action
pub signature: SignatureBundle,
pub signature: HashchainSignatureBundle,
}

impl Operation {
Expand Down Expand Up @@ -148,11 +159,11 @@ impl Operation {
let op_to_sign = Operation::AddKey(KeyOperationArgs {
id: id.clone(),
value: value.clone(),
signature: SignatureBundle::empty_with_idx(key_idx),
signature: HashchainSignatureBundle::empty_with_idx(key_idx),
});

let message = bincode::serialize(&op_to_sign)?;
let signature = SignatureBundle {
let signature = HashchainSignatureBundle {
key_idx,
signature: signing_key.sign(&message).to_vec(),
};
Expand All @@ -173,11 +184,11 @@ impl Operation {
let op_to_sign = Operation::RevokeKey(KeyOperationArgs {
id: id.clone(),
value: value.clone(),
signature: SignatureBundle::empty_with_idx(key_idx),
signature: HashchainSignatureBundle::empty_with_idx(key_idx),
});

let message = bincode::serialize(&op_to_sign)?;
let signature = SignatureBundle {
let signature = HashchainSignatureBundle {
key_idx,
signature: signing_key.sign(&message).to_vec(),
};
Expand All @@ -192,21 +203,36 @@ impl Operation {
pub fn new_add_signed_data(
id: String,
value: Vec<u8>,
signature: Vec<u8>,
value_signature: Option<SignatureBundle>,
signing_key: &SigningKey,
key_idx: usize,
) -> Result<Self> {
Ok(Operation::AddSignedData(AddSignedDataArgs {
let op_to_sign = Operation::AddData(AddDataArgs {
id: id.clone(),
value: value.clone(),
value_signature: value_signature.clone(),
op_signature: HashchainSignatureBundle::empty_with_idx(key_idx),
});

let message = { bincode::serialize(&op_to_sign)? };
let op_signature = HashchainSignatureBundle {
key_idx,
signature: signing_key.sign(&message).to_vec(),
};

Ok(Operation::AddData(AddDataArgs {
id,
value,
signature: SignatureBundle { key_idx, signature },
value_signature,
op_signature,
}))
}

pub fn id(&self) -> String {
match self {
Operation::CreateAccount(args) => args.id.clone(),
Operation::AddKey(args) | Operation::RevokeKey(args) => args.id.clone(),
Operation::AddSignedData(args) => args.id.clone(),
Operation::AddData(args) => args.id.clone(),
Operation::RegisterService(args) => args.id.clone(),
}
}
Expand All @@ -215,16 +241,7 @@ impl Operation {
match self {
Operation::RevokeKey(args) | Operation::AddKey(args) => Some(&args.value),
Operation::CreateAccount(args) => Some(&args.value),
Operation::RegisterService(_) | Operation::AddSignedData(_) => None,
}
}

pub fn get_signature_bundle(&self) -> Option<SignatureBundle> {
match self {
Operation::AddKey(args) => Some(args.signature.clone()),
Operation::RevokeKey(args) => Some(args.signature.clone()),
Operation::AddSignedData(args) => Some(args.signature.clone()),
Operation::RegisterService(_) | Operation::CreateAccount(_) => None,
Operation::RegisterService(_) | Operation::AddData(_) => None,
}
}

Expand Down Expand Up @@ -260,24 +277,25 @@ impl Operation {
Operation::AddKey(args) => Operation::AddKey(KeyOperationArgs {
id: args.id.clone(),
value: args.value.clone(),
signature: SignatureBundle {
signature: HashchainSignatureBundle {
key_idx: args.signature.key_idx,
signature: Vec::new(),
},
}),
Operation::RevokeKey(args) => Operation::RevokeKey(KeyOperationArgs {
id: args.id.clone(),
value: args.value.clone(),
signature: SignatureBundle {
signature: HashchainSignatureBundle {
key_idx: args.signature.key_idx,
signature: Vec::new(),
},
}),
Operation::AddSignedData(args) => Operation::AddSignedData(AddSignedDataArgs {
Operation::AddData(args) => Operation::AddData(AddDataArgs {
id: args.id.clone(),
value: args.value.clone(),
signature: SignatureBundle {
key_idx: args.signature.key_idx,
value_signature: args.value_signature.clone(),
op_signature: HashchainSignatureBundle {
key_idx: args.op_signature.key_idx,
signature: Vec::new(),
},
}),
Expand Down Expand Up @@ -308,8 +326,22 @@ impl Operation {
.context("User signature failed")?;
pubkey.verify_signature(&message, &args.signature.signature)
}
Operation::AddSignedData(args) => {
pubkey.verify_signature(&args.value, &args.signature.signature)
Operation::AddData(args) => {
let message = bincode::serialize(&self.without_signature())
.context("Serializing operation failed")?;
pubkey
.verify_signature(&message, &args.op_signature.signature)
.context("Verifying operation signature failed")?;

let Some(value_signature) = &args.value_signature else {
return Ok(());
};

// If data to be added is signed, also validate its signature
value_signature
.verifying_key
.verify_signature(&args.value, &value_signature.signature)
.context("Verifying value signature failed")
}
}
}
Expand All @@ -318,7 +350,11 @@ impl Operation {
match &self {
Operation::AddKey(KeyOperationArgs { id, signature, .. })
| Operation::RevokeKey(KeyOperationArgs { id, signature, .. })
| Operation::AddSignedData(AddSignedDataArgs { id, signature, .. }) => {
| Operation::AddData(AddDataArgs {
id,
op_signature: signature,
..
}) => {
if id.is_empty() {
return Err(
GeneralError::MissingArgumentError("id is empty".to_string()).into(),
Expand Down
31 changes: 27 additions & 4 deletions crates/common/src/test_utils.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
hashchain::Hashchain,
keys::{SigningKey, VerifyingKey},
operation::{Operation, ServiceChallenge},
operation::{Operation, ServiceChallenge, SignatureBundle},
tree::{
HashchainResponse::*, InsertProof, KeyDirectoryTree, Proof, SnarkableTree, UpdateProof,
},
Expand Down Expand Up @@ -124,18 +124,41 @@ impl TestTreeState {
Ok(())
}

pub fn add_unsigned_data_to_account(
&mut self,
data: &[u8],
account: &mut TestAccount,
) -> Result<()> {
self.add_data_to_account(data, account, None)
}

pub fn add_signed_data_to_account(
&mut self,
data: &[u8],
account: &mut TestAccount,
) -> Result<()> {
let signing_key = self.signing_keys.get(&account.hashchain.id).unwrap();
let signature = signing_key.sign(data);
let random_signing_key = create_mock_signing_key();
self.add_data_to_account(data, account, Some(&random_signing_key))
}

fn add_data_to_account(
&mut self,
data: &[u8],
account: &mut TestAccount,
signing_key: Option<&SigningKey>,
) -> Result<()> {
let signature_bundle = signing_key.map(|sk| SignatureBundle {
verifying_key: sk.verifying_key(),
signature: sk.sign(data),
});

let op_signing_key = self.signing_keys.get(&account.hashchain.id).unwrap();

let op = Operation::new_add_signed_data(
account.hashchain.id.clone(),
data.to_vec(),
signature,
signature_bundle,
op_signing_key,
0,
)?;

Expand Down
Loading