From e8d4b98f52f8c67502453dc7e5b00d2d3e935bee Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Wed, 16 Sep 2026 22:24:53 +0200 Subject: [PATCH] put: verify the changes signature locally with gpgme --- src/put/changes.rs | 60 ++++++--- src/utils/gpg.rs | 330 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 370 insertions(+), 20 deletions(-) diff --git a/src/put/changes.rs b/src/put/changes.rs index 6a13fb1..0ad6df5 100644 --- a/src/put/changes.rs +++ b/src/put/changes.rs @@ -1,9 +1,10 @@ -//! `.changes` file parsing and pre-upload validation: signature presence, -//! field extraction, and verification that every file listed in the -//! checksums sections exists and matches on disk. +//! `.changes` file parsing and pre-upload validation: OpenPGP signature +//! verification, field extraction, and verification that every file listed +//! 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, } -/// Parse a `.changes` file: extract the upload fields and the file list from -/// the checksums sections. Rejects unsigned files (Launchpad requires a +/// Parse a `.changes` file: verify its OpenPGP signature against the local +/// 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> { let content = std::fs::read_to_string(path) @@ -77,6 +79,20 @@ pub fn parse(path: &Path) -> Result> { .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> { // 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> { 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, Vec) { 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(); - assert!(err.contains("UNRELEASED"), "unexpected error: {err}"); + let err = parse_body(&content, &path).unwrap_err().to_string(); + 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()); diff --git a/src/utils/gpg.rs b/src/utils/gpg.rs index 4599999..962c429 100644 --- a/src/utils/gpg.rs +++ b/src/utils/gpg.rs @@ -1,11 +1,18 @@ -//! GPG / OpenPGP helpers: secret key discovery and inline (clear) signing -//! of Debian artifacts such as `.dsc`, `.buildinfo` and `.changes` files. +//! GPG / OpenPGP helpers: secret key discovery, inline (clear) signing of +//! 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> { .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> { + 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> { + 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> { + let signatures: Vec> = 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("") + .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 ", + "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 = 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") }, + } + } }