Skip to content

Commit

Permalink
Improve misuse-resistance by removing public-key as public API parame…
Browse files Browse the repository at this point in the history
…ter in `sign()` operations (#65)

* Add test for Double PK Attack on V2/V4 with Ed25519

* WiP

* v2/v4: AsymmetricSecret holds seed||pk and uses only this to sign

* v3: Make sing() API consistent with V3/V4

* v2/v4: Provide TryFrom<AsymmetricSecretKey> for AsymmetricPublicKey now that secret conatins both seed and pk

* v2/v4: PASERK is now implemented on AsymmetricSecretKey instead of keypair

* Revert to normal default features

* Bump version

* Update changelog

* Update fuzzing targets

* nit
  • Loading branch information
brycx authored Jun 20, 2022
1 parent 5a64c6a commit 829402f
Show file tree
Hide file tree
Showing 11 changed files with 188 additions and 194 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
### 0.6.0

__Date:__ June 20, 2022.

__Changelog:__
- PASERK operations are now implemented for `AsymmetricSecretKey<V2>` and `AsymmetricSecretKey<V4>` instead of `AsymmetricKeyPair<V2>` and `AsymmetricKeyPair<V4>`, respectively
- All `sign()` operations with public tokens now take only the secret key
- `V2` and `V4` token's `AsymmetricSecretKey<>` are now defined to contain both the Ed25519 secret seed and the public key (see https://github.com/MystenLabs/ed25519-unsafe-libs)
- `TryFrom<AsymmetricSecretKey<>> for AsymmetricPublicKey<>` is now provided for `V2` and `V4` as well


### 0.5.0

__Date:__ June 4, 2022.
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pasetors"
version = "0.5.0" # Update html_root_url in lib.rs along with this.
version = "0.6.0" # Update html_root_url in lib.rs along with this.
authors = ["brycx <[email protected]>"]
edition = "2018"
description = "PASETO: Platform-Agnostic Security Tokens (in Rust)"
Expand Down
4 changes: 2 additions & 2 deletions fuzz/fuzz_targets/fuzz_paserk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use pasetors::{version2::V2, version3::V3, version4::V4};
fuzz_target!(|data: &[u8]| {
let data: String = String::from_utf8_lossy(data).into();

if let Ok(valid_paserk) = AsymmetricKeyPair::<V2>::try_from(data.as_str()) {
if let Ok(valid_paserk) = AsymmetricSecretKey::<V2>::try_from(data.as_str()) {
let mut buf = String::new();
valid_paserk.fmt(&mut buf).unwrap();
assert_eq!(&data, &buf);
Expand All @@ -22,7 +22,7 @@ fuzz_target!(|data: &[u8]| {
assert_eq!(&data, &buf);
let _ = Id::from(&valid_paserk);
}
if let Ok(valid_paserk) = AsymmetricKeyPair::<V4>::try_from(data.as_str()) {
if let Ok(valid_paserk) = AsymmetricSecretKey::<V4>::try_from(data.as_str()) {
let mut buf = String::new();
valid_paserk.fmt(&mut buf).unwrap();
assert_eq!(&data, &buf);
Expand Down
14 changes: 7 additions & 7 deletions fuzz/fuzz_targets/fuzz_pasetors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ fn fuzztest_v2(data: &[u8], csprng: &mut ChaCha20Rng) {
csprng.fill_bytes(&mut seed_bytes);
let seed = Seed::from_slice(&seed_bytes).unwrap();
let keypair: KeyPair = KeyPair::from_seed(seed);
let sk = AsymmetricSecretKey::<V2>::from(&keypair.sk[..32]).unwrap();
let sk = AsymmetricSecretKey::<V2>::from(keypair.sk.as_ref()).unwrap();
let pk = AsymmetricPublicKey::<V2>::from(keypair.pk.as_ref()).unwrap();
let mut key = [0u8; 32];
csprng.fill_bytes(&mut key);
Expand All @@ -39,7 +39,7 @@ fn fuzztest_v2(data: &[u8], csprng: &mut ChaCha20Rng) {
}

let public_token = UntrustedToken::<Public, V2>::try_from(
&version2::PublicToken::sign(&sk, &pk, message.as_bytes(), None).unwrap(),
&version2::PublicToken::sign(&sk, message.as_bytes(), None).unwrap(),
)
.unwrap();
match version2::PublicToken::verify(&pk, &public_token, None) {
Expand Down Expand Up @@ -87,7 +87,7 @@ fn fuzztest_v3(data: &[u8]) {
}

let public_token = UntrustedToken::<Public, V3>::try_from(
&version3::PublicToken::sign(&kp.secret, &kp.public, message.as_bytes(), None, None)
&version3::PublicToken::sign(&kp.secret, message.as_bytes(), None, None)
.unwrap(),
)
.unwrap();
Expand All @@ -106,7 +106,7 @@ fn fuzztest_v4(data: &[u8], csprng: &mut ChaCha20Rng) {
csprng.fill_bytes(&mut seed_bytes);
let seed = Seed::from_slice(&seed_bytes).unwrap();
let keypair: KeyPair = KeyPair::from_seed(seed);
let sk = AsymmetricSecretKey::<V4>::from(&keypair.sk[..32]).unwrap();
let sk = AsymmetricSecretKey::<V4>::from(keypair.sk.as_ref()).unwrap();
let pk = AsymmetricPublicKey::<V4>::from(keypair.pk.as_ref()).unwrap();
let mut key = [0u8; 32];
csprng.fill_bytes(&mut key);
Expand All @@ -124,7 +124,7 @@ fn fuzztest_v4(data: &[u8], csprng: &mut ChaCha20Rng) {
}

let public_token = UntrustedToken::<Public, V4>::try_from(
&version4::PublicToken::sign(&sk, &pk, message.as_bytes(), None, None).unwrap(),
&version4::PublicToken::sign(&sk, message.as_bytes(), None, None).unwrap(),
)
.unwrap();
match version4::PublicToken::verify(&pk, &public_token, None, None) {
Expand Down Expand Up @@ -161,7 +161,7 @@ fn fuzz_highlevel(data: &[u8], csprng: &mut ChaCha20Rng) {
csprng.fill_bytes(&mut seed_bytes);
let seed = Seed::from_slice(&seed_bytes).unwrap();
let keypair: KeyPair = KeyPair::from_seed(seed);
let sk = AsymmetricSecretKey::<V4>::from(&keypair.sk[..32]).unwrap();
let sk = AsymmetricSecretKey::<V4>::from(keypair.sk.as_ref()).unwrap();
let pk = AsymmetricPublicKey::<V4>::from(keypair.pk.as_ref()).unwrap();
let mut key = [0u8; 32];
csprng.fill_bytes(&mut key);
Expand All @@ -177,7 +177,7 @@ fn fuzz_highlevel(data: &[u8], csprng: &mut ChaCha20Rng) {
let validation_rules = ClaimsValidationRules::new();

let public_token = UntrustedToken::<Public, V4>::try_from(
&pasetors::public::sign(&sk, &pk, &claims, None, None).unwrap(),
&pasetors::public::sign(&sk, &claims, None, None).unwrap(),
)
.unwrap();
if let Ok(trusted_token) =
Expand Down
8 changes: 4 additions & 4 deletions src/footer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ mod tests {
let skv4 = SymmetricKey::<V4>::generate().unwrap();

let mut buf = String::new();
kpv2.fmt(&mut buf).unwrap();
kpv2.secret.fmt(&mut buf).unwrap();
assert!(footer.add_additional("wpk", &buf).is_err());
assert!(footer.add_additional("kid", &buf).is_err());
assert!(footer.add_additional("custom", &buf).is_err());
Expand All @@ -280,7 +280,7 @@ mod tests {
assert!(footer.add_additional("custom", &buf).is_err());

let mut buf = String::new();
kpv4.fmt(&mut buf).unwrap();
kpv4.secret.fmt(&mut buf).unwrap();
assert!(footer.add_additional("wpk", &buf).is_err());
assert!(footer.add_additional("kid", &buf).is_err());
assert!(footer.add_additional("custom", &buf).is_err());
Expand Down Expand Up @@ -322,7 +322,7 @@ mod tests {
let skv4 = SymmetricKey::<V4>::generate().unwrap();

let mut buf = String::new();
let paserk_id = Id::from(&kpv2);
let paserk_id = Id::from(&kpv2.secret);
paserk_id.fmt(&mut buf).unwrap();
assert!(footer.add_additional("kid", &buf).is_err());
assert!(footer.add_additional("custom", &buf).is_ok());
Expand Down Expand Up @@ -358,7 +358,7 @@ mod tests {
assert_eq!(footer.get_claim("kid").unwrap().as_str().unwrap(), buf);

let mut buf = String::new();
let paserk_id = Id::from(&kpv4);
let paserk_id = Id::from(&kpv4.secret);
paserk_id.fmt(&mut buf).unwrap();
assert!(footer.add_additional("kid", &buf).is_err());
assert!(footer.add_additional("custom", &buf).is_ok());
Expand Down
2 changes: 2 additions & 0 deletions src/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ impl<V: Version> PartialEq<SymmetricKey<V>> for SymmetricKey<V> {
}

/// An asymmetric secret key used for `.public` tokens, given a version `V`.
///
/// In case of Ed25519, which is used in V2 and V4, this is the seed concatenated with the public key.
pub struct AsymmetricSecretKey<V> {
pub(crate) bytes: Vec<u8>,
pub(crate) phantom: PhantomData<V>,
Expand Down
9 changes: 3 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
//!
//! // Generate the keys and sign the claims.
//! let kp = AsymmetricKeyPair::<V4>::generate()?;
//! let pub_token = public::sign(&kp.secret, &kp.public, &claims, None, Some(b"implicit assertion"))?;
//! let pub_token = public::sign(&kp.secret, &claims, None, Some(b"implicit assertion"))?;
//!
//! // Decide how we want to validate the claims after verifying the token itself.
//! // The default verifies the `nbf`, `iat` and `exp` claims. `nbf` and `iat` are always
Expand Down Expand Up @@ -141,7 +141,7 @@
//! footer.add_additional("custom_footer_claim", "custom_value")?;
//!
//! let mut claims = Claims::new()?;
//! let pub_token = public::sign(&kp.secret, &kp.public, &claims, Some(&footer), Some(b"implicit assertion"))?;
//! let pub_token = public::sign(&kp.secret, &claims, Some(&footer), Some(b"implicit assertion"))?;
//!
//! // If we receive a token that needs to be verified, we can still try to parse a Footer from it
//! // as long one was used during creation, if we don't know it beforehand.
Expand Down Expand Up @@ -183,7 +183,7 @@
unused_qualifications,
overflowing_literals
)]
#![doc(html_root_url = "https://docs.rs/pasetors/0.5.0")]
#![doc(html_root_url = "https://docs.rs/pasetors/0.6.0")]
#![cfg_attr(docsrs, feature(doc_cfg))]

#[macro_use]
Expand Down Expand Up @@ -246,22 +246,19 @@ pub mod public {
/// Create a public token using the latest PASETO version (v4).
pub fn sign(
secret_key: &AsymmetricSecretKey<V4>,
public_key: &AsymmetricPublicKey<V4>,
message: &Claims,
footer: Option<&Footer>,
implicit_assert: Option<&[u8]>,
) -> Result<String, Error> {
match footer {
Some(f) => crate::version4::PublicToken::sign(
secret_key,
public_key,
message.to_string()?.as_bytes(),
Some(f.to_string()?.as_bytes()),
implicit_assert,
),
None => crate::version4::PublicToken::sign(
secret_key,
public_key,
message.to_string()?.as_bytes(),
None,
implicit_assert,
Expand Down
87 changes: 31 additions & 56 deletions src/paserk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use crate::common::{decode_b64, encode_b64};
use crate::errors::Error;
use crate::keys::{AsymmetricKeyPair, AsymmetricPublicKey, AsymmetricSecretKey, SymmetricKey};
use crate::keys::{AsymmetricPublicKey, AsymmetricSecretKey, SymmetricKey};
use crate::version::private::Version;
use alloc::string::String;
use alloc::vec::Vec;
Expand Down Expand Up @@ -96,34 +96,20 @@ impl TryFrom<&str> for SymmetricKey<V4> {
}

#[cfg(feature = "v2")]
impl FormatAsPaserk for AsymmetricKeyPair<V2> {
impl FormatAsPaserk for AsymmetricSecretKey<V2> {
fn fmt(&self, write: &mut dyn Write) -> core::fmt::Result {
write.write_str("k2.secret.")?;

// See spec: "Here, Ed25519 secret key means the clamped 32-byte seed followed by the
// 32-byte public key, as used in the NaCl and libsodium APIs, rather than just the
// clamped 32-byte seed."
let mut buf = [0u8; V2::SECRET_KEY + V2::PUBLIC_KEY];
buf[..V2::SECRET_KEY].copy_from_slice(self.secret.as_bytes());
buf[V2::SECRET_KEY..].copy_from_slice(self.public.as_bytes());
write.write_str(&encode_b64(buf).map_err(|_| core::fmt::Error)?)?;
buf.iter_mut().zeroize();

Ok(())
write.write_str(&encode_b64(self.as_bytes()).map_err(|_| core::fmt::Error)?)
}
}

#[cfg(feature = "v2")]
impl TryFrom<&str> for AsymmetricKeyPair<V2> {
impl TryFrom<&str> for AsymmetricSecretKey<V2> {
type Error = Error;

fn try_from(value: &str) -> Result<Self, Self::Error> {
let mut buf =
validate_paserk_string(value, "k2", "secret", V2::SECRET_KEY + V2::PUBLIC_KEY)?;
let ret = Self {
secret: AsymmetricSecretKey::from(&buf[..V2::SECRET_KEY])?,
public: AsymmetricPublicKey::from(&buf[V2::SECRET_KEY..])?,
};
let mut buf = validate_paserk_string(value, "k2", "secret", V2::SECRET_KEY)?;
let ret = Self::from(&buf)?;
buf.iter_mut().zeroize();

Ok(ret)
Expand Down Expand Up @@ -154,31 +140,20 @@ impl TryFrom<&str> for AsymmetricSecretKey<V3> {
}

#[cfg(feature = "v4")]
impl FormatAsPaserk for AsymmetricKeyPair<V4> {
impl FormatAsPaserk for AsymmetricSecretKey<V4> {
fn fmt(&self, write: &mut dyn Write) -> core::fmt::Result {
write.write_str("k4.secret.")?;

let mut buf = [0u8; V4::SECRET_KEY + V4::PUBLIC_KEY];
buf[..V4::SECRET_KEY].copy_from_slice(self.secret.as_bytes());
buf[V4::SECRET_KEY..].copy_from_slice(self.public.as_bytes());
write.write_str(&encode_b64(buf).map_err(|_| core::fmt::Error)?)?;
buf.iter_mut().zeroize();

Ok(())
write.write_str(&encode_b64(self.as_bytes()).map_err(|_| core::fmt::Error)?)
}
}

#[cfg(feature = "v4")]
impl TryFrom<&str> for AsymmetricKeyPair<V4> {
impl TryFrom<&str> for AsymmetricSecretKey<V4> {
type Error = Error;

fn try_from(value: &str) -> Result<Self, Self::Error> {
let mut buf =
validate_paserk_string(value, "k4", "secret", V4::SECRET_KEY + V4::PUBLIC_KEY)?;
let ret = Self {
secret: AsymmetricSecretKey::from(&buf[..V4::SECRET_KEY])?,
public: AsymmetricPublicKey::from(&buf[V4::SECRET_KEY..])?,
};
let mut buf = validate_paserk_string(value, "k4", "secret", V4::SECRET_KEY)?;
let ret = Self::from(&buf)?;
buf.iter_mut().zeroize();

Ok(ret)
Expand Down Expand Up @@ -337,8 +312,8 @@ impl From<&SymmetricKey<V4>> for Id {
}

#[cfg(feature = "v2")]
impl From<&AsymmetricKeyPair<V2>> for Id {
fn from(key: &AsymmetricKeyPair<V2>) -> Self {
impl From<&AsymmetricSecretKey<V2>> for Id {
fn from(key: &AsymmetricSecretKey<V2>) -> Self {
let header = String::from("k2.sid.");
let mut hasher = blake2b::Blake2b::new(33).unwrap();
hasher.update(header.as_bytes()).unwrap();
Expand All @@ -354,8 +329,8 @@ impl From<&AsymmetricKeyPair<V2>> for Id {
}

#[cfg(feature = "v4")]
impl From<&AsymmetricKeyPair<V4>> for Id {
fn from(key: &AsymmetricKeyPair<V4>) -> Self {
impl From<&AsymmetricSecretKey<V4>> for Id {
fn from(key: &AsymmetricSecretKey<V4>) -> Self {
let header = String::from("k4.sid.");
let mut hasher = blake2b::Blake2b::new(33).unwrap();
hasher.update(header.as_bytes()).unwrap();
Expand Down Expand Up @@ -535,7 +510,7 @@ mod tests {

test_id_type!(
test_secret_k2_id,
AsymmetricKeyPair,
AsymmetricSecretKey,
V2,
"./test_vectors/PASERK/k2.sid.json"
);
Expand Down Expand Up @@ -563,7 +538,7 @@ mod tests {

test_paserk_type!(
test_secret_k2,
AsymmetricKeyPair,
AsymmetricSecretKey,
V2,
"./test_vectors/PASERK/k2.secret.json"
);
Expand Down Expand Up @@ -604,10 +579,10 @@ mod tests {
)
.is_err());

assert!(AsymmetricKeyPair::<V2>::try_from("k2.secret.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_ok());
assert!(AsymmetricKeyPair::<V2>::try_from("k4.secret.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
assert!(AsymmetricKeyPair::<V2>::try_from("k2.local.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
assert!(AsymmetricKeyPair::<V2>::try_from("k4.local.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
assert!(AsymmetricSecretKey::<V2>::try_from("k2.secret.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_ok());
assert!(AsymmetricSecretKey::<V2>::try_from("k4.secret.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
assert!(AsymmetricSecretKey::<V2>::try_from("k2.local.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
assert!(AsymmetricSecretKey::<V2>::try_from("k4.local.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
}
}

Expand Down Expand Up @@ -696,7 +671,7 @@ mod tests {

test_id_type!(
test_secret_k4_id,
AsymmetricKeyPair,
AsymmetricSecretKey,
V4,
"./test_vectors/PASERK/k4.sid.json"
);
Expand Down Expand Up @@ -724,7 +699,7 @@ mod tests {

test_paserk_type!(
test_secret_k4,
AsymmetricKeyPair,
AsymmetricSecretKey,
V4,
"./test_vectors/PASERK/k4.secret.json"
);
Expand Down Expand Up @@ -765,23 +740,23 @@ mod tests {
)
.is_err());

assert!(AsymmetricKeyPair::<V4>::try_from("k4.secret.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_ok());
assert!(AsymmetricKeyPair::<V4>::try_from("k2.secret.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
assert!(AsymmetricKeyPair::<V4>::try_from("k4.local.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
assert!(AsymmetricKeyPair::<V4>::try_from("k2.local.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
assert!(AsymmetricSecretKey::<V4>::try_from("k4.secret.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_ok());
assert!(AsymmetricSecretKey::<V4>::try_from("k2.secret.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
assert!(AsymmetricSecretKey::<V4>::try_from("k4.local.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
assert!(AsymmetricSecretKey::<V4>::try_from("k2.local.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7aie8zrakLWKjqNAqbw1zZTIVdx3iQ6Y6wEihi1naKQ").is_err());
}
}

#[test]
#[cfg(all(feature = "v4", feature = "v3"))]
fn test_partial_eq_id() {
use crate::keys::Generate;
use crate::keys::{AsymmetricKeyPair, Generate};

let kpv4 = AsymmetricKeyPair::<V4>::generate().unwrap();
assert_eq!(Id::from(&kpv4), Id::from(&kpv4));
assert_ne!(Id::from(&kpv4), Id::from(&kpv4.public));
assert_eq!(Id::from(&kpv4.secret), Id::from(&kpv4.secret));
assert_ne!(Id::from(&kpv4.secret), Id::from(&kpv4.public));
let kpv3 = AsymmetricKeyPair::<V3>::generate().unwrap();
assert_ne!(Id::from(&kpv4), Id::from(&kpv3.secret));
assert_ne!(Id::from(&kpv4.secret), Id::from(&kpv3.secret));
}

#[test]
Expand Down
Loading

0 comments on commit 829402f

Please sign in to comment.