Skip to content

Commit 04b9a64

Browse files
csweichelroboquat
authored andcommitted
[ws-manager] Limit max secret length
to introduce a benign failure mode compared to the Kubernetes provided message.
1 parent 34e5648 commit 04b9a64

File tree

4 files changed

+75
-1
lines changed

4 files changed

+75
-1
lines changed

components/content-service-api/go/initializer.go

+4
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ func InjectSecretsToInitializer(init *WorkspaceInitializer, secrets map[string][
8686

8787
// WalkInitializer walks the initializer structure
8888
func WalkInitializer(path []string, init *WorkspaceInitializer, visitor func(path []string, init *WorkspaceInitializer) error) error {
89+
if init == nil {
90+
return nil
91+
}
92+
8993
switch spec := init.Spec.(type) {
9094
case *WorkspaceInitializer_Backup:
9195
return visitor(append(path, "backup"), init)

components/content-service-api/go/initializer_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ func TestGetCheckoutLocationsFromInitializer(t *testing.T) {
100100
},
101101
Expectation: "/foo,/bar",
102102
},
103+
{
104+
Name: "nil initializer",
105+
},
103106
}
104107

105108
for _, test := range tests {

components/ws-manager/pkg/manager/create.go

+16-1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,13 @@ var (
4242
boolTrue = true
4343
)
4444

45+
const (
46+
// maxSecretsLength is the maximum number of bytes a workspace secret may contain. This size is exhausted by
47+
// environment variables provided as part of the start workspace request.
48+
// The value of 768kb is a somewhat arbitrary choice, but steers way clear of the 1MiB Kubernetes imposes.
49+
maxSecretsLength = 768 * 1024 * 1024
50+
)
51+
4552
// createWorkspacePod creates the actual workspace pod based on the definite workspace pod and appropriate
4653
// templates. The result of this function is not expected to be modified prior to being passed to Kubernetes.
4754
func (m *Manager) createWorkspacePod(startContext *startWorkspaceContext) (*corev1.Pod, error) {
@@ -962,7 +969,10 @@ func (m *Manager) newStartWorkspaceContext(ctx context.Context, req *api.StartWo
962969
}
963970
}
964971

965-
secrets := make(map[string]string)
972+
var (
973+
secrets = make(map[string]string)
974+
secretsLen int
975+
)
966976
for _, env := range req.Spec.Envvars {
967977
if env.Secret != nil {
968978
continue
@@ -973,9 +983,14 @@ func (m *Manager) newStartWorkspaceContext(ctx context.Context, req *api.StartWo
973983

974984
name := fmt.Sprintf("%x", sha256.Sum256([]byte(env.Name)))
975985
secrets[name] = env.Value
986+
secretsLen += len(env.Value)
976987
}
977988
for k, v := range csapi.ExtractSecretsFromInitializer(req.Spec.Initializer) {
978989
secrets[k] = v
990+
secretsLen += len(v)
991+
}
992+
if secretsLen > maxSecretsLength {
993+
return nil, xerrors.Errorf("secrets exceed maximum permitted length (%d > %d bytes): please reduce the numer or length of environment variables", secretsLen, maxSecretsLength)
979994
}
980995

981996
return &startWorkspaceContext{

components/ws-manager/pkg/manager/create_test.go

+52
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,67 @@ import (
1717
"sigs.k8s.io/yaml"
1818

1919
ctesting "github.com/gitpod-io/gitpod/common-go/testing"
20+
csapi "github.com/gitpod-io/gitpod/content-service/api"
2021
"github.com/gitpod-io/gitpod/ws-manager/api"
2122
config "github.com/gitpod-io/gitpod/ws-manager/api/config"
23+
"github.com/google/go-cmp/cmp"
2224
)
2325

2426
var (
2527
team = "awesome"
2628
project = "gitpod"
2729
)
2830

31+
func TestNewStartWorkspaceContext(t *testing.T) {
32+
type Expectation struct {
33+
Context *startWorkspaceContext
34+
Error string
35+
}
36+
tests := []struct {
37+
Name string
38+
Req *api.StartWorkspaceRequest
39+
Expectation Expectation
40+
}{
41+
{
42+
Name: "oversized secrets",
43+
Req: &api.StartWorkspaceRequest{
44+
Metadata: &api.WorkspaceMetadata{Owner: "foo"},
45+
Spec: &api.StartWorkspaceSpec{
46+
Initializer: &csapi.WorkspaceInitializer{
47+
Spec: &csapi.WorkspaceInitializer_Empty{},
48+
},
49+
Envvars: []*api.EnvironmentVariable{
50+
{Name: "too_large", Value: string(func() []byte { return make([]byte, 2*maxSecretsLength) }())},
51+
},
52+
},
53+
},
54+
Expectation: Expectation{
55+
Error: "secrets exceed maximum permitted length (1610612736 > 805306368 bytes): please reduce the numer or length of environment variables",
56+
},
57+
},
58+
}
59+
60+
for _, test := range tests {
61+
t.Run(test.Name, func(t *testing.T) {
62+
mgmtCfg := forTestingOnlyManagerConfig()
63+
manager := &Manager{Config: mgmtCfg}
64+
65+
sctx, err := manager.newStartWorkspaceContext(context.Background(), test.Req)
66+
67+
act := Expectation{
68+
Context: sctx,
69+
}
70+
if err != nil {
71+
act.Error = err.Error()
72+
}
73+
74+
if diff := cmp.Diff(test.Expectation, act); diff != "" {
75+
t.Errorf("unexpected newStartWorkspaceContext (-want +got):\n%s", diff)
76+
}
77+
})
78+
}
79+
}
80+
2981
func TestCreateDefiniteWorkspacePod(t *testing.T) {
3082
type WorkspaceClass struct {
3183
DefaultTemplate *corev1.Pod `json:"defaultTemplate,omitempty"`

0 commit comments

Comments
 (0)