Read reserved ports from /proc/sys/net/ipv4/ip_local_reserved_ports,
store them into bit-wise array and before binding to random port check
if port is not reserved.
Currently, there is no way how to reserve ports so then will not be
used by rpcbind.
Random ports are opened by rpcbind because of rmtcalls. There is
compile-time flag for disabling them, but in some cases we can not
simply disable them.
One solution would be run time option --enable-rmtcalls as already
discussed, but it was rejected. So if we want to keep rmtcalls enabled
and also be able to reserve some ports, there is no other way than
filtering available ports. The easiest and clearest way seems to be
just respect kernel list of ip_reserved_ports.
Unfortunately there is one known disadvantage/side effect - it affects
probability of ports which are right after reserved ones. The bigger
reserved block is, the higher is probability of selecting following
unreserved port. But if there is no reserved port, impact of this patch
is minimal/none.
Signed-off-by: Otto Hollmann <[email protected]>
---
src/binddynport.c | 107 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 99 insertions(+), 8 deletions(-)
diff --git a/src/binddynport.c b/src/binddynport.c
index 062629a..6f78ebe 100644
--- a/src/binddynport.c
+++ b/src/binddynport.c
@@ -37,6 +37,7 @@
#include <unistd.h>
#include <errno.h>
#include <string.h>
+#include <syslog.h>
#include <rpc/rpc.h>
@@ -56,6 +57,84 @@ enum {
NPORTS = ENDPORT - LOWPORT + 1,
};
+/*
+ * This function decodes information about given port from provided array and
+ * return if port is reserved or not.
+ *
+ * @reserved_ports an array of size at least "NPORTS / (8*sizeof(char)) + 1".
+ * @port port number within range LOWPORT and ENDPORT
+ *
+ * Returns 0 if port is not reserved, non-negative if port is reserved.
+ */
+int is_reserved(char *reserved_ports, int port) {
+ port -= LOWPORT;
+ if (port < 0 || port >= NPORTS)
+ return 0;
+ return reserved_ports[port/(8*sizeof(char))] & 1<<(port%(8*sizeof(char)));
+}
+
+/*
+ * This function encodes information about given *reserved* port into provided
+ * array. Don't call this function for ports which are not reserved.
+ *
+ * @reserved_ports array TODO .
+ * @port port number within range LOWPORT and ENDPORT
+ *
+ */
+void set_reserved(char *reserved_ports, int port) {
+ port -= LOWPORT;
+ if (port < 0 || port >= NPORTS)
+ return;
+ reserved_ports[port/(8*sizeof(char))] |= 1<<(port%(8*sizeof(char)));
+}
+
+/*
+ * Parse local reserved ports obtained from
+ * /proc/sys/net/ipv4/ip_local_reserved_ports into bit array.
+ *
+ * @reserved_ports a zeroed array of size at least
+ * "NPORTS / (8*sizeof(char)) + 1". Will be used for bit-wise encoding of
+ * reserved ports.
+ *
+ * On each call, reserved ports are read from /proc and bit-wise stored into
+ * provided array
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+
+int parse_reserved_ports(char *reserved_ports) {
+ int from, to;
+ char delimiter = ',';
+ int res;
+ FILE * file_ptr = fopen("/proc/sys/net/ipv4/ip_local_reserved_ports","r");
+ if (file_ptr == NULL) {
+ (void) syslog(LOG_ERR,
+ "Unable to open open /proc/sys/net/ipv4/ip_local_reserved_ports.");
+ return -1;
+ }
+ do {
+ if ((res = fscanf(file_ptr, "%d", &to)) != 1) {
+ if (res == EOF) break;
+ goto err;
+ }
+ if (delimiter != '-') {
+ from = to;
+ }
+ for (int i = from; i <= to; ++i) {
+ set_reserved(reserved_ports, i);
+ }
+ } while ((res = fscanf(file_ptr, "%c", &delimiter)) == 1);
+ if (res != EOF)
+ goto err;
+ fclose(file_ptr);
+ return 0;
+err:
+ (void) syslog(LOG_ERR,
+ "An error occurred while parsing ip_local_reserved_ports.");
+ fclose(file_ptr);
+ return -1;
+}
+
/*
* Bind a socket to a dynamically-assigned IP port.
*
@@ -81,7 +160,8 @@ int __binddynport(int fd)
in_port_t port, *portp;
struct sockaddr *sap;
socklen_t salen;
- int i, res;
+ int i, res, array_size;
+ char *reserved_ports;
if (__rpc_sockisbound(fd))
return 0;
@@ -119,21 +199,32 @@ int __binddynport(int fd)
gettimeofday(&tv, NULL);
seed = tv.tv_usec * getpid();
}
+ array_size = NPORTS / (8*sizeof(char)) + 1;
+ reserved_ports = malloc(array_size);
+ if (!reserved_ports) {
+ goto out;
+ }
+ memset(reserved_ports, 0, array_size);
+ parse_reserved_ports(reserved_ports);
+
port = (rand_r(&seed) % NPORTS) + LOWPORT;
for (i = 0; i < NPORTS; ++i) {
- *portp = htons(port++);
- res = bind(fd, sap, salen);
- if (res >= 0) {
- res = 0;
- break;
+ *portp = htons(port);
+ if (!is_reserved(reserved_ports, port++)) {
+ res = bind(fd, sap, salen);
+ if (res >= 0) {
+ res = 0;
+ break;
+ }
+ if (errno != EADDRINUSE)
+ break;
}
- if (errno != EADDRINUSE)
- break;
if (port > ENDPORT)
port = LOWPORT;
}
out:
+ free(reserved_ports);
mutex_unlock(&port_lock);
return res;
}
--
2.26.2
Hey,
My apologies for taking so long to get to this...
A couple questions:
1) How well was tested... Is in your distro already?
2) Those new functions the patch introduces...
Don't effect the API? Meaning shouldn't they
declared as static?
steved.
On 6/9/22 4:26 AM, Otto Hollmann wrote:
> Read reserved ports from /proc/sys/net/ipv4/ip_local_reserved_ports,
> store them into bit-wise array and before binding to random port check
> if port is not reserved.
>
>
> Currently, there is no way how to reserve ports so then will not be
> used by rpcbind.
>
> Random ports are opened by rpcbind because of rmtcalls. There is
> compile-time flag for disabling them, but in some cases we can not
> simply disable them.
>
> One solution would be run time option --enable-rmtcalls as already
> discussed, but it was rejected. So if we want to keep rmtcalls enabled
> and also be able to reserve some ports, there is no other way than
> filtering available ports. The easiest and clearest way seems to be
> just respect kernel list of ip_reserved_ports.
>
> Unfortunately there is one known disadvantage/side effect - it affects
> probability of ports which are right after reserved ones. The bigger
> reserved block is, the higher is probability of selecting following
> unreserved port. But if there is no reserved port, impact of this patch
> is minimal/none.
>
> Signed-off-by: Otto Hollmann <[email protected]>
> ---
> src/binddynport.c | 107 ++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 99 insertions(+), 8 deletions(-)
>
> diff --git a/src/binddynport.c b/src/binddynport.c
> index 062629a..6f78ebe 100644
> --- a/src/binddynport.c
> +++ b/src/binddynport.c
> @@ -37,6 +37,7 @@
> #include <unistd.h>
> #include <errno.h>
> #include <string.h>
> +#include <syslog.h>
>
> #include <rpc/rpc.h>
>
> @@ -56,6 +57,84 @@ enum {
> NPORTS = ENDPORT - LOWPORT + 1,
> };
>
> +/*
> + * This function decodes information about given port from provided array and
> + * return if port is reserved or not.
> + *
> + * @reserved_ports an array of size at least "NPORTS / (8*sizeof(char)) + 1".
> + * @port port number within range LOWPORT and ENDPORT
> + *
> + * Returns 0 if port is not reserved, non-negative if port is reserved.
> + */
> +int is_reserved(char *reserved_ports, int port) {
> + port -= LOWPORT;
> + if (port < 0 || port >= NPORTS)
> + return 0;
> + return reserved_ports[port/(8*sizeof(char))] & 1<<(port%(8*sizeof(char)));
> +}
> +
> +/*
> + * This function encodes information about given *reserved* port into provided
> + * array. Don't call this function for ports which are not reserved.
> + *
> + * @reserved_ports array TODO .
> + * @port port number within range LOWPORT and ENDPORT
> + *
> + */
> +void set_reserved(char *reserved_ports, int port) {
> + port -= LOWPORT;
> + if (port < 0 || port >= NPORTS)
> + return;
> + reserved_ports[port/(8*sizeof(char))] |= 1<<(port%(8*sizeof(char)));
> +}
> +
> +/*
> + * Parse local reserved ports obtained from
> + * /proc/sys/net/ipv4/ip_local_reserved_ports into bit array.
> + *
> + * @reserved_ports a zeroed array of size at least
> + * "NPORTS / (8*sizeof(char)) + 1". Will be used for bit-wise encoding of
> + * reserved ports.
> + *
> + * On each call, reserved ports are read from /proc and bit-wise stored into
> + * provided array
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> +
> +int parse_reserved_ports(char *reserved_ports) {
> + int from, to;
> + char delimiter = ',';
> + int res;
> + FILE * file_ptr = fopen("/proc/sys/net/ipv4/ip_local_reserved_ports","r");
> + if (file_ptr == NULL) {
> + (void) syslog(LOG_ERR,
> + "Unable to open open /proc/sys/net/ipv4/ip_local_reserved_ports.");
> + return -1;
> + }
> + do {
> + if ((res = fscanf(file_ptr, "%d", &to)) != 1) {
> + if (res == EOF) break;
> + goto err;
> + }
> + if (delimiter != '-') {
> + from = to;
> + }
> + for (int i = from; i <= to; ++i) {
> + set_reserved(reserved_ports, i);
> + }
> + } while ((res = fscanf(file_ptr, "%c", &delimiter)) == 1);
> + if (res != EOF)
> + goto err;
> + fclose(file_ptr);
> + return 0;
> +err:
> + (void) syslog(LOG_ERR,
> + "An error occurred while parsing ip_local_reserved_ports.");
> + fclose(file_ptr);
> + return -1;
> +}
> +
> /*
> * Bind a socket to a dynamically-assigned IP port.
> *
> @@ -81,7 +160,8 @@ int __binddynport(int fd)
> in_port_t port, *portp;
> struct sockaddr *sap;
> socklen_t salen;
> - int i, res;
> + int i, res, array_size;
> + char *reserved_ports;
>
> if (__rpc_sockisbound(fd))
> return 0;
> @@ -119,21 +199,32 @@ int __binddynport(int fd)
> gettimeofday(&tv, NULL);
> seed = tv.tv_usec * getpid();
> }
> + array_size = NPORTS / (8*sizeof(char)) + 1;
> + reserved_ports = malloc(array_size);
> + if (!reserved_ports) {
> + goto out;
> + }
> + memset(reserved_ports, 0, array_size);
> + parse_reserved_ports(reserved_ports);
> +
> port = (rand_r(&seed) % NPORTS) + LOWPORT;
> for (i = 0; i < NPORTS; ++i) {
> - *portp = htons(port++);
> - res = bind(fd, sap, salen);
> - if (res >= 0) {
> - res = 0;
> - break;
> + *portp = htons(port);
> + if (!is_reserved(reserved_ports, port++)) {
> + res = bind(fd, sap, salen);
> + if (res >= 0) {
> + res = 0;
> + break;
> + }
> + if (errno != EADDRINUSE)
> + break;
> }
> - if (errno != EADDRINUSE)
> - break;
> if (port > ENDPORT)
> port = LOWPORT;
> }
>
> out:
> + free(reserved_ports);
> mutex_unlock(&port_lock);
> return res;
> }
Hi Steve
1) I tested various combinations of ip_local_port_range and
ip_local_reserved_ports, including edge cases like ip_local_port_range
equal to ip_local_reserved_ports. In all such tests library behaved as
expected. Let me know if I should test anything else.
No, not yet. We wanted to firstly open discussion, but now we can add
this patch into our distribution.
2) You are right, there is no reason why it shouldn't be declared as
static. Should I send updated patch or you will do this minor
modification yourself?
Otto
On Thu, 2022-07-14 at 13:01 -0500, Steve Dickson wrote:
> Hey,
>
> My apologies for taking so long to get to this...
>
> A couple questions:
>
> 1) How well was tested... Is in your distro already?
> 2) Those new functions the patch introduces...
> Don't effect the API? Meaning shouldn't they
> declared as static?
>
> steved.
>
> On 6/9/22 4:26 AM, Otto Hollmann wrote:
> > Read reserved ports from
> > /proc/sys/net/ipv4/ip_local_reserved_ports,
> > store them into bit-wise array and before binding to random port
> > check
> > if port is not reserved.
> >
> >
> > Currently, there is no way how to reserve ports so then will not be
> > used by rpcbind.
> >
> > Random ports are opened by rpcbind because of rmtcalls. There is
> > compile-time flag for disabling them, but in some cases we can not
> > simply disable them.
> >
> > One solution would be run time option --enable-rmtcalls as already
> > discussed, but it was rejected. So if we want to keep rmtcalls
> > enabled
> > and also be able to reserve some ports, there is no other way than
> > filtering available ports. The easiest and clearest way seems to be
> > just respect kernel list of ip_reserved_ports.
> >
> > Unfortunately there is one known disadvantage/side effect - it
> > affects
> > probability of ports which are right after reserved ones. The
> > bigger
> > reserved block is, the higher is probability of selecting following
> > unreserved port. But if there is no reserved port, impact of this
> > patch
> > is minimal/none.
> >
> > Signed-off-by: Otto Hollmann <[email protected]>
> > ---
> > src/binddynport.c | 107
> > ++++++++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 99 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/binddynport.c b/src/binddynport.c
> > index 062629a..6f78ebe 100644
> > --- a/src/binddynport.c
> > +++ b/src/binddynport.c
> > @@ -37,6 +37,7 @@
> > #include <unistd.h>
> > #include <errno.h>
> > #include <string.h>
> > +#include <syslog.h>
> >
> > #include <rpc/rpc.h>
> >
> > @@ -56,6 +57,84 @@ enum {
> > NPORTS = ENDPORT - LOWPORT + 1,
> > };
> >
> > +/*
> > + * This function decodes information about given port from
> > provided array and
> > + * return if port is reserved or not.
> > + *
> > + * @reserved_ports an array of size at least "NPORTS /
> > (8*sizeof(char)) + 1".
> > + * @port port number within range LOWPORT and ENDPORT
> > + *
> > + * Returns 0 if port is not reserved, non-negative if port is
> > reserved.
> > + */
> > +int is_reserved(char *reserved_ports, int port) {
> > + port -= LOWPORT;
> > + if (port < 0 || port >= NPORTS)
> > + return 0;
> > + return reserved_ports[port/(8*sizeof(char))] &
> > 1<<(port%(8*sizeof(char)));
> > +}
> > +
> > +/*
> > + * This function encodes information about given *reserved* port
> > into provided
> > + * array. Don't call this function for ports which are not
> > reserved.
> > + *
> > + * @reserved_ports array TODO .
> > + * @port port number within range LOWPORT and ENDPORT
> > + *
> > + */
> > +void set_reserved(char *reserved_ports, int port) {
> > + port -= LOWPORT;
> > + if (port < 0 || port >= NPORTS)
> > + return;
> > + reserved_ports[port/(8*sizeof(char))] |=
> > 1<<(port%(8*sizeof(char)));
> > +}
> > +
> > +/*
> > + * Parse local reserved ports obtained from
> > + * /proc/sys/net/ipv4/ip_local_reserved_ports into bit array.
> > + *
> > + * @reserved_ports a zeroed array of size at least
> > + * "NPORTS / (8*sizeof(char)) + 1". Will be used for bit-wise
> > encoding of
> > + * reserved ports.
> > + *
> > + * On each call, reserved ports are read from /proc and bit-wise
> > stored into
> > + * provided array
> > + *
> > + * Returns 0 on success, -1 on failure.
> > + */
> > +
> > +int parse_reserved_ports(char *reserved_ports) {
> > + int from, to;
> > + char delimiter = ',';
> > + int res;
> > + FILE * file_ptr =
> > fopen("/proc/sys/net/ipv4/ip_local_reserved_ports","r");
> > + if (file_ptr == NULL) {
> > + (void) syslog(LOG_ERR,
> > + "Unable to open open
> > /proc/sys/net/ipv4/ip_local_reserved_ports.");
> > + return -1;
> > + }
> > + do {
> > + if ((res = fscanf(file_ptr, "%d", &to)) != 1) {
> > + if (res == EOF) break;
> > + goto err;
> > + }
> > + if (delimiter != '-') {
> > + from = to;
> > + }
> > + for (int i = from; i <= to; ++i) {
> > + set_reserved(reserved_ports, i);
> > + }
> > + } while ((res = fscanf(file_ptr, "%c", &delimiter)) == 1);
> > + if (res != EOF)
> > + goto err;
> > + fclose(file_ptr);
> > + return 0;
> > +err:
> > + (void) syslog(LOG_ERR,
> > + "An error occurred while parsing
> > ip_local_reserved_ports.");
> > + fclose(file_ptr);
> > + return -1;
> > +}
> > +
> > /*
> > * Bind a socket to a dynamically-assigned IP port.
> > *
> > @@ -81,7 +160,8 @@ int __binddynport(int fd)
> > in_port_t port, *portp;
> > struct sockaddr *sap;
> > socklen_t salen;
> > - int i, res;
> > + int i, res, array_size;
> > + char *reserved_ports;
> >
> > if (__rpc_sockisbound(fd))
> > return 0;
> > @@ -119,21 +199,32 @@ int __binddynport(int fd)
> > gettimeofday(&tv, NULL);
> > seed = tv.tv_usec * getpid();
> > }
> > + array_size = NPORTS / (8*sizeof(char)) + 1;
> > + reserved_ports = malloc(array_size);
> > + if (!reserved_ports) {
> > + goto out;
> > + }
> > + memset(reserved_ports, 0, array_size);
> > + parse_reserved_ports(reserved_ports);
> > +
> > port = (rand_r(&seed) % NPORTS) + LOWPORT;
> > for (i = 0; i < NPORTS; ++i) {
> > - *portp = htons(port++);
> > - res = bind(fd, sap, salen);
> > - if (res >= 0) {
> > - res = 0;
> > - break;
> > + *portp = htons(port);
> > + if (!is_reserved(reserved_ports, port++)) {
> > + res = bind(fd, sap, salen);
> > + if (res >= 0) {
> > + res = 0;
> > + break;
> > + }
> > + if (errno != EADDRINUSE)
> > + break;
> > }
> > - if (errno != EADDRINUSE)
> > - break;
> > if (port > ENDPORT)
> > port = LOWPORT;
> > }
> >
> > out:
> > + free(reserved_ports);
> > mutex_unlock(&port_lock);
> > return res;
> > }
>
For the record, I'm sending an updated patch with static functions.
Signed-off-by: Otto Hollmann <[email protected]>
---
src/binddynport.c | 107 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 99 insertions(+), 8 deletions(-)
diff --git a/src/binddynport.c b/src/binddynport.c
index 062629a..90e2748 100644
--- a/src/binddynport.c
+++ b/src/binddynport.c
@@ -37,6 +37,7 @@
#include <unistd.h>
#include <errno.h>
#include <string.h>
+#include <syslog.h>
#include <rpc/rpc.h>
@@ -56,6 +57,84 @@ enum {
NPORTS = ENDPORT - LOWPORT + 1,
};
+/*
+ * This function decodes information about given port from provided array and
+ * return if port is reserved or not.
+ *
+ * @reserved_ports an array of size at least "NPORTS / (8*sizeof(char)) + 1".
+ * @port port number within range LOWPORT and ENDPORT
+ *
+ * Returns 0 if port is not reserved, non-negative if port is reserved.
+ */
+static int is_reserved(char *reserved_ports, int port) {
+ port -= LOWPORT;
+ if (port < 0 || port >= NPORTS)
+ return 0;
+ return reserved_ports[port/(8*sizeof(char))] & 1<<(port%(8*sizeof(char)));
+}
+
+/*
+ * This function encodes information about given *reserved* port into provided
+ * array. Don't call this function for ports which are not reserved.
+ *
+ * @reserved_ports an array of size at least "NPORTS / (8*sizeof(char)) + 1".
+ * @port port number within range LOWPORT and ENDPORT
+ *
+ */
+static void set_reserved(char *reserved_ports, int port) {
+ port -= LOWPORT;
+ if (port < 0 || port >= NPORTS)
+ return;
+ reserved_ports[port/(8*sizeof(char))] |= 1<<(port%(8*sizeof(char)));
+}
+
+/*
+ * Parse local reserved ports obtained from
+ * /proc/sys/net/ipv4/ip_local_reserved_ports into bit array.
+ *
+ * @reserved_ports a zeroed array of size at least
+ * "NPORTS / (8*sizeof(char)) + 1". Will be used for bit-wise encoding of
+ * reserved ports.
+ *
+ * On each call, reserved ports are read from /proc and bit-wise stored into
+ * provided array
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+
+static int parse_reserved_ports(char *reserved_ports) {
+ int from, to;
+ char delimiter = ',';
+ int res;
+ FILE * file_ptr = fopen("/proc/sys/net/ipv4/ip_local_reserved_ports","r");
+ if (file_ptr == NULL) {
+ (void) syslog(LOG_ERR,
+ "Unable to open open /proc/sys/net/ipv4/ip_local_reserved_ports.");
+ return -1;
+ }
+ do {
+ if ((res = fscanf(file_ptr, "%d", &to)) != 1) {
+ if (res == EOF) break;
+ goto err;
+ }
+ if (delimiter != '-') {
+ from = to;
+ }
+ for (int i = from; i <= to; ++i) {
+ set_reserved(reserved_ports, i);
+ }
+ } while ((res = fscanf(file_ptr, "%c", &delimiter)) == 1);
+ if (res != EOF)
+ goto err;
+ fclose(file_ptr);
+ return 0;
+err:
+ (void) syslog(LOG_ERR,
+ "An error occurred while parsing ip_local_reserved_ports.");
+ fclose(file_ptr);
+ return -1;
+}
+
/*
* Bind a socket to a dynamically-assigned IP port.
*
@@ -81,7 +160,8 @@ int __binddynport(int fd)
in_port_t port, *portp;
struct sockaddr *sap;
socklen_t salen;
- int i, res;
+ int i, res, array_size;
+ char *reserved_ports;
if (__rpc_sockisbound(fd))
return 0;
@@ -119,21 +199,32 @@ int __binddynport(int fd)
gettimeofday(&tv, NULL);
seed = tv.tv_usec * getpid();
}
+ array_size = NPORTS / (8*sizeof(char)) + 1;
+ reserved_ports = malloc(array_size);
+ if (!reserved_ports) {
+ goto out;
+ }
+ memset(reserved_ports, 0, array_size);
+ parse_reserved_ports(reserved_ports);
+
port = (rand_r(&seed) % NPORTS) + LOWPORT;
for (i = 0; i < NPORTS; ++i) {
- *portp = htons(port++);
- res = bind(fd, sap, salen);
- if (res >= 0) {
- res = 0;
- break;
+ *portp = htons(port);
+ if (!is_reserved(reserved_ports, port++)) {
+ res = bind(fd, sap, salen);
+ if (res >= 0) {
+ res = 0;
+ break;
+ }
+ if (errno != EADDRINUSE)
+ break;
}
- if (errno != EADDRINUSE)
- break;
if (port > ENDPORT)
port = LOWPORT;
}
out:
+ free(reserved_ports);
mutex_unlock(&port_lock);
return res;
}
--
2.26.2
On Wed, 2022-10-05 at 10:13 +0200, Otto Hollmann wrote:
> Hi Steve
>
> 1) I tested various combinations of ip_local_port_range and
> ip_local_reserved_ports, including edge cases like
> ip_local_port_range
> equal to ip_local_reserved_ports. In all such tests library behaved
> as
> expected. Let me know if I should test anything else.
>
> No, not yet. We wanted to firstly open discussion, but now we can add
> this patch into our distribution.
>
> 2) You are right, there is no reason why it shouldn't be declared as
> static. Should I send updated patch or you will do this minor
> modification yourself?
>
>
> Otto
>
> On Thu, 2022-07-14 at 13:01 -0500, Steve Dickson wrote:
> > Hey,
> >
> > My apologies for taking so long to get to this...
> >
> > A couple questions:
> >
> > 1) How well was tested... Is in your distro already?
> > 2) Those new functions the patch introduces...
> > Don't effect the API? Meaning shouldn't they
> > declared as static?
> >
> > steved.
> >
> > On 6/9/22 4:26 AM, Otto Hollmann wrote:
> > > Read reserved ports from
> > > /proc/sys/net/ipv4/ip_local_reserved_ports,
> > > store them into bit-wise array and before binding to random port
> > > check
> > > if port is not reserved.
> > >
> > >
> > > Currently, there is no way how to reserve ports so then will not
> > > be
> > > used by rpcbind.
> > >
> > > Random ports are opened by rpcbind because of rmtcalls. There is
> > > compile-time flag for disabling them, but in some cases we can
> > > not
> > > simply disable them.
> > >
> > > One solution would be run time option --enable-rmtcalls as
> > > already
> > > discussed, but it was rejected. So if we want to keep rmtcalls
> > > enabled
> > > and also be able to reserve some ports, there is no other way
> > > than
> > > filtering available ports. The easiest and clearest way seems to
> > > be
> > > just respect kernel list of ip_reserved_ports.
> > >
> > > Unfortunately there is one known disadvantage/side effect - it
> > > affects
> > > probability of ports which are right after reserved ones. The
> > > bigger
> > > reserved block is, the higher is probability of selecting
> > > following
> > > unreserved port. But if there is no reserved port, impact of this
> > > patch
> > > is minimal/none.
> > >
> > > Signed-off-by: Otto Hollmann <[email protected]>
> > > ---
> > > src/binddynport.c | 107
> > > ++++++++++++++++++++++++++++++++++++++++++----
> > > 1 file changed, 99 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/src/binddynport.c b/src/binddynport.c
> > > index 062629a..6f78ebe 100644
> > > --- a/src/binddynport.c
> > > +++ b/src/binddynport.c
> > > @@ -37,6 +37,7 @@
> > > #include <unistd.h>
> > > #include <errno.h>
> > > #include <string.h>
> > > +#include <syslog.h>
> > >
> > > #include <rpc/rpc.h>
> > >
> > > @@ -56,6 +57,84 @@ enum {
> > > NPORTS = ENDPORT - LOWPORT + 1,
> > > };
> > >
> > > +/*
> > > + * This function decodes information about given port from
> > > provided array and
> > > + * return if port is reserved or not.
> > > + *
> > > + * @reserved_ports an array of size at least "NPORTS /
> > > (8*sizeof(char)) + 1".
> > > + * @port port number within range LOWPORT and ENDPORT
> > > + *
> > > + * Returns 0 if port is not reserved, non-negative if port is
> > > reserved.
> > > + */
> > > +int is_reserved(char *reserved_ports, int port) {
> > > + port -= LOWPORT;
> > > + if (port < 0 || port >= NPORTS)
> > > + return 0;
> > > + return reserved_ports[port/(8*sizeof(char))] &
> > > 1<<(port%(8*sizeof(char)));
> > > +}
> > > +
> > > +/*
> > > + * This function encodes information about given *reserved* port
> > > into provided
> > > + * array. Don't call this function for ports which are not
> > > reserved.
> > > + *
> > > + * @reserved_ports array TODO .
> > > + * @port port number within range LOWPORT and ENDPORT
> > > + *
> > > + */
> > > +void set_reserved(char *reserved_ports, int port) {
> > > + port -= LOWPORT;
> > > + if (port < 0 || port >= NPORTS)
> > > + return;
> > > + reserved_ports[port/(8*sizeof(char))] |=
> > > 1<<(port%(8*sizeof(char)));
> > > +}
> > > +
> > > +/*
> > > + * Parse local reserved ports obtained from
> > > + * /proc/sys/net/ipv4/ip_local_reserved_ports into bit array.
> > > + *
> > > + * @reserved_ports a zeroed array of size at least
> > > + * "NPORTS / (8*sizeof(char)) + 1". Will be used for bit-wise
> > > encoding of
> > > + * reserved ports.
> > > + *
> > > + * On each call, reserved ports are read from /proc and bit-wise
> > > stored into
> > > + * provided array
> > > + *
> > > + * Returns 0 on success, -1 on failure.
> > > + */
> > > +
> > > +int parse_reserved_ports(char *reserved_ports) {
> > > + int from, to;
> > > + char delimiter = ',';
> > > + int res;
> > > + FILE * file_ptr =
> > > fopen("/proc/sys/net/ipv4/ip_local_reserved_ports","r");
> > > + if (file_ptr == NULL) {
> > > + (void) syslog(LOG_ERR,
> > > + "Unable to open open
> > > /proc/sys/net/ipv4/ip_local_reserved_ports.");
> > > + return -1;
> > > + }
> > > + do {
> > > + if ((res = fscanf(file_ptr, "%d", &to)) != 1) {
> > > + if (res == EOF) break;
> > > + goto err;
> > > + }
> > > + if (delimiter != '-') {
> > > + from = to;
> > > + }
> > > + for (int i = from; i <= to; ++i) {
> > > + set_reserved(reserved_ports, i);
> > > + }
> > > + } while ((res = fscanf(file_ptr, "%c", &delimiter)) ==
> > > 1);
> > > + if (res != EOF)
> > > + goto err;
> > > + fclose(file_ptr);
> > > + return 0;
> > > +err:
> > > + (void) syslog(LOG_ERR,
> > > + "An error occurred while parsing
> > > ip_local_reserved_ports.");
> > > + fclose(file_ptr);
> > > + return -1;
> > > +}
> > > +
> > > /*
> > > * Bind a socket to a dynamically-assigned IP port.
> > > *
> > > @@ -81,7 +160,8 @@ int __binddynport(int fd)
> > > in_port_t port, *portp;
> > > struct sockaddr *sap;
> > > socklen_t salen;
> > > - int i, res;
> > > + int i, res, array_size;
> > > + char *reserved_ports;
> > >
> > > if (__rpc_sockisbound(fd))
> > > return 0;
> > > @@ -119,21 +199,32 @@ int __binddynport(int fd)
> > > gettimeofday(&tv, NULL);
> > > seed = tv.tv_usec * getpid();
> > > }
> > > + array_size = NPORTS / (8*sizeof(char)) + 1;
> > > + reserved_ports = malloc(array_size);
> > > + if (!reserved_ports) {
> > > + goto out;
> > > + }
> > > + memset(reserved_ports, 0, array_size);
> > > + parse_reserved_ports(reserved_ports);
> > > +
> > > port = (rand_r(&seed) % NPORTS) + LOWPORT;
> > > for (i = 0; i < NPORTS; ++i) {
> > > - *portp = htons(port++);
> > > - res = bind(fd, sap, salen);
> > > - if (res >= 0) {
> > > - res = 0;
> > > - break;
> > > + *portp = htons(port);
> > > + if (!is_reserved(reserved_ports, port++)) {
> > > + res = bind(fd, sap, salen);
> > > + if (res >= 0) {
> > > + res = 0;
> > > + break;
> > > + }
> > > + if (errno != EADDRINUSE)
> > > + break;
> > > }
> > > - if (errno != EADDRINUSE)
> > > - break;
> > > if (port > ENDPORT)
> > > port = LOWPORT;
> > > }
> > >
> > > out:
> > > + free(reserved_ports);
> > > mutex_unlock(&port_lock);
> > > return res;
> > > }
> >
>
On 6/9/22 5:26 AM, Otto Hollmann wrote:
> Read reserved ports from /proc/sys/net/ipv4/ip_local_reserved_ports,
> store them into bit-wise array and before binding to random port check
> if port is not reserved.
>
>
> Currently, there is no way how to reserve ports so then will not be
> used by rpcbind.
>
> Random ports are opened by rpcbind because of rmtcalls. There is
> compile-time flag for disabling them, but in some cases we can not
> simply disable them.
>
> One solution would be run time option --enable-rmtcalls as already
> discussed, but it was rejected. So if we want to keep rmtcalls enabled
> and also be able to reserve some ports, there is no other way than
> filtering available ports. The easiest and clearest way seems to be
> just respect kernel list of ip_reserved_ports.
>
> Unfortunately there is one known disadvantage/side effect - it affects
> probability of ports which are right after reserved ones. The bigger
> reserved block is, the higher is probability of selecting following
> unreserved port. But if there is no reserved port, impact of this patch
> is minimal/none.
>
> Signed-off-by: Otto Hollmann <[email protected]>
Committed... (tag libtirpc-1-3-4-rc4)
steved
> ---
> src/binddynport.c | 107 ++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 99 insertions(+), 8 deletions(-)
>
> diff --git a/src/binddynport.c b/src/binddynport.c
> index 062629a..6f78ebe 100644
> --- a/src/binddynport.c
> +++ b/src/binddynport.c
> @@ -37,6 +37,7 @@
> #include <unistd.h>
> #include <errno.h>
> #include <string.h>
> +#include <syslog.h>
>
> #include <rpc/rpc.h>
>
> @@ -56,6 +57,84 @@ enum {
> NPORTS = ENDPORT - LOWPORT + 1,
> };
>
> +/*
> + * This function decodes information about given port from provided array and
> + * return if port is reserved or not.
> + *
> + * @reserved_ports an array of size at least "NPORTS / (8*sizeof(char)) + 1".
> + * @port port number within range LOWPORT and ENDPORT
> + *
> + * Returns 0 if port is not reserved, non-negative if port is reserved.
> + */
> +int is_reserved(char *reserved_ports, int port) {
> + port -= LOWPORT;
> + if (port < 0 || port >= NPORTS)
> + return 0;
> + return reserved_ports[port/(8*sizeof(char))] & 1<<(port%(8*sizeof(char)));
> +}
> +
> +/*
> + * This function encodes information about given *reserved* port into provided
> + * array. Don't call this function for ports which are not reserved.
> + *
> + * @reserved_ports array TODO .
> + * @port port number within range LOWPORT and ENDPORT
> + *
> + */
> +void set_reserved(char *reserved_ports, int port) {
> + port -= LOWPORT;
> + if (port < 0 || port >= NPORTS)
> + return;
> + reserved_ports[port/(8*sizeof(char))] |= 1<<(port%(8*sizeof(char)));
> +}
> +
> +/*
> + * Parse local reserved ports obtained from
> + * /proc/sys/net/ipv4/ip_local_reserved_ports into bit array.
> + *
> + * @reserved_ports a zeroed array of size at least
> + * "NPORTS / (8*sizeof(char)) + 1". Will be used for bit-wise encoding of
> + * reserved ports.
> + *
> + * On each call, reserved ports are read from /proc and bit-wise stored into
> + * provided array
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> +
> +int parse_reserved_ports(char *reserved_ports) {
> + int from, to;
> + char delimiter = ',';
> + int res;
> + FILE * file_ptr = fopen("/proc/sys/net/ipv4/ip_local_reserved_ports","r");
> + if (file_ptr == NULL) {
> + (void) syslog(LOG_ERR,
> + "Unable to open open /proc/sys/net/ipv4/ip_local_reserved_ports.");
> + return -1;
> + }
> + do {
> + if ((res = fscanf(file_ptr, "%d", &to)) != 1) {
> + if (res == EOF) break;
> + goto err;
> + }
> + if (delimiter != '-') {
> + from = to;
> + }
> + for (int i = from; i <= to; ++i) {
> + set_reserved(reserved_ports, i);
> + }
> + } while ((res = fscanf(file_ptr, "%c", &delimiter)) == 1);
> + if (res != EOF)
> + goto err;
> + fclose(file_ptr);
> + return 0;
> +err:
> + (void) syslog(LOG_ERR,
> + "An error occurred while parsing ip_local_reserved_ports.");
> + fclose(file_ptr);
> + return -1;
> +}
> +
> /*
> * Bind a socket to a dynamically-assigned IP port.
> *
> @@ -81,7 +160,8 @@ int __binddynport(int fd)
> in_port_t port, *portp;
> struct sockaddr *sap;
> socklen_t salen;
> - int i, res;
> + int i, res, array_size;
> + char *reserved_ports;
>
> if (__rpc_sockisbound(fd))
> return 0;
> @@ -119,21 +199,32 @@ int __binddynport(int fd)
> gettimeofday(&tv, NULL);
> seed = tv.tv_usec * getpid();
> }
> + array_size = NPORTS / (8*sizeof(char)) + 1;
> + reserved_ports = malloc(array_size);
> + if (!reserved_ports) {
> + goto out;
> + }
> + memset(reserved_ports, 0, array_size);
> + parse_reserved_ports(reserved_ports);
> +
> port = (rand_r(&seed) % NPORTS) + LOWPORT;
> for (i = 0; i < NPORTS; ++i) {
> - *portp = htons(port++);
> - res = bind(fd, sap, salen);
> - if (res >= 0) {
> - res = 0;
> - break;
> + *portp = htons(port);
> + if (!is_reserved(reserved_ports, port++)) {
> + res = bind(fd, sap, salen);
> + if (res >= 0) {
> + res = 0;
> + break;
> + }
> + if (errno != EADDRINUSE)
> + break;
> }
> - if (errno != EADDRINUSE)
> - break;
> if (port > ENDPORT)
> port = LOWPORT;
> }
>
> out:
> + free(reserved_ports);
> mutex_unlock(&port_lock);
> return res;
> }