Review 1.3: an unreadable agent key file is kept, not overwritten
LoadOrCreate replaced a key file it could not decrypt, which made the old key unrecoverable even when only the machine (DPAPI) had changed and the file could have been put back. The file is now moved aside as credential-key.bin.unreadable-* and the RSA object of the failed import is disposed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
356e102126
commit
f5ff42ca83
3 changed files with 16 additions and 7 deletions
|
|
@ -61,7 +61,7 @@ En adgangskode krypteres i webbrugerfladen, i det øjeblik den modtages, med **a
|
|||
|
||||
- Agentens nøglepar ligger i `C:\ProgramData\SQLInsight\keys`, beskyttet med DPAPI (LocalMachine) og en mappe, kun SYSTEM, Administrators og agentens konto har adgang til. Den offentlige nøgle står i `insight.AgentKeys`.
|
||||
- Den krypterede adgangskode er bundet til server, port og login. Ændres en af dem, skal adgangskoden indtastes igen, så den aldrig kan sendes til en anden server, end den blev indtastet til.
|
||||
- Geninstalleres agentens server (eller slettes nøglen), laves en ny nøgle. Gemte adgangskoder kan så ikke længere bruges; siden viser det, og de indtastes igen. Windows-godkendelse berøres ikke.
|
||||
- Geninstalleres agentens server (eller slettes nøglen), laves en ny nøgle. Gemte adgangskoder kan så ikke længere bruges; siden viser det, og de indtastes igen. Windows-godkendelse berøres ikke. En nøglefil, agenten ikke kan læse (fx kopieret fra en anden server), overskrives ikke, men gemmes som `credential-key.bin.unreadable-<tidspunkt>` ved siden af den nye.
|
||||
|
||||
Hvorfor ikke ASP.NET Data Protection eller DPAPI alene: agent og web kan ligge på forskellige servere og køre som forskellige konti, og webbrugerfladen skal aldrig kunne læse en adgangskode. Med et nøglepar har webbrugerfladen kun den offentlige nøgle.
|
||||
|
||||
|
|
|
|||
|
|
@ -39,16 +39,19 @@ public sealed partial class AgentCredentialKey : IDisposable
|
|||
/// <summary>SubjectPublicKeyInfo (DER): what SQLInsight Web seals passwords with.</summary>
|
||||
public byte[] PublicKey { get; }
|
||||
|
||||
/// <summary>The key in <paramref name="directory"/>, created the first time. A file that cannot be read is replaced.</summary>
|
||||
/// <summary>
|
||||
/// The key in <paramref name="directory"/>, created the first time. A file that cannot be decrypted is kept beside
|
||||
/// the new key (credential-key.bin.unreadable-…), not overwritten, so an administrator can still restore it.
|
||||
/// </summary>
|
||||
public static AgentCredentialKey LoadOrCreate(string directory, ILogger? logger = null)
|
||||
{
|
||||
ArgumentException.ThrowIfNullOrEmpty(directory);
|
||||
var path = Path.Combine(directory, FileName);
|
||||
if (File.Exists(path))
|
||||
{
|
||||
var rsa = RSA.Create();
|
||||
try
|
||||
{
|
||||
var rsa = RSA.Create();
|
||||
var pkcs8 = Unprotect(File.ReadAllBytes(path));
|
||||
try
|
||||
{
|
||||
|
|
@ -63,10 +66,14 @@ public sealed partial class AgentCredentialKey : IDisposable
|
|||
}
|
||||
catch (CryptographicException ex)
|
||||
{
|
||||
// Copied from another computer, or damaged: passwords sealed for it cannot be opened anyway.
|
||||
// Copied from another computer (DPAPI LocalMachine), or damaged: passwords sealed for it cannot be opened
|
||||
// here. Keep the file, in case it is the machine that changed and the file can be put back.
|
||||
rsa.Dispose();
|
||||
var kept = path + ".unreadable-" + DateTime.UtcNow.ToString("yyyyMMddHHmmss", System.Globalization.CultureInfo.InvariantCulture);
|
||||
File.Move(path, kept, overwrite: true);
|
||||
if (logger is not null)
|
||||
{
|
||||
LogUnreadable(logger, ex, path);
|
||||
LogUnreadable(logger, ex, path, kept);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -165,6 +172,6 @@ public sealed partial class AgentCredentialKey : IDisposable
|
|||
}
|
||||
}
|
||||
|
||||
[LoggerMessage(Level = LogLevel.Warning, Message = "The credential key {Path} could not be read; a new key is created. SQL login passwords entered before must be entered again.")]
|
||||
private static partial void LogUnreadable(ILogger logger, Exception exception, string path);
|
||||
[LoggerMessage(Level = LogLevel.Warning, Message = "The credential key {Path} could not be read (kept as {Kept}); a new key is created. SQL login passwords entered before must be entered again.")]
|
||||
private static partial void LogUnreadable(ILogger logger, Exception exception, string path, string kept);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -55,6 +55,8 @@ public sealed class AgentCredentialKeyTests : IDisposable
|
|||
using var replaced = AgentCredentialKey.LoadOrCreate(_directory);
|
||||
|
||||
Assert.NotEqual(oldId, replaced.KeyId);
|
||||
// The unreadable file is kept beside the new key, not overwritten, in case it can be restored.
|
||||
Assert.Single(Directory.GetFiles(_directory, AgentCredentialKey.FileName + ".unreadable-*"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue