diff --git a/spinifex/services/vpcd/mock_ovn.go b/spinifex/services/vpcd/mock_ovn.go index 24cbb1ca1..01bf67096 100644 --- a/spinifex/services/vpcd/mock_ovn.go +++ b/spinifex/services/vpcd/mock_ovn.go @@ -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) diff --git a/spinifex/services/vpcd/nbdb/model.go b/spinifex/services/vpcd/nbdb/model.go index a98eade4d..811df1ef3 100644 --- a/spinifex/services/vpcd/nbdb/model.go +++ b/spinifex/services/vpcd/nbdb/model.go @@ -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"` } diff --git a/spinifex/services/vpcd/ovn.go b/spinifex/services/vpcd/ovn.go index 18332845e..a8ada215f 100644 --- a/spinifex/services/vpcd/ovn.go +++ b/spinifex/services/vpcd/ovn.go @@ -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") } diff --git a/spinifex/services/vpcd/security.go b/spinifex/services/vpcd/security.go index a8cd05ca2..c19f74e2e 100644 --- a/spinifex/services/vpcd/security.go +++ b/spinifex/services/vpcd/security.go @@ -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, + } +} diff --git a/spinifex/services/vpcd/security_test.go b/spinifex/services/vpcd/security_test.go index 3417782c5..a953c33d0 100644 --- a/spinifex/services/vpcd/security_test.go +++ b/spinifex/services/vpcd/security_test.go @@ -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 } }