Bug 481422

This message is a reminder that Fedora 10 is nearing its end of life.
Approximately 30 (thirty) days from now Fedora will stop maintaining
and issuing updates for Fedora 10.  It is Fedora's policy to close all
bug reports from releases that are no longer maintained.  At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '10'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 10's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 10 is end of life.  If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora please change the 'version' of this 
bug to the applicable version.  If you are unable to change the version, 
please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events.  Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

The source code in src/security.c is the same in version 0.2.0 in Fedora 12 as it was in version 0.1.7 in Fedora 10.  So the bug is still there.

(In reply to comment #0)
> Description of problem:
> By default in Fedora 10, rpcbind runs with no options.  In this mode,
> it denies set/unset requests unless the is_loopback function returns
> true.  But is_loopback does more than its name would suggest: it
> requires that the call arrive from a loopback IP address and from
> a reserved port.  This means that programs not running as root are
> unable to register services with rpcbind.  But if we add the -i option
> to rpcbind, then it allows set/unset calls from remote hosts in
> addition to non-root local users.
> 
> 
> Version-Release number of selected component (if applicable):
> 0.1.7-1.fc10
> 
> How reproducible:
> Allow rpcbind to start normally without options, and then try to
> run a program as a non-root user that calls pmap_set.  The call
> will fail.
> 
> Steps to Reproduce:
> 1. Run rpcbind without any options.
> 2. As a non-root user, run a program that calls pmap_set
> 3.
> 
> Actual results:
> pmap_set returns 0 for failure and fails to register the service.
> 
> Expected results:
> pmap_set should succeed in registering the service.
> 
> Additional info:
> The man page claims that the -i option is just to allow access
> from remote hosts, and that recompiling code should solve the
> problem.  But in fact, recompiling the code does not solve the
> problem for non-root users, since the current glibc pmap_set
> call will still fail.  At the very least, the man page should
> be corrected to indicate what -i actually does and that
> recompilation will not solve the problem for non-root users.
> A better solution would be to break the -i option into two
> finer-grained options, one to allow SET/UNSET from remote hosts,
> and another to allow SET/UNSET from non-reserved ports accessible
> to non-root users.
The '-i' flag has nothing to do with root and non-root users.
All that flag does is allow request from another network interface
other than the loopback interface. Basically what the man page
says.... 

Now if the glibc clnt_call() does not allow non-root calls, that's
not a rpcbind problem... Open a bz against glibc... but I would 
suggested try using the cln_call() in libtirpc.

I respectfully disagree.  If you look in rpcbind.c:main, you will see
that the -i flag results in setting insecure = 1.  And if you then grep
for where the insecure flag is reference, you will see this in
security.c:check_access:

                if (!insecure && !is_loopback(caller)) {
                      <deny the request>
                }

And if you look in the is_loopback function in the same file, you will see this:

                return ((sin->sin_addr.s_addr == htonl(INADDR_LOOPBACK)) &&
                    (ntohs(sin->sin_port) < IPPORT_RESERVED));

In other words, if -i is not specified, then requests are accepted only
from the local host and from a reserved port.   Since reserved ports are
available only to root, this means that non-root programs are unable
to register services with rpcbind, even when run on the local host.

The man page says that the purpose of the "-i" flag is to allow calls
from any host.  It does not mention anything about reserved ports.

Turning on -i to enable local programs to register services is a security
hole -- there is no need to open this up to remote programs.

Did you read my problem report carefully?   It's been almost 2 years now...

Regards,
Andy

(In reply to comment #4)
> I respectfully disagree.  If you look in rpcbind.c:main, you will see
> that the -i flag results in setting insecure = 1.  And if you then grep
> for where the insecure flag is reference, you will see this in
> security.c:check_access:
> 
>                 if (!insecure && !is_loopback(caller)) {
>                       <deny the request>
>                 }
> 
> And if you look in the is_loopback function in the same file, you will see
> this:
> 
>                 return ((sin->sin_addr.s_addr == htonl(INADDR_LOOPBACK)) &&
>                     (ntohs(sin->sin_port) < IPPORT_RESERVED));
> 
> In other words, if -i is not specified, then requests are accepted only
> from the local host and from a reserved port.   Since reserved ports are
> available only to root, this means that non-root programs are unable
> to register services with rpcbind, even when run on the local host.
Yes.. I do now see you point... 

> 
> The man page says that the purpose of the "-i" flag is to allow calls
> from any host.  It does not mention anything about reserved ports.
True...

> 
> Turning on -i to enable local programs to register services is a security
> hole -- there is no need to open this up to remote programs.
I agree... But it sounds its there for backward compatibility...
  
> 
> Did you read my problem report carefully? 
Obviously not close enough.. ;-) So in your description you're suggesting
having two options. One to allow local non-root set/unsets and the other
to allow remote machines do sets/unsets... As you have already pointed
out the latter is a huge security hole... So I'm thinking it would 
make sense to deprecate the -i flag. Then possibly introduce a new
-I flag allowing non-root sets/unsets...

Looking at other implementations, these options do not exist, which
could be an argument for doing neither...    

Just curious, why do you need non-root sets/unsets?

Thank you for your patiences....

The easiest thing to do is simply update the man page to
more accurate:

diff --git a/man/rpcbind.8 b/man/rpcbind.8
index c5b8fb7..3686969 100644
--- a/man/rpcbind.8
+++ b/man/rpcbind.8
@@ -116,7 +116,8 @@ Normally
 accepts these requests only from the loopback interface for security reasons.
 This change is necessary for programs that were compiled with earlier
 versions of the rpc library and do not make those requests using the
-loopback interface.
+loopback interface. Note, the requests must come from a local process
+since connection has to bound on a privileged port.
 .It Fl l
 Turn on libwrap connection logging.
 .It Fl s

If the goal is merely to correct the man page, I'd say this language is
more accurate:

   In the absence of the -i option, calls are accepted only from privileged
   ports via the loopback interface.

But as you agreed, -i is a big security hole for normal usage cases where simply
allowing access to local, but non-superuser, processes is needed.

I have many such programs on my system.  We used the SunOS portmapper heavily
back in the 90s to advertise local services.  I suppose nobody else uses
rpcbind any more, otherwise somebody else would have complained.

If I were to supply a patch for the code to add a new -I flag, would it be
considered? Or is it a waste of time?

Regards,
Andy

(In reply to comment #7)
> If the goal is merely to correct the man page, I'd say this language is
> more accurate:
> 
>    In the absence of the -i option, calls are accepted only from privileged
>    ports via the loopback interface.
Sounds fine... 

> 
> But as you agreed, -i is a big security hole for normal usage cases where
> simply
> allowing access to local, but non-superuser, processes is needed.
> 
> I have many such programs on my system.  We used the SunOS portmapper heavily
> back in the 90s to advertise local services.  I suppose nobody else uses
> rpcbind any more, otherwise somebody else would have complained.
> 
> If I were to supply a patch for the code to add a new -I flag, would it be
> considered? Or is it a waste of time?
No guarantees... but there I know there has been some talk about this
in the past... I think it would be a good discussion to have... 

If you are so incline.. Post the patches to linux-nfs.org and
lets start the discussion....

Created attachment 467840 [details]
Add new -c, -r, and -u options to allow fine-tuning of security holes.

This patch adds -c, -r, and -u options to break down the functionality currently provided by the -i option.  This allows the user to choose how much "insecurity" to introduce.

Created attachment 467841 [details]
Patch the spec file to apply the new security patch

To be used in conjunction with the security patch to build the new rpm.

I think this upstream patch is what you are looking for

commit 1d9fba5b631b517094c85a80f45f6f7ba1665e2a
Author: Olaf Kirch <okir>
Date:   Wed Mar 16 14:19:37 2011 -0400

    Make is_loopback check more permissive
    
    This patch relaxes the is_loopback() check to its original meaning;
    i.e. verify that the caller is local. We no longer check whether
    the source port is privileged, for a number of reasons.
    
    1) The existing check did not allow *any* non-root program to register
       a services via UDP or TCP transport. It did however allow
       *any* registration when using the AF_LOCAL transport.
    
    2) Unregistration of services is only possible if the caller has
       the same "user name", i.e.
        "superuser"
                for root (when connecting through AF_LOCAL sockets,
                or when using pmap_set with a privileged port)
        numeric uid
                for non-root users when connecting through AF_LOCAL
                sockets
        "unknown"
                for all other users
    
    This seems safe enough to allow the removal of the privileged port
    check in is_localhost.
    
    Signed-off-by: Olaf Kirch <okir>
    Signed-off-by: Steve Dickson <steved>

diff --git a/src/security.c b/src/security.c
index 07c8933..d272f74 100644
--- a/src/security.c
+++ b/src/security.c
@@ -138,8 +138,7 @@ is_loopback(struct netbuf *nbuf)
                                  "Checking caller's adress (port = %d)\n",
                                  ntohs(sin->sin_port));
 #endif
-               return ((sin->sin_addr.s_addr == htonl(INADDR_LOOPBACK)) &&
-                   (ntohs(sin->sin_port) < IPPORT_RESERVED));
+               return (sin->sin_addr.s_addr == htonl(INADDR_LOOPBACK));
 #ifdef INET6
        case AF_INET6:
                if (!oldstyle_local)
@@ -151,10 +150,9 @@ is_loopback(struct netbuf *nbuf)
                                  "Checking caller's adress (port = %d)\n",
                                  ntohs(sin6->sin6_port));
 #endif
-               return ((IN6_IS_ADDR_LOOPBACK(&sin6->sin6_addr) ||
+               return (IN6_IS_ADDR_LOOPBACK(&sin6->sin6_addr) ||
                         (IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr) &&
-                         sin6->sin6_addr.s6_addr32[3] == htonl(INADDR_LOOPBACK)
-                       (ntohs(sin6->sin6_port) < IPV6PORT_RESERVED));
+                         sin6->sin6_addr.s6_addr32[3] == htonl(INADDR_LOOPBACK)
 #endif
        case AF_LOCAL:
                return 1;

(In reply to comment #11)
> I think this upstream patch is what you are looking for
> 
> commit 1d9fba5b631b517094c85a80f45f6f7ba1665e2a
> Author: Olaf Kirch <okir>
> Date:   Wed Mar 16 14:19:37 2011 -0400
> 
>     Make is_loopback check more permissive

This patch went in with the 

* Thu Mar 17 2011 Steve Dickson <steved> - 0.2.0-11
- Updated to the latest upstream release: rpcbind-0_2_1-rc3