Auto merge of #8934 - ehuss:token-process, r=alexcrichton

Implement external credential process. (RFC 2730)

This adds a config setting for an external process to run to fetch the token for a registry. See `unstable.md` for more details.

As part of this, it adds a new `logout` command. This is currently gated on nightly with the appropriate `-Z` flag.

I have included four sample wrappers that integrate with the macOS Keychain, Windows Credential Manager, GNOME libsecret, and 1password. I'm not sure if we'll ultimately ship these, but I would like to. Primarily this provided a proof-of-concept to see if the design works.

**Patch Walkthrough**

This is a brief overview of the changes:

- Adds the `logout` command. With `cargo logout -Z unstable-options`, this allows removing the `token` from `.cargo/credentials`.  With `cargo logout -Z credential-process`, this launches the process with the `erase` argument to remove the token from storage.
- Credential-process handling is in the `ops/registry/auth.rs` module. I think it is pretty straightforward, it just launches the process with the appropriate store/get/erase argument.
- `ops::registry::registry()` now returns the `RegistryConfig` to make it easier to pass the config information around.
- `crates/credential/cargo-credential` is a helper crate for writing credential processes.
- A special shorthand of the `cargo:` prefix for a credential process will launch the named process from the `libexec` directory in the sysroot (or, more specifically, the `libexec` directory next to the `cargo` process). For example `credential-process = "cargo:macos-keychain"`. My intent is to bundle these in the pre-built rust-lang distributions, and this should "just work" when used with rustup. I'm not sure how that will work with other Rust distributions, but I'm guessing they can figure it out. This should make it much easier for users to get started, but does add some integration complexity.

**Questions**

- I'm on the fence about the name `credential-process` vs `credentials-process`, which sounds more natural? (Or something else?)
- I'm uneasy about the behavior when both `token` and `credential-process` is specified (see `warn_both_token_and_process` test). Currently it issues a warning and uses `token`. Does that make sense? What about the case where you have `registries.foo.token` for a specific registry, but then have a general `registry.credential-process` for the default (it currently warns and uses the token, maybe it should not warn?)?
- I am still pretty uneasy with writing FFI wrappers, so maybe those could get a little extra scrutiny? They seem to work, but I have not extensively tested them (I tried login, publish, and logout). I have not previously used these APIs, so I am not familiar with them.
- Testing the wrappers I think will be quite difficult, because some require TTY interaction (and 1password requires an online account). Or, for example in the macOS case, it has GUI dialog box where I can use my fingerprint scanner. Right now, I just build them in CI to make sure they compile.
- 1password is a little weird in that it passes the token on the command-line, which is not very secure on some systems (other processes can see these sometimes). The only alternative I can think of is to not support `cargo login` and require the user to manually enter the token in the 1password GUI. I don't think the concern is too large (1password themselves seem to think it is acceptable). Should this be OK?
- I'm a little uneasy with the design of `cargo login`, where it passes the token in stdin. If the wrapper requires stdin for user interaction (such as entering a password to unlock), this is quite awkward. There is a hack in the 1password example that demonstrates using `/dev/tty` and `CONIN$`, which *seems* to work, but I'm worried is fragile. I'm not very comfortable passing the token in environment variables, because those can be visible to other processes (like CLI args), but in some situations that shouldn't be too risky. Another option is to use a separate file descriptor/handle to pass the token in. Implementing that in Rust in a cross-platform way is not terribly easy, so I wanted to open this up for discussion first.

Author: bors

.github/workflows/main.yml

@@ -18,12 +18,12 @@ jobs:
     - run: rustup update stable && rustup default stable
     - run: rustup component add rustfmt
     - run: cargo fmt --all -- --check
-    - run: cd crates/cargo-test-macro && cargo fmt --all -- --check
-    - run: cd crates/cargo-test-support && cargo fmt --all -- --check
-    - run: cd crates/crates-io && cargo fmt --all -- --check
-    - run: cd crates/resolver-tests && cargo fmt --all -- --check
-    - run: cd crates/cargo-platform && cargo fmt --all -- --check
-    - run: cd crates/mdman && cargo fmt --all -- --check
+    - run: |
+        for manifest in `find crates -name Cargo.toml`
+        do
+          echo check fmt for $manifest
+          cargo fmt --all --manifest-path $manifest -- --check
+        done
 
   test:
     runs-on: ${{ matrix.os }}
@@ -58,7 +58,7 @@ jobs:
     - run: rustup target add ${{ matrix.other }}
     - run: rustup component add rustc-dev llvm-tools-preview rust-docs
       if: startsWith(matrix.rust, 'nightly')
-    - run: sudo apt update -y && sudo apt install gcc-multilib -y
+    - run: sudo apt update -y && sudo apt install gcc-multilib libsecret-1-0 libsecret-1-dev -y
       if: matrix.os == 'ubuntu-latest'
     - run: rustup component add rustfmt || echo "rustfmt not available"
 
@@ -67,6 +67,13 @@ jobs:
     - run: cargo test --features 'deny-warnings' -p cargo-test-support
     - run: cargo test -p cargo-platform
     - run: cargo test --manifest-path crates/mdman/Cargo.toml
+    - run: cargo build --manifest-path crates/credential/cargo-credential-1password/Cargo.toml
+    - run: cargo build --manifest-path crates/credential/cargo-credential-gnome-secret/Cargo.toml
+      if: matrix.os == 'ubuntu-latest'
+    - run: cargo build --manifest-path crates/credential/cargo-credential-macos-keychain/Cargo.toml
+      if: matrix.os == 'macos-latest'
+    - run: cargo build --manifest-path crates/credential/cargo-credential-wincred/Cargo.toml
+      if: matrix.os == 'windows-latest'
 
   resolver:
     runs-on: ubuntu-latest

crates/cargo-test-support/src/lib.rs

@@ -1544,6 +1544,10 @@ fn substitute_macros(input: &str) -> String {
         ("[INSTALLED]", "   Installed"),
         ("[REPLACED]", "    Replaced"),
         ("[BUILDING]", "    Building"),
+        ("[LOGIN]", "       Login"),
+        ("[LOGOUT]", "      Logout"),
+        ("[YANK]", "        Yank"),
+        ("[OWNER]", "       Owner"),
     ];
     let mut result = input.to_owned();
     for &(pat, subst) in &macros {

crates/cargo-test-support/src/paths.rs

@@ -110,14 +110,27 @@ pub trait CargoPathExt {
 }
 
 impl CargoPathExt for Path {
-    /* Technically there is a potential race condition, but we don't
-     * care all that much for our tests
-     */
     fn rm_rf(&self) {
-        if self.exists() {
+        let meta = match self.symlink_metadata() {
+            Ok(meta) => meta,
+            Err(e) => {
+                if e.kind() == ErrorKind::NotFound {
+                    return;
+                }
+                panic!("failed to remove {:?}, could not read: {:?}", self, e);
+            }
+        };
+        // There is a race condition between fetching the metadata and
+        // actually performing the removal, but we don't care all that much
+        // for our tests.
+        if meta.is_dir() {
             if let Err(e) = remove_dir_all::remove_dir_all(self) {
                 panic!("failed to remove {:?}: {:?}", self, e)
             }
+        } else {
+            if let Err(e) = fs::remove_file(self) {
+                panic!("failed to remove {:?}: {:?}", self, e)
+            }
         }
     }
 

crates/credential/README.md

@@ -0,0 +1,8 @@
+# Cargo Credential Packages
+
+This directory contains Cargo packages for handling storage of tokens in a
+secure manner.
+
+`cargo-credential` is a generic library to assist writing a credential
+process. The other directories contain implementations that integrate with
+specific credential systems.

crates/credential/cargo-credential-1password/Cargo.toml

@@ -0,0 +1,13 @@
+[package]
+name = "cargo-credential-1password"
+version = "0.1.0"
+authors = ["The Rust Project Developers"]
+edition = "2018"
+license = "MIT OR Apache-2.0"
+repository = "https://github.com/rust-lang/cargo"
+description = "A Cargo credential process that stores tokens in a 1password vault."
+
+[dependencies]
+cargo-credential = { path = "../cargo-credential" }
+serde = { version = "1.0.117", features = ["derive"] }
+serde_json = "1.0.59"