Skip to content

Commit

Permalink
Fix key principals not being used when identity files are being used (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Alex McGrath authored Apr 7, 2022
1 parent 52e7ace commit 32cb76e
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,10 @@ type Config struct {
// against Teleport client and obtaining credentials from elsewhere.
SkipLocalAuth bool

// UseKeyPrincipals forces the use of the username from the key principals rather than using
// the current user username.
UseKeyPrincipals bool

// Agent is used when SkipLocalAuth is true
Agent agent.Agent

Expand Down Expand Up @@ -2166,7 +2170,7 @@ func (tc *TeleportClient) getProxySSHPrincipal() string {
proxyPrincipal = tc.JumpHosts[0].Username
}
// see if we already have a signed key in the cache, we'll use that instead
if !tc.Config.SkipLocalAuth && tc.localAgent != nil {
if (!tc.Config.SkipLocalAuth || tc.UseKeyPrincipals) && tc.localAgent != nil {
signers, err := tc.localAgent.Signers()
if err != nil || len(signers) == 0 {
return proxyPrincipal
Expand Down
51 changes: 51 additions & 0 deletions lib/client/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@ limitations under the License.
package client

import (
"io"
"os"
"testing"

"github.com/gravitational/teleport/api/client/webclient"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/trace"
"golang.org/x/crypto/ssh"
"golang.org/x/crypto/ssh/agent"

"github.com/stretchr/testify/require"
"gopkg.in/check.v1"
Expand Down Expand Up @@ -499,6 +503,53 @@ func TestApplyProxySettings(t *testing.T) {
}
}

type mockAgent struct {
// Agent is embedded to avoid redeclaring all interface methods.
// Only the Signers method is implemented by testAgent.
agent.Agent
ValidPrincipals []string
}

type mockSigner struct {
ValidPrincipals []string
}

func (s *mockSigner) PublicKey() ssh.PublicKey {
return &ssh.Certificate{
ValidPrincipals: s.ValidPrincipals,
}
}

func (s *mockSigner) Sign(rand io.Reader, b []byte) (*ssh.Signature, error) {
return nil, trace.Errorf("mockSigner does not implement Sign")
}

// Signers implements agent.Agent.Signers.
func (m *mockAgent) Signers() ([]ssh.Signer, error) {
return []ssh.Signer{&mockSigner{ValidPrincipals: m.ValidPrincipals}}, nil
}

func TestNewClient_UseKeyPrincipals(t *testing.T) {
cfg := &Config{
Username: "xyz",
HostLogin: "xyz",
WebProxyAddr: "localhost",
SkipLocalAuth: true,
UseKeyPrincipals: true, // causes VALID to be returned, as key was used
Agent: &mockAgent{ValidPrincipals: []string{"VALID"}},
AuthMethods: []ssh.AuthMethod{ssh.Password("xyz") /* placeholder authmethod */},
}
client, err := NewClient(cfg)
require.NoError(t, err)
require.Equal(t, "VALID", client.getProxySSHPrincipal(), "ProxySSHPrincipal mismatch")

cfg.UseKeyPrincipals = false // causes xyz to be returned as key was not used

client, err = NewClient(cfg)
require.NoError(t, err)
require.Equal(t, "xyz", client.getProxySSHPrincipal(), "ProxySSHPrincipal mismatch")
}

func TestParseSearchKeywords(t *testing.T) {
t.Parallel()

Expand Down
4 changes: 4 additions & 0 deletions tool/tsh/tsh.go
Original file line number Diff line number Diff line change
Expand Up @@ -1910,6 +1910,10 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (*client.TeleportClient, erro
if cf.IdentityFileIn != "" {
// Ignore local authentication methods when identity file is provided
c.SkipLocalAuth = true
// Force the use of the certificate principals so Unix
// username does not get used when logging in
c.UseKeyPrincipals = hostLogin == ""

var (
key *client.Key
identityAuth ssh.AuthMethod
Expand Down

0 comments on commit 32cb76e

Please sign in to comment.