-
Notifications
You must be signed in to change notification settings - Fork 45
Firewall Implementation: NAT #323
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
base: main
Are you sure you want to change the base?
Changes from 15 commits
f512a17
fbb237b
158eb13
1082b7f
9193b2c
e3b2605
07a8196
200786f
a28d369
f5c2a4b
9d53f17
a4e1e52
e7fc873
df38de9
12c9bb2
284666e
ec8419d
f4a8e45
8882aee
9f89270
3482f32
162930f
e29581a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,8 @@ | |
| #include <lions/firewall/filter.h> | ||
| #include <lions/firewall/ip.h> | ||
| #include <lions/firewall/routing.h> | ||
| #include <lions/firewall/nat_module.h> | ||
| #include <lions/firewall/nat_protocol.h> | ||
|
|
||
| #include "mpfirewallport.h" | ||
|
|
||
|
|
@@ -509,6 +511,71 @@ static mp_obj_t rule_get_nth(mp_obj_t interface_idx_in, mp_obj_t protocol_in, mp | |
|
|
||
| static MP_DEFINE_CONST_FUN_OBJ_3(rule_get_nth_obj, rule_get_nth); | ||
|
|
||
| /* NAT API functions */ | ||
|
|
||
| /* nat_set_enabled(interface, protocol, enabled) — PPC call to TX and RX virtualizers */ | ||
| static mp_obj_t nat_set_enabled(mp_obj_t interface_idx_in, mp_obj_t protocol_in, mp_obj_t enabled_in) | ||
| { | ||
| uint8_t interface_idx = mp_obj_get_int(interface_idx_in); | ||
| if (interface_idx >= FW_NUM_INTERFACES) { | ||
| sddf_dprintf("WEBSERVER|LOG: %s\n", fw_os_err_str[OS_ERR_INVALID_INTERFACE]); | ||
| mp_raise_OSError(OS_ERR_INVALID_INTERFACE); | ||
| return mp_const_none; | ||
| } | ||
|
|
||
| uint8_t protocol = mp_obj_get_int(protocol_in); | ||
| bool enabled = mp_obj_is_true(enabled_in); | ||
|
|
||
| for (uint8_t i = 0; i < fw_config.num_nat_state; i++) { | ||
| if (fw_config.nat_state[i].protocol == protocol && | ||
| fw_config.nat_state[i].interface == interface_idx) { | ||
| microkit_mr_set(NAT_SET_ENABLED_ARG_ENABLED, (seL4_Word)enabled); | ||
| (void)microkit_ppcall(fw_config.nat_state[i].tx_ch, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I am not so happy about calling two virtualisers. For one, it introduces a race condition. I propose that you just call one, and you designate responsibility to the "master" virtualiser for updating the NOTE: Let's return to this later, as it can be needlessly expensive for a non-NAT'd Rx virtualiser to have to check the table each packet. I think we could store a cached value in the Rx virt, so if the cached variable says NAT is disabled, you don't bother checking the table. Then, the Tx virt could PPC (or signal?) the Rx virt when it changes.... We can discuss later. Additionally, you need to check for errors here, i.e. fw_os_err_t os_err = filter_err_to_os_err(microkit_mr_get(FILTER_RET_ERR)); |
||
| microkit_msginfo_new(NAT_SET_ENABLED, NAT_SET_ENABLED_NUM_ARGS)); | ||
| microkit_mr_set(NAT_SET_ENABLED_ARG_ENABLED, (seL4_Word)enabled); | ||
| (void)microkit_ppcall(fw_config.nat_state[i].rx_ch, | ||
| microkit_msginfo_new(NAT_SET_ENABLED, NAT_SET_ENABLED_NUM_ARGS)); | ||
| return mp_const_none; | ||
| } | ||
| } | ||
|
|
||
| sddf_dprintf("WEBSERVER|LOG: %s\n", fw_os_err_str[OS_ERR_INVALID_PROTOCOL]); | ||
| mp_raise_OSError(OS_ERR_INVALID_PROTOCOL); | ||
| return mp_const_none; | ||
| } | ||
|
|
||
| static MP_DEFINE_CONST_FUN_OBJ_3(nat_set_enabled_obj, nat_set_enabled); | ||
|
|
||
| /* nat_get_enabled(interface, protocol) — PPC call to TX virtualizer, returns False if not configured */ | ||
| static mp_obj_t nat_get_enabled(mp_obj_t interface_idx_in, mp_obj_t protocol_in) | ||
| { | ||
| uint8_t interface_idx = mp_obj_get_int(interface_idx_in); | ||
| if (interface_idx >= FW_NUM_INTERFACES) { | ||
| return mp_const_false; | ||
| } | ||
|
|
||
| uint8_t protocol = mp_obj_get_int(protocol_in); | ||
|
|
||
| for (uint8_t i = 0; i < fw_config.num_nat_state; i++) { | ||
| if (fw_config.nat_state[i].protocol == protocol && | ||
| fw_config.nat_state[i].interface == interface_idx) { | ||
| microkit_msginfo reply = microkit_ppcall(fw_config.nat_state[i].tx_ch, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed in the config file, this can be read directly from the table without a ppc |
||
| microkit_msginfo_new(NAT_GET_ENABLED, 0)); | ||
| fw_nat_err_t err = (fw_nat_err_t)microkit_mr_get(NAT_RET_ERR); | ||
| (void)reply; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what this line of code does, but regardless please handle errors in the same way as the other functions in this file. The wrappers I wrote ensure the error gets propagated to Python in a useful way. |
||
| if (err != NAT_ERR_OKAY) { | ||
| return mp_const_false; | ||
| } | ||
| return mp_obj_new_bool((bool)microkit_mr_get(NAT_RET_ENABLED)); | ||
| } | ||
| } | ||
|
|
||
| /* NAT not configured for this interface/protocol — not an error, just disabled */ | ||
| return mp_const_false; | ||
| } | ||
|
|
||
| static MP_DEFINE_CONST_FUN_OBJ_2(nat_get_enabled_obj, nat_get_enabled); | ||
|
|
||
| static const mp_rom_map_elem_t lions_firewall_module_globals_table[] = { | ||
| { MP_OBJ_NEW_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_lions_firewall) }, | ||
| { MP_ROM_QSTR(MP_QSTR_interface_ip_get), MP_ROM_PTR(&interface_get_ip_obj) }, | ||
|
|
@@ -527,6 +594,8 @@ static const mp_rom_map_elem_t lions_firewall_module_globals_table[] = { | |
| { MP_ROM_QSTR(MP_QSTR_rule_count), MP_ROM_PTR(&rule_count_obj) }, | ||
| { MP_ROM_QSTR(MP_QSTR_filter_get_default_action), MP_ROM_PTR(&filter_get_default_action_obj) }, | ||
| { MP_ROM_QSTR(MP_QSTR_filter_set_default_action), MP_ROM_PTR(&filter_set_default_action_obj) }, | ||
| { MP_ROM_QSTR(MP_QSTR_nat_set_enabled), MP_ROM_PTR(&nat_set_enabled_obj) }, | ||
| { MP_ROM_QSTR(MP_QSTR_nat_get_enabled), MP_ROM_PTR(&nat_get_enabled_obj) }, | ||
| }; | ||
|
|
||
| static MP_DEFINE_CONST_DICT(lions_firewall_module_globals, lions_firewall_module_globals_table); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ FIREWALL_NETWORK_IMAGES:= firewall_network_virt_rx.elf firewall_network_virt_tx. | |
| firewall_network/net_components/%.o: ${FIREWALL_COMPONENTS}/%.c | ||
| ${CC} ${CFLAGS} -c -o $@ $< | ||
|
|
||
| FIREWALL_NETWORK_COMPONENT_OBJ := $(addprefix firewall_network/net_components/, network_virt_tx.o network_virt_rx.o) | ||
| FIREWALL_NETWORK_COMPONENT_OBJ := $(addprefix firewall_network/net_components/, network_virt_tx.o network_virt_rx.o nat_module.o) | ||
|
|
||
| CHECK_FIREWALL_NETWORK_FLAGS_MD5:=.firewall_network_cflags-$(shell echo -- ${CFLAGS} ${CFLAGS_network} | shasum | sed 's/ *-//') | ||
|
|
||
|
|
@@ -33,11 +33,17 @@ ${FIREWALL_NETWORK_COMPONENT_OBJ}: |firewall_network/net_components $(SDDF_LIBC_ | |
| ${FIREWALL_NETWORK_COMPONENT_OBJ}: ${CHECK_FIREWALL_NETWORK_FLAGS_MD5} | ||
| ${FIREWALL_NETWORK_COMPONENT_OBJ}: CFLAGS+=${CFLAGS_FIREWALL_NETWORK} | ||
|
|
||
| firewall_network/net_components/firewall_network_virt_%.o: ${SDDF}/firewall_network/net_components/virt_%.c |firewall_network/net_components | ||
| firewall_network/net_components/firewall_network_virt_%.o: ${FIREWALL_NETWORK_COMPONENTS_DIR}/firewall_network_virt_%.c |firewall_network/net_components | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, but please create a similar rule for the nat object file. |
||
| ${CC} ${CFLAGS} -c -o $@ $< | ||
|
|
||
| %.elf: firewall_network/net_components/%.o |firewall_network/net_components | ||
| ${LD} ${LDFLAGS} -o $@ $< ${LIBS} | ||
| firewall_network/net_components/nat_module.o: ${FIREWALL_NETWORK_COMPONENTS_DIR}/nat_module.c |firewall_network/net_components | ||
| ${CC} ${CFLAGS} -c -o $@ $< | ||
|
|
||
| firewall_network_virt_rx.elf: firewall_network/net_components/firewall_network_virt_rx.o firewall_network/net_components/nat_module.o |firewall_network/net_components | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should be able to combine these two rules using a wildcard, as was previously done. You are just adding one extra dependency. Look into the definitions of the automatic variables ( |
||
| ${LD} ${LDFLAGS} -o $@ firewall_network/net_components/firewall_network_virt_rx.o firewall_network/net_components/nat_module.o ${LIBS} | ||
|
|
||
| firewall_network_virt_tx.elf: firewall_network/net_components/firewall_network_virt_tx.o firewall_network/net_components/nat_module.o |firewall_network/net_components | ||
| ${LD} ${LDFLAGS} -o $@ firewall_network/net_components/firewall_network_virt_tx.o firewall_network/net_components/nat_module.o ${LIBS} | ||
|
|
||
| clean:: | ||
| ${RM} -f firewall_network_virt_[rt]x.[od] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have now wrapped this error handling in a function, please do the same: