Skip to content

feat: add simple Firewall design #114

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat: add simple Firewall design #114

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
155 changes: 153 additions & 2 deletions docs/proposed-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ spec:
- cidr: 10.1.1.0/24
as: # Means static Src/Dst NAT for vpc1
- cidr: 192.168.1.0/24
ingress:
firewall:
- allow:
stateless: true # it's the only options supported in the first release
tcp:
Expand All @@ -187,7 +187,7 @@ spec:
expose:
- ips:
- cidr: 10.2.1.1/32
ingress:
firewall:
- allow:
stateless: true
tcp:
Expand Down Expand Up @@ -288,3 +288,154 @@ spec:
- not: 192.168.0.0/16
- not: 3.2.1.0/30
```

## Simple firewall implementation

VPC Firewall implies zero trust if firewall pattern is specified.
Copy link
Member

Choose a reason for hiding this comment

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

Do I understand this right?:

  • No firewall policy specified: all traffic allowed
  • allow policy specified: all traffic denied, save for allowed patterns
  • deny-only policy specified: all traffic denied
  • allow + deny policies: traffic not matching allowed patterns is denied; traffic matching allowed patterns is allowed, except if it's explicitly denied by the deny policy, which always takes precedence

Is this correct?

Copy link
Author

Choose a reason for hiding this comment

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

Yes. If there is deny only policy specified, allow should be added for any other traffic.
Also, order is defined by order in the API message.

Copy link
Member

Choose a reason for hiding this comment

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

Yes. If there is deny only policy specified, allow should be added for any other traffic.

Sounds good to me, thanks!

Also, order is defined by order in the API message.

This is more concerning to me. It sounds super-easy to enable by mistake the traffic you specifically wanted to deny, because a user didn't understand the significance of the order, or they got confused and mentally swapped the order, or did a copy-paste error and put the deny policy at the wrong place. Are we sure we need to nest multiple levels of allow and deny policies? Wouldn't a single level of allow + a preceding level of deny be safer (and easier) to use?


```yaml
...
firewall:
- deny:
stateless: true

log: # log traffic event for given pattern
level: debug # warning | info | error
message: "custom log message"
tcp:
src:
cidrs:
- 10.0.0.0/8
- 172.16.0.0/12
ports: [80, 443]
portRanges:
- start: 8000
end: 8999
- start: 3000
end: 3999
dst:
cidrs:
- 192.168.1.0/24
ports: [22, 23]
portRanges:
- start: 1234
end: 2222
udp:
src:
cidrs:
- 10.0.0.0/8
ports: [53, 123]
portRanges:
- start: 5000
end: 5999
dst:
cidrs:
- 8.8.8.8/32
ports: [53]
icmp:
src:
cidrs:
- 10.0.0.0/8
dst:
cidrs:
- 192.168.1.0/24
# TODO: ICMP type/code options
protocol: 47 # Raw protocol number (optional, alternative to tcp/udp/icmp)
- allow:
stateless: true
# All same options as deny
```

### Example 1: VPC-1 To VPC-DB access limitation (no NAT)

```yaml
apiVersion: gateway.githedgehog.com/v1alpha1
kind: Peering
metadata:
name: vpc-1-to-vpc-db
spec:
peering:
vpc-1:
expose:
- ips:
- cidr: 10.1.0.0/24
firewall:
# Allow database responses back
- allow:
stateless: true
log:
level: info
message: "DB traffic hit"
tcp:
src:
ports: [3306, 5432, 1433] # MySQL, PostgreSQL, SQL Server
vpc-db:
expose:
- ips:
- cidr: 10.2.0.0/24
firewall:
# Allow legitimate database connections
- allow:
stateless: true
log:
level: info
message: "Database connection"
tcp:
dst:
ports: [3306, 5432, 1433]
```

### Example 2: VPC-1 access to VPC with k8s API (overlapping IPs, NAT)

```yaml
apiVersion: gateway.githedgehog.com/v1alpha1
kind: Peering
metadata:
name: vpc-1-to-k8s-with-nat
spec:
peering:
vpc-1:
expose:
- ips:
- cidr: 10.1.10.0/24
as:
- cidr: 192.168.10.0/24
firewall:
# Allow K8s API responses back to vpc-1 clients
- allow:
stateless: true
log:
level: debug
message: "Kubernetes API response to vpc-1"
tcp:
src:
ports: [6443]
dst:
cidrs:
- 10.1.10.0/24
- deny:
stateless: true
log:
level: error
message: "vpc-k8s tries to access vpc-1"
vpc-kubernetes:
expose:
- ips:
- cidr: 10.1.10.0/24 # Overlap with vpc-1
as:
- cidr: 192.168.100.0/24 # NAT to different range for vpc-1 to see
firewall:
- allow:
stateless: true
log:
level: info
message: "vpc-1 Kubernetes API access"
tcp:
src:
cidrs:
- 192.168.10.0/24 # From NAT'd vpc-1 subnet
dst:
cidrs:
- 10.1.10.0/24 # To K8s control plane subnet
ports: [6443] # K8s API server only
```