Skip to content

Commit feefce6

Browse files
smarterclaytonsmarterclayton
committed
Router metrics should be protected by delegated auth
Allow additional authorization checking from the master by performing delegating auth when the router metrics endpoint is accessed. Use the cmux library to put HTTP and HTTPS on the same port so that we can avoid complex setup requirements for routers which must be HTTP available for healthchecks, but should be HTTPS protected for accepting client authentication. Uses the delegating authorizer to protect the endpoint with the SAR for group: route.openshift.io resource: routers name: <router-name> subresource: metrics for /metrics and debug for /debug
1 parent a8af212 commit feefce6

File tree

2 files changed

+197
-32
lines changed

2 files changed

+197
-32
lines changed

Diff for: ‎pkg/cmd/infra/router/template.go

+60-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package router
22

33
import (
4+
"crypto/tls"
45
"errors"
56
"fmt"
67
"net"
@@ -17,9 +18,16 @@ import (
1718
ktypes "k8s.io/apimachinery/pkg/types"
1819
"k8s.io/apimachinery/pkg/util/sets"
1920
"k8s.io/apimachinery/pkg/util/validation"
21+
"k8s.io/apiserver/pkg/authentication/authenticatorfactory"
22+
"k8s.io/apiserver/pkg/authorization/authorizer"
23+
"k8s.io/apiserver/pkg/authorization/authorizerfactory"
24+
"k8s.io/apiserver/pkg/server/healthz"
25+
authenticationclient "k8s.io/client-go/kubernetes/typed/authentication/v1beta1"
26+
authorizationclient "k8s.io/client-go/kubernetes/typed/authorization/v1beta1"
2027
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
2128
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
2229

30+
"github.com/openshift/origin/pkg/cmd/server/crypto"
2331
"github.com/openshift/origin/pkg/cmd/util"
2432
"github.com/openshift/origin/pkg/cmd/util/clientcmd"
2533
ocmd "github.com/openshift/origin/pkg/oc/cli/cmd"
@@ -248,9 +256,6 @@ func (o *TemplateRouterOptions) Run() error {
248256
statsPort := o.StatsPort
249257
switch {
250258
case o.MetricsType == "haproxy" && statsPort != 0:
251-
if len(o.StatsUsername) == 0 || len(o.StatsPassword) == 0 {
252-
glog.Warningf("Metrics were requested but no username or password has been provided - the metrics endpoint will not be accessible to prevent accidental security breaches")
253-
}
254259
// Exposed to allow tuning in production if this becomes an issue
255260
var timeout time.Duration
256261
if t := util.Env("ROUTER_METRICS_HAPROXY_TIMEOUT", ""); len(t) > 0 {
@@ -316,7 +321,58 @@ func (o *TemplateRouterOptions) Run() error {
316321
if useProxy := util.Env("ROUTER_USE_PROXY_PROTOCOL", ""); useProxy == "true" || useProxy == "TRUE" {
317322
check = metrics.ProxyProtocolHTTPBackendAvailable(u)
318323
}
319-
metrics.Listen(o.ListenAddr, o.StatsUsername, o.StatsPassword, check)
324+
325+
kubeconfig := o.Config.KubeConfig()
326+
client, err := authorizationclient.NewForConfig(kubeconfig)
327+
if err != nil {
328+
return err
329+
}
330+
authz, err := authorizerfactory.DelegatingAuthorizerConfig{
331+
SubjectAccessReviewClient: client.SubjectAccessReviews(),
332+
AllowCacheTTL: 2 * time.Minute,
333+
DenyCacheTTL: 5 * time.Second,
334+
}.New()
335+
if err != nil {
336+
return err
337+
}
338+
tokenClient, err := authenticationclient.NewForConfig(kubeconfig)
339+
if err != nil {
340+
return err
341+
}
342+
authn, _, err := authenticatorfactory.DelegatingAuthenticatorConfig{
343+
Anonymous: true,
344+
TokenAccessReviewClient: tokenClient.TokenReviews(),
345+
CacheTTL: 10 * time.Second,
346+
ClientCAFile: util.Env("ROUTER_METRICS_AUTHENTICATOR_CA_FILE", ""),
347+
}.New()
348+
if err != nil {
349+
return err
350+
}
351+
l := metrics.Listener{
352+
Addr: o.ListenAddr,
353+
Username: o.StatsUsername,
354+
Password: o.StatsPassword,
355+
Authenticator: authn,
356+
Authorizer: authz,
357+
Record: authorizer.AttributesRecord{
358+
ResourceRequest: true,
359+
APIGroup: "route.openshift.io",
360+
Resource: "routers",
361+
Name: o.RouterName,
362+
},
363+
Checks: []healthz.HealthzChecker{check},
364+
}
365+
if certFile := util.Env("ROUTER_METRICS_TLS_CERT_FILE", ""); len(certFile) > 0 {
366+
certificate, err := tls.LoadX509KeyPair(certFile, util.Env("ROUTER_METRICS_TLS_KEY_FILE", ""))
367+
if err != nil {
368+
return err
369+
}
370+
l.TLSConfig = crypto.SecureTLSConfig(&tls.Config{
371+
Certificates: []tls.Certificate{certificate},
372+
ClientAuth: tls.RequestClientCert,
373+
})
374+
}
375+
l.Listen()
320376
}
321377

322378
pluginCfg := templateplugin.TemplatePluginConfig{

Diff for: ‎pkg/router/metrics/metrics.go

+137-28
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,156 @@
11
package metrics
22

33
import (
4+
"crypto/tls"
5+
"fmt"
6+
"net"
47
"net/http"
58
"net/http/pprof"
9+
"strings"
610

7-
"k8s.io/apiserver/pkg/server/healthz"
8-
11+
"github.com/cockroachdb/cmux"
912
"github.com/golang/glog"
1013
"github.com/prometheus/client_golang/prometheus"
14+
15+
"k8s.io/apiserver/pkg/server/healthz"
16+
17+
"k8s.io/apiserver/pkg/authentication/authenticator"
18+
"k8s.io/apiserver/pkg/authorization/authorizer"
1119
)
1220

21+
type Listener struct {
22+
Addr string
23+
24+
TLSConfig *tls.Config
25+
26+
Username string
27+
Password string
28+
29+
Authenticator authenticator.Request
30+
Authorizer authorizer.Authorizer
31+
Record authorizer.AttributesRecord
32+
33+
Checks []healthz.HealthzChecker
34+
}
35+
36+
func (l Listener) handler() http.Handler {
37+
mux := http.NewServeMux()
38+
healthz.InstallHandler(mux, l.Checks...)
39+
40+
if l.Authenticator != nil {
41+
protected := http.NewServeMux()
42+
protected.HandleFunc("/debug/pprof/", pprof.Index)
43+
protected.HandleFunc("/debug/pprof/profile", pprof.Profile)
44+
protected.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
45+
protected.Handle("/metrics", prometheus.Handler())
46+
mux.Handle("/", l.authorizeHandler(protected))
47+
}
48+
return mux
49+
}
50+
51+
func (l Listener) authorizeHandler(protected http.Handler) http.Handler {
52+
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
53+
if len(l.Username) > 0 || len(l.Password) > 0 {
54+
if u, p, ok := req.BasicAuth(); ok {
55+
if u == l.Username && p == l.Password {
56+
protected.ServeHTTP(w, req)
57+
} else {
58+
http.Error(w, fmt.Sprintf("Unauthorized"), http.StatusUnauthorized)
59+
}
60+
return
61+
}
62+
}
63+
64+
user, ok, err := l.Authenticator.AuthenticateRequest(req)
65+
if err != nil {
66+
glog.V(3).Infof("Unable to authenticate: %v", err)
67+
http.Error(w, "Unable to authenticate due to an error", http.StatusInternalServerError)
68+
return
69+
}
70+
scopedRecord := l.Record
71+
switch req.Method {
72+
case "POST":
73+
scopedRecord.Verb = "create"
74+
case "GET", "HEAD":
75+
scopedRecord.Verb = "get"
76+
case "PUT":
77+
scopedRecord.Verb = "update"
78+
case "PATCH":
79+
scopedRecord.Verb = "patch"
80+
case "DELETE":
81+
scopedRecord.Verb = "delete"
82+
default:
83+
scopedRecord.Verb = ""
84+
}
85+
switch {
86+
case req.URL.Path == "/metrics":
87+
scopedRecord.Subresource = "metrics"
88+
case strings.HasPrefix(req.URL.Path, "/debug/"):
89+
scopedRecord.Subresource = "debug"
90+
}
91+
scopedRecord.User = user
92+
ok, reason, err := l.Authorizer.Authorize(scopedRecord)
93+
if err != nil {
94+
glog.V(3).Infof("Unable to authenticate: %v", err)
95+
http.Error(w, "Unable to authenticate due to an error", http.StatusInternalServerError)
96+
return
97+
}
98+
if !ok {
99+
http.Error(w, fmt.Sprintf("Unauthorized %s", reason), http.StatusUnauthorized)
100+
return
101+
}
102+
protected.ServeHTTP(w, req)
103+
})
104+
}
105+
13106
// Listen starts a server for health, metrics, and profiling on the provided listen port.
14107
// It will terminate the process if the server fails. Metrics and profiling are only exposed
15108
// if username and password are provided and the user's input matches.
16-
func Listen(listenAddr string, username, password string, checks ...healthz.HealthzChecker) {
109+
func (l Listener) Listen() {
110+
handler := l.handler()
111+
112+
tcpl, err := net.Listen("tcp", l.Addr)
113+
if err != nil {
114+
glog.Fatal(err)
115+
}
116+
117+
// if a TLS connection was requested, set up a connection mux that will send TLS requests to
118+
// the TLS server but send HTTP requests to the HTTP server. Preserves the ability for HTTP
119+
// health checks to call HTTP on the router while still allowing TLS certs and end to end
120+
// metrics protection.
121+
m := cmux.New(tcpl)
122+
123+
// match HTTP first
124+
httpl := m.Match(cmux.HTTP1Fast())
17125
go func() {
18-
mux := http.NewServeMux()
19-
healthz.InstallHandler(mux, checks...)
20-
21-
// TODO: exclude etcd and other unused metrics
22-
23-
// never enable profiling or metrics without protection
24-
if len(username) > 0 && len(password) > 0 {
25-
protected := http.NewServeMux()
26-
protected.HandleFunc("/debug/pprof/", pprof.Index)
27-
protected.HandleFunc("/debug/pprof/profile", pprof.Profile)
28-
protected.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
29-
protected.Handle("/metrics", prometheus.Handler())
30-
mux.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
31-
if u, p, ok := req.BasicAuth(); !ok || u != username || p != password {
32-
http.Error(w, "Unauthorized", http.StatusUnauthorized)
33-
return
34-
}
35-
protected.ServeHTTP(w, req)
36-
})
126+
s := &http.Server{
127+
Handler: handler,
37128
}
38-
39-
server := &http.Server{
40-
Addr: listenAddr,
41-
Handler: mux,
129+
if err := s.Serve(httpl); err != cmux.ErrListenerClosed {
130+
glog.Fatal(err)
42131
}
43-
glog.Infof("Router health and metrics port listening at %s", listenAddr)
44-
glog.Fatal(server.ListenAndServe())
45132
}()
46133

134+
// match TLS if configured
135+
if l.TLSConfig != nil {
136+
glog.Infof("Router health and metrics port listening at %s on HTTP and HTTPS", l.Addr)
137+
tlsl := m.Match(cmux.Any())
138+
tlsl = tls.NewListener(tlsl, l.TLSConfig)
139+
go func() {
140+
s := &http.Server{
141+
Handler: handler,
142+
}
143+
if err := s.Serve(tlsl); err != cmux.ErrListenerClosed {
144+
glog.Fatal(err)
145+
}
146+
}()
147+
} else {
148+
glog.Infof("Router health and metrics port listening at %s", l.Addr)
149+
}
150+
151+
go func() {
152+
if err := m.Serve(); !strings.Contains(err.Error(), "use of closed network connection") {
153+
glog.Fatal(err)
154+
}
155+
}()
47156
}

0 commit comments

Comments
 (0)