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

Introduce firewall egress and ingress rules for firewall allocation. #491

Merged
merged 28 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e693aa1
Introduce firewall egress and ingress rules for firewall allocation.
Gerrit91 Jan 22, 2024
e96760e
Fix tests.
Gerrit91 Jan 22, 2024
c54e748
Review comments.
Gerrit91 Jan 22, 2024
0cf1a12
Review comments.
Gerrit91 Jan 22, 2024
b28bca8
Merge master
majst01 Feb 5, 2024
48e169c
Merge master
majst01 Feb 5, 2024
a974798
Fill response
majst01 Feb 5, 2024
1d75046
Better naming
majst01 Feb 5, 2024
641a2a6
API refinement
majst01 Feb 6, 2024
11533de
Consistent api
majst01 Feb 6, 2024
e3d6526
Simplify integration tests
majst01 Feb 6, 2024
dd35380
Fix.
Gerrit91 Feb 6, 2024
46bbc65
Ensure rule comment is not dangerous
majst01 Feb 8, 2024
695f053
Merge branch 'firewall-rules' of https://github.com/metal-stack/metal…
majst01 Feb 8, 2024
62ebcbc
Merge branch 'master' into firewall-rules
majst01 Feb 8, 2024
d2fde04
Update deps
majst01 Feb 8, 2024
2c2afd1
Add validation test
majst01 Feb 8, 2024
105e52f
Merge branch 'master' into firewall-rules
majst01 Feb 12, 2024
5459307
Fix
majst01 Feb 12, 2024
53e8462
Fix protocol
majst01 Feb 12, 2024
8c2bdce
Use range over int
majst01 Feb 13, 2024
84a6e41
update masterdata
majst01 Feb 13, 2024
ff634e9
Merge branch 'master' of https://github.com/metal-stack/metal-api int…
majst01 Feb 13, 2024
f477814
Ingress Rule with toCidrs as well
majst01 Feb 13, 2024
ede5378
More validation
majst01 Feb 14, 2024
01c4d02
Renaming in the api
majst01 Feb 14, 2024
b7cda32
Merge branch 'master' into firewall-rules
majst01 Feb 14, 2024
90b3adc
Better tests
majst01 Feb 14, 2024
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
8 changes: 4 additions & 4 deletions .github/workflows/docker.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:

steps:
- name: Log in to the container registry
uses: docker/login-action@v2
uses: docker/login-action@v3
with:
registry: ${{ env.REGISTRY }}
username: ${{ secrets.DOCKER_REGISTRY_USER }}
Expand All @@ -32,7 +32,7 @@ jobs:
uses: actions/checkout@v3

- name: Setup Go
uses: actions/setup-go@v4
uses: actions/setup-go@v5
with:
go-version-file: 'go.mod'
cache: false
Expand Down Expand Up @@ -63,10 +63,10 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Setup Go
uses: actions/setup-go@v4
uses: actions/setup-go@v5
with:
go-version-file: 'go.mod'

Expand Down
4 changes: 2 additions & 2 deletions cmd/metal-api/internal/datastore/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ func BenchmarkElectMachine(b *testing.B) {
}
for _, t := range tests {
b.Run(t.name, func(b *testing.B) {
for i := 0; i < b.N; i++ {
for range b.N {
spreadAcrossRacks(t.args.allMachines, t.args.projectMachines, t.args.tags)
}
})
Expand All @@ -707,7 +707,7 @@ func getTestMachines(numPerRack int, rackids []string, tags []string) metal.Mach
machines := make(metal.Machines, 0)

for _, id := range rackids {
for i := 0; i < numPerRack; i++ {
for range numPerRack {
m := metal.Machine{
RackID: id,
Tags: tags,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func (t *test) run() {
}

ds, mock := datastore.InitMockDB(t.T)
for i := 0; i < t.numberMachineInstances; i++ {
for i := range t.numberMachineInstances {
machineID := strconv.Itoa(i)
mock.On(r.DB("mockdb").Table("machine").Get(machineID)).Return(metal.Machine{Base: metal.Base{ID: machineID}}, nil)
mock.On(insertMock(true, machineID)).Return(returnMock(true, machineID), nil)
Expand Down Expand Up @@ -214,7 +214,7 @@ func (t *test) stopMachineInstances() {
}

func (t *test) startApiInstances(ds *datastore.RethinkStore) {
for i := 0; i < t.numberApiInstances; i++ {
for i := range t.numberApiInstances {
ctx, cancel := context.WithCancel(context.Background())
allocate := make(chan string)

Expand Down Expand Up @@ -252,7 +252,7 @@ func (t *test) startMachineInstances() {
grpc.WithTransportCredentials(insecure.NewCredentials()),
grpc.WithBlock(),
}
for i := 0; i < t.numberMachineInstances; i++ {
for i := range t.numberMachineInstances {
machineID := strconv.Itoa(i)
port := 50005 + t.randNumber(t.numberApiInstances)
ctx, cancel := context.WithCancel(context.Background())
Expand Down Expand Up @@ -317,7 +317,7 @@ func (t *test) waitForAllocation(machineID string, c v1.BootServiceClient, ctx c

func (t *test) allocateMachines() {
var alreadyAllocated []string
for i := 0; i < t.numberAllocations; i++ {
for range t.numberAllocations {
machineID := t.selectMachine(alreadyAllocated)
alreadyAllocated = append(alreadyAllocated, machineID)
t.mtx.Lock()
Expand Down
138 changes: 138 additions & 0 deletions cmd/metal-api/internal/metal/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package metal

import (
"fmt"
"net/netip"
"os"
"slices"
"strings"
"time"

Expand Down Expand Up @@ -148,6 +150,142 @@ type MachineAllocation struct {
Role Role `rethinkdb:"role" json:"role"`
VPN *MachineVPN `rethinkdb:"vpn" json:"vpn"`
UUID string `rethinkdb:"uuid" json:"uuid"`
FirewallRules *FirewallRules `rethinkdb:"firewall_rules" json:"firewall_rules"`
}

type FirewallRules struct {
Egress []EgressRule `rethinkdb:"egress" json:"egress"`
Ingress []IngressRule `rethinkdb:"ingress" json:"ingress"`
}

type EgressRule struct {
Protocol Protocol `rethinkdb:"protocol" json:"protocol"`
Ports []int `rethinkdb:"ports" json:"ports"`
majst01 marked this conversation as resolved.
Show resolved Hide resolved
To []string `rethinkdb:"to" json:"to"`
Comment string `rethinkdb:"comment" json:"comment"`
}

type IngressRule struct {
Protocol Protocol `rethinkdb:"protocol" json:"protocol"`
Ports []int `rethinkdb:"ports" json:"ports"`
To []string `rethinkdb:"to" json:"to"`
From []string `rethinkdb:"from" json:"from"`
Comment string `rethinkdb:"comment" json:"comment"`
}

type Protocol string

const (
ProtocolTCP Protocol = "TCP"
ProtocolUDP Protocol = "UDP"
)

func ProtocolFromString(s string) (Protocol, error) {
switch strings.ToLower(s) {
case "tcp":
return ProtocolTCP, nil
case "udp":
return ProtocolUDP, nil
default:
return Protocol(""), fmt.Errorf("no such protocol: %s", s)
}
}

func (r EgressRule) Validate() error {
switch r.Protocol {
case ProtocolTCP, ProtocolUDP:
// ok
default:
return fmt.Errorf("invalid procotol: %s", r.Protocol)
}

if err := validateComment(r.Comment); err != nil {
return err
}
if err := validatePorts(r.Ports); err != nil {
return err
}

if err := validateCIDRs(r.To); err != nil {
return err
}

return nil
}

func (r IngressRule) Validate() error {
switch r.Protocol {
case ProtocolTCP, ProtocolUDP:
// ok
default:
return fmt.Errorf("invalid protocol: %s", r.Protocol)
}
if err := validateComment(r.Comment); err != nil {
return err
}

if err := validatePorts(r.Ports); err != nil {
return err
}
if err := validateCIDRs(r.To); err != nil {
return err
}
if err := validateCIDRs(r.From); err != nil {
return err
}
if err := validateCIDRs(slices.Concat(r.From, r.To)); err != nil {
return err
}

return nil
}

const (
allowedCharacters = "abcdefghijklmnopqrstuvwxyz_- "
maxCommentLength = 100
)

func validateComment(comment string) error {
for _, c := range comment {
majst01 marked this conversation as resolved.
Show resolved Hide resolved
if !strings.Contains(allowedCharacters, strings.ToLower(string(c))) {
return fmt.Errorf("illegal character in comment found, only: %q allowed", allowedCharacters)
}
}
if len(comment) > maxCommentLength {
return fmt.Errorf("comments can not exceed %d characters", maxCommentLength)
}
return nil
}

func validatePorts(ports []int) error {
for _, port := range ports {
if port < 0 || port > 65535 {
return fmt.Errorf("port is out of range")
}
}

return nil
}

func validateCIDRs(cidrs []string) error {
af := ""
for _, cidr := range cidrs {
p, err := netip.ParsePrefix(cidr)
if err != nil {
return fmt.Errorf("invalid cidr: %w", err)
}
var newaf string
if p.Addr().Is4() {
newaf = "ipv4"
} else {
newaf = "ipv6"
}
if af != "" && af != newaf {
return fmt.Errorf("mixed address family in one rule is not supported:%v", cidrs)
}
af = newaf
}
return nil
}

// A MachineSetup stores the data used for machine reinstallations.
Expand Down
149 changes: 149 additions & 0 deletions cmd/metal-api/internal/metal/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,152 @@ func TestMachineNetwork_NetworkType(t *testing.T) {
}

// TODO: Write tests for machine allocation

func TestEgressRule_Validate(t *testing.T) {
tests := []struct {
name string
Protocol Protocol
Ports []int
To []string
Comment string
wantErr bool
wantErrmsg string
}{
{
name: "valid egress rule",
Protocol: ProtocolTCP,
Ports: []int{1, 2, 3},
To: []string{"1.2.3.0/24", "2.3.4.5/32"},
Comment: "allow apt update",
},
{
name: "wrong protocol",
Protocol: Protocol("sctp"),
Ports: []int{1, 2, 3},
To: []string{"1.2.3.0/24", "2.3.4.5/32"},
Comment: "allow apt update",
wantErr: true,
wantErrmsg: "invalid procotol: sctp",
},
{
name: "wrong port",
Protocol: ProtocolTCP,
Ports: []int{1, 2, 3, -1},
To: []string{"1.2.3.0/24", "2.3.4.5/32"},
Comment: "allow apt update",
wantErr: true,
wantErrmsg: "port is out of range",
},
{
name: "wrong cidr",
Protocol: ProtocolTCP,
Ports: []int{1, 2, 3},
To: []string{"1.2.3.0/24", "2.3.4.5/33"},
Comment: "allow apt update",
wantErr: true,
wantErrmsg: "invalid cidr: netip.ParsePrefix(\"2.3.4.5/33\"): prefix length out of range",
},
{
name: "wrong comment",
Protocol: ProtocolTCP,
Ports: []int{1, 2, 3},
To: []string{"1.2.3.0/24", "2.3.4.5/32"},
Comment: "allow apt update\n",
wantErr: true,
wantErrmsg: "illegal character in comment found, only: \"abcdefghijklmnopqrstuvwxyz_- \" allowed",
},
{
name: "too long comment",
Protocol: ProtocolTCP,
Ports: []int{1, 2, 3},
To: []string{"1.2.3.0/24", "2.3.4.5/32"},
Comment: "much too long comment aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
wantErr: true,
wantErrmsg: "comments can not exceed 100 characters",
},
{
name: "mixed address family in cidrs",
Protocol: ProtocolTCP,
Ports: []int{1, 2, 3},
To: []string{"1.2.3.0/24", "2.3.4.5/32", "2001:db8::/32"},
Comment: "mixed address family",
wantErr: true,
wantErrmsg: "mixed address family in one rule is not supported:[1.2.3.0/24 2.3.4.5/32 2001:db8::/32]",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := EgressRule{
Protocol: tt.Protocol,
Ports: tt.Ports,
To: tt.To,
Comment: tt.Comment,
}
if err := r.Validate(); (err != nil) != tt.wantErr {
t.Errorf("EgressRule.Validate() error = %v, wantErr %v", err, tt.wantErr)
}
if err := r.Validate(); err != nil {
if tt.wantErrmsg != err.Error() {
t.Errorf("IngressRule.Validate() error = %v, wantErrmsg %v", err.Error(), tt.wantErrmsg)
}
}
})
}
}
func TestIngressRule_Validate(t *testing.T) {
tests := []struct {
name string
Protocol Protocol
Ports []int
To []string
From []string
Comment string
wantErr bool
wantErrmsg string
}{
{
name: "valid ingress rule",
Protocol: ProtocolTCP,
Ports: []int{1, 2, 3},
From: []string{"1.2.3.0/24", "2.3.4.5/32"},
Comment: "allow apt update",
},
{
name: "valid ingress rule",
Protocol: ProtocolTCP,
Ports: []int{1, 2, 3},
From: []string{"1.2.3.0/24", "2.3.4.5/32"},
To: []string{"100.2.3.0/24", "200.3.4.5/32"},
Comment: "allow apt update",
},
{
name: "invalid ingress rule, mixed address families in to and from",
Protocol: ProtocolTCP,
Ports: []int{1, 2, 3},
From: []string{"1.2.3.0/24", "2.3.4.5/32"},
To: []string{"100.2.3.0/24", "2001:db8::/32"},
Comment: "allow apt update",
wantErr: true,
wantErrmsg: "mixed address family in one rule is not supported:[100.2.3.0/24 2001:db8::/32]",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := IngressRule{
Protocol: tt.Protocol,
Ports: tt.Ports,
To: tt.To,
From: tt.From,
Comment: tt.Comment,
}
if err := r.Validate(); (err != nil) != tt.wantErr {
t.Errorf("IngressRule.Validate() error = %v, wantErr %v", err, tt.wantErr)
}
if err := r.Validate(); err != nil {
if tt.wantErrmsg != err.Error() {
t.Errorf("IngressRule.Validate() error = %v, wantErrmsg %v", err.Error(), tt.wantErrmsg)
}
}
})
}
}
Loading
Loading