ADR 0026. Both ADR 0024 and ADR 0025 said this code needed a security review before shipping. This is that review, done adversarially: for each way a signature could be defeated, a test that attempts it. It found a critical vulnerability in the code as shipped last turn. FINDING 1, critical, exploitable with no special access. Verification recovered the certificate and the signature by *scanning* the blob for DER-shaped bytes rather than decoding it. The signature was checked against certificates[0]; trust was checked against ANY certificate present. Two questions, two different certificates. So: the attacker signs a forgery with their own key the attacker appends the victim's trusted certificate to the blob signature_valid = true (their signature over their own content is real) chain_trusted = true (the victim's certificate is present) is_valid() = true Demonstrated before the fix, with the message "I hereby transfer everything to the attacker" verifying as valid. Fixed by decoding the ContentInfo/SignedData structure and finding the certificate the SignerInfo actually names, by issuer AND serial, then evaluating both the signature and the trust path against that one certificate. Trailing data now fails the decode instead of being ignored. The scanning functions are deleted, not left unused: dead code that once returned the wrong answer is an invitation to call it again. FINDING 2, moderate. signer_certificate() returned chain[0] unconditionally, so a chain whose first entry was not the signing key's certificate made the SignerInfo name the wrong one. Not a forgery route — the signature fails — but a UI showing "signed by <somebody trustworthy>" beside a failed check is its own kind of dangerous. Now it finds the entry whose public key matches the key doing the signing. FINDING 3, informational. digest_matches was hardcoded true under a comment claiming it was computed. Not exploitable, because is_valid() also requires signature_valid and the signature covers the bytes — but a field asserting an unperformed check is ADR 0017's pattern exactly. The four items ADR 0025 left unticked are closed: PKIX chain building, with each link's issuer signature verified. A name match alone is not a chain; anyone can put any name in a certificate. Pinning still short-circuits first. Stapled revocation from /DSS, offline only. Unknown is the default and a first-class answer: treating "no information" as "not revoked" is a claim a verifier cannot support. Signature appearances, with the claimed time labelled "Time claimed" because a self-declared /M carries no authority. One-call sign_document. Three things were wrong first: the /ByteRange placeholder was too narrow for real offsets so patching them moved every later byte; /Contents must be a hex string because a literal full of NULs needs escaping and changes length; and a signature dictionary nothing points at is invisible — the first version wrote one and the reader reported zero signatures over a correctly signed document. Four mutations, all killed — two only after strengthening the tests. My first smuggling test put the attacker's certificate first, where certificates[0] finds it anyway, so it passed with or without the issuer/serial match. Putting the TRUSTED certificate first is what distinguishes them, and writing that test is what exposed Finding 2. qpdf --check accepts the signed documents. pdf: 1276 passed. Coverage 87.98%. Left unticked, deliberately: an independent review by someone who did not write the code. This is a self-review; it found two real vulnerabilities, which is evidence the method works and not evidence that nothing remains. Also untested against Acrobat, which is stricter than the spec, and sign_document replaces rather than merges an existing AcroForm.
258 lines
8.4 KiB
Rust
258 lines
8.4 KiB
Rust
// Real keys and real certificates, generated at run time.
|
|
//
|
|
// Shared by `signing_roundtrip.rs` and `signing_security.rs`.
|
|
//
|
|
// Certificates are **genuinely signed by their issuer**, not stamped with
|
|
// a placeholder. That matters: chain validation checks issuer signatures,
|
|
// so a fixture with a fake signature would make every chain test pass
|
|
// vacuously — it would prove the chain walk found a name match and
|
|
// nothing about whether it verified anything.
|
|
|
|
use der::asn1::{BitString, SetOfVec, UtcTime};
|
|
use der::{Encode, Sequence};
|
|
use x509_cert::name::{Name, RdnSequence};
|
|
|
|
/// A deterministic RNG adaptor: `rsa` wants `rand_core` 0.6 and this crate
|
|
/// does not otherwise depend on it.
|
|
pub struct TestRng;
|
|
|
|
impl rsa::rand_core::RngCore for TestRng {
|
|
fn next_u32(&mut self) -> u32 {
|
|
let mut b = [0u8; 4];
|
|
getrandom::getrandom(&mut b).expect("entropy");
|
|
u32::from_le_bytes(b)
|
|
}
|
|
fn next_u64(&mut self) -> u64 {
|
|
let mut b = [0u8; 8];
|
|
getrandom::getrandom(&mut b).expect("entropy");
|
|
u64::from_le_bytes(b)
|
|
}
|
|
fn fill_bytes(&mut self, dest: &mut [u8]) {
|
|
getrandom::getrandom(dest).expect("entropy");
|
|
}
|
|
fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), rsa::rand_core::Error> {
|
|
getrandom::getrandom(dest).map_err(rsa::rand_core::Error::new)
|
|
}
|
|
}
|
|
|
|
impl rsa::rand_core::CryptoRng for TestRng {}
|
|
|
|
/// A distinguished name with a single CN.
|
|
fn name_with_cn(common_name: &str) -> Name {
|
|
let cn: der::asn1::ObjectIdentifier = "2.5.4.3".parse().expect("static OID");
|
|
let value = der::asn1::Utf8StringRef::new(common_name).expect("utf8");
|
|
let atv = x509_cert::attr::AttributeTypeAndValue {
|
|
oid: cn,
|
|
value: der::Any::from(value),
|
|
};
|
|
let mut set = SetOfVec::new();
|
|
set.insert(atv).expect("set");
|
|
let rdn = x509_cert::name::RelativeDistinguishedName::from(set);
|
|
Name::from(RdnSequence::from(vec![rdn]))
|
|
}
|
|
|
|
fn validity() -> x509_cert::time::Validity {
|
|
// UTCTime encodes a two-digit year and cannot represent anything past
|
|
// 2049 (RFC 5280 §4.1.2.5.1); 2_400_000_000 is 2046.
|
|
x509_cert::time::Validity {
|
|
not_before: x509_cert::time::Time::UtcTime(
|
|
UtcTime::from_unix_duration(std::time::Duration::from_secs(1_600_000_000))
|
|
.expect("time"),
|
|
),
|
|
not_after: x509_cert::time::Time::UtcTime(
|
|
UtcTime::from_unix_duration(std::time::Duration::from_secs(2_400_000_000))
|
|
.expect("time"),
|
|
),
|
|
}
|
|
}
|
|
|
|
/// Build a certificate for `subject`, signed by `issuer_key` under
|
|
/// `issuer_name`.
|
|
///
|
|
/// Pass the subject's own key and name to produce a self-signed root.
|
|
#[allow(clippy::too_many_arguments)]
|
|
pub fn signed_certificate(
|
|
subject_cn: &str,
|
|
subject_key_oid: &str,
|
|
subject_public_key_bits: &[u8],
|
|
issuer_cn: &str,
|
|
issuer_key: &rsa::RsaPrivateKey,
|
|
serial: u8,
|
|
) -> Vec<u8> {
|
|
use sha2::{Digest, Sha256};
|
|
|
|
const SHA256_WITH_RSA: &str = "1.2.840.113549.1.1.11";
|
|
|
|
#[derive(Sequence)]
|
|
struct AlgId {
|
|
algorithm: der::asn1::ObjectIdentifier,
|
|
}
|
|
let _ = AlgId {
|
|
algorithm: SHA256_WITH_RSA.parse().expect("static OID"),
|
|
};
|
|
|
|
let spki = x509_cert::spki::SubjectPublicKeyInfoOwned {
|
|
algorithm: x509_cert::spki::AlgorithmIdentifierOwned {
|
|
oid: subject_key_oid.parse().expect("static OID"),
|
|
parameters: None,
|
|
},
|
|
subject_public_key: BitString::from_bytes(subject_public_key_bits).expect("bit string"),
|
|
};
|
|
|
|
let tbs = x509_cert::TbsCertificate {
|
|
version: x509_cert::Version::V3,
|
|
serial_number: x509_cert::serial_number::SerialNumber::new(&[serial]).expect("serial"),
|
|
signature: x509_cert::spki::AlgorithmIdentifierOwned {
|
|
oid: SHA256_WITH_RSA.parse().expect("static OID"),
|
|
parameters: None,
|
|
},
|
|
issuer: name_with_cn(issuer_cn),
|
|
validity: validity(),
|
|
subject: name_with_cn(subject_cn),
|
|
subject_public_key_info: spki,
|
|
issuer_unique_id: None,
|
|
subject_unique_id: None,
|
|
extensions: None,
|
|
};
|
|
|
|
// Sign the TBS for real: chain validation checks this, and a
|
|
// placeholder would make every chain test pass without proving
|
|
// anything.
|
|
let tbs_der = tbs.to_der().expect("TBS DER");
|
|
let digest = Sha256::digest(&tbs_der);
|
|
let signature = issuer_key
|
|
.sign(rsa::Pkcs1v15Sign::new::<Sha256>(), &digest)
|
|
.expect("sign certificate");
|
|
|
|
x509_cert::Certificate {
|
|
tbs_certificate: tbs,
|
|
signature_algorithm: x509_cert::spki::AlgorithmIdentifierOwned {
|
|
oid: SHA256_WITH_RSA.parse().expect("static OID"),
|
|
parameters: None,
|
|
},
|
|
signature: BitString::from_bytes(&signature).expect("bit string"),
|
|
}
|
|
.to_der()
|
|
.expect("certificate DER")
|
|
}
|
|
|
|
/// An RSA key and a self-signed certificate for it.
|
|
pub fn self_signed_rsa() -> (rsa::RsaPrivateKey, Vec<u8>) {
|
|
self_signed_rsa_named("nigig-test-rsa")
|
|
}
|
|
|
|
pub fn self_signed_rsa_named(cn: &str) -> (rsa::RsaPrivateKey, Vec<u8>) {
|
|
use rsa::pkcs1::EncodeRsaPublicKey;
|
|
|
|
// 1024 bits: too small for production, fast enough for a suite that
|
|
// generates keys on every run. The algorithm under test is unchanged.
|
|
let mut rng = TestRng;
|
|
let key = rsa::RsaPrivateKey::new(&mut rng, 1024).expect("generate RSA key");
|
|
let public = key.to_public_key().to_pkcs1_der().expect("public key DER");
|
|
let der = signed_certificate(
|
|
cn,
|
|
"1.2.840.113549.1.1.1",
|
|
public.as_bytes(),
|
|
cn,
|
|
&key,
|
|
0x01,
|
|
);
|
|
(key, der)
|
|
}
|
|
|
|
/// A P-256 key and a self-signed certificate.
|
|
///
|
|
/// The certificate's own signature is RSA, because the chain walk needs an
|
|
/// issuer key and generating a second EC identity to sign it would test
|
|
/// nothing extra here.
|
|
pub fn self_signed_p256() -> (p256::ecdsa::SigningKey, Vec<u8>) {
|
|
let mut seed = [0u8; 32];
|
|
getrandom::getrandom(&mut seed).expect("entropy");
|
|
let key = p256::ecdsa::SigningKey::from_bytes(&seed.into()).expect("P-256 key");
|
|
let point = key.verifying_key().to_encoded_point(false);
|
|
|
|
let mut rng = TestRng;
|
|
let issuer = rsa::RsaPrivateKey::new(&mut rng, 1024).expect("issuer key");
|
|
let der = signed_certificate(
|
|
"nigig-test-p256",
|
|
"1.2.840.10045.2.1",
|
|
point.as_bytes(),
|
|
"nigig-test-p256",
|
|
&issuer,
|
|
0x02,
|
|
);
|
|
(key, der)
|
|
}
|
|
|
|
/// An Ed25519 key and a self-signed certificate.
|
|
pub fn self_signed_ed25519() -> (ed25519_dalek::SigningKey, Vec<u8>) {
|
|
let mut seed = [0u8; 32];
|
|
getrandom::getrandom(&mut seed).expect("entropy");
|
|
let key = ed25519_dalek::SigningKey::from_bytes(&seed);
|
|
|
|
let mut rng = TestRng;
|
|
let issuer = rsa::RsaPrivateKey::new(&mut rng, 1024).expect("issuer key");
|
|
let der = signed_certificate(
|
|
"nigig-test-ed25519",
|
|
"1.3.101.112",
|
|
key.verifying_key().as_bytes(),
|
|
"nigig-test-ed25519",
|
|
&issuer,
|
|
0x03,
|
|
);
|
|
(key, der)
|
|
}
|
|
|
|
/// A three-level PKI: root → intermediate → leaf, every link really
|
|
/// signed by the one above it.
|
|
pub struct Hierarchy {
|
|
pub root_der: Vec<u8>,
|
|
pub intermediate_der: Vec<u8>,
|
|
pub leaf_key: rsa::RsaPrivateKey,
|
|
pub leaf_der: Vec<u8>,
|
|
}
|
|
|
|
pub fn rsa_hierarchy() -> Hierarchy {
|
|
use rsa::pkcs1::EncodeRsaPublicKey;
|
|
let mut rng = TestRng;
|
|
|
|
let root_key = rsa::RsaPrivateKey::new(&mut rng, 1024).expect("root key");
|
|
let root_pub = root_key.to_public_key().to_pkcs1_der().expect("root pub");
|
|
let root_der = signed_certificate(
|
|
"nigig-root",
|
|
"1.2.840.113549.1.1.1",
|
|
root_pub.as_bytes(),
|
|
"nigig-root",
|
|
&root_key,
|
|
0x10,
|
|
);
|
|
|
|
let inter_key = rsa::RsaPrivateKey::new(&mut rng, 1024).expect("intermediate key");
|
|
let inter_pub = inter_key.to_public_key().to_pkcs1_der().expect("inter pub");
|
|
let intermediate_der = signed_certificate(
|
|
"nigig-intermediate",
|
|
"1.2.840.113549.1.1.1",
|
|
inter_pub.as_bytes(),
|
|
"nigig-root",
|
|
&root_key,
|
|
0x11,
|
|
);
|
|
|
|
let leaf_key = rsa::RsaPrivateKey::new(&mut rng, 1024).expect("leaf key");
|
|
let leaf_pub = leaf_key.to_public_key().to_pkcs1_der().expect("leaf pub");
|
|
let leaf_der = signed_certificate(
|
|
"nigig-leaf",
|
|
"1.2.840.113549.1.1.1",
|
|
leaf_pub.as_bytes(),
|
|
"nigig-intermediate",
|
|
&inter_key,
|
|
0x12,
|
|
);
|
|
|
|
Hierarchy {
|
|
root_der,
|
|
intermediate_der,
|
|
leaf_key,
|
|
leaf_der,
|
|
}
|
|
}
|