Don't try to certify invalid user IDs.

- `sq pki vouch authorize` certifies all self-signed user IDs when
    no user IDs are provided.  Change it to silently ignore invalid
    self-signed user IDs (e.g., revoked user IDs) in this case.

src/commands/pki/vouch/authorize.rs

@@ -34,7 +34,7 @@ pub fn authorize(sq: Sq, mut c: authorize::Command)
 
     let vc = cert.with_policy(sq.policy, Some(sq.time))?;
     let mut userids = c.userids.resolve(&vc)?;
-    if userids.is_empty() {
+    let user_supplied_userids = if userids.is_empty() {
         // Use all self-signed User IDs.
         userids = vc.userids()
             .map(|ua| ua.userid().clone())
@@ -46,6 +46,10 @@ pub fn authorize(sq: Sq, mut c: authorize::Command)
                  an alternate user ID",
                 vc.fingerprint()));
         }
+
+        false
+    } else {
+        true
     };
 
     let notations = parse_notations(&c.notation)?;
@@ -57,7 +61,7 @@ pub fn authorize(sq: Sq, mut c: authorize::Command)
         &cert,
         &userids[..],
         c.userids.add_userid().unwrap_or(false),
-        true, // User supplied user IDs.
+        user_supplied_userids,
         &[(c.amount, c.expiration)],
         c.depth,
         &c.domain[..],

tests/integration/common.rs

@@ -1241,6 +1241,86 @@ impl Sq {
         Ok(out_key)
     }
 
+    /// Revokes a user ID.
+    pub fn key_userid_revoke_maybe<'a, C, O>(&self, args: &[&str], cert: C, userid: &str,
+                                             reason: &str, message: &str,
+                                             output_file: O)
+        -> Result<Cert>
+    where C: Into<FileOrKeyHandle>,
+          O: Into<Option<&'a Path>>,
+    {
+        let cert = cert.into();
+        let output_file = output_file.into();
+
+        let mut cmd = self.command();
+        cmd.args(["key", "userid", "revoke", reason, message]);
+        for arg in args {
+            cmd.arg(arg);
+        }
+
+        match &cert {
+            FileOrKeyHandle::FileOrStdin(file) => {
+                cmd.arg("--cert-file").arg(file);
+            }
+            FileOrKeyHandle::KeyHandle((_kh, s)) => {
+                cmd.arg("--cert").arg(s);
+            }
+        }
+        cmd.arg("--userid").arg(userid);
+
+        if let Some(output_file) = output_file {
+            cmd.arg("--overwrite").arg("--output").arg(output_file);
+        }
+
+        let output = self.run(cmd, None);
+        if output.status.success() {
+            if let Some(output_file) = output_file {
+                // The output was explicitly written to a file.
+                if output_file == &PathBuf::from("-") {
+                    Ok(Cert::from_bytes(&output.stdout)
+                       .expect("can parse certificate"))
+                } else {
+                    Ok(Cert::from_file(&output_file)
+                       .expect("can parse certificate"))
+                }
+            } else {
+                match cert {
+                    FileOrKeyHandle::FileOrStdin(_) => {
+                        // When the cert is from a file, the output is
+                        // written to stdout by default.
+                        Ok(Cert::from_bytes(&output.stdout)
+                           .with_context(|| {
+                               format!("Importing result from the file {:?}",
+                                       cert)
+                           })
+                           .expect("can parse certificate"))
+                    }
+                    FileOrKeyHandle::KeyHandle((kh, _s)) => {
+                        // When the cert is from the cert store, the
+                        // output is written to the cert store by
+                        // default.
+                        Ok(self.cert_export(kh.clone()))
+                    }
+                }
+            }
+        } else {
+            Err(anyhow::anyhow!(format!(
+                "Failed (expected):\n{}",
+                String::from_utf8_lossy(&output.stderr))))
+        }
+    }
+
+    pub fn key_userid_revoke<'a, C, O>(&self, args: &[&str], cert: C, userid: &str,
+                                       reason: &str, message: &str,
+                                       output_file: O)
+        -> Cert
+    where C: Into<FileOrKeyHandle>,
+          O: Into<Option<&'a Path>>,
+    {
+        self.key_userid_revoke_maybe(args, cert, userid, reason, message, output_file)
+            .expect("succeeds")
+    }
+
     /// Strips user IDs to the given key.
     pub fn toolbox_strip_userid(&self, key: Cert, args: &[&str]) -> Result<Cert> {
         let mut cmd = self.command();

tests/integration/sq_pki_authorize.rs

@@ -250,3 +250,87 @@ fn sq_pki_authorize_then_authenticate() {
         false, // bob@example.org
         false);// bob@other.org
 }
+
+#[test]
+fn sq_pki_authorize_all_revoked() {
+    // When we don't provide any user IDs, `sq pki vouch authorize`
+    // certifies all of the self signed user IDs.  Make sure this
+    // works in the presence of a revoked user ID, which should be
+    // ignored.
+
+    let mut sq = Sq::new();
+
+    let otto_somewhere_com = "<otto@somewhere.com>";
+    let (otto, otto_pgp, _otto_rev)
+        = sq.key_generate(&[], &[otto_somewhere_com]);
+    sq.key_import(&otto_pgp);
+
+    let ca_example_org = "<ca@example.org>";
+    let ca_example_com = "<ca@example.com>";
+    let (ca, ca_pgp, _ca_rev)
+        = sq.key_generate(&[], &[ca_example_org, ca_example_com]);
+    sq.key_import(&ca_pgp);
+
+    // Revoke ca@example.com.
+    sq.tick(1);
+    let revocation = sq.scratch_file("revocation");
+    sq.key_userid_revoke(&[], ca.fingerprint(), ca_example_com,
+                         "retired", "bye", Some(revocation.as_path()));
+    sq.cert_import(&revocation);
+
+    let alice_example_org = "<alice@example.org>";
+    let (alice, alice_pgp, _alice_rev)
+        = sq.key_generate(&[], &[alice_example_org]);
+    sq.key_import(&alice_pgp);
+
+    sq.tick(1);
+
+    // The ca certifies alice's certificate.
+    let certification = sq.scratch_file(None);
+    sq.pki_vouch_certify(
+        &[],
+        ca.key_handle(), alice.key_handle(),
+        &[ alice_example_org ],
+        certification.as_path());
+    sq.cert_import(&certification);
+
+    // Check whether we can authenticate alice's and bob's
+    // certificates for their user ID using otto as the trust root.
+    let check = |sq: &Sq, can_authenticate|
+    {
+        let r = sq.pki_authenticate(
+            &["--trust-root", &otto.fingerprint().to_string() ],
+            &alice.fingerprint().to_string(),
+            alice_example_org);
+
+        match (can_authenticate, r.is_ok()) {
+            (true, false) => {
+                panic!("Expected to authenticated {}, but didn't.",
+                       alice_example_org);
+            }
+            (false, true) => {
+                panic!("Expected to NOT authenticated {}, but did.",
+                       alice_example_org);
+            }
+            _ => (),
+        }
+    };
+
+    // No delegation yet.
+    println!("CA: not authorized");
+    check(&sq, false);
+
+    // Otto completely authorizes the CA.  Note: we don't specify any
+    // user IDs so only valid self-signed user IDs should be used.
+    // That means the revoked user ID should be skipped.
+    let certification = sq.scratch_file(None);
+    sq.tick(1);
+    sq.pki_vouch_authorize(&["--unconstrained"],
+                           otto.key_handle(), ca.key_handle(),
+                           &[],
+                           certification.as_path());
+    sq.cert_import(certification);
+
+    println!("CA: authorized, and unconstrained");
+    check(&sq, true);
+}