Skip to content

Commit

Permalink
policy: Fix Unspecified Protocol Type
Browse files Browse the repository at this point in the history
[ upstream commit 58b67dc ]

[ backporter's note: merge conflict in mergeIngress and mergeEgress due
  to the missing auth parameter to mergeIngressPortProto and
  mergeEgressPortProto. Resolved by using the upstream commit's version
  and removing the non-existent auth parameter in the calls.

According to the policy API documentation, if a
protocol is not specified in a PortProtocol type its
protocol is supposed to be presumed as "ANY". The
policy package inconsistently enforces this logic.
As a result, empty Protocol fields are validated as
"ANY", but, in the actual rule logic that splits
the "ANY" protocol into the supported protocol
types, the comparison was made only to the actual
"ANY" protocol constant. This is fixed with an
L4Proto method `IsAny` that does both the constant
and empty string comparison.

Signed-off-by: Nate Sweet <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
  • Loading branch information
nathanjsweet authored and dylandreimerink committed Oct 26, 2023
1 parent 49b39a0 commit 7b9b6d5
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 13 deletions.
7 changes: 6 additions & 1 deletion pkg/policy/api/l4.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ const (
PortProtocolAny = "0/ANY"
)

// IsAny returns true if an L4Proto represents ANY protocol
func (l4 L4Proto) IsAny() bool {
return l4 == ProtoAny || string(l4) == ""
}

// PortProtocol specifies an L4 port with an optional transport protocol
type PortProtocol struct {
// Port is an L4 port number. For now the string will be strictly
Expand Down Expand Up @@ -55,7 +60,7 @@ func (p PortProtocol) Covers(other PortProtocol) bool {
return false
}
if p.Protocol != other.Protocol {
return p.Protocol == "" || p.Protocol == ProtoAny
return p.Protocol.IsAny()
}
return true
}
Expand Down
24 changes: 12 additions & 12 deletions pkg/policy/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,20 +388,20 @@ func mergeIngress(policyCtx PolicyContext, ctx *SearchContext, fromEndpoints api
}

for _, p := range r.GetPortProtocols() {
if p.Protocol != api.ProtoAny {
cnt, err := mergeIngressPortProto(policyCtx, ctx, fromEndpoints, hostWildcardL7, r, p, p.Protocol, ruleLabels, resMap)
if p.Protocol.IsAny() {
cnt, err := mergeIngressPortProto(policyCtx, ctx, fromEndpoints, hostWildcardL7, r, p, api.ProtoTCP, ruleLabels, resMap)
if err != nil {
return err
}
found += cnt
} else {
cnt, err := mergeIngressPortProto(policyCtx, ctx, fromEndpoints, hostWildcardL7, r, p, api.ProtoTCP, ruleLabels, resMap)

cnt, err = mergeIngressPortProto(policyCtx, ctx, fromEndpoints, hostWildcardL7, r, p, api.ProtoUDP, ruleLabels, resMap)
if err != nil {
return err
}
found += cnt

cnt, err = mergeIngressPortProto(policyCtx, ctx, fromEndpoints, hostWildcardL7, r, p, api.ProtoUDP, ruleLabels, resMap)
} else {
cnt, err := mergeIngressPortProto(policyCtx, ctx, fromEndpoints, hostWildcardL7, r, p, p.Protocol, ruleLabels, resMap)
if err != nil {
return err
}
Expand Down Expand Up @@ -601,20 +601,20 @@ func mergeEgress(policyCtx PolicyContext, ctx *SearchContext, toEndpoints api.En
}

for _, p := range r.GetPortProtocols() {
if p.Protocol != api.ProtoAny {
cnt, err := mergeEgressPortProto(policyCtx, ctx, toEndpoints, r, p, p.Protocol, ruleLabels, resMap, fqdns)
if p.Protocol.IsAny() {
cnt, err := mergeEgressPortProto(policyCtx, ctx, toEndpoints, r, p, api.ProtoTCP, ruleLabels, resMap, fqdns)
if err != nil {
return err
}
found += cnt
} else {
cnt, err := mergeEgressPortProto(policyCtx, ctx, toEndpoints, r, p, api.ProtoTCP, ruleLabels, resMap, fqdns)

cnt, err = mergeEgressPortProto(policyCtx, ctx, toEndpoints, r, p, api.ProtoUDP, ruleLabels, resMap, fqdns)
if err != nil {
return err
}
found += cnt

cnt, err = mergeEgressPortProto(policyCtx, ctx, toEndpoints, r, p, api.ProtoUDP, ruleLabels, resMap, fqdns)
} else {
cnt, err := mergeEgressPortProto(policyCtx, ctx, toEndpoints, r, p, p.Protocol, ruleLabels, resMap, fqdns)
if err != nil {
return err
}
Expand Down

0 comments on commit 7b9b6d5

Please sign in to comment.