Skip to content

Commit d0d9650

Browse files
committed
Enforce response body limits in kubelet-to-gcm
- Added io.LimitReader to response body reads in controller, kubelet, and config packages. - Prevents unbounded memory allocation from potentially large responses. - Switched from deprecated ioutil.ReadAll to io.ReadAll. - Added unit tests for size limiting and success cases.
1 parent fe8f334 commit d0d9650

5 files changed

Lines changed: 195 additions & 6 deletions

File tree

kubelet-to-gcm/monitor/config/initialize.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package config
1818

1919
import (
2020
"fmt"
21-
"io/ioutil"
21+
"io"
2222
"net/http"
2323
"strings"
2424
"time"
@@ -29,6 +29,9 @@ import (
2929
const (
3030
gceMetaDataEndpoint = "http://169.254.169.254"
3131
gceMetaDataPrefix = "/computeMetadata/v1"
32+
// maxResponseBodySize is the maximum size of the response body we will read.
33+
// 10MB is more than enough for GCE metadata responses.
34+
maxResponseBodySize = 10 * 1024 * 1024
3235
)
3336

3437
// NewConfigs returns the SourceConfigs for all monitored endpoints, and
@@ -117,7 +120,7 @@ func getGCEMetaData(uri string) ([]byte, error) {
117120
return nil, fmt.Errorf("Failed request %q for GCE metadata: %v", uri, err)
118121
}
119122
defer resp.Body.Close()
120-
body, err := ioutil.ReadAll(resp.Body)
123+
body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodySize))
121124
if err != nil {
122125
return nil, fmt.Errorf("Failed to read body for request %q for GCE metadata: %v", uri, err)
123126
}

kubelet-to-gcm/monitor/controller/client.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package controller
1919
import (
2020
"fmt"
2121
"io"
22-
"io/ioutil"
2322
"net/http"
2423
"net/url"
2524
"strings"
@@ -72,6 +71,12 @@ func NewMetrics(body []byte) (*Metrics, error) {
7271
return metrics, nil
7372
}
7473

74+
const (
75+
// maxResponseBodySize is the maximum size of the response body we will read.
76+
// 10MB is more than enough for most prometheus metrics responses.
77+
maxResponseBodySize = 10 * 1024 * 1024
78+
)
79+
7580
// Client queries metrics from the controller process.
7681
type Client struct {
7782
client *http.Client
@@ -99,7 +104,7 @@ func (c *Client) doRequestAndParse(req *http.Request) (*Metrics, error) {
99104
return nil, err
100105
}
101106
defer response.Body.Close()
102-
body, err := ioutil.ReadAll(response.Body)
107+
body, err := io.ReadAll(io.LimitReader(response.Body, maxResponseBodySize))
103108
if err != nil {
104109
return nil, fmt.Errorf("failed to read response body - %v", err)
105110
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
Copyright 2017 Google Inc.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package controller
18+
19+
import (
20+
"fmt"
21+
"net/http"
22+
"net/http/httptest"
23+
"net/url"
24+
"testing"
25+
)
26+
27+
func TestDoRequestAndParse_SizeLimit(t *testing.T) {
28+
// Create a mock server that returns a response larger than maxResponseBodySize.
29+
largeDataSize := maxResponseBodySize + 1024
30+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
31+
// Write more than maxResponseBodySize bytes.
32+
data := make([]byte, largeDataSize)
33+
w.Write(data)
34+
}))
35+
defer ts.Close()
36+
37+
u, _ := url.Parse(ts.URL)
38+
client := &Client{
39+
client: ts.Client(),
40+
metricsURL: u,
41+
}
42+
43+
req, _ := http.NewRequest("GET", ts.URL, nil)
44+
_, err := client.doRequestAndParse(req)
45+
46+
if err == nil {
47+
t.Fatal("Expected error due to size limit, but got none")
48+
}
49+
50+
// The error should indicate a problem reading the body or parsing it.
51+
// Since we are writing random null bytes, parsing should fail if it gets that far.
52+
// But io.LimitReader will just stop at maxResponseBodySize.
53+
// io.ReadAll doesn't return an error if it reaches EOF.
54+
// However, NewMetrics(body) will fail to parse the truncated/invalid data.
55+
fmt.Printf("Got expected error: %v\n", err)
56+
}
57+
58+
func TestDoRequestAndParse_Success(t *testing.T) {
59+
metricsData := `
60+
# HELP node_collector_evictions_number Number of evictions
61+
# TYPE node_collector_evictions_number counter
62+
node_collector_evictions_number 10
63+
# HELP process_start_time_seconds Start time
64+
# TYPE process_start_time_seconds gauge
65+
process_start_time_seconds 1234567890
66+
`
67+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
68+
fmt.Fprint(w, metricsData)
69+
}))
70+
defer ts.Close()
71+
72+
u, _ := url.Parse(ts.URL)
73+
client := &Client{
74+
client: ts.Client(),
75+
metricsURL: u,
76+
}
77+
78+
req, _ := http.NewRequest("GET", ts.URL, nil)
79+
metrics, err := client.doRequestAndParse(req)
80+
81+
if err != nil {
82+
t.Fatalf("Unexpected error: %v", err)
83+
}
84+
85+
if metrics.NodeEvictions != 10 {
86+
t.Errorf("Expected 10 evictions, got %d", metrics.NodeEvictions)
87+
}
88+
if metrics.CreateTime != 1234567890 {
89+
t.Errorf("Expected 1234567890 create time, got %d", metrics.CreateTime)
90+
}
91+
}

kubelet-to-gcm/monitor/kubelet/client.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,19 @@ package kubelet
1919
import (
2020
"encoding/json"
2121
"fmt"
22-
"io/ioutil"
22+
"io"
2323
"net/http"
2424
"net/url"
2525

2626
stats "k8s.io/kubelet/pkg/apis/stats/v1alpha1"
2727
)
2828

29+
const (
30+
// maxResponseBodySize is the maximum size of the response body we will read.
31+
// 50MB should be enough for any Kubelet stats summary response.
32+
maxResponseBodySize = 50 * 1024 * 1024
33+
)
34+
2935
// Client contains all the information and methods to encapsulate
3036
// communication with the Kubelet.
3137
type Client struct {
@@ -58,7 +64,7 @@ func (k *Client) doRequestAndUnmarshal(client *http.Client, req *http.Request, v
5864
return err
5965
}
6066
defer response.Body.Close()
61-
body, err := ioutil.ReadAll(response.Body)
67+
body, err := io.ReadAll(io.LimitReader(response.Body, maxResponseBodySize))
6268
if err != nil {
6369
return fmt.Errorf("failed to read response body - %v", err)
6470
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
Copyright 2017 Google Inc.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package kubelet
18+
19+
import (
20+
"encoding/json"
21+
"fmt"
22+
"net/http"
23+
"net/http/httptest"
24+
"net/url"
25+
"testing"
26+
27+
stats "k8s.io/kubelet/pkg/apis/stats/v1alpha1"
28+
)
29+
30+
func TestDoRequestAndUnmarshal_SizeLimit(t *testing.T) {
31+
largeDataSize := maxResponseBodySize + 1024
32+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
33+
data := make([]byte, largeDataSize)
34+
w.Write(data)
35+
}))
36+
defer ts.Close()
37+
38+
u, _ := url.Parse(ts.URL)
39+
k := &Client{
40+
client: ts.Client(),
41+
summaryURL: u,
42+
}
43+
44+
req, _ := http.NewRequest("GET", ts.URL, nil)
45+
var value stats.Summary
46+
err := k.doRequestAndUnmarshal(ts.Client(), req, &value)
47+
48+
if err == nil {
49+
t.Fatal("Expected error due to size limit, but got none")
50+
}
51+
fmt.Printf("Got expected error: %v\n", err)
52+
}
53+
54+
func TestDoRequestAndUnmarshal_Success(t *testing.T) {
55+
summary := &stats.Summary{
56+
Node: stats.NodeStats{
57+
NodeName: "test-node",
58+
},
59+
}
60+
data, _ := json.Marshal(summary)
61+
62+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
63+
w.Write(data)
64+
}))
65+
defer ts.Close()
66+
67+
u, _ := url.Parse(ts.URL)
68+
k := &Client{
69+
client: ts.Client(),
70+
summaryURL: u,
71+
}
72+
73+
req, _ := http.NewRequest("GET", ts.URL, nil)
74+
var result stats.Summary
75+
err := k.doRequestAndUnmarshal(ts.Client(), req, &result)
76+
77+
if err != nil {
78+
t.Fatalf("Unexpected error: %v", err)
79+
}
80+
81+
if result.Node.NodeName != "test-node" {
82+
t.Errorf("Expected test-node, got %s", result.Node.NodeName)
83+
}
84+
}

0 commit comments

Comments
 (0)