Public Access
Let a connection be reached through a proxy on this machine's loopback
Step 1 of docs/reaching-a-host-you-cannot-dial.md, and it is not the step that document said it was. SshConnectionRequest carries an optional SshLoopbackProxy and BuildConnectionInfo hands SSH.NET its proxy ConnectionInfo when there is one. Nothing passes one yet: the callers are jump hosts and the relay, which are steps 2 and 3. ◆ THE BRIDGE WAS THE WRONG FIRST STEP, AND BUILDING IT WOULD HAVE BEEN THE MISTAKE THIS DOCUMENT IS ABOUT. ADR 0004 says the relay's loopback bridge "also provides ProxyJump via a SOCKS5 dynamic forward — one mechanism, two features", and the plan took that to mean the bridge was the shared foundation. It is not: ForwardedPortDynamic *is* the listener for a jump host — SSH.NET accepts on it, speaks SOCKS5 on it and tunnels through the bastion — so nothing is left for a bridge of ours to do on that path. The relay is the case with no SshClient to hang a forward off, so it is the bridge's only consumer, and the bridge belongs in the commit that uses it. What the two actually share is one level down and a tenth of the size: being told to reach a target through a loopback proxy while staying about the target. That is what this is. Three properties, one test each. A port and nothing else, so a proxy anywhere but loopback cannot be expressed. The failure that shape rules out is an open SOCKS proxy on the user's network for the life of a shell, which nothing would report — so it is made unrepresentable rather than validated, on the same grounds AuthenticationChoice carries a kind. SOCKS5 rather than a dumb pipe, which is what keeps host key pinning honest. The target's own name and port stay in the request, travel to the proxy in the CONNECT, and are what the gate pins — so a machine reached through a bastion is pinned under its own name instead of under 127.0.0.1 on whatever ephemeral port that day's forward got, which is not an identity at all. A pipe would have meant handing SSH.NET a stand-in and remembering everywhere else that it was one. And a proxy that is not listening fails as a connection error rather than as an unknown host key. The gate turns "no host key seen" into a fingerprint prompt, and a connection that never reached a server has seen none either; the prompt would offer to fix the wrong thing, with no fingerprint to show. TWO THINGS THE TESTS MEASURED RATHER THAN ASSUMED, both found by the first run failing. The target is resolved at the *bastion*, not here — a SOCKS CONNECT names it and the far end looks it up. So the test asks for localhost:2222, the address inside the container, and the published port this host would use means nothing there. That is not a quirk of the fixture; it is what ProxyJump means, and it is why an ssh_config writes the target's internal address beside its jump host. Getting it wrong is a SOCKS "general failure" that names neither end. And the test server refuses forwarding. linuxserver/openssh-server ships AllowTcpForwarding no, which a dynamic forward does not notice — opening one asks the server nothing — so every connection through it is refused at channel-open and reported as the same general failure. The fixture patches it and HUPs sshd. There are two sshd_config files in that image and the running server uses /config/sshd/sshd_config; the first attempt patched /etc/ssh/sshd_config, which is the one a search finds first, changed the text and nothing else, and left the failure exactly where it was. VERIFIED. Build clean with no new warnings, 85 tests in Client.Ssh.Tests against the real sshd, and the solution builds. The proxy test was seen to fail — proxy.Port + 1 in BuildConnectionInfo — and seen green again. An earlier mutation attempt did not compile, and the log said 85 passing because the run never started and the previous log was still on disk; the second attempt deletes the log first, which is worth copying whenever a mutation "passes". dotnet format reports one pre-existing IDE1006 in DodoSSH.Api/Features/Events/EventsEndpoint.cs, in a project nothing here touches. Left alone.
This commit is contained in:
@@ -0,0 +1,225 @@
|
||||
using System.Net;
|
||||
using System.Net.Sockets;
|
||||
using Renci.SshNet;
|
||||
|
||||
namespace DodoSSH.Client.Ssh.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Reaching a host through a SOCKS5 proxy on loopback, which is how this client will reach one it cannot
|
||||
/// dial: through a bastion, or through the server relay.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <b>One container, used as both ends.</b> The fixture's sshd is the bastion <em>and</em> the target — a
|
||||
/// dynamic forward is opened on a connection to it, and the connection under test goes back to the same
|
||||
/// server through that forward. Two containers would look more like the real topology and would test
|
||||
/// nothing extra: what is being established is that the request's proxy is honoured, that the target is
|
||||
/// what gets pinned, and that a failure on the way through is reported as itself. None of the three is
|
||||
/// about the far end being a different machine.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The forward is SSH.NET's own <c>ForwardedPortDynamic</c>, which is what the jump-host path will use in
|
||||
/// earnest — so this is not a stub standing in for the eventual proxy, it is the eventual proxy. The relay
|
||||
/// will put a bridge of this repository's own on the same loopback interface and speak the same protocol
|
||||
/// to it. See <c>docs/reaching-a-host-you-cannot-dial.md</c>.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
[Collection(SshCollection.Name)]
|
||||
public sealed class LoopbackProxyTests(SshServerFixture fixture)
|
||||
{
|
||||
private static CancellationToken Token => TestContext.Current.CancellationToken;
|
||||
|
||||
/// <summary>
|
||||
/// The whole of what this change buys, and the property that makes it safe.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// A connection through the proxy has to arrive, and it has to arrive <em>as the target</em>. The pin is
|
||||
/// keyed on the host and port the request names, so if the proxy's address leaked into that identity
|
||||
/// every machine reached through a bastion would be pinned as <c>127.0.0.1</c> on whatever ephemeral
|
||||
/// port that day's forward happened to get — which is not an identity at all, and would mean a trusted
|
||||
/// first contact for anything reached the same way afterwards.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// ◆ <b>The target is named as the bastion can reach it, not as this machine can.</b> The forward runs
|
||||
/// inside the container, so the address in the CONNECT request is resolved there —
|
||||
/// <c>localhost:2222</c> — and the published port this test host would use means nothing in that
|
||||
/// namespace. That is not a quirk of the fixture: it is what <c>ProxyJump</c> means, and it is why an
|
||||
/// <c>ssh_config</c> writes the target's *internal* address beside its jump host. Getting it wrong is a
|
||||
/// SOCKS "general failure" from the bastion, which is what the first draft of this test collected.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Started from an empty store, so the assertion is not merely that the right string was recorded: the
|
||||
/// same server is unknown under this identity until it is trusted under it, and being trusted under its
|
||||
/// direct name would not do. Both halves of that are the point.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task AHostReachedThroughAProxy_ConnectsAndIsPinnedUnderItsOwnName()
|
||||
{
|
||||
using var bastion = OpenBastion();
|
||||
using var forward = StartDynamicForward(bastion);
|
||||
|
||||
var request = ThroughTheBastion(forward);
|
||||
|
||||
var knownHosts = new InMemoryKnownHostStore();
|
||||
var factory = new SshNetConnectionFactory(knownHosts);
|
||||
|
||||
var unknown = await Should.ThrowAsync<SshHostKeyUnknownException>(async () =>
|
||||
await factory.ConnectAsync(request, Token));
|
||||
|
||||
unknown.Presentation.Host.ShouldBe(InternalHost, "the target's name, not the proxy's");
|
||||
unknown.Presentation.Port.ShouldBe(SshServerFixture.InternalPort);
|
||||
((int)forward.BoundPort).ShouldNotBe(
|
||||
SshServerFixture.InternalPort, "or the two identities would be indistinguishable");
|
||||
|
||||
await knownHosts.TrustAsync(unknown.Presentation, Token);
|
||||
|
||||
await using var connection = await factory.ConnectAsync(request, Token);
|
||||
|
||||
connection.IsConnected.ShouldBeTrue();
|
||||
connection.HostKey.Host.ShouldBe(InternalHost);
|
||||
|
||||
// Authenticated is not the same as usable, and a proxied transport is exactly where a channel might
|
||||
// not open: everything from here is SSH.NET's own framing over a socket it did not dial itself.
|
||||
await using var shell = await connection.OpenShellAsync(TerminalSize.Default, Token);
|
||||
|
||||
shell.IsOpen.ShouldBeTrue();
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// The forward binds an ephemeral port and reports it, which is the one thing about
|
||||
/// <c>ForwardedPortDynamic</c> this code depends on and the XML documentation does not state. Held here
|
||||
/// so that an SSH.NET that stopped filling it in fails by name instead of leaving the test above
|
||||
/// dialling port zero and reporting a connection error.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public void ADynamicForward_ReportsThePortItWasGiven()
|
||||
{
|
||||
using var bastion = OpenBastion();
|
||||
using var forward = StartDynamicForward(bastion);
|
||||
|
||||
forward.BoundHost.ShouldBe("127.0.0.1", "a SOCKS proxy on any other interface is an open proxy");
|
||||
forward.BoundPort.ShouldBeGreaterThan(0u, "an ephemeral bind has to report what it got");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A proxy that is not there is a connection failure, and must not be dressed up as a host key problem.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The same misreport <c>KeyAuthenticationTests</c> guards for authentication, one layer lower and
|
||||
/// easier to get wrong: the gate translates a refusal into <see cref="SshHostKeyUnknownException"/> on
|
||||
/// the strength of having seen no host key, and a connection that never reached a server has seen none
|
||||
/// either. Showing a fingerprint prompt for an unreachable bastion would offer to fix the wrong thing —
|
||||
/// and there would be no fingerprint to show.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task AProxyThatIsNotListening_FailsAsAConnectionErrorRatherThanAnUnknownHostKey()
|
||||
{
|
||||
// An empty store, so the wrong answer is available: had the connection reached a server, this is
|
||||
// exactly the setup that produces SshHostKeyUnknownException. It never gets that far.
|
||||
var request = Request(Credential(), new SshLoopbackProxy(DeadPort()));
|
||||
|
||||
var failure = await Should.ThrowAsync<Exception>(async () =>
|
||||
await new SshNetConnectionFactory(new InMemoryKnownHostStore()).ConnectAsync(request, Token));
|
||||
|
||||
failure.ShouldNotBeOfType<SshHostKeyUnknownException>();
|
||||
failure.ShouldNotBeOfType<SshHostKeyMismatchException>();
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// The connection every other test in this assembly makes, asserted once to be unchanged: the proxy is
|
||||
/// an optional last parameter, so a request that names none has to build the connection it always did.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task AHostWithNoProxy_IsStillDialledDirectly()
|
||||
{
|
||||
var knownHosts = await TrustedStoreAsync();
|
||||
|
||||
await using var connection = await new SshNetConnectionFactory(knownHosts)
|
||||
.ConnectAsync(Request(Credential(), proxy: null), Token);
|
||||
|
||||
connection.IsConnected.ShouldBeTrue();
|
||||
}
|
||||
|
||||
/// <summary>A port nothing is listening on, found by binding one and letting it go.</summary>
|
||||
/// <remarks>
|
||||
/// Racy in principle and not in practice: nothing else in this process binds ephemeral ports, and the
|
||||
/// consequence of losing the race is a connection that succeeds where the test wanted a refusal, which
|
||||
/// fails the assertion rather than passing quietly.
|
||||
/// </remarks>
|
||||
private static int DeadPort()
|
||||
{
|
||||
using var probe = new TcpListener(IPAddress.Loopback, 0);
|
||||
|
||||
probe.Start();
|
||||
|
||||
var port = ((IPEndPoint)probe.LocalEndpoint).Port;
|
||||
|
||||
probe.Stop();
|
||||
|
||||
return port;
|
||||
}
|
||||
|
||||
private SshClient OpenBastion()
|
||||
{
|
||||
var client = new SshClient(
|
||||
fixture.Host,
|
||||
fixture.Port,
|
||||
SshServerFixture.Username,
|
||||
SshServerFixture.Password);
|
||||
|
||||
client.Connect();
|
||||
|
||||
return client;
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// Bound to <c>127.0.0.1</c> explicitly. The single-argument constructor's default is undocumented, and
|
||||
/// the failure it would produce if that default is <c>0.0.0.0</c> is not a test failure — it is a SOCKS
|
||||
/// proxy into the developer's network, open for as long as the connection lives, that nothing would
|
||||
/// report. The test above asserts the bound host for the same reason.
|
||||
/// </remarks>
|
||||
private static ForwardedPortDynamic StartDynamicForward(SshClient bastion)
|
||||
{
|
||||
var forward = new ForwardedPortDynamic("127.0.0.1", 0);
|
||||
|
||||
bastion.AddForwardedPort(forward);
|
||||
forward.Start();
|
||||
|
||||
return forward;
|
||||
}
|
||||
|
||||
/// <summary>What the container calls itself, which is the only name the forward inside it can resolve.</summary>
|
||||
private const string InternalHost = "localhost";
|
||||
|
||||
private static SshPasswordCredential Credential() => new(SshServerFixture.Password);
|
||||
|
||||
private SshConnectionRequest Request(SshCredential credential, SshLoopbackProxy? proxy) =>
|
||||
new(fixture.Host, fixture.Port, SshServerFixture.Username, credential, ConnectTimeout: null, proxy);
|
||||
|
||||
/// <summary>The same server, addressed as the machine running the forward can reach it.</summary>
|
||||
private static SshConnectionRequest ThroughTheBastion(ForwardedPortDynamic forward) =>
|
||||
new(
|
||||
InternalHost,
|
||||
SshServerFixture.InternalPort,
|
||||
SshServerFixture.Username,
|
||||
Credential(),
|
||||
ConnectTimeout: null,
|
||||
new SshLoopbackProxy((int)forward.BoundPort));
|
||||
|
||||
/// <summary>A store that already trusts the container's host key, so first contact is not the subject.</summary>
|
||||
/// <remarks>Learned by being refused, which is the only way this client learns a host key.</remarks>
|
||||
private async Task<InMemoryKnownHostStore> TrustedStoreAsync()
|
||||
{
|
||||
var knownHosts = new InMemoryKnownHostStore();
|
||||
|
||||
var unknown = await Should.ThrowAsync<SshHostKeyUnknownException>(async () =>
|
||||
await new SshNetConnectionFactory(knownHosts)
|
||||
.ConnectAsync(Request(Credential(), proxy: null), Token));
|
||||
|
||||
await knownHosts.TrustAsync(unknown.Presentation, Token);
|
||||
|
||||
return knownHosts;
|
||||
}
|
||||
}
|
||||
@@ -27,6 +27,17 @@ public sealed class SshServerFixture : IAsyncLifetime
|
||||
/// <summary>Password for password authentication.</summary>
|
||||
public const string Password = "correct-horse-battery-staple";
|
||||
|
||||
/// <summary>
|
||||
/// The port sshd listens on <em>inside</em> the container, rather than the one it is published on.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// What anything reaching this server from within the container's own network namespace has to use —
|
||||
/// which includes a forward opened on a connection to it. <see cref="Port"/> is the mapped port on the
|
||||
/// host and is not listening in there. See <c>LoopbackProxyTests</c>, where the difference is the whole
|
||||
/// shape of the test.
|
||||
/// </remarks>
|
||||
public const int InternalPort = SshPort;
|
||||
|
||||
private const int SshPort = 2222;
|
||||
|
||||
private readonly SemaphoreSlim sftpGate = new(1, 1);
|
||||
@@ -69,6 +80,53 @@ public sealed class SshServerFixture : IAsyncLifetime
|
||||
.Build();
|
||||
|
||||
await container.StartAsync();
|
||||
await AllowTcpForwardingAsync();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Lets this server open the direct-tcpip channels a forward is made of.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// ◆ <b>The image ships <c>AllowTcpForwarding no</c>, and nothing says so at the point it bites.</b> A
|
||||
/// dynamic forward starts perfectly happily — it is a local listener, and opening it asks the server
|
||||
/// nothing — and then every connection through it is refused when the channel is opened. SSH.NET
|
||||
/// reports that as <c>SOCKS5: General failure</c> from the proxy, which names neither the server nor
|
||||
/// the setting, and is what the first run of <c>LoopbackProxyTests</c> collected.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Patched after start rather than baked in, because the image's entrypoint writes its configuration
|
||||
/// itself on every boot — a mounted file would be overwritten before sshd read it. sshd re-reads on
|
||||
/// <c>SIGHUP</c> and applies the result to connections made after that, and the readiness wait has
|
||||
/// already run, so nothing here races the boot.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// ◆ <b><c>/config/sshd/sshd_config</c>, and there are two.</b> The image also carries
|
||||
/// <c>/etc/ssh/sshd_config</c>, which looks like the file to patch, reads identically, and is not the
|
||||
/// one the running server was started with — patching it changes the text and nothing else, which is a
|
||||
/// fix that appears to work and leaves the failure exactly where it was. Measured with <c>find</c>
|
||||
/// rather than assumed, after the first version of this method did precisely that.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// It is on for the whole assembly rather than for the one test that needs it. Forwarding is off in
|
||||
/// this image as hardening, not as a behaviour worth reproducing: nothing else here opens a channel of
|
||||
/// any kind, so allowing it changes what exactly one suite can do and what none of the others see.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
private async Task AllowTcpForwardingAsync()
|
||||
{
|
||||
var result = await container!.ExecAsync([
|
||||
"sh",
|
||||
"-c",
|
||||
"sed -i 's/^AllowTcpForwarding no/AllowTcpForwarding yes/' /config/sshd/sshd_config"
|
||||
+ " && pkill -HUP sshd",
|
||||
]);
|
||||
|
||||
if (result.ExitCode != 0)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
$"Could not enable TCP forwarding on the test server: {result.Stderr}");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
Reference in New Issue
Block a user