Skip to content

Commit

Permalink
fix: validate proto messages before converting them to anypb.Any (env…
Browse files Browse the repository at this point in the history
…oyproxy#4499)

* validate proto message before converting to any

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
(cherry picked from commit 05817fc)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
  • Loading branch information
zhaohuabing committed Oct 29, 2024
1 parent 7391c1c commit aa1e618
Show file tree
Hide file tree
Showing 24 changed files with 173 additions and 116 deletions.
36 changes: 18 additions & 18 deletions internal/utils/protocov/protocov.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,30 +12,30 @@ import (
"google.golang.org/protobuf/types/known/anypb"
)

const (
APIPrefix = "type.googleapis.com/"
)

var marshalOpts = proto.MarshalOptions{}
// Deprecated: error should not be ignored, use ToAnyWithValidation instead.
func ToAny(msg proto.Message) *anypb.Any {
res, err := ToAnyWithValidation(msg)
if err != nil {
return nil
}
return res
}

func ToAnyWithError(msg proto.Message) (*anypb.Any, error) {
func ToAnyWithValidation(msg proto.Message) (*anypb.Any, error) {
if msg == nil {
return nil, errors.New("empty message received")
}
b, err := marshalOpts.Marshal(msg)
if err != nil {
return nil, err

// If the message has a ValidateAll method, call it before marshaling.
if validator, ok := msg.(interface{ ValidateAll() error }); ok {
if err := validator.ValidateAll(); err != nil {
return nil, err
}
}
return &anypb.Any{
TypeUrl: APIPrefix + string(msg.ProtoReflect().Descriptor().FullName()),
Value: b,
}, nil
}

func ToAny(msg proto.Message) *anypb.Any {
res, err := ToAnyWithError(msg)
any, err := anypb.New(msg)
if err != nil {
return nil
return nil, err
}
return res
return any, nil
}
34 changes: 24 additions & 10 deletions internal/xds/translator/accesslog.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/envoyproxy/go-control-plane/pkg/wellknown"
otlpcommonv1 "go.opentelemetry.io/proto/otlp/common/v1"
"golang.org/x/exp/maps"
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/structpb"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
Expand Down Expand Up @@ -90,9 +89,10 @@ var (
}
)

func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLog {

func buildXdsAccessLog(al *ir.AccessLog, forListener bool) ([]*accesslog.AccessLog, error) {
if al == nil {
return nil
return nil, nil
}

totalLen := len(al.Text) + len(al.JSON) + len(al.OpenTelemetry)
Expand Down Expand Up @@ -124,8 +124,10 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo
filelog.GetLogFormat().Formatters = formatters
}

// TODO: find a better way to handle this
accesslogAny, _ := anypb.New(filelog)
accesslogAny, err := protocov.ToAnyWithValidation(filelog)
if err != nil {
return nil, err
}
accessLogs = append(accessLogs, &accesslog.AccessLog{
Name: wellknown.FileAccessLog,
ConfigType: &accesslog.AccessLog_TypedConfig{
Expand Down Expand Up @@ -168,7 +170,10 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo
filelog.GetLogFormat().Formatters = formatters
}

accesslogAny, _ := anypb.New(filelog)
accesslogAny, err := protocov.ToAnyWithValidation(filelog)
if err != nil {
return nil, err
}
accessLogs = append(accessLogs, &accesslog.AccessLog{
Name: wellknown.FileAccessLog,
ConfigType: &accesslog.AccessLog_TypedConfig{
Expand Down Expand Up @@ -203,7 +208,10 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo
alCfg.AdditionalResponseTrailersToLog = als.HTTP.ResponseTrailers
}

accesslogAny, _ := anypb.New(alCfg)
accesslogAny, err := protocov.ToAnyWithValidation(alCfg)
if err != nil {
return nil, err
}
accessLogs = append(accessLogs, &accesslog.AccessLog{
Name: wellknown.HTTPGRPCAccessLog,
ConfigType: &accesslog.AccessLog_TypedConfig{
Expand All @@ -216,7 +224,10 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo
CommonConfig: cc,
}

accesslogAny, _ := anypb.New(alCfg)
accesslogAny, err := protocov.ToAnyWithValidation(alCfg)
if err != nil {
return nil, err
}
accessLogs = append(accessLogs, &accesslog.AccessLog{
Name: tcpGRPCAccessLog,
ConfigType: &accesslog.AccessLog_TypedConfig{
Expand Down Expand Up @@ -264,7 +275,10 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo
al.Formatters = formatters
}

accesslogAny, _ := anypb.New(al)
accesslogAny, err := protocov.ToAnyWithValidation(al)
if err != nil {
return nil, err
}
accessLogs = append(accessLogs, &accesslog.AccessLog{
Name: otelAccessLog,
ConfigType: &accesslog.AccessLog_TypedConfig{
Expand All @@ -274,7 +288,7 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo
})
}

return accessLogs
return accessLogs, nil
}

func celAccessLogFilter(expr string) *accesslog.AccessLogFilter {
Expand Down
18 changes: 7 additions & 11 deletions internal/xds/translator/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/utils/protocov"
"github.com/envoyproxy/gateway/internal/xds/types"
)

Expand Down Expand Up @@ -72,7 +73,7 @@ func (*rbac) patchHCM(
// buildHCMRBACFilter returns a RBAC filter from the provided IR listener.
func buildHCMRBACFilter() (*hcmv3.HttpFilter, error) {
rbacProto := &rbacv3.RBAC{}
rbacAny, err := anypb.New(rbacProto)
rbacAny, err := protocov.ToAnyWithValidation(rbacProto)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -134,15 +135,15 @@ func (*rbac) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error {
Name: "ALLOW",
Action: rbacconfigv3.RBAC_ALLOW,
}
if allowAction, err = anypb.New(allow); err != nil {
if allowAction, err = protocov.ToAnyWithValidation(allow); err != nil {
return err
}

deny := &rbacconfigv3.Action{
Name: "DENY",
Action: rbacconfigv3.RBAC_DENY,
}
if denyAction, err = anypb.New(deny); err != nil {
if denyAction, err = protocov.ToAnyWithValidation(deny); err != nil {
return err
}

Expand All @@ -166,11 +167,11 @@ func (*rbac) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error {
})
}

if ipMatcher, err = anypb.New(ipRangeMatcher); err != nil {
if ipMatcher, err = protocov.ToAnyWithValidation(ipRangeMatcher); err != nil {
return err
}

if sourceIPInput, err = anypb.New(&networkinput.SourceIPInput{}); err != nil {
if sourceIPInput, err = protocov.ToAnyWithValidation(&networkinput.SourceIPInput{}); err != nil {
return err
}

Expand Down Expand Up @@ -243,12 +244,7 @@ func (*rbac) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error {
routeCfgProto.Rbac.Matcher.MatcherType = nil
}

// We need to validate the RBACPerRoute message before converting it to an Any.
if err = routeCfgProto.ValidateAll(); err != nil {
return err
}

routeCfgAny, err := anypb.New(routeCfgProto)
routeCfgAny, err := protocov.ToAnyWithValidation(routeCfgProto)
if err != nil {
return err
}
Expand Down
5 changes: 3 additions & 2 deletions internal/xds/translator/basicauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"google.golang.org/protobuf/types/known/anypb"

"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/utils/protocov"
"github.com/envoyproxy/gateway/internal/xds/types"
)

Expand Down Expand Up @@ -87,7 +88,7 @@ func buildHCMBasicAuthFilter(basicAuth *ir.BasicAuth) (*hcmv3.HttpFilter, error)
if err = basicAuthProto.ValidateAll(); err != nil {
return nil, err
}
if basicAuthAny, err = anypb.New(basicAuthProto); err != nil {
if basicAuthAny, err = protocov.ToAnyWithValidation(basicAuthProto); err != nil {
return nil, err
}

Expand Down Expand Up @@ -137,7 +138,7 @@ func (*basicAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error
return err
}

if basicAuthAny, err = anypb.New(basicAuthProto); err != nil {
if basicAuthAny, err = protocov.ToAnyWithValidation(basicAuthProto); err != nil {
return err
}

Expand Down
9 changes: 5 additions & 4 deletions internal/xds/translator/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"k8s.io/utils/ptr"

"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/utils/protocov"
)

const (
Expand Down Expand Up @@ -478,7 +479,7 @@ func buildTypedExtensionProtocolOptions(args *xdsClusterArgs) map[string]*anypb.
if args.http1Settings != nil {
http1opts.EnableTrailers = args.http1Settings.EnableTrailers
if args.http1Settings.PreserveHeaderCase {
preservecaseAny, _ := anypb.New(&preservecasev3.PreserveCaseFormatterConfig{})
preservecaseAny, _ := protocov.ToAnyWithValidation(&preservecasev3.PreserveCaseFormatterConfig{})
http1opts.HeaderKeyFormat = &corev3.Http1ProtocolOptions_HeaderKeyFormat{
HeaderFormat: &corev3.Http1ProtocolOptions_HeaderKeyFormat_StatefulFormatter{
StatefulFormatter: &corev3.TypedExtensionConfig{
Expand Down Expand Up @@ -529,7 +530,7 @@ func buildTypedExtensionProtocolOptions(args *xdsClusterArgs) map[string]*anypb.
}
}

anyProtocolOptions, _ := anypb.New(&protocolOptions)
anyProtocolOptions, _ := protocov.ToAnyWithValidation(&protocolOptions)

extensionOptions := map[string]*anypb.Any{
extensionOptionsKey: anyProtocolOptions,
Expand Down Expand Up @@ -560,7 +561,7 @@ func buildProxyProtocolSocket(proxyProtocol *ir.ProxyProtocol, tSocket *corev3.T
// If existing transport socket does not exist wrap around raw buffer
if tSocket == nil {
rawCtx := &rawbufferv3.RawBuffer{}
rawCtxAny, err := anypb.New(rawCtx)
rawCtxAny, err := protocov.ToAnyWithValidation(rawCtx)
if err != nil {
return nil
}
Expand All @@ -575,7 +576,7 @@ func buildProxyProtocolSocket(proxyProtocol *ir.ProxyProtocol, tSocket *corev3.T
ppCtx.TransportSocket = tSocket
}

ppCtxAny, err := anypb.New(ppCtx)
ppCtxAny, err := protocov.ToAnyWithValidation(ppCtx)
if err != nil {
return nil
}
Expand Down
5 changes: 3 additions & 2 deletions internal/xds/translator/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"google.golang.org/protobuf/types/known/wrapperspb"

"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/utils/protocov"
"github.com/envoyproxy/gateway/internal/xds/types"
)

Expand Down Expand Up @@ -70,7 +71,7 @@ func (*cors) patchHCM(
func buildHCMCORSFilter() (*hcmv3.HttpFilter, error) {
corsProto := &corsv3.Cors{}

corsAny, err := anypb.New(corsProto)
corsAny, err := protocov.ToAnyWithValidation(corsProto)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -152,7 +153,7 @@ func (*cors) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error {
ForwardNotMatchingPreflights: &wrapperspb.BoolValue{Value: false},
}

routeCfgAny, err := anypb.New(routeCfgProto)
routeCfgAny, err := protocov.ToAnyWithValidation(routeCfgProto)
if err != nil {
return err
}
Expand Down
7 changes: 2 additions & 5 deletions internal/xds/translator/extauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import (
extauthv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_authz/v3"
hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
matcherv3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/durationpb"

"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/utils/protocov"
"github.com/envoyproxy/gateway/internal/xds/types"
)

Expand Down Expand Up @@ -75,11 +75,8 @@ func (*extAuth) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPLi
// buildHCMExtAuthFilter returns an ext_authz HTTP filter from the provided IR HTTPRoute.
func buildHCMExtAuthFilter(extAuth *ir.ExtAuth) (*hcmv3.HttpFilter, error) {
extAuthProto := extAuthConfig(extAuth)
if err := extAuthProto.ValidateAll(); err != nil {
return nil, err
}

extAuthAny, err := anypb.New(extAuthProto)
extAuthAny, err := protocov.ToAnyWithValidation(extAuthProto)
if err != nil {
return nil, err
}
Expand Down
7 changes: 2 additions & 5 deletions internal/xds/translator/extproc.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import (
routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
extprocv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_proc/v3"
hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/durationpb"

"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/utils/protocov"
"github.com/envoyproxy/gateway/internal/xds/types"
)

Expand Down Expand Up @@ -72,11 +72,8 @@ func (*extProc) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPLi
// buildHCMExtProcFilter returns an ext_proc HTTP filter from the provided IR HTTPRoute.
func buildHCMExtProcFilter(extProc ir.ExtProc) (*hcmv3.HttpFilter, error) {
extAuthProto := extProcConfig(extProc)
if err := extAuthProto.ValidateAll(); err != nil {
return nil, err
}

extAuthAny, err := anypb.New(extAuthProto)
extAuthAny, err := protocov.ToAnyWithValidation(extAuthProto)
if err != nil {
return nil, err
}
Expand Down
5 changes: 3 additions & 2 deletions internal/xds/translator/fault.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"google.golang.org/protobuf/types/known/durationpb"

"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/utils/protocov"
"github.com/envoyproxy/gateway/internal/xds/types"
)

Expand Down Expand Up @@ -74,7 +75,7 @@ func buildHCMFaultFilter() (*hcmv3.HttpFilter, error) {
return nil, err
}

faultAny, err := anypb.New(faultProto)
faultAny, err := protocov.ToAnyWithValidation(faultProto)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -168,7 +169,7 @@ func (*fault) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error {
return nil
}

routeCfgAny, err := anypb.New(routeCfgProto)
routeCfgAny, err := protocov.ToAnyWithValidation(routeCfgProto)
if err != nil {
return err
}
Expand Down
6 changes: 2 additions & 4 deletions internal/xds/translator/healthcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"google.golang.org/protobuf/types/known/wrapperspb"

"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/utils/protocov"
"github.com/envoyproxy/gateway/internal/xds/types"
)

Expand Down Expand Up @@ -82,10 +83,7 @@ func buildHealthCheckFilter(healthCheck *ir.HealthCheckSettings) (*hcmv3.HttpFil
}},
}

if err = healthCheckProto.ValidateAll(); err != nil {
return nil, err
}
if healthCheckAny, err = anypb.New(healthCheckProto); err != nil {
if healthCheckAny, err = protocov.ToAnyWithValidation(healthCheckProto); err != nil {
return nil, err
}

Expand Down
Loading

0 comments on commit aa1e618

Please sign in to comment.