Skip to content
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.

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

Allow filtering ARP packets in fw rule. #316

Merged
merged 2 commits into from
Jan 10, 2023
Merged

Conversation

luqmana
Copy link
Contributor

@luqmana luqmana commented Jan 7, 2023

I know the external IP hack is on the way out but in the meantime you need to add a firewall rule to allow incoming ARP packets past the firewall layer.

It's currently possible to do this via adding an overly permissive rule:

$ opteadm add-fw-rule -p vm_test0 --action allow --dir in --hosts any --ports any --priority 65534 --protocol any
$ opteadm dump-layer -p vm_test0 firewall
[...snip...]
Inbound Rules
----------------------------------------------------------------------
ID     PRI    HITS   PREDICATES                             ACTION            
0      65534  0      *                                      "Stateful Allow"

With this PR you can make that a bit more fine grained:

$ opteadm add-fw-rule -p vm_test0 --action allow --dir in --hosts any --ports any --priority 65534 --protocol arp
$ opteadm dump-layer -p vm_test0 firewall
[...snip...]
Inbound Rules
----------------------------------------------------------------------
ID     PRI    HITS   PREDICATES                             ACTION            
0      65534  0      inner.ether.ether_type=ARP             "Stateful Allow"

@@ -730,6 +731,7 @@ impl FromStr for ProtoFilter {
fn from_str(s: &str) -> Result<Self, Self::Err> {
match s.to_ascii_lowercase().as_str() {
"any" => Ok(ProtoFilter::Any),
"arp" => Ok(ProtoFilter::Arp),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be

ProtoFilter::Proto(Protocol::ARP)

to be consistent with the others?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Considered it but if anything maybe that variant should just be called ProtoFilter::IpProto since into_predicate just converts it as-is:

ProtoFilter::Proto(p) => Predicate::InnerIpProto(vec![IpProtoMatch::Exact(p)])

which wouldn't work for ARP.

Copy link
Contributor

@rcgoodfellow rcgoodfellow Jan 9, 2023

Choose a reason for hiding this comment

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

Ahh I see. Yeah seems like ProtoFilter::IpProto would be better there.

Copy link
Contributor

@rcgoodfellow rcgoodfellow left a comment

Choose a reason for hiding this comment

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

LGTM!

@luqmana luqmana merged commit f501445 into master Jan 10, 2023
@luqmana luqmana deleted the luqmana/arp-fw-filter branch January 10, 2023 00:22
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