@@ -3,15 +3,16 @@ use std::io::{BufRead, BufReader, ErrorKind, Write}; |
| 3 | 3 | use std::os::unix::fs::{MetadataExt, PermissionsExt}; |
| 4 | 4 | use std::os::unix::net::UnixStream; |
| 5 | 5 | use std::path::{Path, PathBuf}; |
| 6 | | -use std::process::{Command, Stdio}; |
| 6 | +use std::process::{Child, Command, ExitStatus, Stdio}; |
| 7 | 7 | use std::sync::{Arc, Mutex}; |
| 8 | | -use std::time::Duration; |
| 8 | +use std::time::{Duration, Instant}; |
| 9 | 9 | |
| 10 | 10 | pub const DEFAULT_HELPER_SOCKET: &str = "/run/polkit/agent-helper.socket"; |
| 11 | 11 | const HELPER_TRANSPORT_ENV: &str = "GARCARD_POLKIT_HELPER_TRANSPORT"; |
| 12 | 12 | const HELPER_SOCKET_PROTOCOL_ENV: &str = "GARCARD_POLKIT_SOCKET_PROTOCOL"; |
| 13 | 13 | const HELPER_CONVERSATION_BACKEND_ENV: &str = "GARCARD_POLKIT_CONVERSATION_BACKEND"; |
| 14 | 14 | const SOCKET_FIRST_RESPONSE_TIMEOUT: Duration = Duration::from_millis(1500); |
| 15 | +const HELPER_EXIT_GRACE_TIMEOUT: Duration = Duration::from_millis(500); |
| 15 | 16 | |
| 16 | 17 | #[derive(Debug, Clone, Copy, PartialEq, Eq)] |
| 17 | 18 | pub enum HelperOutcome { |
@@ -520,10 +521,6 @@ impl HelperSocketClient { |
| 520 | 521 | .stdout |
| 521 | 522 | .take() |
| 522 | 523 | .context("failed to capture helper process stdout")?; |
| 523 | | - let stdin = child |
| 524 | | - .stdin |
| 525 | | - .as_mut() |
| 526 | | - .context("failed to capture helper process stdin")?; |
| 527 | 524 | let mut reader = BufReader::new(stdout); |
| 528 | 525 | |
| 529 | 526 | loop { |
@@ -565,13 +562,31 @@ impl HelperSocketClient { |
| 565 | 562 | "Submitting secret prompt response to direct helper" |
| 566 | 563 | ); |
| 567 | 564 | let mut sanitized = sanitize_response(&response); |
| 568 | | - write_line(stdin, &sanitized) |
| 569 | | - .context("failed to send direct helper secret response")?; |
| 565 | + { |
| 566 | + let stdin = child |
| 567 | + .stdin |
| 568 | + .as_mut() |
| 569 | + .context("failed to capture helper process stdin")?; |
| 570 | + write_line(stdin, &sanitized) |
| 571 | + .context("failed to send direct helper secret response")?; |
| 572 | + } |
| 570 | 573 | scrub_string(&mut sanitized); |
| 571 | 574 | scrub_string(&mut response); |
| 572 | 575 | } |
| 573 | | - PromptResponse::Canceled => return Ok(HelperOutcome::Canceled), |
| 574 | | - PromptResponse::TimedOut => return Ok(HelperOutcome::Timeout), |
| 576 | + PromptResponse::Canceled => { |
| 577 | + return finalize_direct_helper_outcome( |
| 578 | + &mut child, |
| 579 | + helper, |
| 580 | + HelperOutcome::Canceled, |
| 581 | + ); |
| 582 | + } |
| 583 | + PromptResponse::TimedOut => { |
| 584 | + return finalize_direct_helper_outcome( |
| 585 | + &mut child, |
| 586 | + helper, |
| 587 | + HelperOutcome::Timeout, |
| 588 | + ); |
| 589 | + } |
| 575 | 590 | } |
| 576 | 591 | } |
| 577 | 592 | HelperEvent::PromptVisible(prompt) => { |
@@ -585,13 +600,31 @@ impl HelperSocketClient { |
| 585 | 600 | "Submitting visible prompt response to direct helper" |
| 586 | 601 | ); |
| 587 | 602 | let mut sanitized = sanitize_response(&response); |
| 588 | | - write_line(stdin, &sanitized) |
| 589 | | - .context("failed to send direct helper visible response")?; |
| 603 | + { |
| 604 | + let stdin = child |
| 605 | + .stdin |
| 606 | + .as_mut() |
| 607 | + .context("failed to capture helper process stdin")?; |
| 608 | + write_line(stdin, &sanitized) |
| 609 | + .context("failed to send direct helper visible response")?; |
| 610 | + } |
| 590 | 611 | scrub_string(&mut sanitized); |
| 591 | 612 | scrub_string(&mut response); |
| 592 | 613 | } |
| 593 | | - PromptResponse::Canceled => return Ok(HelperOutcome::Canceled), |
| 594 | | - PromptResponse::TimedOut => return Ok(HelperOutcome::Timeout), |
| 614 | + PromptResponse::Canceled => { |
| 615 | + return finalize_direct_helper_outcome( |
| 616 | + &mut child, |
| 617 | + helper, |
| 618 | + HelperOutcome::Canceled, |
| 619 | + ); |
| 620 | + } |
| 621 | + PromptResponse::TimedOut => { |
| 622 | + return finalize_direct_helper_outcome( |
| 623 | + &mut child, |
| 624 | + helper, |
| 625 | + HelperOutcome::Timeout, |
| 626 | + ); |
| 627 | + } |
| 595 | 628 | } |
| 596 | 629 | } |
| 597 | 630 | HelperEvent::Error(message) => { |
@@ -608,13 +641,21 @@ impl HelperSocketClient { |
| 608 | 641 | prompts |
| 609 | 642 | .auth_succeeded() |
| 610 | 643 | .context("prompt success callback failed")?; |
| 611 | | - return Ok(HelperOutcome::Authorized); |
| 644 | + return finalize_direct_helper_outcome( |
| 645 | + &mut child, |
| 646 | + helper, |
| 647 | + HelperOutcome::Authorized, |
| 648 | + ); |
| 612 | 649 | } |
| 613 | 650 | HelperEvent::Failure => { |
| 614 | 651 | prompts |
| 615 | 652 | .auth_failed("Authentication failed") |
| 616 | 653 | .context("prompt failure callback failed")?; |
| 617 | | - return Ok(HelperOutcome::Denied); |
| 654 | + return finalize_direct_helper_outcome( |
| 655 | + &mut child, |
| 656 | + helper, |
| 657 | + HelperOutcome::Denied, |
| 658 | + ); |
| 618 | 659 | } |
| 619 | 660 | } |
| 620 | 661 | } |
@@ -675,6 +716,90 @@ fn write_line(stream: &mut impl Write, value: &str) -> Result<()> { |
| 675 | 716 | Ok(()) |
| 676 | 717 | } |
| 677 | 718 | |
| 719 | +fn finalize_direct_helper_outcome( |
| 720 | + child: &mut Child, |
| 721 | + helper: &Path, |
| 722 | + outcome: HelperOutcome, |
| 723 | +) -> Result<HelperOutcome> { |
| 724 | + match outcome { |
| 725 | + HelperOutcome::Canceled | HelperOutcome::Timeout => { |
| 726 | + terminate_direct_helper_process(child, helper, "prompt canceled before completion")?; |
| 727 | + } |
| 728 | + HelperOutcome::Authorized | HelperOutcome::Denied => { |
| 729 | + if let Some(status) = wait_for_helper_exit(child, HELPER_EXIT_GRACE_TIMEOUT)? { |
| 730 | + tracing::debug!( |
| 731 | + helper = %helper.display(), |
| 732 | + status = %status, |
| 733 | + "Direct helper exited after terminal outcome" |
| 734 | + ); |
| 735 | + } else { |
| 736 | + tracing::warn!( |
| 737 | + helper = %helper.display(), |
| 738 | + grace_ms = HELPER_EXIT_GRACE_TIMEOUT.as_millis(), |
| 739 | + "Direct helper did not exit during grace period; terminating process" |
| 740 | + ); |
| 741 | + terminate_direct_helper_process( |
| 742 | + child, |
| 743 | + helper, |
| 744 | + "helper did not exit after terminal outcome", |
| 745 | + )?; |
| 746 | + } |
| 747 | + } |
| 748 | + } |
| 749 | + |
| 750 | + Ok(outcome) |
| 751 | +} |
| 752 | + |
| 753 | +fn wait_for_helper_exit(child: &mut Child, timeout: Duration) -> Result<Option<ExitStatus>> { |
| 754 | + let deadline = Instant::now() + timeout; |
| 755 | + loop { |
| 756 | + if let Some(status) = child |
| 757 | + .try_wait() |
| 758 | + .context("failed to poll direct helper process state")? |
| 759 | + { |
| 760 | + return Ok(Some(status)); |
| 761 | + } |
| 762 | + if Instant::now() >= deadline { |
| 763 | + return Ok(None); |
| 764 | + } |
| 765 | + std::thread::sleep(Duration::from_millis(10)); |
| 766 | + } |
| 767 | +} |
| 768 | + |
| 769 | +fn terminate_direct_helper_process(child: &mut Child, helper: &Path, reason: &str) -> Result<()> { |
| 770 | + if child |
| 771 | + .try_wait() |
| 772 | + .context("failed to poll direct helper before termination")? |
| 773 | + .is_some() |
| 774 | + { |
| 775 | + return Ok(()); |
| 776 | + } |
| 777 | + |
| 778 | + let _ = child.stdin.take(); |
| 779 | + let _ = child.stdout.take(); |
| 780 | + let _ = child.stderr.take(); |
| 781 | + match child.kill() { |
| 782 | + Ok(()) => {} |
| 783 | + Err(err) if err.kind() == ErrorKind::InvalidInput => return Ok(()), |
| 784 | + Err(err) => { |
| 785 | + return Err(anyhow::Error::new(err).context(format!( |
| 786 | + "failed to terminate direct helper process ({})", |
| 787 | + reason |
| 788 | + ))); |
| 789 | + } |
| 790 | + } |
| 791 | + let status = child |
| 792 | + .wait() |
| 793 | + .context("failed waiting for direct helper process termination")?; |
| 794 | + tracing::debug!( |
| 795 | + helper = %helper.display(), |
| 796 | + status = %status, |
| 797 | + reason, |
| 798 | + "Terminated direct helper process" |
| 799 | + ); |
| 800 | + Ok(()) |
| 801 | +} |
| 802 | + |
| 678 | 803 | fn sanitize_response(raw: &str) -> String { |
| 679 | 804 | raw.lines().collect::<Vec<_>>().join(" ") |
| 680 | 805 | } |
@@ -1567,6 +1692,55 @@ echo "SUCCESS" |
| 1567 | 1692 | let _ = std::fs::remove_file(&helper_path); |
| 1568 | 1693 | } |
| 1569 | 1694 | |
| 1695 | + #[test] |
| 1696 | + fn direct_helper_canceled_prompt_terminates_helper_process() { |
| 1697 | + let helper_path = temp_helper_path(); |
| 1698 | + let helper_script = r#"#!/usr/bin/env bash |
| 1699 | +pidfile="${0}.pid" |
| 1700 | +echo "$$" > "$pidfile" |
| 1701 | +echo "PAM_PROMPT_ECHO_OFF Password:" |
| 1702 | +read -r _response |
| 1703 | +echo "SUCCESS" |
| 1704 | +"#; |
| 1705 | + std::fs::write(&helper_path, helper_script).expect("write helper script"); |
| 1706 | + let mut perms = std::fs::metadata(&helper_path) |
| 1707 | + .expect("helper metadata") |
| 1708 | + .permissions(); |
| 1709 | + perms.set_mode(0o755); |
| 1710 | + std::fs::set_permissions(&helper_path, perms).expect("set helper executable"); |
| 1711 | + |
| 1712 | + let pid_path = PathBuf::from(format!("{}.pid", helper_path.to_string_lossy())); |
| 1713 | + let client = HelperSocketClient::new(temp_socket_path()); |
| 1714 | + let mut prompts = FakePrompt { |
| 1715 | + secret_response: PromptResponse::Canceled, |
| 1716 | + ..FakePrompt::default() |
| 1717 | + }; |
| 1718 | + |
| 1719 | + let outcome = client |
| 1720 | + .authenticate_via_helper_process_with_helper( |
| 1721 | + &helper_path, |
| 1722 | + "operator", |
| 1723 | + "cookie-cancel", |
| 1724 | + &mut prompts, |
| 1725 | + ) |
| 1726 | + .expect("authenticate canceled"); |
| 1727 | + assert_eq!(outcome, HelperOutcome::Canceled); |
| 1728 | + |
| 1729 | + let helper_pid = std::fs::read_to_string(&pid_path) |
| 1730 | + .expect("read helper pid") |
| 1731 | + .trim() |
| 1732 | + .parse::<u32>() |
| 1733 | + .expect("parse helper pid"); |
| 1734 | + std::thread::sleep(Duration::from_millis(25)); |
| 1735 | + assert!( |
| 1736 | + !PathBuf::from(format!("/proc/{}", helper_pid)).exists(), |
| 1737 | + "helper process should be terminated after canceled prompt" |
| 1738 | + ); |
| 1739 | + |
| 1740 | + let _ = std::fs::remove_file(&helper_path); |
| 1741 | + let _ = std::fs::remove_file(&pid_path); |
| 1742 | + } |
| 1743 | + |
| 1570 | 1744 | #[test] |
| 1571 | 1745 | fn scrub_string_clears_input() { |
| 1572 | 1746 | let mut value = "top-secret".to_string(); |