@@ -187,10 +187,25 @@ fn handle_connection(stream: UnixStream, state: &mut DaemonState) -> io::Result< |
| 187 | } | 187 | } |
| 188 | Some(ControlRequest::BeginRequest { | 188 | Some(ControlRequest::BeginRequest { |
| 189 | id, | 189 | id, |
| 190 | - sender, | 190 | + sender: _sender, |
| 191 | app_id, | 191 | app_id, |
| 192 | parent_window, | 192 | parent_window, |
| 193 | }) => { | 193 | }) => { |
| | 194 | + let sender = match trusted_sender(reader.get_ref()) { |
| | 195 | + Ok(sender) => sender, |
| | 196 | + Err(error) => { |
| | 197 | + let mapping = map_portal_error(&PortalError::InternalFailure); |
| | 198 | + logging::warn(&format!("peer_identity_error={error}")); |
| | 199 | + return write_response( |
| | 200 | + reader.into_inner(), |
| | 201 | + ControlResponse::Error { |
| | 202 | + code: mapping.code as u32, |
| | 203 | + reason: mapping.reason.to_string(), |
| | 204 | + }, |
| | 205 | + ); |
| | 206 | + } |
| | 207 | + }; |
| | 208 | + |
| 194 | let validation = validate_request_identity(&id, &sender, app_id.as_deref()); | 209 | let validation = validate_request_identity(&id, &sender, app_id.as_deref()); |
| 195 | if let Err(error) = validation { | 210 | if let Err(error) = validation { |
| 196 | let mapping = map_portal_error(&error); | 211 | let mapping = map_portal_error(&error); |
@@ -266,10 +281,25 @@ fn handle_connection(stream: UnixStream, state: &mut DaemonState) -> io::Result< |
| 266 | } | 281 | } |
| 267 | Some(ControlRequest::TransitionRequest { | 282 | Some(ControlRequest::TransitionRequest { |
| 268 | id, | 283 | id, |
| 269 | - sender, | 284 | + sender: _sender, |
| 270 | app_id, | 285 | app_id, |
| 271 | target, | 286 | target, |
| 272 | }) => { | 287 | }) => { |
| | 288 | + let sender = match trusted_sender(reader.get_ref()) { |
| | 289 | + Ok(sender) => sender, |
| | 290 | + Err(error) => { |
| | 291 | + let mapping = map_portal_error(&PortalError::InternalFailure); |
| | 292 | + logging::warn(&format!("peer_identity_error={error}")); |
| | 293 | + return write_response( |
| | 294 | + reader.into_inner(), |
| | 295 | + ControlResponse::Error { |
| | 296 | + code: mapping.code as u32, |
| | 297 | + reason: mapping.reason.to_string(), |
| | 298 | + }, |
| | 299 | + ); |
| | 300 | + } |
| | 301 | + }; |
| | 302 | + |
| 273 | let validation = validate_request_identity(&id, &sender, app_id.as_deref()); | 303 | let validation = validate_request_identity(&id, &sender, app_id.as_deref()); |
| 274 | if let Err(error) = validation { | 304 | if let Err(error) = validation { |
| 275 | let mapping = map_portal_error(&error); | 305 | let mapping = map_portal_error(&error); |
@@ -384,6 +414,51 @@ fn quarantine_request_store(path: &Path) -> io::Result<Option<std::path::PathBuf |
| 384 | Ok(Some(quarantined)) | 414 | Ok(Some(quarantined)) |
| 385 | } | 415 | } |
| 386 | | 416 | |
| | 417 | +fn trusted_sender(stream: &UnixStream) -> io::Result<String> { |
| | 418 | + #[cfg(target_os = "linux")] |
| | 419 | + { |
| | 420 | + use std::mem; |
| | 421 | + use std::os::fd::AsRawFd; |
| | 422 | + |
| | 423 | + let fd = stream.as_raw_fd(); |
| | 424 | + let mut cred = libc::ucred { |
| | 425 | + pid: 0, |
| | 426 | + uid: 0, |
| | 427 | + gid: 0, |
| | 428 | + }; |
| | 429 | + let mut len = mem::size_of::<libc::ucred>() as libc::socklen_t; |
| | 430 | + let rc = unsafe { |
| | 431 | + libc::getsockopt( |
| | 432 | + fd, |
| | 433 | + libc::SOL_SOCKET, |
| | 434 | + libc::SO_PEERCRED, |
| | 435 | + (&mut cred as *mut libc::ucred).cast::<libc::c_void>(), |
| | 436 | + &mut len, |
| | 437 | + ) |
| | 438 | + }; |
| | 439 | + if rc == -1 { |
| | 440 | + return Err(io::Error::last_os_error()); |
| | 441 | + } |
| | 442 | + if len as usize != mem::size_of::<libc::ucred>() { |
| | 443 | + return Err(io::Error::other("invalid peer credential size")); |
| | 444 | + } |
| | 445 | + return Ok(canonical_sender_for_uid(cred.uid)); |
| | 446 | + } |
| | 447 | + |
| | 448 | + #[cfg(not(target_os = "linux"))] |
| | 449 | + { |
| | 450 | + let _ = stream; |
| | 451 | + Err(io::Error::new( |
| | 452 | + io::ErrorKind::Unsupported, |
| | 453 | + "peer credentials are unsupported on this platform", |
| | 454 | + )) |
| | 455 | + } |
| | 456 | +} |
| | 457 | + |
| | 458 | +fn canonical_sender_for_uid(uid: u32) -> String { |
| | 459 | + format!(":uid.{uid}") |
| | 460 | +} |
| | 461 | + |
| 387 | fn persist_registry_state(path: &std::path::Path, registry: &RequestRegistry) { | 462 | fn persist_registry_state(path: &std::path::Path, registry: &RequestRegistry) { |
| 388 | if let Err(error) = request_store::persist_registry(path, registry) { | 463 | if let Err(error) = request_store::persist_registry(path, registry) { |
| 389 | logging::warn(&format!("request_store_write_failed error={error}")); | 464 | logging::warn(&format!("request_store_write_failed error={error}")); |
@@ -393,8 +468,8 @@ fn persist_registry_state(path: &std::path::Path, registry: &RequestRegistry) { |
| 393 | #[cfg(test)] | 468 | #[cfg(test)] |
| 394 | mod tests { | 469 | mod tests { |
| 395 | use super::{ | 470 | use super::{ |
| 396 | - DaemonState, MAX_CONTROL_LINE_BYTES, handle_connection, load_registry_with_fallback, | 471 | + DaemonState, MAX_CONTROL_LINE_BYTES, canonical_sender_for_uid, handle_connection, |
| 397 | - load_registry_with_recovery, | 472 | + load_registry_with_fallback, load_registry_with_recovery, |
| 398 | }; | 473 | }; |
| 399 | use garwarp_ipc::{ControlResponse, HealthStatus}; | 474 | use garwarp_ipc::{ControlResponse, HealthStatus}; |
| 400 | use std::fs; | 475 | use std::fs; |
@@ -414,6 +489,10 @@ mod tests { |
| 414 | std::env::temp_dir().join(format!("garwarp-daemon-recovery-{nanos}.state")) | 489 | std::env::temp_dir().join(format!("garwarp-daemon-recovery-{nanos}.state")) |
| 415 | } | 490 | } |
| 416 | | 491 | |
| | 492 | + fn local_sender() -> String { |
| | 493 | + canonical_sender_for_uid(unsafe { libc::geteuid() }) |
| | 494 | + } |
| | 495 | + |
| 417 | #[test] | 496 | #[test] |
| 418 | fn status_request_returns_status_response() { | 497 | fn status_request_returns_status_response() { |
| 419 | let (mut client, server) = UnixStream::pair().expect("pair should be created"); | 498 | let (mut client, server) = UnixStream::pair().expect("pair should be created"); |
@@ -430,7 +509,7 @@ mod tests { |
| 430 | .requests | 509 | .requests |
| 431 | .begin_at( | 510 | .begin_at( |
| 432 | "req-1", | 511 | "req-1", |
| 433 | - RequestOwner::new(":1.2", None), | 512 | + RequestOwner::new(local_sender(), None), |
| 434 | None, | 513 | None, |
| 435 | Instant::now(), | 514 | Instant::now(), |
| 436 | ) | 515 | ) |
@@ -439,7 +518,7 @@ mod tests { |
| 439 | .requests | 518 | .requests |
| 440 | .transition( | 519 | .transition( |
| 441 | "req-1", | 520 | "req-1", |
| 442 | - &RequestOwner::new(":1.2", None), | 521 | + &RequestOwner::new(local_sender(), None), |
| 443 | RequestState::AwaitingUser, | 522 | RequestState::AwaitingUser, |
| 444 | ) | 523 | ) |
| 445 | .expect("request should transition"); | 524 | .expect("request should transition"); |
@@ -704,7 +783,7 @@ mod tests { |
| 704 | .requests | 783 | .requests |
| 705 | .begin_at( | 784 | .begin_at( |
| 706 | "req-1", | 785 | "req-1", |
| 707 | - RequestOwner::new(":1.2", None), | 786 | + RequestOwner::new(local_sender(), None), |
| 708 | Some(ParentWindowContext::X11 { window_id: 42 }), | 787 | Some(ParentWindowContext::X11 { window_id: 42 }), |
| 709 | Instant::now(), | 788 | Instant::now(), |
| 710 | ) | 789 | ) |
@@ -713,7 +792,7 @@ mod tests { |
| 713 | .requests | 792 | .requests |
| 714 | .transition( | 793 | .transition( |
| 715 | "req-1", | 794 | "req-1", |
| 716 | - &RequestOwner::new(":1.2", None), | 795 | + &RequestOwner::new(local_sender(), None), |
| 717 | RequestState::AwaitingUser, | 796 | RequestState::AwaitingUser, |
| 718 | ) | 797 | ) |
| 719 | .expect("request should transition"); | 798 | .expect("request should transition"); |
@@ -751,7 +830,7 @@ mod tests { |
| 751 | .requests | 830 | .requests |
| 752 | .begin_at( | 831 | .begin_at( |
| 753 | "req-1", | 832 | "req-1", |
| 754 | - RequestOwner::new(":1.2", None), | 833 | + RequestOwner::new(":uid.4242", None), |
| 755 | Some(ParentWindowContext::X11 { window_id: 42 }), | 834 | Some(ParentWindowContext::X11 { window_id: 42 }), |
| 756 | Instant::now(), | 835 | Instant::now(), |
| 757 | ) | 836 | ) |
@@ -832,18 +911,18 @@ mod tests { |
| 832 | } | 911 | } |
| 833 | | 912 | |
| 834 | #[test] | 913 | #[test] |
| 835 | - fn invalid_sender_maps_to_invalid_request() { | 914 | + fn payload_sender_is_ignored_for_begin() { |
| 836 | let (mut client, server) = UnixStream::pair().expect("pair should be created"); | 915 | let (mut client, server) = UnixStream::pair().expect("pair should be created"); |
| 837 | client | 916 | client |
| 838 | - .write_all(b"transition id=req-1 sender=org.test.App state=cancelled\n") | 917 | + .write_all(b"begin id=req-1 sender=org.test.App parent=x11:0x2a\n") |
| 839 | - .expect("transition request should be written"); | 918 | + .expect("begin request should be written"); |
| 840 | | 919 | |
| 841 | let mut state = DaemonState { | 920 | let mut state = DaemonState { |
| 842 | health: HealthStatus::Healthy, | 921 | health: HealthStatus::Healthy, |
| 843 | requests: RequestRegistry::new(Duration::from_secs(5)), | 922 | requests: RequestRegistry::new(Duration::from_secs(5)), |
| 844 | running: true, | 923 | running: true, |
| 845 | }; | 924 | }; |
| 846 | - handle_connection(server, &mut state).expect("transition should be handled"); | 925 | + handle_connection(server, &mut state).expect("begin should be handled"); |
| 847 | | 926 | |
| 848 | let mut response_line = String::new(); | 927 | let mut response_line = String::new(); |
| 849 | let mut reader = BufReader::new(client); | 928 | let mut reader = BufReader::new(client); |
@@ -853,11 +932,15 @@ mod tests { |
| 853 | let response = ControlResponse::parse_line(&response_line).expect("response should parse"); | 932 | let response = ControlResponse::parse_line(&response_line).expect("response should parse"); |
| 854 | assert_eq!( | 933 | assert_eq!( |
| 855 | response, | 934 | response, |
| 856 | - ControlResponse::Error { | 935 | + ControlResponse::AckRequest { |
| 857 | - code: 2, | 936 | + id: "req-1".to_string(), |
| 858 | - reason: "invalid_request".to_string(), | 937 | + state: "pending".to_string(), |
| 859 | } | 938 | } |
| 860 | ); | 939 | ); |
| | 940 | + assert_eq!( |
| | 941 | + state.requests.owner("req-1").map(|owner| owner.sender), |
| | 942 | + Some(local_sender()) |
| | 943 | + ); |
| 861 | } | 944 | } |
| 862 | | 945 | |
| 863 | #[test] | 946 | #[test] |
@@ -876,7 +959,7 @@ mod tests { |
| 876 | .requests | 959 | .requests |
| 877 | .begin_at( | 960 | .begin_at( |
| 878 | "req-1", | 961 | "req-1", |
| 879 | - RequestOwner::new(":1.2", None), | 962 | + RequestOwner::new(":uid.4242", None), |
| 880 | Some(ParentWindowContext::X11 { window_id: 42 }), | 963 | Some(ParentWindowContext::X11 { window_id: 42 }), |
| 881 | Instant::now(), | 964 | Instant::now(), |
| 882 | ) | 965 | ) |
@@ -915,7 +998,7 @@ mod tests { |
| 915 | .requests | 998 | .requests |
| 916 | .begin_at( | 999 | .begin_at( |
| 917 | "req-1", | 1000 | "req-1", |
| 918 | - RequestOwner::new(":1.2", None), | 1001 | + RequestOwner::new(local_sender(), None), |
| 919 | None, | 1002 | None, |
| 920 | Instant::now(), | 1003 | Instant::now(), |
| 921 | ) | 1004 | ) |
@@ -924,7 +1007,7 @@ mod tests { |
| 924 | .requests | 1007 | .requests |
| 925 | .transition( | 1008 | .transition( |
| 926 | "req-1", | 1009 | "req-1", |
| 927 | - &RequestOwner::new(":1.2", None), | 1010 | + &RequestOwner::new(local_sender(), None), |
| 928 | RequestState::Cancelled, | 1011 | RequestState::Cancelled, |
| 929 | ) | 1012 | ) |
| 930 | .expect("first cancel should transition"); | 1013 | .expect("first cancel should transition"); |
@@ -962,7 +1045,7 @@ mod tests { |
| 962 | .requests | 1045 | .requests |
| 963 | .begin_at( | 1046 | .begin_at( |
| 964 | "req-1", | 1047 | "req-1", |
| 965 | - RequestOwner::new(":1.2", None), | 1048 | + RequestOwner::new(local_sender(), None), |
| 966 | None, | 1049 | None, |
| 967 | Instant::now(), | 1050 | Instant::now(), |
| 968 | ) | 1051 | ) |
@@ -971,7 +1054,7 @@ mod tests { |
| 971 | .requests | 1054 | .requests |
| 972 | .transition( | 1055 | .transition( |
| 973 | "req-1", | 1056 | "req-1", |
| 974 | - &RequestOwner::new(":1.2", None), | 1057 | + &RequestOwner::new(local_sender(), None), |
| 975 | RequestState::AwaitingUser, | 1058 | RequestState::AwaitingUser, |
| 976 | ) | 1059 | ) |
| 977 | .expect("first awaiting_user should transition"); | 1060 | .expect("first awaiting_user should transition"); |
@@ -1012,7 +1095,7 @@ mod tests { |
| 1012 | .requests | 1095 | .requests |
| 1013 | .begin_at( | 1096 | .begin_at( |
| 1014 | "req-1", | 1097 | "req-1", |
| 1015 | - RequestOwner::new(":1.2", Some("org.test.App".to_string())), | 1098 | + RequestOwner::new(local_sender(), Some("org.test.App".to_string())), |
| 1016 | Some(ParentWindowContext::X11 { window_id: 42 }), | 1099 | Some(ParentWindowContext::X11 { window_id: 42 }), |
| 1017 | Instant::now(), | 1100 | Instant::now(), |
| 1018 | ) | 1101 | ) |
@@ -1021,7 +1104,7 @@ mod tests { |
| 1021 | .requests | 1104 | .requests |
| 1022 | .transition( | 1105 | .transition( |
| 1023 | "req-1", | 1106 | "req-1", |
| 1024 | - &RequestOwner::new(":1.2", Some("org.test.App".to_string())), | 1107 | + &RequestOwner::new(local_sender(), Some("org.test.App".to_string())), |
| 1025 | RequestState::AwaitingUser, | 1108 | RequestState::AwaitingUser, |
| 1026 | ) | 1109 | ) |
| 1027 | .expect("request should transition"); | 1110 | .expect("request should transition"); |
@@ -1038,7 +1121,7 @@ mod tests { |
| 1038 | ControlResponse::RequestSnapshot { | 1121 | ControlResponse::RequestSnapshot { |
| 1039 | id: "req-1".to_string(), | 1122 | id: "req-1".to_string(), |
| 1040 | state: "awaiting_user".to_string(), | 1123 | state: "awaiting_user".to_string(), |
| 1041 | - sender: ":1.2".to_string(), | 1124 | + sender: local_sender(), |
| 1042 | app_id: Some("org.test.App".to_string()), | 1125 | app_id: Some("org.test.App".to_string()), |
| 1043 | parent_window: Some("x11:0x2a".to_string()), | 1126 | parent_window: Some("x11:0x2a".to_string()), |
| 1044 | } | 1127 | } |