Skip to content

Commit

Permalink
Backport fixes to stable branch (#300)
Browse files Browse the repository at this point in the history 
Backports #289, #290, and #291
  • Loading branch information
@mkeeter
mkeeter authored Oct 15, 2024
1 parent bea6fe4 commit 291671c
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 44 deletions.
38 changes: 25 additions & 13 deletions ssh-key/src/authorized_keys.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const COMMENT_DELIMITER: char = '#';
/// the key).
pub struct AuthorizedKeys<'a> {
/// Lines of the file being iterated over
lines: core::str::Lines<'a>,
lines: str::Lines<'a>,
}

impl<'a> AuthorizedKeys<'a> {
Expand Down Expand Up @@ -137,25 +137,37 @@ impl str::FromStr for Entry {
type Err = Error;

fn from_str(line: &str) -> Result<Self> {
// TODO(tarcieri): more liberal whitespace handling?
match line.matches(' ').count() {
1..=2 => Ok(Self {
#[cfg(feature = "alloc")]
config_opts: Default::default(),
public_key: line.parse()?,
}),
3 => line
.split_once(' ')
.map(|(config_opts_str, public_key_str)| {
ConfigOptsIter(config_opts_str).validate()?;

Ok(Self {
3.. => {
// Having >= 3 spaces is ambiguous: it's either a key preceded
// by options, or a key with spaces in its comment. We'll try
// parsing as a single key first, then fall back to parsing as
// option + key.
match line.parse() {
Ok(public_key) => Ok(Self {
#[cfg(feature = "alloc")]
config_opts: ConfigOpts(config_opts_str.to_string()),
public_key: public_key_str.parse()?,
})
})
.ok_or(Error::FormatEncoding)?,
config_opts: Default::default(),
public_key,
}),
Err(_) => line
.split_once(' ')
.map(|(config_opts_str, public_key_str)| {
ConfigOptsIter(config_opts_str).validate()?;

Ok(Self {
#[cfg(feature = "alloc")]
config_opts: ConfigOpts(config_opts_str.to_string()),
public_key: public_key_str.parse()?,
})
})
.ok_or(Error::FormatEncoding)?,
}
}
_ => Err(Error::FormatEncoding),
}
}
Expand Down
2 changes: 1 addition & 1 deletion ssh-key/src/known_hosts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const MAGIC_HASH_PREFIX: &str = "|1|";
/// the key).
pub struct KnownHosts<'a> {
/// Lines of the file being iterated over
lines: core::str::Lines<'a>,
lines: str::Lines<'a>,
}

impl<'a> KnownHosts<'a> {
Expand Down
91 changes: 62 additions & 29 deletions ssh-key/src/signature.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use crate::{private::Ed25519Keypair, public::Ed25519PublicKey};
use {
crate::{private::DsaKeypair, public::DsaPublicKey},
bigint::BigUint,
core::iter,
sha1::Sha1,
signature::{DigestSigner, DigestVerifier},
};
Expand All @@ -24,6 +23,9 @@ use crate::{
public::EcdsaPublicKey,
};

#[cfg(any(feature = "dsa", feature = "p256", feature = "p384", feature = "p521"))]
use core::iter;

#[cfg(feature = "rsa")]
use {
crate::{private::RsaKeypair, public::RsaPublicKey, HashAlg},
Expand Down Expand Up @@ -151,7 +153,7 @@ fn ecdsa_sig_size(data: &Vec<u8>, curve: EcdsaCurve, sk_trailer: bool) -> Result
for _ in 0..2 {
let component = Mpint::decode(reader)?;

if component.as_positive_bytes().ok_or(Error::Crypto)?.len() != curve.field_size() {
if component.as_positive_bytes().ok_or(Error::Crypto)?.len() > curve.field_size() {
return Err(encoding::Error::Length.into());
}
}
Expand Down Expand Up @@ -519,6 +521,17 @@ impl_signature_for_curve!(p256, "p256", NistP256, 32);
impl_signature_for_curve!(p384, "p384", NistP384, 48);
impl_signature_for_curve!(p521, "p521", NistP521, 66);

/// Build a generic sized object from a `u8` iterator, with leading zero padding
#[cfg(any(feature = "p256", feature = "p384", feature = "p521"))]
fn zero_pad_field_bytes<B: FromIterator<u8> + Copy>(m: Mpint) -> Option<B> {
use core::mem::size_of;

let bytes = m.as_positive_bytes()?;
size_of::<B>()
.checked_sub(bytes.len())
.map(|i| B::from_iter(iter::repeat(0u8).take(i).chain(bytes.iter().cloned())))
}

#[cfg(feature = "p256")]
impl TryFrom<&Signature> for p256::ecdsa::Signature {
type Error = Error;
Expand All @@ -534,19 +547,15 @@ impl TryFrom<&Signature> for p256::ecdsa::Signature {
}
#[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),
)?)
}
match (
zero_pad_field_bytes::<p256::FieldBytes>(r),
zero_pad_field_bytes::<p256::FieldBytes>(s),
) {
(Some(r), Some(s)) => Ok(p256::ecdsa::Signature::from_scalars(r, s)?),
_ => Err(Error::Crypto),
}
}
Expand All @@ -556,8 +565,6 @@ impl TryFrom<&Signature> for p384::ecdsa::Signature {
type Error = Error;

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

match signature.algorithm {
Algorithm::Ecdsa {
curve: EcdsaCurve::NistP384,
Expand All @@ -566,13 +573,11 @@ impl TryFrom<&Signature> for p384::ecdsa::Signature {
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(p384::ecdsa::Signature::from_scalars(
*p384::FieldBytes::from_slice(r),
*p384::FieldBytes::from_slice(s),
)?)
}
match (
zero_pad_field_bytes::<p384::FieldBytes>(r),
zero_pad_field_bytes::<p384::FieldBytes>(s),
) {
(Some(r), Some(s)) => Ok(p384::ecdsa::Signature::from_scalars(r, s)?),
_ => Err(Error::Crypto),
}
}
Expand All @@ -586,8 +591,6 @@ impl TryFrom<&Signature> for p521::ecdsa::Signature {
type Error = Error;

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

match signature.algorithm {
Algorithm::Ecdsa {
curve: EcdsaCurve::NistP521,
Expand All @@ -596,13 +599,11 @@ impl TryFrom<&Signature> for p521::ecdsa::Signature {
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(p521::ecdsa::Signature::from_scalars(
*p521::FieldBytes::from_slice(r),
*p521::FieldBytes::from_slice(s),
)?)
}
match (
zero_pad_field_bytes::<p521::FieldBytes>(r),
zero_pad_field_bytes::<p521::FieldBytes>(s),
) {
(Some(r), Some(s)) => Ok(p521::ecdsa::Signature::from_scalars(r, s)?),
_ => Err(Error::Crypto),
}
}
Expand Down Expand Up @@ -712,6 +713,9 @@ mod tests {
signature::{Signer, Verifier},
};

#[cfg(feature = "p256")]
use super::{zero_pad_field_bytes, Mpint};

const DSA_SIGNATURE: &[u8] = &hex!("000000077373682d6473730000002866725bf3c56100e975e21fff28a60f73717534d285ea3e1beefc2891f7189d00bd4d94627e84c55c");
const ECDSA_SHA2_P256_SIGNATURE: &[u8] = &hex!("0000001365636473612d736861322d6e6973747032353600000048000000201298ab320720a32139cda8a40c97a13dc54ce032ea3c6f09ea9e87501e48fa1d0000002046e4ac697a6424a9870b9ef04ca1182cd741965f989bd1f1f4a26fd83cf70348");
const ED25519_SIGNATURE: &[u8] = &hex!("0000000b7373682d65643235353139000000403d6b9906b76875aef1e7b2f1e02078a94f439aebb9a4734da1a851a81e22ce0199bbf820387a8de9c834c9c3cc778d9972dcbe70f68d53cc6bc9e26b02b46d04");
Expand All @@ -729,6 +733,35 @@ mod tests {
let _ssh_signature = Signature::try_from(p256_signature).unwrap();
}

#[cfg(feature = "p256")]
#[test]
fn zero_pad_field_bytes_p256() {
let i = Mpint::from_bytes(&hex!(
"1122334455667788112233445566778811223344556677881122334455667788"
))
.unwrap();
let fb = zero_pad_field_bytes::<p256::FieldBytes>(i);
assert!(fb.is_some());

// too long
let i = Mpint::from_bytes(&hex!(
"991122334455667788112233445566778811223344556677881122334455667788"
))
.unwrap();
let fb = zero_pad_field_bytes::<p256::FieldBytes>(i);
assert!(fb.is_none());

// short is okay
let i = Mpint::from_bytes(&hex!(
"22334455667788112233445566778811223344556677881122334455667788"
))
.unwrap();
let fb = zero_pad_field_bytes::<p256::FieldBytes>(i)
.expect("failed to build FieldBytes from short hex string");
assert_eq!(fb[0], 0x00);
assert_eq!(fb[1], 0x22);
}

#[test]
fn decode_dsa() {
let signature = Signature::try_from(DSA_SIGNATURE).unwrap();
Expand Down
8 changes: 7 additions & 1 deletion ssh-key/tests/authorized_keys.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use ssh_key::AuthorizedKeys;
#[test]
fn read_example_file() {
let authorized_keys = AuthorizedKeys::read_file("./tests/examples/authorized_keys").unwrap();
assert_eq!(authorized_keys.len(), 4);
assert_eq!(authorized_keys.len(), 5);

assert_eq!(authorized_keys[0].config_opts().to_string(), "");
assert_eq!(
Expand Down Expand Up @@ -46,4 +46,10 @@ fn read_example_file() {
authorized_keys[3].public_key().comment(),
"[email protected]"
);

assert_eq!(authorized_keys[4].public_key().to_string(), "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBN76zuqnjypL54/w4763l7q1Sn3IBYHptJ5wcYfEWkzeNTvpexr05Z18m2yPT2SWRd1JJ8Aj5TYidG9MdSS5J78= hello world this is a long comment");
assert_eq!(
authorized_keys[4].public_key().comment(),
"hello world this is a long comment"
);
}
3 changes: 3 additions & 0 deletions ssh-key/tests/examples/authorized_keys
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,6 @@ environment="PATH=/bin:/usr/bin" ssh-dss AAAAB3NzaC1kc3MAAACBANw9iSUO2UYhFMssjUg

# Public key which can only be used from certain source addresses and disallows X11 forwarding
from="10.0.0.?,*.example.com",no-X11-forwarding ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQC0WRHtxuxefSJhpIxGq4ibGFgwYnESPm8C3JFM88A1JJLoprenklrd7VJ+VH3Ov/bQwZwLyRU5dRmfR/SWTtIPWs7tToJVayKKDB+/qoXmM5ui/0CU2U4rCdQ6PdaCJdC7yFgpPL8WexjWN06+eSIKYz1AAXbx9rRv1iasslK/KUqtsqzVliagI6jl7FPO2GhRZMcso6LsZGgSxuYf/Lp0D/FcBU8GkeOo1Sx5xEt8H8bJcErtCe4Blb8JxcW6EXO3sReb4z+zcR07gumPgFITZ6hDA8sSNuvo/AlWg0IKTeZSwHHVknWdQqDJ0uczE837caBxyTZllDNIGkBjCIIOFzuTT76HfYc/7CTTGk07uaNkUFXKN79xDiFOX8JQ1ZZMZvGOTwWjuT9CqgdTvQRORbRWwOYv3MH8re9ykw3Ip6lrPifY7s6hOaAKry/nkGPMt40m1TdiW98MTIpooE7W+WXu96ax2l2OJvxX8QR7l+LFlKnkIEEJd/ItF1G22UmOjkVwNASTwza/hlY+8DoVvEmwum/nMgH2TwQT3bTQzF9s9DOJkH4d8p4Mw4gEDjNx0EgUFA91ysCAeUMQQyIvuR8HXXa+VcvhOOO5mmBcVhxJ3qUOJTyDBsT0932Zb4mNtkxdigoVxu+iiwk0vwtvKwGVDYdyMP5EAQeEIP1t0w== [email protected]

# Public key with a comment that contains multiple spaces
ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBN76zuqnjypL54/w4763l7q1Sn3IBYHptJ5wcYfEWkzeNTvpexr05Z18m2yPT2SWRd1JJ8Aj5TYidG9MdSS5J78= hello world this is a long comment

0 comments on commit 291671c

Please sign in to comment.