Skip to content

Commit

Permalink
subject_name: rm unused err return for IP subj. cmp.
Browse files Browse the repository at this point in the history
Previously the ip address module's `presented_id_matches_reference_id`
function returned `Result<bool, Error>`, similar to the DNS subject name
function of the same name.

In the case of the IP address implementation there is no codepath that
returns an error. The lengths of the inputs are checked ahead of time,
and the DER reads are infallible for the byte-for-byte compare. All of
our unit tests also expect a true/false return, not an error.

With the above in mind this commit changes the
`presented_id_matches_reference_id` return type to `bool` and updates
the call-sites and unit tests accordingly.
  • Loading branch information
cpu authored and djc committed May 31, 2023
1 parent 13d057b commit 731817a
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 64 deletions.
97 changes: 38 additions & 59 deletions src/subject_name/ip_address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,12 @@ impl<'a> From<IpAddrRef<'a>> for &'a [u8] {
pub(super) fn presented_id_matches_reference_id(
presented_id: untrusted::Input,
reference_id: untrusted::Input,
) -> Result<bool, Error> {
) -> bool {
match (presented_id.len(), reference_id.len()) {
(4, 4) => (),
(16, 16) => (),
_ => {
return Ok(false);
return false;
}
};

Expand All @@ -218,11 +218,11 @@ pub(super) fn presented_id_matches_reference_id(
let presented_ip_address_byte = presented_ip_address.read_byte().unwrap();
let reference_ip_address_byte = reference_ip_address.read_byte().unwrap();
if presented_ip_address_byte != reference_ip_address_byte {
return Ok(false);
return false;
}
}

Ok(true)
true
}

// https://tools.ietf.org/html/rfc5280#section-4.2.1.10 says:
Expand Down Expand Up @@ -1138,61 +1138,40 @@ mod tests {

#[test]
fn test_presented_id_matches_reference_id() {
assert_eq!(
presented_id_matches_reference_id(
untrusted::Input::from(&[]),
untrusted::Input::from(&[])
),
Ok(false),
);

assert_eq!(
presented_id_matches_reference_id(
untrusted::Input::from(&[0x01]),
untrusted::Input::from(&[])
),
Ok(false),
);

assert_eq!(
presented_id_matches_reference_id(
untrusted::Input::from(&[]),
untrusted::Input::from(&[0x01])
),
Ok(false),
);

assert_eq!(
presented_id_matches_reference_id(
untrusted::Input::from(&[1, 2, 3, 4]),
untrusted::Input::from(&[1, 2, 3, 4])
),
Ok(true),
);

assert_eq!(
presented_id_matches_reference_id(
untrusted::Input::from(&[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]),
untrusted::Input::from(&[1, 2, 3, 4])
),
Ok(false),
);

assert_eq!(
presented_id_matches_reference_id(
untrusted::Input::from(&[1, 2, 3, 4]),
untrusted::Input::from(&[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
),
Ok(false),
);

assert_eq!(
presented_id_matches_reference_id(
untrusted::Input::from(&[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]),
untrusted::Input::from(&[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
),
Ok(true),
);
assert!(!presented_id_matches_reference_id(
untrusted::Input::from(&[]),
untrusted::Input::from(&[]),
));

assert!(!presented_id_matches_reference_id(
untrusted::Input::from(&[0x01]),
untrusted::Input::from(&[])
));

assert!(!presented_id_matches_reference_id(
untrusted::Input::from(&[]),
untrusted::Input::from(&[0x01])
));

assert!(presented_id_matches_reference_id(
untrusted::Input::from(&[1, 2, 3, 4]),
untrusted::Input::from(&[1, 2, 3, 4])
));

assert!(!presented_id_matches_reference_id(
untrusted::Input::from(&[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]),
untrusted::Input::from(&[1, 2, 3, 4])
));

assert!(!presented_id_matches_reference_id(
untrusted::Input::from(&[1, 2, 3, 4]),
untrusted::Input::from(&[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
));

assert!(presented_id_matches_reference_id(
untrusted::Input::from(&[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]),
untrusted::Input::from(&[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
));
}

#[test]
Expand Down
7 changes: 2 additions & 5 deletions src/subject_name/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,8 @@ pub(crate) fn verify_cert_subject_name(
&mut |name| {
if let GeneralName::IpAddress(presented_id) = name {
match ip_address::presented_id_matches_reference_id(presented_id, ip_address) {
Ok(true) => return NameIteration::Stop(Ok(())),
Ok(false) => (),
Err(_) => {
return NameIteration::Stop(Err(Error::BadDer));
}
true => return NameIteration::Stop(Ok(())),
false => (),
}
}
NameIteration::KeepGoing
Expand Down

0 comments on commit 731817a

Please sign in to comment.