Add sq key subkey add command to add newly generated subkeys

- Add `sq key subkey add` to allow to add a newly generated `SubKey` to
  an existing key.
- Add `sq_cli::types::Expiry` to allow providing expiry with a
  single `--expiry` input argument, that covers providing an ISO 8601
  timestamp, a custom duration and "never".
- Add impl block for `sq_cli:🔑:CipherSuite` to allow returning a
  `sequoia_openpgp::cert::CipherSuite`.

src/commands/key.rs

@@ -14,6 +14,8 @@ mod generate;
 use generate::generate;
 mod password;
 use password::password;
+mod subkey;
+use subkey::subkey;
 mod userid;
 use userid::userid;
 
@@ -23,6 +25,7 @@ pub fn dispatch(config: Config, command: sq_cli::key::Command) -> Result<()> {
         Generate(c) => generate(config, c)?,
         Password(c) => password(config, c)?,
         Userid(c) => userid(config, c)?,
+        Subkey(c) => subkey(config, c)?,
         ExtractCert(c) => extract_cert(config, c)?,
         Adopt(c) => adopt(config, c)?,
         AttestCertifications(c) => attest_certifications(config, c)?,

src/commands/key/subkey.rs

@@ -0,0 +1,70 @@
+use chrono::DateTime;
+use chrono::Utc;
+
+use openpgp::cert::KeyBuilder;
+use openpgp::parse::Parse;
+use openpgp::serialize::Serialize;
+use openpgp::types::KeyFlags;
+use openpgp::Cert;
+use openpgp::Result;
+use sequoia_openpgp as openpgp;
+
+use crate::open_or_stdin;
+use crate::sq_cli::key::EncryptPurpose;
+use crate::sq_cli::key::SubkeyCommand;
+use crate::sq_cli::key::SubkeyAddCommand;
+use crate::Config;
+
+pub fn subkey(config: Config, command: SubkeyCommand) -> Result<()> {
+    match command {
+        SubkeyCommand::Add(c) => subkey_add(config, c)?,
+    }
+
+    Ok(())
+}
+
+/// Add a new Subkey for an existing primary key
+///
+/// Creates a subkey with features (e.g. `KeyFlags`, `CipherSuite`) based on
+/// user input (or application-wide defaults if not specified).
+/// If no specific expiry is requested, the subkey never expires.
+fn subkey_add(
+    config: Config,
+    command: SubkeyAddCommand,
+) -> Result<()> {
+    let input = open_or_stdin(command.io.input.as_deref())?;
+    let cert = Cert::from_reader(input)?;
+    let valid_cert = cert.with_policy(&config.policy, config.time)?;
+
+    let validity = command
+        .expiry
+        .as_duration(DateTime::<Utc>::from(config.time))?;
+
+    let keyflags = KeyFlags::empty()
+        .set_authentication_to(command.can_authenticate)
+        .set_signing_to(command.can_sign)
+        .set_storage_encryption_to(matches!(
+            command.can_encrypt,
+            Some(EncryptPurpose::Storage) | Some(EncryptPurpose::Universal)
+        ))
+        .set_transport_encryption_to(matches!(
+            command.can_encrypt,
+            Some(EncryptPurpose::Transport) | Some(EncryptPurpose::Universal)
+        ));
+
+    let new_cert = KeyBuilder::new(keyflags)
+        .set_creation_time(config.time)
+        .set_cipher_suite(command.cipher_suite.as_ciphersuite())
+        .subkey(valid_cert)?
+        .set_key_validity_period(validity)?
+        .attach_cert()?;
+
+    let mut sink =
+        config.create_or_stdout_safe(command.io.output.as_deref())?;
+    if command.binary {
+        new_cert.as_tsk().serialize(&mut sink)?;
+    } else {
+        new_cert.as_tsk().armored().serialize(&mut sink)?;
+    }
+    Ok(())
+}

src/sq_cli/key.rs

@@ -1,6 +1,10 @@
 use clap::{ValueEnum, ArgGroup, Args, Parser, Subcommand};
 
-use crate::sq_cli::types::{self, IoArgs};
+use sequoia_openpgp::cert::CipherSuite as SqCipherSuite;
+
+use crate::sq_cli::types::IoArgs;
+use crate::sq_cli::types::Expiry;
+use crate::sq_cli::types::Time;
 
 #[derive(Parser, Debug)]
 #[clap(
@@ -31,6 +35,8 @@ pub enum Subcommands {
     Password(PasswordCommand),
     #[clap(subcommand)]
     Userid(UseridCommand),
+    #[clap(subcommand)]
+    Subkey(SubkeyCommand),
     ExtractCert(ExtractCertCommand),
     AttestCertifications(AttestCertificationsCommand),
     Adopt(AdoptCommand),
@@ -194,6 +200,18 @@ pub enum CipherSuite {
     Cv25519
 }
 
+impl CipherSuite {
+
+    /// Return a matching `sequoia_openpgp::cert::CipherSuite`
+    pub fn as_ciphersuite(&self) -> SqCipherSuite {
+        match self {
+            CipherSuite::Rsa3k => SqCipherSuite::RSA3k,
+            CipherSuite::Rsa4k => SqCipherSuite::RSA4k,
+            CipherSuite::Cv25519 => SqCipherSuite::Cv25519,
+        }
+    }
+}
+
 #[derive(ValueEnum, Clone, Debug)]
 pub enum EncryptPurpose {
     Transport,
@@ -447,7 +465,7 @@ pub struct AdoptCommand {
         value_name = "KEY-EXPIRATION-TIME",
         help = "Makes adopted subkeys expire at the given time",
     )]
-    pub expire: Option<types::Time>,
+    pub expire: Option<Time>,
     #[clap(
         long = "allow-broken-crypto",
         help = "Allows adopting keys from certificates \
@@ -534,3 +552,138 @@ pub struct AttestCertificationsCommand {
     pub binary: bool,
 
 }
+
+#[derive(Debug, Subcommand)]
+#[clap(
+    name = "subkey",
+    about = "Manages Subkeys",
+    long_about =
+"Manages Subkeys
+
+Add new subkeys to an existing key.
+",
+    subcommand_required = true,
+    arg_required_else_help = true,
+)]
+#[non_exhaustive]
+pub enum SubkeyCommand {
+    Add(SubkeyAddCommand),
+}
+
+#[derive(Debug, Args)]
+#[clap(
+    about = "Adds a newly generated Subkey",
+    long_about =
+"Adds a newly generated Subkey
+
+A subkey has one or more flags. \"--can-sign\" sets the signing flag,
+and means that the key may be used for signing. \"--can-authenticate\"
+sets the authentication flags, and means that the key may be used for
+authentication (e.g., as an SSH key). These two flags may be combined.
+
+\"--can-encrypt=storage\" sets the storage encryption flag, and means that the key
+may be used for storage encryption. \"--can-encrypt=transport\" sets the transport
+encryption flag, and means that the key may be used for transport encryption.
+\"--can-encrypt=universal\" sets both the storage and the transport encryption
+flag, and means that the key may be used for both storage and transport
+encryption. Only one of the encryption flags may be used and it can not be
+combined with the signing or authentication flag.
+
+At least one flag must be chosen.
+
+Furthermore the subkey may use one of several available cipher suites, that can
+be selected using \"--cipher-suite\".
+
+By default a new subkey never expires. However, its validity period is limited
+by that of the primary key it is added for.
+Using the \"--expiry=EXPIRY\" argument specific validity periods may be defined.
+It allows for providing a point in time for validity to end or a validity
+duration.
+
+\"sq key subkey add\" respects the reference time set by the top-level
+\"--time\" argument. It sets the creation time of the subkey to the specified
+time.
+",
+    after_help =
+"EXAMPLES:
+
+# First, this generates a key
+$ sq key generate --userid \"alice <alice@example.org>\" --export alice.key.pgp
+
+# Add a new Subkey for universal encryption which expires at the same time as
+# the primary key
+$ sq key subkey add --output alice-new.key.pgp --can-encrypt universal alice.key.pgp
+
+# Add a new Subkey for signing using the rsa3k cipher suite which expires in five days
+$ sq key subkey add --output alice-new.key.pgp --can-sign --cipher-suite rsa3k --expiry 5d alice.key.pgp
+",
+)]
+#[clap(group(ArgGroup::new("authentication-group").args(&["can_authenticate", "can_encrypt"])))]
+#[clap(group(ArgGroup::new("sign-group").args(&["can_sign", "can_encrypt"])))]
+#[clap(group(ArgGroup::new("required-group").args(&["can_authenticate", "can_sign", "can_encrypt"]).required(true)))]
+pub struct SubkeyAddCommand {
+    #[clap(flatten)]
+    pub io: IoArgs,
+    #[clap(
+        long = "private-key-store",
+        value_name = "KEY_STORE",
+        help = "Provides parameters for private key store",
+    )]
+    pub private_key_store: Option<String>,
+    #[clap(
+        short = 'B',
+        long,
+        help = "Emits binary data",
+    )]
+    pub binary: bool,
+    #[clap(
+        short = 'c',
+        long = "cipher-suite",
+        value_name = "CIPHER-SUITE",
+        default_value_t = CipherSuite::Cv25519,
+        help = "Selects the cryptographic algorithms for the subkey",
+        value_enum,
+    )]
+    pub cipher_suite: CipherSuite,
+    #[clap(
+        long = "expiry",
+        value_name = "EXPIRY",
+        default_value_t = Expiry::Never,
+        help =
+            "Defines EXPIRY for the subkey as ISO 8601 formatted string or \
+            custom duration.",
+        long_help =
+            "Defines EXPIRY for the subkey as ISO 8601 formatted string or \
+            custom duration. \
+            If an ISO 8601 formatted string is provided, the validity period \
+            reaches from the reference time (may be set using \"--time\") to \
+            the provided time. \
+            Custom durations starting from the reference time may be set using \
+            \"N[ymwds]\", for N years, months, weeks, days, or seconds. \
+            The special keyword \"never\" sets an unlimited expiry.",
+    )]
+    pub expiry: Expiry,
+    #[clap(
+        long = "can-sign",
+        help = "Adds signing capability to subkey",
+    )]
+    pub can_sign: bool,
+    #[clap(
+        long = "can-authenticate",
+        help = "Adds authentication capability to subkey",
+    )]
+    pub can_authenticate: bool,
+    #[clap(
+        long = "can-encrypt",
+        value_name = "PURPOSE",
+        help = "Adds an encryption capability to subkey [default: universal]",
+        long_help =
+            "Adds an encryption capability to subkey. \
+            Encryption-capable subkeys can be marked as \
+            suitable for transport encryption, storage \
+            encryption, or both, i.e., universal. \
+            [default: universal]",
+        value_enum,
+    )]
+    pub can_encrypt: Option<EncryptPurpose>,
+}

src/sq_cli/types.rs

@@ -1,4 +1,13 @@
-use anyhow::{anyhow, Result};
+use std::fmt::Display;
+use std::fmt::Formatter;
+use std::str::FromStr;
+use std::time::Duration;
+use std::time::SystemTime;
+
+use anyhow::anyhow;
+use anyhow::Context;
+use anyhow::Result;
+
 use chrono::{offset::Utc, DateTime};
 /// Common types for arguments of sq.
 use clap::{ValueEnum, Args};
@@ -7,6 +16,9 @@ use openpgp::fmt::hex;
 use openpgp::types::SymmetricAlgorithm;
 use sequoia_openpgp as openpgp;
 
+use crate::sq_cli::SECONDS_IN_DAY;
+use crate::sq_cli::SECONDS_IN_YEAR;
+
 #[derive(Debug, Args)]
 pub struct IoArgs {
     #[clap(value_name = "FILE", help = "Reads from FILE or stdin if omitted")]
@@ -46,6 +58,150 @@ impl From<ArmorKind> for Option<openpgp::armor::Kind> {
     }
 }
 
+/// Expiry information
+///
+/// This enum tracks expiry information either in the form of a timestamp or
+/// a duration.
+#[derive(Debug, Clone, Eq, PartialEq)]
+#[non_exhaustive]
+pub enum Expiry {
+    /// An expiry timestamp
+    Timestamp(Time),
+    /// A validity duration
+    Duration(Duration),
+    /// There is no expiry
+    Never,
+}
+
+impl Expiry {
+    /// Create a new Expiry in a Result
+    ///
+    /// If `expiry` ends with `"y"`, `"m"`, `"w"`, `"w"`, `"d"` or `"s"` it
+    /// is treated as a duration, which is parsed using `parse_duration()` and
+    /// returned in an `Expiry::Duration`.
+    /// If the special keyword `"never"` is provided as `expiry`,
+    /// `Expiry::Never` is returned.
+    /// If `expiry` is an ISO 8601 compatible string it is returned as
+    /// `sq_cli::types::Time` in an `Expiry::Timestamp`.
+    pub fn new(expiry: &str) -> Result<Self> {
+        match expiry {
+            "never" => Ok(Expiry::Never),
+            _ if expiry.ends_with("y")
+                || expiry.ends_with("m")
+                || expiry.ends_with("w")
+                || expiry.ends_with("d")
+                || expiry.ends_with("s") =>
+            {
+                Ok(Expiry::Duration(Expiry::parse_duration(expiry)?))
+            }
+            _ => Ok(Expiry::Timestamp(Time::from_str(expiry)?)),
+        }
+    }
+
+    /// Parse a string as Duration and return it in a Result
+    ///
+    /// The `expiry` must be at least two chars long, and consist of digits and
+    /// a trailing factor identifier (one of `"y"`, `"m"`, `"w"`, `"d"`, `"s"`
+    /// for year, month, week, day or second, respectively).
+    fn parse_duration(expiry: &str) -> Result<Duration> {
+        if expiry.len() < 2 {
+            return Err(anyhow::anyhow!(
+                "Expiry must contain at least one digit and one factor."
+            ));
+        }
+
+        match expiry.strip_suffix(['y', 'm', 'w', 'd', 's']) {
+            Some(digits) => Ok(Duration::new(
+                match digits.parse::<i64>() {
+                    Ok(count) if count < 0 => {
+                        return Err(anyhow::anyhow!(
+                            "Negative expiry ('{}') detected. \
+                            Did you mean '{}'?",
+                            expiry,
+                            expiry.trim_start_matches("-")
+                        ))
+                    }
+                    Ok(count) => count as u64,
+                    Err(err) => return Err(err).context(
+                        format!("Expiry '{}' is out of range", digits)
+                    ),
+                } * match expiry.chars().last() {
+                    Some('y') => SECONDS_IN_YEAR,
+                    Some('m') => SECONDS_IN_YEAR / 12,
+                    Some('w') => 7 * SECONDS_IN_DAY,
+                    Some('d') => SECONDS_IN_DAY,
+                    Some('s') => 1,
+                    _ => unreachable!(
+                        "Expiry without 'y', 'm', 'w', 'd' or 's' \
+                                suffix impossible since checked for it."
+                    ),
+                },
+                0,
+            )),
+            None => {
+                return Err(anyhow::anyhow!(
+                    if let Some(suffix) = expiry.chars().last() {
+                        format!(
+                            "Invalid suffix '{}' in duration '{}' \
+                        (try <digits><y|m|w|d|s>, e.g. '1y')",
+                            suffix,
+                            expiry
+                        )
+                    } else {
+                        format!(
+                            "Invalid duration: {} \
+                        (try <digits><y|m|w|d|s>, e.g. '1y')",
+                            expiry
+                        )
+                    }
+                ))
+            }
+        }
+    }
+
+    /// Return the expiry as an optional Duration in a Result
+    ///
+    /// This method returns an Error if the reference time is later than the
+    /// time provided in an `Expiry::Timestamp(Time)`.
+    ///
+    /// If self is `Expiry::Timestamp(Time)`, `reference` is used as the start
+    /// of a period, `Some(Time - reference)` is returned.
+    /// If self is `Expiry::Duration(duration)`, `Some(duration)` is returned.
+    /// If self is `Expiry::Never`, `None` is returned.
+    pub fn as_duration(
+        &self,
+        reference: DateTime<Utc>,
+    ) -> Result<Option<Duration>> {
+        match self {
+            Expiry::Timestamp(time) => Ok(
+                Some(
+                    SystemTime::from(time.time).duration_since(reference.into())?
+                )
+            ),
+            Expiry::Duration(duration) => Ok(Some(duration.clone())),
+            Expiry::Never => Ok(None),
+        }
+    }
+}
+
+impl FromStr for Expiry {
+    type Err = anyhow::Error;
+
+    fn from_str(s: &str) -> Result<Expiry> {
+        Expiry::new(s)
+    }
+}
+
+impl Display for Expiry {
+    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
+        match self {
+            Expiry::Timestamp(time) => write!(f, "{:?}", time),
+            Expiry::Duration(duration) => write!(f, "{:?}", duration),
+            Expiry::Never => write!(f, "never"),
+        }
+    }
+}
+
 #[derive(ValueEnum, Clone, Debug)]
 pub enum NetworkPolicy {
     Offline,
@@ -132,7 +288,7 @@ impl<'a> std::fmt::Display for SessionKeyDisplay<'a> {
     }
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct Time {
     pub time: DateTime<Utc>,
 }
@@ -197,6 +353,8 @@ impl Time {
 
 #[cfg(test)]
 mod test {
+    use chrono::NaiveDateTime;
+
     use super::*;
 
     #[test]
@@ -225,4 +383,77 @@ mod test {
         // CliTime::parse_iso8601("2017", z)?; // ditto
         Ok(())
     }
+
+    #[test]
+    fn test_expiry() {
+        assert_eq!(
+            Expiry::new("1y").unwrap(),
+            Expiry::Duration(Duration::new(SECONDS_IN_YEAR, 0)),
+        );
+        assert_eq!(
+            Expiry::new("2023-05-15T20:00:00Z").unwrap(),
+            Expiry::Timestamp(Time::from_str("2023-05-15T20:00:00Z").unwrap()),
+        );
+        assert_eq!(
+            Expiry::new("never").unwrap(),
+            Expiry::Never,
+        );
+    }
+
+    #[test]
+    fn test_expiry_parse_duration() {
+        assert_eq!(
+            Expiry::parse_duration("1y").unwrap(),
+            Duration::new(SECONDS_IN_YEAR, 0),
+        );
+        assert!(Expiry::parse_duration("f").is_err());
+        assert!(Expiry::parse_duration("-1y").is_err());
+        assert!(Expiry::parse_duration("foo").is_err());
+        assert!(Expiry::parse_duration("1o").is_err());
+    }
+
+    #[test]
+    fn test_expiry_as_duration() {
+        let reference = DateTime::from_utc(
+            NaiveDateTime::from_timestamp_opt(1, 0).unwrap(),
+            Utc,
+        );
+
+        let expiry = Expiry::Timestamp(
+            Time{
+                time: DateTime::from_utc(
+                    NaiveDateTime::from_timestamp_opt(2, 0).unwrap(),
+                    Utc,
+                )}
+        );
+        assert_eq!(
+            expiry.as_duration(reference).unwrap(),
+            Some(Duration::new(1, 0)),
+        );
+
+        let expiry = Expiry::Duration(Duration::new(2,0));
+        assert_eq!(
+            expiry.as_duration(reference).unwrap(),
+            Some(Duration::new(2, 0)),
+        );
+
+        let expiry = Expiry::Never;
+        assert_eq!(expiry.as_duration(reference).unwrap(), None);
+    }
+
+    #[test]
+    fn test_expiry_as_duration_errors() {
+        let reference = DateTime::from_utc(
+            NaiveDateTime::from_timestamp_opt(2, 0).unwrap(),
+            Utc,
+        );
+        let expiry = Expiry::Timestamp(
+            Time{
+                time: DateTime::from_utc(
+                    NaiveDateTime::from_timestamp_opt(1, 0).unwrap(),
+                    Utc,
+                )}
+        );
+        assert!(expiry.as_duration(reference).is_err());
+    }
 }

tests/sq-key-subkey.rs

@@ -0,0 +1,172 @@
+use assert_cmd::Command;
+
+use openpgp::parse::Parse;
+use openpgp::policy::StandardPolicy;
+use openpgp::Cert;
+use openpgp::Result;
+use sequoia_openpgp as openpgp;
+
+mod integration {
+    use super::*;
+
+    use std::path::PathBuf;
+    use tempfile::TempDir;
+
+    const P: &StandardPolicy = &StandardPolicy::new();
+
+    /// Generate a new key in a temporary directory and return its TempDir,
+    /// PathBuf and creation times in a Result
+    fn sq_key_generate() -> Result<(TempDir, PathBuf, String, u64)> {
+        let tmpdir = TempDir::new().unwrap();
+        let path = tmpdir.path().join("key.pgp");
+        let timestamp = "20220120T163236+0100";
+        let seconds = 1642692756;
+
+        let mut cmd = Command::cargo_bin("sq")?;
+        cmd.args([
+            "--no-cert-store",
+            "key",
+            "generate",
+            "--time",
+            timestamp,
+            "--expires", "never",
+            "--export",
+            &*path.to_string_lossy(),
+        ]);
+        cmd.assert().success();
+
+        let original_cert = Cert::from_file(&path)?;
+        let original_valid_cert = original_cert.with_policy(P, None)?;
+        assert_eq!(
+            original_valid_cert
+                .keys()
+                .filter(|x| x.for_authentication())
+                .count(),
+            1
+        );
+        assert_eq!(
+            original_valid_cert
+                .keys()
+                .filter(|x| x.for_certification())
+                .count(),
+            1
+        );
+        assert_eq!(
+            original_valid_cert
+                .keys()
+                .filter(|x| x.for_signing())
+                .count(),
+            1
+        );
+        assert_eq!(
+            original_valid_cert
+                .keys()
+                .filter(|x| x.for_storage_encryption())
+                .count(),
+            1
+        );
+        assert_eq!(
+            original_valid_cert
+                .keys()
+                .filter(|x| x.for_transport_encryption())
+                .count(),
+            1
+        );
+
+        Ok((tmpdir, path, timestamp.to_string(), seconds))
+    }
+
+    #[test]
+    fn sq_key_subkey_generate_authentication_subkey() -> Result<()> {
+        let (tmpdir, path, _, _) = sq_key_generate().unwrap();
+        let output = path.parent().unwrap().join("new_key.pgp");
+
+        let mut cmd = Command::cargo_bin("sq")?;
+        cmd.args([
+            "--no-cert-store",
+            "key",
+            "subkey",
+            "add",
+            "--output",
+            &output.to_string_lossy(),
+            "--can-authenticate",
+            &path.to_string_lossy(),
+        ]);
+        cmd.assert().success();
+
+        let cert = Cert::from_file(&output)?;
+        let valid_cert = cert.with_policy(P, None)?;
+
+        assert_eq!(
+            valid_cert.keys().filter(|x| x.for_authentication()).count(),
+            2
+        );
+        tmpdir.close()?;
+        Ok(())
+    }
+
+    #[test]
+    fn sq_key_subkey_generate_encryption_subkey() -> Result<()> {
+        let (tmpdir, path, _, _) = sq_key_generate().unwrap();
+        let output = path.parent().unwrap().join("new_key.pgp");
+
+        let mut cmd = Command::cargo_bin("sq")?;
+        cmd.args([
+            "--no-cert-store",
+            "key",
+            "subkey",
+            "add",
+            "--output",
+            &output.to_string_lossy(),
+            "--can-encrypt=universal",
+            &path.to_string_lossy(),
+        ]);
+        cmd.assert().success();
+
+        let cert = Cert::from_file(&output)?;
+        let valid_cert = cert.with_policy(P, None)?;
+
+        assert_eq!(
+            valid_cert
+                .keys()
+                .filter(|x| x.for_storage_encryption())
+                .count(),
+            2
+        );
+        assert_eq!(
+            valid_cert
+                .keys()
+                .filter(|x| x.for_transport_encryption())
+                .count(),
+            2
+        );
+        tmpdir.close()?;
+        Ok(())
+    }
+
+    #[test]
+    fn sq_key_subkey_generate_signing_subkey() -> Result<()> {
+        let (tmpdir, path, _, _) = sq_key_generate().unwrap();
+        let output = path.parent().unwrap().join("new_key.pgp");
+
+        let mut cmd = Command::cargo_bin("sq")?;
+        cmd.args([
+            "--no-cert-store",
+            "key",
+            "subkey",
+            "add",
+            "--output",
+            &output.to_string_lossy(),
+            "--can-sign",
+            &path.to_string_lossy(),
+        ]);
+        cmd.assert().success();
+
+        let cert = Cert::from_file(&output)?;
+        let valid_cert = cert.with_policy(P, None)?;
+
+        assert_eq!(valid_cert.keys().filter(|x| x.for_signing()).count(), 2);
+        tmpdir.close()?;
+        Ok(())
+    }
+}