From 782291af1426e35ae36a3a52ea5c551bd2348c67 Mon Sep 17 00:00:00 2001
From: Justus Winter <justus@sequoia-pgp.org>
Date: Thu, 21 Jan 2021 23:36:46 +0100
Subject: sq: Align sq armor with SOP.

  - Use --label, use the labels from SOP.  Make auto-detection the
    default.

  - Likewise for sq packet join.
---
 sq/src/commands/mod.rs |  56 ++++++++++++++++++++++---
 sq/src/sq-usage.rs     |  12 +++---
 sq/src/sq.rs           | 112 +++++++++++++++++++++++++++++++++++++++----------
 sq/src/sq_cli.rs       |  19 +++++----
 4 files changed, 155 insertions(+), 44 deletions(-)

diff --git a/sq/src/commands/mod.rs b/sq/src/commands/mod.rs
index 45c1201c..0150441b 100644
--- a/sq/src/commands/mod.rs
+++ b/sq/src/commands/mod.rs
@@ -7,6 +7,9 @@ use std::time::SystemTime;
 use rpassword;
 
 use sequoia_openpgp as openpgp;
+use crate::openpgp::{
+    armor,
+};
 use crate::openpgp::types::{
     CompressionAlgorithm,
 };
@@ -28,6 +31,8 @@ use crate::openpgp::policy::Policy;
 
 use crate::{
     Config,
+    parse_armor_kind,
+    create_or_stdout_pgp,
 };
 
 pub mod decrypt;
@@ -438,17 +443,54 @@ pub fn split(input: &mut (dyn io::Read + Sync + Send), prefix: &str)
 }
 
 /// Joins the given files.
-pub fn join(inputs: Option<clap::Values>, output: &mut dyn io::Write)
+pub fn join(config: Config, m: &clap::ArgMatches)
             -> Result<()> {
+    // Either we know what kind of armor we want to produce, or we
+    // need to detect it using the first packet we see.
+    let kind = parse_armor_kind(m.value_of("kind"));
+    let output = m.value_of("output");
+    let mut sink =
+        if m.is_present("binary") {
+            // No need for any auto-detection.
+            Some(create_or_stdout_pgp(output, config.force,
+                                      true, // Binary.
+                                      armor::Kind::File)?)
+        } else if let Some(kind) = kind {
+            Some(create_or_stdout_pgp(output, config.force,
+                                      false, // Armored.
+                                      kind)?)
+        } else {
+            None // Defer.
+        };
+
     /// Writes a bit-accurate copy of all top-level packets in PPR to
     /// OUTPUT.
-    fn copy(mut ppr: PacketParserResult, output: &mut dyn io::Write)
+    fn copy(mut ppr: PacketParserResult,
+            output: Option<&str>, force: bool,
+            sink: &mut Option<Message>)
             -> Result<()> {
         while let PacketParserResult::Some(pp) = ppr {
+            if sink.is_none() {
+                // Autodetect using the first packet.
+                let kind = match pp.packet {
+                    Packet::Signature(_) => armor::Kind::Signature,
+                    Packet::SecretKey(_) => armor::Kind::SecretKey,
+                    Packet::PublicKey(_) => armor::Kind::PublicKey,
+                    Packet::PKESK(_) | Packet::SKESK(_) =>
+                        armor::Kind::Message,
+                    _ => armor::Kind::File,
+                };
+
+                *sink = Some(create_or_stdout_pgp(output, force,
+                                                  false, // Armored.
+                                                  kind)?);
+            }
+
             // We (ab)use the mapping feature to create byte-accurate
             // copies.
             for field in pp.map().expect("must be mapped").iter() {
-                output.write_all(field.as_bytes())?;
+                sink.as_mut().expect("initialized at this point")
+                    .write_all(field.as_bytes())?;
             }
 
             ppr = pp.next()?.1;
@@ -456,18 +498,20 @@ pub fn join(inputs: Option<clap::Values>, output: &mut dyn io::Write)
         Ok(())
     }
 
-    if let Some(inputs) = inputs {
+    if let Some(inputs) = m.values_of("input") {
         for name in inputs {
             let ppr =
                 openpgp::parse::PacketParserBuilder::from_file(name)?
                 .map(true).build()?;
-            copy(ppr, output)?;
+            copy(ppr, output, config.force, &mut sink)?;
         }
     } else {
         let ppr =
             openpgp::parse::PacketParserBuilder::from_reader(io::stdin())?
             .map(true).build()?;
-        copy(ppr, output)?;
+        copy(ppr, output, config.force, &mut sink)?;
     }
+
+    sink.unwrap().finalize()?;
     Ok(())
 }
diff --git a/sq/src/sq-usage.rs b/sq/src/sq-usage.rs
index cbf29837..57506231 100644
--- a/sq/src/sq-usage.rs
+++ b/sq/src/sq-usage.rs
@@ -761,9 +761,9 @@
 //!     -h, --help    Prints help information
 //!
 //! OPTIONS:
-//!         --kind <KIND>      Selects the kind of armor header [default: file]
-//!                            [possible values: message, publickey, secretkey,
-//!                            signature, file]
+//!         --label <LABEL>    Selects the kind of armor header [default: auto]
+//!                            [possible values: auto, message, cert, key, sig,
+//!                            file]
 //!     -o, --output <FILE>    Writes to FILE or stdout if omitted
 //!
 //! ARGS:
@@ -919,9 +919,9 @@
 //!     -V, --version    Prints version information
 //!
 //! OPTIONS:
-//!         --kind <KIND>      Selects the kind of armor header [default: file]
-//!                            [possible values: message, publickey, secretkey,
-//!                            signature, file]
+//!         --label <LABEL>    Selects the kind of armor header [default: auto]
+//!                            [possible values: auto, message, cert, key, sig,
+//!                            file]
 //!     -o, --output <FILE>    Writes to FILE or stdout if omitted
 //!
 //! ARGS:
diff --git a/sq/src/sq.rs b/sq/src/sq.rs
index 8d4752ac..c7968704 100644
--- a/sq/src/sq.rs
+++ b/sq/src/sq.rs
@@ -2,13 +2,13 @@
 
 use anyhow::Context as _;
 use std::fs::OpenOptions;
-use std::io::{self, Write};
+use std::io::{self, Read, Write};
 use std::path::{Path, PathBuf};
 use std::time::Duration;
 use chrono::{DateTime, offset::Utc};
 use itertools::Itertools;
 
-use buffered_reader::File;
+use buffered_reader::{BufferedReader, Dup, File, Generic, Limitor};
 use sequoia_openpgp as openpgp;
 
 use openpgp::{
@@ -20,7 +20,7 @@ use crate::openpgp::crypto::Password;
 use crate::openpgp::fmt::hex;
 use crate::openpgp::types::KeyFlags;
 use crate::openpgp::packet::prelude::*;
-use crate::openpgp::parse::Parse;
+use crate::openpgp::parse::{Parse, PacketParser, PacketParserResult};
 use crate::openpgp::serialize::{Serialize, stream::{Message, Armorer}};
 use crate::openpgp::cert::prelude::*;
 use crate::openpgp::policy::StandardPolicy as P;
@@ -28,11 +28,12 @@ use crate::openpgp::policy::StandardPolicy as P;
 mod sq_cli;
 mod commands;
 
-fn open_or_stdin(f: Option<&str>) -> Result<Box<dyn io::Read + Send + Sync>> {
+fn open_or_stdin(f: Option<&str>)
+                 -> Result<Box<dyn BufferedReader<()>>> {
     match f {
         Some(f) => Ok(Box::new(File::open(f)
                                .context("Failed to open input file")?)),
-        None => Ok(Box::new(io::stdin())),
+        None => Ok(Box::new(Generic::new(io::stdin(), None))),
     }
 }
 
@@ -207,17 +208,47 @@ fn serialize_keyring(mut output: &mut dyn io::Write, certs: &[Cert], binary: boo
     Ok(())
 }
 
-fn parse_armor_kind(kind: Option<&str>) -> armor::Kind {
+fn parse_armor_kind(kind: Option<&str>) -> Option<armor::Kind> {
     match kind.expect("has default value") {
-        "message" => armor::Kind::Message,
-        "publickey" => armor::Kind::PublicKey,
-        "secretkey" => armor::Kind::SecretKey,
-        "signature" => armor::Kind::Signature,
-        "file" => armor::Kind::File,
+        "auto" =>    None,
+        "message" => Some(armor::Kind::Message),
+        "cert" =>    Some(armor::Kind::PublicKey),
+        "key" =>     Some(armor::Kind::SecretKey),
+        "sig" =>     Some(armor::Kind::Signature),
+        "file" =>    Some(armor::Kind::File),
         _ => unreachable!(),
     }
 }
 
+/// Peeks at the first packet to guess the type.
+///
+/// Returns the given reader unchanged.  If the detection fails,
+/// armor::Kind::File is returned as safe default.
+fn detect_armor_kind(input: Box<dyn BufferedReader<()>>)
+                     -> (Box<dyn BufferedReader<()>>, armor::Kind) {
+    let mut dup = Limitor::new(Dup::new(input), 1 << 24).as_boxed();
+    let kind = 'detection: loop {
+        if let Ok(ppr) = PacketParser::from_reader(&mut dup) {
+            if let PacketParserResult::Some(pp) = ppr {
+                let (packet, _) = match pp.next() {
+                    Ok(v) => v,
+                    Err(_) => break 'detection armor::Kind::File,
+                };
+
+                break 'detection match packet {
+                    Packet::Signature(_) => armor::Kind::Signature,
+                    Packet::SecretKey(_) => armor::Kind::SecretKey,
+                    Packet::PublicKey(_) => armor::Kind::PublicKey,
+                    Packet::PKESK(_) | Packet::SKESK(_) =>
+                        armor::Kind::Message,
+                    _ => armor::Kind::File,
+                };
+            }
+        }
+    };
+    (dup.into_inner().unwrap().into_inner().unwrap(), kind)
+}
+
 // Decrypts a key, if possible.
 //
 // The passwords in `passwords` are tried first.  If the key can't be
@@ -424,12 +455,54 @@ fn main() -> Result<()> {
         },
 
         ("armor",  Some(m)) => {
-            let mut input = open_or_stdin(m.value_of("input"))?;
+            let input = open_or_stdin(m.value_of("input"))?;
+            let mut want_kind = parse_armor_kind(m.value_of("kind"));
+
+            // Peek at the data.  If it looks like it is armored
+            // data, avoid armoring it again.
+            let mut dup = Limitor::new(Dup::new(input), 1 << 24);
+            let (already_armored, have_kind) = {
+                let mut reader =
+                    armor::Reader::new(&mut dup,
+                                       armor::ReaderMode::Tolerant(None));
+                let mut buf = [0; 8];
+                (reader.read(&mut buf).is_ok(), reader.kind())
+            };
+            let mut input =
+                dup.as_boxed().into_inner().unwrap().into_inner().unwrap();
+
+            if already_armored
+                && (want_kind.is_none() || want_kind == have_kind)
+            {
+                // It is already armored and has the correct kind.
+                let mut output =
+                    create_or_stdout(m.value_of("output"), force)?;
+                io::copy(&mut input, &mut output)?;
+                return Ok(());
+            }
+
+            if want_kind.is_none() {
+                let (tmp, kind) = detect_armor_kind(input);
+                input = tmp;
+                want_kind = Some(kind);
+            }
+
+            // At this point, want_kind is determined.
+            let want_kind = want_kind.expect("given or detected");
+
             let mut output =
                 create_or_stdout_pgp(m.value_of("output"), force,
-                                     false,
-                                     parse_armor_kind(m.value_of("kind")))?;
-            io::copy(&mut input, &mut output)?;
+                                     false, want_kind)?;
+
+            if already_armored {
+                // Dearmor and copy to change the type.
+                let mut reader =
+                    armor::Reader::new(input,
+                                       armor::ReaderMode::Tolerant(None));
+                io::copy(&mut reader, &mut output)?;
+            } else {
+                io::copy(&mut input, &mut output)?;
+            }
             output.finalize()?;
         },
         ("dearmor",  Some(m)) => {
@@ -536,14 +609,7 @@ fn main() -> Result<()> {
                             + "-");
                 commands::split(&mut input, &prefix)?;
             },
-            ("join",  Some(m)) => {
-                let mut output =
-                    create_or_stdout_pgp(m.value_of("output"), force,
-                                         m.is_present("binary"),
-                                         parse_armor_kind(m.value_of("kind")))?;
-                commands::join(m.values_of("input"), &mut output)?;
-                output.finalize()?;
-            },
+            ("join",  Some(m)) => commands::join(config, m)?,
             _ => unreachable!(),
         },
 
diff --git a/sq/src/sq_cli.rs b/sq/src/sq_cli.rs
index d5dba4aa..edce5f7d 100644
--- a/sq/src/sq_cli.rs
+++ b/sq/src/sq_cli.rs
@@ -218,10 +218,11 @@ pub fn configure(app: App<'static, 'static>) -> App<'static, 'static> {
                          .short("o").long("output").value_name("FILE")
                          .help("Writes to FILE or stdout if omitted"))
                     .arg(Arg::with_name("kind")
-                         .long("kind").value_name("KIND")
-                         .possible_values(&["message", "publickey", "secretkey",
-                                            "signature", "file"])
-                         .default_value("file")
+                         .long("label").value_name("LABEL")
+                         .possible_values(&["auto", "message",
+                                            "cert", "key", "sig",
+                                            "file"])
+                         .default_value("auto")
                          .help("Selects the kind of armor header"))
         )
 
@@ -708,11 +709,11 @@ pub fn configure(app: App<'static, 'static>) -> App<'static, 'static> {
                                      .short("o").long("output").value_name("FILE")
                                      .help("Writes to FILE or stdout if omitted"))
                                 .arg(Arg::with_name("kind")
-                                     .long("kind").value_name("KIND")
-                                     .possible_values(&["message", "publickey",
-                                                        "secretkey",
-                                                        "signature", "file"])
-                                     .default_value("file")
+                                     .long("label").value_name("LABEL")
+                                     .possible_values(&["auto", "message",
+                                                        "cert", "key", "sig",
+                                                        "file"])
+                                     .default_value("auto")
                                      .conflicts_with("binary")
                                      .help("Selects the kind of armor header"))
                                 .arg(Arg::with_name("binary")
-- 
cgit v1.2.3