Subject: [PATCH] rpcb_clnt.c add mechanism to try v2 protocol first

There have been previous attempts to revert protocol tryout
algorithm from v4,v3,v2 to previous v2,v4,v3 :

https://www.spinics.net/lists/linux-nfs/msg89228.html

Apart from GETADDR/NAT issue originating that proposed change,
its possible that some legacy custom applications still use
v2 of protocol with libtirpc.

The change proposed here, introduces an environment variable
"RPCB_V2FIRST" so that, if defined, old behaviour is used.
This is more flexible and allow us to selectively pick what
application reverts to old behaviour instead of a system-wide
change.

Example :

$ tcpdump -s0 -i ens3 port 111 -w /tmp/capture.pcap &> /dev/null &
[1] 13016
$ rpcinfo -T tcp 172.23.1.225 100005 &> /dev/null
$ RPCB_V2FIRST=1 rpcinfo -T tcp 172.23.1.225 100005 &> /dev/null
$ pkill tcpdump
$ tshark -tad -nr /tmp/capture.pcap -Y portmap -T fields -e _ws.col.Info
V4 GETADDR Call
V4 GETADDR Reply (Call In 4)
V2 GETPORT Call MOUNT(100005) V:0 TCP
V2 GETPORT Reply (Call In 14) Port:20048

Signed-off-by: Roberto Bergantinos Corpas <[email protected]>
---
man/rpcbind.3t | 2 ++
src/rpcb_clnt.c | 27 ++++++++++++++++++++++++---
tirpc/rpc/pmap_prot.h | 2 ++
3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/man/rpcbind.3t b/man/rpcbind.3t
index ec492cc..4cb271b 100644
--- a/man/rpcbind.3t
+++ b/man/rpcbind.3t
@@ -187,6 +187,8 @@ in
.El
.Sh AVAILABILITY
These functions are part of libtirpc.
+.Sh ENVIRONMENT
+If RPCB_V2FIRST is defined, rpcbind protocol version tryout algorithm changes from v4,v2,v3 to v2,v4,v3.
.Sh SEE ALSO
.Xr rpc_clnt_calls 3 ,
.Xr rpc_svc_calls 3 ,
diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
index 0c34cb7..db3799e 100644
--- a/src/rpcb_clnt.c
+++ b/src/rpcb_clnt.c
@@ -818,7 +818,8 @@ error:
* The algorithm used: If the transports is TCP or UDP, it first tries
* version 4 (srv4), then 3 and then fall back to version 2 (portmap).
* With this algorithm, we get performance as well as a plan for
- * obsoleting version 2.
+ * obsoleting version 2. This behaviour is reverted to old algorithm
+ * if RPCB_V2FIRST environment var is defined
*
* For all other transports, the algorithm remains as 4 and then 3.
*
@@ -839,6 +840,10 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
#ifdef NOTUSED
static bool_t check_rpcbind = TRUE;
#endif
+
+#ifdef PORTMAP
+ static bool_t portmap_first = FALSE;
+#endif
CLIENT *client = NULL;
RPCB parms;
enum clnt_stat clnt_st;
@@ -895,8 +900,18 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
parms.r_addr = (char *) &nullstring[0];
}

- /* First try from start_vers(4) and then version 3 (RPCBVERS) */
+ /* First try from start_vers(4) and then version 3 (RPCBVERS), except
+ * if env. var RPCB_V2FIRST is defined */
+
+#ifdef PORTMAP
+ if (getenv(V2FIRST)) {
+ portmap_first = TRUE;
+ LIBTIRPC_DEBUG(3, ("__rpcb_findaddr_timed: trying v2-port first\n"));
+ goto portmap;
+ }
+#endif

+rpcbind:
CLNT_CONTROL(client, CLSET_RETRY_TIMEOUT, (char *) &rpcbrmttime);
for (vers = start_vers; vers >= RPCBVERS; vers--) {
/* Set the version */
@@ -944,10 +959,16 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
}

#ifdef PORTMAP /* Try version 2 for TCP or UDP */
+ if (portmap_first)
+ goto error; /* we tried all versions if reached here */
+portmap:
if (strcmp(nconf->nc_protofmly, NC_INET) == 0) {
address = __try_protocol_version_2(program, version, nconf, host, tp);
if (address == NULL)
- goto error;
+ if (portmap_first)
+ goto rpcbind;
+ else
+ goto error;
}
#endif /* PORTMAP */

diff --git a/tirpc/rpc/pmap_prot.h b/tirpc/rpc/pmap_prot.h
index 75354ce..7718b8b 100644
--- a/tirpc/rpc/pmap_prot.h
+++ b/tirpc/rpc/pmap_prot.h
@@ -84,6 +84,8 @@
#define PMAPPROC_DUMP ((u_long)4)
#define PMAPPROC_CALLIT ((u_long)5)

+#define V2FIRST "RPCB_V2FIRST"
+
struct pmap {
long unsigned pm_prog;
long unsigned pm_vers;
--
2.31.1


2022-07-16 18:22:05

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] rpcb_clnt.c add mechanism to try v2 protocol first



On 7/1/22 9:44 AM, Roberto Bergantinos Corpas wrote:
> There have been previous attempts to revert protocol tryout
> algorithm from v4,v3,v2 to previous v2,v4,v3 :
>
> https://www.spinics.net/lists/linux-nfs/msg89228.html
>
> Apart from GETADDR/NAT issue originating that proposed change,
> its possible that some legacy custom applications still use
> v2 of protocol with libtirpc.
>
> The change proposed here, introduces an environment variable
> "RPCB_V2FIRST" so that, if defined, old behaviour is used.
> This is more flexible and allow us to selectively pick what
> application reverts to old behaviour instead of a system-wide
> change.
>
> Example :
>
> $ tcpdump -s0 -i ens3 port 111 -w /tmp/capture.pcap &> /dev/null &
> [1] 13016
> $ rpcinfo -T tcp 172.23.1.225 100005 &> /dev/null
> $ RPCB_V2FIRST=1 rpcinfo -T tcp 172.23.1.225 100005 &> /dev/null
> $ pkill tcpdump
> $ tshark -tad -nr /tmp/capture.pcap -Y portmap -T fields -e _ws.col.Info
> V4 GETADDR Call
> V4 GETADDR Reply (Call In 4)
> V2 GETPORT Call MOUNT(100005) V:0 TCP
> V2 GETPORT Reply (Call In 14) Port:20048
>
> Signed-off-by: Roberto Bergantinos Corpas <[email protected]>
Committed... (tag libtirpc-1-3-3-rc3)

steved.
> ---
> man/rpcbind.3t | 2 ++
> src/rpcb_clnt.c | 27 ++++++++++++++++++++++++---
> tirpc/rpc/pmap_prot.h | 2 ++
> 3 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/man/rpcbind.3t b/man/rpcbind.3t
> index ec492cc..4cb271b 100644
> --- a/man/rpcbind.3t
> +++ b/man/rpcbind.3t
> @@ -187,6 +187,8 @@ in
> .El
> .Sh AVAILABILITY
> These functions are part of libtirpc.
> +.Sh ENVIRONMENT
> +If RPCB_V2FIRST is defined, rpcbind protocol version tryout algorithm changes from v4,v2,v3 to v2,v4,v3.
> .Sh SEE ALSO
> .Xr rpc_clnt_calls 3 ,
> .Xr rpc_svc_calls 3 ,
> diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
> index 0c34cb7..db3799e 100644
> --- a/src/rpcb_clnt.c
> +++ b/src/rpcb_clnt.c
> @@ -818,7 +818,8 @@ error:
> * The algorithm used: If the transports is TCP or UDP, it first tries
> * version 4 (srv4), then 3 and then fall back to version 2 (portmap).
> * With this algorithm, we get performance as well as a plan for
> - * obsoleting version 2.
> + * obsoleting version 2. This behaviour is reverted to old algorithm
> + * if RPCB_V2FIRST environment var is defined
> *
> * For all other transports, the algorithm remains as 4 and then 3.
> *
> @@ -839,6 +840,10 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
> #ifdef NOTUSED
> static bool_t check_rpcbind = TRUE;
> #endif
> +
> +#ifdef PORTMAP
> + static bool_t portmap_first = FALSE;
> +#endif
> CLIENT *client = NULL;
> RPCB parms;
> enum clnt_stat clnt_st;
> @@ -895,8 +900,18 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
> parms.r_addr = (char *) &nullstring[0];
> }
>
> - /* First try from start_vers(4) and then version 3 (RPCBVERS) */
> + /* First try from start_vers(4) and then version 3 (RPCBVERS), except
> + * if env. var RPCB_V2FIRST is defined */
> +
> +#ifdef PORTMAP
> + if (getenv(V2FIRST)) {
> + portmap_first = TRUE;
> + LIBTIRPC_DEBUG(3, ("__rpcb_findaddr_timed: trying v2-port first\n"));
> + goto portmap;
> + }
> +#endif
>
> +rpcbind:
> CLNT_CONTROL(client, CLSET_RETRY_TIMEOUT, (char *) &rpcbrmttime);
> for (vers = start_vers; vers >= RPCBVERS; vers--) {
> /* Set the version */
> @@ -944,10 +959,16 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
> }
>
> #ifdef PORTMAP /* Try version 2 for TCP or UDP */
> + if (portmap_first)
> + goto error; /* we tried all versions if reached here */
> +portmap:
> if (strcmp(nconf->nc_protofmly, NC_INET) == 0) {
> address = __try_protocol_version_2(program, version, nconf, host, tp);
> if (address == NULL)
> - goto error;
> + if (portmap_first)
> + goto rpcbind;
> + else
> + goto error;
> }
> #endif /* PORTMAP */
>
> diff --git a/tirpc/rpc/pmap_prot.h b/tirpc/rpc/pmap_prot.h
> index 75354ce..7718b8b 100644
> --- a/tirpc/rpc/pmap_prot.h
> +++ b/tirpc/rpc/pmap_prot.h
> @@ -84,6 +84,8 @@
> #define PMAPPROC_DUMP ((u_long)4)
> #define PMAPPROC_CALLIT ((u_long)5)
>
> +#define V2FIRST "RPCB_V2FIRST"
> +
> struct pmap {
> long unsigned pm_prog;
> long unsigned pm_vers;