Skip to content

Commit 50fa90e

Browse files
Enforce authz(n) on controller
Give the controller the profiler endpoints and put it behind a remote authn|z story. Move the node authenticator to a reusable spot. Set a small cache size.
1 parent 284abfd commit 50fa90e

File tree

7 files changed

+139
-64
lines changed

7 files changed

+139
-64
lines changed
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package authenticator
2+
3+
import (
4+
"crypto/x509"
5+
"time"
6+
7+
"k8s.io/kubernetes/pkg/auth/authenticator"
8+
unversionedauthentication "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/authentication/internalversion"
9+
10+
oauthenticator "github.com/openshift/origin/pkg/auth/authenticator"
11+
"github.com/openshift/origin/pkg/auth/authenticator/anonymous"
12+
"github.com/openshift/origin/pkg/auth/authenticator/request/bearertoken"
13+
"github.com/openshift/origin/pkg/auth/authenticator/request/unionrequest"
14+
"github.com/openshift/origin/pkg/auth/authenticator/request/x509request"
15+
authncache "github.com/openshift/origin/pkg/auth/authenticator/token/cache"
16+
authnremote "github.com/openshift/origin/pkg/auth/authenticator/token/remotetokenreview"
17+
"github.com/openshift/origin/pkg/auth/group"
18+
"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
19+
)
20+
21+
// NewRemoteAuthenticator creates an authenticator that checks the provided remote endpoint for tokens, allows any linked clientCAs to be checked, and caches
22+
// responses as indicated. If no authentication is possible, the user will be system:anonymous.
23+
func NewRemoteAuthenticator(authenticationClient unversionedauthentication.TokenReviewsGetter, clientCAs *x509.CertPool, cacheTTL time.Duration, cacheSize int) (authenticator.Request, error) {
24+
authenticators := []oauthenticator.Request{}
25+
26+
// API token auth
27+
var (
28+
tokenAuthenticator oauthenticator.Token
29+
err error
30+
)
31+
// Authenticate against the remote master
32+
tokenAuthenticator, err = authnremote.NewAuthenticator(authenticationClient)
33+
if err != nil {
34+
return nil, err
35+
}
36+
// Cache results
37+
if cacheTTL > 0 && cacheSize > 0 {
38+
tokenAuthenticator, err = authncache.NewAuthenticator(tokenAuthenticator, cacheTTL, cacheSize)
39+
if err != nil {
40+
return nil, err
41+
}
42+
}
43+
authenticators = append(authenticators, bearertoken.New(tokenAuthenticator, true))
44+
45+
// Client-cert auth
46+
if clientCAs != nil {
47+
opts := x509request.DefaultVerifyOptions()
48+
opts.Roots = clientCAs
49+
certauth := x509request.New(opts, x509request.SubjectToUserConversion)
50+
authenticators = append(authenticators, certauth)
51+
}
52+
53+
ret := &unionrequest.Authenticator{
54+
// Anonymous requests will pass the token and cert checks without errors
55+
// Bad tokens or bad certs will produce errors, in which case we should not continue to authenticate them as "system:anonymous"
56+
FailOnError: true,
57+
Handlers: []oauthenticator.Request{
58+
// Add the "system:authenticated" group to users that pass token/cert authentication
59+
group.NewGroupAdder(unionrequest.NewUnionAuthentication(authenticators...), []string{bootstrappolicy.AuthenticatedGroup}),
60+
// Fall back to the "system:anonymous" user
61+
anonymous.NewAuthenticator(),
62+
},
63+
}
64+
65+
return ret, nil
66+
}

pkg/cmd/server/handlers/authorization.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,32 @@ import (
1313
kapierrors "k8s.io/kubernetes/pkg/api/errors"
1414
"k8s.io/kubernetes/pkg/api/unversioned"
1515
"k8s.io/kubernetes/pkg/runtime"
16+
"k8s.io/kubernetes/pkg/util/sets"
1617

1718
"github.com/openshift/origin/pkg/authorization/authorizer"
1819
)
1920

21+
type bypassAuthorizer struct {
22+
paths sets.String
23+
authorizer authorizer.Authorizer
24+
}
25+
26+
// NewBypassAuthorizer creates an Authorizer that always allows the exact paths described, and delegates to the nested
27+
// authorizer for everything else.
28+
func NewBypassAuthorizer(auth authorizer.Authorizer, paths ...string) authorizer.Authorizer {
29+
return bypassAuthorizer{paths: sets.NewString(paths...), authorizer: auth}
30+
}
31+
32+
func (a bypassAuthorizer) Authorize(ctx kapi.Context, attributes authorizer.Action) (allowed bool, reason string, err error) {
33+
if attributes.IsNonResourceURL() && a.paths.Has(attributes.GetURL()) {
34+
return true, "always allowed", nil
35+
}
36+
return a.authorizer.Authorize(ctx, attributes)
37+
}
38+
func (a bypassAuthorizer) GetAllowedSubjects(ctx kapi.Context, attributes authorizer.Action) (sets.String, sets.String, error) {
39+
return a.authorizer.GetAllowedSubjects(ctx, attributes)
40+
}
41+
2042
// AuthorizationFilter imposes normal authorization rules
2143
func AuthorizationFilter(handler http.Handler, authorizer authorizer.Authorizer, authorizationAttributeBuilder authorizer.AuthorizationAttributeBuilder, contextMapper kapi.RequestContextMapper) http.Handler {
2244
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {

pkg/cmd/server/kubernetes/node_auth.go

Lines changed: 1 addition & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,80 +1,23 @@
11
package kubernetes
22

33
import (
4-
"crypto/x509"
54
"net/http"
65
"strings"
76
"time"
87

98
"github.com/golang/glog"
109

11-
"k8s.io/kubernetes/pkg/auth/authenticator"
1210
kauthorizer "k8s.io/kubernetes/pkg/auth/authorizer"
1311
"k8s.io/kubernetes/pkg/auth/user"
14-
unversionedauthentication "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/authentication/internalversion"
15-
16-
oauthenticator "github.com/openshift/origin/pkg/auth/authenticator"
17-
"github.com/openshift/origin/pkg/auth/authenticator/anonymous"
18-
"github.com/openshift/origin/pkg/auth/authenticator/request/bearertoken"
19-
"github.com/openshift/origin/pkg/auth/authenticator/request/unionrequest"
20-
"github.com/openshift/origin/pkg/auth/authenticator/request/x509request"
21-
authncache "github.com/openshift/origin/pkg/auth/authenticator/token/cache"
22-
authnremote "github.com/openshift/origin/pkg/auth/authenticator/token/remotetokenreview"
23-
"github.com/openshift/origin/pkg/auth/group"
12+
2413
authorizationapi "github.com/openshift/origin/pkg/authorization/api"
2514
oauthorizer "github.com/openshift/origin/pkg/authorization/authorizer"
2615
authzadapter "github.com/openshift/origin/pkg/authorization/authorizer/adapter"
2716
authzcache "github.com/openshift/origin/pkg/authorization/authorizer/cache"
2817
authzremote "github.com/openshift/origin/pkg/authorization/authorizer/remote"
2918
oclient "github.com/openshift/origin/pkg/client"
30-
"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
3119
)
3220

33-
func newAuthenticator(authenticationClient unversionedauthentication.TokenReviewsGetter, clientCAs *x509.CertPool, cacheTTL time.Duration, cacheSize int) (authenticator.Request, error) {
34-
authenticators := []oauthenticator.Request{}
35-
36-
// API token auth
37-
var (
38-
tokenAuthenticator oauthenticator.Token
39-
err error
40-
)
41-
// Authenticate against the remote master
42-
tokenAuthenticator, err = authnremote.NewAuthenticator(authenticationClient)
43-
if err != nil {
44-
return nil, err
45-
}
46-
// Cache results
47-
if cacheTTL > 0 && cacheSize > 0 {
48-
tokenAuthenticator, err = authncache.NewAuthenticator(tokenAuthenticator, cacheTTL, cacheSize)
49-
if err != nil {
50-
return nil, err
51-
}
52-
}
53-
authenticators = append(authenticators, bearertoken.New(tokenAuthenticator, true))
54-
55-
// Client-cert auth
56-
if clientCAs != nil {
57-
opts := x509request.DefaultVerifyOptions()
58-
opts.Roots = clientCAs
59-
certauth := x509request.New(opts, x509request.SubjectToUserConversion)
60-
authenticators = append(authenticators, certauth)
61-
}
62-
63-
ret := &unionrequest.Authenticator{
64-
// Anonymous requests will pass the token and cert checks without errors
65-
// Bad tokens or bad certs will produce errors, in which case we should not continue to authenticate them as "system:anonymous"
66-
FailOnError: true,
67-
Handlers: []oauthenticator.Request{
68-
// Add the "system:authenticated" group to users that pass token/cert authentication
69-
group.NewGroupAdder(unionrequest.NewUnionAuthentication(authenticators...), []string{bootstrappolicy.AuthenticatedGroup}),
70-
// Fall back to the "system:anonymous" user
71-
anonymous.NewAuthenticator(),
72-
},
73-
}
74-
75-
return ret, nil
76-
}
77-
7821
func newAuthorizerAttributesGetter(nodeName string) (kauthorizer.RequestAttributesGetter, error) {
7922
return NodeAuthorizerAttributesGetter{nodeName}, nil
8023
}

pkg/cmd/server/kubernetes/node_config.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232

3333
osclient "github.com/openshift/origin/pkg/client"
3434
configapi "github.com/openshift/origin/pkg/cmd/server/api"
35+
serverauthenticator "github.com/openshift/origin/pkg/cmd/server/authenticator"
3536
"github.com/openshift/origin/pkg/cmd/server/crypto"
3637
cmdutil "github.com/openshift/origin/pkg/cmd/util"
3738
cmdflags "github.com/openshift/origin/pkg/cmd/util/flags"
@@ -237,7 +238,7 @@ func BuildKubernetesNodeConfig(options configapi.NodeConfig, enableProxy, enable
237238
if err != nil {
238239
return nil, err
239240
}
240-
authn, err := newAuthenticator(kubeClient.Authentication(), clientCAs, authnTTL, options.AuthConfig.AuthenticationCacheSize)
241+
authn, err := serverauthenticator.NewRemoteAuthenticator(kubeClient.Authentication(), clientCAs, authnTTL, options.AuthConfig.AuthenticationCacheSize)
241242
if err != nil {
242243
return nil, err
243244
}

pkg/cmd/server/origin/master.go

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ import (
3939
"k8s.io/kubernetes/pkg/util/sets"
4040
utilwait "k8s.io/kubernetes/pkg/util/wait"
4141

42+
authzcache "github.com/openshift/origin/pkg/authorization/authorizer/cache"
43+
authzremote "github.com/openshift/origin/pkg/authorization/authorizer/remote"
4244
buildclient "github.com/openshift/origin/pkg/build/client"
4345
buildgenerator "github.com/openshift/origin/pkg/build/generator"
4446
buildregistry "github.com/openshift/origin/pkg/build/registry/build"
@@ -49,6 +51,7 @@ import (
4951
"github.com/openshift/origin/pkg/build/webhook"
5052
"github.com/openshift/origin/pkg/build/webhook/generic"
5153
"github.com/openshift/origin/pkg/build/webhook/github"
54+
serverauthenticator "github.com/openshift/origin/pkg/cmd/server/authenticator"
5255
"github.com/openshift/origin/pkg/cmd/server/crypto"
5356
serverhandlers "github.com/openshift/origin/pkg/cmd/server/handlers"
5457
cmdutil "github.com/openshift/origin/pkg/cmd/util"
@@ -328,14 +331,38 @@ func (c *MasterConfig) buildHandlerChain(assetConfig *AssetConfig) (func(http.Ha
328331
}, messages, nil
329332
}
330333

331-
func (c *MasterConfig) RunHealth() {
334+
func (c *MasterConfig) RunHealth() error {
332335
apiContainer := genericmux.NewAPIContainer(http.NewServeMux(), kapi.Codecs)
333336

334337
healthz.InstallHandler(&apiContainer.NonSwaggerRoutes, healthz.PingHealthz)
335338
initReadinessCheckRoute(apiContainer, "/healthz/ready", func() bool { return true })
336-
initMetricsRoute(apiContainer, "/metrics")
339+
genericroutes.Profiling{}.Install(apiContainer)
340+
genericroutes.MetricsWithReset{}.Install(apiContainer)
337341

338-
c.serve(apiContainer.ServeMux, []string{"Started health checks at %s"})
342+
// TODO: replace me with a service account for controller manager
343+
authn, err := serverauthenticator.NewRemoteAuthenticator(c.PrivilegedLoopbackKubernetesClientset.Authentication(), c.APIClientCAs, 5*time.Minute, 10)
344+
if err != nil {
345+
return err
346+
}
347+
authz, err := authzremote.NewAuthorizer(c.PrivilegedLoopbackOpenShiftClient)
348+
if err != nil {
349+
return err
350+
}
351+
authz, err = authzcache.NewAuthorizer(authz, 5*time.Minute, 10)
352+
if err != nil {
353+
return err
354+
}
355+
// we use direct bypass to allow readiness and health to work regardless of the master health
356+
authz = serverhandlers.NewBypassAuthorizer(authz, "/healthz", "/healthz/ready")
357+
contextMapper := c.getRequestContextMapper()
358+
handler := serverhandlers.AuthorizationFilter(apiContainer.ServeMux, authz, c.AuthorizationAttributeBuilder, contextMapper)
359+
handler = serverhandlers.AuthenticationHandlerFilter(handler, authn, contextMapper)
360+
handler = kgenericfilters.WithPanicRecovery(handler, contextMapper)
361+
handler = kapiserverfilters.WithRequestInfo(handler, genericapiserver.NewRequestInfoResolver(&genericapiserver.Config{}), contextMapper)
362+
handler = kapi.WithRequestContext(handler, contextMapper)
363+
364+
c.serve(handler, []string{"Started health checks at %s"})
365+
return nil
339366
}
340367

341368
// serve starts serving the provided http.Handler using security settings derived from the MasterConfig

pkg/cmd/server/start/start_master.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,8 +450,7 @@ func (m *Master) Start() error {
450450
}
451451

452452
func startHealth(openshiftConfig *origin.MasterConfig) error {
453-
openshiftConfig.RunHealth()
454-
return nil
453+
return openshiftConfig.RunHealth()
455454
}
456455

457456
// StartAPI starts the components of the master that are considered part of the API - the Kubernetes

test/cmd/authentication.sh

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ fi
1212
(
1313
set +e
1414
oc delete oauthaccesstokens --all
15+
oadm policy remove-cluster-role-from-user cluster-debugger user3
1516
exit 0
1617
) &>/dev/null
1718

@@ -78,7 +79,23 @@ os::cmd::expect_success_and_text "oc policy can-i create pods --token='${accesst
7879
os::cmd::expect_success_and_text "oc policy can-i --list --token='${accesstoken}' -n '${project}' --scopes='role:admin:*'" 'get.*pods'
7980
os::cmd::expect_success_and_not_text "oc policy can-i --list --token='${accesstoken}' -n '${project}'" 'get.*pods'
8081

82+
os::test::junit::declare_suite_end
8183

84+
os::test::junit::declare_suite_start "cmd/authentication/debugging"
85+
os::cmd::expect_success_and_text 'oc login -u user3 -p pw' 'Login successful'
86+
os::cmd::expect_success 'oc login -u system:admin'
87+
os::cmd::expect_failure_and_text 'oc get --raw /debug/pprof/ --as=user3' 'Forbidden'
88+
os::cmd::expect_failure_and_text 'oc get --raw /metrics --as=user3' 'Forbidden'
89+
os::cmd::expect_success_and_text 'oc get --raw /healthz --as=user3' 'ok'
90+
os::cmd::expect_success 'oadm policy add-cluster-role-to-user cluster-debugger user3'
91+
os::cmd::try_until_text 'oc get --raw /debug/pprof/ --as=user3' 'full goroutine stack dump'
92+
os::cmd::expect_success_and_text 'oc get --raw /debug/pprof/ --as=user3' 'full goroutine stack dump'
93+
os::cmd::expect_success_and_text 'oc get --raw /metrics --as=user3' 'apiserver_request_latencies'
94+
os::cmd::expect_success_and_text 'oc get --raw /healthz --as=user3' 'ok'
95+
# TODO validate controller
8296
os::test::junit::declare_suite_end
8397

98+
os::test::junit::declare_suite_start "cmd/authentication/scopedtokens"
99+
100+
84101
os::test::junit::declare_suite_end

0 commit comments

Comments
 (0)