Resolve security audit findings
ober
8b2fdee7a6ed31e4cc1095595fe2fc302e1ddc86
--- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,10 +12,6 @@ permissions: env: JERBOA_VERSION: v0.2.3 JERBUILD: ${{ github.workspace }}/.jerboa/bin/jerbuild - JERBOA_YUBIKEY_REPO: https://git.sr.ht/~lisp/jerboa-yubikey - JERBOA_MAIL_REPO: https://git.sr.ht/~lisp/jerboa-mail - JERBOA_HTTPS_REPO: https://git.sr.ht/~lisp/jerboa-https - JERBOA_SSL_REPO: https://git.sr.ht/~lisp/jerboa-ssl JERBOA_YUBIKEY_DIR: ${{ github.workspace }}/.deps/jerboa-yubikey JERBOA_MAIL_DIR: ${{ github.workspace }}/.deps/jerboa-mail JERBOA_HTTPS_DIR: ${{ github.workspace }}/.deps/jerboa-https @@ -25,7 +21,9 @@ jobs: build-test-audit: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false - name: Install system tools run: | @@ -39,25 +37,16 @@ jobs: run: sh support/ensure-jerboa.sh "$JERBOA_VERSION" .jerboa/bin - name: Fetch dependencies - run: | - set -eu - mkdir -p .deps - git clone --depth 1 "$JERBOA_YUBIKEY_REPO" "$JERBOA_YUBIKEY_DIR" - git clone --depth 1 "$JERBOA_MAIL_REPO" "$JERBOA_MAIL_DIR" - git clone --depth 1 "$JERBOA_HTTPS_REPO" "$JERBOA_HTTPS_DIR" - git clone --depth 1 "$JERBOA_SSL_REPO" "$JERBOA_SSL_DIR" + run: sh support/fetch-locked-dependencies.sh "$GITHUB_WORKSPACE/.deps" + + - name: Install cargo-audit + run: cargo install cargo-audit --version 0.22.2 --locked - - name: Test - run: make test + - name: Verify + run: make verify - name: CLI status smoke run: make run ARGS='status' - - name: Install cargo-audit - run: cargo install cargo-audit --locked - - - name: Audit Rust dependencies - run: make audit - - name: Release evidence run: make release-evidence --- a/.github/workflows/security-baseline.yml +++ b/.github/workflows/security-baseline.yml @@ -13,7 +13,9 @@ jobs: baseline: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false - name: Required release files run: | --- a/Makefile +++ b/Makefile @@ -35,8 +35,10 @@ else endif LD_ENV = $(LD_VAR)="$(JERBOA_SSL_DIR):$(NATIVE_DIR)" NATIVE_LIB := $(NATIVE_DIR)/libproton_bridge_native$(SO_EXT) +NATIVE_ENV = PROTON_BRIDGE_NATIVE_LIB="$(NATIVE_LIB)" +SSL_ENV = JERBOA_SSL_ALLOW_DYNAMIC_NATIVE=1 JERBOA_SSL_LIB="$(JERBOA_SSL_DIR)" -.PHONY: help ensure-jerboa-tools ensure-deps deps native run test audit verify security sanitize-evidence dependency-evidence sbom reproducibility-report target-evidence release-evidence clean +.PHONY: help ensure-jerboa-tools ensure-deps dependency-lock-check deps native native-test run test native-loader-regression audit verify security sanitize-evidence dependency-evidence sbom reproducibility-report target-evidence release-evidence clean .DEFAULT_GOAL := help help: @@ -45,7 +47,9 @@ help: @echo "Development:" @echo " make run ARGS='--help' Run the CLI" @echo " make native Build the native Proton SRP helper" + @echo " make native-test Run native helper unit tests" @echo " make test Run smoke tests" + @echo " make dependency-lock-check Verify exact dependency/toolchain locks" @echo " make audit Run the RustSec native dependency audit" @echo " make verify Run release verification checks" @echo " make target-evidence Write target Proton/YubiKey proof status" @@ -81,6 +85,28 @@ ensure-deps: @test -d "$(JERBOA_HTTPS_DIR)" || { echo "ERROR: JERBOA_HTTPS_DIR missing: $(JERBOA_HTTPS_DIR)" >&2; exit 1; } @test -d "$(JERBOA_SSL_DIR)" || { echo "ERROR: JERBOA_SSL_DIR missing: $(JERBOA_SSL_DIR)" >&2; exit 1; } +dependency-lock-check: ensure-deps + @sh support/fetch-locked-dependencies.sh "$(DEPS_DIR)" --check-only + @set -eu; \ + while IFS=' ' read -r name source commit extra; do \ + case "$$name" in ''|'#'*) continue ;; esac; \ + case "$$name" in \ + jerboa-yubikey) dir="$(JERBOA_YUBIKEY_DIR)" ;; \ + jerboa-mail) dir="$(JERBOA_MAIL_DIR)" ;; \ + jerboa-https) dir="$(JERBOA_HTTPS_DIR)" ;; \ + jerboa-ssl) dir="$(JERBOA_SSL_DIR)" ;; \ + *) echo "ERROR: unexpected dependency lock name" >&2; exit 1 ;; \ + esac; \ + actual="$$(git -C "$$dir" rev-parse HEAD)"; \ + test "$$actual" = "$$commit" || { echo "ERROR: $$name is not at its locked commit" >&2; exit 1; }; \ + done < dependencies.lock + @awk 'BEGIN { count=0 } \ + /^[[:space:]]*#/ || NF == 0 { next } \ + NF != 3 || $$1 != "v0.2.3" || ($$2 != "linux-amd64" && $$2 != "macos-arm64" && $$2 != "linux-arm64" && $$2 != "freebsd-amd64") || length($$3) != 64 || $$3 !~ /^[0-9a-f]+$$/ || seen[$$1 ":" $$2]++ { exit 1 } \ + { count++ } \ + END { if (count != 4) exit 1 }' toolchains.lock + @echo "locked_checkout_status=pass" + deps: ensure-jerboa-tools ensure-deps @if [ ! -f "$(JERBOA_SSL_DIR)/lib/jerboa-ssl.sls" ] || [ ! -f "$(JERBOA_SSL_DIR)/jerboa_ssl_shim$(SO_EXT)" ]; then \ $(MAKE) -C "$(JERBOA_SSL_DIR)" JERBUILD="$(JERBUILD)" build; \ @@ -92,14 +118,26 @@ deps: ensure-jerboa-tools ensure-deps native: deps cargo build --manifest-path $(NATIVE_MANIFEST) --release +native-test: + cargo test --manifest-path $(NATIVE_MANIFEST) --locked + run: native - JERBOA_SSL_LIB="$(JERBOA_SSL_DIR)" $(LD_ENV) \ + $(SSL_ENV) $(NATIVE_ENV) $(LD_ENV) \ "$(JERBUILD)" exec --libdirs "$(LIBDIRS)" main.ss -- $(ARGS) test: native - JERBOA_SSL_LIB="$(JERBOA_SSL_DIR)" $(LD_ENV) \ + $(SSL_ENV) $(NATIVE_ENV) $(LD_ENV) \ "$(JERBUILD)" exec --libdirs "$(LIBDIRS)" test/test-all.ss +native-loader-regression: native + @JERBUILD="$(JERBUILD)" \ + LIBDIRS="$(LIBDIRS)" \ + JERBOA_SSL_DIR="$(JERBOA_SSL_DIR)" \ + NATIVE_LIB="$(NATIVE_LIB)" \ + LD_VAR="$(LD_VAR)" \ + LD_PATH="$(JERBOA_SSL_DIR):$(NATIVE_DIR)" \ + sh scripts/native-loader-regression.sh + audit: @if cargo audit --version >/dev/null 2>&1; then \ cargo audit --file "$(NATIVE_LOCK)" -D warnings; \ @@ -108,9 +146,9 @@ audit: exit 1; \ fi -verify: security test audit +verify: security native-test test native-loader-regression audit -security: +security: dependency-lock-check @set -eu; \ pattern="(BEGIN (RSA|OPENSSH|EC|DSA|PRIVATE) KEY|ghp_[A-Za-z0-9_]{20,}|github_pat_[A-Za-z0-9_]{20,}|glpat-[A-Za-z0-9_-]{20,}|gldt-[A-Za-z0-9_-]{20,}|glrt-[A-Za-z0-9_-]{20,}|Authorization['\\\"]?[[:space:]]*[:=][[:space:]]*['\\\"]?Bearer[[:space:]]+[A-Za-z0-9_-]{20,}\\.[A-Za-z0-9_-]{20,}\\.[A-Za-z0-9_-]{20,}|PROTON_(PASSWORD|MAILBOX_PASSWORD|ACCESS_TOKEN|REFRESH_TOKEN|TOKEN)=['\\\"]?[A-Za-z0-9_./+=:-]{24,}|AKIA[0-9A-Z]{16}|ASIA[0-9A-Z]{16})"; \ matches="$$(git grep -n -I -E "$$pattern" -- . ":!*.png" ":!*.jpg" ":!*.jpeg" ":!*.gif" ":!*.so" ":!*.dylib" ":!*.o" ":!*.a" ":!*.boot" ":!*.tar.gz" || true)"; \ @@ -125,6 +163,14 @@ security: grep -q 'private_path_scan_status=pass' scripts/release-evidence.sh; \ grep -q 'target-proton-yubikey-proof-contains-sensitive-material' scripts/target-evidence.sh; \ grep -q 'proofMaterialPolicy' .jerboa/security.json; \ + awk '/uses:[[:space:]]/ { ref=$$0; sub(/^.*@/, "", ref); sub(/[[:space:]#].*$$/, "", ref); if (length(ref) != 40 || ref !~ /^[0-9a-f]+$$/) { print; bad=1 } } END { exit bad ? 1 : 0 }' .github/workflows/*.yml; \ + ! grep -R -n -E 'git[[:space:]]+clone([[:space:]]|$$)' .github support scripts Makefile; \ + ! grep -R -n -E '(stty|sh[[:space:]]+-c)' proton-bridge; \ + ! grep -R -n '\*native-paths\*' proton-bridge; \ + grep -q 'PROTON_BRIDGE_NATIVE_LIB must be an absolute path' proton-bridge/native-loader.ss; \ + grep -q 'dynamic native overrides are disabled for privileged execution' proton-bridge/native-loader.ss; \ + grep -q 'terminal-safe-inline' proton-bridge/cli.ss; \ + grep -q 'toolchains.lock' support/ensure-jerboa.sh; \ echo "secret_scan_status=pass" sanitize-evidence: --- a/README.md +++ b/README.md @@ -7,18 +7,9 @@ a GPL-3.0-or-later reference, but it is not intended to reproduce the same security boundary. The specific goal is to avoid a long-lived local IMAP password that allows mail access without a fresh YubiKey-gated authorization. -Upstream reference clone: - -```text -/Users/user/mine/proton-bridge-upstream -``` - -Current companion libraries: - -- `/Users/user/mine/jerboa-yubikey` -- `/Users/user/mine/jerboa-mail` -- `/Users/user/mine/jerboa-https` -- `/Users/user/mine/jerboa-crypto` +The Proton Bridge protocol and GPL source are the upstream reference. Companion +Jerboa libraries are resolved from sibling worktrees for development or from +the exact SourceHut commits in `dependencies.lock` in CI. ## Security Model @@ -89,10 +80,23 @@ make run ARGS='folders --username USER' make run ARGS='list --username USER --folder INBOX --limit 20' make run ARGS='message-json --username USER --id MESSAGE_ID' make run ARGS='show --username USER --id MESSAGE_ID' +make run ARGS='show --username USER --id MESSAGE_ID --body-file private-message.txt' make run ARGS='login --auth-info auth-info.json --username USER' make run ARGS='login --auth-options auth-options.json' ``` +Interactive passwords and PINs are read directly from `/dev/tty`; the native +helper saves and restores the exact terminal attributes and fails if echo +cannot be disabled. Automation should pass an already-open non-terminal +descriptor with `--password-fd`, `--mailbox-password-fd`, or `--pin-fd`. +Environment-name options remain supported, but secrets are never accepted +directly in argv or from ordinary stdin. + +Folder/message fields and the default `show` body are neutralized for ANSI, +OSC, C0/C1, and bidirectional controls before terminal output. Exact decrypted +body bytes are available only through `--body-file`, which creates a new 0600 +file and refuses to follow or replace an existing path. + The live login path is intentionally disabled in the strict build before it prompts for a Proton password. Proton SRP proof generation requires signed modulus verification; the previous rPGP/RSA dependency path was removed until @@ -107,9 +111,15 @@ there is no local protocol listener. The release security model is documented in [`docs/security-model.md`](docs/security-model.md). Current release gates are -`make test`, source scanning, tracked/history secret scanning, Rust dependency -audit, sibling dependency evidence, SBOM output, and native helper -reproducibility evidence. +`make test`, native loader attack regressions, source scanning, locked +dependency verification, Rust dependency audit, sibling dependency evidence, +SBOM output, and native helper reproducibility evidence. + +Dynamic development runs load the Proton helper only from the canonical +absolute `PROTON_BRIDGE_NATIVE_LIB` selected by the Makefile. Bare filenames, +the current directory, relative paths, symlinks, untrusted owners, and +group/world-writable files are rejected. Production static binaries use +registered symbols and do not perform dynamic lookup. `make release-evidence` writes the local release-evidence bundle under `dist/release-evidence/`. It is allowed to fail after writing evidence; that --- a/SECURITY.md +++ b/SECURITY.md @@ -22,12 +22,20 @@ experimental until the release checklist in `~/Release-plan.md` and YubiKey-gated encrypted cache is designed and reviewed. - Diagnostics must not print passwords, refresh/access tokens, SRP secrets, FIDO2 PINs, decrypted key material, raw Proton API responses containing - secrets, or plaintext message bodies except for explicit `show` output. + secrets, or unsanitized remote terminal fields. Default `show` output + neutralizes terminal and bidirectional controls; exact bodies require an + explicit new 0600 `--body-file`. - Target Proton/YubiKey release proof must be marker-validated; raw account output, auth JSON, tokens, mailbox passwords, FIDO2 PINs, decrypted key material, and plaintext mail must never be preserved in evidence. - Native Rust FFI boundaries must validate buffer lengths, return explicit - errors, and avoid top-level dynamic-loader crashes in static binaries. + errors, restore exact terminal state on secret-input failures, and avoid + top-level dynamic-loader crashes in static binaries. +- Dynamic native development loading must use one canonical absolute helper + owned by root/current user and reject symlinks and group/world-writable + files; no CWD, bare-name, or relative fallback is permitted. +- GitHub actions, sibling Git dependencies, and downloaded Jerboa toolchains + must match the immutable revisions/digests checked into this repository. - Release builds must pass `make test`, the baseline secret scan, the MCP source scanner, sibling dependency evidence, SBOM/reproducibility evidence, and a Rust dependency audit. new file mode 100644 --- /dev/null +++ b/dependencies.lock @@ -0,0 +1,5 @@ +# name source commit +jerboa-yubikey https://git.sr.ht/~lisp/jerboa-yubikey 0091e16bfe2f5d96e7fb18ce016e5b1ce010edae +jerboa-mail https://git.sr.ht/~lisp/jerboa-mail 5d6382371decb5ce93016b46a1710d65255401b0 +jerboa-https https://git.sr.ht/~lisp/jerboa-https c3a4be506edcdddeed37c4b7723bdd226a8511c1 +jerboa-ssl https://git.sr.ht/~lisp/jerboa-ssl 270b858534941baad6420e4906a26c279f4a8678 --- a/docs/dependency-provenance.md +++ b/docs/dependency-provenance.md @@ -46,6 +46,18 @@ The CLI depends on these local Jerboa repos: - `jerboa-https` for HTTPS client behavior. - `jerboa-ssl` for TLS/OpenSSL integration. +`dependencies.lock` records each trusted HTTPS SourceHut origin and full +40-hex commit. CI initializes each repository, fetches that exact commit, and +checks out detached; it never clones a mutable default branch. `make +dependency-lock-check` also requires development sibling worktrees to be at +those commits. Dirty-worktree state remains visible in SBOM evidence and is +reviewed separately. + +`toolchains.lock` records the audited SHA-256 for every supported Jerboa +v0.2.3 release archive. `support/ensure-jerboa.sh` verifies downloaded bytes +against this checked-in trust root and does not download a checksum from the +artifact origin. GitHub workflow actions are pinned to full commit SHAs. + `make dependency-evidence` runs each sibling `release-evidence` target when it exists. Missing sibling release evidence is a production blocker, not a warning. Sibling evidence discovery parses Makefiles for the `release-evidence` target --- a/docs/release-evidence.md +++ b/docs/release-evidence.md @@ -10,6 +10,8 @@ The target records: - current Git state, - high-confidence secret scan output, +- dependency/toolchain lock verification, +- native termios unit tests and malicious-loader regression output, - smoke tests and CLI `status`/`--help` output, - RustSec dependency audit output, - sibling Jerboa dependency release evidence, --- a/docs/security-model.md +++ b/docs/security-model.md @@ -19,14 +19,19 @@ Default policy: When the corresponding feature is enabled, the CLI may read: - Proton username from `--username` -- Proton account password from an interactive prompt or `--password-env` -- mailbox password from an interactive prompt or `--mailbox-password-env` when - Proton reports two-password mode -- FIDO2 PIN from an interactive prompt when `--prompt-pin` is used - -Environment-variable password input is for automation only. Prefer a process -supervisor or shell session that avoids persistent command history. Do not pass -passwords directly in argv. +- Proton account password from the controlling-terminal prompt, + `--password-fd`, or `--password-env` +- mailbox password from the controlling-terminal prompt, + `--mailbox-password-fd`, or `--mailbox-password-env` when Proton reports + two-password mode +- FIDO2 PIN from the controlling-terminal prompt or `--pin-fd` + +The prompt opens `/dev/tty` directly, saves the exact termios state, disables +echo without a shell, masks termination signals until restoration, and fails +closed on every open/TTY/termios/read/restore error. Automation should prefer +an inherited pipe/file descriptor; descriptor input rejects terminals and is +bounded to 4096 bytes. Environment-name input is retained for compatibility. +Do not pass secrets directly in argv or ordinary stdin. ## No-Store Requirements @@ -39,7 +44,8 @@ The default path must not write these values to disk: - decrypted private keys - derived mailbox passphrases - FIDO2 PINs -- plaintext message bodies or attachments +- plaintext message bodies or attachments, except an explicit `--body-file` + requested by the user If a future cache is added, it must require an external unlock factor such as YubiKey PIV/WebAuthn plus a user passphrase or OS keyring secret. A generated @@ -51,9 +57,11 @@ Normal diagnostics may report phase, policy, and authenticated UID. They must not print SRP password material, refresh/access tokens, raw auth responses, FIDO2 PINs, decrypted key material, or message bodies. -The `show` command intentionally prints selected decrypted mail content. Treat -that output as plaintext sensitive data because terminals, shells, scrollback, -and caller processes may retain it. +Remote folder/message fields and default `show` content escape C0/C1, ANSI/OSC, +zero-width, line-separator, and bidirectional control characters. Newlines in a +message body remain visible. Exact bodies require `--body-file`; it creates a +new mode-0600 file with exclusive creation and never replaces an existing path. +Both sanitized terminal output and exact files remain plaintext sensitive data. ## Native Boundary @@ -71,6 +79,12 @@ FFI rules: - native errors are surfaced as Scheme errors without embedding secrets - shared libraries are loaded only when an availability check or native call is made +- static registered symbols are probed before any dynamic operation +- dynamic loading requires canonical absolute `PROTON_BRIDGE_NATIVE_LIB`; the + file must be regular, non-symlink, root/current-user owned, and not writable + by group/other +- no current-directory, relative, bare-name, argv0, or loader-path search is + performed - disabled SRP/OpenPGP/Drive symbols fail closed with zero-length outputs ## Release Requirements @@ -78,6 +92,8 @@ FFI rules: Before a production release: - Run `make test` from a clean checkout. +- Run `make native-loader-regression` and `make native-test`. +- Verify `dependencies.lock`, `toolchains.lock`, and full-SHA workflow actions. - Run `jerboa_security_scan` on the project source set. - Run a tracked-file and history secret scan for Proton credentials, tokens, auth JSON, key material, and plaintext mail exports. --- a/proton-bridge-native/Cargo.lock +++ b/proton-bridge-native/Cargo.lock @@ -246,6 +246,7 @@ name = "proton-bridge-native" version = "0.1.0" dependencies = [ "base64", + "libc", "proton-srp", "zeroize", ] --- a/proton-bridge-native/Cargo.toml +++ b/proton-bridge-native/Cargo.toml @@ -10,5 +10,6 @@ crate-type = ["cdylib", "rlib"] [dependencies] base64 = "0.22" +libc = "0.2" proton-srp = { version = "0.8.2", default-features = false } zeroize = "1.8" --- a/proton-bridge-native/src/lib.rs +++ b/proton-bridge-native/src/lib.rs @@ -1,6 +1,8 @@ use base64::{engine::general_purpose::STANDARD as BASE64_STANDARD, Engine as _}; use proton_srp::mailbox_password_hash; use std::ffi::CStr; +use std::io; +use std::mem; use std::os::raw::c_char; use std::slice; use std::sync::{Mutex, OnceLock}; @@ -11,6 +13,7 @@ const PB_SRP_INVALID_ARGUMENT: i32 = -1; const PB_SRP_ERROR: i32 = -3; const PB_SRP_INSUFFICIENT_BUFFER: i32 = -4; const PB_SRP_UNSUPPORTED_FEATURE: i32 = -5; +const MAX_SECRET_BYTES: usize = 4096; static LAST_ERROR: OnceLock<Mutex<String>> = OnceLock::new(); @@ -83,6 +86,215 @@ unsafe fn write_string(value: &str, out: *mut u8, out_len: *mut u32) -> Result<( write_bytes(value.as_bytes(), out, out_len) } +struct FileDescriptor(libc::c_int); + +impl Drop for FileDescriptor { + fn drop(&mut self) { + unsafe { + libc::close(self.0); + } + } +} + +struct SignalMaskGuard { + previous: libc::sigset_t, + active: bool, +} + +impl SignalMaskGuard { + fn block_terminal_interrupts() -> Result<Self, String> { + unsafe { + let mut blocked: libc::sigset_t = mem::zeroed(); + let mut previous: libc::sigset_t = mem::zeroed(); + if libc::sigemptyset(&mut blocked) != 0 { + return Err(io::Error::last_os_error().to_string()); + } + for signal in [ + libc::SIGHUP, + libc::SIGINT, + libc::SIGQUIT, + libc::SIGTERM, + libc::SIGTSTP, + ] { + if libc::sigaddset(&mut blocked, signal) != 0 { + return Err(io::Error::last_os_error().to_string()); + } + } + let rc = libc::pthread_sigmask(libc::SIG_BLOCK, &blocked, &mut previous); + if rc != 0 { + return Err(io::Error::from_raw_os_error(rc).to_string()); + } + Ok(Self { + previous, + active: true, + }) + } + } + + fn restore(&mut self) -> Result<(), String> { + if self.active { + let rc = unsafe { + libc::pthread_sigmask(libc::SIG_SETMASK, &self.previous, std::ptr::null_mut()) + }; + if rc != 0 { + return Err(io::Error::from_raw_os_error(rc).to_string()); + } + self.active = false; + } + Ok(()) + } +} + +impl Drop for SignalMaskGuard { + fn drop(&mut self) { + let _ = self.restore(); + } +} + +struct TermiosGuard { + fd: libc::c_int, + original: libc::termios, + active: bool, +} + +impl TermiosGuard { + fn disable_echo(fd: libc::c_int) -> Result<Self, String> { + unsafe { + let mut original: libc::termios = mem::zeroed(); + if libc::tcgetattr(fd, &mut original) != 0 { + return Err(format!( + "cannot read controlling-terminal attributes: {}", + io::Error::last_os_error() + )); + } + let mut hidden = original; + hidden.c_lflag &= !(libc::ECHO | libc::ECHONL); + if libc::tcsetattr(fd, libc::TCSAFLUSH, &hidden) != 0 { + return Err(format!( + "cannot disable controlling-terminal echo: {}", + io::Error::last_os_error() + )); + } + Ok(Self { + fd, + original, + active: true, + }) + } + } + + fn restore(&mut self) -> Result<(), String> { + if self.active { + loop { + if unsafe { libc::tcsetattr(self.fd, libc::TCSAFLUSH, &self.original) } == 0 { + self.active = false; + break; + } + let err = io::Error::last_os_error(); + if err.raw_os_error() != Some(libc::EINTR) { + return Err(format!( + "cannot restore controlling-terminal attributes: {err}" + )); + } + } + } + Ok(()) + } +} + +impl Drop for TermiosGuard { + fn drop(&mut self) { + let _ = self.restore(); + } +} + +fn write_fd_all(fd: libc::c_int, value: &[u8]) -> Result<(), String> { + let mut offset = 0; + while offset < value.len() { + let rc = unsafe { libc::write(fd, value[offset..].as_ptr().cast(), value.len() - offset) }; + if rc < 0 { + let err = io::Error::last_os_error(); + if err.raw_os_error() == Some(libc::EINTR) { + continue; + } + return Err(err.to_string()); + } + if rc == 0 { + return Err("terminal write returned zero bytes".to_owned()); + } + offset += rc as usize; + } + Ok(()) +} + +fn read_secret_from_terminal_fd(fd: libc::c_int, prompt: &[u8]) -> Result<Vec<u8>, String> { + if unsafe { libc::isatty(fd) } != 1 { + return Err("secret input requires a controlling terminal".to_owned()); + } + + let mut signal_mask = SignalMaskGuard::block_terminal_interrupts() + .map_err(|err| format!("cannot protect terminal restoration: {err}"))?; + write_fd_all(fd, prompt).map_err(|err| format!("cannot write terminal prompt: {err}"))?; + let mut terminal = TermiosGuard::disable_echo(fd)?; + let mut secret = Vec::with_capacity(128); + let read_result = loop { + let mut byte = 0_u8; + let rc = unsafe { libc::read(fd, (&mut byte as *mut u8).cast(), 1) }; + if rc < 0 { + let err = io::Error::last_os_error(); + if err.raw_os_error() == Some(libc::EINTR) { + continue; + } + break Err(format!("cannot read controlling terminal: {err}")); + } + if rc == 0 { + break Err("controlling terminal reached EOF".to_owned()); + } + if byte == b'\n' || byte == b'\r' { + break Ok(()); + } + if byte == 0 { + break Err("secret contains a NUL byte".to_owned()); + } + if secret.len() == MAX_SECRET_BYTES { + break Err("secret exceeds 4096 bytes".to_owned()); + } + secret.push(byte); + }; + + if let Err(err) = terminal.restore() { + secret.zeroize(); + return Err(err); + } + let newline_result = write_fd_all(fd, b"\n"); + let signal_result = signal_mask.restore(); + if let Err(err) = read_result { + secret.zeroize(); + return Err(err); + } + newline_result.map_err(|err| format!("cannot finish terminal prompt: {err}"))?; + signal_result.map_err(|err| format!("cannot restore terminal signal mask: {err}"))?; + Ok(secret) +} + +fn read_secret_from_controlling_tty(prompt: &[u8]) -> Result<Vec<u8>, String> { + let path = b"/dev/tty\0"; + let fd = unsafe { + libc::open( + path.as_ptr().cast::<c_char>(), + libc::O_RDWR | libc::O_NOCTTY | libc::O_CLOEXEC, + ) + }; + if fd < 0 { + return Err(format!( + "cannot open controlling terminal: {}", + io::Error::last_os_error() + )); + } + let fd = FileDescriptor(fd); + read_secret_from_terminal_fd(fd.0, prompt) +} + fn mailbox_password_tail(password: &str, key_salt_b64: &str) -> Result<String, String> { let key_salt = BASE64_STANDARD .decode(key_salt_b64) @@ -113,6 +325,63 @@ pub unsafe extern "C" fn pb_native_srp_last_error(out: *mut u8, out_len: *mut u3 } #[no_mangle] +/// Read one UTF-8 secret from the process controlling terminal. +/// +/// # Safety +/// +/// `prompt` must reference `prompt_len` readable bytes when non-null. +/// `out_len` must be writable, and a non-null `out` must reference at least +/// the capacity initially stored in `out_len` bytes. +pub unsafe extern "C" fn pb_native_read_secret( + prompt: *const u8, + prompt_len: u32, + out: *mut u8, + out_len: *mut u32, +) -> i32 { + set_last_error(""); + if out_len.is_null() { + set_last_error("output length pointer is null"); + return PB_SRP_INVALID_ARGUMENT; + } + if prompt.is_null() && prompt_len != 0 { + clear_len(out_len); + set_last_error("prompt pointer is null"); + return PB_SRP_INVALID_ARGUMENT; + } + let prompt = if prompt_len == 0 { + &[][..] + } else { + slice::from_raw_parts(prompt, prompt_len as usize) + }; + if prompt.contains(&0) { + clear_len(out_len); + set_last_error("prompt contains a NUL byte"); + return PB_SRP_INVALID_ARGUMENT; + } + + let mut secret = match read_secret_from_controlling_tty(prompt) { + Ok(secret) => secret, + Err(err) => { + clear_len(out_len); + set_last_error(err); + return PB_SRP_ERROR; + } + }; + if std::str::from_utf8(&secret).is_err() { + secret.zeroize(); + clear_len(out_len); + set_last_error("secret is not valid UTF-8"); + return PB_SRP_INVALID_ARGUMENT; + } + let rc = match write_bytes(&secret, out, out_len) { + Ok(()) => PB_SRP_OK, + Err(code) => code, + }; + secret.zeroize(); + rc +} + +#[no_mangle] pub unsafe extern "C" fn pb_native_mailbox_password( password: *const c_char, key_salt_b64: *const c_char, @@ -334,6 +603,68 @@ pub unsafe extern "C" fn pb_native_srp_proofs( mod tests { use super::*; use std::ffi::CString; + use std::thread; + + fn open_test_pty() -> (libc::c_int, libc::c_int, libc::termios) { + unsafe { + let mut master = -1; + let mut slave = -1; + let rc = libc::openpty( + &mut master, + &mut slave, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + ); + assert_eq!(rc, 0, "openpty failed: {}", io::Error::last_os_error()); + let mut original: libc::termios = mem::zeroed(); + assert_eq!(libc::tcgetattr(slave, &mut original), 0); + (master, slave, original) + } + } + + fn termios_bytes(value: &libc::termios) -> &[u8] { + unsafe { + slice::from_raw_parts( + (value as *const libc::termios).cast::<u8>(), + mem::size_of::<libc::termios>(), + ) + } + } + + fn read_fd_exact(fd: libc::c_int, length: usize) -> Vec<u8> { + let mut value = vec![0_u8; length]; + let mut offset = 0; + while offset < length { + let rc = + unsafe { libc::read(fd, value[offset..].as_mut_ptr().cast(), length - offset) }; + assert!(rc > 0, "PTY read failed: {}", io::Error::last_os_error()); + offset += rc as usize; + } + value + } + + fn assert_termios_equal(fd: libc::c_int, expected: &libc::termios) { + unsafe { + let mut actual: libc::termios = mem::zeroed(); + assert_eq!(libc::tcgetattr(fd, &mut actual), 0); + assert_eq!(termios_bytes(&actual), termios_bytes(expected)); + } + } + + fn wait_for_echo_disabled(fd: libc::c_int) { + for _ in 0..1_000 { + unsafe { + let mut current: libc::termios = mem::zeroed(); + assert_eq!(libc::tcgetattr(fd, &mut current), 0); + if current.c_lflag & libc::ECHO == 0 { + return; + } + } + thread::sleep(std::time::Duration::from_millis(1)); + } + panic!("PTY echo was not disabled"); + } #[test] fn derives_proton_mailbox_key_password_suffix() { @@ -385,4 +716,51 @@ mod tests { assert_eq!(rc, PB_SRP_UNSUPPORTED_FEATURE); assert_eq!(out_len, 0); } + + #[test] + fn secret_reader_rejects_non_terminal_before_reading() { + let mut fds = [-1; 2]; + assert_eq!(unsafe { libc::pipe(fds.as_mut_ptr()) }, 0); + let result = read_secret_from_terminal_fd(fds[0], b"Password: "); + unsafe { + libc::close(fds[0]); + libc::close(fds[1]); + } + assert_eq!( + result.unwrap_err(), + "secret input requires a controlling terminal" + ); + } + + #[test] + fn secret_reader_restores_exact_termios_after_success() { + let (master, slave, original) = open_test_pty(); + let worker = thread::spawn(move || read_secret_from_terminal_fd(slave, b"Password: ")); + assert_eq!(read_fd_exact(master, 10), b"Password: "); + wait_for_echo_disabled(slave); + write_fd_all(master, b"s3cret\n").unwrap(); + let secret = worker.join().unwrap().unwrap(); + assert_eq!(secret, b"s3cret"); + assert_termios_equal(slave, &original); + unsafe { + libc::close(master); + libc::close(slave); + } + } + + #[test] + fn secret_reader_restores_exact_termios_after_read_failure() { + let (master, slave, original) = open_test_pty(); + let worker = thread::spawn(move || read_secret_from_terminal_fd(slave, b"Password: ")); + assert_eq!(read_fd_exact(master, 10), b"Password: "); + wait_for_echo_disabled(slave); + unsafe { + libc::close(master); + } + assert!(worker.join().unwrap().is_err()); + assert_termios_equal(slave, &original); + unsafe { + libc::close(slave); + } + } } --- a/proton-bridge/cli.ss +++ b/proton-bridge/cli.ss @@ -8,7 +8,7 @@ (except (jerboa prelude) meta atom? partition sort sort! make-date make-time read-file-string) - (only (jerboa core) getenv open-process process-status) + (only (jerboa core) getenv) (only (std text json) string->json-object json-object->string) (proton-bridge security) (proton-bridge api auth) @@ -18,6 +18,7 @@ (proton-bridge crypto) (proton-bridge fido2) (proton-bridge srp) + (proton-bridge terminal) (only (yubikey fido2) fido2-assertion-credential-id)) (define version "0.0.0-rewrite-plan") @@ -35,18 +36,23 @@ " message-json Fetch one raw Proton message JSON object\n" " show Fetch/decrypt/show one message after fresh auth\n" " help Print this help\n" - " version Print version\n")) + " version Print version\n" + "\n" + "secret automation (non-terminal descriptors only):\n" + " --password-fd FD --mailbox-password-fd FD --pin-fd FD\n" + "exact body output:\n" + " --body-file PATH Create a new private 0600 file; never replace\n")) (define (println s) (display s) (newline)) (define (eprintln s) - (display s (current-error-port)) + (display (terminal-safe-block s) (current-error-port)) (newline (current-error-port))) (define (die code msg) - (display msg (current-error-port)) + (display (terminal-safe-block msg) (current-error-port)) (newline (current-error-port)) (exit code)) @@ -68,7 +74,7 @@ (define (print-status) (println "jerboa-proton-bridge") (println (string-append "version: " version)) - (println "upstream reference: /Users/user/mine/proton-bridge-upstream") + (println "upstream reference: Proton Bridge protocol") (println "security:") (for-each (lambda (item) @@ -124,24 +130,6 @@ (call-with-input-file path (lambda (port) (get-string-all port)))) - (define (disable-tty-echo) - (guard (e [#t #f]) - (let ([proc (open-process - (list 'path: "sh" - 'arguments: - (list "-c" "stty -echo </dev/tty 2>/dev/null")))]) - (process-status proc) - #t))) - - (define (enable-tty-echo) - (guard (e [#t #f]) - (let ([proc (open-process - (list 'path: "sh" - 'arguments: - (list "-c" "stty echo </dev/tty 2>/dev/null")))]) - (process-status proc) - #t))) - (define (opt opts k) (let ([p (assoc k opts)]) (and p (cdr p)))) @@ -165,32 +153,49 @@ [else (loop (cdr xs) opts (cons (car xs) pos))]))) (define (read-secret prompt) - (display prompt (current-error-port)) - (let ([line #f]) - (dynamic-wind - (lambda () (disable-tty-echo)) - (lambda () (set! line (get-line (current-input-port)))) - (lambda () - (enable-tty-echo) - (newline (current-error-port)))) - (if (eof-object? line) "" line))) + (read-secret-from-tty prompt)) + + (define (option-file-descriptor opts name) + (let* ([raw (opt opts name)] + [fd (and raw (string->number raw))]) + (cond + [(not raw) #f] + [(and fd (integer? fd) (>= fd 0)) fd] + [else (die 2 (string-append "invalid file descriptor for " name))]))) (define (read-password-from-options opts) - (let ([env-name (opt opts "--password-env")]) + (let ([env-name (opt opts "--password-env")] + [fd (option-file-descriptor opts "--password-fd")]) (cond + [(and env-name fd) + (die 2 "use only one of --password-env and --password-fd")] + [fd (read-secret-from-fd fd)] [env-name (or (getenv env-name) (die 2 (string-append "environment variable is unset: " env-name)))] [else (read-secret "Proton password: ")]))) (define (read-mailbox-password-from-options opts) - (let ([env-name (opt opts "--mailbox-password-env")]) + (let ([env-name (opt opts "--mailbox-password-env")] + [fd (option-file-descriptor opts "--mailbox-password-fd")]) (cond + [(and env-name fd) + (die 2 "use only one of --mailbox-password-env and --mailbox-password-fd")] + [fd (read-secret-from-fd fd)] [env-name (or (getenv env-name) (die 2 (string-append "environment variable is unset: " env-name)))] [else (read-secret "Proton mailbox password: ")]))) + (define (read-pin-from-options opts) + (let ([fd (option-file-descriptor opts "--pin-fd")]) + (cond + [(and fd (opt opts "--prompt-pin")) + (die 2 "use only one of --pin-fd and --prompt-pin")] + [fd (read-secret-from-fd fd)] + [(opt opts "--prompt-pin") (read-secret "FIDO2 PIN: ")] + [else ""]))) + (define (key-pass-from-auth auth password opts) (if (= (proton-auth-password-mode auth) 2) (read-mailbox-password-from-options opts) @@ -218,9 +223,7 @@ (lambda (proofs auth) (cond [(proton-auth-fido2-required? auth) - (let ([pin (if (opt opts "--prompt-pin") - (read-secret "FIDO2 PIN: ") - "")] + (let ([pin (read-pin-from-options opts)] [key-pass (key-pass-from-auth auth password opts)]) (eprintln "Touch your YubiKey when it blinks.") (call-with-values @@ -251,14 +254,16 @@ (lambda () (proton-srp-auth-request-json auth-info username password)) (lambda (proofs payload-json) (println "SRP Auth request body:") - (println payload-json) + (println (terminal-safe-json payload-json)) (println (string-append "expected ServerProof: " - (proton-srp-proofs-expected-server-proof proofs)))))))) + (terminal-safe-inline + (proton-srp-proofs-expected-server-proof proofs))))))))) (define (cmd-login-live opts) (let ([session (authenticated-session-from-options opts)]) (println - (string-append "authenticated UID: " (proton-session-uid session))))) + (string-append "authenticated UID: " + (terminal-safe-inline (proton-session-uid session)))))) (define (option-number opts name default) (let* ([raw (opt opts name)] @@ -300,13 +305,13 @@ (define (print-folder-row label) (println (string-append - (json-value->string (json-ref label "ID" "")) + (terminal-safe-inline (json-value->string (json-ref label "ID" ""))) "\t" - (json-value->string (json-ref label "Type" "")) + (terminal-safe-inline (json-value->string (json-ref label "Type" ""))) "\t" - (json-value->string (json-ref label "Name" "")) + (terminal-safe-inline (json-value->string (json-ref label "Name" ""))) "\t" - (label-path-string label))))