Skip to content

Commit 291671c

Browse files
mkeetermkeeter
authored
Backport fixes to stable branch (#300)
Backports #289, #290, and #291
1 parent bea6fe4 commit 291671c

File tree

5 files changed

+98
-44
lines changed

5 files changed

+98
-44
lines changed

‎ssh-key/src/authorized_keys.rs‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const COMMENT_DELIMITER: char = '#';
3636
/// the key).
3737
pub struct AuthorizedKeys<'a> {
3838
/// Lines of the file being iterated over
39-
lines: core::str::Lines<'a>,
39+
lines: str::Lines<'a>,
4040
}
4141

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

139139
fn from_str(line: &str) -> Result<Self> {
140-
// TODO(tarcieri): more liberal whitespace handling?
141140
match line.matches(' ').count() {
142141
1..=2 => Ok(Self {
143142
#[cfg(feature = "alloc")]
144143
config_opts: Default::default(),
145144
public_key: line.parse()?,
146145
}),
147-
3 => line
148-
.split_once(' ')
149-
.map(|(config_opts_str, public_key_str)| {
150-
ConfigOptsIter(config_opts_str).validate()?;
151-
152-
Ok(Self {
146+
3.. => {
147+
// Having >= 3 spaces is ambiguous: it's either a key preceded
148+
// by options, or a key with spaces in its comment. We'll try
149+
// parsing as a single key first, then fall back to parsing as
150+
// option + key.
151+
match line.parse() {
152+
Ok(public_key) => Ok(Self {
153153
#[cfg(feature = "alloc")]
154-
config_opts: ConfigOpts(config_opts_str.to_string()),
155-
public_key: public_key_str.parse()?,
156-
})
157-
})
158-
.ok_or(Error::FormatEncoding)?,
154+
config_opts: Default::default(),
155+
public_key,
156+
}),
157+
Err(_) => line
158+
.split_once(' ')
159+
.map(|(config_opts_str, public_key_str)| {
160+
ConfigOptsIter(config_opts_str).validate()?;
161+
162+
Ok(Self {
163+
#[cfg(feature = "alloc")]
164+
config_opts: ConfigOpts(config_opts_str.to_string()),
165+
public_key: public_key_str.parse()?,
166+
})
167+
})
168+
.ok_or(Error::FormatEncoding)?,
169+
}
170+
}
159171
_ => Err(Error::FormatEncoding),
160172
}
161173
}

‎ssh-key/src/known_hosts.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ const MAGIC_HASH_PREFIX: &str = "|1|";
4646
/// the key).
4747
pub struct KnownHosts<'a> {
4848
/// Lines of the file being iterated over
49-
lines: core::str::Lines<'a>,
49+
lines: str::Lines<'a>,
5050
}
5151

5252
impl<'a> KnownHosts<'a> {

‎ssh-key/src/signature.rs‎

Lines changed: 62 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ use crate::{private::Ed25519Keypair, public::Ed25519PublicKey};
1313
use {
1414
crate::{private::DsaKeypair, public::DsaPublicKey},
1515
bigint::BigUint,
16-
core::iter,
1716
sha1::Sha1,
1817
signature::{DigestSigner, DigestVerifier},
1918
};
@@ -24,6 +23,9 @@ use crate::{
2423
public::EcdsaPublicKey,
2524
};
2625

26+
#[cfg(any(feature = "dsa", feature = "p256", feature = "p384", feature = "p521"))]
27+
use core::iter;
28+
2729
#[cfg(feature = "rsa")]
2830
use {
2931
crate::{private::RsaKeypair, public::RsaPublicKey, HashAlg},
@@ -151,7 +153,7 @@ fn ecdsa_sig_size(data: &Vec<u8>, curve: EcdsaCurve, sk_trailer: bool) -> Result
151153
for _ in 0..2 {
152154
let component = Mpint::decode(reader)?;
153155

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

524+
/// Build a generic sized object from a `u8` iterator, with leading zero padding
525+
#[cfg(any(feature = "p256", feature = "p384", feature = "p521"))]
526+
fn zero_pad_field_bytes<B: FromIterator<u8> + Copy>(m: Mpint) -> Option<B> {
527+
use core::mem::size_of;
528+
529+
let bytes = m.as_positive_bytes()?;
530+
size_of::<B>()
531+
.checked_sub(bytes.len())
532+
.map(|i| B::from_iter(iter::repeat(0u8).take(i).chain(bytes.iter().cloned())))
533+
}
534+
522535
#[cfg(feature = "p256")]
523536
impl TryFrom<&Signature> for p256::ecdsa::Signature {
524537
type Error = Error;
@@ -534,19 +547,15 @@ impl TryFrom<&Signature> for p256::ecdsa::Signature {
534547
}
535548
#[cfg(feature = "p256")]
536549
fn p256_signature_from_openssh_bytes(mut signature_bytes: &[u8]) -> Result<p256::ecdsa::Signature> {
537-
const FIELD_SIZE: usize = 32;
538-
539550
let reader = &mut signature_bytes;
540551
let r = Mpint::decode(reader)?;
541552
let s = Mpint::decode(reader)?;
542553

543-
match (r.as_positive_bytes(), s.as_positive_bytes()) {
544-
(Some(r), Some(s)) if r.len() == FIELD_SIZE && s.len() == FIELD_SIZE => {
545-
Ok(p256::ecdsa::Signature::from_scalars(
546-
*p256::FieldBytes::from_slice(r),
547-
*p256::FieldBytes::from_slice(s),
548-
)?)
549-
}
554+
match (
555+
zero_pad_field_bytes::<p256::FieldBytes>(r),
556+
zero_pad_field_bytes::<p256::FieldBytes>(s),
557+
) {
558+
(Some(r), Some(s)) => Ok(p256::ecdsa::Signature::from_scalars(r, s)?),
550559
_ => Err(Error::Crypto),
551560
}
552561
}
@@ -556,8 +565,6 @@ impl TryFrom<&Signature> for p384::ecdsa::Signature {
556565
type Error = Error;
557566

558567
fn try_from(signature: &Signature) -> Result<p384::ecdsa::Signature> {
559-
const FIELD_SIZE: usize = 48;
560-
561568
match signature.algorithm {
562569
Algorithm::Ecdsa {
563570
curve: EcdsaCurve::NistP384,
@@ -566,13 +573,11 @@ impl TryFrom<&Signature> for p384::ecdsa::Signature {
566573
let r = Mpint::decode(reader)?;
567574
let s = Mpint::decode(reader)?;
568575

569-
match (r.as_positive_bytes(), s.as_positive_bytes()) {
570-
(Some(r), Some(s)) if r.len() == FIELD_SIZE && s.len() == FIELD_SIZE => {
571-
Ok(p384::ecdsa::Signature::from_scalars(
572-
*p384::FieldBytes::from_slice(r),
573-
*p384::FieldBytes::from_slice(s),
574-
)?)
575-
}
576+
match (
577+
zero_pad_field_bytes::<p384::FieldBytes>(r),
578+
zero_pad_field_bytes::<p384::FieldBytes>(s),
579+
) {
580+
(Some(r), Some(s)) => Ok(p384::ecdsa::Signature::from_scalars(r, s)?),
576581
_ => Err(Error::Crypto),
577582
}
578583
}
@@ -586,8 +591,6 @@ impl TryFrom<&Signature> for p521::ecdsa::Signature {
586591
type Error = Error;
587592

588593
fn try_from(signature: &Signature) -> Result<p521::ecdsa::Signature> {
589-
const FIELD_SIZE: usize = 66;
590-
591594
match signature.algorithm {
592595
Algorithm::Ecdsa {
593596
curve: EcdsaCurve::NistP521,
@@ -596,13 +599,11 @@ impl TryFrom<&Signature> for p521::ecdsa::Signature {
596599
let r = Mpint::decode(reader)?;
597600
let s = Mpint::decode(reader)?;
598601

599-
match (r.as_positive_bytes(), s.as_positive_bytes()) {
600-
(Some(r), Some(s)) if r.len() == FIELD_SIZE && s.len() == FIELD_SIZE => {
601-
Ok(p521::ecdsa::Signature::from_scalars(
602-
*p521::FieldBytes::from_slice(r),
603-
*p521::FieldBytes::from_slice(s),
604-
)?)
605-
}
602+
match (
603+
zero_pad_field_bytes::<p521::FieldBytes>(r),
604+
zero_pad_field_bytes::<p521::FieldBytes>(s),
605+
) {
606+
(Some(r), Some(s)) => Ok(p521::ecdsa::Signature::from_scalars(r, s)?),
606607
_ => Err(Error::Crypto),
607608
}
608609
}
@@ -712,6 +713,9 @@ mod tests {
712713
signature::{Signer, Verifier},
713714
};
714715

716+
#[cfg(feature = "p256")]
717+
use super::{zero_pad_field_bytes, Mpint};
718+
715719
const DSA_SIGNATURE: &[u8] = &hex!("000000077373682d6473730000002866725bf3c56100e975e21fff28a60f73717534d285ea3e1beefc2891f7189d00bd4d94627e84c55c");
716720
const ECDSA_SHA2_P256_SIGNATURE: &[u8] = &hex!("0000001365636473612d736861322d6e6973747032353600000048000000201298ab320720a32139cda8a40c97a13dc54ce032ea3c6f09ea9e87501e48fa1d0000002046e4ac697a6424a9870b9ef04ca1182cd741965f989bd1f1f4a26fd83cf70348");
717721
const ED25519_SIGNATURE: &[u8] = &hex!("0000000b7373682d65643235353139000000403d6b9906b76875aef1e7b2f1e02078a94f439aebb9a4734da1a851a81e22ce0199bbf820387a8de9c834c9c3cc778d9972dcbe70f68d53cc6bc9e26b02b46d04");
@@ -729,6 +733,35 @@ mod tests {
729733
let _ssh_signature = Signature::try_from(p256_signature).unwrap();
730734
}
731735

736+
#[cfg(feature = "p256")]
737+
#[test]
738+
fn zero_pad_field_bytes_p256() {
739+
let i = Mpint::from_bytes(&hex!(
740+
"1122334455667788112233445566778811223344556677881122334455667788"
741+
))
742+
.unwrap();
743+
let fb = zero_pad_field_bytes::<p256::FieldBytes>(i);
744+
assert!(fb.is_some());
745+
746+
// too long
747+
let i = Mpint::from_bytes(&hex!(
748+
"991122334455667788112233445566778811223344556677881122334455667788"
749+
))
750+
.unwrap();
751+
let fb = zero_pad_field_bytes::<p256::FieldBytes>(i);
752+
assert!(fb.is_none());
753+
754+
// short is okay
755+
let i = Mpint::from_bytes(&hex!(
756+
"22334455667788112233445566778811223344556677881122334455667788"
757+
))
758+
.unwrap();
759+
let fb = zero_pad_field_bytes::<p256::FieldBytes>(i)
760+
.expect("failed to build FieldBytes from short hex string");
761+
assert_eq!(fb[0], 0x00);
762+
assert_eq!(fb[1], 0x22);
763+
}
764+
732765
#[test]
733766
fn decode_dsa() {
734767
let signature = Signature::try_from(DSA_SIGNATURE).unwrap();

‎ssh-key/tests/authorized_keys.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use ssh_key::AuthorizedKeys;
88
#[test]
99
fn read_example_file() {
1010
let authorized_keys = AuthorizedKeys::read_file("./tests/examples/authorized_keys").unwrap();
11-
assert_eq!(authorized_keys.len(), 4);
11+
assert_eq!(authorized_keys.len(), 5);
1212

1313
assert_eq!(authorized_keys[0].config_opts().to_string(), "");
1414
assert_eq!(
@@ -46,4 +46,10 @@ fn read_example_file() {
4646
authorized_keys[3].public_key().comment(),
4747
"[email protected]"
4848
);
49+
50+
assert_eq!(authorized_keys[4].public_key().to_string(), "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBN76zuqnjypL54/w4763l7q1Sn3IBYHptJ5wcYfEWkzeNTvpexr05Z18m2yPT2SWRd1JJ8Aj5TYidG9MdSS5J78= hello world this is a long comment");
51+
assert_eq!(
52+
authorized_keys[4].public_key().comment(),
53+
"hello world this is a long comment"
54+
);
4955
}

‎ssh-key/tests/examples/authorized_keys‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,6 @@ environment="PATH=/bin:/usr/bin" ssh-dss AAAAB3NzaC1kc3MAAACBANw9iSUO2UYhFMssjUg
2626

2727
# Public key which can only be used from certain source addresses and disallows X11 forwarding
2828
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]
29+
30+
# Public key with a comment that contains multiple spaces
31+
ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBN76zuqnjypL54/w4763l7q1Sn3IBYHptJ5wcYfEWkzeNTvpexr05Z18m2yPT2SWRd1JJ8Aj5TYidG9MdSS5J78= hello world this is a long comment

0 commit comments

Comments
 (0)