Skip to content

Commit 7ab146f

Browse files
tcsctcsc
authored
Implements RFD-0022 - OpenSSH-compatible Agent Forwarding (#6525) (#6773)
Prior to this change, `tsh` will only ever forward the internal key agent managed by `tsh` to a remote machine. This change allows a user to specify that `tsh` should forward either the `tsh`-internal keystore, or the system key agent at `$SSH_AUTH_SOCK`. This change also brings the `-A` command-line option into line with OpenSSH. For more info refer to RFD-0022. See-Also: #1571
1 parent 60dcb42 commit 7ab146f

File tree

7 files changed

+173
-83
lines changed

7 files changed

+173
-83
lines changed

‎integration/helpers.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -1162,6 +1162,11 @@ func (i *TeleInstance) NewUnauthenticatedClient(cfg ClientConfig) (tc *client.Te
11621162
sshProxyAddr = net.JoinHostPort(proxyHost, strconv.Itoa(cfg.Proxy.SSHPort))
11631163
}
11641164

1165+
fwdAgentMode := client.ForwardAgentNo
1166+
if cfg.ForwardAgent {
1167+
fwdAgentMode = client.ForwardAgentYes
1168+
}
1169+
11651170
cconf := &client.Config{
11661171
Username: cfg.Login,
11671172
Host: cfg.Host,
@@ -1170,7 +1175,7 @@ func (i *TeleInstance) NewUnauthenticatedClient(cfg ClientConfig) (tc *client.Te
11701175
InsecureSkipVerify: true,
11711176
KeysDir: keyDir,
11721177
SiteName: cfg.Cluster,
1173-
ForwardAgent: cfg.ForwardAgent,
1178+
ForwardAgent: fwdAgentMode,
11741179
Labels: cfg.Labels,
11751180
WebProxyAddr: webProxyAddr,
11761181
SSHProxyAddr: sshProxyAddr,

‎lib/client/api.go

+11-1
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,16 @@ func ValidateAgentKeyOption(supplied string) error {
9090
return trace.BadParameter("invalid value %q, must be one of %v", supplied, AllAddKeysOptions)
9191
}
9292

93+
// AgentForwardingMode describes how the user key agent will be forwarded
94+
// to a remote machine, if at all.
95+
type AgentForwardingMode int
96+
97+
const (
98+
ForwardAgentNo AgentForwardingMode = iota
99+
ForwardAgentYes
100+
ForwardAgentLocal
101+
)
102+
93103
var log = logrus.WithFields(logrus.Fields{
94104
trace.Component: teleport.ComponentClient,
95105
})
@@ -202,7 +212,7 @@ type Config struct {
202212
Agent agent.Agent
203213

204214
// ForwardAgent is used by the client to request agent forwarding from the server.
205-
ForwardAgent bool
215+
ForwardAgent AgentForwardingMode
206216

207217
// AuthMethods are used to login into the cluster. If specified, the client will
208218
// use them in addition to certs stored in its local agent (from disk)

‎lib/client/session.go

+21-2
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,11 @@ func (ns *NodeSession) createServerSession() (*ssh.Session, error) {
186186
// if agent forwarding was requested (and we have a agent to forward),
187187
// forward the agent to endpoint.
188188
tc := ns.nodeClient.Proxy.teleportClient
189-
if tc.ForwardAgent && tc.localAgent.Agent != nil {
190-
err = agent.ForwardToAgent(ns.nodeClient.Client, tc.localAgent.Agent)
189+
targetAgent := selectKeyAgent(tc)
190+
191+
if targetAgent != nil {
192+
log.Debugf("Forwarding Selected Key Agent")
193+
err = agent.ForwardToAgent(ns.nodeClient.Client, targetAgent)
191194
if err != nil {
192195
return nil, trace.Wrap(err)
193196
}
@@ -200,6 +203,22 @@ func (ns *NodeSession) createServerSession() (*ssh.Session, error) {
200203
return sess, nil
201204
}
202205

206+
// selectKeyAgent picks the appropriate key agent for forwarding to the
207+
// server, if any.
208+
func selectKeyAgent(tc *TeleportClient) agent.Agent {
209+
switch tc.ForwardAgent {
210+
case ForwardAgentYes:
211+
log.Debugf("Selecting System Key Agent")
212+
return tc.localAgent.sshAgent
213+
case ForwardAgentLocal:
214+
log.Debugf("Selecting local Teleport Key Agent")
215+
return tc.localAgent.Agent
216+
default:
217+
log.Debugf("No Key Agent selected")
218+
return nil
219+
}
220+
}
221+
203222
// interactiveSession creates an interactive session on the remote node, executes
204223
// the given callback on it, and waits for the session to end
205224
func (ns *NodeSession) interactiveSession(callback interactiveCallback) error {

‎lib/web/terminal.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ func (t *TerminalHandler) makeClient(ws *websocket.Conn) (*client.TeleportClient
258258
// communicate over the websocket.
259259
stream := t.asTerminalStream(ws)
260260

261-
clientConfig.ForwardAgent = true
261+
clientConfig.ForwardAgent = client.ForwardAgentLocal
262262
clientConfig.HostLogin = t.params.Login
263263
clientConfig.Namespace = t.params.Namespace
264264
clientConfig.Stdout = stream

‎tool/tsh/options.go

+60-30
Original file line numberDiff line numberDiff line change
@@ -20,40 +20,51 @@ import (
2020
"fmt"
2121
"strings"
2222

23+
"github.com/gravitational/teleport/lib/client"
2324
"github.com/gravitational/teleport/lib/utils"
2425

2526
"github.com/gravitational/trace"
2627
)
2728

29+
const (
30+
forwardAgentTextYes = "yes"
31+
forwardAgentTextNo = "no"
32+
forwardAgentTextLocal = "local"
33+
)
34+
2835
// AllOptions is a listing of all known OpenSSH options.
2936
var AllOptions = map[string]map[string]bool{
30-
"AddKeysToAgent": map[string]bool{"yes": true},
31-
"AddressFamily": map[string]bool{},
32-
"BatchMode": map[string]bool{},
33-
"BindAddress": map[string]bool{},
34-
"CanonicalDomains": map[string]bool{},
35-
"CanonicalizeFallbackLocal": map[string]bool{},
36-
"CanonicalizeHostname": map[string]bool{},
37-
"CanonicalizeMaxDots": map[string]bool{},
38-
"CanonicalizePermittedCNAMEs": map[string]bool{},
39-
"CertificateFile": map[string]bool{},
40-
"ChallengeResponseAuthentication": map[string]bool{},
41-
"CheckHostIP": map[string]bool{},
42-
"Cipher": map[string]bool{},
43-
"Ciphers": map[string]bool{},
44-
"ClearAllForwardings": map[string]bool{},
45-
"Compression": map[string]bool{},
46-
"CompressionLevel": map[string]bool{},
47-
"ConnectionAttempts": map[string]bool{},
48-
"ConnectTimeout": map[string]bool{},
49-
"ControlMaster": map[string]bool{},
50-
"ControlPath": map[string]bool{},
51-
"ControlPersist": map[string]bool{},
52-
"DynamicForward": map[string]bool{},
53-
"EscapeChar": map[string]bool{},
54-
"ExitOnForwardFailure": map[string]bool{},
55-
"FingerprintHash": map[string]bool{},
56-
"ForwardAgent": map[string]bool{"yes": true, "no": true},
37+
"AddKeysToAgent": map[string]bool{"yes": true},
38+
"AddressFamily": map[string]bool{},
39+
"BatchMode": map[string]bool{},
40+
"BindAddress": map[string]bool{},
41+
"CanonicalDomains": map[string]bool{},
42+
"CanonicalizeFallbackLocal": map[string]bool{},
43+
"CanonicalizeHostname": map[string]bool{},
44+
"CanonicalizeMaxDots": map[string]bool{},
45+
"CanonicalizePermittedCNAMEs": map[string]bool{},
46+
"CertificateFile": map[string]bool{},
47+
"ChallengeResponseAuthentication": map[string]bool{},
48+
"CheckHostIP": map[string]bool{},
49+
"Cipher": map[string]bool{},
50+
"Ciphers": map[string]bool{},
51+
"ClearAllForwardings": map[string]bool{},
52+
"Compression": map[string]bool{},
53+
"CompressionLevel": map[string]bool{},
54+
"ConnectionAttempts": map[string]bool{},
55+
"ConnectTimeout": map[string]bool{},
56+
"ControlMaster": map[string]bool{},
57+
"ControlPath": map[string]bool{},
58+
"ControlPersist": map[string]bool{},
59+
"DynamicForward": map[string]bool{},
60+
"EscapeChar": map[string]bool{},
61+
"ExitOnForwardFailure": map[string]bool{},
62+
"FingerprintHash": map[string]bool{},
63+
"ForwardAgent": map[string]bool{
64+
forwardAgentTextYes: true,
65+
forwardAgentTextNo: true,
66+
forwardAgentTextLocal: true,
67+
},
5768
"ForwardX11": map[string]bool{},
5869
"ForwardX11Timeout": map[string]bool{},
5970
"ForwardX11Trusted": map[string]bool{},
@@ -114,6 +125,25 @@ var AllOptions = map[string]map[string]bool{
114125
"XAuthLocation": map[string]bool{},
115126
}
116127

128+
func asAgentForwardingMode(s string) client.AgentForwardingMode {
129+
switch strings.ToLower(s) {
130+
case forwardAgentTextNo:
131+
return client.ForwardAgentNo
132+
133+
case forwardAgentTextYes:
134+
return client.ForwardAgentYes
135+
136+
case forwardAgentTextLocal:
137+
return client.ForwardAgentLocal
138+
139+
default:
140+
log.Errorf(
141+
"Invalid agent forwarding mode %q. Defaulting to %q",
142+
s, forwardAgentTextNo)
143+
return client.ForwardAgentNo
144+
}
145+
}
146+
117147
// Options holds parsed values of OpenSSH options.
118148
type Options struct {
119149
// AddKeysToAgent specifies whether keys should be automatically added to a
@@ -122,8 +152,8 @@ type Options struct {
122152

123153
// ForwardAgent specifies whether the connection to the authentication
124154
// agent will be forwarded to the remote machine. Supported option values
125-
// are "yes" and "no".
126-
ForwardAgent bool
155+
// are "yes", "no", and "local".
156+
ForwardAgent client.AgentForwardingMode
127157

128158
// RequestTTY specifies whether to request a pseudo-tty for the session.
129159
// Supported option values are "yes" and "no".
@@ -168,7 +198,7 @@ func parseOptions(opts []string) (Options, error) {
168198
case "AddKeysToAgent":
169199
options.AddKeysToAgent = utils.AsBool(value)
170200
case "ForwardAgent":
171-
options.ForwardAgent = utils.AsBool(value)
201+
options.ForwardAgent = asAgentForwardingMode(value)
172202
case "RequestTTY":
173203
options.RequestTTY = utils.AsBool(value)
174204
case "StrictHostKeyChecking":

‎tool/tsh/tsh.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -1729,8 +1729,9 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (*client.TeleportClient, erro
17291729
}
17301730

17311731
// If agent forwarding was specified on the command line enable it.
1732-
if cf.ForwardAgent || options.ForwardAgent {
1733-
c.ForwardAgent = true
1732+
c.ForwardAgent = options.ForwardAgent
1733+
if cf.ForwardAgent {
1734+
c.ForwardAgent = client.ForwardAgentYes
17341735
}
17351736

17361737
// If the caller does not want to check host keys, pass in a insecure host

‎tool/tsh/tsh_test.go

+71-46
Original file line numberDiff line numberDiff line change
@@ -442,76 +442,101 @@ func TestIdentityRead(t *testing.T) {
442442
}
443443

444444
func TestOptions(t *testing.T) {
445+
t.Parallel()
445446
tests := []struct {
446-
inOptions []string
447-
outError bool
448-
outOptions Options
447+
desc string
448+
inOptions []string
449+
assertError require.ErrorAssertionFunc
450+
outOptions Options
449451
}{
450-
// Valid
452+
// Generic option-parsing tests
451453
{
452-
inOptions: []string{
453-
"AddKeysToAgent yes",
454-
},
455-
outError: false,
454+
desc: "Space Delimited",
455+
inOptions: []string{"AddKeysToAgent yes"},
456+
assertError: require.NoError,
456457
outOptions: Options{
457458
AddKeysToAgent: true,
458-
ForwardAgent: false,
459+
ForwardAgent: client.ForwardAgentNo,
459460
RequestTTY: false,
460461
StrictHostKeyChecking: true,
461462
},
462-
},
463-
// Valid
464-
{
465-
inOptions: []string{
466-
"AddKeysToAgent=yes",
467-
},
468-
outError: false,
463+
}, {
464+
desc: "Equals Sign Delimited",
465+
inOptions: []string{"AddKeysToAgent=yes"},
466+
assertError: require.NoError,
469467
outOptions: Options{
470468
AddKeysToAgent: true,
471-
ForwardAgent: false,
469+
ForwardAgent: client.ForwardAgentNo,
472470
RequestTTY: false,
473471
StrictHostKeyChecking: true,
474472
},
473+
}, {
474+
desc: "Invalid key",
475+
inOptions: []string{"foo foo"},
476+
assertError: require.Error,
477+
outOptions: Options{},
478+
}, {
479+
desc: "Incomplete option",
480+
inOptions: []string{"AddKeysToAgent"},
481+
assertError: require.Error,
482+
outOptions: Options{},
475483
},
476-
// Invalid value.
484+
// AddKeysToAgent Tests
477485
{
478-
inOptions: []string{
479-
"AddKeysToAgent foo",
480-
},
481-
outError: true,
482-
outOptions: Options{},
486+
desc: "AddKeysToAgent Invalid Value",
487+
inOptions: []string{"AddKeysToAgent foo"},
488+
assertError: require.Error,
489+
outOptions: Options{},
483490
},
484-
// Invalid key.
491+
// ForwardAgent Tests
485492
{
486-
inOptions: []string{
487-
"foo foo",
493+
desc: "Forward Agent Yes",
494+
inOptions: []string{"ForwardAgent yes"},
495+
assertError: require.NoError,
496+
outOptions: Options{
497+
AddKeysToAgent: true,
498+
ForwardAgent: client.ForwardAgentYes,
499+
RequestTTY: false,
500+
StrictHostKeyChecking: true,
488501
},
489-
outError: true,
490-
outOptions: Options{},
491-
},
492-
// Incomplete option.
493-
{
494-
inOptions: []string{
495-
"AddKeysToAgent",
502+
}, {
503+
desc: "Forward Agent No",
504+
inOptions: []string{"ForwardAgent no"},
505+
assertError: require.NoError,
506+
outOptions: Options{
507+
AddKeysToAgent: true,
508+
ForwardAgent: client.ForwardAgentNo,
509+
RequestTTY: false,
510+
StrictHostKeyChecking: true,
496511
},
497-
outError: true,
498-
outOptions: Options{},
512+
}, {
513+
desc: "Forward Agent Local",
514+
inOptions: []string{"ForwardAgent local"},
515+
assertError: require.NoError,
516+
outOptions: Options{
517+
AddKeysToAgent: true,
518+
ForwardAgent: client.ForwardAgentLocal,
519+
RequestTTY: false,
520+
StrictHostKeyChecking: true,
521+
},
522+
}, {
523+
desc: "Forward Agent InvalidValue",
524+
inOptions: []string{"ForwardAgent potato"},
525+
assertError: require.Error,
526+
outOptions: Options{},
499527
},
500528
}
501529

502530
for _, tt := range tests {
503-
options, err := parseOptions(tt.inOptions)
504-
if tt.outError {
505-
require.Error(t, err)
506-
continue
507-
} else {
508-
require.NoError(t, err)
509-
}
531+
t.Run(tt.desc, func(t *testing.T) {
532+
options, err := parseOptions(tt.inOptions)
533+
tt.assertError(t, err)
510534

511-
require.Equal(t, tt.outOptions.AddKeysToAgent, options.AddKeysToAgent)
512-
require.Equal(t, tt.outOptions.ForwardAgent, options.ForwardAgent)
513-
require.Equal(t, tt.outOptions.RequestTTY, options.RequestTTY)
514-
require.Equal(t, tt.outOptions.StrictHostKeyChecking, options.StrictHostKeyChecking)
535+
require.Equal(t, tt.outOptions.AddKeysToAgent, options.AddKeysToAgent)
536+
require.Equal(t, tt.outOptions.ForwardAgent, options.ForwardAgent)
537+
require.Equal(t, tt.outOptions.RequestTTY, options.RequestTTY)
538+
require.Equal(t, tt.outOptions.StrictHostKeyChecking, options.StrictHostKeyChecking)
539+
})
515540
}
516541
}
517542

0 commit comments

Comments
 (0)