tool: Implement appending signatures to detached signatures.

  - See #67.

5 files changed, 197 insertions, 6 deletions

diff --git a/tool/src/commands.rs b/tool/src/commands.rs
index 8ab1ecd4..499976c5 100644
--- a/tool/src/commands.rs
+++ b/tool/src/commands.rs
@@ -1,10 +1,12 @@
 use failure::{self, ResultExt};
 use std::cmp::Ordering;
 use std::collections::{HashMap, HashSet};
-use std::fs::File;
+use std::fs::{self, File};
 use std::io::{self, Read, Write};
+use std::path::PathBuf;
 use time;
 use rpassword;
+use tempfile::NamedTempFile;
 
 extern crate openpgp;
 use openpgp::armor;
@@ -17,6 +19,7 @@ use openpgp::packet::signature::subpacket::{Subpacket, SubpacketValue};
 use openpgp::parse::stream::{
     Verifier, VerificationResult, VerificationHelper,
 };
+use openpgp::serialize::Serialize;
 use openpgp::serialize::stream::{
     wrap, Signer, LiteralWriter, Encryptor, EncryptionMode,
 };
@@ -191,11 +194,45 @@ pub fn encrypt(store: &mut store::Store,
     Ok(())
 }
 
-pub fn sign(input: &mut io::Read, output: Option<&str>,
-            secrets: Vec<openpgp::TPK>, detached: bool, binary: bool)
+pub fn sign(input: &mut io::Read, output_path: Option<&str>,
+            secrets: Vec<openpgp::TPK>, detached: bool, binary: bool,
+            append: bool)
             -> Result<()> {
-    let mut output = create_or_stdout(output)?;
-    let output = if ! binary {
+    let (mut output, prepend_sigs, tmp_path):
+    (Box<io::Write>, Vec<Signature>, Option<PathBuf>) =
+        if detached && append && output_path.is_some() {
+            // First, read the existing signatures.
+            let mut sigs = Vec::new();
+            let reader = openpgp::Reader::from_file(output_path.unwrap())?;
+            let mut ppr
+                = openpgp::parse::PacketParser::from_reader(reader)?;
+
+            while let PacketParserResult::Some(mut pp) = ppr {
+                let ((packet, _), (ppr_tmp, _)) = pp.recurse()?;
+                ppr = ppr_tmp;
+
+                match packet {
+                    Packet::Signature(sig) => sigs.push(sig),
+                    p => return Err(
+                        failure::err_msg(
+                            format!("{} in detached signature", p.tag()))
+                            .context("Invalid detached signature").into()),
+                }
+            }
+
+            // Then, create a temporary file to write to.  If we are
+            // successful with adding our signature(s), we rename the
+            // file replacing the old one.
+            let tmp_file = NamedTempFile::new_in(
+                PathBuf::from(output_path.unwrap()).parent()
+                    .unwrap_or(&PathBuf::from(".")))?;
+            let tmp_path = tmp_file.path().into();
+            (Box::new(tmp_file), sigs, Some(tmp_path))
+        } else {
+            (create_or_stdout(output_path)?, Vec::new(), None)
+        };
+
+    let mut output = if ! binary {
         Box::new(armor::Writer::new(&mut output,
                                     if detached {
                                         armor::Kind::Signature
@@ -207,6 +244,12 @@ pub fn sign(input: &mut io::Read, output: Option<&str>,
         output
     };
 
+    // When extending a detached signature, prepend any existing
+    // signatures first.
+    for sig in prepend_sigs {
+        sig.serialize(&mut output)?;
+    }
+
     let sink = wrap(output);
     // Build a vector of references to hand to Signer.
     let keys: Vec<&openpgp::TPK> = secrets.iter().collect();
@@ -232,6 +275,12 @@ pub fn sign(input: &mut io::Read, output: Option<&str>,
 
     writer.finalize()
         .context("Failed to sign")?;
+
+    if let Some(path) = tmp_path {
+        // Atomically replace the old file.
+        fs::rename(path,
+                   output_path.expect("must be Some if tmp_path is Some"))?;
+    }
     Ok(())
 }
 
diff --git a/tool/src/sq-usage.rs b/tool/src/sq-usage.rs
index 9db7f1a5..6a9a8df7 100644
--- a/tool/src/sq-usage.rs
+++ b/tool/src/sq-usage.rs
@@ -87,6 +87,7 @@
 //!     sq sign [FLAGS] [OPTIONS] [--] [FILE]
 //!
 //! FLAGS:
+//!     -a, --append      Append signature to existing signature
 //!     -B, --binary      Don't ASCII-armor encode the OpenPGP data
 //!         --detached    Create a detached signature
 //!     -h, --help        Prints help information
diff --git a/tool/src/sq.rs b/tool/src/sq.rs
index 8c839060..437da9f6 100644
--- a/tool/src/sq.rs
+++ b/tool/src/sq.rs
@@ -5,6 +5,7 @@ extern crate failure;
 #[macro_use]
 extern crate prettytable;
 extern crate rpassword;
+extern crate tempfile;
 extern crate time;
 
 use failure::ResultExt;
@@ -131,10 +132,12 @@ fn real_main() -> Result<(), failure::Error> {
             let output = m.value_of("output");
             let detached = m.is_present("detached");
             let binary = m.is_present("binary");
+            let append = m.is_present("append");
             let secrets = m.values_of("secret-key-file")
                 .map(load_tpks)
                 .unwrap_or(Ok(vec![]))?;
-            commands::sign(&mut input, output, secrets, detached, binary)?;
+            commands::sign(&mut input, output, secrets, detached, binary,
+                           append)?;
         },
         ("verify",  Some(m)) => {
             let input = open_or_stdin(m.value_of("input"))?;
diff --git a/tool/src/sq_cli.rs b/tool/src/sq_cli.rs
index 72c596d0..da38eaa2 100644
--- a/tool/src/sq_cli.rs
+++ b/tool/src/sq_cli.rs
@@ -93,6 +93,10 @@ pub fn build() -> App<'static, 'static> {
                     .arg(Arg::with_name("detached")
                          .long("detached")
                          .help("Create a detached signature"))
+                    .arg(Arg::with_name("append")
+                         .long("append")
+                         .short("a")
+                         .help("Append signature to existing signature"))
                     .arg(Arg::with_name("secret-key-file")
                          .long("secret-key-file")
                          .multiple(true)
diff --git a/tool/tests/sq-sign.rs b/tool/tests/sq-sign.rs
index e8788e8e..d832a419 100644
--- a/tool/tests/sq-sign.rs
+++ b/tool/tests/sq-sign.rs
@@ -104,3 +104,137 @@ fn sq_sign_detached() {
               &p("messages/a-cypherpunks-manifesto.txt")])
         .unwrap();
 }
+
+#[test]
+fn sq_sign_detached_append() {
+    let tmp_dir = TempDir::new().unwrap();
+    let sig = tmp_dir.path().join("sig0");
+
+    // Sign detached.
+    Assert::cargo_binary("sq")
+        .with_args(
+            &["sign",
+              "--detached",
+              "--secret-key-file",
+              &p("keys/dennis-simon-anton-private.pgp"),
+              "--output",
+              &sig.to_string_lossy(),
+              &p("messages/a-cypherpunks-manifesto.txt")])
+        .unwrap();
+
+    // Check that the content is sane.
+    let packets: Vec<Packet> =
+        PacketPile::from_reader(Reader::from_file(&sig).unwrap())
+        .unwrap().into_children().collect();
+    assert_eq!(packets.len(), 1);
+    if let Packet::Signature(ref sig) = packets[0] {
+        assert_eq!(sig.sigtype(), SignatureType::Binary);
+    } else {
+        panic!("expected signature");
+    }
+
+    let content = fs::read(&sig).unwrap();
+    assert!(&content[..].starts_with(b"-----BEGIN PGP SIGNATURE-----\n\n"));
+
+    // Verify detached.
+    Assert::cargo_binary("sqv")
+        .with_args(
+            &["--keyring",
+              &p("keys/dennis-simon-anton.pgp"),
+              &sig.to_string_lossy(),
+              &p("messages/a-cypherpunks-manifesto.txt")])
+        .unwrap();
+
+    // Check that we don't blindly overwrite signatures.
+    Assert::cargo_binary("sq")
+        .with_args(
+            &["sign",
+              "--detached",
+              "--secret-key-file",
+              &p("keys/erika-corinna-daniela-simone-antonia-nistp256-private.pgp"),
+              "--output",
+              &sig.to_string_lossy(),
+              &p("messages/a-cypherpunks-manifesto.txt")])
+        .fails()
+        .unwrap();
+
+    // Now add a second signature with --append.
+    Assert::cargo_binary("sq")
+        .with_args(
+            &["sign",
+              "--detached",
+              "--append",
+              "--secret-key-file",
+              &p("keys/erika-corinna-daniela-simone-antonia-nistp256-private.pgp"),
+              "--output",
+              &sig.to_string_lossy(),
+              &p("messages/a-cypherpunks-manifesto.txt")])
+        .unwrap();
+
+    // Check that the content is sane.
+    let packets: Vec<Packet> =
+        PacketPile::from_reader(Reader::from_file(&sig).unwrap())
+        .unwrap().into_children().collect();
+    assert_eq!(packets.len(), 2);
+    if let Packet::Signature(ref sig) = packets[0] {
+        assert_eq!(sig.sigtype(), SignatureType::Binary);
+    } else {
+        panic!("expected signature");
+    }
+    if let Packet::Signature(ref sig) = packets[1] {
+        assert_eq!(sig.sigtype(), SignatureType::Binary);
+    } else {
+        panic!("expected signature");
+    }
+
+    let content = fs::read(&sig).unwrap();
+    assert!(&content[..].starts_with(b"-----BEGIN PGP SIGNATURE-----\n\n"));
+
+    // Verify both detached signatures.
+    Assert::cargo_binary("sqv")
+        .with_args(
+            &["--keyring",
+              &p("keys/dennis-simon-anton.pgp"),
+              &sig.to_string_lossy(),
+              &p("messages/a-cypherpunks-manifesto.txt")])
+        .unwrap();
+    Assert::cargo_binary("sqv")
+        .with_args(
+            &["--keyring",
+              &p("keys/erika-corinna-daniela-simone-antonia-nistp256.pgp"),
+              &sig.to_string_lossy(),
+              &p("messages/a-cypherpunks-manifesto.txt")])
+        .unwrap();
+
+    // Finally, check that we don't truncate the file if something
+    // goes wrong.
+    Assert::cargo_binary("sq")
+        .with_args(
+            &["sign",
+              "--detached",
+              "--append",
+              "--secret-key-file",
+              // Not a private key => signing will fail.
+              &p("keys/erika-corinna-daniela-simone-antonia-nistp521.pgp"),
+              "--output",
+              &sig.to_string_lossy(),
+              &p("messages/a-cypherpunks-manifesto.txt")])
+        .fails()
+        .unwrap();
+
+    // Check that the content is still sane.
+    let packets: Vec<Packet> =
+        PacketPile::from_reader(Reader::from_file(&sig).unwrap())
+        .unwrap().into_children().collect();
+    assert_eq!(packets.len(), 2);
+    if let Packet::Signature(ref sig) = packets[0] {
+        assert_eq!(sig.sigtype(), SignatureType::Binary);
+    } else {
+        panic!("expected signature");
+    }
+    if let Packet::Signature(ref sig) = packets[1] {
+        assert_eq!(sig.sigtype(), SignatureType::Binary);
+    } else {
+        panic!("expected signature");
+    }
+}