From 7c61a7eef18e5cbd9af6b73d36c0b752f2bf707d Mon Sep 17 00:00:00 2001 From: Brian Joerger Date: Tue, 7 Oct 2025 09:40:17 -0700 Subject: [PATCH] Fix OpenSSH EICE nodes connections (#59928) * Don't require agentless signer during CheckAndSetDefaults for OpenSSH EICE nodes. * Add CheckDefaults test. * Update lib/reversetunnel/local_cluster.go Co-authored-by: Edoardo Spadolini --------- Co-authored-by: Edoardo Spadolini --- lib/reversetunnel/localsite.go | 2 +- lib/srv/forward/sshserver.go | 13 ++-- lib/srv/forward/sshserver_test.go | 104 ++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 6 deletions(-) diff --git a/lib/reversetunnel/localsite.go b/lib/reversetunnel/localsite.go index 5f4e375c27e53..cb99a6cd0c250 100644 --- a/lib/reversetunnel/localsite.go +++ b/lib/reversetunnel/localsite.go @@ -414,7 +414,7 @@ func (s *localSite) dialAndForward(params reversetunnelclient.DialParams) (_ net ctx := s.srv.ctx if params.GetUserAgent == nil && !params.TargetServer.IsOpenSSHNode() { - return nil, trace.BadParameter("agentless node require an agent getter") + return nil, trace.BadParameter("user agent getter is required for teleport nodes (this is a bug)") } s.logger.DebugContext(ctx, "Initiating dial and forwarding request", "source_addr", logutils.StringerAttr(params.From), diff --git a/lib/srv/forward/sshserver.go b/lib/srv/forward/sshserver.go index 4437dc0115bbe..b6c96dd043f1d 100644 --- a/lib/srv/forward/sshserver.go +++ b/lib/srv/forward/sshserver.go @@ -268,14 +268,17 @@ func (s *ServerConfig) CheckDefaults() error { if s.TargetServer == nil { return trace.BadParameter("target server is required") } - if s.TargetServer.IsOpenSSHNode() { + switch s.TargetServer.GetSubKind() { + case types.SubKindTeleportNode: + if s.UserAgent == nil { + return trace.BadParameter("user agent required for teleport nodes (agentless)") + } + case types.SubKindOpenSSHNode: if s.AgentlessSigner == nil { return trace.BadParameter("agentless signer is required for OpenSSH Nodes") } - } else { - if s.UserAgent == nil { - return trace.BadParameter("user agent required for teleport nodes") - } + case types.SubKindOpenSSHEICENode: + // agentless signer is set once the forwarding server is started. } if s.TargetConn == nil { return trace.BadParameter("connection to target connection required") diff --git a/lib/srv/forward/sshserver_test.go b/lib/srv/forward/sshserver_test.go index d8cb2e0cec7e3..e46201010d898 100644 --- a/lib/srv/forward/sshserver_test.go +++ b/lib/srv/forward/sshserver_test.go @@ -23,10 +23,12 @@ import ( "crypto/ed25519" "crypto/rand" "errors" + "net" "os/user" "sync/atomic" "testing" + "github.com/jonboulle/clockwork" "github.com/stretchr/testify/require" "golang.org/x/crypto/ssh" @@ -34,7 +36,9 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/utils/keys" apisshutils "github.com/gravitational/teleport/api/utils/sshutils" + "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/fixtures" + "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/srv" "github.com/gravitational/teleport/lib/sshutils" "github.com/gravitational/teleport/lib/utils/log/logtest" @@ -276,3 +280,103 @@ func TestCheckTCPIPForward(t *testing.T) { // TODO(atburke): Add test for handleForwardedTCPIPRequest once we have // infrastructure for higher-level tests here. + +func TestServerConfigCheckDefaults(t *testing.T) { + teleportNode, err := types.NewNode("teleport-node", "", types.ServerSpecV2{}, nil) + require.NoError(t, err) + + openSSHNode, err := types.NewNode("openssh-node", types.SubKindOpenSSHNode, types.ServerSpecV2{ + Addr: "openssh.example.com:22", + Hostname: "openssh.example.com", + }, nil) + require.NoError(t, err) + + openSSHEICENode, err := types.NewEICENode(types.ServerSpecV2{ + Addr: "openssheice.example.com:22", + Hostname: "openssheice.example.com", + CloudMetadata: &types.CloudMetadata{ + AWS: &types.AWSInfo{ + AccountID: "123456789012", + InstanceID: "i-123456789012", + Region: "us-east-1", + VPCID: "vpc-abcd", + SubnetID: "subnet-123", + Integration: "teleportdev", + }, + }, + }, nil) + require.NoError(t, err) + + for _, tt := range []struct { + name string + modifyCfg func(c *ServerConfig) + errorAssertion require.ErrorAssertionFunc + }{ + { + name: "no targetServer", + modifyCfg: func(c *ServerConfig) {}, + errorAssertion: func(tt require.TestingT, err error, i ...interface{}) { + require.Error(t, err) + require.ErrorContains(t, err, "target server is required") + }, + }, { + name: "Teleport Node", + modifyCfg: func(c *ServerConfig) { + c.TargetServer = teleportNode + c.UserAgent = &sshutils.AgentChannel{} + }, + errorAssertion: require.NoError, + }, { + name: "Teleport Node no agent", + modifyCfg: func(c *ServerConfig) { + c.TargetServer = teleportNode + }, + errorAssertion: func(tt require.TestingT, err error, i ...interface{}) { + require.Error(t, err) + require.ErrorContains(t, err, "user agent required") + }, + }, { + name: "OpenSSH Node", + modifyCfg: func(c *ServerConfig) { + c.TargetServer = openSSHNode + c.AgentlessSigner = &sshutils.LegacySHA1Signer{} + }, + errorAssertion: require.NoError, + }, { + name: "OpenSSH Node no signer", + modifyCfg: func(c *ServerConfig) { + c.TargetServer = openSSHNode + }, + errorAssertion: func(tt require.TestingT, err error, i ...interface{}) { + require.Error(t, err) + require.ErrorContains(t, err, "agentless signer is required") + }, + }, { + name: "OpenSSH EICE Node", + modifyCfg: func(c *ServerConfig) { + c.TargetServer = openSSHEICENode + }, + errorAssertion: require.NoError, + }, + } { + t.Run(tt.name, func(t *testing.T) { + config := &ServerConfig{ + LocalAuthClient: &authclient.Client{}, + TargetClusterAccessPoint: &authclient.Client{}, + DataDir: "datadir", + TargetConn: &net.UnixConn{}, + SrcAddr: &net.IPAddr{}, + DstAddr: &net.IPAddr{}, + HostCertificate: &sshutils.LegacySHA1Signer{}, + Clock: clockwork.NewFakeClock(), + Emitter: &authclient.Client{}, + LockWatcher: &services.LockWatcher{}, + } + + tt.modifyCfg(config) + + err := config.CheckDefaults() + tt.errorAssertion(t, err) + }) + } +}