Harden the read path, forwarded headers and security docs
Caddy Manager CI build / docker (push) Failing after 1m2s
Caddy Manager CI build / docker (push) Failing after 1m2s
Found while investigating an unrelated Gitea compromise: CaddyManager itself was not involved, but reviewing it turned up three things worth closing. Reading a configuration was the only file operation that did not validate the name. Saving, renaming and deleting all reject `..`, `/` and `\`, so the read path was the one way to leave the configuration directory and pull in any `*.caddy` file on the host. The HTTP API happened to be covered, because GET checks the name against the directory listing first, but the UI calls the service directly and nothing stopped it. Forwarded headers were trusted from any peer. That is correct only while the container port is unreachable except through the proxy; the moment it is published, a caller dictates the scheme, host and client address the app believes in. Loopback and private space cover a proxy on a Docker network or on the host, which is the documented deployment, and ignore everyone else. The README never said that the `X-Api-Key` check guards `/api/*` and nothing else, so the UI - which rewrites Caddyfiles and holds the Docker socket - reads as protected when it is not. It now says so, and warns about the specific shape that bit us: a second hostname added for machine callers whose only extra directive is a `tls` line, which serves the unauthenticated UI to anyone who can resolve it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -210,6 +210,64 @@ public class CaddyServiceTests : IDisposable
|
||||
result.Should().Be(string.Empty);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Tests that the Caddy service refuses to read a configuration whose name walks out of the configuration directory.
|
||||
/// Setup: Writes a .caddy file in the parent of the configuration directory and asks for it through a traversing name.
|
||||
/// Expectation: The service should return an empty string rather than the file's content, so a caller cannot use the read path to reach files the configuration directory does not own — saving, renaming and deleting already reject the same names.
|
||||
/// </summary>
|
||||
[Theory]
|
||||
[InlineData("../outside")]
|
||||
[InlineData("../../outside")]
|
||||
[InlineData("subdir/../../outside")]
|
||||
public void GetCaddyConfigurationContent_WithTraversingName_ReturnsEmptyString(string configurationName)
|
||||
{
|
||||
// Arrange
|
||||
var parentDir = Directory.GetParent(_tempConfigDir)!.FullName;
|
||||
var outsidePath = Path.Combine(parentDir, "outside.caddy");
|
||||
File.WriteAllText(outsidePath, "secret { reverse_proxy 10.0.0.1:80 }");
|
||||
|
||||
try
|
||||
{
|
||||
// Act
|
||||
var result = _service.GetCaddyConfigurationContent(configurationName);
|
||||
|
||||
// Assert
|
||||
result.Should().Be(string.Empty);
|
||||
}
|
||||
finally
|
||||
{
|
||||
File.Delete(outsidePath);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Tests that an absolute path given as a configuration name cannot escape the configuration directory.
|
||||
/// Setup: Passes an absolute path to a .caddy file that exists outside the configuration directory.
|
||||
/// Expectation: The service should return an empty string, because Path.Combine would otherwise discard the configuration directory entirely and read the absolute path.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void GetCaddyConfigurationContent_WithAbsolutePath_ReturnsEmptyString()
|
||||
{
|
||||
// Arrange
|
||||
var parentDir = Directory.GetParent(_tempConfigDir)!.FullName;
|
||||
var outsidePath = Path.Combine(parentDir, "absolute-outside.caddy");
|
||||
File.WriteAllText(outsidePath, "secret { reverse_proxy 10.0.0.1:80 }");
|
||||
|
||||
try
|
||||
{
|
||||
// Act
|
||||
var result = _service.GetCaddyConfigurationContent(
|
||||
Path.Combine(parentDir, "absolute-outside"));
|
||||
|
||||
// Assert
|
||||
result.Should().Be(string.Empty);
|
||||
}
|
||||
finally
|
||||
{
|
||||
File.Delete(outsidePath);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Tests that the Caddy service correctly retrieves the content of the global Caddyfile configuration.
|
||||
/// Setup: Creates a global Caddyfile with known content in the configuration directory.
|
||||
|
||||
Reference in New Issue
Block a user