Compare commits

..

2 Commits

Author SHA1 Message Date
Viktor Liu
be578368d8 Add test for bidirectional SSH rule authorized users on source peers 2026-07-15 19:16:56 +02:00
Viktor Liu
10ba4b368f Collect SSH authorized users for bidirectional rules on source peers 2026-07-15 19:16:56 +02:00
8 changed files with 57 additions and 104 deletions

View File

@@ -1039,7 +1039,12 @@ func (a *Account) GetPeerConnectionResources(ctx context.Context, peer *nbpeer.P
generateResources(rule, sourcePeers, FirewallRuleDirectionIN)
}
if peerInDestinations && rule.Protocol == PolicyRuleProtocolNetbirdSSH {
// Auth is collected when this peer serves the rule. For bidirectional
// rules the peer-in-sources side also serves inbound traffic, so it
// must be treated as a destination too.
peerServesAuth := peerInDestinations || (rule.Bidirectional && peerInSources)
if peerServesAuth && rule.Protocol == PolicyRuleProtocolNetbirdSSH {
sshEnabled = true
switch {
case len(rule.AuthorizedGroups) > 0:
@@ -1071,7 +1076,7 @@ func (a *Account) GetPeerConnectionResources(ctx context.Context, peer *nbpeer.P
default:
authorizedUsers[auth.Wildcard] = a.getAllowedUserIDs()
}
} else if peerInDestinations && policyRuleImpliesLegacySSH(rule) && peer.SSHEnabled {
} else if peerServesAuth && policyRuleImpliesLegacySSH(rule) && peer.SSHEnabled {
sshEnabled = true
authorizedUsers[auth.Wildcard] = a.getAllowedUserIDs()
}

View File

@@ -342,7 +342,12 @@ func (a *Account) getPeersGroupsPoliciesRoutes(
for _, srcGroupID := range rule.Sources {
relevantGroupIDs[srcGroupID] = a.GetGroup(srcGroupID)
}
}
// SSH auth requirements are gathered whenever this peer serves
// the rule. For bidirectional rules the peer-in-sources side
// also serves inbound traffic and must be treated as a destination.
if peerInDestinations || (rule.Bidirectional && peerInSources) {
if rule.Protocol == PolicyRuleProtocolNetbirdSSH {
switch {
case len(rule.AuthorizedGroups) > 0:

View File

@@ -230,7 +230,12 @@ func (c *NetworkMapComponents) getPeerConnectionResources(targetPeerID string) (
generateResources(rule, sourcePeers, FirewallRuleDirectionIN)
}
if peerInDestinations && rule.Protocol == PolicyRuleProtocolNetbirdSSH {
// Auth is collected when this peer serves the rule. For bidirectional
// rules the peer-in-sources side also serves inbound traffic, so it
// must be treated as a destination too.
peerServesAuth := peerInDestinations || (rule.Bidirectional && peerInSources)
if peerServesAuth && rule.Protocol == PolicyRuleProtocolNetbirdSSH {
sshEnabled = true
switch {
case len(rule.AuthorizedGroups) > 0:
@@ -261,7 +266,7 @@ func (c *NetworkMapComponents) getPeerConnectionResources(targetPeerID string) (
default:
authorizedUsers[auth.Wildcard] = c.getAllowedUserIDs()
}
} else if peerInDestinations && policyRuleImpliesLegacySSH(rule) && targetPeer.SSHEnabled {
} else if peerServesAuth && policyRuleImpliesLegacySSH(rule) && targetPeer.SSHEnabled {
sshEnabled = true
authorizedUsers[auth.Wildcard] = c.getAllowedUserIDs()
}

View File

@@ -981,6 +981,44 @@ func TestComponents_SSHAuthorizedUsersContent(t *testing.T) {
assert.True(t, hasRoot || hasAdmin, "AuthorizedUsers should contain 'root' or 'admin' machine user mapping")
}
// TestComponents_SSHAuthorizedUsersBidirectionalSource verifies that a peer
// on the sources side of a bidirectional NetbirdSSH rule receives the rule's
// authorized users. The reverse direction (destinations -> sources) makes
// the source-side peer a destination too, so it must be able to authorize
// inbound SSH from the rule's destinations.
func TestComponents_SSHAuthorizedUsersBidirectionalSource(t *testing.T) {
account, validatedPeers := scalableTestAccountWithoutDefaultPolicy(20, 2)
account.Users["user-dev"] = &types.User{Id: "user-dev", Role: types.UserRoleUser, AccountID: "test-account", AutoGroups: []string{"ssh-users"}}
account.Groups["ssh-users"] = &types.Group{ID: "ssh-users", Name: "SSH Users", Peers: []string{}}
account.Policies = append(account.Policies, &types.Policy{
ID: "policy-ssh-bidir", Name: "Bidirectional SSH", Enabled: true, AccountID: "test-account",
Rules: []*types.PolicyRule{{
ID: "rule-ssh-bidir", Name: "SSH both ways", Enabled: true,
Action: types.PolicyTrafficActionAccept, Protocol: types.PolicyRuleProtocolNetbirdSSH,
Bidirectional: true,
Sources: []string{"group-0"}, Destinations: []string{"group-1"},
AuthorizedGroups: map[string][]string{"ssh-users": {"root"}},
}},
})
nmSrc := componentsNetworkMap(account, "peer-0", validatedPeers)
require.NotNil(t, nmSrc)
assert.True(t, nmSrc.EnableSSH, "source-side peer of bidirectional SSH rule should have SSH enabled")
require.NotEmpty(t, nmSrc.AuthorizedUsers, "source-side peer should receive authorized users from bidirectional rule")
rootUsers, hasRoot := nmSrc.AuthorizedUsers["root"]
require.True(t, hasRoot, "source-side peer should map the 'root' local user")
_, hasDev := rootUsers["user-dev"]
assert.True(t, hasDev, "source-side peer should include 'user-dev' under 'root'")
nmDst := componentsNetworkMap(account, "peer-10", validatedPeers)
require.NotNil(t, nmDst)
assert.True(t, nmDst.EnableSSH, "destination-side peer should also have SSH enabled")
_, hasRoot = nmDst.AuthorizedUsers["root"]
assert.True(t, hasRoot, "destination-side peer should also map the 'root' local user")
}
// TestComponents_SSHLegacyImpliedSSH verifies that a non-SSH ALL protocol policy with
// SSHEnabled peer implies legacy SSH access.
func TestComponents_SSHLegacyImpliedSSH(t *testing.T) {

View File

@@ -1,38 +0,0 @@
package llm
import (
"regexp"
"strings"
)
// bedrockRegionPrefixes are the cross-region inference-profile prefixes that
// front a Bedrock model id (e.g. "eu.anthropic.claude-...").
var bedrockRegionPrefixes = []string{"us.", "eu.", "apac.", "global."}
// bedrockVersionSuffix matches the trailing "-vN[:N]" or "-YYYYMMDD-vN[:N]"
// version/throughput suffix of a Bedrock model id.
var bedrockVersionSuffix = regexp.MustCompile(`-(\d{8}-)?v\d+(:\d+)?$`)
// NormalizeBedrockModel strips an ARN wrapper, a cross-region inference-profile
// prefix, and the version/throughput suffix from a Bedrock model id so it
// matches the catalog/pricing key, e.g.
// "eu.anthropic.claude-sonnet-4-5-20250929-v1:0" -> "anthropic.claude-sonnet-4-5"
// and the inference-profile ARN's last segment likewise. It is the single
// source of truth shared by the request parser (which normalizes the request
// model from the URL path) and the router (which normalizes the operator's
// registered Bedrock model ids so both sides compare equal).
func NormalizeBedrockModel(modelID string) string {
m := modelID
if strings.HasPrefix(m, "arn:") {
if i := strings.LastIndex(m, "/"); i >= 0 {
m = m[i+1:]
}
}
for _, p := range bedrockRegionPrefixes {
if strings.HasPrefix(m, p) {
m = m[len(p):]
break
}
}
return bedrockVersionSuffix.ReplaceAllString(m, "")
}

View File

@@ -1,23 +0,0 @@
package llm
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestNormalizeBedrockModel(t *testing.T) {
cases := map[string]string{
"eu.anthropic.claude-sonnet-4-5-20250929-v1:0": "anthropic.claude-sonnet-4-5",
"us.anthropic.claude-haiku-4-5": "anthropic.claude-haiku-4-5",
"us.anthropic.claude-opus-4-8-20250101-v1:0": "anthropic.claude-opus-4-8",
"anthropic.claude-sonnet-4-5-20250929-v1:0": "anthropic.claude-sonnet-4-5",
"meta.llama3-3-70b-instruct-v1:0": "meta.llama3-3-70b-instruct",
"amazon.nova-pro-v1:0": "amazon.nova-pro",
// Inference-profile ARN — model id lives in the last path segment.
"arn:aws:bedrock:eu-central-1:123456789012:inference-profile/eu.anthropic.claude-sonnet-4-5-20250929-v1:0": "anthropic.claude-sonnet-4-5",
}
for in, want := range cases {
require.Equal(t, want, NormalizeBedrockModel(in), "normalize %q", in)
}
}

View File

@@ -1,30 +0,0 @@
package llm_router
import (
"testing"
"github.com/stretchr/testify/assert"
)
// TestRouteClaimsModel_BedrockNormalizesCandidate guards the fix for the native
// Bedrock routing gap: the request model reaches the router already normalized
// (the parser strips the region/inference-profile prefix and version suffix),
// so a provider registered with the raw inference-profile id must still match.
func TestRouteClaimsModel_BedrockNormalizesCandidate(t *testing.T) {
route := ProviderRoute{Bedrock: true, Models: []string{"us.anthropic.claude-haiku-4-5"}}
assert.True(t, routeClaimsModel(route, "anthropic.claude-haiku-4-5"),
"raw region-prefixed Bedrock model must match the normalized request model")
assert.False(t, routeClaimsModel(route, "anthropic.claude-opus-4-8"),
"a model outside the provider's list must not match")
// A provider registered with the already-normalized id also matches.
normalized := ProviderRoute{Bedrock: true, Models: []string{"anthropic.claude-haiku-4-5"}}
assert.True(t, routeClaimsModel(normalized, "anthropic.claude-haiku-4-5"),
"normalized Bedrock model must match")
// Non-Bedrock routes keep exact matching (no prefix stripping).
openai := ProviderRoute{Models: []string{"gpt-4o"}}
assert.True(t, routeClaimsModel(openai, "gpt-4o"), "exact model must match")
assert.False(t, routeClaimsModel(openai, "us.gpt-4o"),
"non-Bedrock routes must not strip a us. prefix")
}

View File

@@ -23,7 +23,6 @@ import (
"golang.org/x/oauth2"
"golang.org/x/oauth2/google"
"github.com/netbirdio/netbird/proxy/internal/llm"
"github.com/netbirdio/netbird/proxy/internal/middleware"
)
@@ -556,14 +555,6 @@ func routeClaimsModel(route ProviderRoute, model string) bool {
if candidate == model {
return true
}
// Bedrock request models reach the router already normalized (the parser
// strips the region / inference-profile prefix and version suffix), but
// the operator may register the raw inference-profile id (e.g.
// "us.anthropic.claude-haiku-4-5"). Normalize the candidate so both sides
// compare equal; otherwise a native Bedrock request denies as not-routable.
if route.Bedrock && llm.NormalizeBedrockModel(candidate) == model {
return true
}
}
return false
}