Skip to content

Update NodeRestriction to prevent nodes from updating their OwnerReferences #133467

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 1 commit into from
Aug 11, 2025
Merged

Update NodeRestriction to prevent nodes from updating their OwnerReferences #133467

merged 1 commit into from
Aug 11, 2025
+35 −6

Conversation

natherz97
Copy link

@natherz97 natherz97 commented Aug 11, 2025 •
edited
Loading

What type of PR is this?

/kind bug

What this PR does / why we need it:

A vulnerability exists in the NodeRestriction admission controller where node users can delete their corresponding node object by patching themselves with an OwnerReference to a cluster-scoped resource. If the OwnerReference resource does not exist or is subsequently deleted, the given node object will be deleted via garbage collection. This PR adds functionality to the NodeRestriction admission controller to prevent nodes from updating their OwnerReferences.

Which issue(s) this PR is related to:

N/A

Special notes for your reviewer:

This PR was authored by @SergeyKanzhelev

Changed the node restrictions to disallow the node to change it's ownerReferences.

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. 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 Aug 11, 2025
@k8s-ci-robot
Copy link
Contributor

Please note that we're already in Test Freeze for the release-1.34 branch. This means every merged PR will be automatically fast-forwarded via the periodic ci-fast-forward job to the release branch of the upcoming v1.34.0 release.

Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Mon Aug 11 10:25:10 UTC 2025.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 11, 2025
@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 11, 2025
@k8s-ci-robot k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Aug 11, 2025
@SergeyKanzhelev
Copy link
Member

please do a PR for release-1.34 as well

@xmudrii
Copy link
Member

xmudrii commented Aug 11, 2025

@SergeyKanzhelev Not needed, the release-1.34 branch is fast-forwarded automatically every day

@SergeyKanzhelev
Copy link
Member

/priority important-soon
/triage accepted
/release-note-edit

Changed the node restrictions to disallow the node to change it's ownerReferences.

@k8s-ci-robot
Copy link
Contributor

@SergeyKanzhelev: /release-note-edit must be used with a single release note block.

In response to this:

/priority important-soon
/triage accepted
/release-note-edit

Changed the node restrictions to disallow the node to change it's ownerReferences.

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 11, 2025
SergeyKanzhelev
SergeyKanzhelev approved these changes Aug 11, 2025
View reviewed changes
Copy link
Member

@SergeyKanzhelev SergeyKanzhelev left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 11, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 2626337db60a97fbe958c801203db54015a55d51

@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 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 Aug 11, 2025
@dims
Copy link
Member

dims commented Aug 11, 2025

/assign @tallclair @liggitt @deads2k

cc @Vyom-Yadav i think we will need to mark this with 1.34 milestone

@tallclair
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: natherz97, SergeyKanzhelev, tallclair

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 Aug 11, 2025
@liggitt
Copy link
Member

liggitt commented Aug 11, 2025

cc @Vyom-Yadav i think we will need to mark this with 1.34 milestone

yes, definitely. It's a security fix we're backporting to older minors, we don't want a gap in the fixed versions in 1.34.0

@SergeyKanzhelev
Copy link
Member

/milestone v1.34

@k8s-ci-robot k8s-ci-robot added this to the v1.34 milestone Aug 11, 2025
@k8s-ci-robot k8s-ci-robot merged commit 790393a into kubernetes:master Aug 11, 2025
13 checks passed
@github-project-automation github-project-automation bot moved this to Closed / Done in SIG Auth Aug 11, 2025
@BenTheElder
Copy link
Member

Ref backports:
#133470
#133469
#133468

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SergeyKanzhelev SergeyKanzhelev SergeyKanzhelev approved these changes

@liggitt liggitt Awaiting requested review from liggitt

@tallclair tallclair Awaiting requested review from tallclair

Assignees

@liggitt liggitt

@deads2k deads2k

@SergeyKanzhelev SergeyKanzhelev

@tallclair tallclair

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/auth Categorizes an issue or PR as relevant to SIG Auth. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG Auth
Status: Closed / Done
Milestone
v1.34
Development

Successfully merging this pull request may close these issues.
9 participants
@natherz97 @k8s-ci-robot @SergeyKanzhelev @xmudrii @dims @tallclair @liggitt @BenTheElder @deads2k