Fix pass Git config and lazy OTP commits
This commit is contained in:
@@ -2476,20 +2476,16 @@ fn validate_local_config_security(config: &gix_config::File) -> Result<(), GitEr
|
||||
let name = section.header().name().to_str_lossy().to_ascii_lowercase();
|
||||
if matches!(
|
||||
name.as_str(),
|
||||
"include"
|
||||
| "includeif"
|
||||
| "credential"
|
||||
| "filter"
|
||||
| "diff"
|
||||
| "merge"
|
||||
| "protocol"
|
||||
| "url"
|
||||
"include" | "includeif" | "credential" | "filter" | "merge" | "protocol" | "url"
|
||||
) {
|
||||
return Err(GitError::InvalidRepository(format!(
|
||||
"unsafe Git configuration section: {name}"
|
||||
)));
|
||||
}
|
||||
}
|
||||
// Upstream `pass git init` installs diff.gpg.binary and
|
||||
// diff.gpg.textconv. IronStorage parses blobs and renders diffs itself, so
|
||||
// these passive settings are preserved but never evaluated or executed.
|
||||
for key in [
|
||||
"core.hooksPath",
|
||||
"core.sshCommand",
|
||||
|
||||
@@ -11,6 +11,7 @@ use zeroize::Zeroize as _;
|
||||
use crate::{
|
||||
command::{OtpAppendRequest, OtpInputSource, OtpInsertRequest},
|
||||
crypto::{CryptoError, KeyStore, SecretProvider},
|
||||
git::{AutomaticEntryCommitter, GitError, GitIdentity},
|
||||
recipient::{RecipientPolicyError, RecipientPolicyManager, SigningPolicy},
|
||||
repository::{EncryptedEntry, EntryPath, Repository, RepositoryError, SecretBytes},
|
||||
write::{EntryAction, EntryCommit, EntryCommitError, EntryCommitter, OverwriteDecision},
|
||||
@@ -613,12 +614,80 @@ impl<'a> OtpService<'a> {
|
||||
provider: &mut impl SecretProvider,
|
||||
committer: &mut impl EntryCommitter,
|
||||
) -> Result<OtpCodeOutcome, OtpError> {
|
||||
let (path, original, plaintext, range, uri) = self.load_code_entry(entry, provider)?;
|
||||
self.finish_code(
|
||||
path,
|
||||
original,
|
||||
plaintext,
|
||||
range,
|
||||
uri,
|
||||
unix_seconds,
|
||||
signing,
|
||||
committer,
|
||||
)
|
||||
}
|
||||
|
||||
/// Generate a code with storage-owned lazy Git selection. TOTP is
|
||||
/// read-only and never opens Git; HOTP opens the innermost repository only
|
||||
/// after the token has been decrypted and identified as counter based.
|
||||
pub fn code_automatic(
|
||||
&self,
|
||||
entry: &str,
|
||||
unix_seconds: u64,
|
||||
signing: Option<&SigningPolicy>,
|
||||
provider: &mut impl SecretProvider,
|
||||
) -> Result<OtpCodeOutcome, OtpError> {
|
||||
let (path, original, plaintext, range, uri) = self.load_code_entry(entry, provider)?;
|
||||
if uri.kind() == OtpKind::Totp {
|
||||
return Ok(OtpCodeOutcome {
|
||||
code: uri.code_at(unix_seconds)?,
|
||||
counter: None,
|
||||
});
|
||||
}
|
||||
let entry = path.to_string();
|
||||
let mut committer = AutomaticEntryCommitter::for_entry(
|
||||
self.repository,
|
||||
&entry,
|
||||
GitIdentity::ironstorage(),
|
||||
)?;
|
||||
self.finish_code(
|
||||
path,
|
||||
original,
|
||||
plaintext,
|
||||
range,
|
||||
uri,
|
||||
unix_seconds,
|
||||
signing,
|
||||
&mut committer,
|
||||
)
|
||||
}
|
||||
|
||||
fn load_code_entry(
|
||||
&self,
|
||||
entry: &str,
|
||||
provider: &mut impl SecretProvider,
|
||||
) -> Result<(EntryPath, EncryptedEntry, SecretBytes, Range<usize>, OtpUri), OtpError> {
|
||||
let path = parse_entry(entry)?;
|
||||
let original = self.repository.read_entry(&path)?;
|
||||
let plaintext = self.keys.decrypt(&original, provider)?;
|
||||
let (range, uri) = find_uri(&plaintext, &path)?.ok_or_else(|| OtpError::MissingUri {
|
||||
entry: path.clone(),
|
||||
})?;
|
||||
Ok((path, original, plaintext, range, uri))
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn finish_code(
|
||||
&self,
|
||||
path: EntryPath,
|
||||
original: EncryptedEntry,
|
||||
plaintext: SecretBytes,
|
||||
range: Range<usize>,
|
||||
uri: OtpUri,
|
||||
unix_seconds: u64,
|
||||
signing: Option<&SigningPolicy>,
|
||||
committer: &mut impl EntryCommitter,
|
||||
) -> Result<OtpCodeOutcome, OtpError> {
|
||||
match uri.kind() {
|
||||
OtpKind::Totp => Ok(OtpCodeOutcome {
|
||||
code: uri.code_at(unix_seconds)?,
|
||||
@@ -722,6 +791,7 @@ pub enum OtpError {
|
||||
Repository(RepositoryError),
|
||||
Crypto(CryptoError),
|
||||
RecipientPolicy(RecipientPolicyError),
|
||||
Git(GitError),
|
||||
InvalidUri,
|
||||
InvalidScheme,
|
||||
UnsupportedType,
|
||||
@@ -771,6 +841,7 @@ impl fmt::Display for OtpError {
|
||||
Self::Repository(error) => error.fmt(formatter),
|
||||
Self::Crypto(error) => error.fmt(formatter),
|
||||
Self::RecipientPolicy(error) => error.fmt(formatter),
|
||||
Self::Git(error) => error.fmt(formatter),
|
||||
Self::InvalidUri => formatter.write_str("OTP key URI is not valid UTF-8 URI text"),
|
||||
Self::InvalidScheme => formatter.write_str("OTP key URI must use the otpauth scheme"),
|
||||
Self::UnsupportedType => formatter.write_str("OTP key URI type must be totp or hotp"),
|
||||
@@ -848,6 +919,12 @@ impl From<RecipientPolicyError> for OtpError {
|
||||
}
|
||||
}
|
||||
|
||||
impl From<GitError> for OtpError {
|
||||
fn from(error: GitError) -> Self {
|
||||
Self::Git(error)
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_input(input: &[u8]) -> Result<(), OtpError> {
|
||||
if input.is_empty() {
|
||||
Err(OtpError::EmptyInput)
|
||||
|
||||
@@ -30,6 +30,11 @@ password-store tree, and the expected plaintext. It deliberately lives outside
|
||||
the reproducible generator's `stores/`, `expected/`, and `repositories/`
|
||||
directories so a refresh cannot silently replace genuine GnuPG producer output.
|
||||
|
||||
`pass-git-config.ini` records the local diff driver written by upstream
|
||||
`pass git init`. IronStorage preserves this configuration for repository
|
||||
compatibility, but its embedded Rust diff implementation never evaluates the
|
||||
driver or launches the configured GnuPG command.
|
||||
|
||||
Rust tests parse keys, verify signatures, decrypt every entry, compare exact
|
||||
plaintext bytes, validate ciphertext digests, validate Git objects and refs,
|
||||
and materialize isolated stores and repositories. They never invoke `pass`,
|
||||
|
||||
3
crates/storage/tests/fixtures/compatibility/pass-git-config.ini
vendored
Normal file
3
crates/storage/tests/fixtures/compatibility/pass-git-config.ini
vendored
Normal file
@@ -0,0 +1,3 @@
|
||||
[diff "gpg"]
|
||||
binary = true
|
||||
textconv = gpg -d --no-tty --batch --quiet --yes
|
||||
@@ -134,6 +134,10 @@ fn remotes_and_config_are_local_https_only() -> TestResult {
|
||||
git.config_set("user.name", "Local User")?;
|
||||
assert_eq!(git.config_get("user.name")?.as_deref(), Some("Local User"));
|
||||
assert!(git.config_set("credential.helper", "evil").is_err());
|
||||
assert!(
|
||||
git.config_set("diff.gpg.textconv", "external-helper")
|
||||
.is_err()
|
||||
);
|
||||
git.remove_remote("origin")?;
|
||||
assert!(git.remotes().is_empty());
|
||||
Ok(())
|
||||
|
||||
@@ -2,12 +2,13 @@
|
||||
|
||||
mod support;
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
use std::{collections::BTreeMap, fs, io::Write as _};
|
||||
|
||||
use data_encoding::BASE32_NOPAD;
|
||||
use ironstorage::{
|
||||
command::{OtpAppendRequest, OtpInputSource, OtpInsertRequest},
|
||||
crypto::{KeyInfo, KeyStore, SecretProvider, SecretProviderError},
|
||||
git::{GitIdentity, GitRepository},
|
||||
otp::{OtpAlgorithm, OtpError, OtpInput, OtpKind, OtpService, OtpUri},
|
||||
recipient::RecipientPolicyManager,
|
||||
repository::{EntryPath, Repository, SecretBytes},
|
||||
@@ -454,6 +455,84 @@ fn malformed_ambiguous_declined_and_commit_failures_never_mutate() -> TestResult
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn automatic_code_supports_pass_diff_config_and_opens_git_only_for_hotp() -> TestResult {
|
||||
let fixture = FixtureSet::load()?;
|
||||
let store = fixture.materialize_store("basic")?;
|
||||
let repository = Repository::open(store.path())?;
|
||||
let identity = GitIdentity::ironstorage();
|
||||
let git = GitRepository::init(&repository, identity.clone())?;
|
||||
let initial_commits = git.log(None)?.len();
|
||||
drop(git);
|
||||
let config_path = store.path().join(".git/config");
|
||||
let mut config = fs::OpenOptions::new().append(true).open(&config_path)?;
|
||||
config.write_all(&fixture.read("pass-git-config.ini")?)?;
|
||||
drop(config);
|
||||
let original_config = fs::read(&config_path)?;
|
||||
|
||||
let keys = KeyStore::load(fixture.path("keys"))?;
|
||||
let service = OtpService::new(&repository, &keys);
|
||||
let mut provider = FixtureSecrets::all(&fixture);
|
||||
let totp_path = EntryPath::parse("otp/totp")?;
|
||||
let totp_before = repository.read_entry(&totp_path)?;
|
||||
let totp = service.code_automatic("otp/totp", 59, None, &mut provider)?;
|
||||
assert_eq!(totp.counter(), None);
|
||||
assert_eq!(repository.read_entry(&totp_path)?, totp_before);
|
||||
let git = GitRepository::open(&repository, identity.clone())?;
|
||||
assert_eq!(git.log(None)?.len(), initial_commits);
|
||||
drop(git);
|
||||
|
||||
let hotp = service.code_automatic("otp/hotp", 0, None, &mut provider)?;
|
||||
assert_eq!(hotp.counter(), Some(1));
|
||||
assert_eq!(service.uri("otp/hotp", &mut provider)?.counter(), Some(1));
|
||||
let git = GitRepository::open(&repository, identity)?;
|
||||
assert_eq!(git.log(None)?.len(), initial_commits + 1);
|
||||
assert_eq!(
|
||||
git.log(Some(1))?[0].message(),
|
||||
"Increment HOTP counter for otp/hotp."
|
||||
);
|
||||
assert_eq!(fs::read(config_path)?, original_config);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_only_totp_ignores_git_that_mutations_correctly_reject() -> TestResult {
|
||||
let fixture = FixtureSet::load()?;
|
||||
let store = fixture.materialize_store("basic")?;
|
||||
let repository = Repository::open(store.path())?;
|
||||
GitRepository::init(&repository, GitIdentity::ironstorage())?;
|
||||
let config_path = store.path().join(".git/config");
|
||||
let mut config = fs::OpenOptions::new().append(true).open(&config_path)?;
|
||||
config.write_all(b"[filter \"unsafe\"]\n\tclean = external-helper\n")?;
|
||||
drop(config);
|
||||
|
||||
let keys = KeyStore::load(fixture.path("keys"))?;
|
||||
let service = OtpService::new(&repository, &keys);
|
||||
let mut provider = FixtureSecrets::all(&fixture);
|
||||
let totp_path = EntryPath::parse("otp/totp")?;
|
||||
let totp_before = repository.read_entry(&totp_path)?;
|
||||
assert!(
|
||||
service
|
||||
.code_automatic("otp/totp", 59, None, &mut provider)
|
||||
.is_ok()
|
||||
);
|
||||
assert_eq!(repository.read_entry(&totp_path)?, totp_before);
|
||||
|
||||
let hotp_path = EntryPath::parse("otp/hotp")?;
|
||||
let hotp_before = repository.read_entry(&hotp_path)?;
|
||||
let head_before = fs::read(store.path().join(".git/refs/heads/main"))?;
|
||||
assert!(matches!(
|
||||
service.code_automatic("otp/hotp", 0, None, &mut provider),
|
||||
Err(OtpError::Git(_))
|
||||
));
|
||||
assert_eq!(repository.read_entry(&hotp_path)?, hotp_before);
|
||||
assert_eq!(
|
||||
fs::read(store.path().join(".git/refs/heads/main"))?,
|
||||
head_before
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn token(secret: &[u8], algorithm: &str) -> Result<OtpUri, OtpError> {
|
||||
OtpUri::parse_str(&format!(
|
||||
"otpauth://totp/RFC6238?secret={}&algorithm={algorithm}&digits=8&period=30",
|
||||
|
||||
Reference in New Issue
Block a user