Querying several resources via API with invalid resourceVersion query parameter causes internal server error

[matusf]

What happened:

Hello @viralpoetry and I were fuzzing the k8s (via openapi-fuzzer) and found out following bug. Querying several resources via API with invalid resourceVersion query parameter causes internal server error. Affected resources:

• api-v1-namespaces-{namespace}-configmaps/
• api-v1-namespaces-{namespace}-endpoints/
• api-v1-namespaces-{namespace}-events/
• api-v1-namespaces-{namespace}-limitranges/
• api-v1-namespaces-{namespace}-persistentvolumeclaims/
• api-v1-namespaces-{namespace}-pods/
• api-v1-namespaces-{namespace}-podtemplates/
• api-v1-namespaces-{namespace}-replicationcontrollers/
• api-v1-namespaces-{namespace}-resourcequotas/
• api-v1-namespaces-{namespace}-secrets/
• api-v1-namespaces-{namespace}-serviceaccounts/
• api-v1-namespaces-{namespace}-services/

What you expected to happen:

Response with non-500 HTTP status code

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

We were fuzzing k8s locally via minikube.

1. start minukube

minikube start --driver virtualbox

2. install ca certificates

sudo cp ~/.minikube/ca.pem /usr/local/share/ca-certificates/
sudo update-ca-certificates

3. get k8s IP address

kubectl config view --minify -o jsonpath='{.clusters[0].cluster.server}'

4. add the IP to /etc/hosts as minikubeca
5. get access token and export it as token

kubectl create clusterrolebinding permissive-binding \
  --clusterrole=cluster-admin \
  --user=admin \
  --user=kubelet \
  --group=system:serviceaccounts

kubectl create serviceaccount openapifuzz
kubectl get serviceaccounts openapifuzz -o json | jq -r .secrets[].name
kubectl get secret openapifuzz-token-sn9qm -o json | jq -r .data.token
 export token=<retrieved-token>

6. send requests

# configmaps
curl -X GET 'https://minikubeca:8443/api/v1/namespaces/x/configmaps?resourceVersion=x' -H "Authorization: Bearer $token"

{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

# endpoints
curl -X GET -H "authorization: bearer $token" 'https://minikubeca:8443/api/v1/namespaces/x/endpoints?resourceVersion=x'
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

# events
curl -X GET -H "authorization: bearer $token" 'https://minikubeca:8443/api/v1/namespaces/x?events&resourceVersion=x'
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

# limit ranges
curl -X GET -H "authorization: bearer $token" 'https://minikubeca:8443/api/v1/namespaces/x/limitranges?resourceVersion=x'
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

# persistent volume claims
curl -X GET -H "authorization: bearer $token" 'https://minikubeca:8443/api/v1/namespaces/x/persistentvolumeclaims?resourceVersion=x'
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

# pods
curl -X GET -H "authorization: bearer $token" 'https://minikubeca:8443/api/v1/namespaces/x/pods?resourceVersion=x'
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

# pod templates
curl -X GET -H "authorization: bearer $token" 'https://minikubeca:8443/api/v1/namespaces/x/podtemplates?resourceVersion=x'
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

# replication controllers
curl -X GET -H "authorization: bearer $token" 'https://minikubeca:8443/api/v1/namespaces/x/replicationcontrollers?resourceVersion=x'
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

# resource quotas
curl -X GET -H "authorization: bearer $token" 'https://minikubeca:8443/api/v1/namespaces/x/resourcequotas?resourceVersion=x'
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

# secrets
curl -X GET -H "authorization: bearer $token" 'https://minikubeca:8443/api/v1/namespaces/x/secrets?resourceVersion=x'
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

# service accounts
curl -X GET -H "authorization: bearer $token" 'https://minikubeca:8443/api/v1/namespaces/x/serviceaccounts?resourceVersion=x'
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

# services
curl -X GET -H "authorization: bearer $token" 'https://minikubeca:8443/api/v1/namespaces/x/services?resourceVersion=x'
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {
    
  },
  "status": "Failure",
  "message": "resourceVersion: Invalid value: \"x\": strconv.ParseUint: parsing \"x\": invalid syntax",
  "code": 500
}

Environment:

• Kubernetes version (use kubectl version):

clientVersion:
  buildDate: "2021-04-08T16:31:21Z"
  compiler: gc
  gitCommit: cb303e613a121a29364f75cc67d3d580833a7479
  gitTreeState: clean
  gitVersion: v1.21.0
  goVersion: go1.16.1
  major: "1"
  minor: "21"
  platform: linux/amd64
serverVersion:
  buildDate: "2021-01-13T13:20:00Z"
  compiler: gc
  gitCommit: faecb196815e248d3ecfb03c680a4507229c2a56
  gitTreeState: clean
  gitVersion: v1.20.2
  goVersion: go1.15.5
  major: "1"
  minor: "20"
  platform: linux/amd64

• Cloud provider or hardware configuration: minikube with virtual-box driver
• OS (e.g: cat /etc/os-release): Ubuntu 18.04.5 LTS (bionic)
• Kernel (e.g. uname -a): Linux pine 4.15.0-140-generic Refactor controller manager. #144-Ubuntu SMP Fri Mar 19 14:12:35 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
• Install tools: downloaded precompiled binary
• minikube version: v1.19.0 (commit: 15cede53bdc5fe242228853e737333b09d4336b5)

[matusf]

/sig api-machinery

[pacoxu]

IMO, it should return something like

https://127.0.0.1:6443/api/v1/pods?timeoutSeconds=0.1s
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {

  },
  "status": "Failure",
  "message": "strconv.ParseInt: parsing \"0.1s\": invalid syntax",
  "reason": "BadRequest",
  "code": 400
}

/assign

kubernetes/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go

Lines 1165 to 1168 in ea07644

	// {"/simple/foo?export=<invalid>", http.MethodGet}, TODO: there is no invalid bool in conversion. Should we be more strict?
	// {"/simple/foo?resourceVersion=<invalid>", http.MethodGet}, TODO: there is no invalid resourceVersion. Should we be more strict?
	// {"/withoptions?labelSelector=<invalid>", http.MethodGet}, TODO: SimpleGetOptions is always valid. Add more validation that can fail.
	} {

There is no invalid resourceVersion. Should we be more strict?

[pacoxu]

per #46844 which fixed #39730, cc @sttts

[fedebongio]

/triage accepted

[pacoxu]

Some comments from the PR: #101368 (comment).

kubernetes/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/types.go

Lines 47 to 50 in ea07644

	// resourceVersion sets a constraint on what resource versions a request may be served from.
	// See https://kubernetes.io/docs/reference/using-api/api-concepts/#resource-versions for
	// details.
	ResourceVersion string

As the code comment shows, https://kubernetes.io/docs/reference/using-api/api-concepts/#resource-versions

Resource versions must be treated as opaque by clients and passed unmodified back to the server.

[yliaog]

the error should come from here:

kubernetes/staging/src/k8s.io/apiserver/pkg/storage/etcd3/api_object_versioner.go

Line 91 in 2115852

func (a APIObjectVersioner) ParseResourceVersion(resourceVersion string) (uint64, error) {

which is correct, as it is converting/validating the resourceVersion for etcd.

I think the question is whether it should return 500 or 400.

[pacoxu]

I updated #101368.

[k8s-triage-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.