From 63990edc427b9a69c69c64eed2a453d75e190944 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 12:28:17 +0000 Subject: [PATCH 1/2] Harden local key file writes and move remote file access to SFTP Local: - KeyFileWriterService: truncate on overwrite (FileMode.Create), CreateNew otherwise, FileShare.None, and enforce 0600 on overwritten files since UnixCreateMode only applies on creation. - Create the backup directory (holds private key copies) and ~/.ssh with 0700. Remote: - Read/write authorized_keys and known_hosts via SFTP instead of shell command lines built from server-provided paths and file contents (broken quoting on Windows for entries containing '"', e.g. command="..."). - No more eager file creation on connect; missing files read as empty and are created with 0600 (dir 0700) on Unix hosts when written. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XSUJwZ17AWMdDvYXboJAbQ --- .../Extensions/SshConfigFilesExtension.cs | 11 +- OpenSSH_GUI.Core/Lib/Misc/ServerConnection.cs | 205 +++++++----------- .../Services/KeyFileBackupService.cs | 13 +- .../Services/KeyFileWriterService.cs | 17 +- .../Services/KeyFileWriterServiceTests.cs | 66 ++++++ 5 files changed, 177 insertions(+), 135 deletions(-) create mode 100644 OpenSSH_GUI.Tests/Core/Services/KeyFileWriterServiceTests.cs diff --git a/OpenSSH_GUI.Core/Extensions/SshConfigFilesExtension.cs b/OpenSSH_GUI.Core/Extensions/SshConfigFilesExtension.cs index 1e87942..958e9f8 100644 --- a/OpenSSH_GUI.Core/Extensions/SshConfigFilesExtension.cs +++ b/OpenSSH_GUI.Core/Extensions/SshConfigFilesExtension.cs @@ -35,7 +35,16 @@ public static void ValidateDirectories(ILogger? logger = null) try { if (!Directory.Exists(GetRootSshPath())) Directory.CreateDirectory(GetRootSshPath()); - if (!Directory.Exists(GetBaseSshPath())) Directory.CreateDirectory(GetBaseSshPath()); + if (!Directory.Exists(GetBaseSshPath())) + { + // OpenSSH expects the user's .ssh directory to be accessible by the owner only. + if (OperatingSystem.IsWindows()) + Directory.CreateDirectory(GetBaseSshPath()); + else + Directory.CreateDirectory( + GetBaseSshPath(), + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } } catch (Exception e) { diff --git a/OpenSSH_GUI.Core/Lib/Misc/ServerConnection.cs b/OpenSSH_GUI.Core/Lib/Misc/ServerConnection.cs index 3af05d0..0749309 100644 --- a/OpenSSH_GUI.Core/Lib/Misc/ServerConnection.cs +++ b/OpenSSH_GUI.Core/Lib/Misc/ServerConnection.cs @@ -1,6 +1,7 @@ using System.Reactive.Disposables; using System.Reactive.Disposables.Fluent; using System.Reactive.Linq; +using System.Text; using OpenSSH_GUI.Core.Enums; using OpenSSH_GUI.Core.Extensions; using OpenSSH_GUI.Core.Lib.AuthorizedKeys; @@ -13,6 +14,13 @@ namespace OpenSSH_GUI.Core.Lib.Misc; public sealed partial class ServerConnection : ReactiveObject, IDisposable { + private const string RemoteSshDirectory = ".ssh"; + + // SftpClient.ChangePermissions expects the octal digits written as a decimal number (e.g. 700 => rwx------). + private const short RemoteSshDirectoryMode = 700; + + private const short RemoteSshFileMode = 600; + private readonly CompositeDisposable _disposables = new(); [ObservableAsProperty(ReadOnly = true)] @@ -21,44 +29,30 @@ public sealed partial class ServerConnection : ReactiveObject, IDisposable [Reactive(SetModifier = AccessModifier.Private)] private DateTime _connectionTime = DateTime.Now; - [ObservableAsProperty(ReadOnly = true)] - private string _createEmptyFileCommand = string.Empty; - [Reactive(SetModifier = AccessModifier.Private)] private bool _isConnected; [ObservableAsProperty(ReadOnly = true)] private string _lineSeparator = string.Empty; - [ObservableAsProperty(ReadOnly = true)] - private string _readContentsCommand = string.Empty; - [Reactive(SetModifier = AccessModifier.Private)] private PlatformID _serverOs = PlatformID.Other; private ServerConnection(ConnectionCredentials? credentials = null) { ConnectionCredentials = credentials ?? ConnectionCredentials.Empty; - ClientConnection = new SshClient(ConnectionCredentials.GetConnectionInfo()) + var connectionInfo = ConnectionCredentials.GetConnectionInfo(); + ClientConnection = new SshClient(connectionInfo) { KeepAliveInterval = TimeSpan.FromSeconds(10) }; + FileTransferConnection = new SftpClient(connectionInfo); _connectionStringHelper = this.WhenAnyValue(obj => obj.IsConnected) .Select(c => c ? $"{ConnectionCredentials.Username}@{ConnectionCredentials.Hostname}" : string.Empty) .ToProperty(this, obj => obj.ConnectionString) .DisposeWith(_disposables); - _readContentsCommandHelper = this.WhenAnyValue(obj => obj.ServerOs) - .Select(c => c == PlatformID.Win32NT ? "type" : "cat") - .ToProperty(this, obj => obj.ReadContentsCommand) - .DisposeWith(_disposables); - - _createEmptyFileCommandHelper = this.WhenAnyValue(obj => obj.ServerOs) - .Select(c => c == PlatformID.Win32NT ? "echo. >" : "touch") - .ToProperty(this, obj => obj.CreateEmptyFileCommand) - .DisposeWith(_disposables); - _lineSeparatorHelper = this.WhenAnyValue(obj => obj.ServerOs) .Select(e => e.GetLineSeparator()) .ToProperty(this, obj => obj.LineSeparator) @@ -79,18 +73,34 @@ private SshClient ClientConnection init => this.RaiseAndSetIfChanged(ref field, value); } + /// + /// SFTP channel used for all remote file access. Transferring file contents over SFTP avoids + /// building shell command lines from file paths or file contents. + /// + private SftpClient FileTransferConnection + { + get; + init => this.RaiseAndSetIfChanged(ref field, value); + } + /// - public void Dispose() { _disposables.Dispose(); } + public void Dispose() + { + _disposables.Dispose(); + FileTransferConnection.Dispose(); + ClientConnection.Dispose(); + } public static ServerConnection WithCredentials(ConnectionCredentials credentials) => new(credentials); public async ValueTask ConnectToServerAsync(CancellationToken token = default) { await ClientConnection.ConnectAsync(token); - IsConnected = ClientConnection.IsConnected; + if (ClientConnection.IsConnected) + await FileTransferConnection.ConnectAsync(token); + IsConnected = ClientConnection.IsConnected && FileTransferConnection.IsConnected; if (!IsConnected) return ServerOs != PlatformID.Other && IsConnected; ServerOs = await GetServerOsAsync(token); - await CheckForFilesAndCreateThemIfTheyNotExistAsync(token); ConnectionTime = DateTime.Now; return ServerOs != PlatformID.Other && IsConnected; } @@ -99,6 +109,7 @@ public ValueTask DisconnectFromServerAsync(CancellationToken token = defau { try { + FileTransferConnection.Disconnect(); ClientConnection.Disconnect(); IsConnected = ClientConnection.IsConnected; return ValueTask.FromResult(true); @@ -113,12 +124,8 @@ public async ValueTask GetKnownHostsFromServerAsync(Cancellation { if (!IsConnected) throw new InvalidOperationException("No connection to get known hosts from"); - var path = await ResolveRemoteEnvVariablesAsync( - SshConfigFiles.Known_Hosts.GetPathOfFile(false, ServerOs), - token); - using var command = ClientConnection.CreateCommand($"{ReadContentsCommand} {path}"); - await command.ExecuteAsync(token); - return await KnownHostsFile.InitializeAsync(command.OutputStream, true, false, token); + var content = await ReadRemoteFileAsync(SshConfigFiles.Known_Hosts, token); + return await KnownHostsFile.InitializeAsync(content, true, true, token); } public async ValueTask WriteKnownHostsToServerAsync(KnownHostsFile knownHostsFile, @@ -127,12 +134,9 @@ public async ValueTask WriteKnownHostsToServerAsync(KnownHostsFile knownHo if (!knownHostsFile.KnownHosts.Any(e => e.ChangesMade)) return false; if (!IsConnected) return false; - var path = await ResolveRemoteEnvVariablesAsync( - SshConfigFiles.Known_Hosts.GetPathOfFile(false, ServerOs), token); var content = await knownHostsFile.GetUpdatedContentsAsync(ServerOs); - using var command = ClientConnection.CreateCommand(BuildRemoteWriteCommand(ServerOs, content, path)); - await command.ExecuteAsync(token); - return command.ExitStatus == 0; + await WriteRemoteFileAsync(SshConfigFiles.Known_Hosts, content, token); + return true; } public async ValueTask GetAuthorizedKeysFromServerAsync(CancellationToken token = default) @@ -140,11 +144,8 @@ public async ValueTask GetAuthorizedKeysFromServerAsync(Canc if (!IsConnected) throw new InvalidOperationException("No connection to get authorized keys from"); - var path = await ResolveRemoteEnvVariablesAsync( - SshConfigFiles.Authorized_Keys.GetPathOfFile(false, ServerOs), token); - using var command = ClientConnection.CreateCommand($"{ReadContentsCommand} {path}"); - await command.ExecuteAsync(token); - return await AuthorizedKeysFile.ParseAsync(command.OutputStream, token); + await using var content = await ReadRemoteFileAsync(SshConfigFiles.Authorized_Keys, token); + return await AuthorizedKeysFile.ParseAsync(content, token); } public async ValueTask WriteAuthorizedKeysChangesToServerAsync(AuthorizedKeysFile authorizedKeysFile, @@ -153,62 +154,57 @@ public async ValueTask WriteAuthorizedKeysChangesToServerAsync(AuthorizedK if (!authorizedKeysFile.ChangesMade) return false; if (!IsConnected) return false; - var path = await ResolveRemoteEnvVariablesAsync( - SshConfigFiles.Authorized_Keys.GetPathOfFile(false, ServerOs), token); var content = authorizedKeysFile.ExportFileContent(ServerOs); - using var command = ClientConnection.CreateCommand(BuildRemoteWriteCommand(ServerOs, content, path)); - await command.ExecuteAsync(token); - return command.ExitStatus == 0; + await WriteRemoteFileAsync(SshConfigFiles.Authorized_Keys, content, token); + return true; } - private async ValueTask ResolveRemoteEnvVariablesAsync(string originalPath, - CancellationToken token = default) + /// + /// Returns the SFTP path of the given file inside the remote user's SSH directory. + /// The path is relative to the SFTP working directory, which is the user's home directory + /// on both OpenSSH for Unix and OpenSSH for Windows. + /// + private static string GetRemotePath(SshConfigFiles file) => + $"{RemoteSshDirectory}/{Enum.GetName(file)!.ToLowerInvariant()}"; + + /// + /// Reads the given remote file via SFTP. A missing file yields an empty stream. + /// + private async ValueTask ReadRemoteFileAsync(SshConfigFiles file, CancellationToken token) { - if (!IsConnected) return originalPath; - var parts = originalPath.Split('%', StringSplitOptions.RemoveEmptyEntries); - var result = string.Empty; - foreach (var part in parts) - if (part.Contains('\\') || part.Contains('/')) - { - result += part.Trim(); - } - else - { - var cmdText = ServerOs is PlatformID.Unix or PlatformID.MacOSX - ? $"echo ${part}" - : $"echo %{part}%"; - using var command = ClientConnection.CreateCommand(cmdText); - await command.ExecuteAsync(token); - result += command.Result.Trim(); - } - - return result; + var path = GetRemotePath(file); + var content = new MemoryStream(); + if (await FileTransferConnection.ExistsAsync(path, token)) + await FileTransferConnection.DownloadFileAsync(path, content, token); + content.Seek(0, SeekOrigin.Begin); + return content; } - private async ValueTask CheckForFilesAndCreateThemIfTheyNotExistAsync(CancellationToken token = default) + /// + /// Replaces the contents of the given remote file via SFTP. Missing files and the + /// SSH directory are created with owner-only permissions on Unix hosts. + /// + private async ValueTask WriteRemoteFileAsync(SshConfigFiles file, string content, CancellationToken token) { - if (!ClientConnection.IsConnected) return; - - var authKeyPath = SshConfigFiles.Authorized_Keys.GetPathOfFile(false); - var knownHostPath = SshConfigFiles.Known_Hosts.GetPathOfFile(false); - - using var authorizedKeysFileCheck = ClientConnection.CreateCommand($"{ReadContentsCommand} {authKeyPath}"); - await authorizedKeysFileCheck.ExecuteAsync(token); - - using var knownHostsFileCheck = ClientConnection.CreateCommand($"{ReadContentsCommand} {knownHostPath}"); - await knownHostsFileCheck.ExecuteAsync(token); + var path = GetRemotePath(file); + var isUnix = ServerOs is PlatformID.Unix or PlatformID.MacOSX; - if (authorizedKeysFileCheck.ExitStatus != 0) + if (!await FileTransferConnection.ExistsAsync(RemoteSshDirectory, token)) { - using var createAuthCmd = ClientConnection.CreateCommand($"{CreateEmptyFileCommand} {authKeyPath}"); - await createAuthCmd.ExecuteAsync(token); + await FileTransferConnection.CreateDirectoryAsync(RemoteSshDirectory, token); + if (isUnix) FileTransferConnection.ChangePermissions(RemoteSshDirectory, RemoteSshDirectoryMode); } - if (knownHostsFileCheck.ExitStatus != 0) + var isNewFile = !await FileTransferConnection.ExistsAsync(path, token); + + await using (var remoteFile = + await FileTransferConnection.OpenAsync(path, FileMode.Create, FileAccess.Write, token)) { - using var createKnownCmd = ClientConnection.CreateCommand($"{CreateEmptyFileCommand} {knownHostPath}"); - await createKnownCmd.ExecuteAsync(token); + var bytes = new UTF8Encoding(false).GetBytes(content); + await remoteFile.WriteAsync(bytes, token); } + + if (isNewFile && isUnix) FileTransferConnection.ChangePermissions(path, RemoteSshFileMode); } private async ValueTask GetServerOsAsync(CancellationToken token = default) @@ -228,53 +224,4 @@ private async ValueTask GetServerOsAsync(CancellationToken token = d return PlatformID.Other; } - - /// - /// Builds a platform-appropriate shell command to write the given content to a file on the remote host. - /// - /// The of the remote host. - /// The content to write into the file. - /// The full remote path of the target file. - /// If true, appends to the file instead of overwriting it. - /// A shell command string ready to be executed on the remote host. - /// - /// Thrown when no write command can be constructed for the given . - /// - private static string BuildRemoteWriteCommand(PlatformID platformId, string content, string filePath, - bool append = false) - { - var redirectOperator = append ? ">>" : ">"; - - return platformId is PlatformID.Unix or PlatformID.MacOSX - ? BuildUnixCommand(content, filePath, redirectOperator) - : BuildWindowsCommand(content, filePath, redirectOperator); - } - - /// - /// Builds a Unix shell write command using printf for reliable, escape-safe output. - /// - /// The content to write. - /// The target file path on the remote host. - /// Shell redirect operator (> or >>). - /// A Unix shell command string. - private static string BuildUnixCommand(string content, string filePath, string redirectOperator) - { - var escaped = content.Replace("'", "'\\''"); - return $"printf '%s' '{escaped}' {redirectOperator} '{filePath}'"; - } - - /// - /// Builds a Windows shell write command using PowerShell's Set-Content or Add-Content - /// for reliable Unicode-safe file writing. - /// - /// The content to write. - /// The target file path on the remote host. - /// Shell redirect operator (> or >>), used to determine append mode. - /// A PowerShell command string. - private static string BuildWindowsCommand(string content, string filePath, string redirectOperator) - { - var escaped = content.Replace("'", "''"); - var cmdlet = redirectOperator == ">>" ? "Add-Content" : "Set-Content"; - return $"powershell -Command \"{cmdlet} -Path '{filePath}' -Value '{escaped}' -NoNewline -Encoding UTF8\""; - } -} \ No newline at end of file +} diff --git a/OpenSSH_GUI.Core/Services/KeyFileBackupService.cs b/OpenSSH_GUI.Core/Services/KeyFileBackupService.cs index d1485af..72e209c 100644 --- a/OpenSSH_GUI.Core/Services/KeyFileBackupService.cs +++ b/OpenSSH_GUI.Core/Services/KeyFileBackupService.cs @@ -19,6 +19,9 @@ public sealed class KeyFileBackupService : IKeyFileBackupService, IDisposable { private const string BackupFileExtension = "bak"; + private const UnixFileMode OwnerOnlyDirectoryMode = + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute; + private static readonly string BackupDirectory = Path.Combine(SshConfigFilesExtension.GetBaseSshPath(), AppDomain.CurrentDomain.FriendlyName); @@ -76,8 +79,16 @@ public void BeginOperationLog() { if (_operationLogger is not null) return; - if (!Directory.Exists(BackupDirectory)) + // The directory holds copies of private keys - restrict it to the owner. + if (OperatingSystem.IsWindows()) + { Directory.CreateDirectory(BackupDirectory); + } + else + { + Directory.CreateDirectory(BackupDirectory, OwnerOnlyDirectoryMode); + File.SetUnixFileMode(BackupDirectory, OwnerOnlyDirectoryMode); + } var operationLogFile = Path.Combine(BackupDirectory, Path.ChangeExtension("operation_log", "log")); _loggerFactory = new SerilogLoggerFactory( diff --git a/OpenSSH_GUI.Core/Services/KeyFileWriterService.cs b/OpenSSH_GUI.Core/Services/KeyFileWriterService.cs index cd088e4..6f5d3a9 100644 --- a/OpenSSH_GUI.Core/Services/KeyFileWriterService.cs +++ b/OpenSSH_GUI.Core/Services/KeyFileWriterService.cs @@ -15,6 +15,8 @@ namespace OpenSSH_GUI.Core.Services; /// public class KeyFileWriterService(ILogger logger) : IKeyFileWriterService { + private const UnixFileMode OwnerOnlyFileMode = UnixFileMode.UserRead | UnixFileMode.UserWrite; + /// public async ValueTask WriteToFile(string filePath, string content, bool overwrite = false, Encoding? encoding = null) @@ -36,22 +38,29 @@ public async ValueTask WriteToFile(string filePath, string content, throw new IOException("File already exists"); } + // Create truncates an existing file, CreateNew closes the race between the existence check and the open. var options = new FileStreamOptions { BufferSize = 0, - Access = FileAccess.ReadWrite, - Mode = FileMode.OpenOrCreate, - Share = FileShare.ReadWrite + Access = FileAccess.Write, + Mode = overwrite ? FileMode.Create : FileMode.CreateNew, + Share = FileShare.None }; if (!OperatingSystem.IsWindows()) { - options.UnixCreateMode = UnixFileMode.UserRead | UnixFileMode.UserWrite; + options.UnixCreateMode = OwnerOnlyFileMode; } await using var fileStream = fileInfo.Open(options); logger.LogDebug("Opened file {filePath}", filePath); + if (!OperatingSystem.IsWindows()) + { + // UnixCreateMode only applies to newly created files - enforce 0600 on overwritten files as well. + File.SetUnixFileMode(fileStream.SafeFileHandle, OwnerOnlyFileMode); + } + byte[]? rented = null; var maxByteCount = encoding.GetMaxByteCount(content.Length); var buffer = maxByteCount <= 256 diff --git a/OpenSSH_GUI.Tests/Core/Services/KeyFileWriterServiceTests.cs b/OpenSSH_GUI.Tests/Core/Services/KeyFileWriterServiceTests.cs new file mode 100644 index 0000000..4988f1e --- /dev/null +++ b/OpenSSH_GUI.Tests/Core/Services/KeyFileWriterServiceTests.cs @@ -0,0 +1,66 @@ +using Microsoft.Extensions.Logging.Abstractions; +using OpenSSH_GUI.Core.Services; +using Shouldly; +using Xunit; + +namespace OpenSSH_GUI.Tests.Core.Services; + +public sealed class KeyFileWriterServiceTests : IDisposable +{ + private readonly string _directory = Directory.CreateTempSubdirectory("keyfilewriter").FullName; + private readonly KeyFileWriterService _service = new(NullLogger.Instance); + + public void Dispose() { Directory.Delete(_directory, true); } + + [Fact] + public async Task WriteToFile_Overwrite_TruncatesLongerExistingContent() + { + var path = Path.Combine(_directory, "id_test"); + await File.WriteAllTextAsync(path, new string('x', 4096), TestContext.Current.CancellationToken); + + await _service.WriteToFile(path, "short", true); + + (await File.ReadAllTextAsync(path, TestContext.Current.CancellationToken)).ShouldBe("short"); + } + + [Fact] + public async Task WriteToFile_WithoutOverwrite_ThrowsAndKeepsExistingContent() + { + var path = Path.Combine(_directory, "id_test"); + await File.WriteAllTextAsync(path, "original", TestContext.Current.CancellationToken); + + await Should.ThrowAsync(async () => await _service.WriteToFile(path, "new")); + + (await File.ReadAllTextAsync(path, TestContext.Current.CancellationToken)).ShouldBe("original"); + } + + [Fact] + public async Task WriteToFile_Overwrite_RestrictsExistingFileToOwner() + { + Assert.SkipWhen(OperatingSystem.IsWindows(), "Unix file modes only"); + var path = Path.Combine(_directory, "id_test"); + await File.WriteAllTextAsync(path, "public", TestContext.Current.CancellationToken); +#pragma warning disable CA1416 + File.SetUnixFileMode( + path, + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.GroupRead | UnixFileMode.OtherRead); + + await _service.WriteToFile(path, "private", true); + + File.GetUnixFileMode(path).ShouldBe(UnixFileMode.UserRead | UnixFileMode.UserWrite); +#pragma warning restore CA1416 + } + + [Fact] + public async Task WriteToFile_NewFile_IsCreatedOwnerOnly() + { + Assert.SkipWhen(OperatingSystem.IsWindows(), "Unix file modes only"); + var path = Path.Combine(_directory, "id_new"); + + await _service.WriteToFile(path, "private"); + +#pragma warning disable CA1416 + File.GetUnixFileMode(path).ShouldBe(UnixFileMode.UserRead | UnixFileMode.UserWrite); +#pragma warning restore CA1416 + } +} From 194876e208650a29f492b426c0889944bf4cd928 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 12:38:13 +0000 Subject: [PATCH 2/2] Verify server host keys against known_hosts (trust on first use) SshClient was created without a HostKeyReceived handler, so SSH.NET accepted any host key. An attacker in the network path could impersonate the server, capture the password of PasswordConnectionCredentials and tamper with authorized_keys/known_hosts shown and written by the app. - KnownHostKeyStore checks the presented key against ~/.ssh/known_hosts (plain and hashed |1| names, [host]:port, wildcards, negation, @revoked; @cert-authority is ignored). - ServerConnection rejects every key the validator does not trust, for both the SSH and the SFTP channel. - ServerConnectionService: unknown host -> dialog with SHA256 fingerprint, on confirmation the key is appended to known_hosts and the connection is retried; mismatch or revoked -> HostKeyVerificationException. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XSUJwZ17AWMdDvYXboJAbQ --- .../Interfaces/IHostKeyTrustPrompt.cs | 12 ++ .../Interfaces/IKnownHostKeyStore.cs | 19 ++ OpenSSH_GUI.Core/Lib/HostKeys/HostKeyInfo.cs | 49 +++++ .../HostKeys/HostKeyVerificationException.cs | 26 +++ .../Lib/HostKeys/HostKeyVerificationStatus.cs | 19 ++ OpenSSH_GUI.Core/Lib/Misc/ServerConnection.cs | 24 ++- .../Services/KnownHostKeyStore.cs | 168 ++++++++++++++++++ .../Services/ServerConnectionService.cs | 72 +++++++- .../Core/Services/KnownHostKeyStoreTests.cs | 154 ++++++++++++++++ .../DependencyInjectionExtensions.cs | 3 + .../Resources/StringsAndTexts.Designer.cs | 12 ++ OpenSSH_GUI/Resources/StringsAndTexts.de.resx | 11 ++ OpenSSH_GUI/Resources/StringsAndTexts.resx | 11 ++ OpenSSH_GUI/Services/HostKeyTrustPrompt.cs | 28 +++ 14 files changed, 603 insertions(+), 5 deletions(-) create mode 100644 OpenSSH_GUI.Core/Interfaces/IHostKeyTrustPrompt.cs create mode 100644 OpenSSH_GUI.Core/Interfaces/IKnownHostKeyStore.cs create mode 100644 OpenSSH_GUI.Core/Lib/HostKeys/HostKeyInfo.cs create mode 100644 OpenSSH_GUI.Core/Lib/HostKeys/HostKeyVerificationException.cs create mode 100644 OpenSSH_GUI.Core/Lib/HostKeys/HostKeyVerificationStatus.cs create mode 100644 OpenSSH_GUI.Core/Services/KnownHostKeyStore.cs create mode 100644 OpenSSH_GUI.Tests/Core/Services/KnownHostKeyStoreTests.cs create mode 100644 OpenSSH_GUI/Services/HostKeyTrustPrompt.cs diff --git a/OpenSSH_GUI.Core/Interfaces/IHostKeyTrustPrompt.cs b/OpenSSH_GUI.Core/Interfaces/IHostKeyTrustPrompt.cs new file mode 100644 index 0000000..884cdce --- /dev/null +++ b/OpenSSH_GUI.Core/Interfaces/IHostKeyTrustPrompt.cs @@ -0,0 +1,12 @@ +using OpenSSH_GUI.Core.Lib.HostKeys; + +namespace OpenSSH_GUI.Core.Interfaces; + +/// +/// Asks the user whether an unknown server host key should be trusted (trust on first use). +/// +public interface IHostKeyTrustPrompt +{ + /// true if the user confirmed the fingerprint and the key should be trusted. + Task ConfirmUnknownHostKeyAsync(HostKeyInfo hostKey); +} diff --git a/OpenSSH_GUI.Core/Interfaces/IKnownHostKeyStore.cs b/OpenSSH_GUI.Core/Interfaces/IKnownHostKeyStore.cs new file mode 100644 index 0000000..9781e26 --- /dev/null +++ b/OpenSSH_GUI.Core/Interfaces/IKnownHostKeyStore.cs @@ -0,0 +1,19 @@ +using OpenSSH_GUI.Core.Lib.HostKeys; + +namespace OpenSSH_GUI.Core.Interfaces; + +/// +/// Verifies server host keys against the user's known_hosts file and records newly trusted keys. +/// +public interface IKnownHostKeyStore +{ + /// + /// Checks the presented host key against the known_hosts entries of the host. + /// + HostKeyVerificationStatus Verify(HostKeyInfo hostKey); + + /// + /// Appends the host key to known_hosts. + /// + void Add(HostKeyInfo hostKey); +} diff --git a/OpenSSH_GUI.Core/Lib/HostKeys/HostKeyInfo.cs b/OpenSSH_GUI.Core/Lib/HostKeys/HostKeyInfo.cs new file mode 100644 index 0000000..e690c65 --- /dev/null +++ b/OpenSSH_GUI.Core/Lib/HostKeys/HostKeyInfo.cs @@ -0,0 +1,49 @@ +using System.Buffers.Binary; +using System.Security.Cryptography; +using System.Text; + +namespace OpenSSH_GUI.Core.Lib.HostKeys; + +/// +/// A server host key as presented during the SSH key exchange. +/// +/// The host name the connection was made to. +/// The port the connection was made to. +/// The public key in SSH wire format. +public sealed record HostKeyInfo(string Host, int Port, byte[] KeyBlob) +{ + /// + /// The key type encoded in the key blob, e.g. ssh-ed25519. + /// + public string KeyType { get; } = ReadKeyType(KeyBlob) ?? string.Empty; + + /// + /// The fingerprint in OpenSSH notation, e.g. SHA256:.... + /// + public string FingerprintSha256 => + $"SHA256:{Convert.ToBase64String(SHA256.HashData(KeyBlob)).TrimEnd('=')}"; + + /// + /// The host name as written to and matched against known_hosts + /// ([host]:port for non-default ports). + /// + public string KnownHostsName => GetKnownHostsName(Host, Port); + + public static string GetKnownHostsName(string host, int port) + { + // OpenSSH canonicalizes host names to lower case before looking them up in known_hosts. + var name = host.ToLowerInvariant(); + return port == 22 ? name : $"[{name}]:{port}"; + } + + /// + /// Reads the leading key type string of an SSH wire format public key. + /// + public static string? ReadKeyType(ReadOnlySpan keyBlob) + { + if (keyBlob.Length < sizeof(uint)) return null; + var length = BinaryPrimitives.ReadUInt32BigEndian(keyBlob); + if (length == 0 || length > keyBlob.Length - sizeof(uint)) return null; + return Encoding.ASCII.GetString(keyBlob.Slice(sizeof(uint), (int)length)); + } +} diff --git a/OpenSSH_GUI.Core/Lib/HostKeys/HostKeyVerificationException.cs b/OpenSSH_GUI.Core/Lib/HostKeys/HostKeyVerificationException.cs new file mode 100644 index 0000000..3409594 --- /dev/null +++ b/OpenSSH_GUI.Core/Lib/HostKeys/HostKeyVerificationException.cs @@ -0,0 +1,26 @@ +namespace OpenSSH_GUI.Core.Lib.HostKeys; + +/// +/// Thrown when a connection is aborted because the server host key could not be verified. +/// +public sealed class HostKeyVerificationException(HostKeyInfo hostKey, HostKeyVerificationStatus status) + : Exception(CreateMessage(hostKey, status)) +{ + public HostKeyInfo HostKey { get; } = hostKey; + + public HostKeyVerificationStatus Status { get; } = status; + + private static string CreateMessage(HostKeyInfo hostKey, HostKeyVerificationStatus status) => status switch + { + HostKeyVerificationStatus.Mismatch => + $"The {hostKey.KeyType} host key of {hostKey.KnownHostsName} does not match the key in known_hosts " + + $"({hostKey.FingerprintSha256}). Someone could be eavesdropping on the connection (man-in-the-middle " + + "attack), or the host key has been changed. Connection aborted.", + HostKeyVerificationStatus.Revoked => + $"The {hostKey.KeyType} host key of {hostKey.KnownHostsName} ({hostKey.FingerprintSha256}) " + + "is marked as revoked in known_hosts. Connection aborted.", + _ => + $"The authenticity of host {hostKey.KnownHostsName} ({hostKey.KeyType} {hostKey.FingerprintSha256}) " + + "was not confirmed. Connection aborted." + }; +} diff --git a/OpenSSH_GUI.Core/Lib/HostKeys/HostKeyVerificationStatus.cs b/OpenSSH_GUI.Core/Lib/HostKeys/HostKeyVerificationStatus.cs new file mode 100644 index 0000000..96959c1 --- /dev/null +++ b/OpenSSH_GUI.Core/Lib/HostKeys/HostKeyVerificationStatus.cs @@ -0,0 +1,19 @@ +namespace OpenSSH_GUI.Core.Lib.HostKeys; + +/// +/// Result of checking a server host key against the local known_hosts file. +/// +public enum HostKeyVerificationStatus +{ + /// No entry exists for the host and key type. + Unknown, + + /// An entry for the host matches the presented key. + Trusted, + + /// The host is known with a different key of the same type - possible man-in-the-middle attack. + Mismatch, + + /// The presented key is marked as @revoked. + Revoked +} diff --git a/OpenSSH_GUI.Core/Lib/Misc/ServerConnection.cs b/OpenSSH_GUI.Core/Lib/Misc/ServerConnection.cs index 0749309..08528b0 100644 --- a/OpenSSH_GUI.Core/Lib/Misc/ServerConnection.cs +++ b/OpenSSH_GUI.Core/Lib/Misc/ServerConnection.cs @@ -5,10 +5,12 @@ using OpenSSH_GUI.Core.Enums; using OpenSSH_GUI.Core.Extensions; using OpenSSH_GUI.Core.Lib.AuthorizedKeys; +using OpenSSH_GUI.Core.Lib.HostKeys; using OpenSSH_GUI.Core.Lib.KnownHosts; using ReactiveUI; using ReactiveUI.SourceGenerators; using Renci.SshNet; +using Renci.SshNet.Common; namespace OpenSSH_GUI.Core.Lib.Misc; @@ -38,7 +40,8 @@ public sealed partial class ServerConnection : ReactiveObject, IDisposable [Reactive(SetModifier = AccessModifier.Private)] private PlatformID _serverOs = PlatformID.Other; - private ServerConnection(ConnectionCredentials? credentials = null) + private ServerConnection(ConnectionCredentials? credentials = null, + Func? hostKeyValidator = null) { ConnectionCredentials = credentials ?? ConnectionCredentials.Empty; var connectionInfo = ConnectionCredentials.GetConnectionInfo(); @@ -48,6 +51,14 @@ private ServerConnection(ConnectionCredentials? credentials = null) }; FileTransferConnection = new SftpClient(connectionInfo); + // Without a validator every host key would be accepted - reject unless explicitly verified. + hostKeyValidator ??= _ => false; + EventHandler onHostKeyReceived = (_, args) => + args.CanTrust = hostKeyValidator( + new HostKeyInfo(ConnectionCredentials.Hostname, ConnectionCredentials.Port, args.HostKey)); + ClientConnection.HostKeyReceived += onHostKeyReceived; + FileTransferConnection.HostKeyReceived += onHostKeyReceived; + _connectionStringHelper = this.WhenAnyValue(obj => obj.IsConnected) .Select(c => c ? $"{ConnectionCredentials.Username}@{ConnectionCredentials.Hostname}" : string.Empty) .ToProperty(this, obj => obj.ConnectionString) @@ -91,7 +102,16 @@ public void Dispose() ClientConnection.Dispose(); } - public static ServerConnection WithCredentials(ConnectionCredentials credentials) => new(credentials); + /// + /// Creates a connection for the given credentials. + /// + /// The credentials used to authenticate. + /// + /// Decides whether the host key presented by the server is trusted. The connection is aborted + /// during the key exchange when it returns false. + /// + public static ServerConnection WithCredentials(ConnectionCredentials credentials, + Func hostKeyValidator) => new(credentials, hostKeyValidator); public async ValueTask ConnectToServerAsync(CancellationToken token = default) { diff --git a/OpenSSH_GUI.Core/Services/KnownHostKeyStore.cs b/OpenSSH_GUI.Core/Services/KnownHostKeyStore.cs new file mode 100644 index 0000000..7311bb1 --- /dev/null +++ b/OpenSSH_GUI.Core/Services/KnownHostKeyStore.cs @@ -0,0 +1,168 @@ +using System.Security.Cryptography; +using System.Text; +using Microsoft.Extensions.Logging; +using OpenSSH_GUI.Core.Enums; +using OpenSSH_GUI.Core.Extensions; +using OpenSSH_GUI.Core.Interfaces; +using OpenSSH_GUI.Core.Lib.HostKeys; +using OpenSSH_GUI.SshConfig.Parsers; + +namespace OpenSSH_GUI.Core.Services; + +/// +/// backed by the user's known_hosts file. +/// Supports plain and hashed (|1|salt|hash) host names, [host]:port entries, +/// wildcard and negated patterns as well as the @revoked marker. +/// @cert-authority entries are ignored. +/// +public sealed class KnownHostKeyStore : IKnownHostKeyStore +{ + private const string HashedHostPrefix = "|1|"; + private const string RevokedMarker = "@revoked"; + + private readonly Lock _fileLock = new(); + private readonly string _knownHostsPath; + private readonly ILogger _logger; + + public KnownHostKeyStore(ILogger logger) + : this(logger, SshConfigFiles.Known_Hosts.GetPathOfFile()) { } + + public KnownHostKeyStore(ILogger logger, string knownHostsPath) + { + _logger = logger; + _knownHostsPath = knownHostsPath; + } + + /// + public HostKeyVerificationStatus Verify(HostKeyInfo hostKey) + { + string[] lines; + lock (_fileLock) + { + if (!File.Exists(_knownHostsPath)) return HostKeyVerificationStatus.Unknown; + lines = File.ReadAllLines(_knownHostsPath); + } + + var hostName = hostKey.KnownHostsName; + var trusted = false; + var mismatch = false; + + foreach (var rawLine in lines) + { + var line = rawLine.Trim(); + if (line.Length == 0 || line[0] == '#') continue; + + var fields = line.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries); + string? marker = null; + if (fields[0][0] == '@') + { + marker = fields[0]; + fields = fields[1..]; + } + + if (fields.Length < 3) continue; + if (!HostPatternMatches(hostName, fields[0])) continue; + + byte[] storedKey; + try + { + storedKey = Convert.FromBase64String(fields[2]); + } + catch (FormatException) + { + continue; + } + + var sameKey = CryptographicOperations.FixedTimeEquals(storedKey, hostKey.KeyBlob); + switch (marker) + { + case RevokedMarker when sameKey: + return HostKeyVerificationStatus.Revoked; + case null when sameKey: + trusted = true; + break; + case null when string.Equals(HostKeyInfo.ReadKeyType(storedKey), hostKey.KeyType, StringComparison.Ordinal): + mismatch = true; + break; + } + } + + if (trusted) return HostKeyVerificationStatus.Trusted; + if (mismatch) + { + _logger.LogWarning( + "Host key mismatch for {host}: {keyType} {fingerprint}", hostName, hostKey.KeyType, + hostKey.FingerprintSha256); + return HostKeyVerificationStatus.Mismatch; + } + + return HostKeyVerificationStatus.Unknown; + } + + /// + public void Add(HostKeyInfo hostKey) + { + var entry = $"{hostKey.KnownHostsName} {hostKey.KeyType} {Convert.ToBase64String(hostKey.KeyBlob)}\n"; + lock (_fileLock) + { + var directory = Path.GetDirectoryName(_knownHostsPath); + if (!string.IsNullOrEmpty(directory)) + { + if (OperatingSystem.IsWindows()) + Directory.CreateDirectory(directory); + else + Directory.CreateDirectory( + directory, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } + + var options = new FileStreamOptions + { + Access = FileAccess.ReadWrite, + Mode = FileMode.OpenOrCreate, + Share = FileShare.Read + }; + if (!OperatingSystem.IsWindows()) + options.UnixCreateMode = UnixFileMode.UserRead | UnixFileMode.UserWrite; + + using var stream = new FileStream(_knownHostsPath, options); + if (stream.Length > 0) + { + stream.Seek(-1, SeekOrigin.End); + if (stream.ReadByte() != '\n') entry = "\n" + entry; + } + + stream.Seek(0, SeekOrigin.End); + stream.Write(Encoding.ASCII.GetBytes(entry)); + } + + _logger.LogInformation( + "Added {keyType} host key {fingerprint} for {host} to {path}", hostKey.KeyType, + hostKey.FingerprintSha256, hostKey.KnownHostsName, _knownHostsPath); + } + + private static bool HostPatternMatches(string hostName, string patterns) + { + if (patterns.StartsWith(HashedHostPrefix, StringComparison.Ordinal)) + return HashedHostMatches(hostName, patterns); + + return SshWildcardMatcher.Matches(hostName, patterns.Split(',')); + } + + private static bool HashedHostMatches(string hostName, string hashedEntry) + { + // |1|| + var parts = hashedEntry.Split('|'); + if (parts.Length != 4) return false; + try + { + var salt = Convert.FromBase64String(parts[2]); + var expected = Convert.FromBase64String(parts[3]); + var actual = HMACSHA1.HashData(salt, Encoding.UTF8.GetBytes(hostName)); + return CryptographicOperations.FixedTimeEquals(actual, expected); + } + catch (FormatException) + { + return false; + } + } +} diff --git a/OpenSSH_GUI.Core/Services/ServerConnectionService.cs b/OpenSSH_GUI.Core/Services/ServerConnectionService.cs index ad73a81..daef204 100644 --- a/OpenSSH_GUI.Core/Services/ServerConnectionService.cs +++ b/OpenSSH_GUI.Core/Services/ServerConnectionService.cs @@ -2,6 +2,8 @@ using System.Reactive.Disposables.Fluent; using System.Reactive.Linq; using Microsoft.Extensions.Logging; +using OpenSSH_GUI.Core.Interfaces; +using OpenSSH_GUI.Core.Lib.HostKeys; using OpenSSH_GUI.Core.Lib.Misc; using ReactiveUI; using ReactiveUI.SourceGenerators; @@ -12,6 +14,10 @@ public sealed partial class ServerConnectionService : ReactiveObject, IDisposabl { private readonly CompositeDisposable _disposables = new(); + private readonly IHostKeyTrustPrompt _hostKeyTrustPrompt; + + private readonly IKnownHostKeyStore _knownHostKeyStore; + private readonly ILogger _logger; /// @@ -36,9 +42,12 @@ public sealed partial class ServerConnectionService : ReactiveObject, IDisposabl [Reactive(SetModifier = AccessModifier.Private)] private ServerConnection _serverConnection = ServerConnection.Empty; - public ServerConnectionService(ILogger logger) + public ServerConnectionService(ILogger logger, IKnownHostKeyStore knownHostKeyStore, + IHostKeyTrustPrompt hostKeyTrustPrompt) { _logger = logger; + _knownHostKeyStore = knownHostKeyStore; + _hostKeyTrustPrompt = hostKeyTrustPrompt; _isConnectedHelper = this.WhenAnyValue(vm => vm.ServerConnection) .Select(e => e.WhenAnyValue(sc => sc.IsConnected)) @@ -73,8 +82,21 @@ public async ValueTask EstablishConnection(ConnectionCredentials connectio { try { - ServerConnection = ServerConnection.WithCredentials(connectionCredentials); - return await ServerConnection.ConnectToServerAsync(token); + var (connected, rejectedKey) = await TryConnectAsync(connectionCredentials, token); + if (rejectedKey is null) return connected; + + if (rejectedKey.Status is not HostKeyVerificationStatus.Unknown) + throw new HostKeyVerificationException(rejectedKey.HostKey, rejectedKey.Status); + + // Trust on first use: the user has to confirm the fingerprint before the key is stored. + if (!await _hostKeyTrustPrompt.ConfirmUnknownHostKeyAsync(rejectedKey.HostKey)) + throw new HostKeyVerificationException(rejectedKey.HostKey, rejectedKey.Status); + + _knownHostKeyStore.Add(rejectedKey.HostKey); + (connected, rejectedKey) = await TryConnectAsync(connectionCredentials, token); + return rejectedKey is null + ? connected + : throw new HostKeyVerificationException(rejectedKey.HostKey, rejectedKey.Status); } catch (Exception e) { @@ -83,6 +105,50 @@ public async ValueTask EstablishConnection(ConnectionCredentials connectio } } + /// + /// Connects with host key verification against known_hosts. + /// + /// + /// The connection result, or the rejected host key if the connection was aborted + /// because the host key could not be verified. + /// + private async ValueTask<(bool Connected, RejectedHostKey? RejectedKey)> TryConnectAsync( + ConnectionCredentials connectionCredentials, CancellationToken token) + { + RejectedHostKey? rejectedKey = null; + DisposeCurrentConnection(); + ServerConnection = ServerConnection.WithCredentials( + connectionCredentials, hostKey => + { + var status = _knownHostKeyStore.Verify(hostKey); + if (status is HostKeyVerificationStatus.Trusted) return true; + rejectedKey ??= new RejectedHostKey(hostKey, status); + return false; + }); + + try + { + return (await ServerConnection.ConnectToServerAsync(token), null); + } + catch (Exception e) when (rejectedKey is not null) + { + _logger.LogWarning( + e, "Host key {fingerprint} of {host} rejected: {status}", rejectedKey.HostKey.FingerprintSha256, + rejectedKey.HostKey.KnownHostsName, rejectedKey.Status); + DisposeCurrentConnection(); + ServerConnection = ServerConnection.Empty; + return (false, rejectedKey); + } + } + + private void DisposeCurrentConnection() + { + if (!ReferenceEquals(ServerConnection, ServerConnection.Empty)) + ServerConnection.Dispose(); + } + + private sealed record RejectedHostKey(HostKeyInfo HostKey, HostKeyVerificationStatus Status); + /// /// Closes the current connection to the server if a connection exists. /// diff --git a/OpenSSH_GUI.Tests/Core/Services/KnownHostKeyStoreTests.cs b/OpenSSH_GUI.Tests/Core/Services/KnownHostKeyStoreTests.cs new file mode 100644 index 0000000..6f104b3 --- /dev/null +++ b/OpenSSH_GUI.Tests/Core/Services/KnownHostKeyStoreTests.cs @@ -0,0 +1,154 @@ +using System.Security.Cryptography; +using System.Text; +using Microsoft.Extensions.Logging.Abstractions; +using OpenSSH_GUI.Core.Lib.HostKeys; +using OpenSSH_GUI.Core.Services; +using Shouldly; +using Xunit; + +namespace OpenSSH_GUI.Tests.Core.Services; + +public sealed class KnownHostKeyStoreTests : IDisposable +{ + private readonly string _directory = Directory.CreateTempSubdirectory("knownhosts").FullName; + private readonly string _path; + private readonly KnownHostKeyStore _store; + + public KnownHostKeyStoreTests() + { + _path = Path.Combine(_directory, "known_hosts"); + _store = new KnownHostKeyStore(NullLogger.Instance, _path); + } + + public void Dispose() { Directory.Delete(_directory, true); } + + private static byte[] CreateKeyBlob(string keyType, byte fill) + { + var type = Encoding.ASCII.GetBytes(keyType); + var blob = new byte[4 + type.Length + 32]; + blob[3] = (byte)type.Length; + type.CopyTo(blob, 4); + blob.AsSpan(4 + type.Length).Fill(fill); + return blob; + } + + private static readonly byte[] Ed25519Key = CreateKeyBlob("ssh-ed25519", 1); + private static readonly byte[] OtherEd25519Key = CreateKeyBlob("ssh-ed25519", 2); + private static readonly byte[] EcdsaKey = CreateKeyBlob("ecdsa-sha2-nistp256", 3); + + private void WriteKnownHosts(params string[] lines) => File.WriteAllLines(_path, lines); + + private static string Entry(string hosts, byte[] key, string? marker = null) => + $"{(marker is null ? string.Empty : marker + " ")}{hosts} {HostKeyInfo.ReadKeyType(key)} {Convert.ToBase64String(key)}"; + + [Fact] + public void Verify_MissingFile_ReturnsUnknown() + { + _store.Verify(new HostKeyInfo("example.com", 22, Ed25519Key)).ShouldBe(HostKeyVerificationStatus.Unknown); + } + + [Theory] + [InlineData("example.com", 22, "example.com")] + [InlineData("EXAMPLE.com", 22, "other.org,example.com")] + [InlineData("example.com", 2222, "[example.com]:2222")] + [InlineData("host1.example.com", 22, "*.example.com")] + public void Verify_MatchingEntry_ReturnsTrusted(string host, int port, string pattern) + { + WriteKnownHosts("# comment", string.Empty, Entry(pattern, Ed25519Key)); + + _store.Verify(new HostKeyInfo(host, port, Ed25519Key)).ShouldBe(HostKeyVerificationStatus.Trusted); + } + + [Fact] + public void Verify_HashedEntry_ReturnsTrusted() + { + var salt = RandomNumberGenerator.GetBytes(20); + var hash = HMACSHA1.HashData(salt, Encoding.UTF8.GetBytes("example.com")); + WriteKnownHosts(Entry($"|1|{Convert.ToBase64String(salt)}|{Convert.ToBase64String(hash)}", Ed25519Key)); + + _store.Verify(new HostKeyInfo("example.com", 22, Ed25519Key)).ShouldBe(HostKeyVerificationStatus.Trusted); + _store.Verify(new HostKeyInfo("example.org", 22, Ed25519Key)).ShouldBe(HostKeyVerificationStatus.Unknown); + } + + [Fact] + public void Verify_DefaultPortEntry_DoesNotMatchOtherPort() + { + WriteKnownHosts(Entry("example.com", Ed25519Key)); + + _store.Verify(new HostKeyInfo("example.com", 2222, Ed25519Key)).ShouldBe(HostKeyVerificationStatus.Unknown); + } + + [Fact] + public void Verify_NegatedPattern_ReturnsUnknown() + { + WriteKnownHosts(Entry("*.example.com,!evil.example.com", Ed25519Key)); + + _store.Verify(new HostKeyInfo("evil.example.com", 22, Ed25519Key)) + .ShouldBe(HostKeyVerificationStatus.Unknown); + } + + [Fact] + public void Verify_SameTypeDifferentKey_ReturnsMismatch() + { + WriteKnownHosts(Entry("example.com", Ed25519Key)); + + _store.Verify(new HostKeyInfo("example.com", 22, OtherEd25519Key)) + .ShouldBe(HostKeyVerificationStatus.Mismatch); + } + + [Fact] + public void Verify_OnlyOtherKeyTypeKnown_ReturnsUnknown() + { + WriteKnownHosts(Entry("example.com", EcdsaKey)); + + _store.Verify(new HostKeyInfo("example.com", 22, Ed25519Key)).ShouldBe(HostKeyVerificationStatus.Unknown); + } + + [Fact] + public void Verify_RevokedKey_ReturnsRevokedEvenIfTrusted() + { + WriteKnownHosts(Entry("example.com", Ed25519Key), Entry("*", Ed25519Key, "@revoked")); + + _store.Verify(new HostKeyInfo("example.com", 22, Ed25519Key)).ShouldBe(HostKeyVerificationStatus.Revoked); + } + + [Fact] + public void Verify_CertAuthorityEntry_IsIgnored() + { + WriteKnownHosts(Entry("example.com", Ed25519Key, "@cert-authority")); + + _store.Verify(new HostKeyInfo("example.com", 22, Ed25519Key)).ShouldBe(HostKeyVerificationStatus.Unknown); + } + + [Fact] + public void Add_AppendsEntryThatVerifiesAsTrusted() + { + File.WriteAllText(_path, Entry("other.org", EcdsaKey)); // no trailing newline + var hostKey = new HostKeyInfo("Example.com", 2222, Ed25519Key); + + _store.Add(hostKey); + + _store.Verify(hostKey).ShouldBe(HostKeyVerificationStatus.Trusted); + File.ReadAllLines(_path).ShouldBe([Entry("other.org", EcdsaKey), Entry("[example.com]:2222", Ed25519Key)]); + } + + [Fact] + public void Add_NewFile_IsCreatedOwnerOnly() + { + Assert.SkipWhen(OperatingSystem.IsWindows(), "Unix file modes only"); + + _store.Add(new HostKeyInfo("example.com", 22, Ed25519Key)); + +#pragma warning disable CA1416 + File.GetUnixFileMode(_path).ShouldBe(UnixFileMode.UserRead | UnixFileMode.UserWrite); +#pragma warning restore CA1416 + } + + [Fact] + public void HostKeyInfo_FingerprintMatchesOpenSshNotation() + { + var expected = "SHA256:" + Convert.ToBase64String(SHA256.HashData(Ed25519Key)).TrimEnd('='); + + new HostKeyInfo("example.com", 22, Ed25519Key).FingerprintSha256.ShouldBe(expected); + } +} diff --git a/OpenSSH_GUI/Extensions/DependencyInjectionExtensions.cs b/OpenSSH_GUI/Extensions/DependencyInjectionExtensions.cs index caebb19..b5723ef 100644 --- a/OpenSSH_GUI/Extensions/DependencyInjectionExtensions.cs +++ b/OpenSSH_GUI/Extensions/DependencyInjectionExtensions.cs @@ -15,6 +15,7 @@ using OpenSSH_GUI.Core.Services.Hosted; using OpenSSH_GUI.Dialogs.Interfaces; using OpenSSH_GUI.Dialogs.Services; +using OpenSSH_GUI.Services; using OpenSSH_GUI.ViewModels; using OpenSSH_GUI.Views; using Serilog.Core; @@ -34,6 +35,8 @@ internal IHostBuilder RegisterOpenSshGuiServices() services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(sp => sp.GetRequiredService()); diff --git a/OpenSSH_GUI/Resources/StringsAndTexts.Designer.cs b/OpenSSH_GUI/Resources/StringsAndTexts.Designer.cs index 42a90b8..f6e4648 100644 --- a/OpenSSH_GUI/Resources/StringsAndTexts.Designer.cs +++ b/OpenSSH_GUI/Resources/StringsAndTexts.Designer.cs @@ -806,5 +806,17 @@ public static string ApplicationSettingsLookupPaths { return ResourceManager.GetString("ApplicationSettingsLookupPaths", resourceCulture); } } + + public static string HostKeyUnknownTitle { + get { + return ResourceManager.GetString("HostKeyUnknownTitle", resourceCulture); + } + } + + public static string HostKeyUnknownText { + get { + return ResourceManager.GetString("HostKeyUnknownText", resourceCulture); + } + } } } diff --git a/OpenSSH_GUI/Resources/StringsAndTexts.de.resx b/OpenSSH_GUI/Resources/StringsAndTexts.de.resx index 239021b..6f2b024 100644 --- a/OpenSSH_GUI/Resources/StringsAndTexts.de.resx +++ b/OpenSSH_GUI/Resources/StringsAndTexts.de.resx @@ -393,4 +393,15 @@ Suchpfade + + Unbekannter Host-Schlüssel + + + Die Echtheit des Hosts '{0}' kann nicht bestätigt werden. + +{1}-Fingerprint: +{2} + +Fahren Sie nur fort, wenn Sie diesen Fingerprint überprüft haben. Diesem Host vertrauen und den Schlüssel in known_hosts aufnehmen? + \ No newline at end of file diff --git a/OpenSSH_GUI/Resources/StringsAndTexts.resx b/OpenSSH_GUI/Resources/StringsAndTexts.resx index 75f14d8..88384f0 100644 --- a/OpenSSH_GUI/Resources/StringsAndTexts.resx +++ b/OpenSSH_GUI/Resources/StringsAndTexts.resx @@ -406,4 +406,15 @@ Lookup Paths + + Unknown host key + + + The authenticity of host '{0}' can't be established. + +{1} key fingerprint: +{2} + +Only continue if you have verified this fingerprint. Trust this host and add the key to known_hosts? + \ No newline at end of file diff --git a/OpenSSH_GUI/Services/HostKeyTrustPrompt.cs b/OpenSSH_GUI/Services/HostKeyTrustPrompt.cs new file mode 100644 index 0000000..75ba31f --- /dev/null +++ b/OpenSSH_GUI/Services/HostKeyTrustPrompt.cs @@ -0,0 +1,28 @@ +using Avalonia.Threading; +using Material.Icons; +using OpenSSH_GUI.Core.Interfaces; +using OpenSSH_GUI.Core.Lib.HostKeys; +using OpenSSH_GUI.Dialogs.Enums; +using OpenSSH_GUI.Dialogs.Interfaces; +using OpenSSH_GUI.Resources; + +namespace OpenSSH_GUI.Services; + +/// +/// Shows the fingerprint of an unknown server host key and lets the user decide whether to trust it. +/// +public sealed class HostKeyTrustPrompt(IMessageBoxProvider messageBoxProvider) : IHostKeyTrustPrompt +{ + /// + public async Task ConfirmUnknownHostKeyAsync(HostKeyInfo hostKey) + { + var result = await Dispatcher.UIThread.InvokeAsync(() => messageBoxProvider.ShowMessageBoxAsync( + StringsAndTexts.HostKeyUnknownTitle, + string.Format( + StringsAndTexts.HostKeyUnknownText, hostKey.KnownHostsName, hostKey.KeyType, + hostKey.FingerprintSha256), + MessageBoxButtons.YesNo, + MaterialIconKind.ShieldAlertOutline)); + return result is MessageBoxResult.Yes; + } +}