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

limit yaml/json decode size #83261

Merged
merged 2 commits into from Oct 3, 2019
Merged

limit yaml/json decode size #83261

merged 2 commits into from Oct 3, 2019

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Sep 27, 2019 •
edited

What type of PR is this?
/kind bug

What this PR does / why we need it:
Fixes resource exhaustion issues in json and yaml parsers

Which issue(s) this PR fixes:
Fixes #83253

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Fixes a flaw (CVE-2019-11253) in json/yaml decoding where large or malformed documents could consume excessive server resources. Request bodies for normal API requests (create/delete/update/patch operations of regular resources) are now limited to 3MB.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/bug Categorizes issue or PR as related to a bug. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 27, 2019
@k8s-ci-robot k8s-ci-robot added area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 27, 2019
@liggitt
Copy link
Member Author

liggitt commented Sep 27, 2019

/cc @cjcullen

@k8s-ci-robot k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Sep 27, 2019
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 28, 2019
@liggitt liggitt force-pushed the yaml-limits branch 2 times, most recently from b1a0410 to 06e0e8b Compare September 28, 2019 02:42
caesarxuchao
caesarxuchao reviewed Sep 28, 2019
View reviewed changes
Copy link
Member

@caesarxuchao caesarxuchao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stopped when I got confused with how the overhead consts are determined. I'll continue on Monday.

staging/src/k8s.io/apiserver/pkg/server/config.go Outdated Show resolved Hide resolved
staging/src/k8s.io/apiserver/pkg/server/config.go Outdated Show resolved Hide resolved
vendor/gopkg.in/yaml.v2/yaml.go Outdated Show resolved Hide resolved
vendor/gopkg.in/yaml.v2/yaml.go Outdated Show resolved Hide resolved
vendor/gopkg.in/yaml.v2/apic.go Outdated Show resolved Hide resolved
vendor/gopkg.in/yaml.v2/decode.go Outdated Show resolved Hide resolved
@liggitt liggitt force-pushed the yaml-limits branch 4 times, most recently from 0dbd6c2 to 1f5b26e Compare September 30, 2019 14:22
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 30, 2019
@k8s-ci-robot k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Oct 2, 2019
@liggitt
Copy link
Member Author

liggitt commented Oct 2, 2019

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 2, 2019
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 3, 2019
@liggitt
Copy link
Member Author

liggitt commented Oct 3, 2019

/hold cancel

added integration tests against normal and custom resources exercising create and all the patch flows, just above and below the accepted size limit

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 3, 2019
@cjcullen
Copy link
Member

cjcullen commented Oct 3, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 3, 2019
@liggitt
Copy link
Member Author

liggitt commented Oct 3, 2019

/retest
flake #83321

@liggitt liggitt mentioned this pull request Oct 3, 2019
Merged
@k8s-ci-robot k8s-ci-robot merged commit 4afcba4 into kubernetes:master Oct 3, 2019
@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Oct 3, 2019
This was referenced Oct 3, 2019
Merged
Merged
Merged
@liggitt liggitt deleted the yaml-limits branch October 3, 2019 04:34
k8s-ci-robot added a commit that referenced this pull request Oct 4, 2019
@k8s-ci-robot
Merge pull request #83435 from liggitt/automated-cherry-pick-of-#8326…
bdc7ad2 
…1-upstream-release-1.14

[1.14] Automated cherry pick of #83261: bump gopkg.in/yaml.v2 v2.2.4
k8s-ci-robot added a commit that referenced this pull request Oct 4, 2019
@k8s-ci-robot
Merge pull request #83433 from liggitt/automated-cherry-pick-of-#8326…
a2ff5d7 
…1-upstream-release-1.16

[1.16] Automated cherry pick of #83261: bump gopkg.in/yaml.v2 v2.2.4
k8s-ci-robot added a commit that referenced this pull request Oct 4, 2019
@k8s-ci-robot
Merge pull request #83436 from liggitt/automated-cherry-pick-of-#8326…
17c28f0 
…1-upstream-release-1.13-1570075716

[1.13] Automated cherry pick of #83261: bump gopkg.in/yaml.v2 v2.2.4
k8s-ci-robot added a commit that referenced this pull request Oct 4, 2019
@k8s-ci-robot
Merge pull request #83434 from liggitt/automated-cherry-pick-of-#8326…
9a83f46 
…1-upstream-release-1.15

[1.15] Automated cherry pick of #83261: bump gopkg.in/yaml.v2 v2.2.4
@raesene raesene mentioned this pull request Oct 4, 2019
Closed
@rfranzke rfranzke mentioned this pull request Oct 15, 2019
Merged
@rfranzke rfranzke mentioned this pull request Feb 17, 2020
Closed
wking pushed a commit to wking/kubernetes that referenced this pull request Jul 21, 2020
@k8s-publishing-bot
Merge pull request kubernetes#83261 from liggitt/yaml-limits
cc39a16 
limit yaml/json decode size

Kubernetes-commit: 4afcba4
@sharonbz sharonbz mentioned this pull request Jul 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caesarxuchao caesarxuchao caesarxuchao left review comments

@cjcullen cjcullen cjcullen left review comments

@apelisse apelisse Awaiting requested review from apelisse

Assignees

@cjcullen cjcullen

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver area/cloudprovider area/code-generation area/dependency Issues or PRs related to dependency changes area/kubectl area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.17
Development

Successfully merging this pull request may close these issues.

CVE-2019-11253: Kubernetes API Server JSON/YAML parsing vulnerable to resource exhaustion attack
5 participants
@liggitt @k8s-ci-robot @BenTheElder @cjcullen @caesarxuchao