Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions crates/sbm_ffi/src/api/remote_desktop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,8 @@ impl RemoteDesktopSessionHandle {
pub fn start_vnc(params: VncSessionParams) -> Result<Self, String> {
validate_endpoint(&params.connect_host, params.connect_port)?;
if let Some(password) = &params.password {
if !password.is_ascii() || password.len() > 8 {
return Err("VNC passwords must contain at most 8 ASCII bytes".to_owned());
if sbm_parser::desktop::validate_vnc_password(password).is_err() {
return Err("VNC passwords must be ASCII".to_owned());
}
}
Self::spawn(move |commands, events| run_vnc(params, commands, events))
Expand Down Expand Up @@ -1176,7 +1176,14 @@ async fn run_vnc(
return;
}
}
let password = params.password.unwrap_or_default();
// Keyed by its first eight bytes only, as every classic VNC client does;
// said here rather than left to the library's own key derivation.
let password = params
.password
.as_deref()
.map(sbm_parser::desktop::vnc_auth_password)
.unwrap_or_default()
.to_owned();
let shared = params.shared;
let handshake = async move {
let state = VncConnector::new(stream)
Expand Down Expand Up @@ -1899,12 +1906,12 @@ mod tests {
}

#[test]
fn vnc_password_limit_is_enforced_before_spawning() {
fn a_non_ascii_vnc_password_is_refused_before_spawning() {
let result = RemoteDesktopSessionHandle::start_vnc(VncSessionParams {
connect_host: "127.0.0.1".to_owned(),
connect_port: 5900,
access_token: None,
password: Some("123456789".to_owned()),
password: Some("密码".to_owned()),
shared: true,
});
assert!(result.is_err());
Expand Down
32 changes: 21 additions & 11 deletions crates/sbm_parser/src/desktop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ pub const MAX_NAME: usize = 64;
pub const MAX_HOST: usize = 253;
/// A user name or a domain.
pub const MAX_IDENT: usize = 256;
/// Classic VNC authentication is DES keyed by the first eight bytes.
pub const MAX_VNC_PASSWORD: usize = 8;
/// Classic VNC authentication is DES keyed by the first eight bytes; the rest
/// of a longer password takes no part in it.
pub const VNC_PASSWORD_KEY_LEN: usize = 8;

/// The fields of a route that have rules. The port is an `i64` so a form can
/// pass what was typed, out of range included, and be told so.
Expand Down Expand Up @@ -75,14 +76,12 @@ impl ProfileError {
/// Why a classic VNC password would not work.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum VncPasswordError {
TooLong,
NotAscii,
}

impl VncPasswordError {
pub fn as_str(self) -> &'static str {
match self {
VncPasswordError::TooLong => "vncPasswordLength",
VncPasswordError::NotAscii => "vncPasswordAscii",
}
}
Expand Down Expand Up @@ -139,19 +138,30 @@ pub fn validate_profile(input: &ProfileInput) -> Result<(), ProfileError> {
Ok(())
}

/// Classic VNC authentication: at most eight bytes, ASCII only. Counted in
/// UTF-16 units first, as the Dart editor did, so a long non-ASCII password
/// is reported as too long rather than as non-ASCII.
/// Classic VNC authentication: ASCII only. Any length is accepted, as
/// TigerVNC's own tools accept it; only the first [VNC_PASSWORD_KEY_LEN]
/// bytes are used ([vnc_auth_password]). Refusing a longer one (#1637) left
/// a server set up with `vncpasswd` unreachable by its owner's password.
pub fn validate_vnc_password(password: &str) -> Result<(), VncPasswordError> {
if password.encode_utf16().count() > MAX_VNC_PASSWORD {
return Err(VncPasswordError::TooLong);
}
if !password.is_ascii() {
return Err(VncPasswordError::NotAscii);
}
Ok(())
}

/// What classic VNC authentication is keyed by: the first
/// [VNC_PASSWORD_KEY_LEN] bytes of [password], which [validate_vnc_password]
/// has made sure are whole characters.
pub fn vnc_auth_password(password: &str) -> &str {
let end = password
.char_indices()
.map(|(i, c)| i + c.len_utf8())
.take_while(|&end| end <= VNC_PASSWORD_KEY_LEN)
.last()
.unwrap_or(0);
&password[..end]
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -203,6 +213,6 @@ mod tests {
#[test]
fn the_codes_are_the_ones_clients_phrase() {
assert_eq!(ProfileError::UsernameRequired.as_str(), "usernameRequired");
assert_eq!(VncPasswordError::TooLong.as_str(), "vncPasswordLength");
assert_eq!(VncPasswordError::NotAscii.as_str(), "vncPasswordAscii");
}
}
18 changes: 13 additions & 5 deletions crates/sbm_parser/tests/desktop_compat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! before its Dart validator was replaced ("test as spec").

use sbm_parser::desktop::{
ProfileError, ProfileInput, VncPasswordError, validate_profile, validate_vnc_password,
ProfileError, ProfileInput, VncPasswordError, validate_profile, validate_vnc_password, vnc_auth_password,
};

#[test]
Expand All @@ -20,10 +20,18 @@ fn profile_form_requires_the_rdp_username() {
}

#[test]
fn classic_vnc_passwords_are_at_most_eight_ascii_bytes() {
fn classic_vnc_passwords_are_ascii_of_any_length() {
assert_eq!(validate_vnc_password("12345678"), Ok(()));
assert_eq!(validate_vnc_password("123456789"), Err(VncPasswordError::TooLong));
// Longer than the key: accepted, as TigerVNC accepts it (#1637).
assert_eq!(validate_vnc_password("123456789abc"), Ok(()));
assert_eq!(validate_vnc_password("密码"), Err(VncPasswordError::NotAscii));
// The Dart editor checked the length first, in UTF-16 units.
assert_eq!(validate_vnc_password("密码密码密码密码密"), Err(VncPasswordError::TooLong));
assert_eq!(validate_vnc_password("密码密码密码密码密"), Err(VncPasswordError::NotAscii));
}

#[test]
fn classic_vnc_authentication_uses_the_first_eight_bytes() {
assert_eq!(vnc_auth_password("123456789abc"), "12345678");
assert_eq!(vnc_auth_password("12345678"), "12345678");
assert_eq!(vnc_auth_password("short"), "short");
assert_eq!(vnc_auth_password(""), "");
}
3 changes: 2 additions & 1 deletion docs/src/content/docs/advanced/remote-desktop.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ return to the app.
resolution, and text CLIPRDR are supported. Username/password authentication
with an optional domain primarily uses NTLM; Kerberos is not guaranteed.
- **VNC:** RFB 3.3, 3.7, and 3.8 are supported with None or classic VNC
Authentication. Classic VNC passwords are limited to eight ASCII bytes.
Authentication. Classic VNC passwords are ASCII; a longer one is accepted
and, as in TigerVNC, only its first eight characters are used.
Supported updates include Raw, CopyRect, ZRLE, Tight, Tight JPEG, cursor, and
desktop size.
- VNC does not support VeNCrypt, SASL, Apple Remote Desktop authentication, or
Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/zh/advanced/remote-desktop.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ Server Box 会拒绝发送,避免字符损坏。RDP 可以根据查看器调
- **RDP:**支持 TCP、TLS、CredSSP/NLA、图形、键盘与指针输入、动态分辨率和文本 CLIPRDR。
用户名/密码和可选域主要使用 NTLM;不保证支持 Kerberos。
- **VNC:**支持 RFB 3.3、3.7、3.8,以及 None 和经典 VNC Authentication。经典 VNC 密码
最多为 8 个 ASCII 字节。支持 Raw、CopyRect、ZRLE、Tight、Tight JPEG、光标和桌面尺寸更新。
只能是 ASCII;可以输入更长的密码,与 TigerVNC 一样只使用前 8 个字符。支持 Raw、CopyRect、ZRLE、Tight、Tight JPEG、光标和桌面尺寸更新。
- VNC 暂不支持 VeNCrypt、SASL、Apple Remote Desktop 认证和 RealVNC 私有认证。
- 暂不支持音频、麦克风、多显示器、RemoteApp、RDP Gateway、UDP,以及文件、磁盘、打印机、
USB 或智能卡重定向。
Expand Down
6 changes: 5 additions & 1 deletion lib/core/sync.dart
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,11 @@ final class BakSyncer extends SyncIface {
if (includeSettings) return mergeable;

return switch (mergeable) {
final BackupV2 backup => backup.copyWith(settings: const {}),
// The themes go with the settings: they are what the settings select.
final BackupV2 backup => backup.copyWith(
settings: const {},
themes: const {},
),
final Backup backup => Backup(
version: backup.version,
date: backup.date,
Expand Down
36 changes: 36 additions & 0 deletions lib/data/model/app/bak/backup2.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'dart:io';
import 'dart:typed_data';

import 'package:fl_lib/fl_lib.dart';
import 'package:fl_lib/theme.dart' show ThemeBackup, ThemeRestoreReport;
import 'package:freezed_annotation/freezed_annotation.dart';
import 'package:logging/logging.dart';
import 'package:server_box/core/utils/local_file_backend.dart';
Expand Down Expand Up @@ -86,6 +87,12 @@ abstract class BackupV2 with _$BackupV2 implements Mergeable {
/// every file this build writes, for those builds to read. What [merge]
/// makes of either is `_pveToRestore`'s rule.
@Default(<String, Object?>{}) Map<String, Object?> pve,

/// The installed theme packages, by installation id — see
/// [ThemeBackup]. Only beside [settings]: they are what the theme
/// settings point at. Absent from files written before themes were
/// carried (#1637), whose theme settings restore onto the default.
@Default(<String, Object?>{}) Map<String, Object?> themes,
}) = _BackupV2;

/// Must stay a single expression with a cascade, not a block body.
Expand All @@ -100,9 +107,23 @@ abstract class BackupV2 with _$BackupV2 implements Mergeable {

@override
Future<void> merge({bool force = false}) async {
await mergeReporting(force: force);
}

/// [merge], answering what became of [themes]: which came back, and which
/// could not — for a restore the user started to say so.
Future<ThemeRestoreReport?> mergeReporting({bool force = false}) async {
_validateRestorableTypedStores();
_loggerV2.info('Merging...');

// Before the settings that select one: the network is not something to
// hold the transaction open for, and a selection is only restored onto a
// theme that is there.
final themeReport = themes.isEmpty ? null : await ThemeBackup.restore(themes);
if (themeReport != null && themeReport.failed.isNotEmpty) {
_loggerV2.warning('Themes not restored: ${themeReport.failed}');
}

late bool keysChanged;
late bool credsChanged;
late bool serversChanged;
Expand Down Expand Up @@ -264,7 +285,12 @@ abstract class BackupV2 with _$BackupV2 implements Mergeable {
}
if (containerChanged) GlobalRef.gRef?.invalidate(containerProvider);

// After the settings: it points what they restored at this device's
// installations, and falls back where a theme did not come back.
if (themeReport != null) ThemeBackup.reselect(themeReport);

_loggerV2.info('Merge completed');
return themeReport;
}

/// Envelope version. Bumped to 3 with the nested `Spi.ssh` layout, so a
Expand Down Expand Up @@ -294,9 +320,19 @@ abstract class BackupV2 with _$BackupV2 implements Mergeable {
deviceLocal: SettingStore.deviceLocalKeys,
)
: const {},
themes: includeSettings ? await _themes() : const {},
);
}

static Future<Map<String, Object?>> _themes() async {
final export = await ThemeBackup.export();
if (export.skipped.isNotEmpty) {
// Listed with nothing to install them from; the restore names them.
_loggerV2.warning('Themes without a package to carry: ${export.skipped}');
}
return export.entries;
}

static Future<String> backup([
String? name,
String? password,
Expand Down
Loading
Loading