Skip to content

Commit

Permalink
fixup! force using signature v1 in more cases
Browse files Browse the repository at this point in the history
  • Loading branch information
@divarvel
divarvel committed Nov 20, 2024
1 parent e544df5 commit f58c393
Showing 1 changed file with 152 additions and 56 deletions.
208 changes: 152 additions & 56 deletions biscuit-auth/src/format/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,14 +294,13 @@ impl SerializedBiscuit {
next_keypair: &KeyPair,
authority: &Block,
) -> Result<Self, error::Token> {
let authority_signature_version = if authority.version >= DATALOG_3_3 {
DATALOG_3_3_SIGNATURE_VERSION
} else {
match (root_keypair, next_keypair) {
(KeyPair::Ed25519(_), KeyPair::Ed25519(_)) => 0,
_ => NON_ED25519_SIGNATURE_VERSION,
}
};
let authority_signature_version = block_signature_version(
root_keypair,
next_keypair,
&None,
&Some(authority.version),
std::iter::empty(),
);
Self::new_inner(
root_key_id,
root_keypair,
Expand Down Expand Up @@ -363,30 +362,18 @@ impl SerializedBiscuit {
error::Format::SerializationError(format!("serialization error: {:?}", e))
})?;

let signature_version = if external_signature.is_some() {
THIRD_PARTY_SIGNATURE_VERSION
} else if block.version >= DATALOG_3_3 {
DATALOG_3_3_SIGNATURE_VERSION
} else {
match (next_keypair, &keypair) {
(KeyPair::Ed25519(_), KeyPair::Ed25519(_)) => {
let mut has_signature_v1 = self.authority.version > 0;
for block in &self.blocks {
if block.version > 0 {
has_signature_v1 = true;
break;
}
}

if has_signature_v1 {
1
} else {
0
}
}
_ => NON_ED25519_SIGNATURE_VERSION,
}
};
let signature_version = block_signature_version(
&keypair,
next_keypair,
&external_signature,
&Some(block.version),
// std::iter::once(self.authority.version)
// .chain(self.blocks.iter().map(|block| block.version)),
self.blocks
.iter()
.chain([&self.authority])
.map(|block| block.version),
);

let signature = crypto::sign_block(
&keypair,
Expand Down Expand Up @@ -424,30 +411,16 @@ impl SerializedBiscuit {
) -> Result<Self, error::Token> {
let keypair = self.proof.keypair()?;

// The version block is not directly available, so we don’t take it into account here
// `append_serialized` is only used for third-party blocks anyway, so maybe we should make `external_signature` mandatory and not bother
let signature_version = if external_signature.is_some() {
THIRD_PARTY_SIGNATURE_VERSION
} else {
match (next_keypair, &keypair) {
(KeyPair::Ed25519(_), KeyPair::Ed25519(_)) => {
let mut has_signature_v1 = self.authority.version > 0;
for block in &self.blocks {
if block.version > 0 {
has_signature_v1 = true;
break;
}
}

if has_signature_v1 {
1
} else {
0
}
}
_ => NON_ED25519_SIGNATURE_VERSION,
}
};
let signature_version = block_signature_version(
&keypair,
next_keypair,
&external_signature,
// The version block is not directly available, so we don’t take it into account here
// `append_serialized` is only used for third-party blocks anyway, so maybe we should make `external_signature` mandatory and not bother
&None,
std::iter::once(self.authority.version)
.chain(self.blocks.iter().map(|block| block.version)),
);

let signature = crypto::sign_block(
&keypair,
Expand Down Expand Up @@ -572,10 +545,49 @@ pub(crate) enum ThirdPartyVerificationMode {
PreviousSignatureHashing,
}

fn block_signature_version<I>(
block_keypair: &KeyPair,
next_keypair: &KeyPair,
external_signature: &Option<ExternalSignature>,
block_version: &Option<u32>,
previous_blocks_sig_versions: I,
) -> u32
where
I: Iterator<Item = u32>,
{
if external_signature.is_some() {
return THIRD_PARTY_SIGNATURE_VERSION;
}

match block_version {
Some(block_version) if *block_version >= DATALOG_3_3 => {
return DATALOG_3_3_SIGNATURE_VERSION;
}
_ => {}

Check warning on line 566 in biscuit-auth/src/format/mod.rs

View check run for this annotation

Codecov / codecov/patch

biscuit-auth/src/format/mod.rs#L566 

Added line #L566 was not covered by tests
}

match (block_keypair, next_keypair) {
(KeyPair::Ed25519(_), KeyPair::Ed25519(_)) => {}
_ => {

Check warning on line 571 in biscuit-auth/src/format/mod.rs

View check run for this annotation

Codecov / codecov/patch

biscuit-auth/src/format/mod.rs#L570-L571 

Added lines #L570 - L571 were not covered by tests
return NON_ED25519_SIGNATURE_VERSION;
}
}

previous_blocks_sig_versions.max().unwrap_or(0)
}

#[cfg(test)]
mod tests {
use std::io::Read;

use crate::{
builder::Algorithm,
crypto::{ExternalSignature, Signature},
format::block_signature_version,
token::{DATALOG_3_1, DATALOG_3_3},
KeyPair,
};

#[test]
fn proto() {
// somehow when building under cargo-tarpaulin, OUT_DIR is not set
Expand All @@ -601,4 +613,88 @@ mod tests {
panic!();
}
}

#[test]
fn test_block_signature_version() {
assert_eq!(
block_signature_version(
&KeyPair::new(Algorithm::Ed25519),
&KeyPair::new(Algorithm::Ed25519),
&None,
&Some(DATALOG_3_1),
std::iter::empty()
),
0,
"ed25519 everywhere, authority block, no new datalog features"
);
assert_eq!(
block_signature_version(
&KeyPair::new(Algorithm::Secp256r1),
&KeyPair::new(Algorithm::Ed25519),
&None,
&Some(DATALOG_3_1),
std::iter::empty()
),
1,
"s256r1 root key, authority block, no new datalog features"
);
assert_eq!(
block_signature_version(
&KeyPair::new(Algorithm::Ed25519),
&KeyPair::new(Algorithm::Secp256r1),
&None,
&Some(DATALOG_3_1),
std::iter::empty()
),
1,
"s256r1 next key, authority block, no new datalog features"
);
assert_eq!(
block_signature_version(
&KeyPair::new(Algorithm::Secp256r1),
&KeyPair::new(Algorithm::Secp256r1),
&None,
&Some(DATALOG_3_1),
std::iter::empty()
),
1,
"s256r1 root & next key, authority block, no new datalog features"
);
assert_eq!(
block_signature_version(
&KeyPair::new(Algorithm::Ed25519),
&KeyPair::new(Algorithm::Ed25519),
&Some(ExternalSignature {
public_key: KeyPair::new(Algorithm::Ed25519).public(),
signature: Signature::from_vec(Vec::new())
}),
&Some(DATALOG_3_1),
std::iter::once(0)
),
1,
"ed25519 root & next key, third-party block, no new datalog features"
);
assert_eq!(
block_signature_version(
&KeyPair::new(Algorithm::Ed25519),
&KeyPair::new(Algorithm::Ed25519),
&None,
&Some(DATALOG_3_3),
std::iter::empty()
),
1,
"ed25519 root & next key, first-party block, new datalog features"
);
assert_eq!(
block_signature_version(
&KeyPair::new(Algorithm::Ed25519),
&KeyPair::new(Algorithm::Ed25519),
&None,
&Some(DATALOG_3_1),
std::iter::once(1)
),
1,
"ed25519 root & next key, first-party block, no new datalog features, previous v1 block"
);
}
}

0 comments on commit f58c393

Please sign in to comment.