* API Breaking Change: CryptoSystem.verify() should return bool, and reserve errors for error cases, not validation failures.

* API Breaking Change: VeilidAPI.verify_signatures() returns Option<TypedKeySet> now
Fixes #313
This commit is contained in:
Christien Rioux 2024-05-31 16:20:58 -04:00
parent 8e8ee06fe9
commit 05180252e4
36 changed files with 445 additions and 174 deletions

View file

@ -46,7 +46,7 @@ pub trait CryptoSystem {
// Authentication
fn sign(&self, key: &PublicKey, secret: &SecretKey, data: &[u8]) -> VeilidAPIResult<Signature>;
fn verify(&self, key: &PublicKey, data: &[u8], signature: &Signature) -> VeilidAPIResult<()>;
fn verify(&self, key: &PublicKey, data: &[u8], signature: &Signature) -> VeilidAPIResult<bool>;
// AEAD Encrypt/Decrypt
fn aead_overhead(&self) -> usize;

View file

@ -172,9 +172,12 @@ impl Envelope {
);
// Validate signature
vcrypto
if !vcrypto
.verify(&sender_id, &data[0..(data.len() - 64)], &signature)
.map_err(VeilidAPIError::internal)?;
.map_err(VeilidAPIError::internal)?
{
apibail_parse_error!("signature verification of envelope failed", signature);
}
// Return envelope
Ok(Self {

View file

@ -238,27 +238,28 @@ impl Crypto {
}
/// Signature set verification
/// Returns the set of signature cryptokinds that validate and are supported
/// If any cryptokinds are supported and do not validate, the whole operation
/// returns an error
/// Returns Some() the set of signature cryptokinds that validate and are supported
/// Returns None if any cryptokinds are supported and do not validate
pub fn verify_signatures(
&self,
node_ids: &[TypedKey],
public_keys: &[TypedKey],
data: &[u8],
typed_signatures: &[TypedSignature],
) -> VeilidAPIResult<TypedKeyGroup> {
let mut out = TypedKeyGroup::with_capacity(node_ids.len());
) -> VeilidAPIResult<Option<TypedKeyGroup>> {
let mut out = TypedKeyGroup::with_capacity(public_keys.len());
for sig in typed_signatures {
for nid in node_ids {
for nid in public_keys {
if nid.kind == sig.kind {
if let Some(vcrypto) = self.get(sig.kind) {
vcrypto.verify(&nid.value, data, &sig.value)?;
if !vcrypto.verify(&nid.value, data, &sig.value)? {
return Ok(None);
}
out.add(*nid);
}
}
}
}
Ok(out)
Ok(Some(out))
}
/// Signature set generation

View file

@ -143,13 +143,13 @@ impl CryptoSystem for CryptoSystemNONE {
// Validation
fn validate_keypair(&self, dht_key: &PublicKey, dht_key_secret: &SecretKey) -> bool {
let data = vec![0u8; 512];
let sig = match self.sign(dht_key, dht_key_secret, &data) {
Ok(s) => s,
Err(_) => {
return false;
}
let Ok(sig) = self.sign(dht_key, dht_key_secret, &data) else {
return false;
};
self.verify(dht_key, &data, &sig).is_ok()
let Ok(v) = self.verify(dht_key, &data, &sig) else {
return false;
};
v
}
fn validate_hash(&self, data: &[u8], dht_key: &PublicKey) -> bool {
let bytes = *blake3::hash(data).as_bytes();
@ -205,7 +205,7 @@ impl CryptoSystem for CryptoSystemNONE {
dht_key: &PublicKey,
data: &[u8],
signature: &Signature,
) -> VeilidAPIResult<()> {
) -> VeilidAPIResult<bool> {
let mut dig = Blake3Digest512::new();
dig.update(data);
let sig = dig.finalize();
@ -217,19 +217,13 @@ impl CryptoSystem for CryptoSystemNONE {
.copy_from_slice(&do_xor_32(&in_sig_bytes[32..64], &signature.bytes[32..64]));
if !is_bytes_eq_32(&verify_bytes[0..32], 0u8) {
return Err(VeilidAPIError::parse_error(
"Verification failed",
"signature 0..32 is invalid",
));
return Ok(false);
}
if !is_bytes_eq_32(&do_xor_32(&verify_bytes[32..64], &dht_key.bytes), 0xFFu8) {
return Err(VeilidAPIError::parse_error(
"Verification failed",
"signature 32..64 is invalid",
));
return Ok(false);
}
Ok(())
return Ok(true);
}
// AEAD Encrypt/Decrypt

View file

@ -129,9 +129,12 @@ impl Receipt {
);
// Validate signature
vcrypto
if !vcrypto
.verify(&sender_id, &data[0..(data.len() - 64)], &signature)
.map_err(VeilidAPIError::generic)?;
.map_err(VeilidAPIError::generic)?
{
apibail_parse_error!("signature failure in receipt", signature);
}
// Get nonce
let nonce: Nonce = Nonce::new(

View file

@ -64,49 +64,55 @@ pub async fn test_sign_and_verify(vcrypto: CryptoSystemVersion) {
assert_eq!(
vcrypto.verify(&dht_key, LOREM_IPSUM.as_bytes(), &a1),
Ok(())
Ok(true)
);
assert_eq!(
vcrypto.verify(&dht_key2, LOREM_IPSUM.as_bytes(), &a2),
Ok(())
Ok(true)
);
assert_eq!(
vcrypto.verify(&dht_key, LOREM_IPSUM.as_bytes(), &a2),
Ok(false)
);
assert_eq!(
vcrypto.verify(&dht_key2, LOREM_IPSUM.as_bytes(), &a1),
Ok(false)
);
assert!(vcrypto
.verify(&dht_key, LOREM_IPSUM.as_bytes(), &a2)
.is_err());
assert!(vcrypto
.verify(&dht_key2, LOREM_IPSUM.as_bytes(), &a1)
.is_err());
// Try verifications that should work
assert_eq!(
vcrypto.verify(&dht_key, LOREM_IPSUM.as_bytes(), &dht_sig),
Ok(())
Ok(true)
);
assert_eq!(
vcrypto.verify(&dht_key, LOREM_IPSUM.as_bytes(), &dht_sig_b),
Ok(())
Ok(true)
);
assert_eq!(
vcrypto.verify(&dht_key2, LOREM_IPSUM.as_bytes(), &dht_sig2),
Ok(())
Ok(true)
);
assert_eq!(
vcrypto.verify(&dht_key, CHEEZBURGER.as_bytes(), &dht_sig_c),
Ok(())
Ok(true)
);
// Try verifications that shouldn't work
assert!(vcrypto
.verify(&dht_key2, LOREM_IPSUM.as_bytes(), &dht_sig)
.is_err());
assert!(vcrypto
.verify(&dht_key, LOREM_IPSUM.as_bytes(), &dht_sig2)
.is_err());
assert!(vcrypto
.verify(&dht_key2, CHEEZBURGER.as_bytes(), &dht_sig_c)
.is_err());
assert!(vcrypto
.verify(&dht_key, CHEEZBURGER.as_bytes(), &dht_sig)
.is_err());
assert_eq!(
vcrypto.verify(&dht_key2, LOREM_IPSUM.as_bytes(), &dht_sig),
Ok(false)
);
assert_eq!(
vcrypto.verify(&dht_key, LOREM_IPSUM.as_bytes(), &dht_sig2),
Ok(false)
);
assert_eq!(
vcrypto.verify(&dht_key2, CHEEZBURGER.as_bytes(), &dht_sig_c),
Ok(false)
);
assert_eq!(
vcrypto.verify(&dht_key, CHEEZBURGER.as_bytes(), &dht_sig),
Ok(false)
);
}
pub async fn test_key_conversions(vcrypto: CryptoSystemVersion) {

View file

@ -161,13 +161,13 @@ impl CryptoSystem for CryptoSystemVLD0 {
// Validation
fn validate_keypair(&self, dht_key: &PublicKey, dht_key_secret: &SecretKey) -> bool {
let data = vec![0u8; 512];
let sig = match self.sign(dht_key, dht_key_secret, &data) {
Ok(s) => s,
Err(_) => {
return false;
}
let Ok(sig) = self.sign(dht_key, dht_key_secret, &data) else {
return false;
};
self.verify(dht_key, &data, &sig).is_ok()
let Ok(v) = self.verify(dht_key, &data, &sig) else {
return false;
};
v
}
fn validate_hash(&self, data: &[u8], dht_key: &PublicKey) -> bool {
let bytes = *blake3::hash(data).as_bytes();
@ -219,7 +219,9 @@ impl CryptoSystem for CryptoSystemVLD0 {
let sig = Signature::new(sig_bytes.to_bytes());
self.verify(dht_key, data, &sig)?;
if !self.verify(dht_key, data, &sig)? {
apibail_internal!("newly created signature does not verify");
}
Ok(sig)
}
@ -228,7 +230,7 @@ impl CryptoSystem for CryptoSystemVLD0 {
dht_key: &PublicKey,
data: &[u8],
signature: &Signature,
) -> VeilidAPIResult<()> {
) -> VeilidAPIResult<bool> {
let pk = ed::VerifyingKey::from_bytes(&dht_key.bytes)
.map_err(|e| VeilidAPIError::parse_error("Public key is invalid", e))?;
let sig = ed::Signature::from_bytes(&signature.bytes);
@ -236,9 +238,13 @@ impl CryptoSystem for CryptoSystemVLD0 {
let mut dig: ed::Sha512 = ed::Sha512::default();
dig.update(data);
pk.verify_prehashed_strict(dig, Some(VEILID_DOMAIN_SIGN), &sig)
.map_err(|e| VeilidAPIError::parse_error("Verification failed", e))?;
Ok(())
if pk
.verify_prehashed_strict(dig, Some(VEILID_DOMAIN_SIGN), &sig)
.is_err()
{
return Ok(false);
}
Ok(true)
}
// AEAD Encrypt/Decrypt

View file

@ -694,9 +694,16 @@ impl RouteSpecStore {
}
} else {
// Verify a signature for a hop node along the route
if let Err(e) = vcrypto.verify(hop_public_key, data, &signatures[hop_n]) {
log_rpc!(debug "failed to verify signature for hop {} at {} on private route {}: {}", hop_n, hop_public_key, public_key, e);
return None;
match vcrypto.verify(hop_public_key, data, &signatures[hop_n]) {
Ok(true) => {}
Ok(false) => {
log_rpc!(debug "invalid signature for hop {} at {} on private route {}", hop_n, hop_public_key, public_key);
return None;
}
Err(e) => {
log_rpc!(debug "errir verifying signature for hop {} at {} on private route {}: {}", hop_n, hop_public_key, public_key, e);
return None;
}
}
}
}

View file

@ -27,8 +27,11 @@ impl SignedDirectNodeInfo {
let node_info_bytes = Self::make_signature_bytes(&self.node_info, self.timestamp)?;
// Verify the signatures that we can
let validated_node_ids =
let opt_validated_node_ids =
crypto.verify_signatures(node_ids, &node_info_bytes, &self.signatures)?;
let Some(validated_node_ids) = opt_validated_node_ids else {
apibail_generic!("verification error in direct node info");
};
if validated_node_ids.is_empty() {
apibail_generic!("no valid node ids in direct node info");
}

View file

@ -53,8 +53,11 @@ impl SignedRelayedNodeInfo {
&self.relay_info,
self.timestamp,
)?;
let validated_node_ids =
let opt_validated_node_ids =
crypto.verify_signatures(node_ids, &node_info_bytes, &self.signatures)?;
let Some(validated_node_ids) = opt_validated_node_ids else {
apibail_generic!("verification error in relayed node info");
};
if validated_node_ids.is_empty() {
apibail_generic!("no valid node ids in relayed node info");
}

View file

@ -142,13 +142,16 @@ impl RPCOperationGetValueA {
};
// And the signed value data
value
if !value
.validate(
descriptor.owner(),
get_value_context.subkey,
get_value_context.vcrypto.clone(),
)
.map_err(RPCError::protocol)?;
.map_err(RPCError::protocol)?
{
return Err(RPCError::protocol("signed value data did not validate"));
}
}
PeerInfo::validate_vec(&mut self.peers, validate_context.crypto.clone());

View file

@ -149,13 +149,16 @@ impl RPCOperationSetValueA {
if let Some(value) = &self.value {
// And the signed value data
value
if !value
.validate(
set_value_context.descriptor.owner(),
set_value_context.subkey,
set_value_context.vcrypto.clone(),
)
.map_err(RPCError::protocol)?;
.map_err(RPCError::protocol)?
{
return Err(RPCError::protocol("signed value data did not validate"));
}
}
PeerInfo::validate_vec(&mut self.peers, validate_context.crypto.clone());

View file

@ -88,9 +88,12 @@ impl RPCOperationWatchValueQ {
self.count,
self.watch_id,
);
vcrypto
if !vcrypto
.verify(&self.watcher, &sig_data, &self.signature)
.map_err(RPCError::protocol)?;
.map_err(RPCError::protocol)?
{
return Err(RPCError::protocol("failed to validate watcher signature"));
}
// Count is zero means cancelling, so there should always be a watch id in this case
if self.count == 0 && self.watch_id.is_none() {

View file

@ -20,7 +20,7 @@ impl SignedValueData {
owner: &PublicKey,
subkey: ValueSubkey,
vcrypto: CryptoSystemVersion,
) -> VeilidAPIResult<()> {
) -> VeilidAPIResult<bool> {
let node_info_bytes = Self::make_signature_bytes(&self.value_data, owner, subkey)?;
// validate signature
vcrypto.verify(self.value_data.writer(), &node_info_bytes, &self.signature)

View file

@ -19,7 +19,12 @@ impl SignedValueDescriptor {
pub fn validate(&self, vcrypto: CryptoSystemVersion) -> VeilidAPIResult<()> {
// validate signature
vcrypto.verify(&self.owner, &self.schema_data, &self.signature)?;
if !vcrypto.verify(&self.owner, &self.schema_data, &self.signature)? {
apibail_parse_error!(
"failed to validate signature of signed value descriptor",
self.signature
);
}
// validate schema
DHTSchema::try_from(self.schema_data.as_slice())?;
Ok(())

View file

@ -18,6 +18,7 @@ pub struct CryptoSystemResponse {
#[serde(tag = "cs_op")]
pub enum CryptoSystemRequestOp {
Release,
Kind,
CachedDh {
#[schemars(with = "String")]
key: PublicKey,
@ -108,7 +109,7 @@ pub enum CryptoSystemRequestOp {
#[schemars(with = "String")]
data: Vec<u8>,
#[schemars(with = "String")]
secret: Signature,
signature: Signature,
},
AeadOverhead,
DecryptAead {
@ -150,6 +151,10 @@ pub enum CryptoSystemRequestOp {
pub enum CryptoSystemResponseOp {
InvalidId,
Release,
Kind {
#[schemars(with = "String")]
value: CryptoKind,
},
CachedDh {
#[serde(flatten)]
#[schemars(with = "ApiResult<String>")]
@ -219,7 +224,7 @@ pub enum CryptoSystemResponseOp {
},
Verify {
#[serde(flatten)]
result: ApiResult<()>,
result: ApiResult<bool>,
},
AeadOverhead {
value: u32,

View file

@ -201,8 +201,8 @@ pub enum ResponseOp {
CryptoSystem(CryptoSystemResponse),
VerifySignatures {
#[serde(flatten)]
#[schemars(with = "ApiResult<Vec<String>>")]
result: ApiResultWithVecString<TypedKeyGroup>,
#[schemars(with = "ApiResult<Option<Vec<String>>>")]
result: ApiResultWithOptVecString<Option<TypedKeyGroup>>,
},
GenerateSignatures {
#[serde(flatten)]
@ -308,6 +308,21 @@ where
},
}
#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
#[serde(untagged)]
pub enum ApiResultWithOptVecString<T>
where
T: Clone + fmt::Debug,
{
Ok {
#[schemars(with = "Option<Vec<String>>")]
value: T,
},
Err {
error: VeilidAPIError,
},
}
pub fn emit_schemas(out: &mut HashMap<String, String>) {
let schema_request = schema_for!(Request);
let schema_recv_message = schema_for!(RecvMessage);

View file

@ -28,6 +28,15 @@ pub fn to_json_api_result_with_vec_string<T: Clone + fmt::Debug>(
}
}
pub fn to_json_api_result_with_opt_vec_string<T: Clone + fmt::Debug>(
r: VeilidAPIResult<T>,
) -> json_api::ApiResultWithOptVecString<T> {
match r {
Err(e) => json_api::ApiResultWithOptVecString::Err { error: e },
Ok(v) => json_api::ApiResultWithOptVecString::Ok { value: v },
}
}
pub fn to_json_api_result_with_vec_u8(r: VeilidAPIResult<Vec<u8>>) -> json_api::ApiResultWithVecU8 {
match r {
Err(e) => json_api::ApiResultWithVecU8::Err { error: e },
@ -462,6 +471,7 @@ impl JsonRequestProcessor {
self.release_crypto_system(csr.cs_id);
CryptoSystemResponseOp::Release {}
}
CryptoSystemRequestOp::Kind => CryptoSystemResponseOp::Kind { value: csv.kind() },
CryptoSystemRequestOp::CachedDh { key, secret } => CryptoSystemResponseOp::CachedDh {
result: to_json_api_result_with_string(csv.cached_dh(&key, &secret)),
},
@ -532,8 +542,12 @@ impl JsonRequestProcessor {
CryptoSystemRequestOp::Sign { key, secret, data } => CryptoSystemResponseOp::Sign {
result: to_json_api_result_with_string(csv.sign(&key, &secret, &data)),
},
CryptoSystemRequestOp::Verify { key, data, secret } => CryptoSystemResponseOp::Verify {
result: to_json_api_result(csv.verify(&key, &data, &secret)),
CryptoSystemRequestOp::Verify {
key,
data,
signature,
} => CryptoSystemResponseOp::Verify {
result: to_json_api_result(csv.verify(&key, &data, &signature)),
},
CryptoSystemRequestOp::AeadOverhead => CryptoSystemResponseOp::AeadOverhead {
value: csv.aead_overhead() as u32,
@ -766,7 +780,7 @@ impl JsonRequestProcessor {
}
};
ResponseOp::VerifySignatures {
result: to_json_api_result_with_vec_string(crypto.verify_signatures(
result: to_json_api_result_with_opt_vec_string(crypto.verify_signatures(
&node_ids,
&data,
&signatures,