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

cors: do not forwarded preflight request if the origin is not allowed #50452

Merged
merged 7 commits into from
May 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions pilot/pkg/model/context.go
Expand Up @@ -523,6 +523,13 @@ func (node *Proxy) SetSidecarScope(ps *PushContext) {
node.PrevSidecarScope = sidecarScope
}

func (node *Proxy) VersionGreaterAndEqual(inv *IstioVersion) bool {
if inv == nil {
return true
}
return node.IstioVersion.Compare(inv) >= 0
}

// SetGatewaysForProxy merges the Gateway objects associated with this
// proxy and caches the merged object in the proxy Node. This is a convenience hack so that
// callers can simply call push.MergedGateways(node) instead of having to
Expand Down
2 changes: 1 addition & 1 deletion pilot/pkg/networking/core/listener_waypoint.go
Expand Up @@ -563,7 +563,7 @@ func (lb *ListenerBuilder) translateRoute(
out.TypedPerFilterConfig[wellknown.Fault] = protoconv.MessageToAny(istio_route.TranslateFault(in.Fault))
}
if in.CorsPolicy != nil {
out.TypedPerFilterConfig[wellknown.CORS] = protoconv.MessageToAny(istio_route.TranslateCORSPolicy(in.CorsPolicy))
out.TypedPerFilterConfig[wellknown.CORS] = protoconv.MessageToAny(istio_route.TranslateCORSPolicy(lb.node, in.CorsPolicy))
}

return out
Expand Down
19 changes: 17 additions & 2 deletions pilot/pkg/networking/core/route/route.go
Expand Up @@ -505,7 +505,7 @@ func translateRoute(
out.TypedPerFilterConfig[wellknown.Fault] = protoconv.MessageToAny(TranslateFault(in.Fault))
}
if in.CorsPolicy != nil {
out.TypedPerFilterConfig[wellknown.CORS] = protoconv.MessageToAny(TranslateCORSPolicy(in.CorsPolicy))
out.TypedPerFilterConfig[wellknown.CORS] = protoconv.MessageToAny(TranslateCORSPolicy(node, in.CorsPolicy))
}
var statefulConfig *statefulsession.StatefulSession
for _, hostname := range hostnames {
Expand Down Expand Up @@ -1155,14 +1155,29 @@ func translateHeaderMatch(name string, in *networking.StringMatch) *route.Header
return out
}

func forwardNotMatchingPreflights(cors *networking.CorsPolicy) *wrapperspb.BoolValue {
if cors.GetUnmatchedPreflights() == networking.CorsPolicy_IGNORE {
return wrapperspb.Bool(false)
}

// This is the default behavior before envoy 1.30.
return wrapperspb.Bool(true)
}

// TranslateCORSPolicy translates CORS policy
func TranslateCORSPolicy(in *networking.CorsPolicy) *cors.CorsPolicy {
func TranslateCORSPolicy(proxy *model.Proxy, in *networking.CorsPolicy) *cors.CorsPolicy {
if in == nil {
return nil
}

// CORS filter is enabled by default
out := cors.CorsPolicy{}
// Start from Envoy 1.30(istio 1.22), cors filter will not forward preflight requests to upstream by default.
// Istio start support this feature from 1.23.
if proxy.VersionGreaterAndEqual(&model.IstioVersion{Major: 1, Minor: 23, Patch: -1}) {
out.ForwardNotMatchingPreflights = forwardNotMatchingPreflights(in)
}

// nolint: staticcheck
if in.AllowOrigins != nil {
out.AllowOriginStringMatch = util.ConvertToEnvoyMatches(in.AllowOrigins)
Expand Down
51 changes: 50 additions & 1 deletion pilot/pkg/networking/core/route/route_internal_test.go
Expand Up @@ -29,6 +29,7 @@ import (
"google.golang.org/protobuf/types/known/wrapperspb"

networking "istio.io/api/networking/v1alpha3"
"istio.io/istio/pilot/pkg/model"
authzmatcher "istio.io/istio/pilot/pkg/security/authz/matcher"
authz "istio.io/istio/pilot/pkg/security/authz/model"
"istio.io/istio/pkg/config/labels"
Expand Down Expand Up @@ -163,7 +164,55 @@ func TestIsCatchAllRoute(t *testing.T) {
}
}

func TestTranslateCORSPolicyForwardNotMatchingPreflights(t *testing.T) {
node := &model.Proxy{
IstioVersion: &model.IstioVersion{
Major: 1,
Minor: 23,
Patch: 0,
},
}
corsPolicy := &networking.CorsPolicy{
AllowOrigins: []*networking.StringMatch{
{MatchType: &networking.StringMatch_Exact{Exact: "exact"}},
{MatchType: &networking.StringMatch_Prefix{Prefix: "prefix"}},
{MatchType: &networking.StringMatch_Regex{Regex: "regex"}},
},
UnmatchedPreflights: networking.CorsPolicy_IGNORE,
}
expectedCorsPolicy := &cors.CorsPolicy{
ForwardNotMatchingPreflights: wrapperspb.Bool(false),
AllowOriginStringMatch: []*matcher.StringMatcher{
{MatchPattern: &matcher.StringMatcher_Exact{Exact: "exact"}},
{MatchPattern: &matcher.StringMatcher_Prefix{Prefix: "prefix"}},
{
MatchPattern: &matcher.StringMatcher_SafeRegex{
SafeRegex: &matcher.RegexMatcher{
Regex: "regex",
},
},
},
},
FilterEnabled: &core.RuntimeFractionalPercent{
DefaultValue: &xdstype.FractionalPercent{
Numerator: 100,
Denominator: xdstype.FractionalPercent_HUNDRED,
},
},
}
if got := TranslateCORSPolicy(node, corsPolicy); !reflect.DeepEqual(got, expectedCorsPolicy) {
t.Errorf("TranslateCORSPolicy() = \n%v, want \n%v", got, expectedCorsPolicy)
}
}

func TestTranslateCORSPolicy(t *testing.T) {
node := &model.Proxy{
IstioVersion: &model.IstioVersion{
Major: 1,
Minor: 21,
Patch: 0,
},
}
corsPolicy := &networking.CorsPolicy{
AllowOrigins: []*networking.StringMatch{
{MatchType: &networking.StringMatch_Exact{Exact: "exact"}},
Expand All @@ -190,7 +239,7 @@ func TestTranslateCORSPolicy(t *testing.T) {
},
},
}
if got := TranslateCORSPolicy(corsPolicy); !reflect.DeepEqual(got, expectedCorsPolicy) {
if got := TranslateCORSPolicy(node, corsPolicy); !reflect.DeepEqual(got, expectedCorsPolicy) {
t.Errorf("TranslateCORSPolicy() = \n%v, want \n%v", got, expectedCorsPolicy)
}
}
Expand Down
6 changes: 6 additions & 0 deletions releasenotes/notes/50452.yaml
@@ -0,0 +1,6 @@
apiVersion: release-notes/v2
kind: bug-fix
area: traffic-management
releaseNotes:
- |
**Fixed** an issue that CORS filter forwarded preflight request if the origin is not allowed.