Skip to content
This repository has been archived by the owner on Jul 29, 2022. It is now read-only.

Commit

Permalink
Fix critical bug in certificate signature verification
Browse files Browse the repository at this point in the history
The code was using the signature of the issuing certificate, not the
issued certificate.  Found by trying to verify a certificate signed by
OpenSSL.

Also, add new APIs that check that the issuer of the certificate being
verified matches the subject of the certificate it is being verified
against.
  • Loading branch information
Demi-Marie committed Apr 21, 2020
1 parent 9cb8cd3 commit 9022831
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 4 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "x509-signature"
version = "0.3.2"
version = "0.3.3"
authors = ["Parity Technologies <[email protected]>"]
edition = "2018"
license = "MIT OR Apache-2.0"
Expand Down
Binary file modified ca.crt
Binary file not shown.
2 changes: 1 addition & 1 deletion gen-bad-cert.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ openssl ecparam -name prime256v1 > "$ecparams"

openssl req \
-x509 \
-newkey rsa-pss \
-newkey rsa \
-batch \
-days 3000 \
-outform der \
Expand Down
17 changes: 15 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ impl<'a> X509Certificate<'a> {
cert.check_signature(
parse_algorithmid(self.signature_algorithm_id())?,
self.tbs_certificate(),
cert.signature(),
self.signature(),
)
}

Expand All @@ -282,7 +282,12 @@ impl<'a> X509Certificate<'a> {

/// Check that this certificate is self-signed. This does not check that the
/// subject and issuer are equal.
#[deprecated(since = "0.3.3", note = "Use check_self_issued instead")]
pub fn check_self_signature(&self) -> Result<(), Error> { self.check_signature_from(self) }

/// Check that this certificate is self-signed, and that the subject and
/// issuer are equal.
pub fn check_self_issued(&self) -> Result<(), Error> { self.check_issued_by(self) }
}

/// Extracts the algorithm id and public key from a certificate
Expand Down Expand Up @@ -356,14 +361,18 @@ mod tests {
let forged_message = include_bytes!("../forged-message.txt");
let message = include_bytes!("../gen-bad-cert.sh");
let certificate = include_bytes!("../testing.crt");
#[cfg(feature = "rsa")]
let ca_certificate = include_bytes!("../ca.crt");

let cert = parse_certificate(certificate).unwrap();
#[cfg(feature = "rsa")]
let ca_cert = parse_certificate(ca_certificate).unwrap();
assert_eq!(
cert.subject_public_key_info.algorithm(),
include_bytes!("data/alg-ecdsa-p256.der")
);
assert_eq!(cert.subject_public_key_info.key().len(), 65);
cert.valid(1586128701).unwrap();
cert.valid(1587492766).unwrap();
assert_eq!(cert.valid(0), Err(Error::CertNotValidYet));
assert_eq!(cert.valid(u64::max_value()), Err(Error::CertExpired));

Expand Down Expand Up @@ -396,5 +405,9 @@ mod tests {
.expect_err("forgery undetected?"),
Error::InvalidSignatureForPublicKey
);
#[cfg(feature = "rsa")]
ca_cert.check_self_issued().unwrap();
#[cfg(feature = "rsa")]
cert.check_issued_by(&ca_cert).unwrap();
}
}
Binary file modified testing.crt
Binary file not shown.
Binary file modified testing.sig
Binary file not shown.

0 comments on commit 9022831

Please sign in to comment.