Skip to content

Add enable-monit-access command to bosh-agent#406

Merged
mariash merged 12 commits intomainfrom
monit-access-uid-firewall
Feb 25, 2026
Merged

Add enable-monit-access command to bosh-agent#406
mariash merged 12 commits intomainfrom
monit-access-uid-firewall

Conversation

@mariash
Copy link
Member

@mariash mariash commented Feb 17, 2026

This PR adds a new command to bosh-agent "enable-monit-access"

It will add an nftables rule for bosh monit job first based on cgroup and then fall back to UUID based rule.

This PR depends on #399

This pulls in cloudfoundry/pxc-release#97 bosh-monit-access package as a new bosh-agent "enable-monit-access" command.

logger.UseTags([]boshlog.LogTag{{Name: "monit-access", LogLevel: boshlog.LevelDebug}})

// Validate nftables mode: verify if nftables is available
if len(args) > 1 && args[0] == "--validate-nftables-present" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shoudn't this flag parsing be something that sits somewhere closer to main. Like with the compile command: https://github.com/cloudfoundry/bosh-agent/blob/main/main/compile.go

Also nftables should always be there, its available on both jammy and noble so what is the use of this flag?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we can then drop this flag

Copy link
Member

Choose a reason for hiding this comment

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

As long as the failure case is fast (and doesn't impact the system) this seems like a good option.

@github-project-automation github-project-automation bot moved this from Inbox to Waiting for Changes | Open for Contribution in Foundational Infrastructure Working Group Feb 20, 2026
@mariash mariash force-pushed the monit-access-uid-firewall branch from 57b4a2a to c060ae2 Compare February 20, 2026 22:01
rkoster and others added 11 commits February 24, 2026 23:12
Implement a firewall mechanism that restricts NATS (mbus) connections
to the bosh-agent process only, using UID-based filtering with nftables.

Key changes:
- Add platform/firewall package with Manager and NatsFirewallHook interfaces
- Implement NftablesFirewall that creates UID-based egress rules
- Add GetNatsFirewallHook() to Platform interface
- Integrate BeforeConnect hook in nats_handler.go for connection/reconnection
- Support DNS re-resolution on reconnect for HA failover scenarios
- Add stub implementations for Windows and dummy platforms

The firewall rules allow only the agent's UID to connect to NATS/director
ports while blocking other processes, improving security posture.

Co-authored-by: Maria Shaldybin <maria.shaldybin@broadcom.com>
Add comprehensive unit tests for the new firewall functionality:

platform/firewall tests (23 tests):
- SetupMonitFirewall: table/chain/rule creation, error handling
- SetupNATSFirewall: IPv4/IPv6, DNS resolution, https/empty URL handling
- BeforeConnect: delegation to SetupNATSFirewall
- Cleanup: table deletion and error handling

mbus/nats_handler tests (4 new tests):
- Firewall hook is called on Start
- BeforeConnect receives correct mbus URL
- Handler still starts when hook returns nil
- Warning logged but no failure when BeforeConnect errors

Also:
- Add DNSResolver interface for testable DNS resolution
- Inject resolver dependency via NewNftablesFirewallWithDeps
- Configure test logging to use GinkgoWriter for visibility

Co-authored-by: Aram Price <aram.price@broadcom.com>
- Fix ST1023 linter error: omit type from var declaration
- Add linux_header.txt for counterfeiter to add build tags to Linux-only fakes
- Regenerate fake_nftables_conn.go and fake_dnsresolver.go with //go:build linux tag
- This fixes macOS/Windows build failures due to google/nftables being Linux-only

Co-authored-by: Maria Shaldybin <maria.shaldybin@broadcom.com>
- Fix nil pointer dereference in DisconnectErrHandler when err is nil
- Remove iptables-based SetupNatsFirewall code (replaced by nftables)
- Remove unused Cleanup() method from firewall interface
- Move firewall initialization from lazy getter to explicit SetupFirewall()
- Add comment explaining IPv6 loopback is intentionally not protected
  (monit only binds to 127.0.0.1:2822)

Co-authored-by: Aram Price <aram.price@broadcom.com>
The nftables library batches operations until Flush() is called, so
AddTable/AddChain/AddRule never return errors. Removing the misleading
error return types from these internal helper methods.

Co-authored-by: Maria Shaldybin <maria.shaldybin@broadcom.com>
Implement separate chains for agent-managed and job-managed monit access rules:
- monit_access_jobs: Regular chain for job rules (never flushed by agent)
- monit_access: Base chain that jumps to jobs chain first, then applies agent rules

This allows BOSH jobs to add their own monit access rules via pre-start scripts
that persist across agent restarts, while ensuring agent rules are always
up-to-date by flushing and recreating them on each setup call.

Co-authored-by: Aram Price <aram.price@broadcom.com>
Move to firewallfakes/linux_build_constraint.txt to make it clear this file
contains a Go build constraint for counterfeiter-generated fakes, not a C header.

Co-authored-by: Maria Shaldybin <maria.shaldybin@broadcom.com>
Signed-off-by: Maria Shaldybin <maria.shaldybin@broadcom.com>
Co-authored-by: Maria Shaldybin <maria.shaldybin@broadcom.com>
Signed-off-by: Maria Shaldybin <maria.shaldybin@broadcom.com>
Co-authored-by: Maria Shaldybin <maria.shaldybin@broadcom.com>
Signed-off-by: Aram Price <aram.price@broadcom.com>
Co-authored-by: Aram Price <aram.price@broadcom.com>
Signed-off-by: Aram Price <aram.price@broadcom.com>
Co-authored-by: Aram Price <aram.price@broadcom.com>
Signed-off-by: Aram Price <aram.price@broadcom.com>
Co-authored-by: Aram Price <aram.price@broadcom.com>
@mariash mariash force-pushed the monit-access-uid-firewall branch from c060ae2 to 8798736 Compare February 24, 2026 23:13
@mariash mariash marked this pull request as ready for review February 24, 2026 23:14
@mariash
Copy link
Member Author

mariash commented Feb 24, 2026

rebased and tested this change with warden-cpi

Jammy stemcell always has nftables
@mariash mariash requested a review from rkoster February 25, 2026 01:30
@github-project-automation github-project-automation bot moved this from Waiting for Changes | Open for Contribution to Pending Merge | Prioritized in Foundational Infrastructure Working Group Feb 25, 2026
@mariash mariash merged commit 10fc761 into main Feb 25, 2026
16 checks passed
@github-project-automation github-project-automation bot moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group Feb 25, 2026
@mariash mariash deleted the monit-access-uid-firewall branch February 25, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants