Skip to content

Commit

Permalink
ssh-key: fix certificate::OptionsMap encoding (#207)
Browse files Browse the repository at this point in the history
The values in an `OptionsMap` are encoded with a nested length prefix.

https://bugzilla.mindrot.org/show_bug.cgi?id=2389
  • Loading branch information
jeromegn committed Mar 22, 2024
1 parent d611814 commit 01a9eba
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 3 deletions.
25 changes: 22 additions & 3 deletions ssh-key/src/certificate/options_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,13 @@ impl Decode for OptionsMap {

while !reader.is_finished() {
let name = String::decode(reader)?;
let data = String::decode(reader)?;
let data = reader.read_prefixed(|reader| {
if reader.remaining_len() > 0 {
String::decode(reader)
} else {
Ok(String::default())
}
})?;

// Options must be lexically ordered by "name" if they appear in
// the sequence. Each named option may only appear once in a
Expand All @@ -65,7 +71,16 @@ impl Encode for OptionsMap {
fn encoded_len(&self) -> encoding::Result<usize> {
self.iter()
.try_fold(4, |acc, (name, data)| {
[acc, 4, name.len(), 4, data.len()].checked_sum()
[
acc,
name.encoded_len()?,
if data.is_empty() {
4
} else {
data.encoded_len_prefixed()?
},
]
.checked_sum()
})
.map_err(Into::into)
}
Expand All @@ -78,7 +93,11 @@ impl Encode for OptionsMap {

for (name, data) in self.iter() {
name.encode(writer)?;
data.encode(writer)?;
if data.is_empty() {
0usize.encode(writer)?;
} else {
data.encode_prefixed(writer)?
}
}

Ok(())
Expand Down
20 changes: 20 additions & 0 deletions ssh-key/tests/certificate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,26 @@ fn decode_ed25519_openssh() {
assert_eq!(cert.valid_principals()[0], "host.example.com");
}

#[test]
fn decode_ed25519_openssh_with_crit_options() {
let src = "ssh-ed25519-cert-v01@openssh.com AAAAIHNzaC1lZDI1NTE5LWNlcnQtdjAxQG9wZW5zc2guY29tAAAAIBW/4zLqXWROWmN1sPgdySnH1GUsEFBjFrRwKKw71BoBAAAAIH1MFwI1oRdEifXgBQvWQfCBBtA/Pi8YCUE/I3wXFJo2AAAAAAAAAAAAAAABAAAAA2ZvbwAAAAAAAAAAAAAAAH//////////AAAAIwAAABFoZWxsb0BleGFtcGxlLmNvbQAAAAoAAAAGZm9vYmFyAAAAAAAAAAAAAAAzAAAAC3NzaC1lZDI1NTE5AAAAIH1MFwI1oRdEifXgBQvWQfCBBtA/Pi8YCUE/I3wXFJo2AAAAUwAAAAtzc2gtZWQyNTUxOQAAAEDRoPdI48KyoaLgaDZsSGs80qBeYQOXBd84CX8GYzFt/L21rxF1EeuPOkgsx7Q39WllXp+FgMMojsHftK/DJHEN";
let cert = Certificate::from_str(src).unwrap();

assert_eq!(Algorithm::Ed25519, cert.public_key().algorithm());

assert_eq!(cert.critical_options().len(), 1);
assert_eq!(
cert.critical_options().get("hello@example.com").unwrap(),
"foobar"
);

let openssh = cert.to_openssh().unwrap();

assert_eq!(openssh, src);

assert_eq!(cert, Certificate::from_str(&openssh).unwrap());
}

#[test]
fn decode_rsa_4096_openssh() {
let cert = Certificate::from_str(RSA_4096_CERT_EXAMPLE).unwrap();
Expand Down

0 comments on commit 01a9eba

Please sign in to comment.