Expose DisableHTTP2 flag in SecureServingOptions #122176

slashpai · 2023-12-05T06:19:31Z

This is to mitigate CVE-2023-44487
until the Go standard library and golang.org/x/net are fully fixed.

What type of PR is this?

What this PR does / why we need it:

Expose DisableHTTP2 flag in SecureServingOptions

This is to mitigate GHSA-qppj-fm5r-hxr3
until the Go standard library and golang.org/x/net
are fully fixed.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

This PR is created based on discussion in kubernetes-sigs/metrics-server#1364 (comment)

Does this PR introduce a user-facing change?

kube-apiserver: http/2 serving can be disabled with a `--disable-http2-serving` flag

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

cc: @dgrisonnet

k8s-ci-robot · 2023-12-05T06:19:39Z

Please note that we're already in Test Freeze for the release-1.29 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.29.0 release.

Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Tue Dec 5 04:13:37 UTC 2023.

k8s-ci-robot · 2023-12-05T06:19:41Z

Welcome @slashpai!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2023-12-05T06:19:42Z

Hi @slashpai. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

rexagod · 2023-12-05T10:56:21Z

/ok-to-test

aojea · 2023-12-05T11:29:09Z

dup of https://github.com/kubernetes/kubernetes/pull/116762/files

You and @enj should coordinate to only work in one PR

slashpai · 2023-12-05T11:44:18Z

dup of https://github.com/kubernetes/kubernetes/pull/116762/files

You and @enj should coordinate to only work in one PR

Thank you. Didn't know another PR existed for same.
This was created as a followup from metrics-server PR discussion to do same.

cici37 · 2023-12-05T21:13:01Z

/triage accepted
Please feel free to close one of them if it has duplication. Thanks!

slashpai · 2023-12-06T04:46:22Z

@enj Would you be continuing with PR you already created for this? If not I can help 🙂

rexagod · 2024-02-16T07:30:49Z

Since #116762 is closed now, I believe this PR can solely drive the effort forward.

ChristianZaccaria · 2024-04-26T09:56:14Z

Hi @slashpai, I was wondering what is the status of this PR? Is there more to be done on it? Very interested as this could potentially solve this 'High' CVE. Thank you!

slashpai · 2024-04-26T13:05:18Z

I am looking for code review

@dgrisonnet please take a look if you get a chance

dgrisonnet · 2024-04-26T13:34:41Z

staging/src/k8s.io/apiserver/pkg/server/options/serving.go

+	fs.BoolVar(&s.DisableHTTP2, "disable-http2", s.DisableHTTP2,
+		"If true, HTTP2 will be disabled [default=false]")


I think that Jordan's comment still applies here: https://github.com/kubernetes/kubernetes/pull/116762/files#r1261602446

Maybe you could go with something like disable-serving-http2 and try to improve the help message to make it clearer to the users?

Updated please take a look

Looks good to me now

This is to mitigate CVE-2023-44487 until the Go standard library and golang.org/x/net are fully fixed. Signed-off-by: Jayapriya Pai <[email protected]>

dgrisonnet · 2024-04-30T17:30:29Z

@liggitt could you please take a look at this PR that supersede the effort started in #116762?

liggitt · 2024-05-09T14:48:59Z

/lgtm
/approve

k8s-ci-robot · 2024-05-09T14:49:06Z

LGTM label has been added.

Details

Git tree hash: 44730b32d902c1cc3d3cd9ca76452daaf03618a5

k8s-ci-robot · 2024-05-09T14:49:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, slashpai

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~staging/src/k8s.io/apiserver/pkg/server/options/OWNERS~~ [liggitt]

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

liggitt · 2024-05-09T14:49:11Z

/kind feature

disable-http2-serving was added in kubernetes/kubernetes/pull/122176 Signed-off-by: Jayapriya Pai <[email protected]>

@benluddy

Installation kubectl apply -f https://github.com/kubernetes-sigs/metrics-server/releases/download/v0.8.0/components.yaml Changes since v0.7.2 Improvements * Wire server run options to flags kubernetes-sigs#1560 kubernetes-sigs#1656 (@benluddy, @slashpai) * Bump Golang to v1.24.4 and to Kubernetes clients to v0.33.2 kubernetes-sigs#1662, kubernetes-sigs#1665 (@dgrisonnet,@yangjunmyfm192085,@slashpai) * This will help to include the `disable-http2-serving` flag added in SecureServingOptions (kubernetes/kubernetes#122176) for metrics-server * Bump Prometheus module to v0.304.2 kubernetes-sigs#1652, kubernetes-sigs#1675 (@slashpai , @dependabot) Tooling * Add support for specifying other container engines kubernetes-sigs#1649 (@slashpai)

k8s-ci-robot requested review from smarterclayton and wojtek-t December 5, 2023 06:19

k8s-ci-robot added the area/apiserver label Dec 5, 2023

k8s-ci-robot added do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 5, 2023

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 5, 2023

k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Dec 5, 2023

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 5, 2023

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 5, 2023

dgrisonnet reviewed Apr 26, 2024

View reviewed changes

Expose DisableHTTP2 flag in SecureServingOptions

e2503e5

This is to mitigate CVE-2023-44487 until the Go standard library and golang.org/x/net are fully fixed. Signed-off-by: Jayapriya Pai <[email protected]>

slashpai force-pushed the disable_http2_flag branch from fcdafd5 to e2503e5 Compare April 27, 2024 13:08

k8s-ci-robot assigned liggitt May 9, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 9, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 9, 2024

k8s-ci-robot merged commit dfcb447 into kubernetes:master May 9, 2024

k8s-ci-robot added this to the v1.31 milestone May 9, 2024

slashpai added a commit to slashpai/metrics-server that referenced this pull request Aug 20, 2024

docs/command-line-flags.txt: add http2 disable flag

6c39aeb

disable-http2-serving was added in kubernetes/kubernetes/pull/122176 Signed-off-by: Jayapriya Pai <[email protected]>

slashpai mentioned this pull request Aug 20, 2024

docs/command-line-flags.txt: add http2 disable flag kubernetes-sigs/metrics-server#1544

Closed

slashpai mentioned this pull request Sep 10, 2024

Release v0.8.0 kubernetes-sigs/metrics-server#1565

Closed

slashpai mentioned this pull request Apr 25, 2025

REQUEST: New membership for slashpai kubernetes/org#5562

Closed

11 tasks

		fs.BoolVar(&s.DisableHTTP2, "disable-http2", s.DisableHTTP2,
		"If true, HTTP2 will be disabled [default=false]")

Expose DisableHTTP2 flag in SecureServingOptions #122176

Expose DisableHTTP2 flag in SecureServingOptions #122176

Uh oh!

Conversation

slashpai commented Dec 5, 2023 • edited by liggitt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

k8s-ci-robot commented Dec 5, 2023

Uh oh!

k8s-ci-robot commented Dec 5, 2023

Uh oh!

k8s-ci-robot commented Dec 5, 2023

Uh oh!

rexagod commented Dec 5, 2023

Uh oh!

aojea commented Dec 5, 2023

Uh oh!

slashpai commented Dec 5, 2023

Uh oh!

cici37 commented Dec 5, 2023

Uh oh!

slashpai commented Dec 6, 2023

Uh oh!

rexagod commented Feb 16, 2024

Uh oh!

ChristianZaccaria commented Apr 26, 2024

Uh oh!

slashpai commented Apr 26, 2024

Uh oh!

dgrisonnet Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

slashpai Apr 27, 2024

Choose a reason for hiding this comment

Uh oh!

dgrisonnet Apr 30, 2024

Choose a reason for hiding this comment

Uh oh!

dgrisonnet commented Apr 30, 2024

Uh oh!

liggitt commented May 9, 2024

Uh oh!

k8s-ci-robot commented May 9, 2024

Uh oh!

k8s-ci-robot commented May 9, 2024

Uh oh!

liggitt commented May 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

slashpai commented Dec 5, 2023 •

edited by liggitt

Loading