Harden IPC and secret cleanup
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
1b09a6f9c7626b737d2dc6239aa89fe57e44cc05- Parents
-
74f7ad1 - Tree
6931c45
1b09a6f
1b09a6f9c7626b737d2dc6239aa89fe57e44cc0574f7ad1
6931c45| Status | File | + | - |
|---|---|---|---|
| M |
garcard/src/daemon.rs
|
34 | 0 |
| M |
garcard/src/polkit_helper.rs
|
27 | 6 |
| M |
garcard/src/prompt_ui.rs
|
8 | 5 |
garcard/src/daemon.rsmodified@@ -3,6 +3,7 @@ use crate::config::{AgentBackendMode, Config}; | ||
| 3 | 3 | use crate::state::{AuthState, RuntimeState}; |
| 4 | 4 | use anyhow::{Context, Result}; |
| 5 | 5 | use garcard_ipc::{Command, Response}; |
| 6 | +use nix::unistd::Uid; | |
| 6 | 7 | use serde_json::json; |
| 7 | 8 | use std::os::unix::fs::PermissionsExt; |
| 8 | 9 | use std::path::{Path, PathBuf}; |
@@ -217,6 +218,8 @@ async fn handle_client( | ||
| 217 | 218 | state: Arc<RuntimeState>, |
| 218 | 219 | shutdown_tx: mpsc::UnboundedSender<()>, |
| 219 | 220 | ) -> Result<()> { |
| 221 | + authorize_ipc_peer(&stream)?; | |
| 222 | + | |
| 220 | 223 | let (reader, mut writer) = stream.into_split(); |
| 221 | 224 | let mut reader = BufReader::new(reader); |
| 222 | 225 | let mut line = String::new(); |
@@ -246,6 +249,26 @@ async fn handle_client( | ||
| 246 | 249 | Ok(()) |
| 247 | 250 | } |
| 248 | 251 | |
| 252 | +fn authorize_ipc_peer(stream: &UnixStream) -> Result<()> { | |
| 253 | + let peer = stream | |
| 254 | + .peer_cred() | |
| 255 | + .context("Failed to read IPC peer credentials")?; | |
| 256 | + let peer_uid = peer.uid(); | |
| 257 | + let expected_uid = Uid::effective().as_raw(); | |
| 258 | + validate_ipc_peer_uid(peer_uid, expected_uid) | |
| 259 | +} | |
| 260 | + | |
| 261 | +fn validate_ipc_peer_uid(peer_uid: u32, expected_uid: u32) -> Result<()> { | |
| 262 | + if peer_uid != expected_uid { | |
| 263 | + anyhow::bail!( | |
| 264 | + "IPC peer uid {} does not match daemon uid {}", | |
| 265 | + peer_uid, | |
| 266 | + expected_uid | |
| 267 | + ); | |
| 268 | + } | |
| 269 | + Ok(()) | |
| 270 | +} | |
| 271 | + | |
| 249 | 272 | fn dispatch( |
| 250 | 273 | command: Command, |
| 251 | 274 | state: &RuntimeState, |
@@ -349,4 +372,15 @@ mod tests { | ||
| 349 | 372 | assert_eq!(unregister_calls.load(Ordering::Relaxed), 1); |
| 350 | 373 | assert_eq!(register_calls.load(Ordering::Relaxed), 1); |
| 351 | 374 | } |
| 375 | + | |
| 376 | + #[test] | |
| 377 | + fn validate_ipc_peer_uid_allows_daemon_owner() { | |
| 378 | + assert!(validate_ipc_peer_uid(1000, 1000).is_ok()); | |
| 379 | + } | |
| 380 | + | |
| 381 | + #[test] | |
| 382 | + fn validate_ipc_peer_uid_rejects_other_user() { | |
| 383 | + let err = validate_ipc_peer_uid(1001, 1000).expect_err("must fail"); | |
| 384 | + assert!(err.to_string().contains("does not match daemon uid")); | |
| 385 | + } | |
| 352 | 386 | } |
garcard/src/polkit_helper.rsmodified@@ -98,9 +98,12 @@ impl HelperSocketClient { | ||
| 98 | 98 | .prompt_secret(&prompt) |
| 99 | 99 | .context("prompt handler failed")? |
| 100 | 100 | { |
| 101 | - PromptResponse::Submitted(response) => { | |
| 102 | - write_line(&mut stream, &sanitize_response(&response)) | |
| 103 | - .context("failed to send helper secret response")? | |
| 101 | + PromptResponse::Submitted(mut response) => { | |
| 102 | + let mut sanitized = sanitize_response(&response); | |
| 103 | + write_line(&mut stream, &sanitized) | |
| 104 | + .context("failed to send helper secret response")?; | |
| 105 | + scrub_string(&mut sanitized); | |
| 106 | + scrub_string(&mut response); | |
| 104 | 107 | } |
| 105 | 108 | PromptResponse::Canceled => return Ok(HelperOutcome::Canceled), |
| 106 | 109 | PromptResponse::TimedOut => return Ok(HelperOutcome::Timeout), |
@@ -111,9 +114,12 @@ impl HelperSocketClient { | ||
| 111 | 114 | .prompt_plain(&prompt) |
| 112 | 115 | .context("prompt handler failed")? |
| 113 | 116 | { |
| 114 | - PromptResponse::Submitted(response) => { | |
| 115 | - write_line(&mut stream, &sanitize_response(&response)) | |
| 116 | - .context("failed to send helper visible response")? | |
| 117 | + PromptResponse::Submitted(mut response) => { | |
| 118 | + let mut sanitized = sanitize_response(&response); | |
| 119 | + write_line(&mut stream, &sanitized) | |
| 120 | + .context("failed to send helper visible response")?; | |
| 121 | + scrub_string(&mut sanitized); | |
| 122 | + scrub_string(&mut response); | |
| 117 | 123 | } |
| 118 | 124 | PromptResponse::Canceled => return Ok(HelperOutcome::Canceled), |
| 119 | 125 | PromptResponse::TimedOut => return Ok(HelperOutcome::Timeout), |
@@ -147,6 +153,14 @@ fn sanitize_response(raw: &str) -> String { | ||
| 147 | 153 | raw.lines().collect::<Vec<_>>().join(" ") |
| 148 | 154 | } |
| 149 | 155 | |
| 156 | +fn scrub_string(value: &mut String) { | |
| 157 | + if value.is_empty() { | |
| 158 | + return; | |
| 159 | + } | |
| 160 | + let mut bytes = std::mem::take(value).into_bytes(); | |
| 161 | + bytes.fill(0); | |
| 162 | +} | |
| 163 | + | |
| 150 | 164 | pub fn parse_helper_line(raw: &str) -> Result<HelperEvent> { |
| 151 | 165 | let line = raw.trim_end_matches('\n').trim_end_matches('\r'); |
| 152 | 166 | if line == "SUCCESS" { |
@@ -361,4 +375,11 @@ mod tests { | ||
| 361 | 375 | server.join().expect("server join"); |
| 362 | 376 | let _ = std::fs::remove_file(&socket_path); |
| 363 | 377 | } |
| 378 | + | |
| 379 | + #[test] | |
| 380 | + fn scrub_string_clears_input() { | |
| 381 | + let mut value = "top-secret".to_string(); | |
| 382 | + scrub_string(&mut value); | |
| 383 | + assert!(value.is_empty()); | |
| 384 | + } | |
| 364 | 385 | } |
garcard/src/prompt_ui.rsmodified@@ -42,6 +42,7 @@ struct PromptDialog { | ||
| 42 | 42 | exit: Option<PromptExit>, |
| 43 | 43 | deadline: Option<Instant>, |
| 44 | 44 | remaining_secs: Option<u64>, |
| 45 | + backdrop: Color, | |
| 45 | 46 | card_background: Color, |
| 46 | 47 | card_border: Color, |
| 47 | 48 | accent: Color, |
@@ -88,9 +89,11 @@ impl PromptDialog { | ||
| 88 | 89 | let mut theme = Theme::dark(); |
| 89 | 90 | theme.font_family = "Sans".to_string(); |
| 90 | 91 | theme.font_size = 14.0; |
| 91 | - let card_background = Color::from_hex("#111318").expect("valid card color"); | |
| 92 | - let card_border = Color::from_hex("#2c3442").expect("valid border color"); | |
| 93 | - let accent = Color::from_hex("#8ab4f8").expect("valid accent color"); | |
| 92 | + let backdrop = Color::from_hex("#0a0b10").context("invalid prompt backdrop color")?; | |
| 93 | + let card_background = | |
| 94 | + Color::from_hex("#111318").context("invalid prompt card background color")?; | |
| 95 | + let card_border = Color::from_hex("#2c3442").context("invalid prompt card border color")?; | |
| 96 | + let accent = Color::from_hex("#8ab4f8").context("invalid prompt accent color")?; | |
| 94 | 97 | let size = window.size(); |
| 95 | 98 | let renderer = Renderer::with_theme(size.width, size.height, theme)?; |
| 96 | 99 | |
@@ -116,6 +119,7 @@ impl PromptDialog { | ||
| 116 | 119 | exit: None, |
| 117 | 120 | deadline, |
| 118 | 121 | remaining_secs: None, |
| 122 | + backdrop, | |
| 119 | 123 | card_background, |
| 120 | 124 | card_border, |
| 121 | 125 | accent, |
@@ -244,8 +248,7 @@ impl PromptDialog { | ||
| 244 | 248 | let height = size.height as i32; |
| 245 | 249 | let theme = self.renderer.theme().clone(); |
| 246 | 250 | |
| 247 | - self.renderer | |
| 248 | - .clear_color(Color::from_hex("#0a0b10").expect("valid backdrop color"))?; | |
| 251 | + self.renderer.clear_color(self.backdrop)?; | |
| 249 | 252 | |
| 250 | 253 | let card_rect = Rect::new( |
| 251 | 254 | CARD_PADDING, |