2010-08-30 13:05:06

by Olaf Kirch

[permalink] [raw]
Subject: [PATCH 0/2] Make libtirpc work with old style portmapper


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)


2010-08-30 15:59:18

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH 0/2] Make libtirpc work with old style portmapper


On Aug 30, 2010, at 9:03 AM, Olaf Kirch wrote:

>=20
> Hi Steve et al,
>=20
> 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.
>=20
> 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.
>=20
> I looked into the issue and put together the following two patches,
> which I'm submitting for your kindly review.

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.


> Thanks
> Olaf
> --=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)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--=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

2010-08-30 16:19:21

by Olaf Kirch

[permalink] [raw]
Subject: Re: [PATCH 0/2] Make libtirpc work with old style portmapper

On Monday 30 August 2010 17:59:18 Chuck Lever wrote:
> 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
> authenticated, but pmap uses sockets with privileged ports to detect
> authorized users. I see that your logic uses the pmap SET/UNSET calls by
> default. This bypasses AF_UNIX completely in pretty much all local cases,

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.

> which changes the behavior of rpcb_set() and rpcb_unset(), and could break
> the local rpcbind security model. It might be better to use
> pmap_setunset() only when local_rpcb() fails.

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.

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

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)

2010-08-30 13:05:07

by Olaf Kirch

[permalink] [raw]
Subject: [PATCH 1/2] Introduce new helper function getpmaphandle


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.

Signed-off-by: Olaf Kirch <[email protected]>
---
src/rpcb_clnt.c | 62 ++++++++++++++++++++++++++++++++++++++----------------
1 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
index a800128..531c619 100644
--- a/src/rpcb_clnt.c
+++ b/src/rpcb_clnt.c
@@ -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)

2010-08-30 23:48:02

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH 0/2] Make libtirpc work with old style portmapper


On Aug 30, 2010, at 12:19 PM, Olaf Kirch wrote:

> On Monday 30 August 2010 17:59:18 Chuck Lever wrote:
>> Another minor problem I think I remember is that if libtirpc is used on a
>> system (perhaps because it is statically linked with said ISV RPC-enabled
>> application) that does not have /etc/netconfig installed, the transport
>> creation logic in rpcb_clnt.c simply doesn't work.
>
> Well, but that's something that's fixed easily - we can always tell
> such customer to install an /etc/netconfig on their system.

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

2010-08-30 13:04:57

by Olaf Kirch

[permalink] [raw]
Subject: [PATCH 2/2] Introduce new helper function getpmaphandle


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.

Signed-off-by: Olaf Kirch <[email protected]>
---
src/rpcb_clnt.c | 70=20
+++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
index 531c619..021525b 100644
--- a/src/rpcb_clnt.c
+++ b/src/rpcb_clnt.c
@@ -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)-
>sin_port);
+ }
+
+ 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

2010-09-07 20:58:24

by Olaf Kirch

[permalink] [raw]
Subject: Re: [PATCH 0/2] Make libtirpc work with old style portmapper

On Tuesday 07 September 2010 17:36:58 Chuck Lever wrote:
> 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?

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

> 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.

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.

> 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.

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)

2010-09-07 15:39:08

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH 0/2] Make libtirpc work with old style portmapper


On Sep 7, 2010, at 7:27 AM, Olaf Kirch wrote:

> On Tuesday 31 August 2010 01:48:02 Chuck Lever wrote:
>> On Aug 30, 2010, at 12:19 PM, Olaf Kirch wrote:
>>> On Monday 30 August 2010 17:59:18 Chuck Lever wrote:
>>>> Another minor problem I think I remember is that if libtirpc is used on
>>>> a system (perhaps because it is statically linked with said ISV
>>>> RPC-enabled application) that does not have /etc/netconfig installed,
>>>> the transport creation logic in rpcb_clnt.c simply doesn't work.
>>>
>>> Well, but that's something that's fixed easily - we can always tell
>>> such customer to install an /etc/netconfig on their system.
>>
>> Agreed, but for various reasons, the symptoms don't necessarily immediately
>> imply what is needed to correct the problem.
>
> I've updated the second patch, which now modifies pmap_set/unset
> directly, without messing with the functionality of rpbc_set/unset.

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?

> With this change, the compatibility functions (pmap_{,un}set) will
> first try the old API and then fall through to the new rpcbind
> interface.

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.

> 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]>
>
> diff --git a/src/pmap_clnt.c b/src/pmap_clnt.c
> index 1d5d153..24cf94a 100644
> --- a/src/pmap_clnt.c
> +++ b/src/pmap_clnt.c
> @@ -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,
> diff --git a/src/rpc_com.h b/src/rpc_com.h
> index 38c2cfe..0a20a01 100644
> --- a/src/rpc_com.h
> +++ b/src/rpc_com.h
> @@ -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;
> diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
> index 531c619..9562646 100644
> --- a/src/rpcb_clnt.c
> +++ b/src/rpcb_clnt.c
> @@ -476,6 +476,55 @@ getpmaphandle(nconf, hostname, tgtaddr)
> #define IN4_LOCALHOST_STRING "127.0.0.1"
> #define IN6_LOCALHOST_STRING "::1"
>
> +#ifdef PORTMAP
> +/*
> + * Perform a PMAP_SET or PMAP_UNSET call to the
> + * local rpcbind/portmap service.
> + */
> +bool_t
> +__pmap_set(program, version, protocol, port)
> + rpcprog_t program;
> + rpcvers_t version;
> + int protocol;
> + int port;
> +{
> + CLIENT *client;
> + rpcproc_t pmapproc;
> + struct pmap pmapparms;
> + bool_t rslt = FALSE;
> + enum clnt_stat clnt_st;
> +
> + /* Arguments should already have been checked by caller */
> +
> + pmapproc = port? PMAPPROC_SET : PMAPPROC_UNSET;
> + pmapparms.pm_prog = program;
> + pmapparms.pm_vers = version;
> + pmapparms.pm_prot = protocol;
> + pmapparms.pm_port = port;
> +
> + client = getpmaphandle(NULL, IN4_LOCALHOST_STRING, NULL);
> + if (client == NULL)
> + return (FALSE);
> +
> + clnt_st = CLNT_CALL(client, pmapproc,
> + (xdrproc_t) xdr_pmap, (caddr_t)(void *) &pmapparms,
> + (xdrproc_t) xdr_bool, (caddr_t)(void *) &rslt,
> + tottimeout);
> +
> + if (clnt_st == RPC_SUCCESS)
> + return rslt;
> +
> + if (clnt_st != RPC_PROGVERSMISMATCH &&
> + clnt_st != RPC_PROGUNAVAIL) {
> + rpc_createerr.cf_stat = 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.

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





2010-09-07 21:15:30

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH 0/2] Make libtirpc work with old style portmapper


On Sep 7, 2010, at 4:58 PM, Olaf Kirch wrote:

> On Tuesday 07 September 2010 17:36:58 Chuck Lever wrote:
>> 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?
>
> No, it's a problem with legacy apps calling pmap_set/unset. They simply
> stop working if you use pmap_set + libtirpc + portmap.

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

>> 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.
>
> 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.

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

>> 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.
>
> 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

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





2010-09-07 11:27:10

by Olaf Kirch

[permalink] [raw]
Subject: Re: [PATCH 0/2] Make libtirpc work with old style portmapper

On Tuesday 31 August 2010 01:48:02 Chuck Lever wrote:
> On Aug 30, 2010, at 12:19 PM, Olaf Kirch wrote:
> > On Monday 30 August 2010 17:59:18 Chuck Lever wrote:
> >> Another minor problem I think I remember is that if libtirpc is used on
> >> a system (perhaps because it is statically linked with said ISV
> >> RPC-enabled application) that does not have /etc/netconfig installed,
> >> the transport creation logic in rpcb_clnt.c simply doesn't work.
> >
> > Well, but that's something that's fixed easily - we can always tell
> > such customer to install an /etc/netconfig on their system.
>
> Agreed, but for various reasons, the symptoms don't necessarily immediately
> imply what is needed to correct the problem.

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]>

diff --git a/src/pmap_clnt.c b/src/pmap_clnt.c
index 1d5d153..24cf94a 100644
--- a/src/pmap_clnt.c
+++ b/src/pmap_clnt.c
@@ -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,
diff --git a/src/rpc_com.h b/src/rpc_com.h
index 38c2cfe..0a20a01 100644
--- a/src/rpc_com.h
+++ b/src/rpc_com.h
@@ -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;
diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
index 531c619..9562646 100644
--- a/src/rpcb_clnt.c
+++ b/src/rpcb_clnt.c
@@ -476,6 +476,55 @@ getpmaphandle(nconf, hostname, tgtaddr)
#define IN4_LOCALHOST_STRING "127.0.0.1"
#define IN6_LOCALHOST_STRING "::1"

+#ifdef PORTMAP
+/*
+ * Perform a PMAP_SET or PMAP_UNSET call to the
+ * local rpcbind/portmap service.
+ */
+bool_t
+__pmap_set(program, version, protocol, port)
+ rpcprog_t program;
+ rpcvers_t version;
+ int protocol;
+ int port;
+{
+ CLIENT *client;
+ rpcproc_t pmapproc;
+ struct pmap pmapparms;
+ bool_t rslt = FALSE;
+ enum clnt_stat clnt_st;
+
+ /* Arguments should already have been checked by caller */
+
+ pmapproc = port? PMAPPROC_SET : PMAPPROC_UNSET;
+ pmapparms.pm_prog = program;
+ pmapparms.pm_vers = version;
+ pmapparms.pm_prot = protocol;
+ pmapparms.pm_port = port;
+
+ client = getpmaphandle(NULL, IN4_LOCALHOST_STRING, NULL);
+ if (client == NULL)
+ return (FALSE);
+
+ clnt_st = CLNT_CALL(client, pmapproc,
+ (xdrproc_t) xdr_pmap, (caddr_t)(void *) &pmapparms,
+ (xdrproc_t) xdr_bool, (caddr_t)(void *) &rslt,
+ tottimeout);
+
+ if (clnt_st == RPC_SUCCESS)
+ return rslt;
+
+ if (clnt_st != RPC_PROGVERSMISMATCH &&
+ clnt_st != RPC_PROGUNAVAIL) {
+ rpc_createerr.cf_stat = 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.