cors: do not forwarded preflight request if the origin is not allowed (…

…#50452)

* cors: do not forwarded to the upstream if the origin is not allowed

* check proxy version

* release-notes

* update

* implement UnmatchedPreflights

* nit

* nit

pilot/pkg/model/context.go

@@ -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

pilot/pkg/networking/core/listener_waypoint.go

@@ -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

pilot/pkg/networking/core/route/route.go

@@ -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 {
@@ -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)

pilot/pkg/networking/core/route/route_internal_test.go

@@ -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"
@@ -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"}},
@@ -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)
 	}
 }

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.