Skip to content
Permalink
Browse files Browse the repository at this point in the history
[release-1.13] Harden validation of untrusted inputs (#90) (#96)
* Harden validation of untrusted inputs (#90)

* Use latest proxy sha

Co-authored-by: John Howard <howardjohn@google.com>
  • Loading branch information
jacob-delgado and howardjohn committed Feb 18, 2022
1 parent ca06fda commit 5f3b5ed
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 13 deletions.
2 changes: 1 addition & 1 deletion istio.deps
Expand Up @@ -4,6 +4,6 @@
"name": "PROXY_REPO_SHA",
"repoName": "proxy",
"file": "",
"lastStableSHA": "9335502ad4a84f380d7d5e0f3ba35a088501ef02"
"lastStableSHA": "a971bfa7fddce41c1795fe2ce279735131c08878"
}
]
2 changes: 1 addition & 1 deletion pilot/pkg/model/context.go
Expand Up @@ -348,7 +348,7 @@ func (l StringList) MarshalJSON() ([]byte, error) {
}

func (l *StringList) UnmarshalJSON(data []byte) error {
if len(data) == 0 || string(data) == `""` {
if len(data) < 2 || string(data) == `""` {
*l = []string{}
} else {
*l = strings.Split(string(data[1:len(data)-1]), ",")
Expand Down
17 changes: 11 additions & 6 deletions pilot/pkg/model/context_test.go
Expand Up @@ -145,13 +145,15 @@ func TestNodeMetadata(t *testing.T) {

func TestStringList(t *testing.T) {
cases := []struct {
in string
expect model.StringList
in string
expect model.StringList
noRoundTrip bool
}{
{`"a,b,c"`, []string{"a", "b", "c"}},
{`"a"`, []string{"a"}},
{`""`, []string{}},
{`"123,@#$#,abcdef"`, []string{"123", "@#$#", "abcdef"}},
{in: `"a,b,c"`, expect: []string{"a", "b", "c"}},
{in: `"a"`, expect: []string{"a"}},
{in: `""`, expect: []string{}},
{in: `"123,@#$#,abcdef"`, expect: []string{"123", "@#$#", "abcdef"}},
{in: `1`, expect: []string{}, noRoundTrip: true},
}
for _, tt := range cases {
t.Run(tt.in, func(t *testing.T) {
Expand All @@ -166,6 +168,9 @@ func TestStringList(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if tt.noRoundTrip {
return
}
if !reflect.DeepEqual(string(b), tt.in) {
t.Fatalf("Expected %v, got %v", tt.in, string(b))
}
Expand Down
2 changes: 1 addition & 1 deletion pilot/pkg/networking/core/v1alpha3/cluster_builder_test.go
Expand Up @@ -3055,7 +3055,7 @@ func TestApplyDestinationRuleOSCACert(t *testing.T) {
t.Errorf("Could not parse destination rule: %v", err)
}
dr := &networking.DestinationRule{}
err = json.Unmarshal(byteArray, &dr)
err = json.Unmarshal(byteArray, dr)
if err != nil {
t.Errorf("Could not unmarshal destination rule: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion security/pkg/server/ca/authenticate/oidc.go
Expand Up @@ -92,7 +92,7 @@ func (j *JwtAuthenticator) authenticate(ctx context.Context, bearerToken string)
return nil, fmt.Errorf("failed to verify the JWT token (error %v)", err)
}

sa := &JwtPayload{}
sa := JwtPayload{}
// "aud" for trust domain, "sub" has "system:serviceaccount:$namespace:$serviceaccount".
// in future trust domain may use another field as a standard is defined.
if err := idToken.Claims(&sa); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion security/pkg/util/jwtutil.go
Expand Up @@ -112,7 +112,7 @@ func ExtractJwtAud(jwt string) ([]string, bool) {
return nil, false
}

structuredPayload := &jwtPayload{}
structuredPayload := jwtPayload{}
err = json.Unmarshal(payloadBytes, &structuredPayload)
if err != nil {
return nil, false
Expand Down
6 changes: 4 additions & 2 deletions security/pkg/util/jwtutil_test.go
Expand Up @@ -133,7 +133,9 @@ func Test3p(t *testing.T) {
t.Error("Expecting bound token, detected unbound ", s)
}
}
if !IsK8SUnbound(firstPartyJwt) {
t.Error("Expecting unbound, detected bound ", firstPartyJwt)
for _, s := range []string{firstPartyJwt, ".bnVsbM."} {
if !IsK8SUnbound(s) {
t.Error("Expecting unbound, detected bound ", s)
}
}
}
1 change: 1 addition & 0 deletions tests/fuzz/testdata/FuzzBNMUnmarshalJSON/4811475191988224
@@ -0,0 +1 @@
{"INSTANCE_IPS":1}
1 change: 1 addition & 0 deletions tests/fuzz/testdata/FuzzJwtUtil/5085913745588224
@@ -0,0 +1 @@
.bnVsbM.

0 comments on commit 5f3b5ed

Please sign in to comment.