feat(firewall): open XNet port to all nodes in the network#10263
Open
pierugo-dfinity wants to merge 2 commits into
Open
feat(firewall): open XNet port to all nodes in the network#10263pierugo-dfinity wants to merge 2 commits into
pierugo-dfinity wants to merge 2 commits into
Conversation
eichhorl
reviewed
May 20, 2026
Comment on lines
+324
to
+327
| trusted_tcp_ports_for_node_whitelist: [22, 4100, 8080], | ||
| trusted_udp_ports_for_node_whitelist: [4100], | ||
| untrusted_tcp_ports_for_node_whitelist: [2497], | ||
| untrusted_udp_ports_for_node_whitelist: [], |
Contributor
There was a problem hiding this comment.
I'm not sure I understand what trusted/untrusted means in this context. Maybe something like this?
Suggested change
| trusted_tcp_ports_for_node_whitelist: [22, 4100, 8080], | |
| trusted_udp_ports_for_node_whitelist: [4100], | |
| untrusted_tcp_ports_for_node_whitelist: [2497], | |
| untrusted_udp_ports_for_node_whitelist: [], | |
| whitelisted_node_tcp_ports: [22, 4100, 8080], | |
| whitelisted_node_udp_ports: [4100], | |
| all_node_tcp_ports: [2497], | |
| all_node_udp_ports: [], |
I think that would also be closer to the existing naming in the orchestrator
Contributor
Author
There was a problem hiding this comment.
Right, renamed here. I kept the "whitelist" at the end in contrast to the next field ports_for_http_adapter_blacklist
d9cd7d0 to
712017d
Compare
eichhorl
reviewed
May 21, 2026
| // Insert the whitelisting rules at the top of the list (highest priority) | ||
| tcp_rules.insert(0, tcp_node_whitelisting_rule); | ||
| udp_rules.insert(0, udp_node_whitelisting_rule); | ||
| // Insert the ic-http-adapter rule at the top of the list (highest priority) |
Contributor
There was a problem hiding this comment.
The comment here suggests that the order matters. Should we preserve it, and also add one above in get_node_whitelisting_rules?
Comment on lines
+324
to
+327
| whitelisted_nodes_tcp_ports_whitelist: [22, 4100, 8080], | ||
| whitelisted_nodes_udp_ports_whitelist: [4100], | ||
| all_nodes_tcp_ports_whitelist: [2497], | ||
| all_nodes_udp_ports_whitelist: [], |
Contributor
There was a problem hiding this comment.
Is there any system test we could adapt to test this?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In an effort to allow XNet connections between cloud engines and other subnets, this PR opens port 2497 (the XNet port) to all nodes in the network.
This is done by splitting the current
[tcp/udp]_ports_for_node_whitelistconfig field into twotrusted_[tcp/udp]_ports_for_node_whitelistanduntrusted_[tcp/udp]_ports_for_node_whitelist. The trusted ports are open only to "whitelisted" nodes (i.e., non-type4nodes for non-type4nodes, and all nodes fortype4nodes, as dictated by theis_whitelistedfunction infirewall.rs), while the untrusted ports are always open to all nodes.