mirror of
https://github.com/gravitational/teleport.git
synced 2026-10-11 22:49:54 +00:00
* types: Declare all app_resources fields in the proto
Declare the eight reserved AppResource fields (paths, methods, where,
allow_encoded, allow_code, allow_reason, deny_code_hint,
deny_reason_hint) and the AppResourcesExpressions role condition, and
regenerate the proto stubs, the role CRDs, the Terraform schema and
docs, and the derived equality functions. Declaring the fields before
the fine-grained matcher merges lets every client of this version name
them, so a client that round-trips a role written by a newer auth
preserves the fields instead of silently dropping them on write-back,
which would widen the rule.
Tighten IsAllowAllOnly to require every declared field besides
allow_all to be unset. Without this, a rule such as {allow_all: true,
methods: [GET]} written by a newer auth would pass the check once the
fields are named, and this version would treat a method-restricted
rule as unrestricted.
Add GetAppResourcesExpressions to the Role interface, and read it in
decideMinimalV9 so the app agent denies a request governed by a
predicate it cannot evaluate. A predicate restricts the rules it
accompanies, so a role that pairs allow_all with one is not
unrestricted, and a deny-side predicate blocks another role's
allow_all the same way a deny-side rule does. Auth already treats both
as restricting when it downgrades a role for a pre-v9 client, so
without this the agent and auth disagree about the same role.
Strip AppResourcesExpressions in the v9-to-v8 downgrade alongside
AppResources, and reject it at write, so a role can neither reach a
pre-v9 client with the restriction silently removed nor be stored
while this version cannot enforce it.
* docs: List the declared app_resources fields
Honour the docs review on the app-resources reference page. A run of
inline code-style field names inside a sentence is hard to scan, so the
eight rejected `app_resources` fields become a list, and
`app_resources_expressions` gets its own sentence rather than trailing
the same run.
* buf: Trim the reserved-deletion except comment
Drop the two sentences explaining why buf's ignore lists cannot scope the
except. The rule name and the removal condition are what a reader needs
here, and the scoping attempts are recorded on the pull request.
* types: Reword the app_resources_expressions and Methods docs
Honour the review on the proto field comments. Drop "desugared" from
`AppResourcesExpressions`, which needs the RFD to gloss, and state the
relation between the two forms directly instead. Open `Methods` with the
plural, matching `Paths` on the row above.
Regenerate the proto stubs, the role CRDs, the Terraform schema and docs.
312 lines
11 KiB
Go
312 lines
11 KiB
Go
/*
|
|
* Teleport
|
|
* Copyright (C) 2026 Gravitational, Inc.
|
|
*
|
|
* This program is free software: you can redistribute it and/or modify
|
|
* it under the terms of the GNU Affero General Public License as published by
|
|
* the Free Software Foundation, either version 3 of the License, or
|
|
* (at your option) any later version.
|
|
*
|
|
* This program is distributed in the hope that it will be useful,
|
|
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
* GNU Affero General Public License for more details.
|
|
*
|
|
* You should have received a copy of the GNU Affero General Public License
|
|
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
|
*/
|
|
|
|
package app
|
|
|
|
import (
|
|
"log/slog"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"testing"
|
|
|
|
lru "github.com/hashicorp/golang-lru/v2"
|
|
"github.com/stretchr/testify/require"
|
|
|
|
apidefaults "github.com/gravitational/teleport/api/defaults"
|
|
"github.com/gravitational/teleport/api/types"
|
|
"github.com/gravitational/teleport/lib/authz"
|
|
"github.com/gravitational/teleport/lib/services"
|
|
"github.com/gravitational/teleport/lib/tlsca"
|
|
)
|
|
|
|
func newTestRole(t *testing.T, name, version string, allow types.RoleConditions) types.Role {
|
|
t.Helper()
|
|
role, err := types.NewRoleWithVersion(name, version, types.RoleSpecV6{Allow: allow})
|
|
require.NoError(t, err)
|
|
return role
|
|
}
|
|
|
|
func TestDecideMinimalV9(t *testing.T) {
|
|
appMeta := types.Metadata{Name: "dev-app", Labels: map[string]string{"env": "dev"}}
|
|
app, err := types.NewAppV3(appMeta, types.AppSpecV3{URI: "http://localhost"})
|
|
require.NoError(t, err)
|
|
|
|
devLabels := types.Labels{"env": []string{"dev"}}
|
|
prodLabels := types.Labels{"env": []string{"prod"}}
|
|
|
|
v8Grants := newTestRole(t, "v8-grants", types.V8, types.RoleConditions{AppLabels: devLabels})
|
|
v8Other := newTestRole(t, "v8-other", types.V8, types.RoleConditions{AppLabels: prodLabels})
|
|
v9AllowAll := newTestRole(t, "v9-allow-all", types.V9, types.RoleConditions{
|
|
AppLabels: devLabels,
|
|
AppResources: []types.AppResource{{AllowAll: true}},
|
|
})
|
|
// The write path rejects such rules; a newer auth may still send one,
|
|
// and the agent must deny it.
|
|
v9NoAllowAll := newTestRole(t, "v9-no-allow-all", types.V9, types.RoleConditions{
|
|
AppLabels: devLabels,
|
|
AppResources: []types.AppResource{{}},
|
|
})
|
|
// The deny rule names a namespace that does not exist, so it is
|
|
// inert, exactly as in RoleSet.checkAccess.
|
|
v9DenyOtherNamespace := &types.RoleV6{
|
|
Metadata: types.Metadata{Name: "v9-deny-other-ns"},
|
|
Version: types.V9,
|
|
Spec: types.RoleSpecV6{
|
|
Allow: types.RoleConditions{
|
|
Namespaces: []string{apidefaults.Namespace},
|
|
AppLabels: devLabels,
|
|
},
|
|
Deny: types.RoleConditions{
|
|
Namespaces: []string{"other"},
|
|
AppLabels: devLabels,
|
|
},
|
|
},
|
|
}
|
|
v9DenyLabel := newTestRole(t, "v9-deny", types.V9, types.RoleConditions{
|
|
AppLabels: devLabels,
|
|
AppResources: []types.AppResource{{AllowAll: true}},
|
|
})
|
|
v9DenyLabel.SetAppLabels(types.Deny, devLabels)
|
|
// A role newer than v9 must deny even when it looks like plain allow_all.
|
|
v10Grants := &types.RoleV6{
|
|
Metadata: types.Metadata{Name: "v10-grants"},
|
|
Version: "v10",
|
|
Spec: types.RoleSpecV6{Allow: types.RoleConditions{
|
|
Namespaces: []string{apidefaults.Namespace},
|
|
AppLabels: devLabels,
|
|
AppResources: []types.AppResource{{AllowAll: true}},
|
|
}},
|
|
}
|
|
// Deny-side app rules must not open the app.
|
|
v9AllowAllWithDenyRules := newTestRole(t, "v9-allow-all-deny-rules", types.V9, types.RoleConditions{
|
|
AppLabels: devLabels,
|
|
AppResources: []types.AppResource{{AllowAll: true}},
|
|
})
|
|
v9AllowAllWithDenyRules.(*types.RoleV6).Spec.Deny.AppResources = []types.AppResource{{}}
|
|
// Deny rules are greedy across roles, so a deny-carrying role
|
|
// blocks another role's allow_all.
|
|
v9OtherDenyRules := newTestRole(t, "v9-other-deny-rules", types.V9, types.RoleConditions{
|
|
AppLabels: prodLabels,
|
|
})
|
|
v9OtherDenyRules.(*types.RoleV6).Spec.Deny.AppResources = []types.AppResource{{}}
|
|
// A predicate restricts the rules it accompanies, so allow_all next to
|
|
// one is not unrestricted.
|
|
v9AllowAllWithExpressions := newTestRole(t, "v9-allow-all-expressions", types.V9, types.RoleConditions{
|
|
AppLabels: devLabels,
|
|
AppResources: []types.AppResource{{AllowAll: true}},
|
|
})
|
|
v9AllowAllWithExpressions.(*types.RoleV6).Spec.Allow.AppResourcesExpressions = []string{`path.match(literal("api"))`}
|
|
v9ExpressionsOnly := newTestRole(t, "v9-expressions-only", types.V9, types.RoleConditions{
|
|
AppLabels: devLabels,
|
|
})
|
|
v9ExpressionsOnly.(*types.RoleV6).Spec.Allow.AppResourcesExpressions = []string{`path.match(literal("api"))`}
|
|
v9DenyExpressions := newTestRole(t, "v9-deny-expressions", types.V9, types.RoleConditions{
|
|
AppLabels: prodLabels,
|
|
})
|
|
v9DenyExpressions.(*types.RoleV6).Spec.Deny.AppResourcesExpressions = []string{`path.match(literal("admin"))`}
|
|
|
|
for _, tc := range []struct {
|
|
desc string
|
|
roles []types.Role
|
|
want minimalV9Decision
|
|
}{
|
|
{
|
|
desc: "only v8 grants, no enforcement",
|
|
roles: []types.Role{v8Grants},
|
|
want: minimalV9Decision{},
|
|
},
|
|
{
|
|
desc: "no role matches the app",
|
|
roles: []types.Role{v8Other},
|
|
want: minimalV9Decision{},
|
|
},
|
|
{
|
|
desc: "v9 allow_all forwards untouched",
|
|
roles: []types.Role{v9AllowAll},
|
|
want: minimalV9Decision{enforced: true, allowed: true},
|
|
},
|
|
{
|
|
desc: "v9 rule without allow_all denies",
|
|
roles: []types.Role{v9NoAllowAll},
|
|
want: minimalV9Decision{enforced: true, versionSkew: true},
|
|
},
|
|
{
|
|
desc: "v9 drops a conflicting v8 role",
|
|
roles: []types.Role{v9NoAllowAll, v8Grants},
|
|
want: minimalV9Decision{enforced: true, versionSkew: true, droppedRoles: []string{"v8-grants"}},
|
|
},
|
|
{
|
|
desc: "v9 allow_all still drops the v8 role",
|
|
roles: []types.Role{v9AllowAll, v8Grants},
|
|
want: minimalV9Decision{enforced: true, allowed: true, droppedRoles: []string{"v8-grants"}},
|
|
},
|
|
{
|
|
desc: "v9 role excluded by its own deny label",
|
|
roles: []types.Role{v9DenyLabel},
|
|
want: minimalV9Decision{},
|
|
},
|
|
{
|
|
desc: "deny rule in another namespace stays inert",
|
|
roles: []types.Role{v9DenyOtherNamespace},
|
|
want: minimalV9Decision{enforced: true},
|
|
},
|
|
{
|
|
desc: "version above v9 enforces default-deny",
|
|
roles: []types.Role{v10Grants},
|
|
want: minimalV9Decision{enforced: true, versionSkew: true},
|
|
},
|
|
{
|
|
desc: "deny app rules block allow_all",
|
|
roles: []types.Role{v9AllowAllWithDenyRules},
|
|
want: minimalV9Decision{enforced: true, versionSkew: true},
|
|
},
|
|
{
|
|
desc: "deny app rules in another role block allow_all",
|
|
roles: []types.Role{v9AllowAll, v9OtherDenyRules},
|
|
want: minimalV9Decision{enforced: true, versionSkew: true},
|
|
},
|
|
{
|
|
desc: "allow expressions block allow_all",
|
|
roles: []types.Role{v9AllowAllWithExpressions},
|
|
want: minimalV9Decision{enforced: true, versionSkew: true},
|
|
},
|
|
{
|
|
desc: "allow expressions without rules deny",
|
|
roles: []types.Role{v9ExpressionsOnly},
|
|
want: minimalV9Decision{enforced: true, versionSkew: true},
|
|
},
|
|
{
|
|
desc: "deny expressions in another role block allow_all",
|
|
roles: []types.Role{v9AllowAll, v9DenyExpressions},
|
|
want: minimalV9Decision{enforced: true, versionSkew: true},
|
|
},
|
|
} {
|
|
t.Run(tc.desc, func(t *testing.T) {
|
|
require.Equal(t, tc.want, decideMinimalV9(tc.roles, app, "alice", nil))
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestIsCORSPreflight(t *testing.T) {
|
|
newRequest := func(method string, headers map[string]string) *http.Request {
|
|
r := httptest.NewRequest(method, "http://app/", nil)
|
|
for k, v := range headers {
|
|
r.Header.Set(k, v)
|
|
}
|
|
return r
|
|
}
|
|
|
|
corsHeaders := map[string]string{
|
|
"Origin": "http://origin",
|
|
"Access-Control-Request-Method": http.MethodPost,
|
|
}
|
|
|
|
require.True(t, isCORSPreflight(newRequest(http.MethodOptions, corsHeaders)))
|
|
require.False(t, isCORSPreflight(newRequest(http.MethodGet, corsHeaders)))
|
|
require.False(t, isCORSPreflight(newRequest(http.MethodOptions, map[string]string{"Origin": "http://origin"})))
|
|
require.False(t, isCORSPreflight(newRequest(http.MethodOptions, nil)))
|
|
}
|
|
|
|
func TestEnforceMinimalV9(t *testing.T) {
|
|
appMeta := types.Metadata{Name: "dev-app", Labels: map[string]string{"env": "dev"}}
|
|
app, err := types.NewAppV3(appMeta, types.AppSpecV3{URI: "http://localhost"})
|
|
require.NoError(t, err)
|
|
|
|
devLabels := types.Labels{"env": []string{"dev"}}
|
|
|
|
// The write path rejects such rules; the agent must still deny them.
|
|
denyAll := newTestRole(t, "deny-all", types.V9, types.RoleConditions{
|
|
AppLabels: devLabels,
|
|
AppResources: []types.AppResource{{}},
|
|
})
|
|
allowAll := newTestRole(t, "allow-all", types.V9, types.RoleConditions{
|
|
AppLabels: devLabels,
|
|
AppResources: []types.AppResource{{AllowAll: true}},
|
|
})
|
|
|
|
authContext := func(roles ...types.Role) *authz.Context {
|
|
names := make([]string, len(roles))
|
|
for i, role := range roles {
|
|
names[i] = role.GetName()
|
|
}
|
|
info := &services.AccessInfo{Username: "alice", Roles: names}
|
|
checker := services.NewAccessCheckerWithRoleSet(info, "cluster", services.NewRoleSet(roles...))
|
|
return &authz.Context{
|
|
Identity: authz.WrapIdentity(tlsca.Identity{Username: "alice"}),
|
|
Checker: checker,
|
|
}
|
|
}
|
|
|
|
newHandler := func(logs *strings.Builder) *ConnectionsHandler {
|
|
v9Warned, err := lru.New[v9WarnKey, struct{}](v9WarnedSize)
|
|
require.NoError(t, err)
|
|
return &ConnectionsHandler{log: slog.New(slog.NewTextHandler(logs, nil)), v9Warned: v9Warned}
|
|
}
|
|
|
|
t.Run("denies a plain request", func(t *testing.T) {
|
|
rec := httptest.NewRecorder()
|
|
req := httptest.NewRequest(http.MethodGet, "http://dev-app/", nil)
|
|
denied, err := newHandler(&strings.Builder{}).enforceMinimalV9(rec, req, authContext(denyAll), app)
|
|
require.NoError(t, err)
|
|
require.True(t, denied)
|
|
require.Equal(t, http.StatusForbidden, rec.Code)
|
|
require.Contains(t, rec.Body.String(), denyKindRequestNotAllowed)
|
|
})
|
|
|
|
t.Run("allows allow_all", func(t *testing.T) {
|
|
rec := httptest.NewRecorder()
|
|
req := httptest.NewRequest(http.MethodGet, "http://dev-app/", nil)
|
|
denied, err := newHandler(&strings.Builder{}).enforceMinimalV9(rec, req, authContext(allowAll), app)
|
|
require.NoError(t, err)
|
|
require.False(t, denied)
|
|
require.Zero(t, rec.Body.Len(), "the handler must not write on the allow path")
|
|
})
|
|
|
|
t.Run("denies and warns once on CORS preflights", func(t *testing.T) {
|
|
logs := &strings.Builder{}
|
|
handler := newHandler(logs)
|
|
for range 2 {
|
|
rec := httptest.NewRecorder()
|
|
req := httptest.NewRequest(http.MethodOptions, "http://dev-app/", nil)
|
|
req.Header.Set("Origin", "http://origin")
|
|
req.Header.Set("Access-Control-Request-Method", http.MethodPost)
|
|
denied, err := handler.enforceMinimalV9(rec, req, authContext(denyAll), app)
|
|
require.NoError(t, err)
|
|
require.True(t, denied)
|
|
require.Equal(t, http.StatusForbidden, rec.Code)
|
|
}
|
|
require.Equal(t, 1, strings.Count(logs.String(), "Denied CORS preflight"))
|
|
})
|
|
|
|
t.Run("warns once about dropped v8 roles", func(t *testing.T) {
|
|
v8Grants := newTestRole(t, "v8-grants", types.V8, types.RoleConditions{AppLabels: devLabels})
|
|
logs := &strings.Builder{}
|
|
handler := newHandler(logs)
|
|
ctx := authContext(allowAll, v8Grants)
|
|
for range 2 {
|
|
rec := httptest.NewRecorder()
|
|
req := httptest.NewRequest(http.MethodGet, "http://dev-app/", nil)
|
|
denied, err := handler.enforceMinimalV9(rec, req, ctx, app)
|
|
require.NoError(t, err)
|
|
require.False(t, denied)
|
|
}
|
|
require.Equal(t, 1, strings.Count(logs.String(), "Dropped v8-or-older roles"))
|
|
})
|
|
}
|