Skip to content

apiextensions: check request scope against CRD scope correctly #80750

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
Merged
merged 2 commits into from
Jul 31, 2019
Merged

apiextensions: check request scope against CRD scope correctly #80750

merged 2 commits into from
Jul 31, 2019

Conversation

sttts
Copy link
Contributor

@sttts sttts commented Jul 30, 2019 •
edited by tpepper
Loading

Before this PR we did not always check that the request scope was matching the CRD. This adds consequent checks, and exhaustive tests.

Fix CVE-2019-11247: API server allows access to custom resources via wrong scope

@k8s-ci-robot k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. 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. labels Jul 30, 2019
@sttts sttts added release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 30, 2019
@k8s-ci-robot k8s-ci-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jul 30, 2019
@sttts sttts added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jul 30, 2019
@k8s-ci-robot k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jul 30, 2019
@sttts sttts added the kind/bug Categorizes issue or PR as related to a bug. label Jul 30, 2019
@k8s-ci-robot k8s-ci-robot removed the needs-kind Indicates a PR lacks a `kind/foo` label and requires one. label Jul 30, 2019
@sttts sttts removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jul 30, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 30, 2019
@k8s-ci-robot k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jul 30, 2019
@sttts sttts added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 30, 2019
@deads2k
Copy link
Contributor

deads2k commented Jul 30, 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 Jul 30, 2019
@yue9944882
Copy link
Member

i tried POST a pod against a non-namespace path /api/v1/pods locally, and it's returning 405 instead of 404. i presume we should keep that behavior consistent between built-ins and CRDs

$ curl --cert /tmp/crt --key /tmp/key -XPOST -d@/tmp/pod -H 'Content-Type: application/json' -k https://127.0.0.1:6443/api/v1/pods
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {

  },
  "status": "Failure",
  "message": "the server does not allow this method on the requested resource",
  "reason": "MethodNotAllowed",
  "details": {

  },
  "code": 405
}

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

2 similar comments
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@sttts sttts force-pushed the sttts-crd-scoping branch from a5fc861 to caa7a76 Compare July 31, 2019 10:34
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 31, 2019
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@sttts
Copy link
Contributor Author

sttts commented Jul 31, 2019

fixed typo

@sttts sttts added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 31, 2019
@sttts sttts force-pushed the sttts-crd-scoping branch from caa7a76 to df75700 Compare July 31, 2019 11:30
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 31, 2019
@sttts sttts added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 31, 2019
@k8s-ci-robot k8s-ci-robot merged commit 2abc859 into kubernetes:master Jul 31, 2019
@k8s-ci-robot k8s-ci-robot added this to the v1.16 milestone Jul 31, 2019
This was referenced Aug 1, 2019
Closed
Merged
Merged
Merged
@jennybuckley
Copy link

/cc @roycaihw

@k8s-ci-robot k8s-ci-robot requested a review from roycaihw August 1, 2019 20:07
@roycaihw
Copy link
Member

roycaihw commented Aug 1, 2019

LGTM

@joelsmith joelsmith mentioned this pull request Aug 5, 2019
Closed
@joelsmith
Copy link
Contributor

We need to add the following release note:

Fix CVE-2019-11247: API server allows access to custom resources via wrong scope

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Aug 5, 2019
@israel-hdez israel-hdez mentioned this pull request Aug 15, 2019
Closed
@mcieplak mcieplak mentioned this pull request Aug 27, 2019
Merged
@yqwang-ms yqwang-ms mentioned this pull request Jan 8, 2020
Closed
6 tasks
@sonyafenge sonyafenge mentioned this pull request Feb 22, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deads2k deads2k Awaiting requested review from deads2k

@yue9944882 yue9944882 Awaiting requested review from yue9944882

@roycaihw roycaihw Awaiting requested review from roycaihw

Assignees

@deads2k deads2k

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. 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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.16
Development

Successfully merging this pull request may close these issues.
9 participants
@sttts @k8s-ci-robot @deads2k @yue9944882 @fejta-bot @liggitt @jennybuckley @roycaihw @joelsmith