diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/KekDoctor.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/KekDoctor.cs index 6e3a017..98afb1e 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/KekDoctor.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/KekDoctor.cs @@ -34,7 +34,12 @@ public sealed record KekDiagnosis(string SecretName, RowKekStatus Status, string /// /// The kek_id of the KEK the session is currently using. /// The per-row verdicts, ordered by secret name. -public sealed record KekDoctorReport(string SessionKekId, IReadOnlyList Rows) +/// +/// The number of rows scanned from the store (including tombstones). A value greater than +/// count means some rows vanished between the metadata list and the per-row fetch +/// (a concurrent hard-absence) and were dropped — the discrepancy makes that visible rather than silent. +/// +public sealed record KekDoctorReport(string SessionKekId, IReadOnlyList Rows, int Total) { /// Whether every diagnosed row opens cleanly under the session KEK. public bool Healthy => Rows.All(r => r.Status == RowKekStatus.Ok); @@ -92,37 +97,42 @@ public sealed class KekDoctor { ct.ThrowIfCancellationRequested(); - // A row under a different KEK cannot be opened by this session — report it (with its own - // kek_id so the operator knows which key to hunt) WITHOUT attempting a doomed decrypt. - if (!string.Equals(meta.KekId, sessionKekId, StringComparison.Ordinal)) - { - rows.Add(new KekDiagnosis(meta.Name.Value, RowKekStatus.WrongKek, meta.KekId)); - continue; - } - + // Fetch the FRESH row and classify from ITS kek_id — never the (possibly stale) list + // metadata. A concurrent re-wrap between the list and this fetch would otherwise mislabel a + // moved row (a benign WrongKek row read as Corrupt, or a re-homed row read as WrongKek). StoredSecret? row = await session.Store.GetAsync(meta.Name, ct).ConfigureAwait(false); if (row is null) { - // Vanished between the list and the fetch (a concurrent hard-absence) — nothing to probe. + // Vanished between the list and the fetch (a concurrent hard-absence). Not added to + // rows; the Total-vs-Rows.Count gap in the report keeps the drop visible. continue; } - RowKekStatus status; - try - { - // The kek_id matches: probe the actual unwrap/decrypt. Plaintext is discarded at once. - _ = session.Cipher.Decrypt(row); - status = RowKekStatus.Ok; - } - catch (SecretDecryptionException) - { - status = RowKekStatus.Corrupt; - } - - rows.Add(new KekDiagnosis(meta.Name.Value, status, row.KekId)); + rows.Add(Classify(row, sessionKekId, session.Cipher)); } - return new KekDoctorReport(sessionKekId, rows); + return new KekDoctorReport(sessionKekId, rows, all.Count); + } + + // Classifies a single FRESH row against the session KEK. A row under a different kek_id is + // WrongKek (reported with its own id so the operator knows which key to hunt) and is NOT decrypted; + // a matching row is decrypt-probed (plaintext discarded at once) → Ok, or Corrupt if it fails closed. + private static KekDiagnosis Classify(StoredSecret row, string sessionKekId, ISecretCipher cipher) + { + if (!string.Equals(row.KekId, sessionKekId, StringComparison.Ordinal)) + { + return new KekDiagnosis(row.Name.Value, RowKekStatus.WrongKek, row.KekId); + } + + try + { + _ = cipher.Decrypt(row); + return new KekDiagnosis(row.Name.Value, RowKekStatus.Ok, row.KekId); + } + catch (SecretDecryptionException) + { + return new KekDiagnosis(row.Name.Value, RowKekStatus.Corrupt, row.KekId); + } } /// diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/KekDoctorTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/KekDoctorTests.cs index c2f5b4d..a36b8a3 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/KekDoctorTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/KekDoctorTests.cs @@ -86,11 +86,31 @@ public sealed class KekDoctorTests : IAsyncLifetime, IDisposable Assert.True(report.Healthy); Assert.Equal(_kekA.KekId, report.SessionKekId); Assert.Equal(2, report.Rows.Count); + Assert.Equal(2, report.Total); // no rows vanished → Total equals Rows.Count. Assert.All(report.Rows, r => Assert.Equal(RowKekStatus.Ok, r.Status)); // Rows ordered by name: "ldap/b" precedes "sql/a". Assert.Equal(["ldap/b", "sql/a"], report.Rows.Select(r => r.SecretName)); } + [Fact] + public async Task Diagnose_verdict_reads_fresh_row_kek_not_stale_metadata() + { + // Seed under A; a first diagnosis on session A reports Ok. + await SeedAsync("sql/a", "value-a", _kekA); + KekDoctorReport first = await new KekDoctor().DiagnoseAsync(Session(_kekA), CancellationToken.None); + Assert.Equal(RowKekStatus.Ok, Assert.Single(first.Rows).Status); + + // Re-seal the SAME row under B (simulates a concurrent re-wrap/re-set moving its KEK). A second + // diagnosis on session A must classify from the FRESH row.KekId (B) → WrongKek surfacing B's id, + // never a false Corrupt from probing a row the session cannot open. + await SeedAsync("sql/a", "value-a", _kekB); + KekDoctorReport second = await new KekDoctor().DiagnoseAsync(Session(_kekA), CancellationToken.None); + + KekDiagnosis row = Assert.Single(second.Rows); + Assert.Equal(RowKekStatus.WrongKek, row.Status); + Assert.Equal(_kekB.KekId, row.RowKekId); + } + [Fact] public async Task Diagnose_reports_wrong_kek_rows_with_their_kekid() { @@ -149,6 +169,15 @@ public sealed class KekDoctorTests : IAsyncLifetime, IDisposable Assert.Contains("key", ex.Message, StringComparison.OrdinalIgnoreCase); } + [Fact] + public async Task RewrapAll_throws_on_degraded_session() + { + InvalidOperationException ex = await Assert.ThrowsAsync( + () => new KekDoctor().RewrapAllAsync(DegradedSession(), _kekA, CancellationToken.None)); + + Assert.Contains("key", ex.Message, StringComparison.OrdinalIgnoreCase); + } + [Fact] public async Task RewrapAll_moves_wrong_kek_rows_onto_session_kek() {