Make libtirpc work with old style portmapper

From: Olaf Kirch <[email protected]>
Date: Mon, 23 Aug 2010 14:36:13 +0200
Subject: [PATCH] pmap_set/unset: allow compat functions to work with old-st= yle=20
portmap

This change fixes a bug when running applications compiled against libtirpc on a host with old-style portmap. Without this change, the pmap_set/pmap_unset compatibility functions will actually be mapped to a RPCB_SET/UNSET call. If the server does not support anything more recent than PMAP, the operations will fail completely.

One choice to fix this problem would have been to reimplement just pmap_set/pmap_unset. The alternative approach, which this patch takes, it to make rpcb_set/unset default to the PMAP protocol when changing an IPv4 registration.

---
src/rpcb_clnt.c | 70=20
+++++++++++++++++++++++++++++++++++++++++++++++++++++++

 1 files changed, 70 insertions(+), 0 deletions(-)

index 531c619..021525b 100644
@@ -476,6 +476,65 @@ getpmaphandle(nconf, hostname, tgtaddr)
 #define IN4_LOCALHOST_STRING	"127.0.0.1"
 #define IN6_LOCALHOST_STRING	"::1"

=20
+#ifdef PORTMAP
+/*
+ * Perform a PMAP_SET or PMAP_UNSET call to the
+ * local rpcbind/portmap service.
+ */
+static bool_t
+pmap_setunset(pmapproc, program, version, nconf, address)
+ rpcproc_t pmapproc;
+ rpcprog_t program;
+ rpcvers_t version;
+ const struct netconfig *nconf; /* Network structure of transport */
+ const struct netbuf *address; /* Services netconfig address */
+{
+ CLIENT *client;
+ struct pmap pmapparms;
+ bool_t rslt =3D FALSE;
+ enum clnt_stat clnt_st;
+
+ if (strcmp(nconf->nc_protofmly, NC_INET) !=3D 0)
+ return (FALSE);
+
+ if (strcmp(nconf->nc_proto, NC_UDP) !=3D 0
+ && strcmp(nconf->nc_proto, NC_TCP) !=3D 0)
+ return (FALSE);
+
+ pmapparms.pm_prog =3D program;
+ pmapparms.pm_vers =3D version;
+ pmapparms.pm_prot =3D strcmp(nconf->nc_proto, NC_TCP) ? IPPROTO_UDP :=20
IPPROTO_TCP;
+ if (pmapproc =3D=3D PMAPPROC_UNSET) {
+ pmapparms.pm_port =3D 0;
+ } else {
+ if (address =3D=3D NULL)
+ return (FALSE);
+ pmapparms.pm_port =3D ntohs(((struct sockaddr_in *) address->buf)-

+ }
+
+ client =3D getpmaphandle(nconf, IN4_LOCALHOST_STRING, NULL);
+ if (client =3D=3D NULL)
+ return (FALSE);
+
+ clnt_st =3D CLNT_CALL(client, pmapproc,

+	    (xdrproc_t) xdr_pmap, (caddr_t)(void *) &pmapparms,
+	    (xdrproc_t) xdr_bool, (caddr_t)(void *) &rslt,
+	    tottimeout);

+
+ if (clnt_st =3D=3D RPC_SUCCESS)
+ return rslt;
+
+ if (clnt_st !=3D RPC_PROGVERSMISMATCH &&
+ clnt_st !=3D RPC_PROGUNAVAIL) {
+ rpc_createerr.cf_stat =3D RPC_PMAPFAILURE;
+ clnt_geterr(client, &rpc_createerr.cf_error);
+ return (FALSE);
+ }
+
+ return (TRUE);
+}
+#endif
+
/*
* This routine will return a client handle that is connected to the local
* rpcbind. Returns NULL on error and free's everything.
@@ -600,6 +659,12 @@ rpcb_set(program, version, nconf, address)
rpc_createerr.cf_stat =3D RPC_UNKNOWNADDR; return (FALSE);
}
+
+#ifdef PORTMAP
+ if (pmap_setunset(PMAPPROC_SET, program, version, nconf, address))
+ return (TRUE);
+#endif
+
client =3D local_rpcb();
if (! client) {
return (FALSE);
@@ -651,6 +716,11 @@ rpcb_unset(program, version, nconf)
RPCB parms;
char uidbuf[32];
=20
+#ifdef PORTMAP
+ if (pmap_setunset(PMAPPROC_UNSET, program, version, nconf, NULL))
+ return (TRUE);
+#endif
+
client =3D local_rpcb();
if (! client) {
return (FALSE);
--=20
1.6.0.2

--=20
Neo didn't bring down the Matrix. SOA did. (soafacts.com)
--------------------------------------------
Olaf Kirch - Director Server ([email protected])
SUSE LINUX Products GmbH, Maxfeldstr. 5, D-90409 N=FCrnberg
GF: Markus Rex, HRB 16746 (AG N=FCrnberg)
_______________________________________________
NOTE: THIS LIST IS DEPRECATED. Please use [email protected] instea= d.
(To subscribe to [email protected]: send "subscribe linux-nfs" in t= he body of a message to [email protected].)

NFSv4 mailing list
[email protected]
http://linux-nfs.org/cgi-bin/mailman/listinfo/nfsv4

Hi Steve et al,

We recently got a bug report from a customer trying to run nfs-utils (which is compiled against libtirpc on SLES 11) on a system with portmapper installed instead of rpcbind. Which failed miserably, because none of the RPC servers was able to register with portmap.

One might argue, if it hurts don't do it, but OTOH this configuration isn't totally outlandish. In particular, ISVs may decide they want to compile an RPC enabled application against libtirpc, but still want it to run on a wide range of Linux versions.

I looked into the issue and put together the following two patches, which I'm submitting for your kindly review.

Thanks
Olaf

Neo didn't bring down the Matrix. SOA did. (soafacts.com)
--------------------------------------------
Olaf Kirch - Director Server ([email protected])
SUSE LINUX Products GmbH, Maxfeldstr. 5, D-90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

From: Olaf Kirch <[email protected]>
Date: Mon, 23 Aug 2010 12:58:41 +0200
Subject: [PATCH] Introduce new helper function getpmaphandle

This moves some code for creation of PMAP handles out of the getaddr code and into a function of its own.

---
 src/rpcb_clnt.c |   62 ++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 43 insertions(+), 19 deletions(-)

index a800128..531c619 100644
@@ -434,6 +434,44 @@ out_err:
 	return (client);
 }
 
+/*
+ * Create a PMAP client handle.
+ */
+static CLIENT *
+getpmaphandle(nconf, hostname, tgtaddr)
+	const struct netconfig *nconf;
+	const char *hostname;
+	char **tgtaddr;
+{
+	CLIENT *client = NULL;
+	rpcvers_t pmapvers = 2;
+
+	/*
+	 * Try UDP only - there are some portmappers out
+	 * there that use UDP only.
+	 */
+	if (nconf == NULL || strcmp(nconf->nc_proto, NC_TCP) == 0) {
+		struct netconfig *newnconf;
+
+		if ((newnconf = getnetconfigent("udp")) == NULL) {
+			rpc_createerr.cf_stat = RPC_UNKNOWNPROTO;
+			return NULL;
+		}
+		client = getclnthandle(hostname, newnconf, tgtaddr);
+		freenetconfigent(newnconf);
+	} else if (strcmp(nconf->nc_proto, NC_UDP) == 0) {
+		if (strcmp(nconf->nc_protofmly, NC_INET) != 0)
+			return NULL;
+		client = getclnthandle(hostname, nconf, tgtaddr);
+	}
+
+	/* Set version */
+	if (client != NULL)
+		CLNT_CONTROL(client, CLSET_VERS, (char *)&pmapvers);
+
+	return client;
+}
+
 /* XXX */
 #define IN4_LOCALHOST_STRING	"127.0.0.1"
 #define IN6_LOCALHOST_STRING	"::1"

@@ -770,34 +808,20 @@ __rpcb_findaddr_timed(program, version, nconf, host, 

clpp, tp)
if (strcmp(nconf->nc_protofmly, NC_INET) == 0) {
u_short port = 0;
struct netbuf remote;
- rpcvers_t pmapvers = 2;
struct pmap pmapparms;

- /*
- * Try UDP only - there are some portmappers out
- * there that use UDP only.
- */
- if (strcmp(nconf->nc_proto, NC_TCP) == 0) {
- struct netconfig *newnconf;
-
- if ((newnconf = getnetconfigent("udp")) == NULL) {
- rpc_createerr.cf_stat = RPC_UNKNOWNPROTO;
- return (NULL);
- }
- client = getclnthandle(host, newnconf, &parms.r_addr);
- freenetconfigent(newnconf);
- } else if (strcmp(nconf->nc_proto, NC_UDP) == 0)
- client = getclnthandle(host, nconf, &parms.r_addr);
- else
+ if (strcmp(nconf->nc_proto, NC_UDP) != 0
+ && strcmp(nconf->nc_proto, NC_TCP) != 0)
goto try_rpcbind;
+
+ client = getpmaphandle(nconf, IN4_LOCALHOST_STRING, &parms.r_addr);
if (client == NULL)
return (NULL);

/*
- * Set version and retry timeout.
+ * Set retry timeout.
*/
CLNT_CONTROL(client, CLSET_RETRY_TIMEOUT, (char *)&rpcbrmttime);
- CLNT_CONTROL(client, CLSET_VERS, (char *)&pmapvers);

pmapparms.pm_prog = program;
pmapparms.pm_vers = version;

1.6.0.2


-- 
Neo didn't bring down the Matrix. SOA did. (soafacts.com)
--------------------------------------------
Olaf Kirch - Director Server ([email protected])
SUSE LINUX Products GmbH, Maxfeldstr. 5, D-90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

I've seen a couple of other requests for this feature, and wrote some patch= es last year that did something similar. I never got around to finishing t= hem.

I worried at the time that this might introduce a security weakness, since,=
after all, the rpcbind SET operation goes over AF_UNIX, which is authentic=
ated, but pmap uses sockets with privileged ports to detect authorized user= s. I see that your logic uses the pmap SET/UNSET calls by default. This b= ypasses AF_UNIX completely in pretty much all local cases, which changes th= e behavior of rpcb_set() and rpcb_unset(), and could break the local rpcbin= d security model. It might be better to use pmap_setunset() only when loca= l_rpcb() fails.

Another minor problem I think I remember is that if libtirpc is used on a s= ystem (perhaps because it is statically linked with said ISV RPC-enabled ap= plication) that does not have /etc/netconfig installed, the transport creat= ion logic in rpcb_clnt.c simply doesn't work.

--=20
chuck[dot]lever[at]oracle[dot]com

_______________________________________________
NOTE: THIS LIST IS DEPRECATED. Please use [email protected] instea= d.
(To subscribe to [email protected]: send "subscribe linux-nfs" in t= he body of a message to [email protected].)

NFSv4 mailing list
[email protected]
http://linux-nfs.org/cgi-bin/mailman/listinfo/nfsv4

That is admittedly a problem, at least for services not running as root. For services running as root, there's no change in behavior when talking to rpcbind - the registration will be owned by the superuser in both cases, because instead of checking the AF_LOCAL credentials for uid 0 it will check for a privileged source port. I agree though, that this part of the patch doesn't leave me totally at ease.

If it helps, I could do the old PMAP calls as a fallback rather than trying these by default, agreed. Let me see what I can come up with tomorrow.

Well, but that's something that's fixed easily - we can always tell such customer to install an /etc/netconfig on their system.

Olaf

Neo didn't bring down the Matrix. SOA did. (soafacts.com)
--------------------------------------------
Olaf Kirch - Director Server ([email protected])
SUSE LINUX Products GmbH, Maxfeldstr. 5, D-90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

Agreed, but for various reasons, the symptoms don't necessarily immediately imply what is needed to correct the problem.

chuck[dot]lever[at]oracle[dot]com




_______________________________________________
NOTE: THIS LIST IS DEPRECATED.  Please use [email protected] instead.
(To subscribe to [email protected]: send "subscribe linux-nfs" in the body of a message to [email protected].)

NFSv4 mailing list
[email protected]
http://linux-nfs.org/cgi-bin/mailman/listinfo/nfsv4

I've updated the second patch, which now modifies pmap_set/unset directly, without messing with the functionality of rpbc_set/unset. With this change, the compatibility functions (pmap_{,un}set) will first try the old API and then fall through to the new rpcbind interface.

Olaf

Neo didn't bring down the Matrix. SOA did. (soafacts.com)
--------------------------------------------
Olaf Kirch - Director Server ([email protected])
SUSE LINUX Products GmbH, Maxfeldstr. 5, D-90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
-----------------------------

commit 30feadb3ae5a8d001aabc44f8ddad44298ec61a2
Author: Olaf Kirch <[email protected]>
Date:   Mon Aug 23 14:36:13 2010 +0200

    pmap_set/unset: allow compat functions to work with old-style portmap
    
    This change fixes a bug when running applications compiled against
    libtirpc on a host with old-style portmap. Without this change, the
    pmap_set/pmap_unset compatibility functions will actually be mapped
    to a RPCB_SET/UNSET call. If the server does not support anything more
    recent than PMAP, the operations will fail completely.
    
    This fix makes pmap_set/unset try the old portmapper functions
    first, and if those fail, try to fall back to the new rpcbind
    interface.
    
    Signed-off-by: Olaf Kirch <[email protected]>

index 1d5d153..24cf94a 100644
@@ -58,6 +58,10 @@ pmap_set(u_long program, u_long version, int protocol, int 

port)
struct netconfig *nconf;
char buf[32];

+#ifdef PORTMAP
+ if (__pmap_set(program, version, protocol, port))
+ return (TRUE);
+#endif
if ((protocol != IPPROTO_UDP) && (protocol != IPPROTO_TCP)) {
return (FALSE);
}
@@ -89,6 +93,11 @@ pmap_unset(u_long program, u_long version)
bool_t udp_rslt = FALSE;
bool_t tcp_rslt = FALSE;

+#ifdef PORTMAP
+ if (__pmap_set(program, version, IPPROTO_UDP, 0)
+ && __pmap_set(program, version, IPPROTO_TCP, 0))
+ return (TRUE);
+#endif
nconf = __rpc_getconfip("udp");
if (nconf != NULL) {
udp_rslt = rpcb_unset((rpcprog_t)program, (rpcvers_t)version,

index 38c2cfe..0a20a01 100644
@@ -86,6 +86,9 @@ bool_t __xdrrec_getrec(XDR *, enum xprt_stat *, bool_t);
 void __xprt_unregister_unlocked(SVCXPRT *);
 void __xprt_set_raddr(SVCXPRT *, const struct sockaddr_storage *);
 
+#ifdef PORTMAP
+bool_t __pmap_set(rpcprog_t, rpcvers_t, int protocol, int port);
+#endif
 
 SVCXPRT **__svc_xports;
 int __svc_maxrec;

Probably not a bad idea. But I thought this was a problem for applications that are calling rpcb_{un,}set(3t), not apps that are invoking the legacy variants. How does this new patch address the problem?

What happens if you try the other way around: try the AF_UNIX mechanism first, then the legacy mechanism? It's arguable that any application calling the pmap_{un,}set(3t) interface would likely expect the old behavior anyway, but could probably benefit from the additional security if rpcbind, rather than portmap, is running locally.

Unless I've misunderstood the problem, you've done exactly the opposite of what needs to be done:

1. rpcb_{un,}set(3t) need to work whether rpcbind or portmapper are running
2. pmap_{un,}set(3t) should still invoke rpcb_{un,}set(3t) under the covers to ensure they get advanced security when available

This is because you can build legacy apps against libtirpc (which want robust pmap_{un,}set(3t) no matter which portmapper is running), but you can also construct TI-RPC enabled apps that may be running on a system with only portmap (which want robust rpcb_{un,}set(3t)). So I think you have to fix both APIs.

chuck[dot]lever[at]oracle[dot]com

No, it's a problem with legacy apps calling pmap_set/unset. They simply stop working if you use pmap_set + libtirpc + portmap.

I don't think apps coded to the old pmap_* interface will benefit in any way from the new security model. These services expect a root/non-root split of privilege, and that's what they get with PMAP_SET/UNSET, no matter whether they talk to rpcbind or portmap.

The latter is a problem indeed, and I guess we need to fix it as well. The one that I wanted to address first and foremost is applications using the legacy API

Olaf

Neo didn't bring down the Matrix. SOA did. (soafacts.com)
--------------------------------------------
Olaf Kirch - Director Server ([email protected])
SUSE LINUX Products GmbH, Maxfeldstr. 5, D-90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

I think the crux of the problem is the latter half: libtirpc + portmap.

I haven't thought carefully about it, but that's probably correct.

Yes, I agree both are a problem. The "rpcb_{un,}set(3t) + TI-RPC-enabled application running on a host with only portmap running" was the problem that was reported to me (twice, separately) a while ago.

chuck[dot]lever[at]oracle[dot]com