Keep keyring session payloads out of process arguments
This commit is contained in:
@@ -18,6 +18,7 @@ jobs:
|
|||||||
- run: npm run format:check
|
- run: npm run format:check
|
||||||
- run: npm run lint
|
- run: npm run lint
|
||||||
- run: npm run check
|
- run: npm run check
|
||||||
|
- run: npm test
|
||||||
- run: npm run build
|
- run: npm run build
|
||||||
- uses: actions-rust-lang/setup-rust-toolchain@v1
|
- uses: actions-rust-lang/setup-rust-toolchain@v1
|
||||||
- working-directory: native
|
- working-directory: native
|
||||||
|
|||||||
@@ -15,3 +15,4 @@ jobs:
|
|||||||
node-version: 22
|
node-version: 22
|
||||||
cache: npm
|
cache: npm
|
||||||
- run: test -s CHANGELOG.md && npm ci && npm run format:check && npm run lint && npm run check && npm run build
|
- run: test -s CHANGELOG.md && npm ci && npm run format:check && npm run lint && npm run check && npm run build
|
||||||
|
- run: npm test
|
||||||
|
|||||||
@@ -20,3 +20,8 @@ failures do not invalidate a successfully verified session.
|
|||||||
|
|
||||||
Run `node --test src/session-recovery.test.js` for response classification and
|
Run `node --test src/session-recovery.test.js` for response classification and
|
||||||
retry-schedule coverage. This is not a substitute for native AppImage acceptance.
|
retry-schedule coverage. This is not a substitute for native AppImage acceptance.
|
||||||
|
|
||||||
|
Keyring writes use `keyctl padd` with the session supplied through standard input,
|
||||||
|
never process arguments. Helper stdout/stderr are discarded so credential payloads
|
||||||
|
cannot enter diagnostics. Native tests cover the argument boundary, stdin delivery,
|
||||||
|
and helper failure handling without reading or modifying real keyring entries.
|
||||||
|
|||||||
+47
-2
@@ -9,8 +9,9 @@ use serde::{Deserialize, Serialize};
|
|||||||
use sha2::{Digest, Sha256};
|
use sha2::{Digest, Sha256};
|
||||||
use std::{
|
use std::{
|
||||||
fs, io,
|
fs, io,
|
||||||
|
io::Write,
|
||||||
path::{Path, PathBuf},
|
path::{Path, PathBuf},
|
||||||
process::Command,
|
process::{Command, Stdio},
|
||||||
};
|
};
|
||||||
|
|
||||||
pub const KEYRING_SERVICE: &str = "trixie.server.session";
|
pub const KEYRING_SERVICE: &str = "trixie.server.session";
|
||||||
@@ -84,7 +85,34 @@ pub fn store_session(settings: &Settings, session: &str) -> io::Result<()> {
|
|||||||
return Err(io::Error::new(io::ErrorKind::InvalidInput, "empty session"));
|
return Err(io::Error::new(io::ErrorKind::InvalidInput, "empty session"));
|
||||||
}
|
}
|
||||||
let description = key_description(settings);
|
let description = key_description(settings);
|
||||||
keyctl(&["add", "user", &description, session, "@s"]).map(|_| ())
|
write_key_payload(key_write_command(&description), session.as_bytes())
|
||||||
|
}
|
||||||
|
|
||||||
|
// Secret payloads must never be arguments: process listings can expose argv.
|
||||||
|
fn key_write_command(description: &str) -> Command {
|
||||||
|
let mut command = Command::new("keyctl");
|
||||||
|
command.args(["padd", "user", description, "@s"]);
|
||||||
|
command
|
||||||
|
}
|
||||||
|
|
||||||
|
fn write_key_payload(mut command: Command, payload: &[u8]) -> io::Result<()> {
|
||||||
|
let mut child = command
|
||||||
|
.stdin(Stdio::piped())
|
||||||
|
.stdout(Stdio::null())
|
||||||
|
.stderr(Stdio::null())
|
||||||
|
.spawn()?;
|
||||||
|
let result = child.stdin.take().expect("piped stdin").write_all(payload);
|
||||||
|
// Closing stdin above signals EOF. Reap the child even if writing failed.
|
||||||
|
let status = child.wait()?;
|
||||||
|
result?;
|
||||||
|
if status.success() {
|
||||||
|
Ok(())
|
||||||
|
} else {
|
||||||
|
Err(io::Error::new(
|
||||||
|
io::ErrorKind::PermissionDenied,
|
||||||
|
"Linux kernel keyring access failed",
|
||||||
|
))
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn load_session(settings: &Settings) -> io::Result<Option<String>> {
|
pub fn load_session(settings: &Settings) -> io::Result<Option<String>> {
|
||||||
@@ -117,6 +145,23 @@ pub fn clear_session(settings: &Settings) -> io::Result<()> {
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn key_write_arguments_contain_only_metadata() {
|
||||||
|
let command = key_write_command("test-description");
|
||||||
|
let args: Vec<_> = command.get_args().collect();
|
||||||
|
assert_eq!(args, ["padd", "user", "test-description", "@s"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn payload_is_delivered_via_stdin_and_failures_are_reported() {
|
||||||
|
let mut receiver = Command::new("sh");
|
||||||
|
receiver.args(["-c", "test \"$(cat)\" = fixture-payload"]);
|
||||||
|
write_key_payload(receiver, b"fixture-payload").unwrap();
|
||||||
|
let mut failure = Command::new("sh");
|
||||||
|
failure.args(["-c", "cat >/dev/null; exit 1"]);
|
||||||
|
assert!(write_key_payload(failure, b"fixture-payload").is_err());
|
||||||
|
}
|
||||||
|
|
||||||
fn settings() -> Settings {
|
fn settings() -> Settings {
|
||||||
Settings {
|
Settings {
|
||||||
server_address: "https://trixie.example".into(),
|
server_address: "https://trixie.example".into(),
|
||||||
|
|||||||
@@ -4,6 +4,7 @@
|
|||||||
"version": "0.1.0-dev",
|
"version": "0.1.0-dev",
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
|
"test": "node --test src/*.test.js",
|
||||||
"dev": "vite",
|
"dev": "vite",
|
||||||
"tauri": "tauri",
|
"tauri": "tauri",
|
||||||
"tauri:dev": "tauri dev",
|
"tauri:dev": "tauri dev",
|
||||||
|
|||||||
Reference in New Issue
Block a user