Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add manual opt-in for legacy service account tokens #357

Open
wants to merge 3 commits into
base: sig-auth-acceptance
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,12 @@ Delegating authorization flags:

Proxy flags:

--allow-legacy-serviceaccount-tokens If true, allow legacy service account tokens (without an audience). Legacy tokens are less secure and are disabled by default.
--allow-paths strings Comma-separated list of paths against which kube-rbac-proxy pattern-matches the incoming request. If the request doesn't match, kube-rbac-proxy responds with a 404 status code. If omitted, the incoming request path isn't checked. Cannot be used with --ignore-paths.
--auth-header-groups-field-name string The name of the field inside a http(2) request header to tell the upstream server about the user's groups (default "x-remote-groups")
--auth-header-groups-field-separator string The separator string used for concatenating multiple group names in a groups header field's value (default "|")
--auth-header-user-field-name string The name of the field inside a http(2) request header to tell the upstream server about the user's name (default "x-remote-user")
--auth-token-audiences strings Comma-separated list of token audiences to accept. By default a token does not have to have any specific audience. It is recommended to set a specific audience.
--auth-token-audiences strings Comma-separated list of token audiences to accept. Tokens must have at least one audience from this list. Must be set unless --allow-legacy-serviceaccount-tokens is true.
--config-file string Configuration file to configure static and rewrites authorization of the kube-rbac-proxy.
--disable-http2-serving If true, HTTP2 serving will be disabled [default=false]
--ignore-paths strings Comma-separated list of paths against which kube-rbac-proxy pattern-matches the incoming request. If the requst matches, it will proxy the request without performing an authentication or authorization check. Cannot be used with --allow-paths.
Expand Down
20 changes: 17 additions & 3 deletions cmd/kube-rbac-proxy/app/options/proxyoptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ type ProxyOptions struct {

TokenAudiences []string

AllowLegacyServiceAccountTokens bool

DisableHTTP2Serving bool
}

Expand All @@ -79,7 +81,10 @@ func (o *ProxyOptions) AddFlags(flagset *pflag.FlagSet) {
flagset.StringVar(&o.UpstreamHeader.GroupsFieldName, "auth-header-groups-field-name", "x-remote-groups", "The name of the field inside a http(2) request header to tell the upstream server about the user's groups")
flagset.StringVar(&o.UpstreamHeader.GroupSeparator, "auth-header-groups-field-separator", "|", "The separator string used for concatenating multiple group names in a groups header field's value")

flagset.StringSliceVar(&o.TokenAudiences, "auth-token-audiences", []string{}, "Comma-separated list of token audiences to accept. By default a token does not have to have any specific audience. It is recommended to set a specific audience.")
flagset.StringSliceVar(&o.TokenAudiences, "auth-token-audiences", []string{}, "Comma-separated list of token audiences to accept. Tokens must have at least one audience from this list. Must be set unless --allow-legacy-serviceaccount-tokens is true.")

// legacy tokens are disabled by default.
flagset.BoolVar(&o.AllowLegacyServiceAccountTokens, "allow-legacy-serviceaccount-tokens", false, "If true, allow legacy service account tokens (without an audience). Legacy tokens are less secure and are disabled by default.")

// proxy endpoints flag
flagset.IntVar(&o.ProxyEndpointsPort, "proxy-endpoints-port", 0, "The port to securely serve proxy-specific endpoints (such as '/healthz'). Uses the host from the '--secure-listen-address'.")
Expand All @@ -91,8 +96,10 @@ func (o *ProxyOptions) AddFlags(flagset *pflag.FlagSet) {
func (o *ProxyOptions) Validate() []error {
var errs []error

if len(o.UpstreamHeader.GroupSeparator) > 0 && len(o.UpstreamHeader.GroupsFieldName) == 0 {
errs = append(errs, fmt.Errorf("--auth-header-groups-field-name must be set along with --auth-header-groups-field-separator"))
if o.UpstreamHeader != nil {
if len(o.UpstreamHeader.GroupSeparator) > 0 && len(o.UpstreamHeader.GroupsFieldName) == 0 {
errs = append(errs, fmt.Errorf("--auth-header-groups-field-name must be set along with --auth-header-groups-field-separator"))
}
}

if len(o.AllowPaths) > 0 && len(o.IgnorePaths) > 0 {
Expand All @@ -118,6 +125,13 @@ func (o *ProxyOptions) Validate() []error {
errs = append(errs, err)
}

// If no token audiences are provided, then tokens will be legacy.
// By default, we do not allow legacy tokens unless the user explicitly opts in.
if len(o.TokenAudiences) == 0 && !o.AllowLegacyServiceAccountTokens {
errs = append(errs, fmt.Errorf("legacy service account tokens (tokens without audience) are disabled "+
"by default. Use --allow-legacy-serviceaccount-tokens to opt in"))
}

return errs
}

Expand Down
157 changes: 157 additions & 0 deletions cmd/kube-rbac-proxy/app/options/proxyoptions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/google/go-cmp/cmp"

"github.com/brancz/kube-rbac-proxy/pkg/authn/identityheaders"
authz "github.com/brancz/kube-rbac-proxy/pkg/authorization"
"github.com/brancz/kube-rbac-proxy/pkg/authorization/rewrite"
"github.com/brancz/kube-rbac-proxy/pkg/authorization/static"
Expand Down Expand Up @@ -125,3 +126,159 @@ func Test_parseAuthorizationConfigFile(t *testing.T) {
})
}
}

func TestProxyOptions_Validate(t *testing.T) {
type fields struct {
Upstream string
UpstreamForceH2C bool
UpstreamCAFile string
UpstreamClientCertFile string
UpstreamClientKeyFile string
UpstreamHeader *identityheaders.AuthnHeaderConfig
AuthzConfigFileName string
AllowPaths []string
IgnorePaths []string
ProxyEndpointsPort int
TokenAudiences []string
AllowLegacyServiceAccountTokens bool
DisableHTTP2Serving bool
Comment on lines +132 to +144
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove fields that are currently untested. Or, preferrably, write tests for the rest of the fields 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I've added additional test cases to cover those fields :)

}

const (
userKey = "User"
groupKey = "Group"
)

tests := []struct {
name string
fields fields
wantErr bool
}{
{
name: "valid config with client cert, key, config, port, and token audience",
fields: fields{
Upstream: "http://127.0.0.1",
UpstreamForceH2C: true,
UpstreamCAFile: "ca.crt",
UpstreamClientCertFile: "client.crt",
UpstreamClientKeyFile: "client.key",
AuthzConfigFileName: "authz.yaml",
AllowPaths: []string{"/path1", "/path2"},
ProxyEndpointsPort: 8081,
TokenAudiences: []string{"kube-apiserver"},
AllowLegacyServiceAccountTokens: false,
DisableHTTP2Serving: true,
UpstreamHeader: &identityheaders.AuthnHeaderConfig{
UserFieldName: userKey,
GroupsFieldName: groupKey,
},
},
wantErr: false,
},
{
name: "valid config with explicit token audience",
fields: fields{
Upstream: "http://127.0.0.1",
TokenAudiences: []string{"kube-apiserver"},
AllowLegacyServiceAccountTokens: false,
UpstreamHeader: &identityheaders.AuthnHeaderConfig{
UserFieldName: userKey,
GroupsFieldName: groupKey,
},
},
wantErr: false,
},
{
name: "legacy tokens not allowed (empty audiences, flag false)",
fields: fields{
Upstream: "http://127.0.0.1",
TokenAudiences: []string{},
AllowLegacyServiceAccountTokens: false,
UpstreamHeader: &identityheaders.AuthnHeaderConfig{
UserFieldName: userKey,
GroupsFieldName: groupKey,
},
},
wantErr: true,
},
{
name: "legacy tokens allowed (empty audiences, flag true)",
fields: fields{
Upstream: "http://127.0.0.1",
TokenAudiences: []string{},
AllowLegacyServiceAccountTokens: true,
UpstreamHeader: &identityheaders.AuthnHeaderConfig{
UserFieldName: userKey,
GroupsFieldName: groupKey,
},
},
wantErr: false,
},
{
name: "invalid combination (both AllowPaths and IgnorePaths set)",
fields: fields{
Upstream: "http://127.0.0.1",
AllowPaths: []string{"/path1"},
IgnorePaths: []string{"/path2"},
TokenAudiences: []string{"kube-apiserver"},
AllowLegacyServiceAccountTokens: false,
UpstreamHeader: &identityheaders.AuthnHeaderConfig{
UserFieldName: userKey,
GroupsFieldName: groupKey,
},
},
wantErr: true,
},
{
name: "invalid AllowPaths",
fields: fields{
Upstream: "http://127.0.0.1",
AllowPaths: []string{"[path"},
TokenAudiences: []string{"kube-apiserver"},
AllowLegacyServiceAccountTokens: false,
UpstreamHeader: &identityheaders.AuthnHeaderConfig{
UserFieldName: userKey,
GroupsFieldName: groupKey,
},
},
wantErr: true,
},
{
name: "invalid IgnorePaths",
fields: fields{
Upstream: "http://127.0.0.1",
IgnorePaths: []string{"path\\"},
TokenAudiences: []string{"kube-apiserver"},
AllowLegacyServiceAccountTokens: false,
UpstreamHeader: &identityheaders.AuthnHeaderConfig{
UserFieldName: userKey,
GroupsFieldName: groupKey,
},
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
o := &ProxyOptions{
Upstream: tt.fields.Upstream,
UpstreamForceH2C: tt.fields.UpstreamForceH2C,
UpstreamCAFile: tt.fields.UpstreamCAFile,
UpstreamClientCertFile: tt.fields.UpstreamClientCertFile,
UpstreamClientKeyFile: tt.fields.UpstreamClientKeyFile,
UpstreamHeader: tt.fields.UpstreamHeader,
AuthzConfigFileName: tt.fields.AuthzConfigFileName,
AllowPaths: tt.fields.AllowPaths,
IgnorePaths: tt.fields.IgnorePaths,
ProxyEndpointsPort: tt.fields.ProxyEndpointsPort,
TokenAudiences: tt.fields.TokenAudiences,
AllowLegacyServiceAccountTokens: tt.fields.AllowLegacyServiceAccountTokens,
DisableHTTP2Serving: tt.fields.DisableHTTP2Serving,
}
errs := o.Validate()
if (len(errs) > 0) != tt.wantErr {
t.Errorf("Validate() errors = %v, wantErr %v", errs, tt.wantErr)
}
})
}
}
1 change: 1 addition & 0 deletions test/e2e/allowpaths/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ spec:
- "--proxy-endpoints-port=8643"
- "--upstream=http://127.0.0.1:8081/"
- "--allow-paths=/metrics,/api/v1/label/*"
- "--allow-legacy-serviceaccount-tokens=true"
Copy link
Collaborator

@stlaz stlaz Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add e2e tests for scenarios that actually use the legacy token according to #357 (review)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops completely forgot about this comment. will do, thanks for the reminder!

- "--authentication-skip-lookup"
- "--v=10"
ports:
Expand Down
124 changes: 124 additions & 0 deletions test/e2e/basics.go
Original file line number Diff line number Diff line change
Expand Up @@ -519,3 +519,127 @@ func testIgnorePaths(client kubernetes.Interface) kubetest.TestSuite {
}.Run(t)
}
}

func testLegacyTokenFlag(client kubernetes.Interface) kubetest.TestSuite {
return func(t *testing.T) {
legacyCommand := `curl --connect-timeout 5 -v -s -k --fail -H "Authorization: Bearer $(cat /var/run/secrets/tokens/requestedtoken)" https://kube-rbac-proxy.default.svc.cluster.local:8443/metrics`
command := `curl --connect-timeout 5 -v -s -k --fail -H "Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)" https://kube-rbac-proxy.default.svc.cluster.local:8443/metrics`

kubetest.Scenario{
Name: "LegacyAllowedAudienceSet",
Description: `
As a client with --allow-legacy-serviceaccount-tokens=true and a valid audience,
I succeed with my request
`,

Given: kubetest.Actions(
kubetest.CreatedManifests(
client,
"legacy/clusterRole.yaml",
"legacy/clusterRoleBinding.yaml",
"legacy/deployment-allowed.yaml",
"legacy/service.yaml",
"legacy/serviceAccount.yaml",
"legacy/clusterRole-client.yaml",
"legacy/clusterRoleBinding-client.yaml",
),
),
When: kubetest.Actions(
kubetest.PodsAreReady(
client,
1,
"app=kube-rbac-proxy",
),
kubetest.ServiceIsReady(
client,
"kube-rbac-proxy",
),
),
Then: kubetest.Actions(
kubetest.ClientSucceeds(
client,
legacyCommand,
&kubetest.RunOptions{TokenAudience: "kube-rbac-proxy"},
),
),
}.Run(t)

kubetest.Scenario{
Name: "LegacyDisallowedAudienceSet",
Description: `
As a client with --allow-legacy-serviceaccount-tokens=false and a valid audience,
I succeed with my request
`,

Given: kubetest.Actions(
kubetest.CreatedManifests(
client,
"legacy/clusterRole.yaml",
"legacy/clusterRoleBinding.yaml",
"legacy/deployment-disallowed.yaml",
"legacy/service.yaml",
"legacy/serviceAccount.yaml",
"legacy/clusterRole-client.yaml",
"legacy/clusterRoleBinding-client.yaml",
),
),
When: kubetest.Actions(
kubetest.PodsAreReady(
client,
1,
"app=kube-rbac-proxy",
),
kubetest.ServiceIsReady(
client,
"kube-rbac-proxy",
),
),
Then: kubetest.Actions(
kubetest.ClientSucceeds(
client,
legacyCommand,
&kubetest.RunOptions{TokenAudience: "kube-rbac-proxy"},
),
),
}.Run(t)

kubetest.Scenario{
Name: "LegacyAllowedNoAudience",
Description: `
As a client with --allow-legacy-serviceaccount-tokens=true and no audience set,
I succeed with my request
`,

Given: kubetest.Actions(
kubetest.CreatedManifests(
client,
"legacy/clusterRole.yaml",
"legacy/clusterRoleBinding.yaml",
"legacy/deployment-allowed-noaud.yaml",
"legacy/service.yaml",
"legacy/serviceAccount.yaml",
"legacy/clusterRole-client.yaml",
"legacy/clusterRoleBinding-client.yaml",
),
),
When: kubetest.Actions(
kubetest.PodsAreReady(
client,
1,
"app=kube-rbac-proxy",
),
kubetest.ServiceIsReady(
client,
"kube-rbac-proxy",
),
),
Then: kubetest.Actions(
kubetest.ClientSucceeds(
client,
command,
&kubetest.RunOptions{TokenAudience: ""},
),
),
}.Run(t)
}
}
1 change: 1 addition & 0 deletions test/e2e/basics/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ spec:
args:
- "--secure-port=8443"
- "--upstream=http://127.0.0.1:8081/"
- "--allow-legacy-serviceaccount-tokens=true"
- "--authentication-skip-lookup"
- "--v=10"
ports:
Expand Down
1 change: 1 addition & 0 deletions test/e2e/clientcertificates/deployment-wrongca.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ spec:
- "--secure-port=8443"
- "--upstream=http://127.0.0.1:8081/"
- "--client-ca-file=/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
- "--allow-legacy-serviceaccount-tokens=true"
- "--authentication-skip-lookup"
- "--v=10"
ports:
Expand Down
1 change: 1 addition & 0 deletions test/e2e/clientcertificates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ spec:
- "--secure-port=8443"
- "--upstream=http://127.0.0.1:8081/"
- "--client-ca-file=/certs/ca.crt"
- "--allow-legacy-serviceaccount-tokens=true"
- "--authentication-skip-lookup"
- "--v=10"
ports:
Expand Down
1 change: 1 addition & 0 deletions test/e2e/h2c-upstream/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ spec:
- "--secure-port=8443"
- "--upstream=http://127.0.0.1:8081/"
- "--authentication-skip-lookup"
- "--allow-legacy-serviceaccount-tokens=true"
- "--upstream-force-h2c=true"
- "--v=10"
ports:
Expand Down
Loading
Loading