fix(siteruntime): CertStoreActor — dispose listed certs + reject path-traversal thumbprints (T17)
This commit is contained in:
@@ -64,6 +64,13 @@ public class CertStoreActor : ReceiveActor
|
||||
|
||||
private void HandleWrite(WriteCertToLocalStore msg)
|
||||
{
|
||||
if (!IsSafeThumbprint(msg.Thumbprint))
|
||||
{
|
||||
_log.Warning("Rejecting write for invalid thumbprint {Thumbprint}", msg.Thumbprint);
|
||||
Sender.Tell(new LocalCertOpAck(false, "invalid thumbprint", null));
|
||||
return;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
Directory.CreateDirectory(_trustedStoreDir);
|
||||
@@ -82,6 +89,13 @@ public class CertStoreActor : ReceiveActor
|
||||
|
||||
private void HandleRemove(RemoveCertFromLocalStore msg)
|
||||
{
|
||||
if (!IsSafeThumbprint(msg.Thumbprint))
|
||||
{
|
||||
_log.Warning("Rejecting remove for invalid thumbprint {Thumbprint}", msg.Thumbprint);
|
||||
Sender.Tell(new LocalCertOpAck(false, "invalid thumbprint", null));
|
||||
return;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
var path = Path.Combine(_trustedStoreDir, FileNameFor(msg.Thumbprint));
|
||||
@@ -133,7 +147,10 @@ public class CertStoreActor : ReceiveActor
|
||||
TrustedCertInfo? info = null;
|
||||
try
|
||||
{
|
||||
var cert = X509CertificateLoader.LoadCertificate(File.ReadAllBytes(file));
|
||||
// The TrustedCertInfo projection copies all needed strings/dates
|
||||
// out before this scope ends, so disposing the cert is safe and
|
||||
// releases the native handle held per loaded certificate.
|
||||
using var cert = X509CertificateLoader.LoadCertificate(File.ReadAllBytes(file));
|
||||
info = new TrustedCertInfo(
|
||||
cert.Thumbprint,
|
||||
cert.Subject,
|
||||
@@ -156,4 +173,17 @@ public class CertStoreActor : ReceiveActor
|
||||
}
|
||||
|
||||
private static string FileNameFor(string thumbprint) => $"{thumbprint}.der";
|
||||
|
||||
/// <summary>
|
||||
/// Defense-in-depth path-traversal guard. In production a thumbprint is a
|
||||
/// hex SHA1 (always safe), but <c>RemoveServerCertCommand</c> is CLI-exposed
|
||||
/// and this actor accepts the string unchecked, so a thumbprint such as
|
||||
/// <c>../../etc/foo</c> would otherwise resolve OUTSIDE the trusted-store
|
||||
/// directory once combined into a <c>.der</c> file name. Rejects empty,
|
||||
/// separator-bearing, or dot-dot thumbprints before any filesystem touch.
|
||||
/// </summary>
|
||||
private static bool IsSafeThumbprint(string thumbprint) =>
|
||||
!string.IsNullOrWhiteSpace(thumbprint)
|
||||
&& thumbprint.IndexOfAny(Path.GetInvalidFileNameChars()) < 0
|
||||
&& !thumbprint.Contains("..", StringComparison.Ordinal);
|
||||
}
|
||||
|
||||
@@ -126,4 +126,44 @@ public class CertStoreActorTests : TestKit, IDisposable
|
||||
var ack = ExpectMsg<LocalCertOpAck>();
|
||||
ack.Success.Should().BeTrue("removing an absent cert is an idempotent no-op");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Write_PathTraversalThumbprint_RejectedWithoutTouchingFilesystem()
|
||||
{
|
||||
var (derBase64, _) = BuildSelfSignedCert();
|
||||
var actor = Sys.ActorOf(Props.Create(() => new CertStoreActor(_options)));
|
||||
|
||||
// A "../escape" thumbprint would otherwise resolve to "<root>/escape.der",
|
||||
// OUTSIDE the trusted-store dir, once combined into a ".der" file name.
|
||||
actor.Tell(new WriteCertToLocalStore(derBase64, "../escape"));
|
||||
var ack = ExpectMsg<LocalCertOpAck>();
|
||||
ack.Success.Should().BeFalse("a path-traversal thumbprint must be rejected");
|
||||
ack.Error.Should().Be("invalid thumbprint");
|
||||
|
||||
// No file escaped above the trusted-store dir (the shared temp root).
|
||||
var escapeRoot = Path.GetDirectoryName(_trustedDir)!;
|
||||
File.Exists(Path.Combine(escapeRoot, "escape.der")).Should().BeFalse(
|
||||
"the actor must not write outside the trusted-store directory");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Remove_PathTraversalThumbprint_RejectedWithoutTouchingFilesystem()
|
||||
{
|
||||
// Seed a decoy file one level up to prove the rejected remove never
|
||||
// reaches the filesystem and therefore cannot delete it.
|
||||
var escapeRoot = Path.GetDirectoryName(_trustedDir)!;
|
||||
Directory.CreateDirectory(escapeRoot);
|
||||
var decoy = Path.Combine(escapeRoot, "escape.der");
|
||||
File.WriteAllBytes(decoy, new byte[] { 0x01, 0x02, 0x03 });
|
||||
|
||||
var actor = Sys.ActorOf(Props.Create(() => new CertStoreActor(_options)));
|
||||
|
||||
actor.Tell(new RemoveCertFromLocalStore("../escape"));
|
||||
var ack = ExpectMsg<LocalCertOpAck>();
|
||||
ack.Success.Should().BeFalse("a path-traversal thumbprint must be rejected");
|
||||
ack.Error.Should().Be("invalid thumbprint");
|
||||
|
||||
File.Exists(decoy).Should().BeTrue(
|
||||
"the actor must not delete files outside the trusted-store directory");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user