* Save ssh_service.public_addr values to Server.PublicAddrs instead of discarding them (#24070)

* Use Server.PublicAddrs when checking if session MFA is required

* Deprecate server PublicAddr in favor of PublicAddrs
This commit is contained in:
Brian Joerger
2023-04-26 17:35:33 +00:00
committed by GitHub
parent 704a104397
commit 0ae212cf0a
21 changed files with 1419 additions and 1215 deletions
+5 -2
View File
@@ -704,9 +704,10 @@ message ServerSpecV2 {
reserved 8;
reserved 10;
// Addr is server host:port address
// Addr is a host:port address where this server can be reached.
string Addr = 1 [(gogoproto.jsontag) = "addr"];
// PublicAddr is the public address this cluster can be reached at.
// PublicAddr is the public address where this server can be reached.
// DELETE IN 15.0. (joerger) Deprecated in favor of public_addrs.
string PublicAddr = 2 [(gogoproto.jsontag) = "public_addr,omitempty"];
// Hostname is server hostname
string Hostname = 3 [(gogoproto.jsontag) = "hostname"];
@@ -736,6 +737,8 @@ message ServerSpecV2 {
string PeerAddr = 11 [(gogoproto.jsontag) = "peer_addr,omitempty"];
// ProxyIDs is a list of proxy IDs this server is expected to be connected to.
repeated string ProxyIDs = 12 [(gogoproto.jsontag) = "proxy_ids,omitempty"];
// PublicAddrs is a list of public addresses where this server can be reached.
repeated string public_addrs = 13;
}
// AppServerV3 represents a single proxied web app.
+30 -10
View File
@@ -49,8 +49,10 @@ type Server interface {
GetCmdLabels() map[string]CommandLabel
// SetCmdLabels sets command labels.
SetCmdLabels(cmdLabels map[string]CommandLabel)
// GetPublicAddr is an optional field that returns the public address this cluster can be reached at.
// GetPublicAddr returns a public address where this server can be reached.
GetPublicAddr() string
// GetPublicAddrs returns a list of public addresses where this server can be reached.
GetPublicAddrs() []string
// GetRotation gets the state of certificate authority rotation.
GetRotation() Rotation
// SetRotation sets the state of certificate authority rotation.
@@ -63,8 +65,8 @@ type Server interface {
String() string
// SetAddr sets server address
SetAddr(addr string)
// SetPublicAddr sets the public address this cluster can be reached at.
SetPublicAddr(string)
// SetPublicAddrs sets the public addresses where this server can be reached.
SetPublicAddrs([]string)
// SetNamespace sets server namespace
SetNamespace(namespace string)
// GetApps gets the list of applications this server is proxying.
@@ -178,9 +180,13 @@ func (s *ServerV2) Expiry() time.Time {
return s.Metadata.Expiry()
}
// SetPublicAddr sets the public address this cluster can be reached at.
func (s *ServerV2) SetPublicAddr(addr string) {
s.Spec.PublicAddr = addr
// SetPublicAddrs sets the public proxy addresses where this server can be reached.
func (s *ServerV2) SetPublicAddrs(addrs []string) {
s.Spec.PublicAddrs = addrs
// DELETE IN 15.0. (Joerger) PublicAddr deprecated in favor of PublicAddrs
if len(addrs) != 0 {
s.Spec.PublicAddr = addrs[0]
}
}
// GetName returns server name
@@ -198,9 +204,22 @@ func (s *ServerV2) GetAddr() string {
return s.Spec.Addr
}
// GetPublicAddr is an optional field that returns the public address this cluster can be reached at.
// GetPublicAddr returns a public address where this server can be reached.
func (s *ServerV2) GetPublicAddr() string {
return s.Spec.PublicAddr
addrs := s.GetPublicAddrs()
if len(addrs) != 0 {
return addrs[0]
}
return ""
}
// GetPublicAddrs returns a list of public addresses where this server can be reached.
func (s *ServerV2) GetPublicAddrs() []string {
// DELETE IN 15.0. (Joerger) PublicAddr deprecated in favor of PublicAddrs
if len(s.Spec.PublicAddrs) == 0 && s.Spec.PublicAddr != "" {
return []string{s.Spec.PublicAddr}
}
return s.Spec.PublicAddrs
}
// GetRotation gets the state of certificate authority rotation.
@@ -404,8 +423,8 @@ func (s *ServerV2) CheckAndSetDefaults() error {
if s.Spec.Addr == "" {
return trace.BadParameter(`Addr must be set when server SubKind is "openssh"`)
}
if s.Spec.PublicAddr != "" {
return trace.BadParameter(`PublicAddr must not be set when server SubKind is "openssh"`)
if len(s.GetPublicAddrs()) != 0 {
return trace.BadParameter(`PublicAddrs must not be set when server SubKind is "openssh"`)
}
if s.Spec.Hostname == "" {
return trace.BadParameter(`Hostname must be set when server SubKind is "openssh"`)
@@ -436,6 +455,7 @@ func (s *ServerV2) MatchSearch(values []string) bool {
if s.GetKind() == KindNode {
fieldVals = append(utils.MapToStrings(s.GetAllLabels()), s.GetName(), s.GetHostname(), s.GetAddr())
fieldVals = append(fieldVals, s.GetPublicAddrs()...)
if s.GetUseTunnel() {
custom = func(val string) bool {
+16 -16
View File
@@ -121,9 +121,9 @@ func TestServerCheckAndSetDefaults(t *testing.T) {
Namespace: defaults.Namespace,
},
Spec: ServerSpecV2{
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddr: "1.2.3.4:3080",
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddrs: []string{"1.2.3.4:3080"},
},
},
assertion: func(t *testing.T, s *ServerV2, err error) {
@@ -137,9 +137,9 @@ func TestServerCheckAndSetDefaults(t *testing.T) {
Namespace: defaults.Namespace,
},
Spec: ServerSpecV2{
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddr: "1.2.3.4:3080",
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddrs: []string{"1.2.3.4:3080"},
},
}
require.Equal(t, expectedServer, s)
@@ -155,9 +155,9 @@ func TestServerCheckAndSetDefaults(t *testing.T) {
Namespace: defaults.Namespace,
},
Spec: ServerSpecV2{
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddr: "1.2.3.4:3080",
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddrs: []string{"1.2.3.4:3080"},
},
},
assertion: func(t *testing.T, s *ServerV2, err error) {
@@ -170,9 +170,9 @@ func TestServerCheckAndSetDefaults(t *testing.T) {
Namespace: defaults.Namespace,
},
Spec: ServerSpecV2{
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddr: "1.2.3.4:3080",
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddrs: []string{"1.2.3.4:3080"},
},
}
require.Equal(t, expectedServer, s)
@@ -274,13 +274,13 @@ func TestServerCheckAndSetDefaults(t *testing.T) {
Namespace: defaults.Namespace,
},
Spec: ServerSpecV2{
Addr: "1.2.3.4:3022",
Hostname: "openssh-node",
PublicAddr: "1.2.3.4:80",
Addr: "1.2.3.4:3022",
Hostname: "openssh-node",
PublicAddrs: []string{"1.2.3.4:80"},
},
},
assertion: func(t *testing.T, s *ServerV2, err error) {
require.EqualError(t, err, `PublicAddr must not be set when server SubKind is "openssh"`)
require.EqualError(t, err, `PublicAddrs must not be set when server SubKind is "openssh"`)
},
},
{
+1197 -1146
View File
File diff suppressed because it is too large Load Diff
+3 -7
View File
@@ -34,7 +34,6 @@ import (
"math"
"math/big"
insecurerand "math/rand"
"net"
"os"
"sort"
"strings"
@@ -4669,13 +4668,10 @@ func (a *Server) isMFARequired(ctx context.Context, checker services.AccessCheck
if !ok {
continue
}
// Get the server address without port number.
addr, _, err := net.SplitHostPort(srv.GetAddr())
if err != nil {
addr = srv.GetAddr()
}
// Filter out any matches on labels before checking access
if n.GetName() != t.Node.Node && srv.GetHostname() != t.Node.Node && addr != t.Node.Node {
fieldVals := append(srv.GetPublicAddrs(), srv.GetName(), srv.GetHostname(), srv.GetAddr())
if !types.MatchSearch(fieldVals, []string{t.Node.Node}, nil) {
continue
}
+1 -2
View File
@@ -3362,8 +3362,7 @@ func TestListResources_WithRoles(t *testing.T) {
Labels: labels,
},
Spec: types.ServerSpecV2{
Addr: addr,
PublicAddr: addr,
Addr: addr,
},
}
+98 -1
View File
@@ -56,6 +56,7 @@ import (
"github.com/gravitational/teleport/api/observability/tracing"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/api/types/installers"
"github.com/gravitational/teleport/api/utils"
apiutils "github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/api/utils/sshutils"
"github.com/gravitational/teleport/lib/auth/mocku2f"
@@ -1716,7 +1717,7 @@ func TestIsMFARequired(t *testing.T) {
Kind: types.KindNode,
Version: types.V2,
Metadata: types.Metadata{
Name: "node-a",
Name: uuid.NewString(),
},
Spec: types.ServerSpecV2{
Hostname: "node-a",
@@ -1859,6 +1860,102 @@ func TestIsMFARequiredUnauthorized(t *testing.T) {
require.False(t, resp.Required)
}
func TestIsMFARequired_NodeMatch(t *testing.T) {
modules.SetTestModules(t, &modules.TestModules{TestBuildType: modules.BuildEnterprise})
ctx := context.Background()
srv := newTestTLSServer(t)
// Register an SSH node.
node, err := types.NewServerWithLabels(uuid.NewString(), types.KindNode, types.ServerSpecV2{
Hostname: "node-a",
Addr: "127.0.0.1:3022",
PublicAddrs: []string{"node.example.com:3022", "localhost:3022"},
}, map[string]string{"foo": "bar"})
require.NoError(t, err)
_, err = srv.Auth().UpsertNode(ctx, node)
require.NoError(t, err)
// Create a fake user with per session mfa required for all nodes.
role, err := CreateRole(ctx, srv.Auth(), "mfa-user", types.RoleSpecV6{
Options: types.RoleOptions{
RequireMFAType: types.RequireMFAType_SESSION,
},
Allow: types.RoleConditions{
Logins: []string{"mfa-user"},
NodeLabels: types.Labels{types.Wildcard: utils.Strings{types.Wildcard}},
},
})
require.NoError(t, err)
user, err := CreateUser(srv.Auth(), "mfa-user", role)
require.NoError(t, err)
cl, err := srv.NewClient(TestUser(user.GetName()))
require.NoError(t, err)
for _, tc := range []struct {
desc string
// IsMFARequired only expects a host name or ip without the port.
node string
expectMatch require.BoolAssertionFunc
}{
{
desc: "OK uuid match",
node: node.GetName(),
expectMatch: require.True,
},
{
desc: "OK host name match",
node: node.GetHostname(),
expectMatch: require.True,
},
{
desc: "OK addr match",
node: node.GetAddr(),
expectMatch: require.True,
},
{
desc: "OK public addr 1 match",
node: "node.example.com",
expectMatch: require.True,
},
{
desc: "OK public addr 2 match",
node: "localhost",
expectMatch: require.True,
},
{
desc: "NOK label match",
node: "foo",
expectMatch: require.False,
},
{
desc: "NOK unknown ip",
node: "1.2.3.4",
expectMatch: require.False,
},
{
desc: "NOK unknown addr",
node: "unknown.example.com",
expectMatch: require.False,
},
} {
tc := tc
t.Run(tc.desc, func(t *testing.T) {
t.Parallel()
resp, err := cl.IsMFARequired(ctx, &proto.IsMFARequiredRequest{
Target: &proto.IsMFARequiredRequest_Node{Node: &proto.NodeLogin{
Login: user.GetName(),
Node: tc.node,
}},
})
require.NoError(t, err)
tc.expectMatch(t, resp.Required)
})
}
}
// testOriginDynamicStored tests setting a ResourceWithOrigin via the server
// API always results in the resource being stored with OriginDynamic.
func testOriginDynamicStored(t *testing.T, setWithOrigin func(*Client, string) error, getStored func(*Server) (types.ResourceWithOrigin, error)) {
+2 -2
View File
@@ -173,8 +173,8 @@ func TestFormatAccountName(t *testing.T) {
proxies: []types.Server{
&types.ServerV2{
Spec: types.ServerSpecV2{
PublicAddr: "foo",
Version: "bar",
PublicAddrs: []string{"foo"},
Version: "bar",
},
},
},
+2
View File
@@ -2439,6 +2439,7 @@ func (process *TeleportProcess) initSSH() error {
regular.SetTracerProvider(process.TracingProvider),
regular.SetSessionController(sessionController),
regular.SetCAGetter(caGetter),
regular.SetPublicAddrs(cfg.SSH.PublicAddrs),
)
if err != nil {
return trace.Wrap(err)
@@ -3880,6 +3881,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error {
regular.SetSessionController(sessionController),
regular.SetIngressReporter(ingress.SSH, ingressReporter),
regular.SetPROXYSigner(proxySigner),
regular.SetPublicAddrs(cfg.Proxy.PublicAddrs),
)
if err != nil {
return trace.Wrap(err)
+1 -2
View File
@@ -108,8 +108,7 @@ func insertNodes(ctx context.Context, b *testing.B, svc services.Presence, nodeC
Labels: labels,
},
Spec: types.ServerSpecV2{
Addr: addr,
PublicAddr: addr,
Addr: addr,
},
}
_, err := svc.UpsertNode(ctx, node)
+27 -2
View File
@@ -139,8 +139,9 @@ func TestMatchResourceByFilters_Helper(t *testing.T) {
t.Parallel()
server, err := types.NewServerWithLabels("banana", types.KindNode, types.ServerSpecV2{
Hostname: "foo",
Addr: "bar",
Hostname: "foo",
Addr: "bar",
PublicAddrs: []string{"foo.example.com:3080"},
}, map[string]string{"env": "prod", "os": "mac"})
require.NoError(t, err)
@@ -177,6 +178,30 @@ func TestMatchResourceByFilters_Helper(t *testing.T) {
assertErr: require.NoError,
assertMatch: require.False,
},
{
name: "search keywords hostname match",
filters: MatchResourceFilter{
SearchKeywords: []string{"foo"},
},
assertErr: require.NoError,
assertMatch: require.True,
},
{
name: "search keywords addr match",
filters: MatchResourceFilter{
SearchKeywords: []string{"bar"},
},
assertErr: require.NoError,
assertMatch: require.True,
},
{
name: "search keywords public addr match",
filters: MatchResourceFilter{
SearchKeywords: []string{"foo.example.com"},
},
assertErr: require.NoError,
assertMatch: require.True,
},
{
name: "expression match",
filters: MatchResourceFilter{
+6 -1
View File
@@ -91,9 +91,14 @@ func compareServers(a, b types.Server) int {
if a.GetNamespace() != b.GetNamespace() {
return Different
}
if a.GetPublicAddr() != b.GetPublicAddr() {
if len(a.GetPublicAddrs()) != len(b.GetPublicAddrs()) {
return Different
}
for i := range a.GetPublicAddrs() {
if a.GetPublicAddrs()[i] != b.GetPublicAddrs()[i] {
return Different
}
}
r := a.GetRotation()
if !r.Matches(b.GetRotation()) {
return Different
+5 -5
View File
@@ -72,9 +72,9 @@ func TestServersCompare(t *testing.T) {
node2.Spec.Addr = "localhost:3033"
require.Equal(t, CompareServers(node, &node2), Different)
// Public addr has changed
// Proxy addr has changed
node2 = *node
node2.Spec.PublicAddr = "localhost:3033"
node2.Spec.PublicAddrs = []string{"localhost:3033"}
require.Equal(t, CompareServers(node, &node2), Different)
// Hostname has changed
@@ -159,13 +159,13 @@ func TestGuessProxyHostAndVersion(t *testing.T) {
require.Equal(t, version, proxyA.Spec.Version)
require.NoError(t, err)
// At least one proxy has public address set.
// At least one proxy has proxy address set.
proxyB := types.ServerV2{}
proxyB.Spec.PublicAddr = "test-B"
proxyB.Spec.PublicAddrs = []string{"test-B"}
proxyB.Spec.Version = "test-B"
host, version, err = GuessProxyHostAndVersion([]types.Server{&proxyA, &proxyB})
require.Equal(t, host, proxyB.Spec.PublicAddr)
require.Equal(t, host, proxyB.Spec.PublicAddrs[0])
require.Equal(t, version, proxyB.Spec.Version)
require.NoError(t, err)
}
+1 -2
View File
@@ -363,8 +363,7 @@ func NewServer(kind, name, addr, namespace string) *types.ServerV2 {
Namespace: namespace,
},
Spec: types.ServerSpecV2{
Addr: addr,
PublicAddr: addr,
Addr: addr,
},
}
}
-2
View File
@@ -1444,8 +1444,6 @@ type Node interface {
GetNamespace() string
// GetCmdLabels gets command labels
GetCmdLabels() map[string]types.CommandLabel
// GetPublicAddr is an optional field that returns the public address this cluster can be reached at.
GetPublicAddr() string
// GetRotation gets the state of certificate authority rotation.
GetRotation() types.Rotation
// GetUseTunnel gets if a reverse tunnel should be used to connect to this node.
+4 -5
View File
@@ -193,8 +193,8 @@ func TestProxyWatcher(t *testing.T) {
func newProxyServer(t *testing.T, name, addr string) types.Server {
s, err := types.NewServer(name, types.KindProxy, types.ServerSpecV2{
Addr: addr,
PublicAddr: addr,
Addr: addr,
PublicAddrs: []string{addr},
})
require.NoError(t, err)
return s
@@ -990,9 +990,8 @@ func TestNodeWatcher(t *testing.T) {
func newNodeServer(t *testing.T, name, addr string, tunnel bool) types.Server {
s, err := types.NewServer(name, types.KindNode, types.ServerSpecV2{
Addr: addr,
PublicAddr: addr,
UseTunnel: tunnel,
Addr: addr,
UseTunnel: tunnel,
})
require.NoError(t, err)
return s
+13 -2
View File
@@ -109,6 +109,7 @@ type Server struct {
advertiseAddr *utils.NetAddr
proxyPublicAddr utils.NetAddr
publicAddrs []utils.NetAddr
// server UUID gets generated once on the first start and never changes
// usually stored in a file inside the data dir
@@ -703,6 +704,14 @@ func SetCAGetter(caGetter CertAuthorityGetter) ServerOption {
}
}
// SetPublicAddrs sets the server's public addresses
func SetPublicAddrs(addrs []utils.NetAddr) ServerOption {
return func(s *Server) error {
s.publicAddrs = addrs
return nil
}
}
// New returns an unstarted server
func New(
ctx context.Context,
@@ -1009,7 +1018,7 @@ func (s *Server) getBasicInfo() *types.ServerV2 {
addr = s.AdvertiseAddr()
}
return &types.ServerV2{
srv := &types.ServerV2{
Kind: types.KindNode,
Version: types.V2,
Metadata: types.Metadata{
@@ -1026,6 +1035,9 @@ func (s *Server) getBasicInfo() *types.ServerV2 {
ProxyIDs: s.connectedProxyGetter.GetProxyIDs(),
},
}
srv.SetPublicAddrs(utils.NetAddrsToStrings(s.publicAddrs))
return srv
}
func (s *Server) getServerInfo() *types.ServerV2 {
@@ -1042,7 +1054,6 @@ func (s *Server) getServerInfo() *types.ServerV2 {
}
server.SetExpiry(s.clock.Now().UTC().Add(apidefaults.ServerAnnounceTTL))
server.SetPublicAddr(s.proxyPublicAddr.String())
server.SetPeerAddr(s.peerAddr)
return server
}
+1
View File
@@ -7254,6 +7254,7 @@ func createProxy(ctx context.Context, t *testing.T, proxyID string, node *regula
regular.SetLockWatcher(proxyLockWatcher),
regular.SetNodeWatcher(proxyNodeWatcher),
regular.SetSessionController(sessionController),
regular.SetPublicAddrs([]utils.NetAddr{{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}}),
)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, proxyServer.Close()) })
+4 -4
View File
@@ -353,7 +353,7 @@ func TestGetNodeJoinScript(t *testing.T) {
m := &mockedNodeAPIGetter{
mockGetProxyServers: func() ([]types.Server, error) {
var s types.ServerV2
s.SetPublicAddr("test-host:12345678")
s.SetPublicAddrs([]string{"test-host:12345678"})
return []types.Server{&s}, nil
},
@@ -469,7 +469,7 @@ func TestGetAppJoinScript(t *testing.T) {
},
mockGetProxyServers: func() ([]types.Server, error) {
var s types.ServerV2
s.SetPublicAddr("test-host:12345678")
s.SetPublicAddrs([]string{"test-host:12345678"})
return []types.Server{&s}, nil
},
@@ -648,7 +648,7 @@ func TestGetDatabaseJoinScript(t *testing.T) {
m := &mockedNodeAPIGetter{
mockGetProxyServers: func() ([]types.Server, error) {
var s types.ServerV2
s.SetPublicAddr("test-host:12345678")
s.SetPublicAddrs([]string{"test-host:12345678"})
return []types.Server{&s}, nil
},
@@ -873,7 +873,7 @@ func TestJoinScript(t *testing.T) {
mockGetProxyServers: func() ([]types.Server, error) {
return []types.Server{
&types.ServerV2{
Spec: types.ServerSpecV2{PublicAddr: "test-host:12345678"},
Spec: types.ServerSpecV2{PublicAddrs: []string{"test-host:12345678"}},
},
}, nil
},
+2 -3
View File
@@ -124,9 +124,8 @@ func insertServers(ctx context.Context, b *testing.B, svc services.Presence, kin
Labels: labels,
},
Spec: types.ServerSpecV2{
Addr: addr,
PublicAddr: addr,
Version: teleport.Version,
Addr: addr,
Version: teleport.Version,
},
}
var err error
+1 -1
View File
@@ -97,7 +97,7 @@ func TestAuthSignKubeconfig(t *testing.T) {
Name: "proxy",
},
Spec: types.ServerSpecV2{
PublicAddr: "proxy-from-api.example.com:3080",
PublicAddrs: []string{"proxy-from-api.example.com:3080"},
},
},
},