Skip to content

Require BOOTP chaddr match for host→VM DHCP responses - #192

Merged
edi-oai merged 2 commits into
openai:mainfrom
genforAI:fix/host-dhcp-chaddr-forward
Aug 14, 2026
Merged

Require BOOTP chaddr match for host→VM DHCP responses#192
edi-oai merged 2 commits into
openai:mainfrom
genforAI:fix/host-dhcp-chaddr-forward

Conversation

@genforAI

@genforAI genforAI commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Host→VM DHCP admit/forward previously checked gateway + UDP 67→68 only.
  • After Validate DHCP client identity and source address #191, VM→host requests already require BOOTP identity; this mirrors that check on BootReply using message_matches_bootp_client.
  • Foreign-chaddr DHCP replies are no longer treated as allowed DHCP (so they are not vm.writen via the DHCP carve-out).

Test plan

  • cargo test -p softnet --lib proxy::host::tests::dhcp_boot_reply_chaddr_must_match_vm
  • CI on PR

Notes

Low-severity / hardening completion relative to #186/#191. Not framed as a high-impact isolation bypass.

Mirror openai#191 request-path identity checks: admit/forward DHCP
BootReplies only when chaddr matches the VM MAC, so foreign
client replies are not written into the guest fd.

Co-authored-by: Cursor <cursoragent@cursor.com>
edi-oai
edi-oai previously approved these changes Aug 14, 2026

@edi-oai edi-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you!

@edi-oai
edi-oai self-requested a review August 14, 2026 12:40

@edi-oai edi-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ran cargo fmt.

@edi-oai
edi-oai merged commit 18f5a33 into openai:main Aug 14, 2026
2 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 37d4a80528

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread lib/proxy/host.rs
return false;
};

message_matches_bootp_client(&message, Opcode::BootReply, self.vm_mac_address.0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject foreign DHCP replies instead of falling through policy

When a gateway UDP 67→68 packet contains another client's MAC, returning false here only denies the DHCP bypass; it does not reject the packet. allowed_from_host_ipv4 still immediately returns Some(()) for every legacy/stateless configuration where flows.is_none(), while stateful configurations without a matching inbound rule also default to allowing it. Consequently, foreign broadcast DHCP replies are still written to the VM under common configurations; identify DHCP-shaped responses and explicitly reject mismatched clients before the generic policy paths.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants