rpcb_getaddr: Always do PMAP_GETPORT first for NC_INET transports

For PF_INET transports, use PMAP_GETPORT.  This is what network
captures show that Solaris user space does, what the Linux mount.nfs
command does, and what the Linux kernel rpcbind client does.  It's
more efficient when querying legacy hosts, of which there are still
many.

An additional benefit of this change is that since libtirpc uses
only UDP for PMAP_GETPORT requests, and it now tries PMAP_GETPORT
first, the creation of a TCP socket in a very common case is now
avoided.  This reduces the consumption of ephemeral ports that
can be left in TIME_WAIT after a request.

It appears that the order in which __rpcb_findaddr_timed() tries the
rpcbind protocol versions was changed in the FreeBSD port of libtirpc.
The documenting comment that appears before __rpcb_findaddr_timed()
was never updated.  This patch restores the original behavior, which
is "try v2 first if the remote is a PF_INET; then try v4, then v3."

The FreeBSD change introduced two bugs: one serious, the other
harmless but wasteful.

The PORTMAP logic overwrites the pointer in "client" instead of
invoking CLNT_DESTROY().  Since the portmap code was originally
executed first, it had no need to invoke CLNT_DESTROY().

This orphans the RPC client previously created for the v3/v4 query.
If a connection-oriented socket was used during the v3/v4 query,
this socket is left connected to the remote portmapper, resulting in
an inadvertant denial of service attack on the remote.

For short-lived programs, this bug is hidden, because a program
exit causes all sockets to be closed automatically.  Long-running
programs leave these sockets connected indefinitely.

The harmless bug is that even though a portmapper replies to a v4
RPCB_GETADDR request with "version 4 not supported; use only version
2", libtirpc tries again with a v3 RPCB_GETADDR anyway.  Though
harmless, this is obviously a wasted round trip.  Perform these
requests in the original order (v2, then v4, then v3), then the
original code makes sense, and the extra v3 round trip is avoided.

Reverting the FreeBSD change fixes both problems.

Reported-by: Jens-Uwe Mozdzen <jmozdzen@nde.ag>.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
index 64943707bf7670648be65f9fcc3c6a317843853b..e8a5d27212c0667c761574dc392067c18d83149e 100644 (file)
--- a/src/rpcb_clnt.c
+++ b/src/rpcb_clnt.c
@@ -731,16 +731,6 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
        }
 
        parms.r_addr = NULL;
-       parms.r_prog = program;
-       parms.r_vers = version;
-       parms.r_netid = nconf->nc_netid;
-
-       /*
-        * rpcbind ignores the r_owner field in GETADDR requests, but we
-        * need to give xdr_rpcb something to gnaw on. Might as well make
-        * it something human readable for when we see these in captures.
-        */
-       parms.r_owner = RPCB_OWNER_STRING;
 
        /*
         * Use default total timeout if no timeout is specified.
@@ -748,73 +738,6 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
        if (tp == NULL)
                tp = &tottimeout;
        
-       /* try rpcbind */
-       /* Now the same transport is to be used to get the address */
-       if (client && ((nconf->nc_semantics == NC_TPI_COTS_ORD) ||
-                       (nconf->nc_semantics == NC_TPI_COTS))) {
-               /* A CLTS type of client - destroy it */
-               CLNT_DESTROY(client);
-               client = NULL;
-       }
-
-       if (client == NULL) {
-               client = getclnthandle(host, nconf, &parms.r_addr);
-               if (client == NULL) {
-                       goto error;
-               }
-       }
-       if (parms.r_addr == NULL) {
-               /*LINTED const castaway*/
-               parms.r_addr = (char *) &nullstring[0];
-       }
-
-       /* First try from start_vers(4) and then version 3 (RPCBVERS) */
-
-       CLNT_CONTROL(client, CLSET_RETRY_TIMEOUT, (char *) &rpcbrmttime);
-       for (vers = start_vers;  vers >= RPCBVERS; vers--) {
-               /* Set the version */
-               CLNT_CONTROL(client, CLSET_VERS, (char *)(void *)&vers);
-               clnt_st = CLNT_CALL(client, (rpcproc_t)RPCBPROC_GETADDR,
-                   (xdrproc_t) xdr_rpcb, (char *)(void *)&parms,
-                   (xdrproc_t) xdr_wrapstring, (char *)(void *) &ua, *tp);
-               if (clnt_st == RPC_SUCCESS) {
-                       if ((ua == NULL) || (ua[0] == 0)) {
-                               /* address unknown */
-                               rpc_createerr.cf_stat = RPC_PROGNOTREGISTERED;
-                               goto error;
-                       }
-                       address = uaddr2taddr(nconf, ua);
-#ifdef ND_DEBUG
-                       fprintf(stderr, "\tRemote address is [%s]\n", ua);
-                       if (!address)
-                               fprintf(stderr,
-                                       "\tCouldn't resolve remote address!\n");
-#endif
-                       xdr_free((xdrproc_t)xdr_wrapstring,
-                           (char *)(void *)&ua);
-
-                       if (! address) {
-                               /* We don't know about your universal address */
-                               rpc_createerr.cf_stat = RPC_N2AXLATEFAILURE;
-                               goto error;
-                       }
-                       CLNT_CONTROL(client, CLGET_SVC_ADDR,
-                           (char *)(void *)&servaddr);
-                       __rpc_fixup_addr(address, &servaddr);
-                       goto done;
-               } else if (clnt_st == RPC_PROGVERSMISMATCH) {
-                       struct rpc_err rpcerr;
-                       clnt_geterr(client, &rpcerr);
-                       if (rpcerr.re_vers.low > RPCBVERS4)
-                               goto error;  /* a new version, can't handle */
-               } else if (clnt_st != RPC_PROGUNAVAIL) {
-                       /* Cant handle this error */
-                       rpc_createerr.cf_stat = clnt_st;
-                       clnt_geterr(client, &rpc_createerr.cf_error);
-                       goto error;
-               }
-       }
-       /* if rpcbind requests failed -> try portmapper version 2 */
 #ifdef PORTMAP
        /* Try version 2 for TCP or UDP */
        if (strcmp(nconf->nc_protofmly, NC_INET) == 0) {
@@ -860,7 +783,7 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
                if (clnt_st != RPC_SUCCESS) {
                        if ((clnt_st == RPC_PROGVERSMISMATCH) ||
                                (clnt_st == RPC_PROGUNAVAIL))
-                               goto error; /* All portmap/rpcbind versions failed */
+                               goto try_rpcbind;
                        rpc_createerr.cf_stat = RPC_PMAPFAILURE;
                        clnt_geterr(client, &rpc_createerr.cf_error);
                        goto error;
@@ -890,10 +813,88 @@ __rpcb_findaddr_timed(program, version, nconf, host, clpp, tp)
                goto done;
        }
 
-
-       //try_rpcbind:
+try_rpcbind:
 #endif                         /* PORTMAP */
 
+       parms.r_prog = program;
+       parms.r_vers = version;
+       parms.r_netid = nconf->nc_netid;
+
+       /*
+        * rpcbind ignores the r_owner field in GETADDR requests, but we
+        * need to give xdr_rpcb something to gnaw on. Might as well make
+        * it something human readable for when we see these in captures.
+        */
+       parms.r_owner = RPCB_OWNER_STRING;
+
+       /* Now the same transport is to be used to get the address */
+       if (client && ((nconf->nc_semantics == NC_TPI_COTS_ORD) ||
+                       (nconf->nc_semantics == NC_TPI_COTS))) {
+               /* A CLTS type of client - destroy it */
+               CLNT_DESTROY(client);
+               client = NULL;
+               free(parms.r_addr);
+               parms.r_addr = NULL;
+       }
+
+       if (client == NULL) {
+               client = getclnthandle(host, nconf, &parms.r_addr);
+               if (client == NULL) {
+                       goto error;
+               }
+       }
+       if (parms.r_addr == NULL) {
+               /*LINTED const castaway*/
+               parms.r_addr = (char *) &nullstring[0];
+       }
+
+       /* First try from start_vers(4) and then version 3 (RPCBVERS) */
+
+       CLNT_CONTROL(client, CLSET_RETRY_TIMEOUT, (char *) &rpcbrmttime);
+       for (vers = start_vers;  vers >= RPCBVERS; vers--) {
+               /* Set the version */
+               CLNT_CONTROL(client, CLSET_VERS, (char *)(void *)&vers);
+               clnt_st = CLNT_CALL(client, (rpcproc_t)RPCBPROC_GETADDR,
+                   (xdrproc_t) xdr_rpcb, (char *)(void *)&parms,
+                   (xdrproc_t) xdr_wrapstring, (char *)(void *) &ua, *tp);
+               if (clnt_st == RPC_SUCCESS) {
+                       if ((ua == NULL) || (ua[0] == 0)) {
+                               /* address unknown */
+                               rpc_createerr.cf_stat = RPC_PROGNOTREGISTERED;
+                               goto error;
+                       }
+                       address = uaddr2taddr(nconf, ua);
+#ifdef ND_DEBUG
+                       fprintf(stderr, "\tRemote address is [%s]\n", ua);
+                       if (!address)
+                               fprintf(stderr,
+                                       "\tCouldn't resolve remote address!\n");
+#endif
+                       xdr_free((xdrproc_t)xdr_wrapstring,
+                           (char *)(void *)&ua);
+
+                       if (! address) {
+                               /* We don't know about your universal address */
+                               rpc_createerr.cf_stat = RPC_N2AXLATEFAILURE;
+                               goto error;
+                       }
+                       CLNT_CONTROL(client, CLGET_SVC_ADDR,
+                           (char *)(void *)&servaddr);
+                       __rpc_fixup_addr(address, &servaddr);
+                       goto done;
+               } else if (clnt_st == RPC_PROGVERSMISMATCH) {
+                       struct rpc_err rpcerr;
+                       clnt_geterr(client, &rpcerr);
+                       if (rpcerr.re_vers.low > RPCBVERS4)
+                               goto error;  /* a new version, can't handle */
+               } else if (clnt_st != RPC_PROGUNAVAIL) {
+                       /* Cant handle this error */
+                       rpc_createerr.cf_stat = clnt_st;
+                       clnt_geterr(client, &rpc_createerr.cf_error);
+                       goto error;
+               }
+       }
+
        if ((address == NULL) || (address->len == 0)) {
          rpc_createerr.cf_stat = RPC_PROGNOTREGISTERED;
          clnt_geterr(client, &rpc_createerr.cf_error);