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

fix: disable HTTP2 connections by default #6028

Merged
merged 1 commit into from Oct 30, 2023
Merged

fix: disable HTTP2 connections by default #6028

merged 1 commit into from Oct 30, 2023

Conversation

simonpasquier
Copy link
Contributor

Description

This change mitigates CVE-2023-44487 by disabling HTTP2 connections by default and forcing HTTP/1.1 until the Go standard library and golang.org/x/net are fully fixed. Right now, it is possible for authenticated and unauthenticated users to hold open HTTP2 connections and consume huge amounts of memory.

It is possible to revert back the change by using the --web.enable-http2 argument.

Before this change:

curl -kv https://localhost:8443/metrics
*   Trying 127.0.0.1:8443...
* Connected to localhost (127.0.0.1) port 8443 (#0)
* ALPN: offers h2,http/1.1
[...]
* ALPN: server accepted h2
[...]
* using HTTP/2
* h2h3 [:method: GET]
* h2h3 [:path: /metrics]
* h2h3 [:scheme: https]
* h2h3 [:authority: localhost:8443]
* h2h3 [user-agent: curl/8.0.1]
* h2h3 [accept: */*]
* Using Stream ID: 1 (easy handle 0x5594d4614b10)
[...]
> GET /metrics HTTP/2
[...]

After this change:

curl -kv https://localhost:8443/metrics
*   Trying 127.0.0.1:8443...
* Connected to localhost (127.0.0.1) port 8443 (#0)
* ALPN: offers h2,http/1.1
[...]
* ALPN: server accepted http/1.1
[...]
* using HTTP/1.1
> GET /metrics HTTP/1.1
> Host: localhost:8443
> User-Agent: curl/8.0.1
> Accept: */*
[...]
< HTTP/1.1 200 OK
[...]

See also:

Type of change

What type of changes does your code introduce to the Prometheus operator? Put an x in the box that apply.

  • CHANGE (fix or feature that would cause existing functionality to not work as expected)
  • FEATURE (non-breaking change which adds functionality)
  • BUGFIX (non-breaking change which fixes an issue)
  • ENHANCEMENT (non-breaking change which improves existing functionality)
  • NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Changelog entry

Please put a one-line changelog entry below. This will be copied to the changelog file during the release process.

Disable HTTP2 connection by default to address CVE-2023-44487

@simonpasquier simonpasquier requested a review from a team as a code owner October 19, 2023 13:30
@simonpasquier
fix: disable HTTP2 connections by default
a62e814 
This change mitigates CVE-2023-44487 by disabling HTTP2 by default and
forcing HTTP/1.1 until the Go standard library and golang.org/x/net are
fully fixed. Right now, it is possible for authenticated and
unauthenticated users to hold open HTTP2 connections and consume huge
amounts of memory.

It is possible to revert back the change by using the
`--web.enable-http2` argument.

Before this change:

```
curl -kv https://localhost:8443/metrics
*   Trying 127.0.0.1:8443...
* Connected to localhost (127.0.0.1) port 8443 (#0)
* ALPN: offers h2,http/1.1
[...]
* ALPN: server accepted h2
[...]
* using HTTP/2
* h2h3 [:method: GET]
* h2h3 [:path: /metrics]
* h2h3 [:scheme: https]
* h2h3 [:authority: localhost:8443]
* h2h3 [user-agent: curl/8.0.1]
* h2h3 [accept: */*]
* Using Stream ID: 1 (easy handle 0x5594d4614b10)
[...]
> GET /metrics HTTP/2
[...]
```

After this change:

```
curl -kv https://localhost:8443/metrics
*   Trying 127.0.0.1:8443...
* Connected to localhost (127.0.0.1) port 8443 (#0)
* ALPN: offers h2,http/1.1
[...]
* ALPN: server accepted http/1.1
[...]
* using HTTP/1.1
> GET /metrics HTTP/1.1
> Host: localhost:8443
> User-Agent: curl/8.0.1
> Accept: */*
[...]
< HTTP/1.1 200 OK
[...]
```

See also:
* kubernetes/kubernetes#121120
* kubernetes/kubernetes#121197
* golang/go#63417 (comment)

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
@skonto skonto mentioned this pull request Oct 20, 2023
Merged
@simonpasquier simonpasquier merged commit dfc150a into prometheus-operator:main Oct 30, 2023
17 checks passed
slashpai added a commit to slashpai/prometheus-operator that referenced this pull request Nov 3, 2023
@slashpai
OCPBUGS-20861: disable HTTP2 connections by default
4b4a2a1 
Backporting prometheus-operator#6028

Signed-off-by: Jayapriya Pai <janantha@redhat.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this pull request Nov 3, 2023
@slashpai
OCPBUGS-20843: disable HTTP2 connections by default
172af86 
Backporting prometheus-operator#6028

Signed-off-by: Jayapriya Pai <janantha@redhat.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this pull request Nov 3, 2023
@slashpai
OCPBUGS-20823: disable HTTP2 connections by default
ab3dd99 
Backporting prometheus-operator#6028

Signed-off-by: Jayapriya Pai <janantha@redhat.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this pull request Nov 6, 2023
@slashpai
fix: remove verbose logging admission-webhook
374f4b3 
After disabling HTTP2 connections by default in prometheus-operator#6028
started seeing verbose logging in admission webhook pods like:

`ts=2023-11-06T01:50:34.601774794Z caller=stdlib.go:105 caller=server.go:3215 msg="http: superfluous response.WriteHeader call from main.newSrv.func1 (main.go:173)"
`

Go doc says If WriteHeader is not called explicitly,
the first call to Write will trigger an implicit
WriteHeader(http.StatusOK).

This fix is to change the order of the method invocation
so log doesn't complain about this

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
@slashpai slashpai mentioned this pull request Nov 6, 2023
Merged
5 tasks
slashpai added a commit to slashpai/prometheus-operator that referenced this pull request Nov 6, 2023
@slashpai
fix: remove verbose logging admission-webhook
7111d67 
After disabling HTTP2 connections by default in prometheus-operator#6028
started seeing verbose logging in admission webhook pods like:

`ts=2023-11-06T01:50:34.601774794Z caller=stdlib.go:105 caller=server.go:3215 msg="http: superfluous response.WriteHeader call from main.newSrv.func1 (main.go:173)"
`

Go doc says If WriteHeader is not called explicitly,
the first call to Write will trigger an implicit
WriteHeader(http.StatusOK).

This fix is to change the order of the method invocation
so log doesn't complain about this

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
(cherry picked from commit 374f4b3)
@slashpai slashpai mentioned this pull request Nov 6, 2023
Merged
5 tasks
slashpai added a commit to slashpai/prometheus-operator that referenced this pull request Nov 6, 2023
@slashpai
fix: remove verbose logging admission-webhook
f858900 
After disabling HTTP2 connections by default in prometheus-operator#6028
started seeing verbose logging in admission webhook pods like:

`ts=2023-11-06T01:50:34.601774794Z caller=stdlib.go:105 caller=server.go:3215 msg="http: superfluous response.WriteHeader call from main.newSrv.func1 (main.go:173)"
`

Go doc says If WriteHeader is not called explicitly,
the first call to Write will trigger an implicit
WriteHeader(http.StatusOK).

This fix is to change the order of the method invocation
so log doesn't complain about this

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
(cherry picked from commit 374f4b3)
slashpai added a commit to slashpai/prometheus-operator that referenced this pull request Nov 6, 2023
@slashpai
fix: remove verbose logging admission-webhook
f43b281 
After disabling HTTP2 connections by default in prometheus-operator#6028
started seeing verbose logging in admission webhook pods like:

`ts=2023-11-06T01:50:34.601774794Z caller=stdlib.go:105 caller=server.go:3215 msg="http: superfluous response.WriteHeader call from main.newSrv.func1 (main.go:173)"
`

Go doc says If WriteHeader is not called explicitly,
the first call to Write will trigger an implicit
WriteHeader(http.StatusOK).

This fix is to change the order of the method invocation
so log doesn't complain about this

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
(cherry picked from commit 374f4b3)
slashpai added a commit to slashpai/prometheus-operator that referenced this pull request Nov 6, 2023
@slashpai
fix: remove verbose logging admission-webhook
2e5b5b5 
After disabling HTTP2 connections by default in prometheus-operator#6028
started seeing verbose logging in admission webhook pods like:

`ts=2023-11-06T01:50:34.601774794Z caller=stdlib.go:105 caller=server.go:3215 msg="http: superfluous response.WriteHeader call from main.newSrv.func1 (main.go:173)"
`

Go doc says If WriteHeader is not called explicitly,
the first call to Write will trigger an implicit
WriteHeader(http.StatusOK).

This fix is to change the order of the method invocation
so log doesn't complain about this

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
(cherry picked from commit 374f4b3)
slashpai added a commit to slashpai/prometheus-operator that referenced this pull request Nov 6, 2023
@slashpai
fix: remove verbose logging admission-webhook
037a2fb 
After disabling HTTP2 connections by default in prometheus-operator#6028
started seeing verbose logging in admission webhook pods like:

`ts=2023-11-06T01:50:34.601774794Z caller=stdlib.go:105 caller=server.go:3215 msg="http: superfluous response.WriteHeader call from main.newSrv.func1 (main.go:173)"
`

Go doc says If WriteHeader is not called explicitly,
the first call to Write will trigger an implicit
WriteHeader(http.StatusOK).

This fix is to change the order of the method invocation
so log doesn't complain about this

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
(cherry picked from commit 374f4b3)
adinhodovic pushed a commit to adinhodovic/prometheus-operator that referenced this pull request Nov 13, 2023
@slashpai @adinhodovic
fix: remove verbose logging admission-webhook
ee6a7eb 
After disabling HTTP2 connections by default in prometheus-operator#6028
started seeing verbose logging in admission webhook pods like:

`ts=2023-11-06T01:50:34.601774794Z caller=stdlib.go:105 caller=server.go:3215 msg="http: superfluous response.WriteHeader call from main.newSrv.func1 (main.go:173)"
`

Go doc says If WriteHeader is not called explicitly,
the first call to Write will trigger an implicit
WriteHeader(http.StatusOK).

This fix is to change the order of the method invocation
so log doesn't complain about this

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolastakashi nicolastakashi nicolastakashi approved these changes

@ArthurSens ArthurSens ArthurSens approved these changes

@philipgough philipgough Awaiting requested review from philipgough

Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@simonpasquier @nicolastakashi @ArthurSens