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.

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

Automated cherry pick of #121201 #121228 upstream release 1.24 #4

Conversation

m-chau
Copy link

@m-chau m-chau commented Oct 24, 2023

What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Cherry pick of #121201 and #121228 on release-1.24.

Fixes #

Special notes for your reviewer:

This change fully addresses CVE-2023-44487 and CVE-2023-39325 for
the API server when the client is unauthenticated.

Does this PR introduce a user-facing change?

NONE

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

@aks-lts-prow
Copy link

aks-lts-prow bot commented Oct 24, 2023

@m-chau: The label(s) kind/bug cannot be applied, because the repository doesn't have them.

In response to this:

What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Cherry pick of #121201 and #121228 on release-1.24.

Fixes #

Special notes for your reviewer:

This PR addresses This change fully addresses CVE-2023-44487 and CVE-2023-39325 for
the API server when the client is unauthenticated.

Does this PR introduce a user-facing change?

NONE

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

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.

@aks-lts-prow
Copy link

aks-lts-prow bot commented Oct 24, 2023

Hi @m-chau. Thanks for your PR.

I'm waiting for a aks-lts 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.

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.

@m-chau
Copy link
Author

m-chau commented Oct 24, 2023

/ok-to-test

@aks-lts-prow
Copy link

aks-lts-prow bot commented Oct 24, 2023

@m-chau: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/ok-to-test

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.

@m-chau
Copy link
Author

m-chau commented Oct 24, 2023

/meow caturday

@aks-lts-prow
Copy link

aks-lts-prow bot commented Oct 24, 2023

@m-chau: cat image

In response to this:

/meow caturday

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.

@m-chau
Copy link
Author

m-chau commented Oct 24, 2023

/ok-to-test

@haitch
Copy link

haitch commented Oct 27, 2023

/retest

3 similar comments
@m-chau
Copy link
Author

m-chau commented Oct 30, 2023

/retest

@m-chau
Copy link
Author

m-chau commented Nov 9, 2023

/retest

@m-chau
Copy link
Author

m-chau commented Nov 9, 2023

/retest

@m-chau
Copy link
Author

m-chau commented Nov 14, 2023

/test

@aks-lts-prow
Copy link

aks-lts-prow bot commented Nov 14, 2023

@m-chau: The /test command needs one or more targets.
The following commands are available to trigger required jobs:

  • /test pull-kubernetes-dependencies
  • /test pull-kubernetes-e2e-gce
  • /test pull-kubernetes-e2e-gce-100-performance
  • /test pull-kubernetes-e2e-gce-canary
  • /test pull-kubernetes-e2e-gce-ubuntu-containerd-canary
  • /test pull-kubernetes-files-remake
  • /test pull-kubernetes-integration
  • /test pull-kubernetes-kubemark-e2e-gce-scale
  • /test pull-kubernetes-typecheck
  • /test pull-kubernetes-unit
  • /test pull-kubernetes-update
  • /test pull-kubernetes-verify-govet-levee

The following commands are available to trigger optional jobs:

  • /test pull-kubernetes-conformance-kind-ipv6-parallel
  • /test pull-kubernetes-e2e-containerd-gce
  • /test pull-kubernetes-e2e-gce-device-plugin-gpu
  • /test pull-kubernetes-e2e-kops-aws
  • /test pull-kubernetes-integration-go-compatibility
  • /test pull-kubernetes-kubemark-e2e-gce-big
  • /test pull-kubernetes-node-e2e-containerd-kubetest2
  • /test pull-kubernetes-unit-go-compatibility

Use /test all to run the following jobs that were automatically triggered:

  • pull-kubernetes-dependencies
  • pull-kubernetes-integration
  • pull-kubernetes-integration-go-compatibility
  • pull-kubernetes-typecheck
  • pull-kubernetes-unit
  • pull-kubernetes-unit-go-compatibility
  • pull-kubernetes-verify-govet-levee

In response to this:

/test

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.

@m-chau
Copy link
Author

m-chau commented Nov 14, 2023

/test all

@m-chau
Copy link
Author

m-chau commented Nov 14, 2023

/retest

haitch
haitch previously approved these changes Nov 14, 2023
Copy link

@haitch haitch left a comment

Choose a reason for hiding this comment

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

identical to upstream PR.
/approve
/lgtm

@m-chau m-chau force-pushed the automated-cherry-pick-of-#121201-#121228-upstream-release-1.24 branch from bb9d6bf to 1cedd95 Compare November 16, 2023 08:44
@enj
Copy link
Member

enj commented Nov 16, 2023

The contents of 62fd9e9 and 1cedd95 do not look correct.

enj and others added 4 commits November 16, 2023 21:25
This change fully addresses CVE-2023-44487 and CVE-2023-39325 for
the API server when the client is unauthenticated.

The changes to util/runtime are required because otherwise a large
number of requests can get blocked on the time.Sleep calls.

For unauthenticated clients (either via 401 or the anonymous user),
we simply no longer allow such clients to hold open http2
connections.  They can use http2, but with the performance of http1
(with keep-alive disabled).

Since this change has the potential to cause issues, the
UnauthenticatedHTTP2DOSMitigation feature gate can be disabled to
remove this protection (it is enabled by default).  For example,
when the API server is fronted by an L7 load balancer that is set up
to mitigate http2 attacks, unauthenticated clients could force
disable connection reuse between the load balancer and the API
server (many incoming connections could share the same backend
connection).  An API server that is on a private network may opt to
disable this protection to prevent performance regressions for
unauthenticated clients.

For all other clients, we rely on the golang.org/x/net fix in
golang/net@b225e7c
That change is not sufficient to adequately protect against a
motivated client - future changes to Kube and/or golang.org/x/net
will be explored to address this gap.

The Kube API server now uses a max stream of 100 instead of 250
(this matches the Go http2 client default).  This lowers the abuse
limit from 1000 to 400.

Signed-off-by: Monis Khan <mok@microsoft.com>
This makes backports safer by not changing any default behavior.

Signed-off-by: Monis Khan <mok@microsoft.com>
These occasionally flake on CI:

https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/121200/pull-kubernetes-unit-go-compatibility/1712589824344461312

=== Failed
=== FAIL: vendor/k8s.io/apiserver/pkg/endpoints/filters TestUnauthenticatedHTTP2ClientConnectionClose/other_skip=true/http/1.1 (0.19s)
    authentication_test.go:653: expect TCP connection: 1, actual: 2
        --- FAIL: TestUnauthenticatedHTTP2ClientConnectionClose/other_skip=true/http/1.1 (0.19s)

=== FAIL: vendor/k8s.io/apiserver/pkg/endpoints/filters TestUnauthenticatedHTTP2ClientConnectionClose/other_skip=true (0.23s)
    --- FAIL: TestUnauthenticatedHTTP2ClientConnectionClose/other_skip=true (0.23s)

=== FAIL: vendor/k8s.io/apiserver/pkg/endpoints/filters TestUnauthenticatedHTTP2ClientConnectionClose (2.30s)

Signed-off-by: Monis Khan <mok@microsoft.com>
@m-chau m-chau force-pushed the automated-cherry-pick-of-#121201-#121228-upstream-release-1.24 branch from 1cedd95 to ee007a0 Compare November 16, 2023 21:30
@enj
Copy link
Member

enj commented Nov 16, 2023

/lgtm
/approve

@aks-lts-prow aks-lts-prow bot added the lgtm label Nov 16, 2023
@aks-lts-prow
Copy link

aks-lts-prow bot commented Nov 16, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: enj, haitch, m-chau

The full list of commands accepted by this bot can be found 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

@haitch haitch merged commit 1e0ea68 into aks-lts:release-1.24 Nov 16, 2023
7 of 8 checks passed
@m-chau m-chau deleted the automated-cherry-pick-of-#121201-#121228-upstream-release-1.24 branch March 25, 2024 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants