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.

Sign up for GitHub

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

Jump to bottom

API server crash on malformed PATCH request #40218

Closed
mikkeloscar opened this issue Jan 20, 2017 · 6 comments
Closed

API server crash on malformed PATCH request #40218

mikkeloscar opened this issue Jan 20, 2017 · 6 comments
Labels
area/reliability area/security kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Milestone
v1.6

Comments

@mikkeloscar
Copy link
Contributor

Is this a BUG REPORT

Kubernetes version (use kubectl version):

kubectl version
Client Version: version.Info{Major:"1", Minor:"5", GitVersion:"v1.5.1", GitCommit:"82450d03cb057bab0950214ef122b67c83fb11df", GitTreeState:"clean", BuildDate:"2016-12-14T00:57:05Z", GoVersion:"go1.7.4", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"", Minor:"", GitVersion:"v1.6.0-alpha.0", GitCommit:"42fe4ab0270e44c750d77c682e2fcab394aeb392", GitTreeState:"dirty", BuildDate:"1970-01-01T00:00:00Z", GoVersion:"go1.7", Compiler:"gc", Platform:"linux/amd64"}

Environment:

  • Cloud provider or hardware configuration: minikube (also happens in clusters on AWS running 1.5.2).
  • OS (e.g. from /etc/os-release):
  • Kernel (e.g. uname -a):
  • Install tools:
  • Others:

What happened:

API server panics on malformed PATCH request.

Observed a panic: "slice bounds out of range" (runtime error: slice bounds out of range)
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/util/runtime/runtime.go:70
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/util/runtime/runtime.go:63
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/util/runtime/runtime.go:49
/usr/local/go/src/runtime/asm_amd64.s:479
/usr/local/go/src/runtime/panic.go:458
/usr/local/go/src/runtime/panic.go:34
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/github.com/evanphx/json-patch/patch.go:265
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/github.com/evanphx/json-patch/patch.go:415
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/github.com/evanphx/json-patch/patch.go:556
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/github.com/evanphx/json-patch/patch.go:537
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/apiserver/resthandler.go:1089
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/apiserver/resthandler.go:566
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/api/rest/update.go:177
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/registry/generic/registry/store.go:365
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/storage/etcd/etcd_helper.go:465
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/storage/cacher.go:475
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/registry/generic/registry/store.go:441
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/apiserver/resthandler.go:645
/usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/apiserver/resthandler.go:960
/usr/local/go/src/runtime/asm_amd64.s:2086
E0120 14:35:19.924157    1882 apiserver.go:201] recover from panic situation: - runtime error: slice bounds out of range
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/github.com/emicklei/go-restful/container.go:206
    /usr/local/go/src/runtime/asm_amd64.s:479
    /usr/local/go/src/runtime/panic.go:458
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/apiserver/resthandler.go:976
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/apiserver/resthandler.go:651
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/apiserver/resthandler.go:503
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/apiserver/metrics/metrics.go:101
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/github.com/emicklei/go-restful/container.go:272
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/github.com/emicklei/go-restful/container.go:120
    /usr/local/go/src/net/http/server.go:1726
    /usr/local/go/src/net/http/server.go:2022
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/apiserver/filters/authorization.go:44
    /usr/local/go/src/net/http/server.go:1726
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/apiserver/filters/impersonation.go:44
    /usr/local/go/src/net/http/server.go:1726
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/auth/handlers/handlers.go:73
    /usr/local/go/src/net/http/server.go:1726
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/api/requestcontext.go:107
    /usr/local/go/src/net/http/server.go:1726
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/genericapiserver/filters/panics.go:75
    /usr/local/go/src/net/http/server.go:1726
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/apiserver/filters/requestinfo.go:45
    /usr/local/go/src/net/http/server.go:1726
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/api/requestcontext.go:107
    /usr/local/go/src/net/http/server.go:1726
    /usr/local/google/home/mrick/go/src/k8s.io/minikube/_gopath/src/k8s.io/minikube/vendor/k8s.io/kubernetes/pkg/genericapiserver/filters/timeout.go:78
    /usr/local/go/src/runtime/asm_amd64.s:2086

What you expected to happen:

It should respond with 400 instead of panicking.

How to reproduce it (as minimally and precisely as possible):

# create minikube cluster
$ minikube start --kubernetes-version v1.6.0-alpha.0
# create a pod
kubectl run hello-minikube --image=gcr.io/google_containers/echoserver:1.4 --port=8080
# get the pod name and make a malformed PATCH request.
$ curl -X PATCH -H "Content-Type: application/json-patch+json" \
  https://192.168.99.100:8443/api/v1/namespaces/default/pods/hello-minikube-3015430129-rexoi \
  -d '[{"op":"add","pointer":"/metadata/labels/foo","value":"hi"}]' -k

Anything else do we need to know:
mikkeloscar added a commit to mikkeloscar/json-patch that referenced this issue Jan 22, 2017
@mikkeloscar
Preven slice out of bound panic on invalid patch
7c9aadd 
This prevents a `slice bound out of range` panic when parsing an invalid
json patch where the path key is missing or the path value is empty
string or doesn't include `/`.

Fix kubernetes/kubernetes#40218
@mikkeloscar mikkeloscar mentioned this issue Jan 22, 2017
Merged
mikkeloscar added a commit to mikkeloscar/json-patch that referenced this issue Jan 22, 2017
@mikkeloscar
Prevent slice out of bound panic on invalid patch
e05b02c 
This prevents a `slice bound out of range` panic when parsing an invalid
json patch where the path key is missing or the path value is empty
string or doesn't include `/`.

Fix kubernetes/kubernetes#40218
mikkeloscar added a commit to mikkeloscar/kubernetes that referenced this issue Jan 24, 2017
@mikkeloscar
vendor: Update github.com/evanphx/json-patch
a408949 
Updates github.com/evanphx/json-patch dependency to a version that
doesn't crash when handling an invalid json patch.

Includes fix from evanphx/json-patch#35

Fix kubernetes#40218
@mikkeloscar mikkeloscar mentioned this issue Jan 24, 2017
Closed
@smarterclayton smarterclayton added this to the v1.6 milestone Feb 28, 2017
@smarterclayton smarterclayton added area/reliability area/security sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Feb 28, 2017
@ethernetdan ethernetdan added the kind/bug Categorizes issue or PR as related to a bug. label Mar 13, 2017
@ethernetdan
Copy link
Contributor

@mikkeloscar @smarterclayton is this blocking for v1.6?

@smarterclayton
Copy link
Contributor

Yes but I think we fixed it. @kubernetes/sig-api-machinery-misc

@liggitt
Copy link
Member

liggitt commented Mar 13, 2017

Looks like a PR went in to fix but no test was added.

@liggitt
Copy link
Member

liggitt commented Mar 14, 2017

actually, #40387 is still open

@mikkeloscar
Copy link
Contributor Author

@liggitt tests were added to the upstream package along with the fix.

#40387 is still open because godep-verify.sh doesn't pass, and I don't understand what I'm doing wrong. I'll have a crack at it later today and see if I can make it work.

@liggitt
Copy link
Member

liggitt commented Mar 14, 2017

I more meant tests to make sure the apiserver doesn't crash if a bad patch is submitted

deads2k pushed a commit to deads2k/kubernetes that referenced this issue Mar 14, 2017
@mikkeloscar @deads2k
vendor: Update github.com/evanphx/json-patch
c02484d 
Updates github.com/evanphx/json-patch dependency to a version that
doesn't crash when handling an invalid json patch.

Includes fix from evanphx/json-patch#35

Fix kubernetes#40218
@deads2k deads2k mentioned this issue Mar 14, 2017
Merged
liggitt pushed a commit to liggitt/kubernetes that referenced this issue Mar 15, 2017
Merge pull request kubernetes#43096 from deads2k/update-patch
a19210f 
Automatic merge from submit-queue

vendor: Update github.com/evanphx/json-patch

Regenerated files for kubernetes#40387


Fixes kubernetes#40218
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/reliability area/security kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Milestone
v1.6
Development

Successfully merging a pull request may close this issue.

vendor: Update github.com/evanphx/json-patch mikkeloscar/kubernetes
4 participants
@mikkeloscar @liggitt @smarterclayton @ethernetdan