Skip to content

Commit

Permalink
ssh-key: Implement SkEcdsaSha2NistP256 signature validation (#169)
Browse files Browse the repository at this point in the history 
This change implements validation of of sk-ecdsa signatures as
emitted by the openssh ssh-agent. The signature format is
analogous to the sk-ed25519 sinature format already supported
by ssh-key
  • Loading branch information
@nresare
nresare authored Nov 18, 2023
1 parent 776d638 commit fc0195b
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 57 deletions.
149 changes: 92 additions & 57 deletions ssh-key/src/signature.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Signatures (e.g. CA signatures over SSH certificates)

use crate::{private, public, Algorithm, Error, Mpint, PrivateKey, PublicKey, Result};
use crate::{private, public, Algorithm, EcdsaCurve, Error, Mpint, PrivateKey, PublicKey, Result};
use alloc::vec::Vec;
use core::fmt;
use encoding::{CheckedSum, Decode, Encode, Reader, Writer};
Expand All @@ -21,7 +21,6 @@ use {
use crate::{
private::{EcdsaKeypair, EcdsaPrivateKey},
public::EcdsaPublicKey,
EcdsaCurve,
};

#[cfg(feature = "rsa")]
Expand All @@ -30,16 +29,16 @@ use {
sha2::Sha512,
};

#[cfg(any(feature = "ed25519", feature = "rsa"))]
#[cfg(any(feature = "ed25519", feature = "rsa", feature = "p256"))]
use sha2::Sha256;

#[cfg(any(feature = "dsa", feature = "ed25519"))]
#[cfg(any(feature = "dsa", feature = "ed25519", feature = "p256"))]
use sha2::Digest;

const DSA_SIGNATURE_SIZE: usize = 40;
const ED25519_SIGNATURE_SIZE: usize = 64;
const SK_ED25519_SIGNATURE_TRAILER_SIZE: usize = 5; // flags(u8), counter(u32)
const SK_ED25519_SIGNATURE_SIZE: usize = ED25519_SIGNATURE_SIZE + SK_ED25519_SIGNATURE_TRAILER_SIZE;
const SK_SIGNATURE_TRAILER_SIZE: usize = 5; // flags(u8), counter(u32)
const SK_ED25519_SIGNATURE_SIZE: usize = ED25519_SIGNATURE_SIZE + SK_SIGNATURE_TRAILER_SIZE;

/// Trait for signing keys which produce a [`Signature`].
///
Expand Down Expand Up @@ -105,23 +104,10 @@ impl Signature {
// Validate signature is well-formed per OpensSH encoding
match algorithm {
Algorithm::Dsa if data.len() == DSA_SIGNATURE_SIZE => (),
Algorithm::Ecdsa { curve } => {
let reader = &mut data.as_slice();

for _ in 0..2 {
let component = Mpint::decode(reader)?;

if component.as_positive_bytes().ok_or(Error::Crypto)?.len()
!= curve.field_size()
{
return Err(encoding::Error::Length.into());
}
}

reader.finish(())?;
}
Algorithm::Ecdsa { curve } => ecdsa_sig_size(&data, curve, false)?,
Algorithm::Ed25519 if data.len() == ED25519_SIGNATURE_SIZE => (),
Algorithm::SkEd25519 if data.len() == SK_ED25519_SIGNATURE_SIZE => (),
Algorithm::SkEcdsaSha2NistP256 => ecdsa_sig_size(&data, EcdsaCurve::NistP256, true)?,
Algorithm::Rsa { hash: Some(_) } => (),
_ => return Err(encoding::Error::Length.into()),
}
Expand Down Expand Up @@ -155,6 +141,26 @@ impl Signature {
}
}

/// Returns Ok() if data holds an ecdsa signature with components of appropriate size
/// according to curve.
fn ecdsa_sig_size(data: &Vec<u8>, curve: EcdsaCurve, sk_trailer: bool) -> Result<()> {
let reader = &mut data.as_slice();

for _ in 0..2 {
let component = Mpint::decode(reader)?;

if component.as_positive_bytes().ok_or(Error::Crypto)?.len() != curve.field_size() {
return Err(encoding::Error::Length.into());
}
}
if sk_trailer {
reader.drain(SK_SIGNATURE_TRAILER_SIZE)?;
}
reader
.finish(())
.map_err(|_| encoding::Error::Length.into())
}

impl AsRef<[u8]> for Signature {
fn as_ref(&self) -> &[u8] {
self.as_bytes()
Expand All @@ -168,14 +174,13 @@ impl Decode for Signature {
let algorithm = Algorithm::decode(reader)?;
let mut data = Vec::decode(reader)?;

if algorithm == Algorithm::SkEd25519 {
if algorithm == Algorithm::SkEd25519 || algorithm == Algorithm::SkEcdsaSha2NistP256 {
let flags = u8::decode(reader)?;
let counter = u32::decode(reader)?;

data.push(flags);
data.extend(counter.to_be_bytes());
}

Self::new(algorithm, data)
}
}
Expand All @@ -200,7 +205,7 @@ impl Encode for Signature {
let signature_length = self
.as_bytes()
.len()
.checked_sub(SK_ED25519_SIGNATURE_TRAILER_SIZE)
.checked_sub(SK_SIGNATURE_TRAILER_SIZE)
.ok_or(encoding::Error::Length)?;
self.as_bytes()[..signature_length].encode(writer)?;
writer.write(&self.as_bytes()[signature_length..])?;
Expand Down Expand Up @@ -304,6 +309,8 @@ impl Verifier<Signature> for public::KeyData {
Self::Ed25519(pk) => pk.verify(message, signature),
#[cfg(feature = "ed25519")]
Self::SkEd25519(pk) => pk.verify(message, signature),
#[cfg(feature = "p256")]
Self::SkEcdsaSha2NistP256(pk) => pk.verify(message, signature),
#[cfg(feature = "rsa")]
Self::Rsa(pk) => pk.verify(message, signature),
#[allow(unreachable_patterns)]
Expand Down Expand Up @@ -406,26 +413,52 @@ impl Verifier<Signature> for Ed25519PublicKey {
#[cfg(feature = "ed25519")]
impl Verifier<Signature> for public::SkEd25519 {
fn verify(&self, message: &[u8], signature: &Signature) -> signature::Result<()> {
let signature_len = signature
.as_bytes()
.len()
.checked_sub(SK_ED25519_SIGNATURE_TRAILER_SIZE)
.ok_or(Error::Encoding(encoding::Error::Length))?;
let signature_bytes = &signature.as_bytes()[..signature_len];
let flags_and_counter = &signature.as_bytes()[signature_len..];

#[allow(clippy::arithmetic_side_effects)]
let mut signed_data =
Vec::with_capacity((2 * Sha256::output_size()) + SK_ED25519_SIGNATURE_TRAILER_SIZE);
signed_data.extend(Sha256::digest(self.application()));
signed_data.extend(flags_and_counter);
signed_data.extend(Sha256::digest(message));
let (signature, flags_and_counter) = split_sk_signature(signature)?;
let signature = ed25519_dalek::Signature::try_from(signature)?;
ed25519_dalek::VerifyingKey::try_from(self.public_key())?.verify(
&make_sk_signed_data(self.application(), flags_and_counter, message),
&signature,
)
}
}

let signature = ed25519_dalek::Signature::try_from(signature_bytes)?;
ed25519_dalek::VerifyingKey::try_from(self.public_key())?.verify(&signed_data, &signature)
#[cfg(feature = "p256")]
impl Verifier<Signature> for public::SkEcdsaSha2NistP256 {
fn verify(&self, message: &[u8], signature: &Signature) -> signature::Result<()> {
let (signature_bytes, flags_and_counter) = split_sk_signature(signature)?;
let signature = p256_signature_from_openssh_bytes(signature_bytes)?;
p256::ecdsa::VerifyingKey::from_encoded_point(self.ec_point())?.verify(
&make_sk_signed_data(self.application(), flags_and_counter, message),
&signature,
)
}
}

#[cfg(any(feature = "p256", feature = "ed25519"))]
fn make_sk_signed_data(application: &str, flags_and_counter: &[u8], message: &[u8]) -> Vec<u8> {
const SHA256_OUTPUT_LENGTH: usize = 32;
const SIGNED_SK_DATA_LENGTH: usize = 2 * SHA256_OUTPUT_LENGTH + SK_SIGNATURE_TRAILER_SIZE;

let mut signed_data = Vec::with_capacity(SIGNED_SK_DATA_LENGTH);
signed_data.extend(Sha256::digest(application));
signed_data.extend(flags_and_counter);
signed_data.extend(Sha256::digest(message));
signed_data
}

#[cfg(any(feature = "p256", feature = "ed25519"))]
fn split_sk_signature(signature: &Signature) -> Result<(&[u8], &[u8])> {
let signature_bytes = signature.as_bytes();
let signature_len = signature_bytes
.len()
.checked_sub(SK_SIGNATURE_TRAILER_SIZE)
.ok_or(Error::Encoding(encoding::Error::Length))?;
Ok((
&signature_bytes[..signature_len],
&signature_bytes[signature_len..],
))
}

#[cfg(feature = "p256")]
impl TryFrom<p256::ecdsa::Signature> for Signature {
type Error = Error;
Expand Down Expand Up @@ -511,30 +544,32 @@ impl TryFrom<&Signature> for p256::ecdsa::Signature {
type Error = Error;

fn try_from(signature: &Signature) -> Result<p256::ecdsa::Signature> {
const FIELD_SIZE: usize = 32;

match signature.algorithm {
Algorithm::Ecdsa {
curve: EcdsaCurve::NistP256,
} => {
let reader = &mut signature.as_bytes();
let r = Mpint::decode(reader)?;
let s = Mpint::decode(reader)?;

match (r.as_positive_bytes(), s.as_positive_bytes()) {
(Some(r), Some(s)) if r.len() == FIELD_SIZE && s.len() == FIELD_SIZE => {
Ok(p256::ecdsa::Signature::from_scalars(
*p256::FieldBytes::from_slice(r),
*p256::FieldBytes::from_slice(s),
)?)
}
_ => Err(Error::Crypto),
}
}
} => p256_signature_from_openssh_bytes(signature.as_bytes()),
_ => Err(signature.algorithm.clone().unsupported_error()),
}
}
}
#[cfg(feature = "p256")]
fn p256_signature_from_openssh_bytes(mut signature_bytes: &[u8]) -> Result<p256::ecdsa::Signature> {
const FIELD_SIZE: usize = 32;

let reader = &mut signature_bytes;
let r = Mpint::decode(reader)?;
let s = Mpint::decode(reader)?;

match (r.as_positive_bytes(), s.as_positive_bytes()) {
(Some(r), Some(s)) if r.len() == FIELD_SIZE && s.len() == FIELD_SIZE => {
Ok(p256::ecdsa::Signature::from_scalars(
*p256::FieldBytes::from_slice(r),
*p256::FieldBytes::from_slice(s),
)?)
}
_ => Err(Error::Crypto),
}
}

#[cfg(feature = "p384")]
impl TryFrom<&Signature> for p384::ecdsa::Signature {
Expand Down
1 change: 1 addition & 0 deletions ssh-key/tests/examples/id_sk_ecdsa_p256_2.pub
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[email protected] AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBNdo6XfhTK080uz5UbGyOcNMo+R3nPXMBxurwH2M1bDtQYbDT6qBE7EdQGkcy/EJDXbzT0KlU9rROjcX+JsgtGAAAAAEc3NoOg== [email protected]
27 changes: 27 additions & 0 deletions ssh-key/tests/sshsig.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ const ED25519_SIGNATURE_BYTES: [u8; 64] = hex!(
/// SkEd25519 OpenSSH-formatted public key.
const SK_ED25519_PUBLIC_KEY: &str = include_str!("examples/id_sk_ed25519_2.pub");

#[cfg(feature = "p256")]
const SK_ECDSA_P256_PUBLIC_KEY: &str = include_str!("examples/id_sk_ecdsa_p256_2.pub");

/// `sshsig`-encoded signature.
const SK_ED25519_SIGNATURE: &str = include_str!("examples/sshsig_sk_ed25519");

Expand All @@ -76,6 +79,14 @@ const MSG_EXAMPLE: &[u8] = b"testing";
/// Example domain/namespace used for the message.
const NAMESPACE_EXAMPLE: &str = "example";

#[cfg(feature = "p256")]
const SK_ECDSA_SIGNATURE_OPENSSH_WIRE: [u8; 120] = hex!(
"00000022736b2d65636473612d736861322d6e69737470323536406f70656e73"
"73682e636f6d00000049000000201b35a1c6469a43a3d09d490d6ff8ca1bc248"
"6a2edeb8aa7d119e4c70b9c1811000000021009724a2a4449a90357485ed1df0"
"161274d20083342b02756794bc3f068fcdc15e01000000ec"
);

/// An ssh-agent signature response signing MSG_EXAMPLE with DSA_PRIVATE_KEY
#[cfg(feature = "dsa")]
const DSA_SIGNATURE_OPENSSH_WIRE: [u8; 55] = hex!(
Expand Down Expand Up @@ -176,6 +187,22 @@ fn sign_dsa() {
);
}

#[test]
#[cfg(feature = "p256")]
fn verify_sk_ecdsa_openssh_wire_format() {
let signature = Signature::decode(&mut SK_ECDSA_SIGNATURE_OPENSSH_WIRE.as_ref()).unwrap();
let verifying_key = SK_ECDSA_P256_PUBLIC_KEY.parse::<PublicKey>().unwrap();
verifying_key
.key_data()
.verify(MSG_EXAMPLE, &signature)
.expect("failed to validate valid signature");

verifying_key
.key_data()
.verify(b"Bogus!", &signature)
.expect_err("good signature from bogus data");
}

#[test]
#[cfg(feature = "dsa")]
fn verify_dsa_openssh_wire_format() {
Expand Down

0 comments on commit fc0195b

Please sign in to comment.