From 96a922c4c9be194aaa4928fb21c9690a5c6e4445 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 12:39:01 +0800 Subject: [PATCH 01/34] An empty crate for git-sec (#386) --- Cargo.lock | 4 ++++ Cargo.toml | 1 + git-sec/CHANGELOG.md | 10 ++++++++++ git-sec/Cargo.toml | 15 +++++++++++++++ git-sec/src/lib.rs | 1 + 5 files changed, 31 insertions(+) create mode 100644 git-sec/CHANGELOG.md create mode 100644 git-sec/Cargo.toml create mode 100644 git-sec/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 40b0c62f58c..bc5db2e3e38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1442,6 +1442,10 @@ dependencies = [ "thiserror", ] +[[package]] +name = "git-sec" +version = "0.0.0" + [[package]] name = "git-submodule" version = "0.0.0" diff --git a/Cargo.toml b/Cargo.toml index 2e6b663e8fe..52b11407bfb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -146,6 +146,7 @@ members = [ "git-packetline", "git-mailmap", "git-note", + "git-sec", "git-submodule", "git-transport", "git-protocol", diff --git a/git-sec/CHANGELOG.md b/git-sec/CHANGELOG.md new file mode 100644 index 00000000000..ea8f0d10c8a --- /dev/null +++ b/git-sec/CHANGELOG.md @@ -0,0 +1,10 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## Unreleased + +An empty crate without any content to reserve the name for the gitoxide project. diff --git a/git-sec/Cargo.toml b/git-sec/Cargo.toml new file mode 100644 index 00000000000..4b14756be5f --- /dev/null +++ b/git-sec/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "git-sec" +version = "0.0.0" +repository = "https://github.com/Byron/gitoxide" +license = "MIT/Apache-2.0" +description = "A WIP crate of the gitoxide project providing a shared trust model" +authors = ["Sebastian Thiel "] +edition = "2018" + +[lib] +doctest = false + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs new file mode 100644 index 00000000000..d7a83e4f525 --- /dev/null +++ b/git-sec/src/lib.rs @@ -0,0 +1 @@ +#![forbid(unsafe_code, rust_2018_idioms)] From 07efb6ff2dfdc03c1867d1bd1fc1350cee134d16 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 12:41:42 +0800 Subject: [PATCH 02/34] Release git-sec v0.0.0 --- git-sec/CHANGELOG.md | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/git-sec/CHANGELOG.md b/git-sec/CHANGELOG.md index ea8f0d10c8a..ac24617d490 100644 --- a/git-sec/CHANGELOG.md +++ b/git-sec/CHANGELOG.md @@ -5,6 +5,25 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## 0.0.0 (2022-04-15) An empty crate without any content to reserve the name for the gitoxide project. + +### Commit Statistics + + + + - 1 commit contributed to the release. + - 0 commits where understood as [conventional](https://www.conventionalcommits.org). + - 1 unique issue was worked on: [#386](https://github.com/Byron/gitoxide/issues/386) + +### Commit Details + + + +
view details + + * **[#386](https://github.com/Byron/gitoxide/issues/386)** + - An empty crate for git-sec ([`96a922c`](https://github.com/Byron/gitoxide/commit/96a922c4c9be194aaa4928fb21c9690a5c6e4445)) +
+ From be7a9cf776f958ac7228457bb4e1415f86f8e575 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 13:21:32 +0800 Subject: [PATCH 03/34] add frame for git-credentials crate (#386) It will hold the current credentials helper implementation and might contain more one day. Primarily meant to interface with `git-sec` when needed, even though currently it deals with git directly. --- Cargo.lock | 4 ++++ Cargo.toml | 1 + README.md | 2 ++ crate-status.md | 7 +++++++ git-credentials/CHANGELOG.md | 11 +++++++++++ git-credentials/Cargo.toml | 15 +++++++++++++++ git-credentials/src/lib.rs | 1 + 7 files changed, 41 insertions(+) create mode 100644 git-credentials/CHANGELOG.md create mode 100644 git-credentials/Cargo.toml create mode 100644 git-credentials/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index bc5db2e3e38..c58de0a52b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1143,6 +1143,10 @@ dependencies = [ "unicase", ] +[[package]] +name = "git-credentials" +version = "0.0.0" + [[package]] name = "git-date" version = "0.0.0" diff --git a/Cargo.toml b/Cargo.toml index 52b11407bfb..3a1285979bc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -149,6 +149,7 @@ members = [ "git-sec", "git-submodule", "git-transport", + "git-credentials", "git-protocol", "git-pack", "git-odb", diff --git a/README.md b/README.md index a8bb132e47c..23842349b53 100644 --- a/README.md +++ b/README.md @@ -121,6 +121,8 @@ Crates that seem feature complete and need to see some more use before they can * [git-attributes](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-attributes) * [git-quote](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-quote) * **idea** + * [git-credentials](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-credentials) + * [git-sec](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-sec) * [git-note](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-note) * [git-date](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-date) * [git-pathspec](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-pathspec) diff --git a/crate-status.md b/crate-status.md index ae3bf0330dc..fa7e3a24c2d 100644 --- a/crate-status.md +++ b/crate-status.md @@ -233,6 +233,13 @@ A mechanism to associate metadata with any object, and keep revisions of it usin ### git-date * [ ] parse git dates + +### git-credentials +* [ ] launch git credentials helpers with a given action + +### git-sec + +Provides a trust model to share across gitoxide crates. It helps configuring how to interact with external processes, among other things. ### git-glob * [x] parse pattern diff --git a/git-credentials/CHANGELOG.md b/git-credentials/CHANGELOG.md new file mode 100644 index 00000000000..59b18626d78 --- /dev/null +++ b/git-credentials/CHANGELOG.md @@ -0,0 +1,11 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## Unreleased + +An empty crate without any content to reserve the name for the gitoxide project. + diff --git a/git-credentials/Cargo.toml b/git-credentials/Cargo.toml new file mode 100644 index 00000000000..e23febf8496 --- /dev/null +++ b/git-credentials/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "git-credentials" +version = "0.0.0" +repository = "https://github.com/Byron/gitoxide" +license = "MIT/Apache-2.0" +description = "A WIP crate of the gitoxide project to interact with git credentials helpers" +authors = ["Sebastian Thiel "] +edition = "2018" + +[lib] +doctest = false + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs new file mode 100644 index 00000000000..d7a83e4f525 --- /dev/null +++ b/git-credentials/src/lib.rs @@ -0,0 +1 @@ +#![forbid(unsafe_code, rust_2018_idioms)] From 7db45abb822b7c28ac84bf0229ec23ce0f46c8f2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 13:23:48 +0800 Subject: [PATCH 04/34] Release git-credentials v0.0.0 --- git-credentials/CHANGELOG.md | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/git-credentials/CHANGELOG.md b/git-credentials/CHANGELOG.md index 59b18626d78..2e51d6646ec 100644 --- a/git-credentials/CHANGELOG.md +++ b/git-credentials/CHANGELOG.md @@ -5,7 +5,25 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## 0.0.0 (2022-04-15) An empty crate without any content to reserve the name for the gitoxide project. +### Commit Statistics + + + + - 1 commit contributed to the release. + - 0 commits where understood as [conventional](https://www.conventionalcommits.org). + - 1 unique issue was worked on: [#386](https://github.com/Byron/gitoxide/issues/386) + +### Commit Details + + + +
view details + + * **[#386](https://github.com/Byron/gitoxide/issues/386)** + - add frame for git-credentials crate ([`be7a9cf`](https://github.com/Byron/gitoxide/commit/be7a9cf776f958ac7228457bb4e1415f86f8e575)) +
+ From 670fb9bb5c6fa6cf6dba457a3c96b9a55e0410c4 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 13:25:07 +0800 Subject: [PATCH 05/34] force re-installing from crates.io (#386) Otherwise we wouldn't know if the dependency resolution breaks. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bd53312ffa3..c51fc6d80e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,7 +60,7 @@ jobs: if: startsWith(matrix.os, 'windows') uses: actions-rs/cargo@v1 with: - command: install + command: install --force args: "gitoxide cargo-smart-release" lint: From 6016c2252aea6892a813b7dc1b0c870a156b3cfd Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 13:44:30 +0800 Subject: [PATCH 06/34] fill git-credentials with existing impleemntation (#386) --- Cargo.lock | 7 +- Makefile | 1 + crate-status.md | 2 +- git-credentials/Cargo.toml | 12 +- git-credentials/src/helper.rs | 242 ++++++++++++++++++++++++++++++++++ git-credentials/src/lib.rs | 20 ++- 6 files changed, 280 insertions(+), 4 deletions(-) create mode 100644 git-credentials/src/helper.rs diff --git a/Cargo.lock b/Cargo.lock index c58de0a52b2..b415273a839 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1145,7 +1145,12 @@ dependencies = [ [[package]] name = "git-credentials" -version = "0.0.0" +version = "0.1.0" +dependencies = [ + "bstr", + "quick-error", + "serde", +] [[package]] name = "git-date" diff --git a/Makefile b/Makefile index 11a4416f7ca..ac9b7ad873e 100644 --- a/Makefile +++ b/Makefile @@ -89,6 +89,7 @@ check: ## Build all code in suitable configurations cd git-object && cargo check --all-features \ && cargo check --features verbose-object-parsing-errors cd git-index && cargo check --features serde1 + cd git-credentials && cargo check --features serde1 cd git-revision && cargo check --features serde1 cd git-attributes && cargo check --features serde1 cd git-glob && cargo check --features serde1 diff --git a/crate-status.md b/crate-status.md index fa7e3a24c2d..c996cc28d84 100644 --- a/crate-status.md +++ b/crate-status.md @@ -235,7 +235,7 @@ A mechanism to associate metadata with any object, and keep revisions of it usin * [ ] parse git dates ### git-credentials -* [ ] launch git credentials helpers with a given action +* [x] launch git credentials helpers with a given action ### git-sec diff --git a/git-credentials/Cargo.toml b/git-credentials/Cargo.toml index e23febf8496..f6fe24bcdac 100644 --- a/git-credentials/Cargo.toml +++ b/git-credentials/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "git-credentials" -version = "0.0.0" +version = "0.1.0" repository = "https://github.com/Byron/gitoxide" license = "MIT/Apache-2.0" description = "A WIP crate of the gitoxide project to interact with git credentials helpers" @@ -10,6 +10,16 @@ edition = "2018" [lib] doctest = false +[features] +## Data structures implement `serde::Serialize` and `serde::Deserialize`. +serde1 = ["serde", "bstr/serde1"] + # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +quick-error = "2.0.0" +serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } +bstr = { version = "0.2.13", default-features = false, features = ["std"]} + +[package.metadata.docs.rs] +all-features = true diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs new file mode 100644 index 00000000000..387c35f5df1 --- /dev/null +++ b/git-credentials/src/helper.rs @@ -0,0 +1,242 @@ +use std::{ + io::{self, Write}, + process::{Command, Stdio}, +}; + +use crate::Identity; +use quick_error::quick_error; + +/// The result used in [`helper()`]. +pub type Result = std::result::Result, Error>; + +quick_error! { + /// The error used in the [credentials helper][helper()]. + #[derive(Debug)] + #[allow(missing_docs)] + pub enum Error { + Io(err: io::Error) { + display("An IO error occurred while communicating to the credentials helper") + from() + source(err) + } + KeyNotFound(name: String) { + display("Could not find '{}' in output of git credentials helper", name) + } + CredentialsHelperFailed(code: Option) { + display("Credentials helper program failed with status code {:?}", code) + } + } +} + +/// The action to perform by the credentials [`helper()`]. +#[derive(Clone, Debug)] +pub enum Action<'a> { + /// Provide credentials using the given repository URL (as &str) as context. + Fill(&'a str), + /// Approve the credentials as identified by the previous input as `Vec`. + Approve(Vec), + /// Reject the credentials as identified by the previous input as `Vec`. + Reject(Vec), +} + +impl<'a> Action<'a> { + fn is_fill(&self) -> bool { + matches!(self, Action::Fill(_)) + } + fn as_str(&self) -> &str { + match self { + Action::Approve(_) => "approve", + Action::Fill(_) => "fill", + Action::Reject(_) => "reject", + } + } +} + +/// A handle to [approve][NextAction::approve()] or [reject][NextAction::reject()] the outcome of the initial action. +#[derive(Clone, Debug)] +pub struct NextAction { + previous_output: Vec, +} + +impl NextAction { + /// Approve the result of the previous [Action]. + pub fn approve(self) -> Action<'static> { + Action::Approve(self.previous_output) + } + /// Reject the result of the previous [Action]. + pub fn reject(self) -> Action<'static> { + Action::Reject(self.previous_output) + } +} + +/// The outcome of [`helper()`]. +pub struct Outcome { + /// The obtained identity. + pub identity: Identity, + /// A handle to the action to perform next using another call to [`helper()`]. + pub next: NextAction, +} + +#[cfg(windows)] +fn git_program() -> &'static str { + "git.exe" +} + +#[cfg(not(windows))] +fn git_program() -> &'static str { + "git" +} + +/// Call the `git` credentials helper program performing the given `action`. +/// +/// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. +/// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. +pub fn helper(action: Action<'_>) -> Result { + let mut cmd = Command::new(git_program()); + cmd.arg("credential") + .arg(action.as_str()) + .stdin(Stdio::piped()) + .stdout(if action.is_fill() { + Stdio::piped() + } else { + Stdio::null() + }); + let mut child = cmd.spawn()?; + let mut stdin = child.stdin.take().expect("stdin to be configured"); + + match action { + Action::Fill(url) => encode_message(url, stdin)?, + Action::Approve(last) | Action::Reject(last) => { + stdin.write_all(&last)?; + stdin.write_all(&[b'\n'])? + } + } + + let output = child.wait_with_output()?; + if !output.status.success() { + return Err(Error::CredentialsHelperFailed(output.status.code())); + } + let stdout = output.stdout; + if stdout.is_empty() { + Ok(None) + } else { + let kvs = decode_message(stdout.as_slice())?; + let find = |name: &str| { + kvs.iter() + .find(|(k, _)| k == name) + .ok_or_else(|| Error::KeyNotFound(name.into())) + .map(|(_, n)| n.to_owned()) + }; + Ok(Some(Outcome { + identity: Identity::Account { + username: find("username")?, + password: find("password")?, + }, + next: NextAction { + previous_output: stdout, + }, + })) + } +} + +/// Encode `url` to `out` for consumption by a `git credentials` helper program. +pub fn encode_message(url: &str, mut out: impl io::Write) -> io::Result<()> { + validate(url)?; + writeln!(out, "url={}\n", url) +} + +fn validate(url: &str) -> io::Result<()> { + if url.contains('\u{0}') || url.contains('\n') { + return Err(io::Error::new( + io::ErrorKind::Other, + "token to encode must not contain newlines or null bytes", + )); + } + Ok(()) +} + +/// Decode all lines in `input` as key-value pairs produced by a `git credentials` helper program. +pub fn decode_message(mut input: impl io::Read) -> io::Result> { + let mut buf = String::new(); + input.read_to_string(&mut buf)?; + buf.lines() + .take_while(|l| !l.is_empty()) + .map(|l| { + let mut iter = l.splitn(2, '=').map(|s| s.to_owned()); + match (iter.next(), iter.next()) { + (Some(key), Some(value)) => validate(&key).and_then(|_| validate(&value)).map(|_| (key, value)), + _ => Err(io::Error::new( + io::ErrorKind::Other, + "Invalid format, expecting key=value", + )), + } + }) + .collect::>>() +} + +#[cfg(test)] +mod tests { + use super::*; + type Result = std::result::Result<(), Box>; + + mod encode_message { + use super::*; + use bstr::ByteSlice; + + #[test] + fn from_url() -> super::Result { + let mut out = Vec::new(); + encode_message("https://github.com/byron/gitoxide", &mut out)?; + assert_eq!(out.as_bstr(), b"url=https://github.com/byron/gitoxide\n\n".as_bstr()); + Ok(()) + } + + mod invalid { + use super::*; + use std::io; + + #[test] + fn contains_null() { + assert_eq!( + encode_message("https://foo\u{0}", Vec::new()).err().map(|e| e.kind()), + Some(io::ErrorKind::Other) + ); + } + #[test] + fn contains_newline() { + assert_eq!( + encode_message("https://foo\n", Vec::new()).err().map(|e| e.kind()), + Some(io::ErrorKind::Other) + ); + } + } + } + + mod decode_message { + use super::*; + + #[test] + fn typical_response() -> super::Result { + assert_eq!( + decode_message( + "protocol=https +host=example.com +username=bob +password=secr3t\n\n +this=is-skipped-past-empty-line" + .as_bytes() + )?, + vec![ + ("protocol", "https"), + ("host", "example.com"), + ("username", "bob"), + ("password", "secr3t") + ] + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect::>() + ); + Ok(()) + } + } +} diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index d7a83e4f525..6511fb03623 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -1 +1,19 @@ -#![forbid(unsafe_code, rust_2018_idioms)] +#![forbid(unsafe_code)] +#![deny(missing_docs, rust_2018_idioms)] +//! Interact with git credentials in various ways and launch helper programs. + +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] +#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] +/// An identity for use when authenticating the transport layer. +pub enum Identity { + /// An account based identity + Account { + /// The user's name + username: String, + /// The user's password + password: String, + }, +} + +/// +pub mod helper; From 0dfbeea34f46848d155d359a94c7be34cda517cf Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 13:53:44 +0800 Subject: [PATCH 07/34] better package size display; add new crates to size-check (#386) --- etc/check-package-size.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/etc/check-package-size.sh b/etc/check-package-size.sh index 605ed86da73..9fd9bf6d429 100755 --- a/etc/check-package-size.sh +++ b/etc/check-package-size.sh @@ -4,7 +4,7 @@ set -eu -o pipefail function enter () { local dir="${1:?need directory to enter}" - echo $' in' $dir + echo -n $' in' $dir $'\t→\t' cd $dir } @@ -36,6 +36,11 @@ echo "in root: gitoxide CLI" (enter git-traverse && indent cargo diet -n --package-size-limit 10KB) (enter git-url && indent cargo diet -n --package-size-limit 15KB) (enter git-validate && indent cargo diet -n --package-size-limit 5KB) +(enter git-date && indent cargo diet -n --package-size-limit 5KB) +(enter git-note && indent cargo diet -n --package-size-limit 5KB) +(enter git-sec && indent cargo diet -n --package-size-limit 5KB) +(enter git-tix && indent cargo diet -n --package-size-limit 5KB) +(enter git-credentials && indent cargo diet -n --package-size-limit 5KB) (enter git-object && indent cargo diet -n --package-size-limit 25KB) (enter git-commitgraph && indent cargo diet -n --package-size-limit 25KB) (enter git-pack && indent cargo diet -n --package-size-limit 115KB) From 6df2881a9155a097e98b239167ad249b5d4cc086 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 13:54:32 +0800 Subject: [PATCH 08/34] reduce API surface (#386) --- git-protocol/src/credentials.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-protocol/src/credentials.rs b/git-protocol/src/credentials.rs index d2db3f75250..839d65007b6 100644 --- a/git-protocol/src/credentials.rs +++ b/git-protocol/src/credentials.rs @@ -140,7 +140,7 @@ pub fn helper(action: Action<'_>) -> Result { } /// Encode `url` to `out` for consumption by a `git credentials` helper program. -pub fn encode_message(url: &str, mut out: impl io::Write) -> io::Result<()> { +fn encode_message(url: &str, mut out: impl io::Write) -> io::Result<()> { validate(url)?; writeln!(out, "url={}\n", url) } @@ -156,7 +156,7 @@ fn validate(url: &str) -> io::Result<()> { } /// Decode all lines in `input` as key-value pairs produced by a `git credentials` helper program. -pub fn decode_message(mut input: impl io::Read) -> io::Result> { +fn decode_message(mut input: impl io::Read) -> io::Result> { let mut buf = String::new(); input.read_to_string(&mut buf)?; buf.lines() From cdf3c3e42433a85e8b47b9dc5558f5c76df3c6ae Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 14:14:12 +0800 Subject: [PATCH 09/34] feat: add `Identity` type (#386) --- git-sec/Cargo.toml | 7 ++++++- git-sec/src/lib.rs | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/git-sec/Cargo.toml b/git-sec/Cargo.toml index 4b14756be5f..43971f20511 100644 --- a/git-sec/Cargo.toml +++ b/git-sec/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "git-sec" -version = "0.0.0" +version = "0.1.0" repository = "https://github.com/Byron/gitoxide" license = "MIT/Apache-2.0" description = "A WIP crate of the gitoxide project providing a shared trust model" @@ -10,6 +10,11 @@ edition = "2018" [lib] doctest = false +[features] +## Data structures implement `serde::Serialize` and `serde::Deserialize`. +serde1 = [ "serde" ] + # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index d7a83e4f525..4d29f6147ef 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -1 +1,16 @@ -#![forbid(unsafe_code, rust_2018_idioms)] +#![forbid(unsafe_code)] +#![deny(rust_2018_idioms, missing_docs)] +//! A shared trust model for `gitoxide` crates. + +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] +#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] +/// An identity for use when authenticating the transport layer. +pub enum Identity { + /// An account based identity + Account { + /// The user's name + username: String, + /// The user's password + password: String, + }, +} From 3d339d5c24630fac0192b5d27f9b1cbd94418730 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 14:14:34 +0800 Subject: [PATCH 10/34] feat: use `git-sec::Identity` type (#386) It's shared across crates. --- git-credentials/Cargo.toml | 3 ++- git-credentials/src/helper.rs | 17 ++++++++--------- git-credentials/src/lib.rs | 14 +------------- 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/git-credentials/Cargo.toml b/git-credentials/Cargo.toml index f6fe24bcdac..a1dc3c665c1 100644 --- a/git-credentials/Cargo.toml +++ b/git-credentials/Cargo.toml @@ -12,11 +12,12 @@ doctest = false [features] ## Data structures implement `serde::Serialize` and `serde::Deserialize`. -serde1 = ["serde", "bstr/serde1"] +serde1 = ["serde", "bstr/serde1", "git-sec/serde1"] # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +git-sec = { version = "^0.1.0", path = "../git-sec" } quick-error = "2.0.0" serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } bstr = { version = "0.2.13", default-features = false, features = ["std"]} diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index 387c35f5df1..77c0556b821 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -3,14 +3,13 @@ use std::{ process::{Command, Stdio}, }; -use crate::Identity; use quick_error::quick_error; -/// The result used in [`helper()`]. +/// The result used in [`action()`]. pub type Result = std::result::Result, Error>; quick_error! { - /// The error used in the [credentials helper][helper()]. + /// The error used in the [credentials helper][action()]. #[derive(Debug)] #[allow(missing_docs)] pub enum Error { @@ -28,7 +27,7 @@ quick_error! { } } -/// The action to perform by the credentials [`helper()`]. +/// The action to perform by the credentials [`action()`]. #[derive(Clone, Debug)] pub enum Action<'a> { /// Provide credentials using the given repository URL (as &str) as context. @@ -69,11 +68,11 @@ impl NextAction { } } -/// The outcome of [`helper()`]. +/// The outcome of [`action()`]. pub struct Outcome { /// The obtained identity. - pub identity: Identity, - /// A handle to the action to perform next using another call to [`helper()`]. + pub identity: git_sec::Identity, + /// A handle to the action to perform next using another call to [`action()`]. pub next: NextAction, } @@ -91,7 +90,7 @@ fn git_program() -> &'static str { /// /// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. /// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. -pub fn helper(action: Action<'_>) -> Result { +pub fn action(action: Action<'_>) -> Result { let mut cmd = Command::new(git_program()); cmd.arg("credential") .arg(action.as_str()) @@ -128,7 +127,7 @@ pub fn helper(action: Action<'_>) -> Result { .map(|(_, n)| n.to_owned()) }; Ok(Some(Outcome { - identity: Identity::Account { + identity: git_sec::Identity::Account { username: find("username")?, password: find("password")?, }, diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index 6511fb03623..b6ff3d34fb1 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -2,18 +2,6 @@ #![deny(missing_docs, rust_2018_idioms)] //! Interact with git credentials in various ways and launch helper programs. -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] -#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] -/// An identity for use when authenticating the transport layer. -pub enum Identity { - /// An account based identity - Account { - /// The user's name - username: String, - /// The user's password - password: String, - }, -} - /// pub mod helper; +pub use helper::action as helper; From 32dc1829a5661f66396d109c8d0a8eaae6b1f532 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 14:15:03 +0800 Subject: [PATCH 11/34] feat!: use `git-credentials` in `git-protocol` (#386) --- Cargo.lock | 10 +- Makefile | 1 + git-protocol/Cargo.toml | 1 + git-protocol/src/credentials.rs | 175 ------------------ git-protocol/src/fetch/error.rs | 2 +- git-protocol/src/fetch_fn.rs | 6 +- git-protocol/src/lib.rs | 3 +- git-protocol/tests/async-protocol.rs | 1 - git-protocol/tests/blocking-protocol.rs | 1 - git-protocol/tests/credentials/mod.rs | 86 --------- git-repository/Cargo.toml | 4 +- git-repository/src/lib.rs | 6 + git-transport/Cargo.toml | 1 + .../src/client/blocking_io/http/mod.rs | 6 +- git-transport/src/client/mod.rs | 4 +- git-transport/src/client/non_io_types.rs | 13 -- git-transport/src/client/traits.rs | 11 +- .../tests/client/blocking_io/http/mod.rs | 4 +- 18 files changed, 38 insertions(+), 297 deletions(-) delete mode 100644 git-protocol/src/credentials.rs delete mode 100644 git-protocol/tests/credentials/mod.rs diff --git a/Cargo.lock b/Cargo.lock index b415273a839..171d196bb7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1148,6 +1148,7 @@ name = "git-credentials" version = "0.1.0" dependencies = [ "bstr", + "git-sec", "quick-error", "serde", ] @@ -1360,6 +1361,7 @@ dependencies = [ "document-features", "futures-io", "futures-lite", + "git-credentials", "git-features", "git-hash", "git-packetline", @@ -1410,6 +1412,7 @@ dependencies = [ "document-features", "git-actor", "git-config", + "git-credentials", "git-diff", "git-features", "git-glob", @@ -1423,6 +1426,7 @@ dependencies = [ "git-protocol", "git-ref", "git-revision", + "git-sec", "git-tempfile", "git-testtools", "git-transport", @@ -1453,7 +1457,10 @@ dependencies = [ [[package]] name = "git-sec" -version = "0.0.0" +version = "0.1.0" +dependencies = [ + "serde", +] [[package]] name = "git-submodule" @@ -1511,6 +1518,7 @@ dependencies = [ "git-hash", "git-pack", "git-packetline", + "git-sec", "git-url", "maybe-async", "pin-project-lite", diff --git a/Makefile b/Makefile index ac9b7ad873e..fa713ab6adc 100644 --- a/Makefile +++ b/Makefile @@ -90,6 +90,7 @@ check: ## Build all code in suitable configurations && cargo check --features verbose-object-parsing-errors cd git-index && cargo check --features serde1 cd git-credentials && cargo check --features serde1 + cd git-sec && cargo check --features serde1 cd git-revision && cargo check --features serde1 cd git-attributes && cargo check --features serde1 cd git-glob && cargo check --features serde1 diff --git a/git-protocol/Cargo.toml b/git-protocol/Cargo.toml index def322526c9..9bef72410f5 100644 --- a/git-protocol/Cargo.toml +++ b/git-protocol/Cargo.toml @@ -42,6 +42,7 @@ required-features = ["async-client"] git-features = { version = "^0.20.0", path = "../git-features", features = ["progress"] } git-transport = { version = "^0.16.0", path = "../git-transport" } git-hash = { version = "^0.9.3", path = "../git-hash" } +git-credentials = { version = "^0.1.0", path = "../git-credentials" } quick-error = "2.0.0" serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"]} diff --git a/git-protocol/src/credentials.rs b/git-protocol/src/credentials.rs deleted file mode 100644 index 839d65007b6..00000000000 --- a/git-protocol/src/credentials.rs +++ /dev/null @@ -1,175 +0,0 @@ -use std::{ - io::{self, Write}, - process::{Command, Stdio}, -}; - -use git_transport::client; -use quick_error::quick_error; - -/// The result used in [`helper()`]. -pub type Result = std::result::Result, Error>; - -quick_error! { - /// The error used in the [credentials helper][helper()]. - #[derive(Debug)] - #[allow(missing_docs)] - pub enum Error { - Io(err: io::Error) { - display("An IO error occurred while communicating to the credentials helper") - from() - source(err) - } - KeyNotFound(name: String) { - display("Could not find '{}' in output of git credentials helper", name) - } - CredentialsHelperFailed(code: Option) { - display("Credentials helper program failed with status code {:?}", code) - } - } -} - -/// The action to perform by the credentials [`helper()`]. -#[derive(Clone, Debug)] -pub enum Action<'a> { - /// Provide credentials using the given repository URL (as &str) as context. - Fill(&'a str), - /// Approve the credentials as identified by the previous input as `Vec`. - Approve(Vec), - /// Reject the credentials as identified by the previous input as `Vec`. - Reject(Vec), -} - -impl<'a> Action<'a> { - fn is_fill(&self) -> bool { - matches!(self, Action::Fill(_)) - } - fn as_str(&self) -> &str { - match self { - Action::Approve(_) => "approve", - Action::Fill(_) => "fill", - Action::Reject(_) => "reject", - } - } -} - -/// A handle to [approve][NextAction::approve()] or [reject][NextAction::reject()] the outcome of the initial action. -#[derive(Clone, Debug)] -pub struct NextAction { - previous_output: Vec, -} - -impl NextAction { - /// Approve the result of the previous [Action]. - pub fn approve(self) -> Action<'static> { - Action::Approve(self.previous_output) - } - /// Reject the result of the previous [Action]. - pub fn reject(self) -> Action<'static> { - Action::Reject(self.previous_output) - } -} - -/// The outcome of [`helper()`]. -pub struct Outcome { - /// The obtained identity. - pub identity: client::Identity, - /// A handle to the action to perform next using another call to [`helper()`]. - pub next: NextAction, -} - -#[cfg(windows)] -fn git_program() -> &'static str { - "git.exe" -} - -#[cfg(not(windows))] -fn git_program() -> &'static str { - "git" -} - -/// Call the `git` credentials helper program performing the given `action`. -/// -/// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. -/// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. -pub fn helper(action: Action<'_>) -> Result { - let mut cmd = Command::new(git_program()); - cmd.arg("credential") - .arg(action.as_str()) - .stdin(Stdio::piped()) - .stdout(if action.is_fill() { - Stdio::piped() - } else { - Stdio::null() - }); - let mut child = cmd.spawn()?; - let mut stdin = child.stdin.take().expect("stdin to be configured"); - - match action { - Action::Fill(url) => encode_message(url, stdin)?, - Action::Approve(last) | Action::Reject(last) => { - stdin.write_all(&last)?; - stdin.write_all(&[b'\n'])? - } - } - - let output = child.wait_with_output()?; - if !output.status.success() { - return Err(Error::CredentialsHelperFailed(output.status.code())); - } - let stdout = output.stdout; - if stdout.is_empty() { - Ok(None) - } else { - let kvs = decode_message(stdout.as_slice())?; - let find = |name: &str| { - kvs.iter() - .find(|(k, _)| k == name) - .ok_or_else(|| Error::KeyNotFound(name.into())) - .map(|(_, n)| n.to_owned()) - }; - Ok(Some(Outcome { - identity: client::Identity::Account { - username: find("username")?, - password: find("password")?, - }, - next: NextAction { - previous_output: stdout, - }, - })) - } -} - -/// Encode `url` to `out` for consumption by a `git credentials` helper program. -fn encode_message(url: &str, mut out: impl io::Write) -> io::Result<()> { - validate(url)?; - writeln!(out, "url={}\n", url) -} - -fn validate(url: &str) -> io::Result<()> { - if url.contains('\u{0}') || url.contains('\n') { - return Err(io::Error::new( - io::ErrorKind::Other, - "token to encode must not contain newlines or null bytes", - )); - } - Ok(()) -} - -/// Decode all lines in `input` as key-value pairs produced by a `git credentials` helper program. -fn decode_message(mut input: impl io::Read) -> io::Result> { - let mut buf = String::new(); - input.read_to_string(&mut buf)?; - buf.lines() - .take_while(|l| !l.is_empty()) - .map(|l| { - let mut iter = l.splitn(2, '=').map(|s| s.to_owned()); - match (iter.next(), iter.next()) { - (Some(key), Some(value)) => validate(&key).and_then(|_| validate(&value)).map(|_| (key, value)), - _ => Err(io::Error::new( - io::ErrorKind::Other, - "Invalid format, expecting key=value", - )), - } - }) - .collect::>>() -} diff --git a/git-protocol/src/fetch/error.rs b/git-protocol/src/fetch/error.rs index 6c401e7eb1b..5459ffc9f35 100644 --- a/git-protocol/src/fetch/error.rs +++ b/git-protocol/src/fetch/error.rs @@ -18,7 +18,7 @@ quick_error! { from() source(err) } - Credentials(err: credentials::Error) { + Credentials(err: credentials::helper::Error) { display("Failed to obtain, approve or reject credentials") from() source(err) diff --git a/git-protocol/src/fetch_fn.rs b/git-protocol/src/fetch_fn.rs index 9b6bdd4141e..a0597341257 100644 --- a/git-protocol/src/fetch_fn.rs +++ b/git-protocol/src/fetch_fn.rs @@ -58,7 +58,7 @@ pub async fn fetch( fetch_mode: FetchConnection, ) -> Result<(), Error> where - F: FnMut(credentials::Action<'_>) -> credentials::Result, + F: FnMut(credentials::helper::Action<'_>) -> credentials::helper::Result, D: Delegate, T: client::Transport, { @@ -85,8 +85,8 @@ where drop(result); // needed to workaround this: https://github.com/rust-lang/rust/issues/76149 let url = transport.to_url(); progress.set_name("authentication"); - let credentials::Outcome { identity, next } = - authenticate(credentials::Action::Fill(&url))?.expect("FILL provides an identity"); + let credentials::helper::Outcome { identity, next } = + authenticate(credentials::helper::Action::Fill(&url))?.expect("FILL provides an identity"); transport.set_identity(identity)?; progress.step(); progress.set_name("handshake (authenticated)"); diff --git a/git-protocol/src/lib.rs b/git-protocol/src/lib.rs index f62cb5c09a0..f346013ea92 100644 --- a/git-protocol/src/lib.rs +++ b/git-protocol/src/lib.rs @@ -10,11 +10,10 @@ #![deny(unsafe_code)] #![deny(rust_2018_idioms, missing_docs)] +pub use git_credentials as credentials; /// A convenience export allowing users of git-protocol to use the transport layer without their own cargo dependency. pub use git_transport as transport; -/// -pub mod credentials; /// #[cfg(any(feature = "blocking-client", feature = "async-client"))] pub mod fetch; diff --git a/git-protocol/tests/async-protocol.rs b/git-protocol/tests/async-protocol.rs index d6af99e9760..772e2bd386d 100644 --- a/git-protocol/tests/async-protocol.rs +++ b/git-protocol/tests/async-protocol.rs @@ -5,6 +5,5 @@ pub fn fixture_bytes(path: &str) -> Vec { .expect("fixture to be present and readable") } -mod credentials; mod fetch; mod remote_progress; diff --git a/git-protocol/tests/blocking-protocol.rs b/git-protocol/tests/blocking-protocol.rs index d6af99e9760..772e2bd386d 100644 --- a/git-protocol/tests/blocking-protocol.rs +++ b/git-protocol/tests/blocking-protocol.rs @@ -5,6 +5,5 @@ pub fn fixture_bytes(path: &str) -> Vec { .expect("fixture to be present and readable") } -mod credentials; mod fetch; mod remote_progress; diff --git a/git-protocol/tests/credentials/mod.rs b/git-protocol/tests/credentials/mod.rs deleted file mode 100644 index 1d7e6ec344b..00000000000 --- a/git-protocol/tests/credentials/mod.rs +++ /dev/null @@ -1,86 +0,0 @@ -mod encode_message { - use bstr::ByteSlice; - use git_protocol::credentials; - - #[test] - fn from_url() -> crate::Result { - let mut out = Vec::new(); - credentials::encode_message("https://github.com/byron/gitoxide", &mut out)?; - assert_eq!(out.as_bstr(), b"url=https://github.com/byron/gitoxide\n\n".as_bstr()); - Ok(()) - } - - mod invalid { - use std::io; - - use git_protocol::credentials; - - #[test] - fn contains_null() { - assert_eq!( - credentials::encode_message("https://foo\u{0}", Vec::new()) - .err() - .map(|e| e.kind()), - Some(io::ErrorKind::Other) - ); - } - #[test] - fn contains_newline() { - assert_eq!( - credentials::encode_message("https://foo\n", Vec::new()) - .err() - .map(|e| e.kind()), - Some(io::ErrorKind::Other) - ); - } - } -} - -mod decode_message { - use git_protocol::credentials; - - #[test] - fn typical_response() -> crate::Result { - assert_eq!( - credentials::decode_message( - "protocol=https -host=example.com -username=bob -password=secr3t\n\n -this=is-skipped-past-empty-line" - .as_bytes() - )?, - vec![ - ("protocol", "https"), - ("host", "example.com"), - ("username", "bob"), - ("password", "secr3t") - ] - .iter() - .map(|(k, v)| (k.to_string(), v.to_string())) - .collect::>() - ); - Ok(()) - } - - mod invalid { - use std::io; - - use git_protocol::credentials; - - #[test] - fn null_in_key() -> crate::Result { - assert_eq!( - credentials::decode_message( - "protocol=https -host=examp\0le.com" - .as_bytes() - ) - .err() - .map(|e| e.kind()), - Some(io::ErrorKind::Other), - ); - Ok(()) - } - } -} diff --git a/git-repository/Cargo.toml b/git-repository/Cargo.toml index f6bf517ed6f..59a46b6f034 100644 --- a/git-repository/Cargo.toml +++ b/git-repository/Cargo.toml @@ -49,7 +49,7 @@ max-performance = ["git-features/parallel", "git-features/zlib-ng-compat", "git- local-time-support = ["git-actor/local-time-support"] ## Re-export stability tier 2 crates for convenience and make `Repository` struct fields with types from these crates publicly accessible. ## Doing so is less stable than the stability tier 1 that `git-repository` is a member of. -unstable = ["git-index", "git-worktree", "git-mailmap", "git-glob"] +unstable = ["git-index", "git-worktree", "git-mailmap", "git-glob", "git-sec", "git-credentials"] ## Print debugging information about usage of object database caches, useful for tuning cache sizes. cache-efficiency-debug = ["git-features/cache-efficiency-debug"] @@ -79,6 +79,8 @@ git-features = { version = "^0.20.0", path = "../git-features", features = ["pro # unstable only git-glob = { version = "^0.2.0", path = "../git-glob", optional = true } +git-sec = { version = "^0.1.0", path = "../git-sec", optional = true } +git-credentials = { version = "^0.1.0", path = "../git-credentials", optional = true } git-index = { version = "^0.2.0", path = "../git-index", optional = true } git-worktree = { version = "^0.1.0", path = "../git-worktree", optional = true } diff --git a/git-repository/src/lib.rs b/git-repository/src/lib.rs index 54c29e8260e..989e04b0220 100644 --- a/git-repository/src/lib.rs +++ b/git-repository/src/lib.rs @@ -89,6 +89,8 @@ //! * [`bstr`][bstr] //! * [`index`] //! * [`glob`] +//! * [`credentials`] +//! * [`sec`] //! * [`worktree`] //! * [`mailmap`] //! * [`objs`] @@ -122,6 +124,8 @@ use std::path::PathBuf; // This also means that their major version changes affect our major version, but that's alright as we directly expose their // APIs/instances anyway. pub use git_actor as actor; +#[cfg(all(feature = "unstable", feature = "git-credentials"))] +pub use git_credentials as credentials; #[cfg(all(feature = "unstable", feature = "git-diff"))] pub use git_diff as diff; use git_features::threading::OwnShared; @@ -142,6 +146,8 @@ pub use git_odb as odb; pub use git_protocol as protocol; pub use git_ref as refs; pub use git_revision as revision; +#[cfg(all(feature = "unstable", feature = "git-sec"))] +pub use git_sec as sec; #[cfg(feature = "unstable")] pub use git_tempfile as tempfile; #[cfg(feature = "unstable")] diff --git a/git-transport/Cargo.toml b/git-transport/Cargo.toml index 401f92e06b5..be6ba876802 100644 --- a/git-transport/Cargo.toml +++ b/git-transport/Cargo.toml @@ -52,6 +52,7 @@ required-features = ["async-client"] [dependencies] git-features = { version = "^0.20.0", path = "../git-features" } git-url = { version = "^0.4.0", path = "../git-url" } +git-sec = { version = "^0.1.0", path = "../git-sec" } git-packetline = { version = "^0.12.4", path = "../git-packetline" } serde = { version = "1.0.114", optional = true, default-features = false, features = ["std", "derive"]} diff --git a/git-transport/src/client/blocking_io/http/mod.rs b/git-transport/src/client/blocking_io/http/mod.rs index 7f30032258a..a9a3c651d1f 100644 --- a/git-transport/src/client/blocking_io/http/mod.rs +++ b/git-transport/src/client/blocking_io/http/mod.rs @@ -32,7 +32,7 @@ pub struct Transport { http: H, service: Option, line_provider: Option>, - identity: Option, + identity: Option, } impl Transport { @@ -73,7 +73,7 @@ impl Transport { fn add_basic_auth_if_present(&self, headers: &mut Vec>) -> Result<(), client::Error> { if let Some(identity) = &self.identity { match identity { - client::Identity::Account { username, password } => { + git_sec::Identity::Account { username, password } => { #[cfg(not(debug_assertions))] if self.url.starts_with("http://") { return Err(client::Error::AuthenticationRefused( @@ -100,7 +100,7 @@ fn append_url(base: &str, suffix: &str) -> String { } impl client::TransportWithoutIO for Transport { - fn set_identity(&mut self, identity: client::Identity) -> Result<(), client::Error> { + fn set_identity(&mut self, identity: git_sec::Identity) -> Result<(), client::Error> { self.identity = Some(identity); Ok(()) } diff --git a/git-transport/src/client/mod.rs b/git-transport/src/client/mod.rs index 7cf1ec4576a..cb3c16684cd 100644 --- a/git-transport/src/client/mod.rs +++ b/git-transport/src/client/mod.rs @@ -26,7 +26,9 @@ pub mod capabilities; pub use capabilities::Capabilities; mod non_io_types; -pub use non_io_types::{Error, Identity, MessageKind, WriteMode}; +pub use non_io_types::{Error, MessageKind, WriteMode}; + +pub use git_sec::Identity; /// #[cfg(any(feature = "blocking-client", feature = "async-client"))] diff --git a/git-transport/src/client/non_io_types.rs b/git-transport/src/client/non_io_types.rs index 7fcbf4078de..6ffabe43500 100644 --- a/git-transport/src/client/non_io_types.rs +++ b/git-transport/src/client/non_io_types.rs @@ -29,19 +29,6 @@ pub enum MessageKind { Text(&'static [u8]), } -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] -#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] -/// An identity for use when authenticating the transport layer. -pub enum Identity { - /// An account based identity - Account { - /// The user's name - username: String, - /// The user's password - password: String, - }, -} - pub(crate) mod connect { use quick_error::quick_error; quick_error! { diff --git a/git-transport/src/client/traits.rs b/git-transport/src/client/traits.rs index e7a5ab40cc0..3e1694f88f2 100644 --- a/git-transport/src/client/traits.rs +++ b/git-transport/src/client/traits.rs @@ -2,10 +2,7 @@ use std::ops::{Deref, DerefMut}; #[cfg(any(feature = "blocking-client", feature = "async-client"))] use crate::client::{MessageKind, RequestWriter, WriteMode}; -use crate::{ - client::{Error, Identity}, - Protocol, -}; +use crate::{client::Error, Protocol}; /// This trait represents all transport related functions that don't require any input/output to be done which helps /// implementation to share more code across blocking and async programs. @@ -16,7 +13,7 @@ pub trait TransportWithoutIO { /// of the identity in order to mark it as invalid. Otherwise the user might have difficulty updating obsolete /// credentials. /// Please note that most transport layers are unauthenticated and thus return [an error][Error::AuthenticationUnsupported] here. - fn set_identity(&mut self, _identity: Identity) -> Result<(), Error> { + fn set_identity(&mut self, _identity: git_sec::Identity) -> Result<(), Error> { Err(Error::AuthenticationUnsupported) } /// Get a writer for sending data and obtaining the response. It can be configured in various ways @@ -53,7 +50,7 @@ pub trait TransportWithoutIO { // Would be nice if the box implementation could auto-forward to all implemented traits. impl TransportWithoutIO for Box { - fn set_identity(&mut self, identity: Identity) -> Result<(), Error> { + fn set_identity(&mut self, identity: git_sec::Identity) -> Result<(), Error> { self.deref_mut().set_identity(identity) } @@ -76,7 +73,7 @@ impl TransportWithoutIO for Box { } impl TransportWithoutIO for &mut T { - fn set_identity(&mut self, identity: Identity) -> Result<(), Error> { + fn set_identity(&mut self, identity: git_sec::Identity) -> Result<(), Error> { self.deref_mut().set_identity(identity) } diff --git a/git-transport/tests/client/blocking_io/http/mod.rs b/git-transport/tests/client/blocking_io/http/mod.rs index b6ad121cd01..7c7b422e564 100644 --- a/git-transport/tests/client/blocking_io/http/mod.rs +++ b/git-transport/tests/client/blocking_io/http/mod.rs @@ -9,7 +9,7 @@ use std::{ use bstr::ByteSlice; use git_transport::{ - client::{self, http, Identity, SetServiceResponse, Transport, TransportV2Ext, TransportWithoutIO}, + client::{self, http, SetServiceResponse, Transport, TransportV2Ext, TransportWithoutIO}, Protocol, Service, }; @@ -42,7 +42,7 @@ fn assert_error_status( fn http_authentication_error_can_be_differentiated_and_identity_is_transmitted() -> crate::Result { let (server, mut client) = assert_error_status(401, std::io::ErrorKind::PermissionDenied)?; server.next_read_and_respond_with(fixture_bytes("v1/http-handshake.response")); - client.set_identity(Identity::Account { + client.set_identity(git_sec::Identity::Account { username: "user".into(), password: "password".into(), })?; From 5cf8c2769dd7b0d8a9ee0e304f255ae124524261 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 14:16:37 +0800 Subject: [PATCH 12/34] fix installation test on windows (#386) --- .github/workflows/ci.yml | 4 ++-- git-sec/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c51fc6d80e8..35935f00326 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,8 +60,8 @@ jobs: if: startsWith(matrix.os, 'windows') uses: actions-rs/cargo@v1 with: - command: install --force - args: "gitoxide cargo-smart-release" + command: install + args: "--force gitoxide cargo-smart-release" lint: runs-on: ubuntu-latest diff --git a/git-sec/Cargo.toml b/git-sec/Cargo.toml index 43971f20511..c1a8829c657 100644 --- a/git-sec/Cargo.toml +++ b/git-sec/Cargo.toml @@ -17,4 +17,4 @@ serde1 = [ "serde" ] # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } +serde = { version = "1.0.114", optional = true, default-features = false, features = ["std", "derive"] } From 37a607db7c09ab897f306e3bbd4e0ca4e4387bae Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 16:37:57 +0800 Subject: [PATCH 13/34] change!: remove `Identity` in favor of `identity::Account` module; add `identity::UserId` (#386) As the fewest consumers will be able to deal with multiple identities, remove the enumeration approach in favor of individual type which deal with one specific way of identifying a user. --- git-sec/src/lib.rs | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index 4d29f6147ef..c74214ed774 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -2,15 +2,24 @@ #![deny(rust_2018_idioms, missing_docs)] //! A shared trust model for `gitoxide` crates. -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] -#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] -/// An identity for use when authenticating the transport layer. -pub enum Identity { +/// Various types to identify entities. +pub mod identity { + /// A unix user id as obtained from the file system. + #[cfg(not(windows))] + pub type UserId = u32; + + /// A windows [security identifier](https://docs.microsoft.com/en-us/windows/security/identity-protection/access-control/security-identifiers) + /// in its stringified form. + #[cfg(windows)] + pub type UserId = String; + + #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] + #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] /// An account based identity - Account { + pub struct Account { /// The user's name - username: String, + pub username: String, /// The user's password - password: String, - }, + pub password: String, + } } From c5e2346cee53019b1b321e45cf080b210e60bb7a Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 16:39:54 +0800 Subject: [PATCH 14/34] adapt to changes in git-sec (#386) --- git-credentials/src/helper.rs | 4 +-- git-protocol/src/fetch/tests/arguments.rs | 8 +++--- .../src/client/blocking_io/http/mod.rs | 28 ++++++++----------- git-transport/src/client/mod.rs | 2 +- git-transport/src/client/traits.rs | 6 ++-- .../tests/client/blocking_io/http/mod.rs | 2 +- 6 files changed, 23 insertions(+), 27 deletions(-) diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index 77c0556b821..8246f5be602 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -71,7 +71,7 @@ impl NextAction { /// The outcome of [`action()`]. pub struct Outcome { /// The obtained identity. - pub identity: git_sec::Identity, + pub identity: git_sec::identity::Account, /// A handle to the action to perform next using another call to [`action()`]. pub next: NextAction, } @@ -127,7 +127,7 @@ pub fn action(action: Action<'_>) -> Result { .map(|(_, n)| n.to_owned()) }; Ok(Some(Outcome { - identity: git_sec::Identity::Account { + identity: git_sec::identity::Account { username: find("username")?, password: find("password")?, }, diff --git a/git-protocol/src/fetch/tests/arguments.rs b/git-protocol/src/fetch/tests/arguments.rs index 874f2469677..c2c102246b7 100644 --- a/git-protocol/src/fetch/tests/arguments.rs +++ b/git-protocol/src/fetch/tests/arguments.rs @@ -20,14 +20,14 @@ struct Transport { mod impls { use git_transport::{ client, - client::{Error, Identity, MessageKind, RequestWriter, SetServiceResponse, WriteMode}, + client::{Error, MessageKind, RequestWriter, SetServiceResponse, WriteMode}, Protocol, Service, }; use crate::fetch::tests::arguments::Transport; impl client::TransportWithoutIO for Transport { - fn set_identity(&mut self, identity: Identity) -> Result<(), Error> { + fn set_identity(&mut self, identity: client::Account) -> Result<(), Error> { self.inner.set_identity(identity) } @@ -64,13 +64,13 @@ mod impls { use async_trait::async_trait; use git_transport::{ client, - client::{Error, Identity, MessageKind, RequestWriter, SetServiceResponse, WriteMode}, + client::{Error, MessageKind, RequestWriter, SetServiceResponse, WriteMode}, Protocol, Service, }; use crate::fetch::tests::arguments::Transport; impl client::TransportWithoutIO for Transport { - fn set_identity(&mut self, identity: Identity) -> Result<(), Error> { + fn set_identity(&mut self, identity: client::Account) -> Result<(), Error> { self.inner.set_identity(identity) } diff --git a/git-transport/src/client/blocking_io/http/mod.rs b/git-transport/src/client/blocking_io/http/mod.rs index a9a3c651d1f..1c8d7053bc9 100644 --- a/git-transport/src/client/blocking_io/http/mod.rs +++ b/git-transport/src/client/blocking_io/http/mod.rs @@ -32,7 +32,7 @@ pub struct Transport { http: H, service: Option, line_provider: Option>, - identity: Option, + identity: Option, } impl Transport { @@ -71,21 +71,17 @@ impl Transport { #[allow(clippy::unnecessary_wraps, unknown_lints)] fn add_basic_auth_if_present(&self, headers: &mut Vec>) -> Result<(), client::Error> { - if let Some(identity) = &self.identity { - match identity { - git_sec::Identity::Account { username, password } => { - #[cfg(not(debug_assertions))] - if self.url.starts_with("http://") { - return Err(client::Error::AuthenticationRefused( - "Will not send credentials in clear text over http", - )); - } - headers.push(Cow::Owned(format!( - "Authorization: Basic {}", - base64::encode(format!("{}:{}", username, password)) - ))) - } + if let Some(git_sec::identity::Account { username, password }) = &self.identity { + #[cfg(not(debug_assertions))] + if self.url.starts_with("http://") { + return Err(client::Error::AuthenticationRefused( + "Will not send credentials in clear text over http", + )); } + headers.push(Cow::Owned(format!( + "Authorization: Basic {}", + base64::encode(format!("{}:{}", username, password)) + ))) } Ok(()) } @@ -100,7 +96,7 @@ fn append_url(base: &str, suffix: &str) -> String { } impl client::TransportWithoutIO for Transport { - fn set_identity(&mut self, identity: git_sec::Identity) -> Result<(), client::Error> { + fn set_identity(&mut self, identity: git_sec::identity::Account) -> Result<(), client::Error> { self.identity = Some(identity); Ok(()) } diff --git a/git-transport/src/client/mod.rs b/git-transport/src/client/mod.rs index cb3c16684cd..1df469d8656 100644 --- a/git-transport/src/client/mod.rs +++ b/git-transport/src/client/mod.rs @@ -28,7 +28,7 @@ pub use capabilities::Capabilities; mod non_io_types; pub use non_io_types::{Error, MessageKind, WriteMode}; -pub use git_sec::Identity; +pub use git_sec::identity::Account; /// #[cfg(any(feature = "blocking-client", feature = "async-client"))] diff --git a/git-transport/src/client/traits.rs b/git-transport/src/client/traits.rs index 3e1694f88f2..f57b2fe2d09 100644 --- a/git-transport/src/client/traits.rs +++ b/git-transport/src/client/traits.rs @@ -13,7 +13,7 @@ pub trait TransportWithoutIO { /// of the identity in order to mark it as invalid. Otherwise the user might have difficulty updating obsolete /// credentials. /// Please note that most transport layers are unauthenticated and thus return [an error][Error::AuthenticationUnsupported] here. - fn set_identity(&mut self, _identity: git_sec::Identity) -> Result<(), Error> { + fn set_identity(&mut self, _identity: git_sec::identity::Account) -> Result<(), Error> { Err(Error::AuthenticationUnsupported) } /// Get a writer for sending data and obtaining the response. It can be configured in various ways @@ -50,7 +50,7 @@ pub trait TransportWithoutIO { // Would be nice if the box implementation could auto-forward to all implemented traits. impl TransportWithoutIO for Box { - fn set_identity(&mut self, identity: git_sec::Identity) -> Result<(), Error> { + fn set_identity(&mut self, identity: git_sec::identity::Account) -> Result<(), Error> { self.deref_mut().set_identity(identity) } @@ -73,7 +73,7 @@ impl TransportWithoutIO for Box { } impl TransportWithoutIO for &mut T { - fn set_identity(&mut self, identity: git_sec::Identity) -> Result<(), Error> { + fn set_identity(&mut self, identity: git_sec::identity::Account) -> Result<(), Error> { self.deref_mut().set_identity(identity) } diff --git a/git-transport/tests/client/blocking_io/http/mod.rs b/git-transport/tests/client/blocking_io/http/mod.rs index 7c7b422e564..1659ad51fa8 100644 --- a/git-transport/tests/client/blocking_io/http/mod.rs +++ b/git-transport/tests/client/blocking_io/http/mod.rs @@ -42,7 +42,7 @@ fn assert_error_status( fn http_authentication_error_can_be_differentiated_and_identity_is_transmitted() -> crate::Result { let (server, mut client) = assert_error_status(401, std::io::ErrorKind::PermissionDenied)?; server.next_read_and_respond_with(fixture_bytes("v1/http-handshake.response")); - client.set_identity(git_sec::Identity::Account { + client.set_identity(git_sec::identity::Account { username: "user".into(), password: "password".into(), })?; From f6077978fd5697bd113a894ba68492213becea41 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 17:18:18 +0800 Subject: [PATCH 15/34] feat: obtain identities `from_path()` or `from_process()` (#386) --- Cargo.lock | 6 ++-- git-sec/Cargo.toml | 7 ++++ git-sec/src/lib.rs | 63 +++++++++++++++++++++++++++++++++-- git-sec/tests/identity/mod.rs | 8 +++++ git-sec/tests/sec.rs | 3 ++ 5 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 git-sec/tests/identity/mod.rs create mode 100644 git-sec/tests/sec.rs diff --git a/Cargo.lock b/Cargo.lock index 171d196bb7f..0eb4bdbeac9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1459,7 +1459,9 @@ dependencies = [ name = "git-sec" version = "0.1.0" dependencies = [ + "libc", "serde", + "tempfile", ] [[package]] @@ -1850,9 +1852,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.122" +version = "0.2.123" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec647867e2bf0772e28c8bcde4f0d19a9216916e890543b5a03ed8ef27b8f259" +checksum = "cb691a747a7ab48abc15c5b42066eaafde10dc427e3b6ee2a1cf43db04c763bd" [[package]] name = "libgit2-sys" diff --git a/git-sec/Cargo.toml b/git-sec/Cargo.toml index c1a8829c657..2ccd1cd1e26 100644 --- a/git-sec/Cargo.toml +++ b/git-sec/Cargo.toml @@ -18,3 +18,10 @@ serde1 = [ "serde" ] [dependencies] serde = { version = "1.0.114", optional = true, default-features = false, features = ["std", "derive"] } + +[target.'cfg(not(windows))'.dependencies] +libc = "0.2.123" + + +[dev-dependencies] +tempfile = "3.3.0" diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index c74214ed774..a60d2e6883e 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -1,5 +1,4 @@ -#![forbid(unsafe_code)] -#![deny(rust_2018_idioms, missing_docs)] +#![deny(unsafe_code, rust_2018_idioms, missing_docs)] //! A shared trust model for `gitoxide` crates. /// Various types to identify entities. @@ -22,4 +21,64 @@ pub mod identity { /// The user's password pub password: String, } + + /// + pub mod user_id { + use crate::identity::UserId; + use std::borrow::Cow; + use std::path::Path; + + /// Obtain the owner of the given `path`. + pub fn from_path(path: Cow<'_, Path>) -> std::io::Result { + impl_::from_path(path) + } + + /// Obtain the of the currently running process. + pub fn from_process() -> Result { + impl_::from_process() + } + + /// + pub mod from_process { + use crate::identity::user_id::impl_; + + /// The error returned by [from_process()][super::from_process()]. + pub type Error = impl_::FromProcessError; + } + + #[cfg(not(windows))] + mod impl_ { + use crate::identity::UserId; + use std::borrow::Cow; + use std::path::Path; + + pub fn from_path(path: Cow<'_, Path>) -> std::io::Result { + use std::os::unix::fs::MetadataExt; + let meta = std::fs::symlink_metadata(path)?; + Ok(meta.uid()) + } + + pub type FromProcessError = std::convert::Infallible; + pub fn from_process() -> Result { + // SAFETY: there is no documented possibility for failure + #[allow(unsafe_code)] + let uid = unsafe { libc::geteuid() }; + Ok(uid) + } + } + + #[cfg(windows)] + mod impl_ { + use crate::identity::UserId; + use std::borrow::Cow; + use std::path::Path; + + pub fn from_path(path: Cow<'_, Path>) -> std::io::Result { + todo!("unix") + } + pub fn from_process() -> std::io::Result { + todo!("process") + } + } + } } diff --git a/git-sec/tests/identity/mod.rs b/git-sec/tests/identity/mod.rs new file mode 100644 index 00000000000..32b5f64bef3 --- /dev/null +++ b/git-sec/tests/identity/mod.rs @@ -0,0 +1,8 @@ +mod uid { + #[test] + fn from_path() { + let dir = tempfile::tempdir().unwrap(); + let owner = git_sec::identity::user_id::from_path(dir.path().into()).unwrap(); + assert_eq!(owner, git_sec::identity::user_id::from_process().unwrap()); + } +} diff --git a/git-sec/tests/sec.rs b/git-sec/tests/sec.rs new file mode 100644 index 00000000000..82f9aca8ce9 --- /dev/null +++ b/git-sec/tests/sec.rs @@ -0,0 +1,3 @@ +pub type Result = std::result::Result>; + +mod identity; From a58d2cf39b47e7a2c69ba639923bbece19f28230 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 17:19:46 +0800 Subject: [PATCH 16/34] refactor (#386) --- git-sec/src/lib.rs | 86 +++++++++++++++++------------------ git-sec/tests/identity/mod.rs | 4 +- 2 files changed, 43 insertions(+), 47 deletions(-) diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index a60d2e6883e..385d9aa2cd3 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -22,63 +22,59 @@ pub mod identity { pub password: String, } + use std::borrow::Cow; + use std::path::Path; + + /// Obtain the owner of the given `path`. + pub fn from_path(path: Cow<'_, Path>) -> std::io::Result { + impl_::from_path(path) + } + + /// Obtain the of the currently running process. + pub fn from_process() -> Result { + impl_::from_process() + } + /// - pub mod user_id { + pub mod from_process { + use crate::identity::impl_; + + /// The error returned by [from_process()][super::from_process()]. + pub type Error = impl_::FromProcessError; + } + + #[cfg(not(windows))] + mod impl_ { use crate::identity::UserId; use std::borrow::Cow; use std::path::Path; - /// Obtain the owner of the given `path`. pub fn from_path(path: Cow<'_, Path>) -> std::io::Result { - impl_::from_path(path) + use std::os::unix::fs::MetadataExt; + let meta = std::fs::symlink_metadata(path)?; + Ok(meta.uid()) } - /// Obtain the of the currently running process. - pub fn from_process() -> Result { - impl_::from_process() + pub type FromProcessError = std::convert::Infallible; + pub fn from_process() -> Result { + // SAFETY: there is no documented possibility for failure + #[allow(unsafe_code)] + let uid = unsafe { libc::geteuid() }; + Ok(uid) } + } - /// - pub mod from_process { - use crate::identity::user_id::impl_; - - /// The error returned by [from_process()][super::from_process()]. - pub type Error = impl_::FromProcessError; - } - - #[cfg(not(windows))] - mod impl_ { - use crate::identity::UserId; - use std::borrow::Cow; - use std::path::Path; - - pub fn from_path(path: Cow<'_, Path>) -> std::io::Result { - use std::os::unix::fs::MetadataExt; - let meta = std::fs::symlink_metadata(path)?; - Ok(meta.uid()) - } + #[cfg(windows)] + mod impl_ { + use crate::identity::UserId; + use std::borrow::Cow; + use std::path::Path; - pub type FromProcessError = std::convert::Infallible; - pub fn from_process() -> Result { - // SAFETY: there is no documented possibility for failure - #[allow(unsafe_code)] - let uid = unsafe { libc::geteuid() }; - Ok(uid) - } + pub fn from_path(path: Cow<'_, Path>) -> std::io::Result { + todo!("unix") } - - #[cfg(windows)] - mod impl_ { - use crate::identity::UserId; - use std::borrow::Cow; - use std::path::Path; - - pub fn from_path(path: Cow<'_, Path>) -> std::io::Result { - todo!("unix") - } - pub fn from_process() -> std::io::Result { - todo!("process") - } + pub fn from_process() -> std::io::Result { + todo!("process") } } } diff --git a/git-sec/tests/identity/mod.rs b/git-sec/tests/identity/mod.rs index 32b5f64bef3..dfc6b6118bf 100644 --- a/git-sec/tests/identity/mod.rs +++ b/git-sec/tests/identity/mod.rs @@ -2,7 +2,7 @@ mod uid { #[test] fn from_path() { let dir = tempfile::tempdir().unwrap(); - let owner = git_sec::identity::user_id::from_path(dir.path().into()).unwrap(); - assert_eq!(owner, git_sec::identity::user_id::from_process().unwrap()); + let owner = git_sec::identity::from_path(dir.path().into()).unwrap(); + assert_eq!(owner, git_sec::identity::from_process().unwrap()); } } From 7bbe44c979bd5ab7077206b6bb3adb1172030a3e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 19:53:22 +0800 Subject: [PATCH 17/34] refactor so that the windows implementation can happen (#386) For posterity, using the windows API is absolutely terrible and I hope I don't have to do it again anytime soon. More importantly, I hope that it works nonetheless. --- Cargo.lock | 54 ++++++++++++-- git-sec/Cargo.toml | 2 + git-sec/src/lib.rs | 135 ++++++++++++++++++++++++---------- git-sec/tests/identity/mod.rs | 8 +- 4 files changed, 150 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0eb4bdbeac9..062f6a50363 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1462,6 +1462,7 @@ dependencies = [ "libc", "serde", "tempfile", + "windows", ] [[package]] @@ -3146,17 +3147,30 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08746b4b7ac95f708b3cccceb97b7f9a21a8916dd47fc99b0e6aaf7208f26fd7" +dependencies = [ + "windows_aarch64_msvc 0.35.0", + "windows_i686_gnu 0.35.0", + "windows_i686_msvc 0.35.0", + "windows_x86_64_gnu 0.35.0", + "windows_x86_64_msvc 0.35.0", +] + [[package]] name = "windows-sys" version = "0.34.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5acdd78cb4ba54c0045ac14f62d8f94a03d10047904ae2a40afa1e99d8f70825" dependencies = [ - "windows_aarch64_msvc", - "windows_i686_gnu", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_msvc", + "windows_aarch64_msvc 0.34.0", + "windows_i686_gnu 0.34.0", + "windows_i686_msvc 0.34.0", + "windows_x86_64_gnu 0.34.0", + "windows_x86_64_msvc 0.34.0", ] [[package]] @@ -3165,30 +3179,60 @@ version = "0.34.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "17cffbe740121affb56fad0fc0e421804adf0ae00891205213b5cecd30db881d" +[[package]] +name = "windows_aarch64_msvc" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "db3bc5134e8ce0da5d64dcec3529793f1d33aee5a51fc2b4662e0f881dd463e6" + [[package]] name = "windows_i686_gnu" version = "0.34.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2564fde759adb79129d9b4f54be42b32c89970c18ebf93124ca8870a498688ed" +[[package]] +name = "windows_i686_gnu" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0343a6f35bf43a07b009b8591b78b10ea03de86b06f48e28c96206cd0f453b50" + [[package]] name = "windows_i686_msvc" version = "0.34.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9cd9d32ba70453522332c14d38814bceeb747d80b3958676007acadd7e166956" +[[package]] +name = "windows_i686_msvc" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1acdcbf4ca63d8e7a501be86fee744347186275ec2754d129ddeab7a1e3a02e4" + [[package]] name = "windows_x86_64_gnu" version = "0.34.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cfce6deae227ee8d356d19effc141a509cc503dfd1f850622ec4b0f84428e1f4" +[[package]] +name = "windows_x86_64_gnu" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "893c0924c5a990ec73cd2264d1c0cba1773a929e1a3f5dbccffd769f8c4edebb" + [[package]] name = "windows_x86_64_msvc" version = "0.34.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d19538ccc21819d01deaf88d6a17eae6596a12e9aafdbb97916fb49896d89de9" +[[package]] +name = "windows_x86_64_msvc" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a29bd61f32889c822c99a8fdf2e93378bd2fae4d7efd2693fab09fcaaf7eff4b" + [[package]] name = "xz2" version = "0.1.6" diff --git a/git-sec/Cargo.toml b/git-sec/Cargo.toml index 2ccd1cd1e26..67a684d2e54 100644 --- a/git-sec/Cargo.toml +++ b/git-sec/Cargo.toml @@ -22,6 +22,8 @@ serde = { version = "1.0.114", optional = true, default-features = false, featur [target.'cfg(not(windows))'.dependencies] libc = "0.2.123" +[target.'cfg(windows)'.dependencies] +windows = { version = "0.35.0", features = ["Win32_System_Threading", "Win32_Foundation", "Win32_Security", "Win32_Security_Authorization"] } [dev-dependencies] tempfile = "3.3.0" diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index 385d9aa2cd3..b357253b67c 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -3,14 +3,6 @@ /// Various types to identify entities. pub mod identity { - /// A unix user id as obtained from the file system. - #[cfg(not(windows))] - pub type UserId = u32; - - /// A windows [security identifier](https://docs.microsoft.com/en-us/windows/security/identity-protection/access-control/security-identifiers) - /// in its stringified form. - #[cfg(windows)] - pub type UserId = String; #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] @@ -25,56 +17,119 @@ pub mod identity { use std::borrow::Cow; use std::path::Path; - /// Obtain the owner of the given `path`. - pub fn from_path(path: Cow<'_, Path>) -> std::io::Result { - impl_::from_path(path) - } - - /// Obtain the of the currently running process. - pub fn from_process() -> Result { - impl_::from_process() - } - + /// Returns true if the given `path` is owned by the user who is executing the current process. /// - pub mod from_process { - use crate::identity::impl_; - - /// The error returned by [from_process()][super::from_process()]. - pub type Error = impl_::FromProcessError; + /// Note that this method is very specific to avoid having to deal with any operating system types. + pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { + impl_::is_path_owned_by_current_user(path) } #[cfg(not(windows))] mod impl_ { - use crate::identity::UserId; use std::borrow::Cow; use std::path::Path; - pub fn from_path(path: Cow<'_, Path>) -> std::io::Result { - use std::os::unix::fs::MetadataExt; - let meta = std::fs::symlink_metadata(path)?; - Ok(meta.uid()) - } + pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { + fn from_path(path: Cow<'_, Path>) -> std::io::Result { + use std::os::unix::fs::MetadataExt; + let meta = std::fs::symlink_metadata(path)?; + Ok(meta.uid()) + } - pub type FromProcessError = std::convert::Infallible; - pub fn from_process() -> Result { - // SAFETY: there is no documented possibility for failure - #[allow(unsafe_code)] - let uid = unsafe { libc::geteuid() }; - Ok(uid) + fn from_process() -> std::io::Result { + // SAFETY: there is no documented possibility for failure + #[allow(unsafe_code)] + let uid = unsafe { libc::geteuid() }; + Ok(uid) + } + + Ok(from_path(path)? == from_process()?) } } #[cfg(windows)] mod impl_ { - use crate::identity::UserId; use std::borrow::Cow; use std::path::Path; - pub fn from_path(path: Cow<'_, Path>) -> std::io::Result { - todo!("unix") + fn err(msg: &str) -> std::io::Error { + std::io::Error::new(std::io::ErrorKind::Other, msg) } - pub fn from_process() -> std::io::Result { - todo!("process") + + pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { + use windows::Win32::{ + Foundation::{CloseHandle, ERROR_SUCCESS, HANDLE, PSID}, + Security, + Security::Authorization::SE_FILE_OBJECT, + System::Threading, + }; + let mut handle = HANDLE::default(); + let mut descriptor = Security::PSECURITY_DESCRIPTOR::default(); + let mut err_msg = None; + let mut is_owned = false; + + #[allow(unsafe_code)] + unsafe { + Threading::OpenProcessToken(Threading::GetCurrentProcess(), Security::TOKEN_QUERY, &mut handle) + .ok() + .map_err(|_| err("Failed to open process token"))?; + + let mut len = 0_u32; + if Security::GetTokenInformation(&handle, Security::TokenUser, std::ptr::null_mut(), 0, &mut len) + .as_bool() + { + let mut token_user = Security::TOKEN_USER::default(); + if Security::GetTokenInformation( + &handle, + Security::TokenUser, + &mut token_user as *mut _ as *mut std::ffi::c_void, + len, + &mut len, + ) + .as_bool() + { + // NOTE: we avoid to copy the sid or cache it in any way for now, even though it should be possible + // with a custom allocation/vec/box and it's just very raw. Can the `windows` crate do better? + // When/If yes, then let's improve this. + if Security::IsValidSid(token_user.User.Sid).as_bool() { + use std::os::windows::ffi::OsStrExt; + let mut wide_path: Vec<_> = path.as_ref().as_os_str().encode_wide().collect(); + // err = GetNamedSecurityInfoW(wpath, SE_FILE_OBJECT, + // OWNER_SECURITY_INFORMATION | + // DACL_SECURITY_INFORMATION, + // &sid, NULL, NULL, NULL, &descriptor); + let mut path_sid = PSID::default(); + let res = Security::Authorization::GetNamedSecurityInfoW( + windows::core::PCWSTR(wide_path.as_mut_ptr()), + SE_FILE_OBJECT, + Security::OWNER_SECURITY_INFORMATION | Security::DACL_SECURITY_INFORMATION, + &mut path_sid, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut descriptor, + ); + + if res == ERROR_SUCCESS.0 && Security::IsValidSid(path_sid).as_bool() { + is_owned = Security::EqualSid(path_sid, token_user.User.Sid).as_bool(); + } else { + err_msg = "couldn't get owner for path or it wasn't valid".into(); + } + } else { + err_msg = "owner id of current process wasn't set or valid".into(); + } + } else { + err_msg = "Could not get information about the token user".into(); + } + } else { + err_msg = "Could not get token information for length of token user".into(); + } + CloseHandle(handle); + if !descriptor.is_invalid() { + windows::core::heap_free(descriptor.0); + } + } + err_msg.map(|msg| Err(err(msg))).unwrap_or(Ok(is_owned)) } } } diff --git a/git-sec/tests/identity/mod.rs b/git-sec/tests/identity/mod.rs index dfc6b6118bf..b418c87a31c 100644 --- a/git-sec/tests/identity/mod.rs +++ b/git-sec/tests/identity/mod.rs @@ -1,8 +1,8 @@ mod uid { #[test] - fn from_path() { - let dir = tempfile::tempdir().unwrap(); - let owner = git_sec::identity::from_path(dir.path().into()).unwrap(); - assert_eq!(owner, git_sec::identity::from_process().unwrap()); + fn from_path() -> crate::Result { + let dir = tempfile::tempdir()?; + assert!(git_sec::identity::is_path_owned_by_current_user(dir.path().into())?); + Ok(()) } } From d6c6ec6aae197762a9ebe617bcca4963194465ff Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 19:56:02 +0800 Subject: [PATCH 18/34] turn on fast windows testing of git-sec for now (#386) --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 35935f00326..9c73072dc24 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,8 @@ jobs: if: startsWith(matrix.os, 'macos') run: brew install tree openssl + - run: cargo test --package git-sec + if: startsWith(matrix.os, 'windows') - name: "cargo check default features" if: startsWith(matrix.os, 'windows') uses: actions-rs/cargo@v1 From 0dac74e83fd8da00fc54765f22b0557f400e08c2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 15 Apr 2022 20:06:26 +0800 Subject: [PATCH 19/34] see if this makes a difference on windows (#386) --- git-sec/src/lib.rs | 48 ++++++++++++++++++++--------------- git-sec/tests/identity/mod.rs | 3 +++ 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index b357253b67c..3363f896f96 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -52,8 +52,8 @@ pub mod identity { use std::borrow::Cow; use std::path::Path; - fn err(msg: &str) -> std::io::Error { - std::io::Error::new(std::io::ErrorKind::Other, msg) + fn err(msg: impl Into) -> std::io::Error { + std::io::Error::new(std::io::ErrorKind::Other, msg.into()) } pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { @@ -75,14 +75,15 @@ pub mod identity { .map_err(|_| err("Failed to open process token"))?; let mut len = 0_u32; - if Security::GetTokenInformation(&handle, Security::TokenUser, std::ptr::null_mut(), 0, &mut len) + if !Security::GetTokenInformation(handle, Security::TokenUser, std::ptr::null_mut(), 0, &mut len) .as_bool() { - let mut token_user = Security::TOKEN_USER::default(); + let mut info_buf = Vec::::new(); + info_buf.reserve_exact(len as usize); if Security::GetTokenInformation( - &handle, + handle, Security::TokenUser, - &mut token_user as *mut _ as *mut std::ffi::c_void, + info_buf.as_mut_ptr() as *mut std::ffi::c_void, len, &mut len, ) @@ -91,38 +92,36 @@ pub mod identity { // NOTE: we avoid to copy the sid or cache it in any way for now, even though it should be possible // with a custom allocation/vec/box and it's just very raw. Can the `windows` crate do better? // When/If yes, then let's improve this. - if Security::IsValidSid(token_user.User.Sid).as_bool() { - use std::os::windows::ffi::OsStrExt; - let mut wide_path: Vec<_> = path.as_ref().as_os_str().encode_wide().collect(); - // err = GetNamedSecurityInfoW(wpath, SE_FILE_OBJECT, - // OWNER_SECURITY_INFORMATION | - // DACL_SECURITY_INFORMATION, - // &sid, NULL, NULL, NULL, &descriptor); + // It should however be possible to create strings from SIDs, check this once more. + let info: *const Security::TOKEN_USER = std::mem::transmute(info_buf.as_ptr()); + if Security::IsValidSid((*info).User.Sid).as_bool() { + let wide_path = to_wide_path(&path); let mut path_sid = PSID::default(); let res = Security::Authorization::GetNamedSecurityInfoW( - windows::core::PCWSTR(wide_path.as_mut_ptr()), + windows::core::PCWSTR(wide_path.as_ptr()), SE_FILE_OBJECT, Security::OWNER_SECURITY_INFORMATION | Security::DACL_SECURITY_INFORMATION, - &mut path_sid, + &mut path_sid as *mut _, std::ptr::null_mut(), std::ptr::null_mut(), std::ptr::null_mut(), - &mut descriptor, + &mut descriptor as *mut _, ); if res == ERROR_SUCCESS.0 && Security::IsValidSid(path_sid).as_bool() { - is_owned = Security::EqualSid(path_sid, token_user.User.Sid).as_bool(); + is_owned = Security::EqualSid(path_sid, (*info).User.Sid).as_bool(); + dbg!(is_owned, path.as_ref()); } else { - err_msg = "couldn't get owner for path or it wasn't valid".into(); + err_msg = format!("couldn't get owner for path or it wasn't valid: {}", res).into(); } } else { - err_msg = "owner id of current process wasn't set or valid".into(); + err_msg = String::from("owner id of current process wasn't set or valid").into(); } } else { - err_msg = "Could not get information about the token user".into(); + err_msg = String::from("Could not get information about the token user").into(); } } else { - err_msg = "Could not get token information for length of token user".into(); + err_msg = String::from("Could not get token information for length of token user").into(); } CloseHandle(handle); if !descriptor.is_invalid() { @@ -131,5 +130,12 @@ pub mod identity { } err_msg.map(|msg| Err(err(msg))).unwrap_or(Ok(is_owned)) } + + fn to_wide_path(path: &Path) -> Vec { + use std::os::windows::ffi::OsStrExt; + let mut wide_path: Vec<_> = path.as_os_str().encode_wide().collect(); + wide_path.push(0); + wide_path + } } } diff --git a/git-sec/tests/identity/mod.rs b/git-sec/tests/identity/mod.rs index b418c87a31c..9ca413c2771 100644 --- a/git-sec/tests/identity/mod.rs +++ b/git-sec/tests/identity/mod.rs @@ -2,6 +2,9 @@ mod uid { #[test] fn from_path() -> crate::Result { let dir = tempfile::tempdir()?; + let file = dir.path().join("file"); + std::fs::write(&file, &[])?; + assert!(git_sec::identity::is_path_owned_by_current_user(file.into())?); assert!(git_sec::identity::is_path_owned_by_current_user(dir.path().into())?); Ok(()) } From de5ff1b5b0d0ba59fa10ec85ed849ed8e1f85f62 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 16 Apr 2022 08:10:37 +0800 Subject: [PATCH 20/34] See if checking for membership instead works (#386) Inspired by this suggestion. https://github.com/microsoft/windows-rs/issues/1697 --- .github/workflows/ci.yml | 2 - git-sec/Cargo.toml | 8 +++- git-sec/src/lib.rs | 95 +++++++++++++++------------------------- 3 files changed, 42 insertions(+), 63 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9c73072dc24..35935f00326 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,8 +40,6 @@ jobs: if: startsWith(matrix.os, 'macos') run: brew install tree openssl - - run: cargo test --package git-sec - if: startsWith(matrix.os, 'windows') - name: "cargo check default features" if: startsWith(matrix.os, 'windows') uses: actions-rs/cargo@v1 diff --git a/git-sec/Cargo.toml b/git-sec/Cargo.toml index 67a684d2e54..f5fe0d9e702 100644 --- a/git-sec/Cargo.toml +++ b/git-sec/Cargo.toml @@ -23,7 +23,13 @@ serde = { version = "1.0.114", optional = true, default-features = false, featur libc = "0.2.123" [target.'cfg(windows)'.dependencies] -windows = { version = "0.35.0", features = ["Win32_System_Threading", "Win32_Foundation", "Win32_Security", "Win32_Security_Authorization"] } +windows = { version = "0.35.0", features = [ "alloc", + "Win32_Foundation", + "Win32_Security_Authorization", + "Win32_Storage_FileSystem", + "Win32_System_Memory", + "Win32_System_Threading" +] } [dev-dependencies] tempfile = "3.3.0" diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index 3363f896f96..94623167006 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -57,77 +57,52 @@ pub mod identity { } pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { - use windows::Win32::{ - Foundation::{CloseHandle, ERROR_SUCCESS, HANDLE, PSID}, - Security, - Security::Authorization::SE_FILE_OBJECT, - System::Threading, + use windows::{ + core::PCWSTR, + Win32::{ + Foundation::{BOOL, ERROR_SUCCESS, HANDLE, PSID}, + Security::{ + Authorization::{GetNamedSecurityInfoW, SE_FILE_OBJECT}, + CheckTokenMembershipEx, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, + }, + System::Memory::LocalFree, + }, }; - let mut handle = HANDLE::default(); - let mut descriptor = Security::PSECURITY_DESCRIPTOR::default(); + let mut err_msg = None; let mut is_owned = false; #[allow(unsafe_code)] unsafe { - Threading::OpenProcessToken(Threading::GetCurrentProcess(), Security::TOKEN_QUERY, &mut handle) - .ok() - .map_err(|_| err("Failed to open process token"))?; - - let mut len = 0_u32; - if !Security::GetTokenInformation(handle, Security::TokenUser, std::ptr::null_mut(), 0, &mut len) - .as_bool() - { - let mut info_buf = Vec::::new(); - info_buf.reserve_exact(len as usize); - if Security::GetTokenInformation( - handle, - Security::TokenUser, - info_buf.as_mut_ptr() as *mut std::ffi::c_void, - len, - &mut len, - ) - .as_bool() - { - // NOTE: we avoid to copy the sid or cache it in any way for now, even though it should be possible - // with a custom allocation/vec/box and it's just very raw. Can the `windows` crate do better? - // When/If yes, then let's improve this. - // It should however be possible to create strings from SIDs, check this once more. - let info: *const Security::TOKEN_USER = std::mem::transmute(info_buf.as_ptr()); - if Security::IsValidSid((*info).User.Sid).as_bool() { - let wide_path = to_wide_path(&path); - let mut path_sid = PSID::default(); - let res = Security::Authorization::GetNamedSecurityInfoW( - windows::core::PCWSTR(wide_path.as_ptr()), - SE_FILE_OBJECT, - Security::OWNER_SECURITY_INFORMATION | Security::DACL_SECURITY_INFORMATION, - &mut path_sid as *mut _, - std::ptr::null_mut(), - std::ptr::null_mut(), - std::ptr::null_mut(), - &mut descriptor as *mut _, - ); - - if res == ERROR_SUCCESS.0 && Security::IsValidSid(path_sid).as_bool() { - is_owned = Security::EqualSid(path_sid, (*info).User.Sid).as_bool(); - dbg!(is_owned, path.as_ref()); - } else { - err_msg = format!("couldn't get owner for path or it wasn't valid: {}", res).into(); - } - } else { - err_msg = String::from("owner id of current process wasn't set or valid").into(); - } + let mut psid = PSID::default(); + let mut pdescriptor = PSECURITY_DESCRIPTOR::default(); + let wpath = to_wide_path(&path); + + let result = GetNamedSecurityInfoW( + PCWSTR(wpath.as_ptr()), + SE_FILE_OBJECT, + OWNER_SECURITY_INFORMATION, + &mut psid, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut pdescriptor, + ); + + if result == ERROR_SUCCESS.0 { + let mut is_member = BOOL(0); + if CheckTokenMembershipEx(HANDLE::default(), psid, 0, &mut is_member).as_bool() { + is_owned = is_member.as_bool(); } else { - err_msg = String::from("Could not get information about the token user").into(); + err_msg = String::from("Could not check token membership").into(); } } else { - err_msg = String::from("Could not get token information for length of token user").into(); - } - CloseHandle(handle); - if !descriptor.is_invalid() { - windows::core::heap_free(descriptor.0); + err_msg = format!("Could not get security information for path with err: {}", result).into(); } + + LocalFree(pdescriptor.0 as isize); } + err_msg.map(|msg| Err(err(msg))).unwrap_or(Ok(is_owned)) } From 9a3f0ba8277d92eb75129931993bddbd9961ccdd Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 16 Apr 2022 11:41:50 +0800 Subject: [PATCH 21/34] refactor (#386) --- git-sec/src/identity.rs | 109 ++++++++++++++++++++++++++++++++++++++ git-sec/src/lib.rs | 113 +--------------------------------------- 2 files changed, 110 insertions(+), 112 deletions(-) create mode 100644 git-sec/src/identity.rs diff --git a/git-sec/src/identity.rs b/git-sec/src/identity.rs new file mode 100644 index 00000000000..7b95cf66acc --- /dev/null +++ b/git-sec/src/identity.rs @@ -0,0 +1,109 @@ +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] +#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] +/// An account based identity +pub struct Account { + /// The user's name + pub username: String, + /// The user's password + pub password: String, +} + +use std::borrow::Cow; +use std::path::Path; + +/// Returns true if the given `path` is owned by the user who is executing the current process. +/// +/// Note that this method is very specific to avoid having to deal with any operating system types. +pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { + impl_::is_path_owned_by_current_user(path) +} + +#[cfg(not(windows))] +mod impl_ { + use std::borrow::Cow; + use std::path::Path; + + pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { + fn from_path(path: Cow<'_, Path>) -> std::io::Result { + use std::os::unix::fs::MetadataExt; + let meta = std::fs::symlink_metadata(path)?; + Ok(meta.uid()) + } + + fn from_process() -> std::io::Result { + // SAFETY: there is no documented possibility for failure + #[allow(unsafe_code)] + let uid = unsafe { libc::geteuid() }; + Ok(uid) + } + + Ok(from_path(path)? == from_process()?) + } +} + +#[cfg(windows)] +mod impl_ { + use std::borrow::Cow; + use std::path::Path; + + fn err(msg: impl Into) -> std::io::Error { + std::io::Error::new(std::io::ErrorKind::Other, msg.into()) + } + + pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { + use windows::{ + core::PCWSTR, + Win32::{ + Foundation::{BOOL, ERROR_SUCCESS, HANDLE, PSID}, + Security::{ + Authorization::{GetNamedSecurityInfoW, SE_FILE_OBJECT}, + CheckTokenMembershipEx, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, + }, + System::Memory::LocalFree, + }, + }; + + let mut err_msg = None; + let mut is_owned = false; + + #[allow(unsafe_code)] + unsafe { + let mut psid = PSID::default(); + let mut pdescriptor = PSECURITY_DESCRIPTOR::default(); + let wpath = to_wide_path(&path); + + let result = GetNamedSecurityInfoW( + PCWSTR(wpath.as_ptr()), + SE_FILE_OBJECT, + OWNER_SECURITY_INFORMATION, + &mut psid, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut pdescriptor, + ); + + if result == ERROR_SUCCESS.0 { + let mut is_member = BOOL(0); + if CheckTokenMembershipEx(HANDLE::default(), psid, 0, &mut is_member).as_bool() { + is_owned = is_member.as_bool(); + } else { + err_msg = String::from("Could not check token membership").into(); + } + } else { + err_msg = format!("Could not get security information for path with err: {}", result).into(); + } + + LocalFree(pdescriptor.0 as isize); + } + + err_msg.map(|msg| Err(err(msg))).unwrap_or(Ok(is_owned)) + } + + fn to_wide_path(path: &Path) -> Vec { + use std::os::windows::ffi::OsStrExt; + let mut wide_path: Vec<_> = path.as_os_str().encode_wide().collect(); + wide_path.push(0); + wide_path + } +} diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index 94623167006..d01a6bd919a 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -2,115 +2,4 @@ //! A shared trust model for `gitoxide` crates. /// Various types to identify entities. -pub mod identity { - - #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] - #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] - /// An account based identity - pub struct Account { - /// The user's name - pub username: String, - /// The user's password - pub password: String, - } - - use std::borrow::Cow; - use std::path::Path; - - /// Returns true if the given `path` is owned by the user who is executing the current process. - /// - /// Note that this method is very specific to avoid having to deal with any operating system types. - pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { - impl_::is_path_owned_by_current_user(path) - } - - #[cfg(not(windows))] - mod impl_ { - use std::borrow::Cow; - use std::path::Path; - - pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { - fn from_path(path: Cow<'_, Path>) -> std::io::Result { - use std::os::unix::fs::MetadataExt; - let meta = std::fs::symlink_metadata(path)?; - Ok(meta.uid()) - } - - fn from_process() -> std::io::Result { - // SAFETY: there is no documented possibility for failure - #[allow(unsafe_code)] - let uid = unsafe { libc::geteuid() }; - Ok(uid) - } - - Ok(from_path(path)? == from_process()?) - } - } - - #[cfg(windows)] - mod impl_ { - use std::borrow::Cow; - use std::path::Path; - - fn err(msg: impl Into) -> std::io::Error { - std::io::Error::new(std::io::ErrorKind::Other, msg.into()) - } - - pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { - use windows::{ - core::PCWSTR, - Win32::{ - Foundation::{BOOL, ERROR_SUCCESS, HANDLE, PSID}, - Security::{ - Authorization::{GetNamedSecurityInfoW, SE_FILE_OBJECT}, - CheckTokenMembershipEx, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, - }, - System::Memory::LocalFree, - }, - }; - - let mut err_msg = None; - let mut is_owned = false; - - #[allow(unsafe_code)] - unsafe { - let mut psid = PSID::default(); - let mut pdescriptor = PSECURITY_DESCRIPTOR::default(); - let wpath = to_wide_path(&path); - - let result = GetNamedSecurityInfoW( - PCWSTR(wpath.as_ptr()), - SE_FILE_OBJECT, - OWNER_SECURITY_INFORMATION, - &mut psid, - std::ptr::null_mut(), - std::ptr::null_mut(), - std::ptr::null_mut(), - &mut pdescriptor, - ); - - if result == ERROR_SUCCESS.0 { - let mut is_member = BOOL(0); - if CheckTokenMembershipEx(HANDLE::default(), psid, 0, &mut is_member).as_bool() { - is_owned = is_member.as_bool(); - } else { - err_msg = String::from("Could not check token membership").into(); - } - } else { - err_msg = format!("Could not get security information for path with err: {}", result).into(); - } - - LocalFree(pdescriptor.0 as isize); - } - - err_msg.map(|msg| Err(err(msg))).unwrap_or(Ok(is_owned)) - } - - fn to_wide_path(path: &Path) -> Vec { - use std::os::windows::ffi::OsStrExt; - let mut wide_path: Vec<_> = path.as_os_str().encode_wide().collect(); - wide_path.push(0); - wide_path - } - } -} +pub mod identity; From c06606991babd947f24e6d934b66b04f62dff1a9 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 16 Apr 2022 12:59:57 +0800 Subject: [PATCH 22/34] a sketch on how to deal with permissions for executables (#386) --- git-sec/src/lib.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index d01a6bd919a..a680458ba6b 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -1,5 +1,26 @@ #![deny(unsafe_code, rust_2018_idioms, missing_docs)] //! A shared trust model for `gitoxide` crates. +/// which programs we may execute in order to run them. +pub mod execute { + /// What kind of executables we can run + pub enum Permission { + /// The greatest permission level without any restrictions. + /// + /// The executables can be found in the PATH or configured from any git config file. + /// + /// Note that, however, some executables still won't be picked up from repository-local configuration + /// for safety reasons. + All, + /// Only run executables if these have been configured by git config files that are owned by the user executing + /// the application. + IfUnderUserControl { + /// If an executable is not under user control, instead of failing, fallback to a configuration setting that + /// is or try to not fail by using suitable defaults if possible. + allow_fallback: bool, + }, + } +} + /// Various types to identify entities. pub mod identity; From ca26659eb870c8e947962fe0647a07d01b3e95e4 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 16 Apr 2022 13:26:00 +0800 Subject: [PATCH 23/34] more details for path permissions (#386) --- git-credentials/src/helper.rs | 2 ++ git-sec/src/lib.rs | 27 ++++++++++++++++++--------- git-sec/tests/identity/mod.rs | 18 ++++++++---------- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index 8246f5be602..32a10c90960 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -86,6 +86,8 @@ fn git_program() -> &'static str { "git" } +// TODO(sec): reimplement helper execution so it won't use the `git credential` anymore to allow enforcing our own security model. +// Currently we support more flexible configuration than downright not working at all. /// Call the `git` credentials helper program performing the given `action`. /// /// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index a680458ba6b..341db772114 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -1,22 +1,31 @@ #![deny(unsafe_code, rust_2018_idioms, missing_docs)] //! A shared trust model for `gitoxide` crates. -/// which programs we may execute in order to run them. -pub mod execute { - /// What kind of executables we can run +/// +pub mod path { + /// Permissions related to _locations_ to executables, resources or destinations for operations. pub enum Permission { - /// The greatest permission level without any restrictions. + /// The greatest permission level without any restrictions, all _locations_ are permitted. /// - /// The executables can be found in the PATH or configured from any git config file. + /// For _locations_ to executables, it can be found in the `PATH` or configured from any git config file. /// /// Note that, however, some executables still won't be picked up from repository-local configuration /// for safety reasons. All, - /// Only run executables if these have been configured by git config files that are owned by the user executing - /// the application. + /// For _locations_ to executables, only run these if these have been configured by git config files + /// that are owned by the user executing the application, or if these are in the `PATH`. + /// Resources or write destinations adhere to the same rules. IfUnderUserControl { - /// If an executable is not under user control, instead of failing, fallback to a configuration setting that - /// is or try to not fail by using suitable defaults if possible. + /// If true, if a _location_ is not under user control, instead of failing, fallback to a configuration setting that + /// is or try to not fail by using suitable defaults. For executables this may mean to search for them in the `PATH` + /// or fall back to another configuration value from configuration files under user control. + allow_fallback: bool, + }, + /// Do not use any _location_ unless it's required for git to function by using defaults. + None { + /// If true, and the _location_ is an executable, do not use any configured location but allow using default + /// executables from the `PATH`. + /// Otherwise any operation requiring an additional executable isn't allowed to proceed. allow_fallback: bool, }, } diff --git a/git-sec/tests/identity/mod.rs b/git-sec/tests/identity/mod.rs index 9ca413c2771..2572daacfa3 100644 --- a/git-sec/tests/identity/mod.rs +++ b/git-sec/tests/identity/mod.rs @@ -1,11 +1,9 @@ -mod uid { - #[test] - fn from_path() -> crate::Result { - let dir = tempfile::tempdir()?; - let file = dir.path().join("file"); - std::fs::write(&file, &[])?; - assert!(git_sec::identity::is_path_owned_by_current_user(file.into())?); - assert!(git_sec::identity::is_path_owned_by_current_user(dir.path().into())?); - Ok(()) - } +#[test] +fn is_path_owned_by_current_user() -> crate::Result { + let dir = tempfile::tempdir()?; + let file = dir.path().join("file"); + std::fs::write(&file, &[])?; + assert!(git_sec::identity::is_path_owned_by_current_user(file.into())?); + assert!(git_sec::identity::is_path_owned_by_current_user(dir.path().into())?); + Ok(()) } From b0d06ca108c7f3f7078a8f00f62edc2011231581 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 16 Apr 2022 13:44:55 +0800 Subject: [PATCH 24/34] abstractions which should be powerful enough to handle our use-cases (#386) --- git-sec/src/identity.rs | 19 +++++++--------- git-sec/src/lib.rs | 42 +++++++++++++++++++++++++++++++++++ git-sec/tests/identity/mod.rs | 4 ++-- 3 files changed, 52 insertions(+), 13 deletions(-) diff --git a/git-sec/src/identity.rs b/git-sec/src/identity.rs index 7b95cf66acc..e618d194089 100644 --- a/git-sec/src/identity.rs +++ b/git-sec/src/identity.rs @@ -1,3 +1,5 @@ +use std::path::Path; + #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] /// An account based identity @@ -8,23 +10,19 @@ pub struct Account { pub password: String, } -use std::borrow::Cow; -use std::path::Path; - /// Returns true if the given `path` is owned by the user who is executing the current process. /// /// Note that this method is very specific to avoid having to deal with any operating system types. -pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { +pub fn is_path_owned_by_current_user(path: impl AsRef) -> std::io::Result { impl_::is_path_owned_by_current_user(path) } #[cfg(not(windows))] mod impl_ { - use std::borrow::Cow; use std::path::Path; - pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { - fn from_path(path: Cow<'_, Path>) -> std::io::Result { + pub fn is_path_owned_by_current_user(path: impl AsRef) -> std::io::Result { + fn from_path(path: impl AsRef) -> std::io::Result { use std::os::unix::fs::MetadataExt; let meta = std::fs::symlink_metadata(path)?; Ok(meta.uid()) @@ -43,14 +41,13 @@ mod impl_ { #[cfg(windows)] mod impl_ { - use std::borrow::Cow; use std::path::Path; fn err(msg: impl Into) -> std::io::Error { std::io::Error::new(std::io::ErrorKind::Other, msg.into()) } - pub fn is_path_owned_by_current_user(path: Cow<'_, Path>) -> std::io::Result { + pub fn is_path_owned_by_current_user(path: impl AsRef) -> std::io::Result { use windows::{ core::PCWSTR, Win32::{ @@ -100,9 +97,9 @@ mod impl_ { err_msg.map(|msg| Err(err(msg))).unwrap_or(Ok(is_owned)) } - fn to_wide_path(path: &Path) -> Vec { + fn to_wide_path(path: impl AsRef) -> Vec { use std::os::windows::ffi::OsStrExt; - let mut wide_path: Vec<_> = path.as_os_str().encode_wide().collect(); + let mut wide_path: Vec<_> = path.as_ref().as_os_str().encode_wide().collect(); wide_path.push(0); wide_path } diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index 341db772114..9c282fd326d 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -1,6 +1,48 @@ #![deny(unsafe_code, rust_2018_idioms, missing_docs)] //! A shared trust model for `gitoxide` crates. +use std::path::Path; + +/// A way to specify how 'safe' we feel about a resource, typically about a git repository. +pub enum Trust { + /// We have no doubts that this resource means no harm and it can be used at will. + Full, + /// Caution is warranted when using the resource. + Reduced, +} + +impl Trust { + /// Derive `Full` trust if `path` is owned by the user executing the current process, or `Reduced` trust otherwise. + pub fn from_path_ownership(path: impl AsRef) -> std::io::Result { + Ok(identity::is_path_owned_by_current_user(path.as_ref())? + .then(|| Trust::Full) + .unwrap_or(Trust::Reduced)) + } +} + +/// +pub mod trust { + use crate::Trust; + + /// Associate instructions for how to deal with various `Trust` levels as they are encountered in the wild. + pub struct Mapping { + /// The value for fully trusted resources. + pub full: T, + /// The value for resources with reduced trust. + pub reduced: T, + } + + impl Mapping { + /// Obtain the value for the given trust `level`. + pub fn by_trust(&self, level: &Trust) -> &T { + match level { + Trust::Full => &self.full, + Trust::Reduced => &self.reduced, + } + } + } +} + /// pub mod path { /// Permissions related to _locations_ to executables, resources or destinations for operations. diff --git a/git-sec/tests/identity/mod.rs b/git-sec/tests/identity/mod.rs index 2572daacfa3..99fc3f71427 100644 --- a/git-sec/tests/identity/mod.rs +++ b/git-sec/tests/identity/mod.rs @@ -3,7 +3,7 @@ fn is_path_owned_by_current_user() -> crate::Result { let dir = tempfile::tempdir()?; let file = dir.path().join("file"); std::fs::write(&file, &[])?; - assert!(git_sec::identity::is_path_owned_by_current_user(file.into())?); - assert!(git_sec::identity::is_path_owned_by_current_user(dir.path().into())?); + assert!(git_sec::identity::is_path_owned_by_current_user(file)?); + assert!(git_sec::identity::is_path_owned_by_current_user(dir.path())?); Ok(()) } From 0e74c7198607e2d44c0fab5a91789821d58ac9dc Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 16 Apr 2022 14:07:22 +0800 Subject: [PATCH 25/34] refactor (#386) --- git-sec/src/lib.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index 9c282fd326d..4f2f29c13ab 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -44,8 +44,12 @@ pub mod trust { } /// -pub mod path { - /// Permissions related to _locations_ to executables, resources or destinations for operations. +pub mod resource { + /// Permissions related to resources at _locations_, like configuration files, executables or destinations for operations. + /// + /// Note that typically the permission refers to the place where the _location_ is configured, not to the _location_ itself. + /// For example, we may trust an owned configuration file, and by relation all the _locations_ inside of it even though + /// these are not owned by us. pub enum Permission { /// The greatest permission level without any restrictions, all _locations_ are permitted. /// @@ -53,21 +57,24 @@ pub mod path { /// /// Note that, however, some executables still won't be picked up from repository-local configuration /// for safety reasons. - All, + Allow, /// For _locations_ to executables, only run these if these have been configured by git config files /// that are owned by the user executing the application, or if these are in the `PATH`. /// Resources or write destinations adhere to the same rules. - IfUnderUserControl { + AllowIfOwned { /// If true, if a _location_ is not under user control, instead of failing, fallback to a configuration setting that /// is or try to not fail by using suitable defaults. For executables this may mean to search for them in the `PATH` /// or fall back to another configuration value from configuration files under user control. allow_fallback: bool, }, /// Do not use any _location_ unless it's required for git to function by using defaults. - None { - /// If true, and the _location_ is an executable, do not use any configured location but allow using default - /// executables from the `PATH`. - /// Otherwise any operation requiring an additional executable isn't allowed to proceed. + /// + /// If such a resource is encountered, the operation may fail. + Deny { + /// If true, operations that would fail may proceed by ignoring the resource if possible or using + /// defaults that are deemed safe. + /// + /// For executables this means using those in the `PATH` only. allow_fallback: bool, }, } From a43e25b2be744a46f2a73690f3cdd2440c3e1070 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 16 Apr 2022 16:32:32 +0800 Subject: [PATCH 26/34] fully typed access control with tagged permissions (#386) Is it desirable have permissions tagged like this? These would typically show up in options, and I have a feeling it makes them more descriptive. That's all really. --- git-sec/src/identity.rs | 6 +-- git-sec/src/lib.rs | 107 ++++++++++++++++++++++++++++------------ 2 files changed, 79 insertions(+), 34 deletions(-) diff --git a/git-sec/src/identity.rs b/git-sec/src/identity.rs index e618d194089..2483c0fc356 100644 --- a/git-sec/src/identity.rs +++ b/git-sec/src/identity.rs @@ -22,20 +22,20 @@ mod impl_ { use std::path::Path; pub fn is_path_owned_by_current_user(path: impl AsRef) -> std::io::Result { - fn from_path(path: impl AsRef) -> std::io::Result { + fn owner_from_path(path: impl AsRef) -> std::io::Result { use std::os::unix::fs::MetadataExt; let meta = std::fs::symlink_metadata(path)?; Ok(meta.uid()) } - fn from_process() -> std::io::Result { + fn owner_of_current_process() -> std::io::Result { // SAFETY: there is no documented possibility for failure #[allow(unsafe_code)] let uid = unsafe { libc::geteuid() }; Ok(uid) } - Ok(from_path(path)? == from_process()?) + Ok(owner_from_path(path)? == owner_of_current_process()?) } } diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index 4f2f29c13ab..6a37b5de3e5 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -1,6 +1,7 @@ #![deny(unsafe_code, rust_2018_idioms, missing_docs)] //! A shared trust model for `gitoxide` crates. +use std::marker::PhantomData; use std::path::Path; /// A way to specify how 'safe' we feel about a resource, typically about a git repository. @@ -44,41 +45,85 @@ pub mod trust { } /// -pub mod resource { - /// Permissions related to resources at _locations_, like configuration files, executables or destinations for operations. - /// - /// Note that typically the permission refers to the place where the _location_ is configured, not to the _location_ itself. - /// For example, we may trust an owned configuration file, and by relation all the _locations_ inside of it even though - /// these are not owned by us. - pub enum Permission { - /// The greatest permission level without any restrictions, all _locations_ are permitted. - /// - /// For _locations_ to executables, it can be found in the `PATH` or configured from any git config file. +pub mod permission { + use crate::{Access, Permission}; + + /// A marker trait to signal tags for permissions. + pub trait Tag {} + + /// A tag indicating that a permission is applying to the contents of a configuration file. + pub struct Config; + impl Tag for Config {} + + /// A tag indicating that a permission is applying to the resource itself. + pub struct File; + impl Tag for File {} + + impl Access { + /// Create a permission for values contained in git configuration files. /// - /// Note that, however, some executables still won't be picked up from repository-local configuration - /// for safety reasons. - Allow, - /// For _locations_ to executables, only run these if these have been configured by git config files - /// that are owned by the user executing the application, or if these are in the `PATH`. - /// Resources or write destinations adhere to the same rules. - AllowIfOwned { - /// If true, if a _location_ is not under user control, instead of failing, fallback to a configuration setting that - /// is or try to not fail by using suitable defaults. For executables this may mean to search for them in the `PATH` - /// or fall back to another configuration value from configuration files under user control. - allow_fallback: bool, - }, - /// Do not use any _location_ unless it's required for git to function by using defaults. + /// This applies permissions to values contained inside of these files. + pub fn config(permission: Permission) -> Self { + Access { + permission, + _data: Default::default(), + } + } + } + + impl Access { + /// Create a permission a file itself. /// - /// If such a resource is encountered, the operation may fail. - Deny { - /// If true, operations that would fail may proceed by ignoring the resource if possible or using - /// defaults that are deemed safe. - /// - /// For executables this means using those in the `PATH` only. - allow_fallback: bool, - }, + /// This applies permissions to a configuration file itself and whether it can be used at all. + pub fn resource(permission: Permission) -> Self { + Access { + permission, + _data: Default::default(), + } + } } } +/// A container to define tagged access permissions +pub struct Access { + /// The access permission itself. + pub permission: Permission, + _data: PhantomData, +} + +/// Permissions related to resources at _locations_, like configuration files, executables or destinations for operations. +/// +/// Note that typically the permission refers to the place where the _location_ is configured, not to the _location_ itself. +/// For example, we may trust an owned configuration file, and by relation all the _locations_ inside of it even though +/// these are not owned by us. The exact place where a permission applies is identified. +pub enum Permission { + /// The greatest permission level without any restrictions, all _locations_ are permitted. + /// + /// For _locations_ to executables, it can be found in the `PATH` or configured from any git config file. + /// + /// Note that, however, some executables still won't be picked up from repository-local configuration + /// for safety reasons. + Allow, + /// For _locations_ to executables, only run these if these have been configured by git config files + /// that are owned by the user executing the application, or if these are in the `PATH`. + /// Resources or write destinations adhere to the same rules. + AllowIfOwned { + /// If true, if a _location_ is not under user control, instead of failing, fallback to a configuration setting that + /// is or try to not fail by using suitable defaults. For executables this may mean to search for them in the `PATH` + /// or fall back to another configuration value from configuration files under user control. + allow_fallback: bool, + }, + /// Do not use any _location_ unless it's required for git to function by using defaults. + /// + /// If such a resource is encountered, the operation may fail. + Deny { + /// If true, operations that would fail may proceed by ignoring the resource if possible or using + /// defaults that are deemed safe. + /// + /// For executables this means using those in the `PATH` only. + allow_fallback: bool, + }, +} + /// Various types to identify entities. pub mod identity; From 9c4516d309fef0c6fa5396e2bc366475182e0690 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 16 Apr 2022 16:58:50 +0800 Subject: [PATCH 27/34] don't assume repos with work-trees are non-bare; make git-sec manadatory (#386) There is also the notion of a 'main' worktree, the one we usually get when cloning non-bare. And we can currently instantiate a Repository from a worktree that isn't the main one. --- git-repository/Cargo.toml | 4 ++-- git-repository/src/lib.rs | 3 +-- git-repository/src/open.rs | 5 +++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/git-repository/Cargo.toml b/git-repository/Cargo.toml index 59a46b6f034..c3865265e90 100644 --- a/git-repository/Cargo.toml +++ b/git-repository/Cargo.toml @@ -49,7 +49,7 @@ max-performance = ["git-features/parallel", "git-features/zlib-ng-compat", "git- local-time-support = ["git-actor/local-time-support"] ## Re-export stability tier 2 crates for convenience and make `Repository` struct fields with types from these crates publicly accessible. ## Doing so is less stable than the stability tier 1 that `git-repository` is a member of. -unstable = ["git-index", "git-worktree", "git-mailmap", "git-glob", "git-sec", "git-credentials"] +unstable = ["git-index", "git-worktree", "git-mailmap", "git-glob", "git-credentials"] ## Print debugging information about usage of object database caches, useful for tuning cache sizes. cache-efficiency-debug = ["git-features/cache-efficiency-debug"] @@ -60,6 +60,7 @@ git-ref = { version = "^0.12.1", path = "../git-ref" } git-tempfile = { version = "^2.0.0", path = "../git-tempfile" } git-lock = { version = "^2.0.0", path = "../git-lock" } git-validate = { version ="^0.5.3", path = "../git-validate" } +git-sec = { version = "^0.1.0", path = "../git-sec" } git-config = { version = "^0.2.1", path = "../git-config" } git-odb = { version = "^0.28.0", path = "../git-odb" } @@ -79,7 +80,6 @@ git-features = { version = "^0.20.0", path = "../git-features", features = ["pro # unstable only git-glob = { version = "^0.2.0", path = "../git-glob", optional = true } -git-sec = { version = "^0.1.0", path = "../git-sec", optional = true } git-credentials = { version = "^0.1.0", path = "../git-credentials", optional = true } git-index = { version = "^0.2.0", path = "../git-index", optional = true } git-worktree = { version = "^0.1.0", path = "../git-worktree", optional = true } diff --git a/git-repository/src/lib.rs b/git-repository/src/lib.rs index 989e04b0220..f494ae4e754 100644 --- a/git-repository/src/lib.rs +++ b/git-repository/src/lib.rs @@ -146,7 +146,6 @@ pub use git_odb as odb; pub use git_protocol as protocol; pub use git_ref as refs; pub use git_revision as revision; -#[cfg(all(feature = "unstable", feature = "git-sec"))] pub use git_sec as sec; #[cfg(feature = "unstable")] pub use git_tempfile as tempfile; @@ -185,7 +184,7 @@ pub(crate) type Config = OwnShared>; /// A repository path which either points to a work tree or the `.git` repository itself. #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub enum Path { - /// The currently checked out or nascent work tree of a git repositore + /// The currently checked out or nascent work tree of a git repository WorkTree(PathBuf), /// The git repository itself Repository(PathBuf), diff --git a/git-repository/src/open.rs b/git-repository/src/open.rs index f52b0aeafed..f7f5d391b6f 100644 --- a/git-repository/src/open.rs +++ b/git-repository/src/open.rs @@ -125,13 +125,14 @@ impl crate::ThreadSafeRepository { replacement_objects, }: Options, ) -> Result { - let mut config = crate::config::Cache::new(&git_dir)?; + let config = crate::config::Cache::new(&git_dir)?; match worktree_dir { None if !config.is_bare => { worktree_dir = Some(git_dir.parent().expect("parent is always available").to_owned()); } Some(_) => { - config.is_bare = false; + // note that we might be bare even with a worktree directory - work trees don't have to be + // the parent of a non-bare repository. } None => {} } From 67d58372a8352da0197ec2992f120bd000ffe5de Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 16 Apr 2022 21:28:16 +0800 Subject: [PATCH 28/34] A first PoC to show how the permissions model works in practice (#386) Hence, when opening a repository permissions are already derived from Trust, and next up is to figure out how to get there so that open/discover can decide what to do while being configurable in full. --- git-repository/src/lib.rs | 1 + git-repository/src/open.rs | 19 +++++++++ git-repository/src/repository/mod.rs | 63 +++++++++++++++++++++++++++ git-sec/src/lib.rs | 64 ++++++++++++++++++++++------ 4 files changed, 134 insertions(+), 13 deletions(-) diff --git a/git-repository/src/lib.rs b/git-repository/src/lib.rs index f494ae4e754..971d34d0f99 100644 --- a/git-repository/src/lib.rs +++ b/git-repository/src/lib.rs @@ -200,6 +200,7 @@ pub mod id; pub mod object; pub mod reference; mod repository; +pub use repository::{permissions, permissions::Permissions}; pub mod tag; /// The kind of `Repository` diff --git a/git-repository/src/open.rs b/git-repository/src/open.rs index f7f5d391b6f..1247a566468 100644 --- a/git-repository/src/open.rs +++ b/git-repository/src/open.rs @@ -61,6 +61,7 @@ impl ReplacementObjects { pub struct Options { object_store_slots: git_odb::store::init::Slots, replacement_objects: ReplacementObjects, + permissions: crate::Permissions, } impl Options { @@ -78,6 +79,13 @@ impl Options { self } + // TODO: tests + /// Set the given permissions, which are typically derived by a `Trust` level. + pub fn permissions(mut self, permissions: crate::Permissions) -> Self { + self.permissions = permissions; + self + } + /// Open a repository at `path` with the options set so far. pub fn open(self, path: impl Into) -> Result { crate::ThreadSafeRepository::open_opts(path, self) @@ -94,6 +102,8 @@ pub enum Error { NotARepository(#[from] crate::path::is::Error), #[error(transparent)] ObjectStoreInitialization(#[from] std::io::Error), + #[error("The git directory at '{}' is considered unsafe as it's not owned by the current user.", .path.display())] + UnsafeGitDir { path: std::path::PathBuf }, } impl crate::ThreadSafeRepository { @@ -123,8 +133,17 @@ impl crate::ThreadSafeRepository { Options { object_store_slots, replacement_objects, + permissions, }: Options, ) -> Result { + if !permissions.git_dir.read_write { + // TODO: respect `save.directory`, which needs more support from git-config to do properly. + return Err(Error::UnsafeGitDir { path: git_dir }); + } + // TODO: assure we handle the worktree-dir properly as we can have config per worktree with an extension. + // This would be something read in later as have to first check for extensions. Also this means + // that each worktree, even if accessible through this instance, has to come in its own Repository instance + // as it may have its own configuration. That's fine actually. let config = crate::config::Cache::new(&git_dir)?; match worktree_dir { None if !config.is_bare => { diff --git a/git-repository/src/repository/mod.rs b/git-repository/src/repository/mod.rs index 532b008fa93..1a51f629311 100644 --- a/git-repository/src/repository/mod.rs +++ b/git-repository/src/repository/mod.rs @@ -39,6 +39,69 @@ impl crate::Repository { } } +/// Various permissions for parts of git repositories. +pub mod permissions { + use git_sec::permission::Resource; + use git_sec::{Access, Trust}; + + /// Permissions for the `.git` directory containing the repository. + pub struct GitDir { + /// If set, we can both read and write a git repository's git dir and by relation, its work tree(s). + /// Otherwise, the git-dir will be rejected and any open operation fails. + pub read_write: bool, + } + + /// Permissions associated with various resources of a git repository + pub struct Permissions { + /// Control how a git-dir can be used. + pub git_dir: Access, + } + + impl Permissions { + /// Return permissions similar to what git does when the repository isn't owned by the current user, + /// thus refusing all operations in it. + pub fn strict() -> Self { + Permissions { + git_dir: Access::resource(GitDir { read_write: false }), + } + } + + /// Return permissions that will not include configuration files not owned by the current user, + /// but trust system and global configuration files along with those which are owned by the current user. + /// + /// This allows to read and write repositories even if they aren't owned by the current user, but avoid using + /// anything else that could cause us to write into unknown locations or use programs beyond our `PATH`. + pub fn secure() -> Self { + Permissions { + git_dir: Access::resource(GitDir { read_write: true }), + } + } + + /// Everything is allowed with this set of permissions, thus we read all configuration and do what git typically + /// does with owned repositories. + pub fn all() -> Self { + Permissions { + git_dir: Access::resource(GitDir { read_write: true }), + } + } + } + + impl git_sec::trust::DefaultForLevel for Permissions { + fn default_for_level(level: Trust) -> Self { + match level { + Trust::Full => Permissions::all(), + Trust::Reduced => Permissions::secure(), + } + } + } + + impl Default for Permissions { + fn default() -> Self { + Permissions::secure() + } + } +} + mod init { use std::cell::RefCell; diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index 6a37b5de3e5..edb642f98ec 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -2,9 +2,11 @@ //! A shared trust model for `gitoxide` crates. use std::marker::PhantomData; +use std::ops::Deref; use std::path::Path; /// A way to specify how 'safe' we feel about a resource, typically about a git repository. +#[derive(Copy, Clone, Ord, PartialOrd, PartialEq, Eq, Debug)] pub enum Trust { /// We have no doubts that this resource means no harm and it can be used at will. Full, @@ -25,6 +27,12 @@ impl Trust { pub mod trust { use crate::Trust; + /// A trait to help creating default values based on a trust level. + pub trait DefaultForLevel { + /// Produce a default value for the given trust `level`. + fn default_for_level(level: Trust) -> Self; + } + /// Associate instructions for how to deal with various `Trust` levels as they are encountered in the wild. pub struct Mapping { /// The value for fully trusted resources. @@ -33,20 +41,40 @@ pub mod trust { pub reduced: T, } + impl Default for Mapping + where + T: DefaultForLevel, + { + fn default() -> Self { + Mapping { + full: T::default_for_level(Trust::Full), + reduced: T::default_for_level(Trust::Reduced), + } + } + } + impl Mapping { /// Obtain the value for the given trust `level`. - pub fn by_trust(&self, level: &Trust) -> &T { + pub fn by_trust(&self, level: Trust) -> &T { match level { Trust::Full => &self.full, Trust::Reduced => &self.reduced, } } + + /// Obtain the contained permission for the given `level` once. + pub fn into_permission(self, level: Trust) -> T { + match level { + Trust::Full => self.full, + Trust::Reduced => self.reduced, + } + } } } /// pub mod permission { - use crate::{Access, Permission}; + use crate::Access; /// A marker trait to signal tags for permissions. pub trait Tag {} @@ -56,14 +84,14 @@ pub mod permission { impl Tag for Config {} /// A tag indicating that a permission is applying to the resource itself. - pub struct File; - impl Tag for File {} + pub struct Resource; + impl Tag for Resource {} - impl Access { + impl

Access { /// Create a permission for values contained in git configuration files. /// /// This applies permissions to values contained inside of these files. - pub fn config(permission: Permission) -> Self { + pub fn config(permission: P) -> Self { Access { permission, _data: Default::default(), @@ -71,11 +99,12 @@ pub mod permission { } } - impl Access { - /// Create a permission a file itself. + impl

Access { + /// Create a permission a file or directory itself. /// - /// This applies permissions to a configuration file itself and whether it can be used at all. - pub fn resource(permission: Permission) -> Self { + /// This applies permissions to a configuration file itself and whether it can be used at all, or to a directory + /// to read from or write to. + pub fn resource(permission: P) -> Self { Access { permission, _data: Default::default(), @@ -84,13 +113,22 @@ pub mod permission { } } -/// A container to define tagged access permissions -pub struct Access { +/// A container to define tagged access permissions, rendering the permission read-only. +pub struct Access { /// The access permission itself. - pub permission: Permission, + permission: P, _data: PhantomData, } +impl Deref for Access { + type Target = P; + + fn deref(&self) -> &Self::Target { + &self.permission + } +} + +// TODO: this probably belongs to `git-config` and can be simplified then as AllowIfOwned and Deny can probably be merged. /// Permissions related to resources at _locations_, like configuration files, executables or destinations for operations. /// /// Note that typically the permission refers to the place where the _location_ is configured, not to the _location_ itself. From b1d319b249fb6c6d4d5197734938836824789053 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 17 Apr 2022 17:18:01 +0800 Subject: [PATCH 29/34] more expressive and fuiture-proof handling of git dir access controls (#386) --- Cargo.lock | 1 + git-repository/src/open.rs | 2 +- git-repository/src/repository/mod.rs | 17 ++++++----------- git-sec/Cargo.toml | 1 + git-sec/src/lib.rs | 11 +++++++++++ 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 062f6a50363..13715aca3db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1459,6 +1459,7 @@ dependencies = [ name = "git-sec" version = "0.1.0" dependencies = [ + "bitflags", "libc", "serde", "tempfile", diff --git a/git-repository/src/open.rs b/git-repository/src/open.rs index 1247a566468..ce8934f010e 100644 --- a/git-repository/src/open.rs +++ b/git-repository/src/open.rs @@ -136,7 +136,7 @@ impl crate::ThreadSafeRepository { permissions, }: Options, ) -> Result { - if !permissions.git_dir.read_write { + if *permissions.git_dir != git_sec::ReadWrite::all() { // TODO: respect `save.directory`, which needs more support from git-config to do properly. return Err(Error::UnsafeGitDir { path: git_dir }); } diff --git a/git-repository/src/repository/mod.rs b/git-repository/src/repository/mod.rs index 1a51f629311..15dca027d23 100644 --- a/git-repository/src/repository/mod.rs +++ b/git-repository/src/repository/mod.rs @@ -44,17 +44,12 @@ pub mod permissions { use git_sec::permission::Resource; use git_sec::{Access, Trust}; - /// Permissions for the `.git` directory containing the repository. - pub struct GitDir { - /// If set, we can both read and write a git repository's git dir and by relation, its work tree(s). - /// Otherwise, the git-dir will be rejected and any open operation fails. - pub read_write: bool, - } - /// Permissions associated with various resources of a git repository pub struct Permissions { /// Control how a git-dir can be used. - pub git_dir: Access, + /// + /// Note that a repository won't be usable at all unless read and write permissions are given. + pub git_dir: Access, } impl Permissions { @@ -62,7 +57,7 @@ pub mod permissions { /// thus refusing all operations in it. pub fn strict() -> Self { Permissions { - git_dir: Access::resource(GitDir { read_write: false }), + git_dir: Access::resource(git_sec::ReadWrite::empty()), } } @@ -73,7 +68,7 @@ pub mod permissions { /// anything else that could cause us to write into unknown locations or use programs beyond our `PATH`. pub fn secure() -> Self { Permissions { - git_dir: Access::resource(GitDir { read_write: true }), + git_dir: Access::resource(git_sec::ReadWrite::all()), } } @@ -81,7 +76,7 @@ pub mod permissions { /// does with owned repositories. pub fn all() -> Self { Permissions { - git_dir: Access::resource(GitDir { read_write: true }), + git_dir: Access::resource(git_sec::ReadWrite::all()), } } } diff --git a/git-sec/Cargo.toml b/git-sec/Cargo.toml index f5fe0d9e702..7a40bcb7b82 100644 --- a/git-sec/Cargo.toml +++ b/git-sec/Cargo.toml @@ -18,6 +18,7 @@ serde1 = [ "serde" ] [dependencies] serde = { version = "1.0.114", optional = true, default-features = false, features = ["std", "derive"] } +bitflags = "1.3.2" [target.'cfg(not(windows))'.dependencies] libc = "0.2.123" diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index edb642f98ec..013b8b8b2a3 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -113,6 +113,17 @@ pub mod permission { } } +bitflags::bitflags! { + /// Whether something can be read or written. + #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] + pub struct ReadWrite: u8 { + /// The item can be read. + const READ = 1 << 0; + /// The item can be written + const WRITE = 1 << 1; + } +} + /// A container to define tagged access permissions, rendering the permission read-only. pub struct Access { /// The access permission itself. From 2e39b0ede98826e6f85c56fef77ac65a5b7e7ac2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 17 Apr 2022 17:28:33 +0800 Subject: [PATCH 30/34] change!: `path::discover::existing()` -> `path::discover()` (#386) --- git-repository/src/lib.rs | 6 +- git-repository/src/path/discover.rs | 179 +++++++++++++-------------- git-repository/src/path/is.rs | 1 - git-repository/src/path/mod.rs | 3 + git-repository/tests/discover/mod.rs | 14 +-- 5 files changed, 102 insertions(+), 101 deletions(-) diff --git a/git-repository/src/lib.rs b/git-repository/src/lib.rs index 971d34d0f99..c89c3b6c74c 100644 --- a/git-repository/src/lib.rs +++ b/git-repository/src/lib.rs @@ -311,14 +311,14 @@ pub mod init { pub mod discover { use std::{convert::TryInto, path::Path}; - use crate::path::discover; + use crate::path; /// The error returned by [`crate::discover()`]. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { #[error(transparent)] - Discover(#[from] discover::existing::Error), + Discover(#[from] path::discover::Error), #[error(transparent)] Open(#[from] crate::open::Error), } @@ -326,7 +326,7 @@ pub mod discover { impl crate::ThreadSafeRepository { /// Try to open a git repository in `directory` and search upwards through its parents until one is found. pub fn discover(directory: impl AsRef) -> Result { - let path = discover::existing(directory)?; + let path = path::discover(directory)?; Ok(path.try_into()?) } } diff --git a/git-repository/src/path/discover.rs b/git-repository/src/path/discover.rs index 0b4e1cff310..1cd5b426db3 100644 --- a/git-repository/src/path/discover.rs +++ b/git-repository/src/path/discover.rs @@ -1,107 +1,106 @@ -//! -use std::{ - borrow::Cow, - path::{Component, Path}, -}; +use std::path::PathBuf; -use crate::path; - -/// -pub mod existing { - use std::path::PathBuf; - - /// The error returned by [path::discover::existing()][super::existing()]. - #[derive(Debug, thiserror::Error)] - #[allow(missing_docs)] - pub enum Error { - #[error("Failed to access a directory, or path is not a directory: '{}'", .path.display())] - InaccessibleDirectory { path: PathBuf }, - #[error("Could find a git repository in '{}' or in any of its parents", .path.display())] - NoGitRepository { path: PathBuf }, - } +/// The error returned by [path::discover::existing()][super::existing()]. +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +pub enum Error { + #[error("Failed to access a directory, or path is not a directory: '{}'", .path.display())] + InaccessibleDirectory { path: PathBuf }, + #[error("Could find a git repository in '{}' or in any of its parents", .path.display())] + NoGitRepository { path: PathBuf }, } -/// Find the location of the git repository directly in `directory` or in any of its parent directories. -/// -/// Fail if no valid-looking git repository could be found. -pub fn existing(directory: impl AsRef) -> Result { - // Canonicalize the path so that `Path::parent` _actually_ gives - // us the parent directory. (`Path::parent` just strips off the last - // path component, which means it will not do what you expect when - // working with paths paths that contain '..'.) - let directory = maybe_canonicalize(directory.as_ref()).map_err(|_| existing::Error::InaccessibleDirectory { - path: directory.as_ref().into(), - })?; - if !directory.is_dir() { - return Err(existing::Error::InaccessibleDirectory { - path: directory.into_owned(), - }); - } +pub(crate) mod function { + use super::Error; + use std::{ + borrow::Cow, + path::{Component, Path}, + }; - let mut cursor: &Path = &directory; - loop { - if let Ok(kind) = path::is::git(cursor) { - break Ok(crate::Path::from_dot_git_dir(cursor, kind)); - } - let git_dir = cursor.join(".git"); - if let Ok(kind) = path::is::git(&git_dir) { - break Ok(crate::Path::from_dot_git_dir(git_dir, kind)); + use crate::path; + + /// Find the location of the git repository directly in `directory` or in any of its parent directories. + /// + /// Fail if no valid-looking git repository could be found. + pub fn discover(directory: impl AsRef) -> Result { + // Canonicalize the path so that `Path::parent` _actually_ gives + // us the parent directory. (`Path::parent` just strips off the last + // path component, which means it will not do what you expect when + // working with paths paths that contain '..'.) + let directory = maybe_canonicalize(directory.as_ref()).map_err(|_| Error::InaccessibleDirectory { + path: directory.as_ref().into(), + })?; + if !directory.is_dir() { + return Err(Error::InaccessibleDirectory { + path: directory.into_owned(), + }); } - match cursor.parent() { - Some(parent) => cursor = parent, - None => { - break Err(existing::Error::NoGitRepository { - path: directory.into_owned(), - }) + + let mut cursor: &Path = &directory; + loop { + if let Ok(kind) = path::is::git(cursor) { + break Ok(crate::Path::from_dot_git_dir(cursor, kind)); + } + let git_dir = cursor.join(".git"); + if let Ok(kind) = path::is::git(&git_dir) { + break Ok(crate::Path::from_dot_git_dir(git_dir, kind)); + } + match cursor.parent() { + Some(parent) => cursor = parent, + None => { + break Err(Error::NoGitRepository { + path: directory.into_owned(), + }) + } } } } -} -fn maybe_canonicalize(path: &Path) -> std::io::Result> { - let ends_with_relative_component = path - .components() - .last() - .map_or(true, |c| matches!(c, Component::CurDir | Component::ParentDir)); - if ends_with_relative_component { - path.canonicalize().map(Into::into) - } else { - Ok(path.into()) + fn maybe_canonicalize(path: &Path) -> std::io::Result> { + let ends_with_relative_component = path + .components() + .last() + .map_or(true, |c| matches!(c, Component::CurDir | Component::ParentDir)); + if ends_with_relative_component { + path.canonicalize().map(Into::into) + } else { + Ok(path.into()) + } } -} -#[cfg(test)] -mod maybe_canonicalize { - use super::*; + #[cfg(test)] + mod maybe_canonicalize { + use super::*; - fn relative_component_count(path: impl AsRef) -> usize { - path.as_ref() - .components() - .filter(|c| matches!(c, Component::CurDir | Component::ParentDir)) - .count() - } + fn relative_component_count(path: impl AsRef) -> usize { + path.as_ref() + .components() + .filter(|c| matches!(c, Component::CurDir | Component::ParentDir)) + .count() + } - #[test] - fn empty_paths_are_invalid() { - assert!( - maybe_canonicalize(Path::new("")).is_err(), - "empty paths are not equivalent to '.' but are non-existing" - ); - } + #[test] + fn empty_paths_are_invalid() { + assert!( + maybe_canonicalize(Path::new("")).is_err(), + "empty paths are not equivalent to '.' but are non-existing" + ); + } - #[test] - fn paths_starting_with_dot_but_end_with_normal_path_are_not_canonicalized() { - assert_eq!( - relative_component_count(maybe_canonicalize(&Path::new(".").join("hello")).unwrap()), - 1, - ); - } + #[test] + fn paths_starting_with_dot_but_end_with_normal_path_are_not_canonicalized() { + assert_eq!( + relative_component_count(maybe_canonicalize(&Path::new(".").join("hello")).unwrap()), + 1, + ); + } - #[test] - fn paths_ending_with_non_normal_component_are_canonicalized() { - assert_eq!( - relative_component_count(maybe_canonicalize(&Path::new(".").join(".")).unwrap()), - 0, - ); + #[test] + fn paths_ending_with_non_normal_component_are_canonicalized() { + assert_eq!( + relative_component_count(maybe_canonicalize(&Path::new(".").join(".")).unwrap()), + 0, + ); + } } } diff --git a/git-repository/src/path/is.rs b/git-repository/src/path/is.rs index 312352e37b7..e6570f4d6d3 100644 --- a/git-repository/src/path/is.rs +++ b/git-repository/src/path/is.rs @@ -1,4 +1,3 @@ -//! use std::{ ffi::OsStr, path::{Path, PathBuf}, diff --git a/git-repository/src/path/mod.rs b/git-repository/src/path/mod.rs index e618c68c457..429d1ed068b 100644 --- a/git-repository/src/path/mod.rs +++ b/git-repository/src/path/mod.rs @@ -4,7 +4,10 @@ use crate::{Kind, Path}; /// pub mod create; +/// pub mod discover; +pub use discover::function::discover; +/// pub mod is; impl AsRef for Path { diff --git a/git-repository/tests/discover/mod.rs b/git-repository/tests/discover/mod.rs index 8f434f8f6fd..ecf581912dd 100644 --- a/git-repository/tests/discover/mod.rs +++ b/git-repository/tests/discover/mod.rs @@ -6,7 +6,7 @@ mod existing { #[test] fn from_bare_git_dir() -> crate::Result { let dir = repo_path()?.join("bare.git"); - let path = git_repository::path::discover::existing(&dir)?; + let path = git_repository::path::discover(&dir)?; assert_eq!(path.as_ref(), dir, "the bare .git dir is directly returned"); assert_eq!(path.kind(), Kind::Bare); Ok(()) @@ -16,7 +16,7 @@ mod existing { fn from_inside_bare_git_dir() -> crate::Result { let git_dir = repo_path()?.join("bare.git"); let dir = git_dir.join("objects"); - let path = git_repository::path::discover::existing(&dir)?; + let path = git_repository::path::discover(&dir)?; assert_eq!( path.as_ref(), git_dir, @@ -29,7 +29,7 @@ mod existing { #[test] fn from_git_dir() -> crate::Result { let dir = repo_path()?.join(".git"); - let path = git_repository::path::discover::existing(&dir)?; + let path = git_repository::path::discover(&dir)?; assert_eq!(path.kind(), Kind::WorkTree); assert_eq!( path.into_repository_and_work_tree_directories().0, @@ -42,7 +42,7 @@ mod existing { #[test] fn from_working_dir() -> crate::Result { let dir = repo_path()?; - let path = git_repository::path::discover::existing(&dir)?; + let path = git_repository::path::discover(&dir)?; assert_eq!(path.as_ref(), dir, "a working tree dir yields the git dir"); assert_eq!(path.kind(), Kind::WorkTree); Ok(()) @@ -52,7 +52,7 @@ mod existing { fn from_nested_dir() -> crate::Result { let working_dir = repo_path()?; let dir = working_dir.join("some/very/deeply/nested/subdir"); - let path = git_repository::path::discover::existing(&dir)?; + let path = git_repository::path::discover(&dir)?; assert_eq!(path.kind(), Kind::WorkTree); assert_eq!(path.as_ref(), working_dir, "a working tree dir yields the git dir"); Ok(()) @@ -67,7 +67,7 @@ mod existing { // exploring ancestors.) let working_dir = repo_path()?; let dir = working_dir.join("some/very/deeply/nested/subdir/../../../../../../.."); - let path = git_repository::path::discover::existing(&dir)?; + let path = git_repository::path::discover(&dir)?; assert_eq!(path.kind(), Kind::WorkTree); assert_eq!( path.as_ref() @@ -89,7 +89,7 @@ mod existing { fn from_nested_dir_inside_a_git_dir() -> crate::Result { let working_dir = repo_path()?; let dir = working_dir.join(".git").join("objects"); - let path = git_repository::path::discover::existing(&dir)?; + let path = git_repository::path::discover(&dir)?; assert_eq!(path.kind(), Kind::WorkTree); assert_eq!(path.as_ref(), working_dir, "we find .git directories on the way"); Ok(()) From b9e307bc9aea52459450c22f398f078f81aeb825 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 17 Apr 2022 19:53:23 +0800 Subject: [PATCH 31/34] refactor (#386) --- git-sec/src/lib.rs | 29 ++++++++++++++--------------- git-sec/tests/sec.rs | 9 +++++++++ 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index 013b8b8b2a3..26a6c549938 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -3,30 +3,29 @@ use std::marker::PhantomData; use std::ops::Deref; -use std::path::Path; /// A way to specify how 'safe' we feel about a resource, typically about a git repository. #[derive(Copy, Clone, Ord, PartialOrd, PartialEq, Eq, Debug)] pub enum Trust { - /// We have no doubts that this resource means no harm and it can be used at will. - Full, /// Caution is warranted when using the resource. Reduced, -} - -impl Trust { - /// Derive `Full` trust if `path` is owned by the user executing the current process, or `Reduced` trust otherwise. - pub fn from_path_ownership(path: impl AsRef) -> std::io::Result { - Ok(identity::is_path_owned_by_current_user(path.as_ref())? - .then(|| Trust::Full) - .unwrap_or(Trust::Reduced)) - } + /// We have no doubts that this resource means no harm and it can be used at will. + Full, } /// pub mod trust { use crate::Trust; + impl Trust { + /// Derive `Full` trust if `path` is owned by the user executing the current process, or `Reduced` trust otherwise. + pub fn from_path_ownership(path: impl AsRef) -> std::io::Result { + Ok(crate::identity::is_path_owned_by_current_user(path.as_ref())? + .then(|| Trust::Full) + .unwrap_or(Trust::Reduced)) + } + } + /// A trait to help creating default values based on a trust level. pub trait DefaultForLevel { /// Produce a default value for the given trust `level`. @@ -55,15 +54,15 @@ pub mod trust { impl Mapping { /// Obtain the value for the given trust `level`. - pub fn by_trust(&self, level: Trust) -> &T { + pub fn by_level(&self, level: Trust) -> &T { match level { Trust::Full => &self.full, Trust::Reduced => &self.reduced, } } - /// Obtain the contained permission for the given `level` once. - pub fn into_permission(self, level: Trust) -> T { + /// Obtain the value for the given `level` once. + pub fn into_value_by_level(self, level: Trust) -> T { match level { Trust::Full => self.full, Trust::Reduced => self.reduced, diff --git a/git-sec/tests/sec.rs b/git-sec/tests/sec.rs index 82f9aca8ce9..245e02b5e4a 100644 --- a/git-sec/tests/sec.rs +++ b/git-sec/tests/sec.rs @@ -1,3 +1,12 @@ pub type Result = std::result::Result>; +mod trust { + use git_sec::Trust; + + #[test] + fn ordering() { + assert!(Trust::Reduced < Trust::Full); + } +} + mod identity; From 80e8fd4a5944890f43f3d888b7a73bb26351b195 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 17 Apr 2022 19:53:38 +0800 Subject: [PATCH 32/34] change!: integrate trust model into repository discovery (#386) That way it's possible to ignore repositories which effectively aren't owned by the current user, or to not ignore them (default) but assign tigher permissions to the repository. --- git-repository/src/lib.rs | 35 ++++++-- git-repository/src/open.rs | 24 +++++- git-repository/src/path/discover.rs | 85 +++++++++++++++++--- git-repository/src/path/mod.rs | 2 +- git-repository/src/repository/thread_safe.rs | 15 ---- git-repository/tests/discover/mod.rs | 21 +++-- 6 files changed, 137 insertions(+), 45 deletions(-) diff --git a/git-repository/src/lib.rs b/git-repository/src/lib.rs index c89c3b6c74c..7303cd73e19 100644 --- a/git-repository/src/lib.rs +++ b/git-repository/src/lib.rs @@ -283,7 +283,8 @@ pub mod rev_parse { /// pub mod init { - use std::{convert::TryInto, path::Path}; + use crate::ThreadSafeRepository; + use std::path::Path; /// The error returned by [`crate::init()`]. #[derive(Debug, thiserror::Error)] @@ -301,17 +302,23 @@ pub mod init { /// Fails without action if there is already a `.git` repository inside of `directory`, but /// won't mind if the `directory` otherwise is non-empty. pub fn init(directory: impl AsRef, kind: crate::Kind) -> Result { + use git_sec::trust::DefaultForLevel; let path = crate::path::create::into(directory.as_ref(), kind)?; - Ok(path.try_into()?) + let (git_dir, worktree_dir) = path.into_repository_and_work_tree_directories(); + ThreadSafeRepository::open_from_paths( + git_dir, + worktree_dir, + crate::open::Options::default_for_level(git_sec::Trust::Full), + ) + .map_err(Into::into) } } } /// pub mod discover { - use std::{convert::TryInto, path::Path}; - - use crate::path; + use crate::{path, ThreadSafeRepository}; + use std::path::Path; /// The error returned by [`crate::discover()`]. #[derive(Debug, thiserror::Error)] @@ -323,11 +330,23 @@ pub mod discover { Open(#[from] crate::open::Error), } - impl crate::ThreadSafeRepository { + impl ThreadSafeRepository { /// Try to open a git repository in `directory` and search upwards through its parents until one is found. pub fn discover(directory: impl AsRef) -> Result { - let path = path::discover(directory)?; - Ok(path.try_into()?) + Self::discover_opts(directory, Default::default(), Default::default()) + } + + /// Try to open a git repository in `directory` and search upwards through its parents until one is found, + /// while applying `options`. Then use the `trust_map` to determine which of our own repository options to use + /// for instantiations. + pub fn discover_opts( + directory: impl AsRef, + options: crate::path::discover::Options, + trust_map: git_sec::trust::Mapping, + ) -> Result { + let (path, trust) = path::discover_opts(directory, options)?; + let (git_dir, worktree_dir) = path.into_repository_and_work_tree_directories(); + Self::open_from_paths(git_dir, worktree_dir, trust_map.into_value_by_level(trust)).map_err(Into::into) } } } diff --git a/git-repository/src/open.rs b/git-repository/src/open.rs index ce8934f010e..0312fe30814 100644 --- a/git-repository/src/open.rs +++ b/git-repository/src/open.rs @@ -1,6 +1,8 @@ use std::path::PathBuf; +use crate::Permissions; use git_features::threading::OwnShared; +use git_sec::Trust; /// A way to configure the usage of replacement objects, see `git replace`. pub enum ReplacementObjects { @@ -92,6 +94,23 @@ impl Options { } } +impl git_sec::trust::DefaultForLevel for Options { + fn default_for_level(level: Trust) -> Self { + match level { + git_sec::Trust::Full => Options { + object_store_slots: Default::default(), + replacement_objects: Default::default(), + permissions: Permissions::all(), + }, + git_sec::Trust::Reduced => Options { + object_store_slots: git_odb::store::init::Slots::Given(32), // limit resource usage + replacement_objects: ReplacementObjects::Disable, // don't be tricked into seeing manufactured objects + permissions: Default::default(), + }, + } + } +} + /// The error returned by [`crate::open()`]. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] @@ -112,8 +131,9 @@ impl crate::ThreadSafeRepository { Self::open_opts(path, Options::default()) } - /// Open a git repository at the given `path`, possibly expanding it to `path/.git` if `path` is a work tree dir. - fn open_opts(path: impl Into, options: Options) -> Result { + /// Open a git repository at the given `path`, possibly expanding it to `path/.git` if `path` is a work tree dir, and use + /// `options` for fine-grained control. + pub fn open_opts(path: impl Into, options: Options) -> Result { let path = path.into(); let (path, kind) = match crate::path::is::git(&path) { Ok(kind) => (path, kind), diff --git a/git-repository/src/path/discover.rs b/git-repository/src/path/discover.rs index 1cd5b426db3..1de4b2da8ed 100644 --- a/git-repository/src/path/discover.rs +++ b/git-repository/src/path/discover.rs @@ -8,10 +8,35 @@ pub enum Error { InaccessibleDirectory { path: PathBuf }, #[error("Could find a git repository in '{}' or in any of its parents", .path.display())] NoGitRepository { path: PathBuf }, + #[error("Could not determine trust level for path '{}'.", .path.display())] + CheckTrust { + path: PathBuf, + #[source] + err: std::io::Error, + }, +} + +/// Options to help guide the [discovery][function::discover()] of repositories, along with their options +/// when instantiated. +pub struct Options { + /// When discovering a repository, assure it has at least this trust level or ignore it otherwise. + /// + /// This defaults to [`Reduced`][git_sec::Trust::Reduced] as our default settings are geared towards avoiding abuse. + /// Set it to `Full` to only see repositories that [are owned by the current user][git_sec::Trust::from_path_ownership()]. + pub required_trust: git_sec::Trust, +} + +impl Default for Options { + fn default() -> Self { + Options { + required_trust: git_sec::Trust::Reduced, + } + } } pub(crate) mod function { - use super::Error; + use super::{Error, Options}; + use git_sec::Trust; use std::{ borrow::Cow, path::{Component, Path}, @@ -19,10 +44,15 @@ pub(crate) mod function { use crate::path; - /// Find the location of the git repository directly in `directory` or in any of its parent directories. + /// Find the location of the git repository directly in `directory` or in any of its parent directories and provide + /// an associated Trust level by looking at the git directory's ownership, and control discovery using `options`. /// /// Fail if no valid-looking git repository could be found. - pub fn discover(directory: impl AsRef) -> Result { + // TODO: tests for trust-based discovery + pub fn discover_opts( + directory: impl AsRef, + Options { required_trust }: Options, + ) -> Result<(crate::Path, git_sec::Trust), Error> { // Canonicalize the path so that `Path::parent` _actually_ gives // us the parent directory. (`Path::parent` just strips off the last // path component, which means it will not do what you expect when @@ -36,17 +66,40 @@ pub(crate) mod function { }); } - let mut cursor: &Path = &directory; - loop { - if let Ok(kind) = path::is::git(cursor) { - break Ok(crate::Path::from_dot_git_dir(cursor, kind)); - } - let git_dir = cursor.join(".git"); - if let Ok(kind) = path::is::git(&git_dir) { - break Ok(crate::Path::from_dot_git_dir(git_dir, kind)); + let filter_by_trust = + |x: &std::path::Path, kind: crate::path::Kind| -> Result, Error> { + let trust = + git_sec::Trust::from_path_ownership(x).map_err(|err| Error::CheckTrust { path: x.into(), err })?; + Ok((trust >= required_trust).then(|| (crate::Path::from_dot_git_dir(x, kind), trust))) + }; + + let mut cursor = directory.clone(); + 'outer: loop { + for append_dot_git in &[false, true] { + if *append_dot_git { + cursor = cursor.into_owned().into(); + if let Cow::Owned(p) = &mut cursor { + p.push(".git"); + } + } + if let Ok(kind) = path::is::git(&cursor) { + match filter_by_trust(&cursor, kind)? { + Some(res) => break 'outer Ok(res), + None => { + break 'outer Err(Error::NoGitRepository { + path: directory.into_owned(), + }) + } + } + } + if *append_dot_git { + if let Cow::Owned(p) = &mut cursor { + p.pop(); + } + } } match cursor.parent() { - Some(parent) => cursor = parent, + Some(parent) => cursor = parent.to_owned().into(), None => { break Err(Error::NoGitRepository { path: directory.into_owned(), @@ -56,6 +109,14 @@ pub(crate) mod function { } } + /// Find the location of the git repository directly in `directory` or in any of its parent directories, and provide + /// the trust level derived from Path ownership. + /// + /// Fail if no valid-looking git repository could be found. + pub fn discover(directory: impl AsRef) -> Result<(crate::Path, Trust), Error> { + discover_opts(directory, Default::default()) + } + fn maybe_canonicalize(path: &Path) -> std::io::Result> { let ends_with_relative_component = path .components() diff --git a/git-repository/src/path/mod.rs b/git-repository/src/path/mod.rs index 429d1ed068b..bc0ab7ce30a 100644 --- a/git-repository/src/path/mod.rs +++ b/git-repository/src/path/mod.rs @@ -6,7 +6,7 @@ use crate::{Kind, Path}; pub mod create; /// pub mod discover; -pub use discover::function::discover; +pub use discover::function::{discover, discover_opts}; /// pub mod is; diff --git a/git-repository/src/repository/thread_safe.rs b/git-repository/src/repository/thread_safe.rs index 10bdaa7f47e..d206d54e87a 100644 --- a/git-repository/src/repository/thread_safe.rs +++ b/git-repository/src/repository/thread_safe.rs @@ -17,21 +17,6 @@ mod access { } } -mod from_path { - use std::convert::TryFrom; - - use crate::Path; - - impl TryFrom for crate::ThreadSafeRepository { - type Error = crate::open::Error; - - fn try_from(value: Path) -> Result { - let (git_dir, worktree_dir) = value.into_repository_and_work_tree_directories(); - crate::ThreadSafeRepository::open_from_paths(git_dir, worktree_dir, Default::default()) - } - } -} - mod location { impl crate::ThreadSafeRepository { diff --git a/git-repository/tests/discover/mod.rs b/git-repository/tests/discover/mod.rs index ecf581912dd..69735a7d222 100644 --- a/git-repository/tests/discover/mod.rs +++ b/git-repository/tests/discover/mod.rs @@ -6,9 +6,10 @@ mod existing { #[test] fn from_bare_git_dir() -> crate::Result { let dir = repo_path()?.join("bare.git"); - let path = git_repository::path::discover(&dir)?; + let (path, trust) = git_repository::path::discover(&dir)?; assert_eq!(path.as_ref(), dir, "the bare .git dir is directly returned"); assert_eq!(path.kind(), Kind::Bare); + assert_eq!(trust, git_sec::Trust::Full); Ok(()) } @@ -16,35 +17,38 @@ mod existing { fn from_inside_bare_git_dir() -> crate::Result { let git_dir = repo_path()?.join("bare.git"); let dir = git_dir.join("objects"); - let path = git_repository::path::discover(&dir)?; + let (path, trust) = git_repository::path::discover(&dir)?; assert_eq!( path.as_ref(), git_dir, "the bare .git dir is found while traversing upwards" ); assert_eq!(path.kind(), Kind::Bare); + assert_eq!(trust, git_sec::Trust::Full); Ok(()) } #[test] fn from_git_dir() -> crate::Result { let dir = repo_path()?.join(".git"); - let path = git_repository::path::discover(&dir)?; + let (path, trust) = git_repository::path::discover(&dir)?; assert_eq!(path.kind(), Kind::WorkTree); assert_eq!( path.into_repository_and_work_tree_directories().0, dir, "the .git dir is directly returned if valid" ); + assert_eq!(trust, git_sec::Trust::Full); Ok(()) } #[test] fn from_working_dir() -> crate::Result { let dir = repo_path()?; - let path = git_repository::path::discover(&dir)?; + let (path, trust) = git_repository::path::discover(&dir)?; assert_eq!(path.as_ref(), dir, "a working tree dir yields the git dir"); assert_eq!(path.kind(), Kind::WorkTree); + assert_eq!(trust, git_sec::Trust::Full); Ok(()) } @@ -52,9 +56,10 @@ mod existing { fn from_nested_dir() -> crate::Result { let working_dir = repo_path()?; let dir = working_dir.join("some/very/deeply/nested/subdir"); - let path = git_repository::path::discover(&dir)?; + let (path, trust) = git_repository::path::discover(&dir)?; assert_eq!(path.kind(), Kind::WorkTree); assert_eq!(path.as_ref(), working_dir, "a working tree dir yields the git dir"); + assert_eq!(trust, git_sec::Trust::Full); Ok(()) } @@ -67,7 +72,7 @@ mod existing { // exploring ancestors.) let working_dir = repo_path()?; let dir = working_dir.join("some/very/deeply/nested/subdir/../../../../../../.."); - let path = git_repository::path::discover(&dir)?; + let (path, trust) = git_repository::path::discover(&dir)?; assert_eq!(path.kind(), Kind::WorkTree); assert_eq!( path.as_ref() @@ -82,6 +87,7 @@ mod existing { working_dir.canonicalize()?, "a relative path that climbs above the test repo should yield the gitoxide repo" ); + assert_eq!(trust, git_sec::Trust::Full); Ok(()) } @@ -89,9 +95,10 @@ mod existing { fn from_nested_dir_inside_a_git_dir() -> crate::Result { let working_dir = repo_path()?; let dir = working_dir.join(".git").join("objects"); - let path = git_repository::path::discover(&dir)?; + let (path, trust) = git_repository::path::discover(&dir)?; assert_eq!(path.kind(), Kind::WorkTree); assert_eq!(path.as_ref(), working_dir, "we find .git directories on the way"); + assert_eq!(trust, git_sec::Trust::Full); Ok(()) } From f00f4a4a3a9149bf5cf925e931a8105aeb9b9db9 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 17 Apr 2022 20:39:43 +0800 Subject: [PATCH 33/34] change!: simplify `Permission` type radically `(#386) --- git-sec/src/lib.rs | 50 +++++++++++++--------------------------------- 1 file changed, 14 insertions(+), 36 deletions(-) diff --git a/git-sec/src/lib.rs b/git-sec/src/lib.rs index 26a6c549938..47d074bfd3f 100644 --- a/git-sec/src/lib.rs +++ b/git-sec/src/lib.rs @@ -5,7 +5,8 @@ use std::marker::PhantomData; use std::ops::Deref; /// A way to specify how 'safe' we feel about a resource, typically about a git repository. -#[derive(Copy, Clone, Ord, PartialOrd, PartialEq, Eq, Debug)] +#[derive(Copy, Clone, Ord, PartialOrd, PartialEq, Eq, Debug, Hash)] +#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] pub enum Trust { /// Caution is warranted when using the resource. Reduced, @@ -112,6 +113,18 @@ pub mod permission { } } +/// Allow, deny or forbid using a resource or performing an action. +#[derive(Debug, Copy, Clone, PartialOrd, PartialEq, Ord, Eq, Hash)] +#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] +pub enum Permission { + /// Fail outright when trying to load a resource or performing an action. + Forbid, + /// Ignore resources or try to avoid performing an operation. + Deny, + /// Allow loading a reasource or performing an action. + Allow, +} + bitflags::bitflags! { /// Whether something can be read or written. #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] @@ -138,40 +151,5 @@ impl Deref for Access { } } -// TODO: this probably belongs to `git-config` and can be simplified then as AllowIfOwned and Deny can probably be merged. -/// Permissions related to resources at _locations_, like configuration files, executables or destinations for operations. -/// -/// Note that typically the permission refers to the place where the _location_ is configured, not to the _location_ itself. -/// For example, we may trust an owned configuration file, and by relation all the _locations_ inside of it even though -/// these are not owned by us. The exact place where a permission applies is identified. -pub enum Permission { - /// The greatest permission level without any restrictions, all _locations_ are permitted. - /// - /// For _locations_ to executables, it can be found in the `PATH` or configured from any git config file. - /// - /// Note that, however, some executables still won't be picked up from repository-local configuration - /// for safety reasons. - Allow, - /// For _locations_ to executables, only run these if these have been configured by git config files - /// that are owned by the user executing the application, or if these are in the `PATH`. - /// Resources or write destinations adhere to the same rules. - AllowIfOwned { - /// If true, if a _location_ is not under user control, instead of failing, fallback to a configuration setting that - /// is or try to not fail by using suitable defaults. For executables this may mean to search for them in the `PATH` - /// or fall back to another configuration value from configuration files under user control. - allow_fallback: bool, - }, - /// Do not use any _location_ unless it's required for git to function by using defaults. - /// - /// If such a resource is encountered, the operation may fail. - Deny { - /// If true, operations that would fail may proceed by ignoring the resource if possible or using - /// defaults that are deemed safe. - /// - /// For executables this means using those in the `PATH` only. - allow_fallback: bool, - }, -} - /// Various types to identify entities. pub mod identity; From 8443330b051c109742fe55928e2afd36fc0172fd Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 17 Apr 2022 20:40:07 +0800 Subject: [PATCH 34/34] Sketch `Permissions` for git-config (#386) It should be possible to control which configuration files are loaded and contribute to the overall configuration. This goes along with at some point allowing to obtain only values which are from trusted configuration files, based on the trust specification passed when loading the configuration. --- Cargo.lock | 1 + git-config/Cargo.toml | 2 ++ git-config/src/lib.rs | 65 +++++++++++++++++++++++++++++++++++++++---- 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 13715aca3db..736915da01a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1121,6 +1121,7 @@ dependencies = [ "criterion", "dirs", "git-features", + "git-sec", "memchr", "nom", "pwd", diff --git a/git-config/Cargo.toml b/git-config/Cargo.toml index f250dba80c2..18a5d9dd077 100644 --- a/git-config/Cargo.toml +++ b/git-config/Cargo.toml @@ -15,6 +15,8 @@ include = ["src/**/*", "LICENSE-*", "README.md", "CHANGELOG.md"] [dependencies] git-features = { version = "^0.20.0", path = "../git-features"} +git-sec = { version = "^0.1.0", path = "../git-sec" } + dirs = "4" nom = { version = "7", default_features = false, features = [ "std" ] } memchr = "2" diff --git a/git-config/src/lib.rs b/git-config/src/lib.rs index cbb883e3162..238b250344f 100644 --- a/git-config/src/lib.rs +++ b/git-config/src/lib.rs @@ -59,12 +59,65 @@ pub mod fs; pub mod parser; pub mod values; -// mod de; -// mod ser; -// mod error; -// pub use de::{from_str, Deserializer}; -// pub use error::{Error, Result}; -// pub use ser::{to_string, Serializer}; +mod permissions { + use crate::Permissions; + + impl Permissions { + /// Allow everything which usually relates to a fully trusted environment + pub fn all() -> Self { + use git_sec::Permission::*; + Permissions { + system: Allow, + global: Allow, + user: Allow, + repository: Allow, + worktree: Allow, + env: Allow, + includes: Allow, + } + } + + /// If in doubt, this configuration can be used to safely load configuration from sources which is usually trusted, + /// that is system and user configuration. Do load any configuration that isn't trusted as it's now owned by the current user. + pub fn secure() -> Self { + use git_sec::Permission::*; + Permissions { + system: Allow, + global: Allow, + user: Allow, + repository: Deny, + worktree: Deny, + env: Allow, + includes: Deny, + } + } + } +} + +/// Configure security relevant options when loading a git configuration. +#[derive(Copy, Clone, Ord, PartialOrd, PartialEq, Eq, Debug, Hash)] +#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] +pub struct Permissions { + /// How to use the system configuration. + /// This is defined as `$(prefix)/etc/gitconfig` on unix. + pub system: git_sec::Permission, + /// How to use the global configuration. + /// This is usually `~/.gitconfig`. + pub global: git_sec::Permission, + /// How to use the user configuration. + /// Second user-specific configuration path; if `$XDG_CONFIG_HOME` is not + /// set or empty, `$HOME/.config/git/config` will be used. + pub user: git_sec::Permission, + /// How to use the repository configuration. + pub repository: git_sec::Permission, + /// How to use worktree configuration from `config.worktree`. + // TODO: figure out how this really applies and provide more information here. + pub worktree: git_sec::Permission, + /// How to use the configuration from environment variables. + pub env: git_sec::Permission, + /// What to do when include files are encountered in loaded configuration. + pub includes: git_sec::Permission, +} #[cfg(test)] pub mod test_util;