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: useable and tested scratchpads #2671

Merged
merged 7 commits into from
Jan 29, 2025
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
4 changes: 2 additions & 2 deletions ant-networking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,15 +639,15 @@ impl Network {
continue;
};

if !scratchpad.is_valid() {
if !scratchpad.verify() {
warn!(
"Rejecting Scratchpad for {pretty_key} PUT with invalid signature"
);
continue;
}

if let Some(old) = &valid_scratchpad {
if old.count() >= scratchpad.count() {
if old.counter() >= scratchpad.counter() {
info!("Rejecting Scratchpad for {pretty_key} with lower count than the previous one");
continue;
}
Expand Down
6 changes: 1 addition & 5 deletions ant-networking/src/record_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1296,12 +1296,8 @@ mod tests {
// Create a scratchpad
let unencrypted_scratchpad_data = Bytes::from_static(b"Test scratchpad data");
let owner_sk = SecretKey::random();
let owner_pk = owner_sk.public_key();

let mut scratchpad = Scratchpad::new(owner_pk, 0);

let _next_version =
scratchpad.update_and_sign(unencrypted_scratchpad_data.clone(), &owner_sk);
let scratchpad = Scratchpad::new(&owner_sk, 0, &unencrypted_scratchpad_data, 0);

let scratchpad_address = *scratchpad.address();

Expand Down
9 changes: 6 additions & 3 deletions ant-node/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use thiserror::Error;

pub(super) type Result<T, E = Error> = std::result::Result<T, E>;

const SCRATCHPAD_MAX_SIZE: usize = ant_protocol::storage::Scratchpad::MAX_SIZE;

/// Internal error.
#[derive(Debug, Error)]
#[allow(missing_docs)]
Expand All @@ -38,12 +40,13 @@ pub enum Error {
#[error("The Record::key does not match with the key derived from Record::value")]
RecordKeyMismatch,

// Scratchpad is old version
// ------------ Scratchpad Errors
#[error("A newer version of this Scratchpad already exists")]
IgnoringOutdatedScratchpadPut,
// Scratchpad is invalid
#[error("Scratchpad signature is invalid over the counter + content hash")]
#[error("Scratchpad signature is invalid")]
InvalidScratchpadSignature,
#[error("Scratchpad too big: {0}, max size is {SCRATCHPAD_MAX_SIZE}")]
ScratchpadTooBig(usize),

#[error("Invalid signature")]
InvalidSignature,
Expand Down
12 changes: 9 additions & 3 deletions ant-node/src/put_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ impl Node {
) -> Result<()> {
// owner PK is defined herein, so as long as record key and this match, we're good
let addr = scratchpad.address();
let count = scratchpad.count();
let count = scratchpad.counter();
debug!("Validating and storing scratchpad {addr:?} with count {count}");

// check if the deserialized value's ScratchpadAddress matches the record's key
Expand All @@ -460,18 +460,24 @@ impl Node {
// check if the Scratchpad is present locally that we don't have a newer version
if let Some(local_pad) = self.network().get_local_record(&scratchpad_key).await? {
let local_pad = try_deserialize_record::<Scratchpad>(&local_pad)?;
if local_pad.count() >= scratchpad.count() {
if local_pad.counter() >= scratchpad.counter() {
warn!("Rejecting Scratchpad PUT with counter less than or equal to the current counter");
return Err(Error::IgnoringOutdatedScratchpadPut);
}
}

// ensure data integrity
if !scratchpad.is_valid() {
if !scratchpad.verify() {
warn!("Rejecting Scratchpad PUT with invalid signature");
return Err(Error::InvalidScratchpadSignature);
}

// ensure the scratchpad is not too big
if scratchpad.is_too_big() {
warn!("Rejecting Scratchpad PUT with too big size");
return Err(Error::ScratchpadTooBig(scratchpad.size()));
}

info!(
"Storing sratchpad {addr:?} with content of {:?} as Record locally",
scratchpad.encrypted_data_hash()
Expand Down
7 changes: 5 additions & 2 deletions ant-node/tests/data_with_churn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,10 @@ fn create_graph_entry_task(

let mut retries = 1;
loop {
match client.graph_entry_put(graph_entry.clone(), &wallet).await {
match client
.graph_entry_put(graph_entry.clone(), (&wallet).into())
.await
{
Ok((cost, addr)) => {
println!("Uploaded graph_entry to {addr:?} with cost of {cost:?} after a delay of: {delay:?}");
let net_addr = NetworkAddress::GraphEntryAddress(addr);
Expand Down Expand Up @@ -498,7 +501,7 @@ fn create_pointers_task(
PointerTarget::ChunkAddress(ChunkAddress::new(XorName(rand::random())));
loop {
match client
.pointer_create(&owner, pointer_target.clone(), &wallet)
.pointer_create(&owner, pointer_target.clone(), (&wallet).into())
.await
{
Ok((cost, addr)) => {
Expand Down
24 changes: 24 additions & 0 deletions ant-protocol/src/storage/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ pub struct GraphEntry {
}

impl GraphEntry {
/// Maximum size of a graph entry
pub const MAX_SIZE: usize = 1024;

/// Create a new graph entry, signing it with the provided secret key.
pub fn new(
owner: PublicKey,
Expand Down Expand Up @@ -101,8 +104,29 @@ impl GraphEntry {
Self::bytes_to_sign(&self.owner, &self.parents, &self.content, &self.outputs)
}

/// Verify the signature of the graph entry
pub fn verify(&self) -> bool {
self.owner
.verify(&self.signature, self.bytes_for_signature())
}

/// Size of the graph entry
pub fn size(&self) -> usize {
size_of::<GraphEntry>()
+ self
.outputs
.iter()
.map(|(p, c)| p.to_bytes().len() + c.len())
.sum::<usize>()
+ self
.parents
.iter()
.map(|p| p.to_bytes().len())
.sum::<usize>()
}

/// Returns true if the graph entry is too big
pub fn is_too_big(&self) -> bool {
self.size() > Self::MAX_SIZE
}
}
7 changes: 7 additions & 0 deletions ant-protocol/src/storage/pointer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ pub enum PointerError {
SerializationError(String),
}

/// Pointer, a mutable address pointing to other data on the Network
/// It is stored at the owner's public key and can only be updated by the owner
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
pub struct Pointer {
owner: PublicKey,
Expand Down Expand Up @@ -130,6 +132,11 @@ impl Pointer {
let bytes = self.bytes_for_signature();
self.owner.verify(&self.signature, &bytes)
}

/// Size of the pointer
pub fn size() -> usize {
size_of::<Pointer>()
}
}

#[cfg(test)]
Expand Down
164 changes: 116 additions & 48 deletions ant-protocol/src/storage/scratchpad.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize};

use xor_name::XorName;

/// Scratchpad, a mutable address for encrypted data
/// Scratchpad, a mutable space for encrypted data on the Network
#[derive(
Hash, Eq, PartialEq, PartialOrd, Ord, Clone, custom_debug::Debug, Serialize, Deserialize,
)]
Expand All @@ -25,30 +25,81 @@ pub struct Scratchpad {
address: ScratchpadAddress,
/// Data encoding: custom apps using scratchpad should use this so they can identify the type of data they are storing
data_encoding: u64,
/// Contained data. This should be encrypted
/// Encrypted data stored in the scratchpad, it is encrypted automatically by the [`Scratchpad::new`] and [`Scratchpad::update`] methods
#[debug(skip)]
encrypted_data: Bytes,
/// Monotonically increasing counter to track the number of times this has been updated.
/// When pushed to the network, the scratchpad with the highest counter is kept.
counter: u64,
/// Signature over `Vec<counter>`.extend(Xorname::from_content(encrypted_data).to_vec()) from the owning key.
/// Required for scratchpad to be valid.
signature: Option<Signature>,
/// Signature over the above fields
signature: Signature,
}

impl Scratchpad {
/// Creates a new instance of `Scratchpad`.
pub fn new(owner: PublicKey, data_encoding: u64) -> Self {
/// Max Scratchpad size is 4MB including the metadata
pub const MAX_SIZE: usize = 4 * 1024 * 1024;

/// Creates a new instance of `Scratchpad`. Encrypts the data, and signs all the elements.
pub fn new(
owner: &SecretKey,
data_encoding: u64,
unencrypted_data: &Bytes,
counter: u64,
) -> Self {
let pk = owner.public_key();
let encrypted_data = Bytes::from(pk.encrypt(unencrypted_data).to_bytes());
let addr = ScratchpadAddress::new(pk);
let signature = owner.sign(Self::bytes_for_signature(
addr,
data_encoding,
&encrypted_data,
counter,
));
Self {
address: addr,
encrypted_data,
data_encoding,
counter,
signature,
}
}
Copy link
Member

Choose a reason for hiding this comment

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

shall the self.verify() function still to be called ?


/// Create a new Scratchpad without provding the secret key
/// It is the caller's responsibility to ensure the signature is valid (signs [`Scratchpad::bytes_for_signature`]) and the data is encrypted
/// It is recommended to use the [`Scratchpad::new`] method instead when possible
pub fn new_with_signature(
Copy link
Member

Choose a reason for hiding this comment

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

to make the public API cleaner, this function shall not be exposed ?
also, this function means input_data is not encrypted, which is un-consistent with new function and claims made in other places.
also, this one doesn't call the is_valid self check ?

I'd suggest:

  • not use this in the public API
  • make new function use this (if retained as a private helper)

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking of keeping this for folks that cannot provide us with the Secret key but can sign messages/encrypt. This is typically the case with hardware wallets, where one cannot get the secret key out, but can perfectly sign messages.

Copy link
Member

Choose a reason for hiding this comment

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

even we accept such input, the signature and size shall still be checked.
i.e. the verify function shall be called

owner: PublicKey,
data_encoding: u64,
encrypted_data: Bytes,
counter: u64,
signature: Signature,
) -> Self {
Self {
address: ScratchpadAddress::new(owner),
encrypted_data: Bytes::new(),
encrypted_data,
data_encoding,
counter: 0,
signature: None,
counter,
signature,
}
}

/// Return the current count
pub fn count(&self) -> u64 {
/// Returns the bytes to sign for the signature
pub fn bytes_for_signature(
address: ScratchpadAddress,
data_encoding: u64,
encrypted_data: &Bytes,
counter: u64,
) -> Vec<u8> {
let mut bytes_to_sign = data_encoding.to_be_bytes().to_vec();
bytes_to_sign.extend(address.to_hex().as_bytes());
bytes_to_sign.extend(counter.to_be_bytes().to_vec());
bytes_to_sign.extend(encrypted_data.to_vec());
bytes_to_sign
}

/// Get the counter of the Scratchpad, the higher the counter, the more recent the Scratchpad is
/// Similarly to counter CRDTs only the latest version (highest counter) of the Scratchpad is kept on the network
pub fn counter(&self) -> u64 {
self.counter
}

Expand All @@ -57,43 +108,32 @@ impl Scratchpad {
self.data_encoding
}

/// Increments the counter value.
pub fn increment(&mut self) -> u64 {
/// Updates the content and encrypts it, increments the counter, re-signs the scratchpad
pub fn update(&mut self, unencrypted_data: &Bytes, sk: &SecretKey) {
self.counter += 1;

self.counter
}

/// Returns the next counter value,
///
/// Encrypts data and updates the signature with provided sk
pub fn update_and_sign(&mut self, unencrypted_data: Bytes, sk: &SecretKey) -> u64 {
let next_count = self.increment();

let pk = self.owner();

let address = ScratchpadAddress::new(*pk);
self.encrypted_data = Bytes::from(pk.encrypt(unencrypted_data).to_bytes());

let encrypted_data_xorname = self.encrypted_data_hash().to_vec();

let mut bytes_to_sign = self.counter.to_be_bytes().to_vec();
bytes_to_sign.extend(encrypted_data_xorname);

self.signature = Some(sk.sign(&bytes_to_sign));
next_count
let bytes_to_sign = Self::bytes_for_signature(
address,
self.data_encoding,
&self.encrypted_data,
self.counter,
);
self.signature = sk.sign(&bytes_to_sign);
debug_assert!(self.verify(), "Must be valid after being signed. This is a bug, please report it by opening an issue on our github");
}

/// Verifies the signature and content of the scratchpad are valid for the
/// owner's public key.
pub fn is_valid(&self) -> bool {
if let Some(signature) = &self.signature {
let mut signing_bytes = self.counter.to_be_bytes().to_vec();
signing_bytes.extend(self.encrypted_data_hash().to_vec()); // add the count

self.owner().verify(signature, &signing_bytes)
} else {
false
}
/// Verifies that the Scratchpad signature is valid
pub fn verify(&self) -> bool {
Copy link
Member

@maqi maqi Jan 28, 2025

Choose a reason for hiding this comment

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

shall the size to be checked as well?
same to the other data_types

Also, I notice there is scratchpad_verify which checks both signature and size for ScratchPad
I think normally, the verify indicates both, verify_signature and verify_size (or is_size_too_big) verifies individual part.
I'd suggest update the names/function calls of all data_types' verify to make them consistant.

let signing_bytes = Self::bytes_for_signature(
self.address,
self.data_encoding,
&self.encrypted_data,
self.counter,
);
self.owner().verify(&self.signature, &signing_bytes)
}

/// Returns the encrypted_data.
Expand Down Expand Up @@ -140,18 +180,46 @@ impl Scratchpad {
pub fn payload_size(&self) -> usize {
self.encrypted_data.len()
}

/// Size of the scratchpad
pub fn size(&self) -> usize {
size_of::<Scratchpad>() + self.payload_size()
}

/// Returns true if the scratchpad is too big
pub fn is_too_big(&self) -> bool {
self.size() > Self::MAX_SIZE
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_scratchpad_is_valid() {
fn test_scratchpad_sig_and_update() {
let sk = SecretKey::random();
let raw_data = Bytes::from_static(b"data to be encrypted");
let mut scratchpad = Scratchpad::new(&sk, 42, &raw_data, 0);
assert!(scratchpad.verify());
assert_eq!(scratchpad.counter(), 0);
assert_ne!(scratchpad.encrypted_data(), &raw_data);

let raw_data2 = Bytes::from_static(b"data to be encrypted v2");
scratchpad.update(&raw_data2, &sk);
assert!(scratchpad.verify());
assert_eq!(scratchpad.counter(), 1);
assert_ne!(scratchpad.encrypted_data(), &raw_data);
assert_ne!(scratchpad.encrypted_data(), &raw_data2);
}

#[test]
fn test_scratchpad_encryption() {
let sk = SecretKey::random();
let pk = sk.public_key();
let mut scratchpad = Scratchpad::new(pk, 42);
scratchpad.update_and_sign(Bytes::from_static(b"data to be encrypted"), &sk);
assert!(scratchpad.is_valid());
let raw_data = Bytes::from_static(b"data to be encrypted");
let scratchpad = Scratchpad::new(&sk, 42, &raw_data, 0);

let decrypted_data = scratchpad.decrypt_data(&sk).unwrap();
assert_eq!(decrypted_data, raw_data);
}
}
Loading
Loading