diff --git a/OpenSSH_GUI.Core/Services/SshKeyManager.cs b/OpenSSH_GUI.Core/Services/SshKeyManager.cs index abbcfd2..2b86522 100644 --- a/OpenSSH_GUI.Core/Services/SshKeyManager.cs +++ b/OpenSSH_GUI.Core/Services/SshKeyManager.cs @@ -1,5 +1,4 @@ using System.Collections.ObjectModel; -using System.Diagnostics; using System.Text; using JetBrains.Annotations; using Microsoft.Extensions.Logging; @@ -11,6 +10,8 @@ using ReactiveUI.SourceGenerators; using Renci.SshNet; using SshNet.Keygen; +using SshNet.Keygen.Extensions; +using SshNet.Keygen.SshKeyEncryption; namespace OpenSSH_GUI.Core.Services; @@ -73,8 +74,8 @@ public async ValueTask InitialSearchAsync(CancellationToken token = default) /// /// Changes the password of an SSH key file, handling both OpenSSH and PuTTY formats transparently. - /// If the key is in PuTTY format, it will be temporarily converted to OpenSSH, the password changed, - /// and then converted back to the original format. + /// The already decrypted key is re-encrypted in-process and written back in its original format. + /// An empty removes the encryption. /// /// The SSH key file whose password should be changed. /// The new password to set, encoded using . @@ -87,7 +88,7 @@ public async ValueTask InitialSearchAsync(CancellationToken token = default) /// Thrown if the resolved key file path is null or whitespace. /// Thrown if the internal semaphore could not be acquired within 5 seconds. /// - /// Thrown if ssh-keygen exits with a non-zero code, or if intermediate key file operations fail. + /// Thrown if writing or reloading the re-encrypted key file fails. /// On failure, all modified files are restored from backup. /// public async ValueTask ChangePasswordOfKeyAsync(SshKeyFile key, @@ -99,7 +100,6 @@ public async ValueTask ChangePasswordOfKeyAsync(SshKe var semaphoreAcquired = false; var errorsOccured = false; BackedUpFile[] backupFiles = []; - string[] additionalDeleteFiles = []; var keyFilePath = string.Empty; try { @@ -117,60 +117,17 @@ public async ValueTask ChangePasswordOfKeyAsync(SshKe semaphoreAcquired = true; backupFiles = _backupService.BackupFiles(key.KeyFiles).ToArray(); - if (key.Format is { } and not SshKeyFormat.OpenSSH) - { - Log(LogLevel.Debug, "Detected PuTTY key {key} - need to change format first", keyFilePath); - additionalDeleteFiles = (await _keyFileWriterService.WriteToFileInSpecificFormat( - SshKeyFormat.OpenSSH, - key.Password.ToSshKeyEncryption(), privateKeyFile, keyFilePath, true)).ToArray(); - - keyFilePath = additionalDeleteFiles.First(e => string.IsNullOrWhiteSpace(Path.GetExtension(e))); - Log(LogLevel.Debug, "New file path: {newFilePath}", keyFilePath); - } + // Re-encrypt in-process: the decrypted key is already loaded, so no passphrase has to be + // handed to an external process (command lines are readable by other local users). + var format = key.Format ?? SshKeyFormat.OpenSSH; + var newEncryption = CreateEncryption(newPassword.Span, encoding, format); + var privateKeyContent = format is SshKeyFormat.OpenSSH + ? privateKeyFile.ToOpenSshFormat(newEncryption) + : privateKeyFile.ToPuttyFormat(newEncryption, format); - using var process = new Process(); - process.StartInfo = new ProcessStartInfo - { - FileName = "ssh-keygen", - Arguments = - $"-p -f {keyFilePath} -P \"{key.Password.GetPasswordString()}\" -N \"{encoding.GetString(newPassword.Span)}\"", - RedirectStandardOutput = true, - RedirectStandardError = true, - UseShellExecute = false, - CreateNoWindow = true - }; - - if (process.Start()) - { - await process.WaitForExitAsync(token); - if (process.ExitCode != 0) - { - var message = await process.StandardError.ReadToEndAsync(token); - Log( - LogLevel.Error, "ssh-keygen exited with code {exitCode} and message: {message}", - process.ExitCode, message); - throw new Exception($"ssh-keygen exited with code {process.ExitCode}"); - } - - var output = await process.StandardOutput.ReadToEndAsync(token); - Log(LogLevel.Debug, "ssh-keygen exited without errors and output: {message}", output); - } - - if (key.Format is { } format and not SshKeyFormat.OpenSSH) - { - var keyFile = _keyFactory.Create(); - keyFile.Load(SshKeyFileSource.FromDisk(keyFilePath), newPassword.Span); - Log( - LogLevel.Debug, - "Changes to the password were made in OpenSSH Format - need to change format to Putty again"); - keyFilePath = (await _keyFileWriterService.WriteToFileInSpecificFormat( - format, keyFile.Password.ToSshKeyEncryption(), - keyFile.PrivateKeyFile ?? throw new Exception("Private key file not found"), keyFilePath, - true)).First(); - - Log(LogLevel.Debug, "New file path: {newFilePath}", keyFilePath); - foreach (var deleteFile in additionalDeleteFiles) File.Delete(deleteFile); - } + // The public key does not change with the passphrase, only the private key file is rewritten. + await _keyFileWriterService.WriteToFile(keyFilePath, privateKeyContent, true); + Log(LogLevel.Debug, "Re-encrypted private key {key}", keyFilePath); key.Load(SshKeyFileSource.FromDisk(keyFilePath), newPassword.Span); Log(LogLevel.Debug, "Successfully changed password of key {key}", keyFilePath); @@ -193,6 +150,16 @@ public async ValueTask ChangePasswordOfKeyAsync(SshKe } } + private static ISshKeyEncryption CreateEncryption(ReadOnlySpan password, Encoding encoding, + SshKeyFormat format) + { + return password.IsEmpty + ? new SshKeyEncryptionNone() + : new SshKeyEncryptionAes256( + encoding.GetString(password), + format is SshKeyFormat.PuTTYv3 ? new PuttyV3Encryption() : null); + } + /// /// Attempts to delete all files associated with the given SSH key. /// Unlike , this method does not throw on failure — diff --git a/OpenSSH_GUI.Tests/Core/Services/SshKeyManagerPasswordTests.cs b/OpenSSH_GUI.Tests/Core/Services/SshKeyManagerPasswordTests.cs new file mode 100644 index 0000000..ddc8d95 --- /dev/null +++ b/OpenSSH_GUI.Tests/Core/Services/SshKeyManagerPasswordTests.cs @@ -0,0 +1,84 @@ +using System.Text; +using Avalonia.Headless.XUnit; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using OpenSSH_GUI.Core.Interfaces; +using OpenSSH_GUI.Core.Lib.Keys; +using OpenSSH_GUI.Core.Lib.Misc; +using OpenSSH_GUI.Core.Services; +using Renci.SshNet; +using Renci.SshNet.Common; +using Shouldly; +using SshNet.Keygen; +using SshNet.Keygen.Extensions; +using SshNet.Keygen.SshKeyEncryption; +using Xunit; + +namespace OpenSSH_GUI.Tests.Core.Services; + +public sealed class SshKeyManagerPasswordTests : IDisposable +{ + private readonly string _directory = Directory.CreateTempSubdirectory("keymanager").FullName; + + public void Dispose() { Directory.Delete(_directory, true); } + + private static SshKeyManager CreateManager() + { + var backupService = Substitute.For(); + backupService.BackupFiles(Arg.Any()).Returns([]); + return new SshKeyManager( + NullLogger.Instance, + Substitute.For(), + Substitute.For(), + Substitute.For(), + new KeyFileWriterService(NullLogger.Instance), + backupService); + } + + private SshKeyFile LoadKey(string? passphrase, out string path) + { + path = Path.Combine(_directory, "id_test"); + var info = new SshKeyGenerateInfo(SshKeyType.ED25519) { Comment = "test" }; + if (passphrase is not null) info.Encryption = new SshKeyEncryptionAes256(passphrase); + var generated = SshKey.Generate(path, FileMode.Create, info); + File.WriteAllText(path + ".pub", generated.ToOpenSshPublicFormat()); + + var key = new SshKeyFile(Substitute.For>()); + if (passphrase is null) + key.Load(SshKeyFileSource.FromDisk(path)); + else + key.Load(SshKeyFileSource.FromDisk(path), Encoding.UTF8.GetBytes(passphrase)); + return key; + } + + [AvaloniaTheory] + [InlineData("new-passphrase")] + [InlineData("with \"quotes\" -N and spaces")] + public async Task ChangePasswordOfKeyAsync_EncryptsKeyWithNewPassphrase(string newPassphrase) + { + using var manager = CreateManager(); + var key = LoadKey(null, out var path); + key.PrivateKeyFile.ShouldNotBeNull(); + + var result = await manager.ChangePasswordOfKeyAsync(key, Encoding.UTF8.GetBytes(newPassphrase)); + + result.IsSuccess.ShouldBeTrue(); + Should.Throw(() => new PrivateKeyFile(path)); + Should.NotThrow(() => new PrivateKeyFile(path, newPassphrase)); + } + + [AvaloniaFact] + public async Task ChangePasswordOfKeyAsync_EmptyPassword_RemovesEncryption() + { + using var manager = CreateManager(); + var key = LoadKey("old-passphrase", out var path); + key.PrivateKeyFile.ShouldNotBeNull(); + + var result = await manager.ChangePasswordOfKeyAsync(key, ReadOnlyMemory.Empty); + + result.IsSuccess.ShouldBeTrue(); + + Should.NotThrow(() => new PrivateKeyFile(path)); + } +}