put: verify the changes signature locally with gpgme

src/put/changes.rs

@@ -1,9 +1,10 @@
-//! `.changes` file parsing and pre-upload validation: signature presence,
+//! `.changes` file parsing and pre-upload validation: OpenPGP signature
-//! field extraction, and verification that every file listed in the
+//! verification, field extraction, and verification that every file listed
-//! checksums sections exists and matches on disk.
+//! in the checksums sections exists and matches on disk.
 //!
 //! Launchpad authenticates the upload through the GPG signature of the
-//! `.changes` file, so an unsigned file is rejected before connecting, and
+//! `.changes` file, so an unsigned file — or one whose signature does not
+//! verify against the local keyring — is rejected before connecting, and
 //! a file whose checksums do not match the artifacts next to it would only
 //! fail server-side after a partial upload.
 
@@ -61,8 +62,9 @@ pub struct ChangesFile {
     pub files: Vec<FileEntry>,
 }
 
-/// Parse a `.changes` file: extract the upload fields and the file list from
+/// Parse a `.changes` file: verify its OpenPGP signature against the local
-/// the checksums sections. Rejects unsigned files (Launchpad requires a
+/// keyring, then extract the upload fields and the file list from the
+/// checksums sections. Rejects unsigned files (Launchpad requires a
 /// signature) and `UNRELEASED` entries (which are not uploadable).
 pub fn parse(path: &Path) -> Result<ChangesFile, Box<dyn std::error::Error>> {
     let content = std::fs::read_to_string(path)
@@ -77,6 +79,20 @@ pub fn parse(path: &Path) -> Result<ChangesFile, Box<dyn std::error::Error>> {
         .into());
     }
 
+    // Launchpad authenticates the upload through the OpenPGP signature, so
+    // verify it locally, like dput-ng with `allow_unsigned_uploads = false`:
+    // a tampered document, or one signed by a key absent from the keyring,
+    // must fail before anything is uploaded.
+    crate::utils::gpg::verify_changes_signature(&content)
+        .map_err(|e| format!("'{}': {e}", path.display()))?;
+
+    parse_body(&content, path)
+}
+
+/// Parse the deb822 content of an already signature-checked `.changes`
+/// document: strip the signing armor, extract the upload fields and the
+/// file list from the checksums sections.
+fn parse_body(content: &str, path: &Path) -> Result<ChangesFile, Box<dyn std::error::Error>> {
     // dpkg signs `.changes` either clearsigned (the whole document wrapped
     // in a `-----BEGIN PGP SIGNED MESSAGE-----` armor, with the fields only
     // starting after the armor headers) or with the signature block merely
@@ -87,7 +103,7 @@ pub fn parse(path: &Path) -> Result<ChangesFile, Box<dyn std::error::Error>> {
         content
             .split("-----BEGIN PGP SIGNATURE-----")
             .next()
-            .unwrap_or(&content),
+            .unwrap_or(content),
     );
 
     let paragraphs = parse_paragraphs(body);
@@ -222,6 +238,11 @@ iQIzBAABCgAdFiEEfakefingerprintXfakecommentlength
 
     /// Write a minimal valid changes file plus its two artifacts; returns
     /// (changes path, dsc content, tarball content)
+    ///
+    /// The signature block is a fake: parsing tests exercise the two
+    /// signing layouts only, the cryptographic verification happens before
+    /// [`parse_body`] and is covered in `utils::gpg` (see the ignored
+    /// throwaway-keyring test there).
     fn fixture(dir: &Path, distribution: &str) -> (PathBuf, Vec<u8>, Vec<u8>) {
         let dsc = b"dsc content";
         let tarball = b"tarball content";
@@ -255,8 +276,9 @@ iQIzBAABCgAdFiEEfakefingerprintXfakecommentlength
     fn parse_and_validate_valid_changes() {
         let dir = tempfile::tempdir().unwrap();
         let (path, _, _) = fixture(dir.path(), "resolute");
+        let content = std::fs::read_to_string(&path).unwrap();
 
-        let changes = parse(&path).unwrap();
+        let changes = parse_body(&content, &path).unwrap();
         assert_eq!(changes.source, "hello");
         assert_eq!(changes.version, "1.0-1");
         assert_eq!(changes.distribution, "resolute");
@@ -285,9 +307,9 @@ iQIzBAABCgAdFiEEfakefingerprintXfakecommentlength
             SIGNATURE
         );
         let path = dir.path().join("hello_1.0-1_source.changes");
-        std::fs::write(&path, content).unwrap();
+        std::fs::write(&path, &content).unwrap();
 
-        let changes = parse(&path).unwrap();
+        let changes = parse_body(&content, &path).unwrap();
         assert_eq!(changes.source, "hello");
         validate(&changes).unwrap();
     }
@@ -303,17 +325,19 @@ iQIzBAABCgAdFiEEfakefingerprintXfakecommentlength
             .unwrap();
         std::fs::write(&path, unsigned).unwrap();
 
+        // Detected before the signature verification, so this stays offline
         let err = parse(&path).unwrap_err().to_string();
-        assert!(err.contains("not signed"), "unexpected error: {err}");
+        assert!(err.contains("not signed"), "unexpected: {err}");
     }
 
     #[test]
     fn parse_rejects_unreleased_distribution() {
         let dir = tempfile::tempdir().unwrap();
         let (path, _, _) = fixture(dir.path(), "UNRELEASED");
+        let content = std::fs::read_to_string(&path).unwrap();
 
-        let err = parse(&path).unwrap_err().to_string();
+        let err = parse_body(&content, &path).unwrap_err().to_string();
-        assert!(err.contains("UNRELEASED"), "unexpected error: {err}");
+        assert!(err.contains("UNRELEASED"), "unexpected: {err}");
     }
 
     #[test]
@@ -321,8 +345,9 @@ iQIzBAABCgAdFiEEfakefingerprintXfakecommentlength
         let dir = tempfile::tempdir().unwrap();
         let (path, _, _) = fixture(dir.path(), "resolute");
         std::fs::write(dir.path().join("hello_1.0-1.dsc"), b"tampered!").unwrap();
+        let content = std::fs::read_to_string(&path).unwrap();
 
-        let changes = parse(&path).unwrap();
+        let changes = parse_body(&content, &path).unwrap();
         let err = validate(&changes).unwrap_err().to_string();
         assert!(
             err.contains("does not match its checksums"),
@@ -335,8 +360,9 @@ iQIzBAABCgAdFiEEfakefingerprintXfakecommentlength
         let dir = tempfile::tempdir().unwrap();
         let (path, _, _) = fixture(dir.path(), "resolute");
         std::fs::remove_file(dir.path().join("hello_1.0-1.dsc")).unwrap();
+        let content = std::fs::read_to_string(&path).unwrap();
 
-        let changes = parse(&path).unwrap();
+        let changes = parse_body(&content, &path).unwrap();
         let err = validate(&changes).unwrap_err().to_string();
         assert!(err.contains("cannot upload"), "unexpected: {err}");
     }
@@ -357,9 +383,9 @@ iQIzBAABCgAdFiEEfakefingerprintXfakecommentlength
             SIGNATURE
         );
         let path = dir.path().join("hello_1.0-1_source.changes");
-        std::fs::write(&path, content).unwrap();
+        std::fs::write(&path, &content).unwrap();
 
-        let changes = parse(&path).unwrap();
+        let changes = parse_body(&content, &path).unwrap();
         assert_eq!(changes.files.len(), 1);
         assert!(changes.files[0].sha256.is_none());
         assert!(changes.files[0].md5.is_some());

src/utils/gpg.rs

@@ -1,11 +1,18 @@
-//! GPG / OpenPGP helpers: secret key discovery and inline (clear) signing
+//! GPG / OpenPGP helpers: secret key discovery, inline (clear) signing of
-//! of Debian artifacts such as `.dsc`, `.buildinfo` and `.changes` files.
+//! Debian artifacts such as `.dsc`, `.buildinfo` and `.changes` files, and
+//! local verification of signed documents.
 
 use std::error::Error;
 use std::io::Read;
 use std::path::Path;
 
-use gpgme::{Context, Data, Protocol};
+use gpgme::error::Error as GpgError;
+use gpgme::{Context, Data, Protocol, Signature, SignatureSummary, VerificationResult};
+
+/// The armor marker opening an OpenPGP signature block.
+const SIGNATURE_BEGIN: &str = "-----BEGIN PGP SIGNATURE-----";
+/// The armor marker opening a clearsigned message.
+const CLEARSIGNED_BEGIN: &str = "-----BEGIN PGP SIGNED MESSAGE-----";
 
 /// Check if a GPG key matching 'email' exists
 /// Returns the key ID if found, None otherwise
@@ -111,6 +118,151 @@ pub fn clearsign_file(path: &Path, keyid: &str) -> Result<(), Box<dyn Error>> {
         .map_err(|e| format!("cannot install signed '{}': {}", path.display(), e).into())
 }
 
+/// How a document carries its OpenPGP signature: Debian tools
+/// (`dpkg-buildpackage`, `debsign`) either wrap the whole document in a
+/// clearsigned armor or append an armored signature block to the plain
+/// deb822 body.
+#[derive(Debug, PartialEq, Eq)]
+enum SignatureLayout<'a> {
+    /// Whole document clearsigned: the signed text and the signature live
+    /// in the same armor buffer.
+    Clearsigned(&'a str),
+    /// Signature block appended after the plain body: the signed text and
+    /// the armored detached signature are distinct parts.
+    Detached {
+        /// The signed text, everything before the signature block.
+        body: &'a str,
+        /// The armored signature block, from its BEGIN marker to the end.
+        signature: &'a str,
+    },
+}
+
+/// Detect how `content` is signed and split it into signed text and
+/// signature. Returns `None` when no signature block is present.
+///
+/// The split preserves the exact bytes of both parts: the body is
+/// byte-for-byte what the signer signed, which is what a detached
+/// verification must hash.
+fn split_signature(content: &str) -> Option<SignatureLayout<'_>> {
+    if content.starts_with(CLEARSIGNED_BEGIN) {
+        return Some(SignatureLayout::Clearsigned(content));
+    }
+    let (body, _) = content.split_once(SIGNATURE_BEGIN)?;
+    Some(SignatureLayout::Detached {
+        body,
+        signature: &content[body.len()..],
+    })
+}
+
+/// Verify the OpenPGP signature of a signed document (`.changes`, `.dsc`,
+/// `.buildinfo`): both dpkg signing layouts are accepted, the whole
+/// document clearsigned or the signature block merely appended to the body.
+///
+/// Like dput-ng with `allow_unsigned_uploads = false`, the signature must
+/// verify against the local keyring and the signing key must be present
+/// locally, but any key will do: Launchpad authenticates the upload
+/// through this signature, so a tampered document, a corrupt signature or
+/// an unattributable signer must be refused before anything is uploaded.
+pub fn verify_changes_signature(content: &str) -> Result<(), Box<dyn Error>> {
+    let layout = split_signature(content)
+        .ok_or("no OpenPGP signature block found: the document is not signed")?;
+
+    let mut ctx = Context::from_protocol(Protocol::OpenPgp)
+        .map_err(|e| format!("cannot initialize GPGME: {e}"))?;
+    ctx.set_armor(true);
+
+    let result = match layout {
+        SignatureLayout::Clearsigned(signed) => {
+            // The clearsigned buffer is both the signed text and the
+            // signature; gpgme only needs a sink for the verified plaintext
+            let mut plaintext = Data::new()?;
+            ctx.verify_opaque(signed, &mut plaintext)
+        }
+        SignatureLayout::Detached { body, signature } => ctx.verify_detached(signature, body),
+    }
+    .map_err(|e| {
+        format!(
+            "signature verification failed ({e}): the signature block is \
+             unreadable, or the document was modified after signing"
+        )
+    })?;
+
+    require_valid_signature(&result)
+}
+
+/// Turn gpgme's per-signature verdicts into an accept/reject decision: at
+/// least one signature must be valid; otherwise the most specific problem
+/// found is reported.
+fn require_valid_signature(result: &VerificationResult) -> Result<(), Box<dyn Error>> {
+    let signatures: Vec<Signature<'_>> = result.signatures().collect();
+    if signatures.is_empty() {
+        return Err("the PGP block contains no signature".into());
+    }
+
+    let mut first_failure = None;
+    for signature in &signatures {
+        match signature_verdict(signature) {
+            Ok(()) => return Ok(()),
+            Err(reason) => first_failure = first_failure.or(Some(reason)),
+        }
+    }
+
+    Err(first_failure
+        .unwrap_or_else(|| "signature verification failed".to_string())
+        .into())
+}
+
+/// A human-readable verdict for one signature: `Ok` when it is valid, or an
+/// actionable reason to reject the document otherwise.
+fn signature_verdict(signature: &Signature<'_>) -> Result<(), String> {
+    let fingerprint = signature
+        .fingerprint()
+        .unwrap_or("<unknown key>")
+        .to_string();
+    let summary = signature.summary();
+
+    // The signing key must be resolvable in the local keyring: without it
+    // the signature cannot be attributed at all. That is a keyring
+    // configuration problem, distinct from a corrupt document.
+    if summary.contains(SignatureSummary::KEY_MISSING)
+        || signature
+            .status()
+            .err()
+            .is_some_and(|e| e.code() == GpgError::NO_PUBKEY.code())
+    {
+        return Err(format!(
+            "the signing key {fingerprint} is not in your local keyring, \
+             its signature cannot be verified; check it with \
+             `gpg --list-keys {fingerprint}` (and receive the key if missing)"
+        ));
+    }
+
+    for (flag, reason) in [
+        (SignatureSummary::KEY_REVOKED, "the signing key is revoked"),
+        (SignatureSummary::KEY_EXPIRED, "the signing key is expired"),
+        (SignatureSummary::SIG_EXPIRED, "the signature is expired"),
+    ] {
+        if summary.contains(flag) {
+            return Err(format!("invalid signature of key {fingerprint}: {reason}"));
+        }
+    }
+
+    // The canonical GPGME validity rule: accept a green signature, or one
+    // neither red nor errored (such a signature is valid although the key
+    // lacks full owner trust, which does not matter here: any locally known
+    // key is accepted). Everything else means the content does not match.
+    if summary.contains(SignatureSummary::GREEN)
+        || (!summary.contains(SignatureSummary::RED) && signature.status().is_ok())
+    {
+        return Ok(());
+    }
+
+    Err(format!(
+        "invalid signature of key {fingerprint}: the document was modified \
+         after signing, or the signature block is corrupt"
+    ))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -122,4 +274,176 @@ mod tests {
         assert!(validate_key_id(&"a".repeat(40)).is_ok());
         assert!(validate_key_id(&"b".repeat(64)).is_ok());
     }
+
+    /// The deb822 body shared by the signing-layout fixtures below
+    const BODY: &str = "Format: 1.8\nSource: hello\nVersion: 1.0-1\n\n";
+
+    /// An unparseable armor body: enough shape for the layout split, but
+    /// never a signature gpg could verify
+    const FAKE_ARMOR: &str = "\
+-----BEGIN PGP SIGNATURE-----
+
+iQIzBAABCgAdFiEEfakesignatureblock
+-----END PGP SIGNATURE-----
+";
+
+    fn clearsigned_fixture() -> String {
+        format!("-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA512\n\n{BODY}{FAKE_ARMOR}")
+    }
+
+    fn appended_fixture() -> String {
+        format!("{BODY}{FAKE_ARMOR}")
+    }
+
+    #[test]
+    fn split_detects_clearsigned_layout() {
+        let content = clearsigned_fixture();
+        assert_eq!(
+            split_signature(&content),
+            Some(SignatureLayout::Clearsigned(&content))
+        );
+    }
+
+    #[test]
+    fn split_detects_appended_signature_layout() {
+        let content = appended_fixture();
+        let Some(SignatureLayout::Detached { body, signature }) = split_signature(&content) else {
+            panic!("expected the appended-signature layout for: {content}");
+        };
+        // The body is the exact signed text: byte-for-byte everything
+        // before the signature marker
+        assert_eq!(body, BODY);
+        assert!(signature.starts_with("-----BEGIN PGP SIGNATURE-----"));
+        assert!(signature.ends_with("-----END PGP SIGNATURE-----\n"));
+        assert_eq!(format!("{body}{signature}"), content);
+    }
+
+    #[test]
+    fn split_rejects_unsigned_content() {
+        assert_eq!(split_signature("Format: 1.8\nSource: hello\n"), None);
+    }
+
+    /// Unsigned content is refused before any GPG context is created, so
+    /// this stays offline and keyring-independent
+    #[test]
+    fn verify_rejects_unsigned_content() {
+        let err = verify_changes_signature("Format: 1.8\nSource: hello\n")
+            .unwrap_err()
+            .to_string();
+        assert!(err.contains("not signed"), "unexpected: {err}");
+    }
+
+    /// Full verification round-trip in a throwaway GNUPGHOME: a generated
+    /// key signs a document in both layouts, then a tampered copy and an
+    /// empty keyring must be rejected with distinct errors.
+    ///
+    /// Ignored by default: it shells out to `gpg` and generates keys (a few
+    /// seconds). Run it explicitly with `cargo test -- --ignored gpg`. It
+    /// never touches the real keyring: GNUPGHOME points at a temporary
+    /// directory for the whole run and is restored afterwards.
+    #[test]
+    #[ignore = "shells out to gpg and generates keys in a throwaway GNUPGHOME"]
+    fn verify_signed_documents_in_throwaway_gnupghome() {
+        let gpg_home = tempfile::tempdir().unwrap();
+        let empty_home = tempfile::tempdir().unwrap();
+
+        // SAFETY: the test runs alone (`-- --ignored` selects only it), and
+        // the previous value is saved and restored below: neither gpg nor
+        // gpgme ever see the user's real keyring.
+        let saved_home = std::env::var("GNUPGHOME").ok();
+        unsafe { std::env::set_var("GNUPGHOME", gpg_home.path()) };
+
+        let run_gpg = |args: &[&str]| {
+            let output = std::process::Command::new("gpg")
+                .args([
+                    "--batch",
+                    "--yes",
+                    "--pinentry-mode",
+                    "loopback",
+                    "--passphrase",
+                    "",
+                ])
+                .args(args)
+                .output()
+                .expect("the gpg binary must be installed");
+            assert!(
+                output.status.success(),
+                "gpg {args:?} failed: {}",
+                String::from_utf8_lossy(&output.stderr)
+            );
+        };
+
+        run_gpg(&[
+            "--quick-gen-key",
+            "pkh test <pkh-test@invalid>",
+            "ed25519",
+            "sign",
+            "2d",
+        ]);
+
+        let scratch = gpg_home.path().join("document");
+        std::fs::write(&scratch, BODY).unwrap();
+        let plain = scratch.to_str().unwrap();
+
+        // Sign in both dpkg layouts: a clearsigned wrapper and a detached
+        // armor block appended after the plain body
+        run_gpg(&["--clearsign", "--output", &format!("{plain}.clear"), plain]);
+        run_gpg(&[
+            "--detach-sign",
+            "--armor",
+            "--output",
+            &format!("{plain}.sig"),
+            plain,
+        ]);
+        let clearsigned = std::fs::read_to_string(format!("{plain}.clear")).unwrap();
+        let appended = format!(
+            "{}{}",
+            BODY,
+            std::fs::read_to_string(format!("{plain}.sig")).unwrap()
+        );
+
+        // A valid signature from any local key is accepted, in both layouts
+        verify_changes_signature(&clearsigned).unwrap();
+        verify_changes_signature(&appended).unwrap();
+
+        // A tampered body no longer matches its signature
+        let tampered = appended.replace("Source: hello", "Source: evil");
+        let err = verify_changes_signature(&tampered).unwrap_err().to_string();
+        assert!(err.contains("modified after signing"), "unexpected: {err}");
+
+        // So does a corrupt signature payload: the armor still parses but
+        // the signature packet can no longer be verified
+        let mut corrupt_lines: Vec<String> = clearsigned.lines().map(str::to_string).collect();
+        let block_begin = corrupt_lines
+            .iter()
+            .position(|line| line == SIGNATURE_BEGIN)
+            .expect("fixture must contain a signature block");
+        // The first base64 line of the signature packet, after the blank
+        // armor-header line
+        corrupt_lines[block_begin + 2].replace_range(..4, "AAAA");
+        let corrupt = corrupt_lines.join("\n");
+        let err = verify_changes_signature(&corrupt).unwrap_err().to_string();
+        assert!(
+            err.contains("modified after signing")
+                || err.contains("signature verification failed")
+                || err.contains("no signature"),
+            "unexpected: {err}"
+        );
+
+        // The same valid document is refused when the signing key is not in
+        // the keyring, with the dedicated missing-key error
+        unsafe { std::env::set_var("GNUPGHOME", empty_home.path()) };
+        let err = verify_changes_signature(&clearsigned)
+            .unwrap_err()
+            .to_string();
+        assert!(
+            err.contains("not in your local keyring"),
+            "unexpected: {err}"
+        );
+
+        match saved_home {
+            Some(home) => unsafe { std::env::set_var("GNUPGHOME", home) },
+            None => unsafe { std::env::remove_var("GNUPGHOME") },
+        }
+    }
 }