Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions api/v2/types_firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ type FirewallSpec struct {
// EgressRules contains egress rules configured for this firewall.
EgressRules []EgressRuleSNAT `json:"egressRules,omitempty"`

// InitialRuleSet is the initial firewall ruleset applied before the firewall-controller starts running.
InitialRuleSet *InitialRuleSet `json:"initialRuleSet,omitempty"`

// Interval on which rule reconciliation by the firewall-controller should happen.
Interval string `json:"interval,omitempty"`
// DryRun if set to true, firewall rules are not applied. For devel-purposes only.
Expand Down Expand Up @@ -122,6 +125,46 @@ type FirewallTemplateSpec struct {
Spec FirewallSpec `json:"spec,omitempty"`
}

// InitialRuleSet is the initial rule set deployed on the firewall.
type InitialRuleSet struct {
// Egress rules to be deployed initially on the firewall.
Egress []EgressRule `json:"egress,omitempty"`
// Ingress rules to be deployed initially on the firewall.
Ingress []IngressRule `json:"ingress,omitempty"`
}

// NetworkProtocol represents the kind of network protocol.
type NetworkProtocol string

const (
// NetworkProtocolTCP represents tcp connections.
NetworkProtocolTCP = "TCP"
// NetworkProtocolUDP represents udp connections.
NetworkProtocolUDP = "UDP"
)

type EgressRule struct {
// Comment provides a human readable description of this rule.
Comment string `json:"comment,omitempty"`
// Ports contains all affected network ports.
Ports []int32 `json:"ports"`
// Protocol constraints the protocol this rule applies to.
Protocol NetworkProtocol `json:"protocol"`
// To source address cidrs this rule applies to.
To []string `json:"to"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not have to be 100% complete (this is done by metal-api anyway) but maybe some very basic validation would be nice to prevent misconfiguration. The validations can be found api/v2/validation/firewall.go.

For me it would be sufficient to add:

  • Valid protocol
  • Valid CIDRs
  • Valid port range

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the metal-api and added some basic checks. Is that what you were thinking about? Could be slightly refactored to remove duplication.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that was what I was thinking about. Thanks for adding these validations. :)
Please apply your refactorings and I'll give it another review.

}

type IngressRule struct {
// Comment provides a human readable description of this rule.
Comment string `json:"comment,omitempty"`
// Ports contains all affected network ports.
Ports []int32 `json:"ports"`
// Protocol constraints the protocol this rule applies to.
Protocol NetworkProtocol `json:"protocol"`
// From source address cidrs this rule applies to.
From []string `json:"from"`
}

// EgressRuleSNAT holds a Source-NAT rule
type EgressRuleSNAT struct {
// NetworkID is the network for which the egress rule will be configured.
Expand Down
57 changes: 57 additions & 0 deletions api/v2/validation/firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,25 @@ func (*firewallValidator) validateSpec(f *v2.FirewallSpec, fldPath *field.Path)
}
}

if f.InitialRuleSet != nil {
basePath := fldPath.Child("initialRuleSet")

for _, rule := range f.InitialRuleSet.Egress {
egressPath := basePath.Child("egress").Child("rule")

allErrs = append(allErrs, validateProtocol(rule.Protocol, egressPath.Child("protocol"))...)
allErrs = append(allErrs, validatePorts(rule.Ports, egressPath.Child("ports"))...)
allErrs = append(allErrs, validateCIDRs(rule.To, egressPath.Child("to"))...)
}
for _, rule := range f.InitialRuleSet.Ingress {
ingressPath := basePath.Child("ingress").Child("rule")

allErrs = append(allErrs, validateProtocol(rule.Protocol, ingressPath.Child("protocol"))...)
allErrs = append(allErrs, validatePorts(rule.Ports, ingressPath.Child("ports"))...)
allErrs = append(allErrs, validateCIDRs(rule.From, ingressPath.Child("from"))...)
}
}

return allErrs
}

Expand Down Expand Up @@ -162,3 +181,41 @@ func validateDistance(distance v2.FirewallDistance, fldPath *field.Path) field.E

return allErrs
}

func validateProtocol(protocol v2.NetworkProtocol, rulesPath *field.Path) field.ErrorList {
var allErrs field.ErrorList

if protocol != v2.NetworkProtocolTCP && protocol != v2.NetworkProtocolUDP {
allErrs = append(allErrs, field.Invalid(rulesPath, protocol, fmt.Sprintf("protocol not supported: %v", protocol)))
}

return allErrs
}

func validatePorts(ports []int32, rulesPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
const (
minPort = 0
maxPort = 65535
)

for _, port := range ports {
if port < minPort || port > maxPort {
allErrs = append(allErrs, field.Invalid(rulesPath, port, fmt.Sprintf("port is out of range: %v", port)))
}
}

return allErrs
}

func validateCIDRs(cidrs []string, rulesPath *field.Path) field.ErrorList {
var allErrs field.ErrorList

for _, cidr := range cidrs {
if _, err := netip.ParsePrefix(cidr); err != nil {
allErrs = append(allErrs, field.Invalid(rulesPath, cidr, fmt.Sprintf("invalid cidr: %v", err)))
}
}

return allErrs
}
58 changes: 58 additions & 0 deletions api/v2/validation/firewall_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,64 @@ func Test_firewallValidator_ValidateCreate(t *testing.T) {
},
},
},

{
name: "invalid egress protocol in initial rule set",
mutateFn: func(f *v2.Firewall) *v2.Firewall {
f.Spec.InitialRuleSet = &v2.InitialRuleSet{
Egress: []v2.EgressRule{
{
Protocol: v2.NetworkProtocol("foo"),
},
},
}
return f
},
wantErr: &apierrors.StatusError{
ErrStatus: metav1.Status{
Message: ` "firewall-123" is invalid: spec.initialRuleSet.egress.rule.protocol: Invalid value: "foo": protocol not supported: foo`,
},
},
},
{
name: "invalid egress port range in initial rule set",
mutateFn: func(f *v2.Firewall) *v2.Firewall {
f.Spec.InitialRuleSet = &v2.InitialRuleSet{
Egress: []v2.EgressRule{
{
Protocol: v2.NetworkProtocolTCP,
Ports: []int32{65536},
},
},
}
return f
},
wantErr: &apierrors.StatusError{
ErrStatus: metav1.Status{
Message: ` "firewall-123" is invalid: spec.initialRuleSet.egress.rule.ports: Invalid value: 65536: port is out of range: 65536`,
},
},
},
{
name: "invalid egress cidr in initial rule set",
mutateFn: func(f *v2.Firewall) *v2.Firewall {
f.Spec.InitialRuleSet = &v2.InitialRuleSet{
Egress: []v2.EgressRule{
{
Protocol: v2.NetworkProtocolTCP,
Ports: []int32{1234},
To: []string{"1.2.3.4", "3.4.5.6/32"},
},
},
}
return f
},
wantErr: &apierrors.StatusError{
ErrStatus: metav1.Status{
Message: ` "firewall-123" is invalid: spec.initialRuleSet.egress.rule.to: Invalid value: "1.2.3.4": invalid cidr: netip.ParsePrefix("1.2.3.4"): no '/'`,
},
},
},
}
for _, tt := range tests {
tt := tt
Expand Down
84 changes: 84 additions & 0 deletions api/v2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

69 changes: 69 additions & 0 deletions config/crds/firewall.metal-stack.io_firewalldeployments.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,75 @@ spec:
Image is the os image of the firewall.
An update on this field requires the recreation of the physical firewall and can therefore lead to traffic interruption for the cluster.
type: string
initialRuleSet:
description: InitialRuleSet is the initial firewall ruleset
applied before the firewall-controller starts running.
properties:
egress:
description: Egress rules to be deployed initially on
the firewall.
items:
properties:
comment:
description: Comment provides a human readable description
of this rule.
type: string
ports:
description: Ports contains all affected network
ports.
items:
format: int32
type: integer
type: array
protocol:
description: Protocol constraints the protocol this
rule applies to.
type: string
to:
description: To target addresses this rule applies
to. May contain IPs or dns names.
items:
type: string
type: array
required:
- ports
- protocol
- to
type: object
type: array
ingress:
description: Ingress rules to be deployed initially on
the firewall.
items:
properties:
comment:
description: Comment provides a human readable description
of this rule.
type: string
from:
description: From source addresses this rule applies
to. May contain IPs or dns names.
items:
type: string
type: array
ports:
description: Ports contains all affected network
ports.
items:
format: int32
type: integer
type: array
protocol:
description: Protocol constraints the protocol this
rule applies to.
type: string
required:
- from
- ports
- protocol
type: object
type: array
type: object
internalPrefixes:
description: |-
InternalPrefixes specify prefixes which are considered local to the partition or all regions. This is used for the traffic counters.
Expand Down
Loading