From ceedc314d7ffcc76c8559d06679a085a821ffaf1 Mon Sep 17 00:00:00 2001 From: 21pages Date: Sun, 8 Nov 2026 23:51:33 +0800 Subject: [PATCH] fix: block sensitive config options in strategy updates (#16696) * fix: block sensitive heartbeat strategy options Filter 25 sensitive server, trust, authentication, access-control, deployment and local service settings before merging heartbeat strategy options. Preserve the existing HTTP protocol, all 27 current server policy options, and other unblocked options for version compatibility. Strategy responses remain unauthenticated. Add regression coverage for blocked writes and clears, including registry recovery data, and preservation of existing and future policy options. Tests: cargo test --lib hbbs_http::sync::strategy_tests --locked (2 passed) * fix: log rejected strategy options and strengthen regression tests * fix: block trusted-device expiration in strategy updates A strategy setting it to 0 would make trusted devices skip 2FA forever. Co-Authored-By: Claude Opus 5.5 * fix: block websocket port in heartbeat strategies * fix: preserve security policy controls in heartbeat strategies Allow the connection-window and scope-violation policy controls. Keep enable-trusted-devices blocked because disabling it clears existing trusted-device grants, which re-enabling does not restore. * fix: acknowledge heartbeat strategies after application errors Skipping modified_at after an application error allows the server to deliver the same strategy version repeatedly. Authentication options (2FA, trusted-device enablement and expiration) are already blocked, and the authentication-update path applies unrelated options first. Keep reporting application errors, but continue processing modified_at so the failed version is acknowledged. Log "strategy updated" only on success. Acknowledgement does not guarantee full application; retrying a failed strategy requires publishing a new version. Validation: cargo test --lib hbbs_http::sync::strategy_tests --locked (2 passed; filter coverage only, no live heartbeat verification). * Revert "fix: acknowledge heartbeat strategies after application errors" This reverts commit a03d235236955f2e75f505b88b04a9df63559b78. * test: fix future strategy option spelling --------- Co-authored-by: rustdesk <71636191+rustdesk@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- src/hbbs_http/sync.rs | 150 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 149 insertions(+), 1 deletion(-) diff --git a/src/hbbs_http/sync.rs b/src/hbbs_http/sync.rs index aac220b9e..c934a79d7 100644 --- a/src/hbbs_http/sync.rs +++ b/src/hbbs_http/sync.rs @@ -292,9 +292,58 @@ fn heartbeat_url() -> String { format!("{}/api/heartbeat", url) } +const BLOCKED_CONFIG_OPTIONS: &[&str] = &[ + // Server endpoints and trust. + keys::OPTION_CUSTOM_RENDEZVOUS_SERVER, + "rendezvous-servers", + keys::OPTION_API_SERVER, + keys::OPTION_RELAY_SERVER, + keys::OPTION_KEY, + keys::OPTION_ICE_SERVERS, + // Transport security. + keys::OPTION_ALLOW_INSECURE_TLS_FALLBACK, + keys::OPTION_ALLOW_WEBSOCKET, + keys::OPTION_WEBSOCKET_PORT, + // Authentication and credentials. + "2fa", + "bot", + keys::OPTION_PRESET_ADDRESS_BOOK_PASSWORD, + keys::OPTION_ENABLE_TRUSTED_DEVICES, + keys::OPTION_TRUSTED_DEVICE_EXPIRATION, + // Deployment and address book metadata. + keys::OPTION_PRESET_ADDRESS_BOOK_NAME, + keys::OPTION_PRESET_ADDRESS_BOOK_TAG, + keys::OPTION_PRESET_ADDRESS_BOOK_ALIAS, + keys::OPTION_PRESET_ADDRESS_BOOK_NOTE, + keys::OPTION_PRESET_DEVICE_NAME, + keys::OPTION_PRESET_DEVICE_USERNAME, + keys::OPTION_PRESET_NOTE, + // Recording location and service state. + keys::OPTION_WINDOWS_SERVICE_VIDEO_SAVE_DIRECTORY, + "stop-service", + "reg_recovery", +]; + +fn filter_config_options(options: &mut HashMap) { + let rejected: Vec<_> = BLOCKED_CONFIG_OPTIONS + .iter() + .copied() + .filter(|key| options.contains_key(*key)) + .collect(); + if !rejected.is_empty() { + hbb_common::throttled_log!( + crate::common::API_LOG_INTERVAL, + warn, + "Rejected heartbeat strategy options: {rejected:?}" + ); + } + options.retain(|key, _| !BLOCKED_CONFIG_OPTIONS.contains(&key.as_str())); +} + async fn handle_config_options( - config_options: HashMap, + mut config_options: HashMap, ) -> hbb_common::ResultType<()> { + filter_config_options(&mut config_options); let mut options = Config::get_options(); let default_settings = config::DEFAULT_SETTINGS.read().unwrap().clone(); config_options @@ -472,3 +521,102 @@ mod tests { assert_eq!(switch_grant_signed_msg("id1", "c1", "1700000000"), expected); } } + +#[cfg(test)] +mod strategy_tests { + use super::{filter_config_options, HashMap}; + + #[test] + fn strategy_cannot_replace_or_clear_server_trust_and_credentials() { + for value in ["attacker", "Y", "N", ""] { + let mut options = [ + "custom-rendezvous-server", + "rendezvous-servers", + "api-server", + "relay-server", + "key", + "ice-servers", + "allow-insecure-tls-fallback", + "allow-websocket", + "websocket-port", + "2fa", + "bot", + "preset-address-book-password", + "enable-trusted-devices", + "trusted-device-expiration-days", + "preset-address-book-name", + "preset-address-book-tag", + "preset-address-book-alias", + "preset-address-book-note", + "preset-device-name", + "preset-device-username", + "preset-note", + "windows-service-video-save-directory", + "stop-service", + "reg_recovery", + ] + .into_iter() + .map(|key| (key.to_owned(), value.to_owned())) + .collect::>(); + options.insert("enable-clipboard".to_owned(), "N".to_owned()); + options.insert("allow-auto-disconnect".to_owned(), String::new()); + filter_config_options(&mut options); + assert_eq!( + options, + HashMap::from([ + ("enable-clipboard".to_owned(), "N".to_owned()), + ("allow-auto-disconnect".to_owned(), String::new()), + ]) + ); + } + } + + #[test] + fn existing_and_unknown_policy_options_are_preserved() { + let policy_keys = [ + // Existing server strategy options. + "access-mode", + "enable-keyboard", + "enable-remote-printer", + "enable-clipboard", + "enable-file-transfer", + "enable-audio", + "enable-terminal", + "enable-tunnel", + "enable-remote-restart", + "enable-record-session", + "enable-block-input", + "enable-privacy-mode", + "enable-camera", + "allow-remote-config-modification", + "approve-mode", + "verification-method", + "temporary-password-length", + "allow-numeric-one-time-password", + "allow-hide-cm", + "enable-lan-discovery", + "direct-server", + "direct-access-port", + "whitelist", + "id-whitelist", + "allow-auto-disconnect", + "auto-disconnect-timeout", + "allow-remove-wallpaper", + // Security policy controls. + "allow-only-conn-window-open", + "allow-scope-violation-close", + "allow-scope-violation-alarm", + // Future strategy option. + "future-strategy-option", + ]; + for value in ["Y", "N", ""] { + let mut options: HashMap<_, _> = policy_keys + .into_iter() + .map(|key| (key.to_owned(), value.to_owned())) + .collect(); + let expected = options.clone(); + filter_config_options(&mut options); + assert_eq!(options, expected); + } + } +}