Use /proc/sys/net/ipv4/ip_local_port_range to determine available outgoing ports #415
Conversation
config.h.in
Outdated
| @@ -846,6 +846,10 @@ | |||
| /* Define if you enable libevent */ | |||
| #undef USE_LIBEVENT | |||
|
|
|||
| /* Define this to enable use of /proc/sys/net/ipv4/ip_local_port_range as a | |||
| default outgoing port range. */ | |||
| #undef USE_LOCAL_PORT_RANGE | |||
There was a problem hiding this comment.
I would rename this to USE_LINUX_IP_LOCAL_PORT_RANGE and to specifically mention linux in the comment.
configure.ac
Outdated
| AC_ARG_ENABLE(local-port-range, AC_HELP_STRING([--enable-local-port-range], [enable use of /proc/sys/net/ipv4/ip_local_port_range as a default outgoing port range])) | ||
| case "$enable_local_port_range" in | ||
| yes) | ||
| AC_DEFINE([USE_LOCAL_PORT_RANGE], [1], [Define this to enable use of /proc/sys/net/ipv4/ip_local_port_range as a default outgoing port range.]) | ||
| ;; | ||
| no|*) | ||
| ;; | ||
| esac | ||
|
|
There was a problem hiding this comment.
Likewise for the rename to linux-ip-local-port-range. A mention that this is only for libunbound on linux and a warning that this may severely limit the number of available outgoing ports and thus decrease randomness would be useful.
I would also like to define this only when the host is linux (similar examples are found in this file).
There was a problem hiding this comment.
Please check if I understood "to define this only when the host is linux" correct
util/config_file.h
Outdated
| #ifdef USE_LOCAL_PORT_RANGE | ||
| #define IP_LOCAL_PORT_RANGE_PATH "/proc/sys/net/ipv4/ip_local_port_range" |
util/config_file.c
Outdated
| log_warn("failed to read from file: %s ", | ||
| IP_LOCAL_PORT_RANGE_PATH); | ||
| perror("fopen"); |
There was a problem hiding this comment.
I would prefer to remove perror and instead:
log_warn("failed to read from file: %s (%s)", IP_LOCAL_PORT_RANGE_PATH, strerror(errno));
There was a problem hiding this comment.
Done. Not sure if it's OK to write in error log when range is unexpected and in warn when failed to open the file.
util/config_file.c
Outdated
| #ifdef USE_LOCAL_PORT_RANGE | ||
| /* exclude ports not in ip_local_port_range */ | ||
| FILE* range_fd; | ||
| if ((range_fd = fopen(IP_LOCAL_PORT_RANGE_PATH, "r")) != NULL) { | ||
| int min_port = 0; | ||
| int max_port = num - 1; | ||
| if (fscanf(range_fd, "%d %d", &min_port, &max_port) == 2) { | ||
| for(i=0; i<min_port; i++) { | ||
| a[i] = 0; | ||
| } | ||
| for(i=max_port+1; i<num; i++) { | ||
| a[i] = 0; | ||
| } | ||
| } else { | ||
| log_err("unexpected port range in %s", | ||
| IP_LOCAL_PORT_RANGE_PATH); | ||
| } | ||
| fclose(range_fd); | ||
| } else { | ||
| log_warn("failed to read from file: %s ", | ||
| IP_LOCAL_PORT_RANGE_PATH); | ||
| perror("fopen"); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
If this code lives here it will also affect unbound itself, which is something we don't want.
This code can be moved in a separate function i.e cfg_apply_local_port_policy(struct config_file* cfg, int num) next to cfg_condense_ports() (which is also called by libunbound) and then be called as cfg_apply_local_port_policy(cfg, 65536) in context.c::context_finalize() right before config_apply().
Then instead of a you would use cfg->outgoing_avail_ports.
Also you would need to use:
(void)cfg;
(void)num;
outside of the #ifdef to keep the compiler happy about unused variables.
There was a problem hiding this comment.
I think it's done. Please check if I understood it correctly.
|
Oh, please ignore that last request for review, I clicked that by accident. I'm still working on the changes. |
sibeream
force-pushed
the
master
branch
4 times, most recently
from
e0b7f2e
to
f173e4e
Compare
|
@gthess I think it's ready to be reviewed. |
…red port range for libunbound on Linux
|
Thanks! LGTM |
- Merge PR #415 from sibeream: Use /proc/sys/net/ipv4/ip_local_port_range to determine available outgoing ports.
Use /proc/sys/net/ipv4/ip_local_port_range file to determine range of available ports.
configure --enable-local-port-rangeis required to make this work.Relates to #257