Commit dd4bb121 authored by Ryan Hitchman's avatar Ryan Hitchman

Escape "<>&" in apiserver errors to avoid triggering vulnerability scanners.

Simple XSS scans might fetch /<script>alert('vulnerable')</script>, and fail when the response body includes the script tag verbatim, despite the headers directing the browser to interpret the response as text. This isn't a real vulnerability, but it's easier to fix this here than it is to fix the scanners.
parent 202a9f84
...@@ -10,13 +10,18 @@ load( ...@@ -10,13 +10,18 @@ load(
go_test( go_test(
name = "go_default_test", name = "go_default_test",
srcs = ["status_test.go"], srcs = [
"errors_test.go",
"status_test.go",
],
library = ":go_default_library", library = ":go_default_library",
tags = ["automanaged"], tags = ["automanaged"],
deps = [ deps = [
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",
], ],
) )
......
...@@ -19,22 +19,26 @@ package responsewriters ...@@ -19,22 +19,26 @@ package responsewriters
import ( import (
"fmt" "fmt"
"net/http" "net/http"
"strings"
"k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/authorization/authorizer"
) )
// Avoid emitting errors that look like valid HTML. Quotes are okay.
var sanitizer = strings.NewReplacer(`&`, "&amp;", `<`, "&lt;", `>`, "&gt;")
// BadGatewayError renders a simple bad gateway error. // BadGatewayError renders a simple bad gateway error.
func BadGatewayError(w http.ResponseWriter, req *http.Request) { func BadGatewayError(w http.ResponseWriter, req *http.Request) {
w.Header().Set("Content-Type", "text/plain") w.Header().Set("Content-Type", "text/plain")
w.Header().Set("X-Content-Type-Options", "nosniff") w.Header().Set("X-Content-Type-Options", "nosniff")
w.WriteHeader(http.StatusBadGateway) w.WriteHeader(http.StatusBadGateway)
fmt.Fprintf(w, "Bad Gateway: %#v", req.RequestURI) fmt.Fprintf(w, "Bad Gateway: %q", sanitizer.Replace(req.RequestURI))
} }
// Forbidden renders a simple forbidden error // Forbidden renders a simple forbidden error
func Forbidden(attributes authorizer.Attributes, w http.ResponseWriter, req *http.Request, reason string) { func Forbidden(attributes authorizer.Attributes, w http.ResponseWriter, req *http.Request, reason string) {
msg := forbiddenMessage(attributes) msg := sanitizer.Replace(forbiddenMessage(attributes))
w.Header().Set("Content-Type", "text/plain") w.Header().Set("Content-Type", "text/plain")
w.Header().Set("X-Content-Type-Options", "nosniff") w.Header().Set("X-Content-Type-Options", "nosniff")
w.WriteHeader(http.StatusForbidden) w.WriteHeader(http.StatusForbidden)
...@@ -76,12 +80,12 @@ func InternalError(w http.ResponseWriter, req *http.Request, err error) { ...@@ -76,12 +80,12 @@ func InternalError(w http.ResponseWriter, req *http.Request, err error) {
w.Header().Set("Content-Type", "text/plain") w.Header().Set("Content-Type", "text/plain")
w.Header().Set("X-Content-Type-Options", "nosniff") w.Header().Set("X-Content-Type-Options", "nosniff")
w.WriteHeader(http.StatusInternalServerError) w.WriteHeader(http.StatusInternalServerError)
fmt.Fprintf(w, "Internal Server Error: %#v: %v", req.RequestURI, err) fmt.Fprintf(w, "Internal Server Error: %q: %v", sanitizer.Replace(req.RequestURI), err)
runtime.HandleError(err) runtime.HandleError(err)
} }
// NotFound renders a simple not found error. // NotFound renders a simple not found error.
func NotFound(w http.ResponseWriter, req *http.Request) { func NotFound(w http.ResponseWriter, req *http.Request) {
w.WriteHeader(http.StatusNotFound) w.WriteHeader(http.StatusNotFound)
fmt.Fprintf(w, "Not Found: %#v", req.RequestURI) fmt.Fprintf(w, "Not Found: %q", sanitizer.Replace(req.RequestURI))
} }
/*
Copyright 2017 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package responsewriters
import (
"errors"
"net/http"
"net/http/httptest"
"testing"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
)
func TestErrors(t *testing.T) {
internalError := errors.New("ARGH")
fns := map[string]func(http.ResponseWriter, *http.Request){
"BadGatewayError": BadGatewayError,
"NotFound": NotFound,
"InternalError": func(w http.ResponseWriter, req *http.Request) {
InternalError(w, req, internalError)
},
}
cases := []struct {
fn string
uri string
expected string
}{
{"BadGatewayError", "/get", `Bad Gateway: "/get"`},
{"BadGatewayError", "/<script>", `Bad Gateway: "/&lt;script&gt;"`},
{"NotFound", "/get", `Not Found: "/get"`},
{"NotFound", "/<script&>", `Not Found: "/&lt;script&amp;&gt;"`},
{"InternalError", "/get", `Internal Server Error: "/get": ARGH`},
{"InternalError", "/<script>", `Internal Server Error: "/&lt;script&gt;": ARGH`},
}
for _, test := range cases {
observer := httptest.NewRecorder()
fns[test.fn](observer, &http.Request{RequestURI: test.uri})
result := string(observer.Body.Bytes())
if result != test.expected {
t.Errorf("%s(..., %q) != %q, got %q", test.fn, test.uri, test.expected, result)
}
}
}
func TestForbidden(t *testing.T) {
u := &user.DefaultInfo{Name: "NAME"}
cases := []struct {
expected string
attributes authorizer.Attributes
reason string
}{
{`User "NAME" cannot GET path "/whatever".`,
authorizer.AttributesRecord{User: u, Verb: "GET", Path: "/whatever"}, ""},
{`User "NAME" cannot GET path "/&lt;script&gt;".`,
authorizer.AttributesRecord{User: u, Verb: "GET", Path: "/<script>"}, ""},
{`User "NAME" cannot GET pod at the cluster scope.`,
authorizer.AttributesRecord{User: u, Verb: "GET", Resource: "pod", ResourceRequest: true}, ""},
{`User "NAME" cannot GET pod.v2/quota in the namespace "test".`,
authorizer.AttributesRecord{User: u, Verb: "GET", Namespace: "test", APIGroup: "v2", Resource: "pod", Subresource: "quota", ResourceRequest: true}, ""},
}
for _, test := range cases {
observer := httptest.NewRecorder()
Forbidden(test.attributes, observer, &http.Request{}, test.reason)
result := string(observer.Body.Bytes())
if result != test.expected {
t.Errorf("Forbidden(%#v...) != %#v, got %#v", test.attributes, test.expected, result)
}
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment