chore: move to ifrit

Cargo.toml

@@ -11,7 +11,9 @@ categories = ["cryptography", "parsing"]
 [dependencies]
 digest = { version = "0.10.6", features = [ "alloc" ] }
 # waiting for a release of goblin
-goblin = { git = "https://github.com/RaitoBezarius/goblin.git", branch = "goblin-signing" }
+goblin = { git = "https://github.com/RaitoBezarius/goblin.git", branch = "preps-for-ifrit-1" }
+# waiting for a release of ifrit
+ifrit = { git = "https://github.com/RaitoBezarius/ifrit.git" }
 cms = { version = "0.2.2", features = [ "builder" ] }
 der = { version = "0.7.7", default-features = false }
 x509-cert = { version = "0.2.3" }
@@ -27,16 +29,16 @@ cryptoki-rustcrypto = { git = "https://github.com/baloo/rust-cryptoki", branch =
 thiserror = "1.0.49"
 stderrlog = "0.5.4"
 x509-verify = { version = "0.4.2", features = [ "x509", "ecdsa" ] }
-scroll = "0.11.0"
+scroll = { version = "0.12.0", features = [ "derive" ] }
 uuid = { version = "1.5.0", features = [ "v4" ] }
 percent-encoding = "2.3.0"
 uriparse = "0.6.4"
 log = "0.4.20"
 pkcs11-uri = "0.1.3"
 p256 = "0.13.2"
 
-[patch.crates-io]
-scroll = { git = "https://github.com/RaitoBezarius/scroll.git", branch = "goblin-signing" }
+[patch."https://github.com/RaitoBezarius/goblin.git"]
+goblin = { path = "../../m4b/goblin" }
 
 [[example]]
 name = "sign_binary"
@@ -47,4 +49,6 @@ name = "verify_binary"
 [dev-dependencies]
 bitflags = "2.4.1"
 clap = { version = "4.4.8", features = [ "derive" ] }
+pretty_assertions = "1.4.0"
 rpassword = "7.3.1"
+stderrlog = "0.5.4"

examples/sign_binary.rs

@@ -20,8 +20,9 @@ use cryptoki::{
 };
 use cryptoki_rustcrypto::{x509::CertPkcs11, SessionLike};
 use der::Encode;
-use goblin::pe::{writer::PEWriter, PE};
+use goblin::pe::PE;
 use goblin_signing::sign::create_certificate;
+use ifrit::writer::PEWriter;
 use pkcs11_uri::Pkcs11Uri;
 use rpassword::read_password;
 use sha2::Sha256;
@@ -381,7 +382,9 @@ fn sign_file<S: SessionLike>(
     .expect("Failed to produce an PE attribute certificate");
     let mut pe_writer = PEWriter::new(pe).expect("Failed to construct the PE writer");
     pe_writer
-        .attach_certificates(vec![pe_certificate])
+        .attach_certificates(vec![pe_certificate
+            .attribute()
+            .expect("Failed to produce a certificate")])
         .expect("Failed to attach a new certificate to PE");
     println!("Signed!");
     std::fs::write(

src/errors.rs

@@ -8,6 +8,8 @@ pub enum SignatureError {
     UnknownSignatureAlgorithmIdentifier(#[from] spki::Error),
     #[error("Failed to run the signer information through the signer")]
     SigningError(#[from] x509_cert::builder::Error),
+    #[error("Malformed PE at assembly time")]
+    MalformedPE(#[from] goblin::error::Error),
 }
 
 #[derive(Error, Debug)]

src/sign.rs

@@ -6,7 +6,12 @@ use cms::{
 };
 use der::Encode;
 use digest::Digest;
-use goblin::pe::{certificate_table::AttributeCertificate, PE};
+use goblin::pe::{
+    certificate_table::{
+        AttributeCertificate, AttributeCertificateRevision, AttributeCertificateType,
+    },
+    PE,
+};
 use signature::{Keypair, Signer};
 use x509_cert::{
     spki::{DynSignatureAlgorithmIdentifier, EncodePublicKey, SignatureBitStringEncoding},
@@ -16,14 +21,45 @@ use x509_cert::{
 use crate::errors::SignatureError;
 use crate::{authenticode::Authenticode, certificate::DigestInfo};
 
+/// Because [`AttributeCertificate`] is a purely borrowing a structure,
+/// we cannot return it naked, we need to own the raw certificate data somewhere.
+#[derive(Debug, Clone)]
+pub struct CertificateBundle {
+    certificate_raw: Vec<u8>,
+    attribute_revision: AttributeCertificateRevision,
+    attribute_type: AttributeCertificateType,
+}
+
+impl CertificateBundle {
+    pub fn new(
+        raw: Vec<u8>,
+        attribute_revision: AttributeCertificateRevision,
+        attribute_type: AttributeCertificateType,
+    ) -> Self {
+        Self {
+            certificate_raw: raw,
+            attribute_revision,
+            attribute_type,
+        }
+    }
+
+    pub fn attribute(&self) -> Result<AttributeCertificate, SignatureError> {
+        Ok(AttributeCertificate::from_bytes(
+            &self.certificate_raw,
+            self.attribute_revision,
+            self.attribute_type,
+        )?)
+    }
+}
+
 /// Produces a certificate for the given PE
 /// with the given signer identifier and signer.
 pub fn create_certificate<'pe, D: Digest, S, Signature>(
     pe: &PE<'pe>,
     certificates: Vec<Certificate>,
     sid: SignerIdentifier,
     signer: &S,
-) -> Result<AttributeCertificate<'pe>, SignatureError>
+) -> Result<CertificateBundle, SignatureError>
 where
     D: const_oid::AssociatedOid,
     S: Keypair + DynSignatureAlgorithmIdentifier,
@@ -66,8 +102,8 @@ where
     let mut certificate_contents = Vec::new();
     signed_data.encode_to_vec(&mut certificate_contents)?;
 
-    Ok(AttributeCertificate::from_bytes(
-        certificate_contents.into(),
+    Ok(CertificateBundle::new(
+        certificate_contents,
         goblin::pe::certificate_table::AttributeCertificateRevision::Revision2_0,
         goblin::pe::certificate_table::AttributeCertificateType::PkcsSignedData,
     ))

src/verify.rs

@@ -24,12 +24,12 @@ pub struct VerificationOptions {
 pub fn certificates_from_pe(pe: &PE) -> Vec<(SignedData, DigestInfo)> {
     pe.certificates
         .iter()
-        .filter_map(|(_, cert)| {
-            match (cert.as_signed_data(), cert.as_spc_indirect_data_content()) {
+        .filter_map(
+            |cert| match (cert.as_signed_data(), cert.as_spc_indirect_data_content()) {
                 (Some(Ok(sdata)), Some(Ok(spc))) => Some((sdata, spc.message_digest)),
                 _ => None,
-            }
-        })
+            },
+        )
         .collect()
 }
 

tests/test_snakeoil_sign.rs

@@ -1,14 +1,18 @@
+use goblin::pe::certificate_table::AttributeCertificate;
+use pretty_assertions::assert_eq;
+use std::cmp::{max, min};
 /// We test various ways to manipulate PE binaries w.r.t. to signatures
 /// with a snakeoil certificate and signer.
 use std::str::FromStr;
 use std::time::Duration;
 
 use cms::cert::IssuerAndSerialNumber;
 use digest::Digest;
-use goblin::pe::{writer::PEWriter, PE};
+use goblin::pe::PE;
 use goblin_signing::authenticode::Authenticode;
 use goblin_signing::sign::create_certificate;
 use goblin_signing::verify::{certificates_from_pe, verify_pe_signatures_no_trust};
+use ifrit::writer::PEWriter;
 use p256::ecdsa::SigningKey;
 use sha2::Sha256;
 use signature::rand_core::OsRng;
@@ -19,6 +23,35 @@ use x509_cert::serial_number::SerialNumber;
 use x509_cert::time::Validity;
 use x509_cert::Certificate;
 
+/// Assert equality modulo alignment.
+fn assert_eq_modulo_alignment(a: &AttributeCertificate, b: &AttributeCertificate) {
+    let m = min(a.certificate.len(), b.certificate.len());
+    let n = max(a.certificate.len(), b.certificate.len());
+    assert_eq!(
+        a.certificate[0..m],
+        b.certificate[0..m],
+        "Certificate does not agree on their common parts"
+    );
+
+    if a.certificate.len() > m {
+        let zeroes = vec![0u8; n - m];
+        assert_eq!(
+            &a.certificate[m..n],
+            &zeroes[..],
+            "Certificate A alignment does not contain only zeroes"
+        );
+    }
+
+    if b.certificate.len() > m {
+        let zeroes = vec![0u8; n - m];
+        assert_eq!(
+            &b.certificate[m..n],
+            &zeroes[..],
+            "Certificate A alignment does not contain only zeroes"
+        );
+    }
+}
+
 fn build_issuer(rdn: &str, serial: u32) -> der::Result<cms::signed_data::SignerIdentifier> {
     Ok(cms::signed_data::SignerIdentifier::IssuerAndSerialNumber(
         IssuerAndSerialNumber {
@@ -67,6 +100,7 @@ fn test_create_attribute_certificate() {
 
 #[test]
 fn test_attaching_attribute_certificate_to_pe() {
+    stderrlog::new().verbosity(4).init().unwrap();
     let file = std::fs::read("tests/bins/nixos-uki.efi").unwrap();
     let pe = PE::parse(&file[..]).unwrap();
     println!("PE original certificates: {:?}", certificates_from_pe(&pe));
@@ -96,8 +130,11 @@ fn test_attaching_attribute_certificate_to_pe() {
         &signing_key,
     )
     .expect("Failed to build an attribute certificate");
+    pe_writer.clear_certificates();
     pe_writer
-        .attach_certificates(vec![attr_cert.clone()])
+        .attach_certificates(vec![attr_cert
+            .attribute()
+            .expect("Failed to produce certificate")])
         .expect("Failed to attach a certificate to PE");
     let new_pe_bytes = pe_writer
         .write_into()
@@ -107,12 +144,19 @@ fn test_attaching_attribute_certificate_to_pe() {
     let new_pe = PE::parse(&new_pe_bytes[..]).expect("Failed to read the new PE");
     assert_eq!(new_pe.certificates.len(), 1);
     let cert = new_pe.certificates.first().unwrap();
+    let attr_cert = attr_cert
+        .attribute()
+        .expect("Failed to assemble an attribute certificate");
     assert!(
-        cert.1.certificate == attr_cert.certificate,
-        "Attribute certificate is different from expected!"
+        cert.length == attr_cert.length,
+        "Attribute certificate differs in length, expected: {} bytes, got: {} bytes",
+        attr_cert.length,
+        cert.length
     );
+
+    assert_eq_modulo_alignment(&cert, &attr_cert);
     assert!(
-        original_cert.1.certificate != cert.1.certificate,
+        original_cert.certificate != cert.certificate,
         "Attribute certificate is same as original!"
     );
     println!("PE new certificates: {:?}", certificates_from_pe(&new_pe));
@@ -130,7 +174,7 @@ fn test_attaching_attribute_certificate_to_pe() {
 fn test_multisig_pe() {
     let file = std::fs::read("tests/bins/nixos-uki.efi").unwrap();
     let pe = PE::parse(&file[..]).unwrap();
-    let (_, original_cert) = pe
+    let original_cert = pe
         .certificates
         .first()
         .expect("Original PE does not have a certificate!")
@@ -150,8 +194,14 @@ fn test_multisig_pe() {
         &signing_key,
     )
     .expect("Failed to build an attribute certificate");
+    pe_writer.clear_certificates();
     pe_writer
-        .attach_certificates(vec![original_cert, attr_cert.clone()])
+        .attach_certificates(vec![
+            original_cert,
+            attr_cert
+                .attribute()
+                .expect("Failed to produce a certificate"),
+        ])
         .expect("Failed to attach a certificate to PE");
     let new_pe_bytes = pe_writer
         .write_into()