mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 04:32:32 +00:00
fix(backend/security): cert-expiry watcher uses bucket-level logic, not per-cert
77fde56f's per-cert alarm fires for every expiring cert independently. That is wrong: when a Root or Intermediate has a fresh successor in the bundle (e.g. intune-intermediate-mdm-2026.pem alongside intune-intermediate-mdm-2028 .pem), the older cert is just a chain-continuity helper for in-flight device certs. Alarming on it would page the operator to "rotate" something that is already rotated. Refactor to bucket-level: per role (Root, Intermediate), pick the cert with the MAXIMUM NotAfter and classify only that. Older expiring/expired siblings are silent. Operator only gets paged when the freshest itself nears expiry, i.e. when there is genuinely no successor yet. Concrete behaviour change for current bundle (today 2026-05-08): Roots: freshest is intune-root-2030.pem (1591d) -> silent Intermediates: freshest is intune-intermediate-mdm-2028.pem (859d) -> silent The expired intune-intermediate-mdm-2026.pem (-3d) no longer triggers a spurious Critical because mdm-2028 has it covered. Tests: 7 new EvaluateBucket cases (empty, single fresh, single soon, old+ fresh successor stays silent, old+soon successor fires on successor, all expired falls back to freshest-expired, two future successors). Total 38 cert-related tests, full suite 1105/1105. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Oliver Kieselbach
@@ -1,3 +1,5 @@
|
||||
using System.Security.Cryptography;
|
||||
using System.Security.Cryptography.X509Certificates;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Tests;
|
||||
@@ -115,4 +117,119 @@ public class MaintenanceServiceCertExpiryTests
|
||||
var json = """{"thumbprint":12345}""";
|
||||
Assert.Equal("", MaintenanceService.ExtractThumbprint(json));
|
||||
}
|
||||
|
||||
// --- EvaluateBucket (bucket-level: only the freshest cert drives the alarm) ---
|
||||
// Correctness invariant under test: an old cert in the same role bucket must NOT
|
||||
// generate its own warning as long as a fresh successor is present. The bucket is
|
||||
// judged by its newest member's NotAfter, because that's when "no chain helper
|
||||
// ever again" actually arrives.
|
||||
|
||||
[Fact]
|
||||
public void EvaluateBucket_EmptyBucket_ReturnsNullEverything()
|
||||
{
|
||||
var eval = MaintenanceService.EvaluateBucket(Array.Empty<X509Certificate2>(), DateTime.UtcNow);
|
||||
|
||||
Assert.Null(eval.EventType);
|
||||
Assert.Null(eval.Freshest);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void EvaluateBucket_SingleFreshCert_StaysSilent()
|
||||
{
|
||||
var now = new DateTime(2026, 5, 8, 12, 0, 0, DateTimeKind.Utc);
|
||||
using var fresh = MakeCert("CN=Fresh Root", notAfter: now.AddDays(180));
|
||||
|
||||
var eval = MaintenanceService.EvaluateBucket(new[] { fresh }, now);
|
||||
|
||||
Assert.Null(eval.EventType);
|
||||
Assert.Equal(fresh.Thumbprint, eval.Freshest!.Thumbprint);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void EvaluateBucket_SingleSoonCert_FiresExpiringSoon()
|
||||
{
|
||||
var now = new DateTime(2026, 5, 8, 12, 0, 0, DateTimeKind.Utc);
|
||||
using var soon = MakeCert("CN=Soon", notAfter: now.AddDays(60));
|
||||
|
||||
var eval = MaintenanceService.EvaluateBucket(new[] { soon }, now);
|
||||
|
||||
Assert.Equal("EmbeddedCertExpiringSoon", eval.EventType);
|
||||
Assert.Equal(soon.Thumbprint, eval.Freshest!.Thumbprint);
|
||||
Assert.Equal(60, eval.DaysUntilExpiry);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void EvaluateBucket_OldExpiredAndFreshSuccessor_StaysSilent()
|
||||
{
|
||||
// The user's correctness ask: bundle has both intune-intermediate-mdm-2026
|
||||
// (expired) AND intune-intermediate-mdm-2028 (fresh). The old one is just a
|
||||
// chain helper for in-flight device certs; the bucket is healthy because the
|
||||
// successor is in place. NO alarm should fire.
|
||||
var now = new DateTime(2026, 5, 8, 12, 0, 0, DateTimeKind.Utc);
|
||||
using var oldExpired = MakeCert("CN=Old Intermediate", notAfter: now.AddDays(-3));
|
||||
using var freshSuccessor = MakeCert("CN=New Intermediate", notAfter: now.AddDays(800));
|
||||
|
||||
var eval = MaintenanceService.EvaluateBucket(new[] { oldExpired, freshSuccessor }, now);
|
||||
|
||||
Assert.Null(eval.EventType);
|
||||
Assert.Equal(freshSuccessor.Thumbprint, eval.Freshest!.Thumbprint);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void EvaluateBucket_OldExpiredAndSoonSuccessor_FiresOnSuccessor()
|
||||
{
|
||||
// Bundle has an expired old + a soon-expiring "newest". The alarm fires for
|
||||
// the soon one (it IS the bucket's successor, so its expiry IS the bucket's
|
||||
// expiry). Critical for the old cert is suppressed.
|
||||
var now = new DateTime(2026, 5, 8, 12, 0, 0, DateTimeKind.Utc);
|
||||
using var oldExpired = MakeCert("CN=Old", notAfter: now.AddDays(-30));
|
||||
using var soonSuccessor = MakeCert("CN=Soon Successor", notAfter: now.AddDays(50));
|
||||
|
||||
var eval = MaintenanceService.EvaluateBucket(new[] { oldExpired, soonSuccessor }, now);
|
||||
|
||||
Assert.Equal("EmbeddedCertExpiringSoon", eval.EventType);
|
||||
Assert.Equal(soonSuccessor.Thumbprint, eval.Freshest!.Thumbprint);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void EvaluateBucket_AllExpired_FiresExpiredOnFreshestExpired()
|
||||
{
|
||||
// Edge case: bundle has only expired certs, no successor. Freshest is still
|
||||
// the latest-expiring one. Alarm fires Critical, message reflects the freshest.
|
||||
var now = new DateTime(2026, 5, 8, 12, 0, 0, DateTimeKind.Utc);
|
||||
using var ancient = MakeCert("CN=Ancient", notAfter: now.AddDays(-100));
|
||||
using var recent = MakeCert("CN=Recent", notAfter: now.AddDays(-3));
|
||||
|
||||
var eval = MaintenanceService.EvaluateBucket(new[] { ancient, recent }, now);
|
||||
|
||||
Assert.Equal("EmbeddedCertExpired", eval.EventType);
|
||||
Assert.Equal(recent.Thumbprint, eval.Freshest!.Thumbprint);
|
||||
Assert.True(eval.DaysUntilExpiry < 0);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void EvaluateBucket_TwoFreshSuccessors_StaysSilentOnLatest()
|
||||
{
|
||||
// Bundle has two valid successors (e.g. a current root and a future root).
|
||||
// Both are far from expiry; pick the latest by NotAfter.
|
||||
var now = new DateTime(2026, 5, 8, 12, 0, 0, DateTimeKind.Utc);
|
||||
using var nearFuture = MakeCert("CN=2026 Root", notAfter: new DateTime(2026, 8, 12, 0, 0, 0, DateTimeKind.Utc));
|
||||
using var farFuture = MakeCert("CN=2030 Root", notAfter: new DateTime(2030, 9, 15, 0, 0, 0, DateTimeKind.Utc));
|
||||
|
||||
var eval = MaintenanceService.EvaluateBucket(new[] { nearFuture, farFuture }, now);
|
||||
|
||||
Assert.Null(eval.EventType);
|
||||
Assert.Equal(farFuture.Thumbprint, eval.Freshest!.Thumbprint);
|
||||
}
|
||||
|
||||
private static X509Certificate2 MakeCert(string subjectCn, DateTime notAfter, DateTime? notBefore = null)
|
||||
{
|
||||
// Self-signed cert factory for unit tests. The validator's chain check is
|
||||
// not part of this code path - we only care about NotBefore/NotAfter and
|
||||
// Thumbprint for bucket evaluation.
|
||||
using var rsa = RSA.Create(2048);
|
||||
var req = new CertificateRequest($"CN={subjectCn}", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
|
||||
var nb = (notBefore ?? notAfter.AddYears(-1)).ToUniversalTime();
|
||||
return req.CreateSelfSigned(new DateTimeOffset(nb, TimeSpan.Zero), new DateTimeOffset(notAfter.ToUniversalTime(), TimeSpan.Zero));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -44,11 +44,47 @@ namespace AutopilotMonitor.Functions.Services
|
||||
return null;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Bucket-level evaluation for a role group (e.g. all "Root" certs).
|
||||
/// Picks the cert with the MAXIMUM NotAfter and classifies its tier;
|
||||
/// older certs in the same bucket are chain-helpers for in-flight
|
||||
/// device certs (e.g. a soon-to-expire intermediate that already has
|
||||
/// a successor in the bundle) and do NOT trigger their own alarm.
|
||||
///
|
||||
/// This is the correctness invariant: if a freshness-successor is
|
||||
/// already embedded for this role, the bundle is healthy regardless
|
||||
/// of how many older expiring/expired siblings ride along for chain
|
||||
/// continuity. The operator only gets paged when the freshest itself
|
||||
/// nears expiry, i.e. when there is genuinely no successor yet.
|
||||
/// </summary>
|
||||
internal static CertBucketEvaluation EvaluateBucket(
|
||||
IReadOnlyList<X509Certificate2> certs, DateTime nowUtc)
|
||||
{
|
||||
if (certs.Count == 0)
|
||||
return new CertBucketEvaluation(null, null, 0);
|
||||
|
||||
var freshest = certs
|
||||
.OrderByDescending(c => c.NotAfter.ToUniversalTime())
|
||||
.First();
|
||||
var daysLeft = (int)Math.Floor(
|
||||
(freshest.NotAfter.ToUniversalTime() - nowUtc).TotalDays);
|
||||
var tier = ClassifyCertExpiryTier(daysLeft);
|
||||
|
||||
return new CertBucketEvaluation(tier, freshest, daysLeft);
|
||||
}
|
||||
|
||||
internal sealed record CertBucketEvaluation(
|
||||
string? EventType,
|
||||
X509Certificate2? Freshest,
|
||||
int DaysUntilExpiry);
|
||||
|
||||
/// <summary>
|
||||
/// Inspects the embedded Intune CA bundle (roots + intermediates) and
|
||||
/// emits Warning/Error/Critical OpsEvents for any cert near expiry.
|
||||
/// Dedup: one event per cert per tier per UTC day, by querying today's
|
||||
/// Security ops events.
|
||||
/// emits Warning/Error/Critical OpsEvents when the FRESHEST cert in
|
||||
/// each role bucket nears expiry. Older expiring siblings stay silent
|
||||
/// as long as a successor is present (they're chain-continuity helpers).
|
||||
/// Dedup: one event per role-bucket per tier per UTC day, by querying
|
||||
/// today's Security ops events.
|
||||
/// </summary>
|
||||
public async Task CheckEmbeddedCertExpiryAsync()
|
||||
{
|
||||
@@ -58,49 +94,45 @@ namespace AutopilotMonitor.Functions.Services
|
||||
|
||||
if (roots.Count == 0)
|
||||
{
|
||||
if (seenToday.Add($"EmbeddedCertBundleEmpty|"))
|
||||
if (seenToday.Add("EmbeddedCertBundleEmpty|"))
|
||||
await _opsEventService.RecordEmbeddedCertBundleEmptyAsync();
|
||||
_logger.LogError("Embedded Intune root bundle is empty - agent mTLS will fail closed");
|
||||
return;
|
||||
}
|
||||
|
||||
await EvaluateCertBucketAsync(roots, "Root", seenToday);
|
||||
await EvaluateCertBucketAsync(intermediates, "Intermediate", seenToday);
|
||||
await EmitBucketAlertIfNeededAsync(roots, "Root", seenToday);
|
||||
await EmitBucketAlertIfNeededAsync(intermediates, "Intermediate", seenToday);
|
||||
}
|
||||
|
||||
private async Task EvaluateCertBucketAsync(
|
||||
private async Task EmitBucketAlertIfNeededAsync(
|
||||
IReadOnlyList<X509Certificate2> certs, string role, HashSet<string> seenToday)
|
||||
{
|
||||
var now = DateTime.UtcNow;
|
||||
foreach (var cert in certs)
|
||||
var eval = EvaluateBucket(certs, DateTime.UtcNow);
|
||||
if (eval.EventType is null || eval.Freshest is null) return;
|
||||
|
||||
var freshest = eval.Freshest;
|
||||
var dedupKey = $"{eval.EventType}|{freshest.Thumbprint}";
|
||||
if (!seenToday.Add(dedupKey))
|
||||
{
|
||||
var notAfterUtc = cert.NotAfter.ToUniversalTime();
|
||||
var daysLeft = (int)Math.Floor((notAfterUtc - now).TotalDays);
|
||||
var eventType = ClassifyCertExpiryTier(daysLeft);
|
||||
if (eventType is null) continue;
|
||||
_logger.LogDebug("Cert bucket alert already emitted today: {Key}", dedupKey);
|
||||
return;
|
||||
}
|
||||
|
||||
var dedupKey = $"{eventType}|{cert.Thumbprint}";
|
||||
if (!seenToday.Add(dedupKey))
|
||||
{
|
||||
_logger.LogDebug("Cert expiry already emitted today: {Key}", dedupKey);
|
||||
continue;
|
||||
}
|
||||
|
||||
switch (eventType)
|
||||
{
|
||||
case "EmbeddedCertExpired":
|
||||
await _opsEventService.RecordEmbeddedCertExpiredAsync(
|
||||
role, cert.Subject, cert.Thumbprint, notAfterUtc, daysLeft);
|
||||
break;
|
||||
case "EmbeddedCertExpiringUrgent":
|
||||
await _opsEventService.RecordEmbeddedCertExpiringUrgentAsync(
|
||||
role, cert.Subject, cert.Thumbprint, notAfterUtc, daysLeft);
|
||||
break;
|
||||
case "EmbeddedCertExpiringSoon":
|
||||
await _opsEventService.RecordEmbeddedCertExpiringSoonAsync(
|
||||
role, cert.Subject, cert.Thumbprint, notAfterUtc, daysLeft);
|
||||
break;
|
||||
}
|
||||
var notAfterUtc = freshest.NotAfter.ToUniversalTime();
|
||||
switch (eval.EventType)
|
||||
{
|
||||
case "EmbeddedCertExpired":
|
||||
await _opsEventService.RecordEmbeddedCertExpiredAsync(
|
||||
role, freshest.Subject, freshest.Thumbprint, notAfterUtc, eval.DaysUntilExpiry);
|
||||
break;
|
||||
case "EmbeddedCertExpiringUrgent":
|
||||
await _opsEventService.RecordEmbeddedCertExpiringUrgentAsync(
|
||||
role, freshest.Subject, freshest.Thumbprint, notAfterUtc, eval.DaysUntilExpiry);
|
||||
break;
|
||||
case "EmbeddedCertExpiringSoon":
|
||||
await _opsEventService.RecordEmbeddedCertExpiringSoonAsync(
|
||||
role, freshest.Subject, freshest.Thumbprint, notAfterUtc, eval.DaysUntilExpiry);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -89,21 +89,21 @@ namespace AutopilotMonitor.Functions.Services
|
||||
|
||||
public Task RecordEmbeddedCertExpiringSoonAsync(string role, string subject, string thumbprint, DateTime notAfterUtc, int daysUntilExpiry)
|
||||
=> WriteAsync(OpsEventCategory.Security, "EmbeddedCertExpiringSoon", OpsEventSeverity.Warning,
|
||||
$"Embedded Intune {role.ToLowerInvariant()} '{subject}' expires in {daysUntilExpiry}d ({notAfterUtc:u}) - rotate the embedded PEM bundle before {notAfterUtc:yyyy-MM-dd}",
|
||||
$"Newest embedded Intune {role.ToLowerInvariant()} '{subject}' expires in {daysUntilExpiry}d ({notAfterUtc:u}) - source a successor PEM and embed it before {notAfterUtc:yyyy-MM-dd}",
|
||||
null, "System.Maintenance",
|
||||
new { role, subject, thumbprint, notAfterUtc = notAfterUtc.ToString("u"), daysUntilExpiry });
|
||||
|
||||
public Task RecordEmbeddedCertExpiringUrgentAsync(string role, string subject, string thumbprint, DateTime notAfterUtc, int daysUntilExpiry)
|
||||
=> WriteAsync(OpsEventCategory.Security, "EmbeddedCertExpiringUrgent", OpsEventSeverity.Error,
|
||||
$"URGENT: embedded Intune {role.ToLowerInvariant()} '{subject}' expires in {daysUntilExpiry}d ({notAfterUtc:u}) - agent mTLS will break without bundle rotation",
|
||||
$"URGENT: newest embedded Intune {role.ToLowerInvariant()} '{subject}' expires in {daysUntilExpiry}d ({notAfterUtc:u}) and no successor is in the bundle - agent mTLS will break without rotation",
|
||||
null, "System.Maintenance",
|
||||
new { role, subject, thumbprint, notAfterUtc = notAfterUtc.ToString("u"), daysUntilExpiry });
|
||||
|
||||
public Task RecordEmbeddedCertExpiredAsync(string role, string subject, string thumbprint, DateTime notAfterUtc, int daysUntilExpiry)
|
||||
=> WriteAsync(OpsEventCategory.Security, "EmbeddedCertExpired", OpsEventSeverity.Critical,
|
||||
daysUntilExpiry < 0
|
||||
? $"CRITICAL: embedded Intune {role.ToLowerInvariant()} '{subject}' EXPIRED {-daysUntilExpiry}d ago ({notAfterUtc:u}) - agent mTLS validation broken"
|
||||
: $"CRITICAL: embedded Intune {role.ToLowerInvariant()} '{subject}' expires in {daysUntilExpiry}d ({notAfterUtc:u})",
|
||||
? $"CRITICAL: newest embedded Intune {role.ToLowerInvariant()} '{subject}' EXPIRED {-daysUntilExpiry}d ago ({notAfterUtc:u}) and no successor is embedded - agent mTLS validation broken"
|
||||
: $"CRITICAL: newest embedded Intune {role.ToLowerInvariant()} '{subject}' expires in {daysUntilExpiry}d ({notAfterUtc:u}) and no successor is embedded",
|
||||
null, "System.Maintenance",
|
||||
new { role, subject, thumbprint, notAfterUtc = notAfterUtc.ToString("u"), daysUntilExpiry });
|
||||
|
||||
|
||||
Reference in New Issue
Block a user