gsadmin/spinifex
1
0
Fork 0
mirror of https://github.com/mulgadc/spinifex.git synced 2026-10-15 17:11:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(vpcd): log=true on default-deny ACLs (#145)

Browse Source
This commit is contained in:
Julian Sommer
2026-04-16 14:45:21 +10:00
committed by GitHub
parent 3986549783
commit 6f02f6bc22
5 changed files with 134 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spinifex/services/vpcd/mock_ovn.go
+14 -5
View File
@@ -532,7 +532,7 @@ func (m *MockOVNClient) SetPortGroupPorts(_ context.Context, name string, ports
// ACLs
func (m *MockOVNClient) AddACL(_ context.Context, portGroupName string, direction string, priority int, match string, action string) error {
func (m *MockOVNClient) AddACL(_ context.Context, portGroupName string, spec ACLSpec) error {
m.mu.Lock()
defer m.mu.Unlock()
pg, exists := m.portGroups[portGroupName]
@@ -541,10 +541,19 @@ func (m *MockOVNClient) AddACL(_ context.Context, portGroupName string, directio
}
acl := &nbdb.ACL{
UUID: utils.GenerateResourceID("acl"),
Direction: direction,
Priority: priority,
Match: match,
Action: action,
Direction: spec.Direction,
Priority: spec.Priority,
Match: spec.Match,
Action: spec.Action,
Log: spec.Log,
}
if spec.Name != "" {
name := spec.Name
acl.Name = &name
}
if spec.Severity != "" {
severity := spec.Severity
acl.Severity = &severity
}
m.acls[acl.UUID] = acl
pg.ACLs = append(pg.ACLs, acl.UUID)
spinifex/services/vpcd/nbdb/model.go
+4
View File
@@ -104,10 +104,14 @@ type PortGroup struct {
// ACL represents an OVN ACL rule attached to a port group or logical switch.
type ACL struct {
UUID string `ovsdb:"_uuid"`
Name *string `ovsdb:"name"`
Direction string `ovsdb:"direction"` // "to-lport" or "from-lport"
Priority int `ovsdb:"priority"`
Match string `ovsdb:"match"`
Action string `ovsdb:"action"` // "allow-related", "drop", "allow", "reject"
Log bool `ovsdb:"log"`
Severity *string `ovsdb:"severity"` // "alert", "warning", "notice", "info", "debug"
Meter *string `ovsdb:"meter"`
ExternalIDs map[string]string `ovsdb:"external_ids"`
}
spinifex/services/vpcd/ovn.go
+14 -2
View File
@@ -34,6 +34,18 @@ func (c *LiveOVNClient) transactOps(ctx context.Context, ops []ovsdb.Operation)
return err
}
// ACLSpec describes an OVN ACL rule for attachment to a port group.
// Name, Severity, and Meter are optional — set Severity when Log is true.
type ACLSpec struct {
Direction string // "to-lport" or "from-lport"
Priority int
Match string
Action string // "allow-related", "drop", "allow", "reject"
Name string
Log bool
Severity string // "alert", "warning", "notice", "info", "debug"
}
// OVNClient defines the interface for interacting with the OVN Northbound Database.
// This abstraction allows for mock implementations in tests.
type OVNClient interface {
@@ -88,7 +100,7 @@ type OVNClient interface {
SetPortGroupPorts(ctx context.Context, name string, ports []string) error
// ACLs (attached to port groups)
AddACL(ctx context.Context, portGroupName string, direction string, priority int, match string, action string) error
AddACL(ctx context.Context, portGroupName string, spec ACLSpec) error
ClearACLs(ctx context.Context, portGroupName string) error
// Gateway Chassis (HA scheduling for gateway router ports)
@@ -821,7 +833,7 @@ func (c *LiveOVNClient) SetPortGroupPorts(_ context.Context, _ string, _ []strin
return fmt.Errorf("SetPortGroupPorts: not yet implemented for live OVN client")
}
func (c *LiveOVNClient) AddACL(_ context.Context, _ string, _ string, _ int, _ string, _ string) error {
func (c *LiveOVNClient) AddACL(_ context.Context, _ string, _ ACLSpec) error {
return fmt.Errorf("AddACL: not yet implemented for live OVN client")
}
spinifex/services/vpcd/security.go
+47 -8
View File
@@ -16,6 +16,12 @@ const (
TopicUpdateSG = "vpc.update-sg"
)
// denyACLSeverity is the syslog severity OVN uses when a packet matches a
// default-deny ACL. "info" is loud enough to be captured by a syslog forwarder
// yet quiet enough to avoid paging on every dropped scan packet. Operators can
// promote it at their log collector if they want higher-priority alerts.
const denyACLSeverity = "info"
// SGEvent carries security group state from the handler to vpcd.
type SGEvent struct {
GroupId string `json:"group_id"`
@@ -48,11 +54,13 @@ func (h *TopologyHandler) handleCreateSG(msg *nats.Msg) {
return
}
// Add default deny ACL (priority 900) — drop all traffic not explicitly allowed
if err := h.ovn.AddACL(ctx, pgName, "to-lport", 900, fmt.Sprintf("outport == @%s && ip4", pgName), "drop"); err != nil {
// Add default deny ACLs (priority 900) — drop all traffic not explicitly
// allowed. Logging is enabled so boundary communications are observable via
// syslog (CMMC SC.L1-3.13.1).
if err := h.ovn.AddACL(ctx, pgName, denyIngressACL(pgName)); err != nil {
slog.Warn("vpcd: failed to add default deny ingress ACL", "pg", pgName, "err", err)
}
if err := h.ovn.AddACL(ctx, pgName, "from-lport", 900, fmt.Sprintf("inport == @%s && ip4", pgName), "drop"); err != nil {
if err := h.ovn.AddACL(ctx, pgName, denyEgressACL(pgName)); err != nil {
slog.Warn("vpcd: failed to add default deny egress ACL", "pg", pgName, "err", err)
}
@@ -127,11 +135,11 @@ func (h *TopologyHandler) handleUpdateSG(msg *nats.Msg) {
slog.Warn("vpcd: failed to clear ACLs for update", "pg", pgName, "err", err)
}
// Re-add default deny ACLs (priority 900)
if err := h.ovn.AddACL(ctx, pgName, "to-lport", 900, fmt.Sprintf("outport == @%s && ip4", pgName), "drop"); err != nil {
// Re-add default deny ACLs (priority 900) with logging enabled.
if err := h.ovn.AddACL(ctx, pgName, denyIngressACL(pgName)); err != nil {
slog.Warn("vpcd: failed to re-add default deny ingress ACL", "pg", pgName, "err", err)
}
if err := h.ovn.AddACL(ctx, pgName, "from-lport", 900, fmt.Sprintf("inport == @%s && ip4", pgName), "drop"); err != nil {
if err := h.ovn.AddACL(ctx, pgName, denyEgressACL(pgName)); err != nil {
slog.Warn("vpcd: failed to re-add default deny egress ACL", "pg", pgName, "err", err)
}
@@ -150,18 +158,49 @@ func (h *TopologyHandler) handleUpdateSG(msg *nats.Msg) {
// addRuleACLs adds OVN ACLs for a set of ingress and egress rules at priority 1000
// (higher than the default deny at 900, so allow rules take precedence).
// Allow rules are not logged — accept logging on a private network is
// high-volume and low-signal. Only denies carry Log=true.
func (h *TopologyHandler) addRuleACLs(ctx context.Context, pgName string, ingress, egress []SGRuleForACL) {
for _, rule := range ingress {
match := BuildIngressACLMatch(pgName, rule)
if err := h.ovn.AddACL(ctx, pgName, "to-lport", 1000, match, "allow-related"); err != nil {
spec := ACLSpec{Direction: "to-lport", Priority: 1000, Match: match, Action: "allow-related"}
if err := h.ovn.AddACL(ctx, pgName, spec); err != nil {
slog.Warn("vpcd: failed to add ingress ACL", "pg", pgName, "match", match, "err", err)
}
}
for _, rule := range egress {
match := BuildEgressACLMatch(pgName, rule)
if err := h.ovn.AddACL(ctx, pgName, "from-lport", 1000, match, "allow-related"); err != nil {
spec := ACLSpec{Direction: "from-lport", Priority: 1000, Match: match, Action: "allow-related"}
if err := h.ovn.AddACL(ctx, pgName, spec); err != nil {
slog.Warn("vpcd: failed to add egress ACL", "pg", pgName, "match", match, "err", err)
}
}
}
// denyIngressACL builds the default-deny ingress ACL for a port group with
// logging enabled (CMMC SC.L1-3.13.1 boundary monitoring).
func denyIngressACL(pgName string) ACLSpec {
return ACLSpec{
Direction: "to-lport",
Priority: 900,
Match: fmt.Sprintf("outport == @%s && ip4", pgName),
Action: "drop",
Name: pgName + "-deny-ingress",
Log: true,
Severity: denyACLSeverity,
}
}
// denyEgressACL is the egress counterpart to denyIngressACL.
func denyEgressACL(pgName string) ACLSpec {
return ACLSpec{
Direction: "from-lport",
Priority: 900,
Match: fmt.Sprintf("inport == @%s && ip4", pgName),
Action: "drop",
Name: pgName + "-deny-egress",
Log: true,
Severity: denyACLSeverity,
}
}
spinifex/services/vpcd/security_test.go
+55 -10
View File
@@ -40,10 +40,44 @@ func TestSecurity_CreatePortGroup(t *testing.T) {
assert.NotNil(t, pg)
// Verify default deny ACLs were created (2 deny ACLs at priority 900)
// and that each has logging enabled per CMMC SC.L1-3.13.1.
type aclSnapshot struct {
action string
log bool
severity string
name string
}
var snapshots []aclSnapshot
mock.mu.Lock()
aclCount := len(pg.ACLs)
for _, aclUUID := range pg.ACLs {
a := mock.acls[aclUUID]
if a == nil {
continue
}
snap := aclSnapshot{action: a.Action, log: a.Log}
if a.Severity != nil {
snap.severity = *a.Severity
}
if a.Name != nil {
snap.name = *a.Name
}
snapshots = append(snapshots, snap)
}
mock.mu.Unlock()
assert.Equal(t, 2, aclCount, "should have 2 default deny ACLs (ingress + egress)")
denyCount := 0
for _, s := range snapshots {
if s.action != "drop" {
continue
}
denyCount++
assert.True(t, s.log, "default deny ACL must have log=true for boundary monitoring")
assert.Equal(t, "info", s.severity, "default deny ACL must use info severity")
assert.Contains(t, s.name, "sg_abc123-deny-", "default deny ACL must have a name for syslog correlation")
}
assert.Equal(t, 2, denyCount, "should have 2 drop ACLs")
}
func TestSecurity_DeletePortGroup(t *testing.T) {
@@ -123,30 +157,41 @@ func TestSecurity_UpdateSGAddRules(t *testing.T) {
assertSuccess(t, resp, "update SG with ingress rules")
// Verify ACLs were created: 2 default deny + 2 ingress allow = 4
type aclSnapshot struct {
action string
match string
log bool
}
var snapshots []aclSnapshot
mock.mu.Lock()
pg := mock.portGroups["sg_upd1"]
aclCount := len(pg.ACLs)
// Collect match strings from the ACLs
var matches []string
for _, aclUUID := range pg.ACLs {
acl := mock.acls[aclUUID]
if acl != nil {
matches = append(matches, acl.Match)
a := mock.acls[aclUUID]
if a == nil {
continue
}
snapshots = append(snapshots, aclSnapshot{action: a.Action, match: a.Match, log: a.Log})
}
mock.mu.Unlock()
assert.Equal(t, 4, aclCount, "should have 4 ACLs (2 deny + 2 ingress allow)")
// Check that at least one match contains tcp.dst == 22
// Check that at least one match contains tcp.dst == 22. Also verify
// logging policy: denies logged, allows not logged (CMMC SC.L1-3.13.1).
foundSSH := false
foundHTTPS := false
for _, m := range matches {
if containsAll(m, "tcp.dst == 22", "ip4.src == 10.0.0.0/8") {
for _, s := range snapshots {
switch s.action {
case "drop":
assert.True(t, s.log, "deny ACL must be logged")
case "allow-related":
assert.False(t, s.log, "allow ACL must not be logged (high volume, low signal)")
}
if containsAll(s.match, "tcp.dst == 22", "ip4.src == 10.0.0.0/8") {
foundSSH = true
}
if containsAll(m, "tcp.dst == 443") {
if containsAll(s.match, "tcp.dst == 443") {
foundHTTPS = true
}
}