diff mbox

Question about random UDP port on rpcbind 0.2.3

Message ID 20180201152918.am75vfq776vtj3i3@tonberry.usersys.redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Scott Mayhew Feb. 1, 2018, 3:29 p.m. UTC
On Wed, 31 Jan 2018, Chuck Lever wrote:

> 
> 
> > On Jan 31, 2018, at 2:57 PM, Steve Dickson <SteveD@RedHat.com> wrote:
> > 
> > 
> > 
> > On 01/31/2018 02:43 PM, Chuck Lever wrote:
> >> 
> >> 
> >>> On Jan 31, 2018, at 2:31 PM, Steve Dickson <SteveD@RedHat.com> wrote:
> >>> 
> >>> 
> >>> 
> >>> On 01/29/2018 01:44 AM, Naruto Nguyen wrote:
> >>>> Hello,
> >>>> 
> >>>> Just would like to add for more information, when I start rpcbind
> >>>> normally, not via systemd, the random UDP is still opened
> >>>> 
> >>>> Could you please share any ideas on this?
> >>> The bound UDP socket is used for remote calls... Where rpcbind
> >>> is asked to make a remote RPC for another caller... 
> >>> 
> >>> Antiquated? yes.. but harmless.
> >> 
> >> Not quite harmless. It can occupy a privileged port that belongs
> >> to a real service.
> > fair enough... 
> > 
> >> 
> >> We should change rpcbind to retain CAP_NET_BIND_SERVICE, so that
> >> it doesn't have to hold onto that port indefinitely. It should
> >> be able to bind to an outgoing privileged port whenever it needs
> >> one.
> > Or we just avoid know ports like sm-notify does.
> 
> statd, you mean. It should also retain CAP_NET_BIND_SERVICE instead
> of what it does now, IMO.
> 
> Note that in both of these cases, that long-lived port is never going
> to be used, going forward: 
> 
> - no one uses the rpcbind port-forward service that I know of
> 
> - NLM is going out of style
> 
> If we can make these two cases on-demand instead, so much the better,
> I say. As Mr. Talpey pointed out recently, the only long-lived
> privileged ports we should see on Linux are well-known service
> listeners, not outgoing ports.

This patch should take care of making rpcbind set up the remote call
port on demand.  I don't have a whole lot of ways to test it though...
just 'rpcinfo -b' and a handful of one-off programs I wrote a while back
trying to mess with the CALLIT/INDIRECT/BCAST procedures.

I'd still need to add the stuff to retain CAP_NET_BIND_SERVICE.

I also like the idea of leaving this off by default and adding a
command-line flag to enable it because I'm also not sure if anyone
actually uses it... not to mention there's been at least one CVE in the
past that exploited it (CVE-2015-7236, not sure if there are others).

-Scott
> 
> A fix for rpcbind might be to add a cmd-line flag to enable the
> rpcbind forwarding service, and have the service default to disabled.
> I'm not sure why rpcbind would list an outgoing port in it's portmap
> menu. Are you sure that this is the forwarding reflector port?
> 
> 
> > steved.
> > 
> >> 
> >> 
> >>> steved.
> >>> 
> >>>> 
> >>>> Brs,
> >>>> Bao
> >>>> 
> >>>> On 27 January 2018 at 19:50, Naruto Nguyen <narutonguyen2018@gmail.com> wrote:
> >>>>> I would like to ask you a question regarding the new random UDP port
> >>>>> in rpcbind 0.2.3.
> >>>>> 
> >>>>> In rpcbind 0.2.3, when I start rpcbind (version 0.2.3) through
> >>>>> rpcbind.service, then I do netstat
> >>>>> 
> >>>>> udp        0      0 0.0.0.0:111             0.0.0.0:*
> >>>>>        10408/rpcbind
> >>>>> udp        0      0 0.0.0.0:831             0.0.0.0:*
> >>>>>        10408/rpcbind
> >>>>> udp6       0      0 :::111                  :::*
> >>>>>        10408/rpcbind
> >>>>> udp6       0      0 :::831                  :::*
> >>>>>        10408/rpcbind
> >>>>> 
> >>>>> The rpcbind does not only listen on port 111 but also on a random udp
> >>>>> port "831" in this case, this port is changed every time the rpcbind
> >>>>> service retstarts. And it listens on 0.0.0.0 so it opens a hole on
> >>>>> security. Could you please let me know what this port is for and is
> >>>>> there any way to avoid that like force it listen on a internal
> >>>>> interface rather than on any interfaces like that? I do not see the
> >>>>> random port on rpcbind 0.2.1, not sure why? As the rpcbind is started
> >>>>> from systemd so "-h" option is invalid as the man page says:
> >>>>> 
> >>>>> 
> >>>>>  -h      Specify specific IP addresses to bind to for UDP requests.
> >>>>> This option may be specified multiple times and can be used to
> >>>>> restrict the interfaces rpcbind will respond to.  Note that when
> >>>>> rpcbind is controlled via sys-
> >>>>>            temd's socket activation, the -h option is ignored. In
> >>>>> this case, you need to edit the ListenStream and ListenDgram
> >>>>> definitions in /usr/lib/systemd/system/rpcbind.socket instead.
> >>>>> 
> >>>>> Thanks a lot,
> >>>>> Brs,
> >>>>> Naruto
> >>>> --
> >>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >>>> the body of a message to majordomo@vger.kernel.org
> >>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>>> 
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >> 
> >> --
> >> Chuck Lever
> 
> --
> Chuck Lever
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
From 317ffdb3cf89375f4624b9f8c0319ae29eaf9ac1 Mon Sep 17 00:00:00 2001
From: Scott Mayhew <smayhew@redhat.com>
Date: Wed, 31 Jan 2018 21:15:14 -0500
Subject: [PATCH] rpcbind: create the remote call plumbing on demand

Rather than squatting on a reserved port indefinitely in order to be
able to service (rarely used) broadcast and/or indirect RPCs, we'll
set up the necessary bits whenever a remote call request comes and and
destroy them when we're done.
---
 src/rpcb_svc_com.c | 111 ++++++++++++++++++++++++++++++++++++-----------------
 src/rpcbind.c      |  19 ---------
 src/rpcbind.h      |   2 +-
 3 files changed, 77 insertions(+), 55 deletions(-)

Comments

Chuck Lever Feb. 1, 2018, 8:15 p.m. UTC | #1
> On Feb 1, 2018, at 10:29 AM, Scott Mayhew <smayhew@redhat.com> wrote:
> 
> On Wed, 31 Jan 2018, Chuck Lever wrote:
> 
>> 
>> 
>>> On Jan 31, 2018, at 2:57 PM, Steve Dickson <SteveD@RedHat.com> wrote:
>>> 
>>> 
>>> 
>>> On 01/31/2018 02:43 PM, Chuck Lever wrote:
>>>> 
>>>> 
>>>>> On Jan 31, 2018, at 2:31 PM, Steve Dickson <SteveD@RedHat.com> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>> On 01/29/2018 01:44 AM, Naruto Nguyen wrote:
>>>>>> Hello,
>>>>>> 
>>>>>> Just would like to add for more information, when I start rpcbind
>>>>>> normally, not via systemd, the random UDP is still opened
>>>>>> 
>>>>>> Could you please share any ideas on this?
>>>>> The bound UDP socket is used for remote calls... Where rpcbind
>>>>> is asked to make a remote RPC for another caller... 
>>>>> 
>>>>> Antiquated? yes.. but harmless.
>>>> 
>>>> Not quite harmless. It can occupy a privileged port that belongs
>>>> to a real service.
>>> fair enough... 
>>> 
>>>> 
>>>> We should change rpcbind to retain CAP_NET_BIND_SERVICE, so that
>>>> it doesn't have to hold onto that port indefinitely. It should
>>>> be able to bind to an outgoing privileged port whenever it needs
>>>> one.
>>> Or we just avoid know ports like sm-notify does.
>> 
>> statd, you mean. It should also retain CAP_NET_BIND_SERVICE instead
>> of what it does now, IMO.
>> 
>> Note that in both of these cases, that long-lived port is never going
>> to be used, going forward: 
>> 
>> - no one uses the rpcbind port-forward service that I know of
>> 
>> - NLM is going out of style
>> 
>> If we can make these two cases on-demand instead, so much the better,
>> I say. As Mr. Talpey pointed out recently, the only long-lived
>> privileged ports we should see on Linux are well-known service
>> listeners, not outgoing ports.
> 
> This patch should take care of making rpcbind set up the remote call
> port on demand.  I don't have a whole lot of ways to test it though...
> just 'rpcinfo -b' and a handful of one-off programs I wrote a while back
> trying to mess with the CALLIT/INDIRECT/BCAST procedures.
> 
> I'd still need to add the stuff to retain CAP_NET_BIND_SERVICE.
> 
> I also like the idea of leaving this off by default and adding a
> command-line flag to enable it because I'm also not sure if anyone
> actually uses it... not to mention there's been at least one CVE in the
> past that exploited it (CVE-2015-7236, not sure if there are others).

I don't see a problem with taking both approaches
(NET_BIND_SERVICE + bindresvport and a new cmdline option)


> -Scott
>> 
>> A fix for rpcbind might be to add a cmd-line flag to enable the
>> rpcbind forwarding service, and have the service default to disabled.
>> I'm not sure why rpcbind would list an outgoing port in it's portmap
>> menu. Are you sure that this is the forwarding reflector port?
>> 
>> 
>>> steved.
>>> 
>>>> 
>>>> 
>>>>> steved.
>>>>> 
>>>>>> 
>>>>>> Brs,
>>>>>> Bao
>>>>>> 
>>>>>> On 27 January 2018 at 19:50, Naruto Nguyen <narutonguyen2018@gmail.com> wrote:
>>>>>>> I would like to ask you a question regarding the new random UDP port
>>>>>>> in rpcbind 0.2.3.
>>>>>>> 
>>>>>>> In rpcbind 0.2.3, when I start rpcbind (version 0.2.3) through
>>>>>>> rpcbind.service, then I do netstat
>>>>>>> 
>>>>>>> udp        0      0 0.0.0.0:111             0.0.0.0:*
>>>>>>>       10408/rpcbind
>>>>>>> udp        0      0 0.0.0.0:831             0.0.0.0:*
>>>>>>>       10408/rpcbind
>>>>>>> udp6       0      0 :::111                  :::*
>>>>>>>       10408/rpcbind
>>>>>>> udp6       0      0 :::831                  :::*
>>>>>>>       10408/rpcbind
>>>>>>> 
>>>>>>> The rpcbind does not only listen on port 111 but also on a random udp
>>>>>>> port "831" in this case, this port is changed every time the rpcbind
>>>>>>> service retstarts. And it listens on 0.0.0.0 so it opens a hole on
>>>>>>> security. Could you please let me know what this port is for and is
>>>>>>> there any way to avoid that like force it listen on a internal
>>>>>>> interface rather than on any interfaces like that? I do not see the
>>>>>>> random port on rpcbind 0.2.1, not sure why? As the rpcbind is started
>>>>>>> from systemd so "-h" option is invalid as the man page says:
>>>>>>> 
>>>>>>> 
>>>>>>> -h      Specify specific IP addresses to bind to for UDP requests.
>>>>>>> This option may be specified multiple times and can be used to
>>>>>>> restrict the interfaces rpcbind will respond to.  Note that when
>>>>>>> rpcbind is controlled via sys-
>>>>>>>           temd's socket activation, the -h option is ignored. In
>>>>>>> this case, you need to edit the ListenStream and ListenDgram
>>>>>>> definitions in /usr/lib/systemd/system/rpcbind.socket instead.
>>>>>>> 
>>>>>>> Thanks a lot,
>>>>>>> Brs,
>>>>>>> Naruto
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>> 
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>> 
>>>> --
>>>> Chuck Lever
>> 
>> --
>> Chuck Lever
>> 
>> 
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> <0001-rpcbind-create-the-remote-call-plumbing-on-demand.patch>

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Dickson Feb. 2, 2018, 3:06 p.m. UTC | #2
On 02/01/2018 10:29 AM, Scott Mayhew wrote:
> 
> This patch should take care of making rpcbind set up the remote call
> port on demand.  I don't have a whole lot of ways to test it though...
> just 'rpcinfo -b' and a handful of one-off programs I wrote a while back
> trying to mess with the CALLIT/INDIRECT/BCAST procedures.
This is where I spent my afternoon yesterday... figuring
out a way to test this code. rpc_call() is my new BFF!

> 
> I'd still need to add the stuff to retain CAP_NET_BIND_SERVICE.
I think we need to do what nsm_clear_capabilities() does.

> 
> I also like the idea of leaving this off by default and adding a
> command-line flag to enable it because I'm also not sure if anyone
> actually uses it... not to mention there's been at least one CVE in the
> past that exploited it (CVE-2015-7236, not sure if there are others).
I'm not a fan of this idea... I think on demand is a better way
to go... but what do I know?? ;-)

steved.

> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever Feb. 2, 2018, 3:22 p.m. UTC | #3
> On Feb 2, 2018, at 10:06 AM, Steve Dickson <SteveD@RedHat.com> wrote:
> 
> 
> 
> On 02/01/2018 10:29 AM, Scott Mayhew wrote:
>> 
>> This patch should take care of making rpcbind set up the remote call
>> port on demand.  I don't have a whole lot of ways to test it though...
>> just 'rpcinfo -b' and a handful of one-off programs I wrote a while back
>> trying to mess with the CALLIT/INDIRECT/BCAST procedures.
> This is where I spent my afternoon yesterday... figuring
> out a way to test this code. rpc_call() is my new BFF!

Thanks!


>> I'd still need to add the stuff to retain CAP_NET_BIND_SERVICE.
> I think we need to do what nsm_clear_capabilities() does.

Agree, that's exactly what is needed to allow bindresvport
on demand, if rpcbind (or statd) does not run as root.

The only issue I see is that some distros might not have
libcap. There is plenty of infrastructure in nsm/file.c that
deals with the "libcap absent" case, which makes me wonder.


>> I also like the idea of leaving this off by default and adding a
>> command-line flag to enable it because I'm also not sure if anyone
>> actually uses it... not to mention there's been at least one CVE in the
>> past that exploited it (CVE-2015-7236, not sure if there are others).
> I'm not a fan of this idea... I think on demand is a better way
> to go... but what do I know?? ;-)
> 
>> I'm not sure why rpcbind would list an outgoing port in it's portmap
>> menu. Are you sure that this is the forwarding reflector port?
>> 
> Yes... the listening fd is created by create_rmtcall_fd()
> This is not the first time people have complained about
> rpbind's rogue listening port :-( 

Hrm. If this is indeed a listening port, on demand bindresvport
won't help (on demand will only fix outgoing ports). I had
assumed there was a long-lived reserved port that was being
used just for outgoing RPCs, just like statd.

Does that listening port have to be in the privileged port range?


--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Dickson Feb. 2, 2018, 4:27 p.m. UTC | #4
On 02/02/2018 10:22 AM, Chuck Lever wrote:
> 
> 
>> On Feb 2, 2018, at 10:06 AM, Steve Dickson <SteveD@RedHat.com> wrote:
>>
>>
>>
>> On 02/01/2018 10:29 AM, Scott Mayhew wrote:
>>>
>>> This patch should take care of making rpcbind set up the remote call
>>> port on demand.  I don't have a whole lot of ways to test it though...
>>> just 'rpcinfo -b' and a handful of one-off programs I wrote a while back
>>> trying to mess with the CALLIT/INDIRECT/BCAST procedures.
>> This is where I spent my afternoon yesterday... figuring
>> out a way to test this code. rpc_call() is my new BFF!
> 
> Thanks!
> 
> 
>>> I'd still need to add the stuff to retain CAP_NET_BIND_SERVICE.
>> I think we need to do what nsm_clear_capabilities() does.
> 
> Agree, that's exactly what is needed to allow bindresvport
> on demand, if rpcbind (or statd) does not run as root.
With Fedora, rpcbind runs as 'rpc' and statd runs as 'rpcuser'
So changing the capabilities would have to happen early on.

> 
> The only issue I see is that some distros might not have
> libcap. There is plenty of infrastructure in nsm/file.c that
> deals with the "libcap absent" case, which makes me wonder.
IDK... Fedora has it... 

> 
> 
>>> I also like the idea of leaving this off by default and adding a
>>> command-line flag to enable it because I'm also not sure if anyone
>>> actually uses it... not to mention there's been at least one CVE in the
>>> past that exploited it (CVE-2015-7236, not sure if there are others).
>> I'm not a fan of this idea... I think on demand is a better way
>> to go... but what do I know?? ;-)
>>
>>> I'm not sure why rpcbind would list an outgoing port in it's portmap
>>> menu. Are you sure that this is the forwarding reflector port?
>>>
>> Yes... the listening fd is created by create_rmtcall_fd()
>> This is not the first time people have complained about
>> rpbind's rogue listening port :-( 
> 
> Hrm. If this is indeed a listening port, on demand bindresvport
> won't help (on demand will only fix outgoing ports). I had
> assumed there was a long-lived reserved port that was being
> used just for outgoing RPCs, just like statd.
No. create_rmtcall_fd() just calls svc_tli_create() which
does the bindresvport()... 

> 
> Does that listening port have to be in the privileged port range?
Taking a quick look at mountd and statd I didn't see any code
requiring a privileged port... Maybe to call into the kernel?

steved.
> 
> 
> --
> Chuck Lever
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Mayhew Feb. 2, 2018, 5 p.m. UTC | #5
On Fri, 02 Feb 2018, Steve Dickson wrote:

> 
> 
> On 02/02/2018 10:22 AM, Chuck Lever wrote:
> > 
> > 
> >> On Feb 2, 2018, at 10:06 AM, Steve Dickson <SteveD@RedHat.com> wrote:
> >>
> >>
> >>
> >> On 02/01/2018 10:29 AM, Scott Mayhew wrote:
> >>>
> >>> This patch should take care of making rpcbind set up the remote call
> >>> port on demand.  I don't have a whole lot of ways to test it though...
> >>> just 'rpcinfo -b' and a handful of one-off programs I wrote a while back
> >>> trying to mess with the CALLIT/INDIRECT/BCAST procedures.
> >> This is where I spent my afternoon yesterday... figuring
> >> out a way to test this code. rpc_call() is my new BFF!
> > 
> > Thanks!
> > 
> > 
> >>> I'd still need to add the stuff to retain CAP_NET_BIND_SERVICE.
> >> I think we need to do what nsm_clear_capabilities() does.

Right, I saw that and was going pretty much copy what it does... I just
got busy bug hunting some other stuff.

> > 
> > Agree, that's exactly what is needed to allow bindresvport
> > on demand, if rpcbind (or statd) does not run as root.
> With Fedora, rpcbind runs as 'rpc' and statd runs as 'rpcuser'
> So changing the capabilities would have to happen early on.

statd calls 'prctl(PR_SET_KEEPCAPS,...', calls setgroups/setgid/setuid,
and then nsm_clear_capabilities right after that.

> 
> > 
> > The only issue I see is that some distros might not have
> > libcap. There is plenty of infrastructure in nsm/file.c that
> > deals with the "libcap absent" case, which makes me wonder.
> IDK... Fedora has it... 

So in the case of libcap being absent, I guess it comes down to who's
actually using the functionality (I've looked a few times in the past
and never really came up with anything), what program they're trying to
call and what procedure they're trying to call... because if we fail to
get a reserved port then we'll do the bind again with a non-reserved
port.  AFAIK non of the programs care if a NULL request comes from a
reserved port or not.  And check_callit() filters out a lot of the other
procedures (e.g. nothing other than NULL is forwarded to nfsd).
> 
> > 
> > 
> >>> I also like the idea of leaving this off by default and adding a
> >>> command-line flag to enable it because I'm also not sure if anyone
> >>> actually uses it... not to mention there's been at least one CVE in the
> >>> past that exploited it (CVE-2015-7236, not sure if there are others).
> >> I'm not a fan of this idea... I think on demand is a better way
> >> to go... but what do I know?? ;-)
> >>
> >>> I'm not sure why rpcbind would list an outgoing port in it's portmap
> >>> menu. Are you sure that this is the forwarding reflector port?
> >>>
> >> Yes... the listening fd is created by create_rmtcall_fd()
> >> This is not the first time people have complained about
> >> rpbind's rogue listening port :-( 
> > 
> > Hrm. If this is indeed a listening port, on demand bindresvport
> > won't help (on demand will only fix outgoing ports). I had
> > assumed there was a long-lived reserved port that was being
> > used just for outgoing RPCs, just like statd.
> No. create_rmtcall_fd() just calls svc_tli_create() which
> does the bindresvport()... 

Right... I was under the impression that was so these requests could be
handled asynchronously.  If rpcbind were to use clnt_call() to forward
the request then I'd think it would be pretty easy to tie up rpcbind,
no?

> 
> > 
> > Does that listening port have to be in the privileged port range?
> Taking a quick look at mountd and statd I didn't see any code
> requiring a privileged port... Maybe to call into the kernel?

statd needs the privileged port to talk to lockd.

> 
> steved.
> > 
> > 
> > --
> > Chuck Lever
> > 
> > 
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever Feb. 2, 2018, 5:42 p.m. UTC | #6
> On Feb 2, 2018, at 12:00 PM, Scott Mayhew <smayhew@redhat.com> wrote:
> 
> On Fri, 02 Feb 2018, Steve Dickson wrote:
> 
>> 
>> 
>> On 02/02/2018 10:22 AM, Chuck Lever wrote:
>>> 
>>> 
>>>> On Feb 2, 2018, at 10:06 AM, Steve Dickson <SteveD@RedHat.com> wrote:
>>>> 
>>>> 
>>>> 
>>>> On 02/01/2018 10:29 AM, Scott Mayhew wrote:
>>>>> 
>>>>> This patch should take care of making rpcbind set up the remote call
>>>>> port on demand.  I don't have a whole lot of ways to test it though...
>>>>> just 'rpcinfo -b' and a handful of one-off programs I wrote a while back
>>>>> trying to mess with the CALLIT/INDIRECT/BCAST procedures.
>>>> This is where I spent my afternoon yesterday... figuring
>>>> out a way to test this code. rpc_call() is my new BFF!
>>> 
>>> Thanks!
>>> 
>>> 
>>>>> I'd still need to add the stuff to retain CAP_NET_BIND_SERVICE.
>>>> I think we need to do what nsm_clear_capabilities() does.
> 
> Right, I saw that and was going pretty much copy what it does... I just
> got busy bug hunting some other stuff.
> 
>>> 
>>> Agree, that's exactly what is needed to allow bindresvport
>>> on demand, if rpcbind (or statd) does not run as root.
>> With Fedora, rpcbind runs as 'rpc' and statd runs as 'rpcuser'
>> So changing the capabilities would have to happen early on.
> 
> statd calls 'prctl(PR_SET_KEEPCAPS,...', calls setgroups/setgid/setuid,
> and then nsm_clear_capabilities right after that.
> 
>> 
>>> 
>>> The only issue I see is that some distros might not have
>>> libcap. There is plenty of infrastructure in nsm/file.c that
>>> deals with the "libcap absent" case, which makes me wonder.
>> IDK... Fedora has it... 
> 
> So in the case of libcap being absent, I guess it comes down to who's
> actually using the functionality (I've looked a few times in the past
> and never really came up with anything), what program they're trying to
> call and what procedure they're trying to call... because if we fail to
> get a reserved port then we'll do the bind again with a non-reserved
> port.  AFAIK non of the programs care if a NULL request comes from a
> reserved port or not.  And check_callit() filters out a lot of the other
> procedures (e.g. nothing other than NULL is forwarded to nfsd).

>>>>> I also like the idea of leaving this off by default and adding a
>>>>> command-line flag to enable it because I'm also not sure if anyone
>>>>> actually uses it... not to mention there's been at least one CVE in the
>>>>> past that exploited it (CVE-2015-7236, not sure if there are others).
>>>> I'm not a fan of this idea... I think on demand is a better way
>>>> to go... but what do I know?? ;-)
>>>> 
>>>>> I'm not sure why rpcbind would list an outgoing port in it's portmap
>>>>> menu. Are you sure that this is the forwarding reflector port?
>>>>> 
>>>> Yes... the listening fd is created by create_rmtcall_fd()
>>>> This is not the first time people have complained about
>>>> rpbind's rogue listening port :-( 
>>> 
>>> Hrm. If this is indeed a listening port, on demand bindresvport
>>> won't help (on demand will only fix outgoing ports). I had
>>> assumed there was a long-lived reserved port that was being
>>> used just for outgoing RPCs, just like statd.
>> No. create_rmtcall_fd() just calls svc_tli_create() which
>> does the bindresvport()... 
> 
> Right... I was under the impression that was so these requests could be
> handled asynchronously.  If rpcbind were to use clnt_call() to forward
> the request then I'd think it would be pretty easy to tie up rpcbind,
> no?

Yes, asynchrony is good. But I don't believe that asynchrony
requires a privileged listener.

IMO it would be better if we could somehow move the CALLIT
listener up into the dynamic port range (above 49151).

In fact that is appropriate for libtirpc to implement for
arbitrary RPC service listeners (including mountd).
svc_tli_create should simply hunt for an available high-numbered
port rather than using bindresvport by default.

_Listeners_ should have no problem using that high port range.

A listener might need to use a well-known port, though, that is
below 1024. In that case, I think svc_tli_create should just use
bind and the library caller needs to have the appropriate
privilege to make it stick.


>>> Does that listening port have to be in the privileged port range?
>> Taking a quick look at mountd and statd I didn't see any code
>> requiring a privileged port... Maybe to call into the kernel?
> 
> statd needs the privileged port to talk to lockd.

Right, and that's an outgoing port, if my memory serves. The
reason statd holds onto this port is because statd was written a
very long time ago before Linux had libcap (I presume).

If instead it could retain CAP_NET_BIND_SERVICE before it dropped
its root privileges, then it could create that socket on demand,
use bindresvport, and then close it immediately.


--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c
index 9c1c3af..3697dac 100644
--- a/src/rpcb_svc_com.c
+++ b/src/rpcb_svc_com.c
@@ -77,6 +77,7 @@  static int rpcb_rmtcalls;
 
 struct rmtcallfd_list {
 	int fd;
+	int nr_rqsts;
 	SVCXPRT *xprt;
 	char *netid;
 	struct rmtcallfd_list *next;
@@ -104,8 +105,9 @@  static bool_t xdr_encap_parms(XDR *, struct encap_parms *);
 static bool_t xdr_rmtcall_args(XDR *, struct r_rmtcall_args *);
 static bool_t xdr_rmtcall_result(XDR *, struct r_rmtcall_args *);
 static bool_t xdr_opaque_parms(XDR *, struct r_rmtcall_args *);
-static int find_rmtcallfd_by_netid(char *);
-static SVCXPRT *find_rmtcallxprt_by_fd(int);
+static struct rmtcallfd_list * find_rmtcallfd_list_by_netid(char *);
+static struct rmtcallfd_list * find_rmtcallfd_list_by_fd(int);
+static void delete_rmtcallfd_list(struct rmtcallfd_list *);
 static int forward_register(u_int32_t, struct netbuf *, int, char *,
     rpcproc_t, rpcvers_t, u_int32_t *);
 static struct finfo *forward_find(u_int32_t);
@@ -117,7 +119,7 @@  static void netbuffree(struct netbuf *);
 static int check_rmtcalls(struct pollfd *, int);
 static void xprt_set_caller(SVCXPRT *, struct finfo *);
 static void send_svcsyserr(SVCXPRT *, struct finfo *);
-static void handle_reply(int, SVCXPRT *);
+static void handle_reply(int, struct rmtcallfd_list *);
 static void find_versions(rpcprog_t, char *, rpcvers_t *, rpcvers_t *);
 static rpcblist_ptr find_service(rpcprog_t, rpcvers_t, char *);
 static char *getowner(SVCXPRT *, char *, size_t);
@@ -498,7 +500,7 @@  xdr_opaque_parms(XDR *xdrs, struct r_rmtcall_args *cap)
 static struct rmtcallfd_list *rmthead;
 static struct rmtcallfd_list *rmttail;
 
-int
+struct rmtcallfd_list *
 create_rmtcall_fd(struct netconfig *nconf)
 {
 	int fd;
@@ -510,24 +512,25 @@  create_rmtcall_fd(struct netconfig *nconf)
 			xlog(LOG_DEBUG,
 	"create_rmtcall_fd: couldn't open \"%s\" (errno %d)\n",
 			nconf->nc_device, errno);
-		return (-1);
+		return (NULL);
 	}
 	xprt = svc_tli_create(fd, 0, (struct t_bind *) 0, 0, 0);
 	if (xprt == NULL) {
 		if (debugging)
 			xlog(LOG_DEBUG,
 				"create_rmtcall_fd: svc_tli_create failed\n");
-		return (-1);
+		return (NULL);
 	}
 	rmt = malloc(sizeof (struct rmtcallfd_list));
 	if (rmt == NULL) {
 		syslog(LOG_ERR, "create_rmtcall_fd: no memory!");
-		return (-1);
+		return (NULL);
 	}
 	rmt->xprt = xprt;
 	rmt->netid = strdup(nconf->nc_netid);
 	xprt->xp_netid = rmt->netid;
 	rmt->fd = fd;
+	rmt->nr_rqsts = 0;
 	rmt->next = NULL;
 	if (rmthead == NULL) {
 		rmthead = rmt;
@@ -536,35 +539,64 @@  create_rmtcall_fd(struct netconfig *nconf)
 		rmttail->next = rmt;
 		rmttail = rmt;
 	}
-	return (fd);
+	return (rmt);
 }
 
-static int
-find_rmtcallfd_by_netid(char *netid)
+static struct rmtcallfd_list *
+find_rmtcallfd_list_by_netid(char *netid)
 {
 	struct rmtcallfd_list *rmt;
 
 	for (rmt = rmthead; rmt != NULL; rmt = rmt->next) {
-		if (strcmp(netid, rmt->netid) == 0) {
-			return (rmt->fd);
-		}
+		if (strlen(netid) != strlen(rmt->netid))
+			continue;
+		if (strcmp(netid, rmt->netid) == 0)
+			break;
 	}
-	return (-1);
+	return (rmt);
 }
 
-static SVCXPRT *
-find_rmtcallxprt_by_fd(int fd)
+static struct rmtcallfd_list *
+find_rmtcallfd_list_by_fd(int fd)
 {
 	struct rmtcallfd_list *rmt;
 
-	for (rmt = rmthead; rmt != NULL; rmt = rmt->next) {
-		if (fd == rmt->fd) {
-			return (rmt->xprt);
+	for (rmt = rmthead; rmt != NULL; rmt = rmt->next)
+		if (fd == rmt->fd)
+			break;
+	return (rmt);
+}
+
+static void
+delete_rmtcallfd_list(struct rmtcallfd_list *to_delete)
+{
+	struct rmtcallfd_list *rmt;
+
+	if (rmthead == to_delete) {
+		rmthead = to_delete->next;
+		if (rmttail == to_delete)
+			rmttail = rmthead;
+	} else {
+		for (rmt = rmthead; rmt->next != NULL; rmt = rmt->next)
+			if (rmt->next == to_delete)
+				break;
+		if (rmt->next != to_delete) {
+#ifdef RMTCALL_DEBUG
+			xlog(LOG_DEBUG,
+				"delete_rmtcallfd_list failed for fd %d, netid %s",
+				to_delete->fd, to_delete->netid);
+#endif
+			return;
 		}
+		rmt->next = to_delete->next;
+		if (rmttail == to_delete)
+			rmttail = rmt;
 	}
-	return (NULL);
-}
 
+	svc_destroy(to_delete->xprt);
+	free(to_delete->netid);
+	free(to_delete);
+}
 
 /*
  * Call a remote procedure service.  This procedure is very quiet when things
@@ -619,13 +651,13 @@  rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 	u_int sendsz;
 	XDR outxdr;
 	AUTH *auth;
-	int fd = -1;
 	char *uaddr, *m_uaddr = NULL, *local_uaddr = NULL;
 	u_int32_t *xidp;
 	struct __rpc_sockinfo si;
 	struct sockaddr *localsa;
 	struct netbuf tbuf;
 	int size = 0;
+	struct rmtcallfd_list *rmt;
 	if (!__rpc_fd2sockinfo(transp->xp_fd, &si)) {
 		if (reply_type == RPCBPROC_INDIRECT)
 			svcerr_systemerr(transp);
@@ -751,13 +783,15 @@  rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 	if (debugging)
 		xlog(LOG_DEBUG, "merged uaddr %s\n", m_uaddr);
 #endif
-	if ((fd = find_rmtcallfd_by_netid(nconf->nc_netid)) == -1) {
-		if (reply_type == RPCBPROC_INDIRECT)
-			svcerr_systemerr(transp);
-		goto error;
+	if ((rmt = find_rmtcallfd_list_by_netid(nconf->nc_netid)) == NULL) {
+		if ((rmt = create_rmtcall_fd(nconf)) == NULL) {
+			if (reply_type == RPCBPROC_INDIRECT)
+				svcerr_systemerr(transp);
+			goto error;
+		}
 	}
 	xidp = __rpcb_get_dg_xidp(transp);
-	switch (forward_register(*xidp, caller, fd, m_uaddr, reply_type,
+	switch (forward_register(*xidp, caller, rmt->fd, m_uaddr, reply_type,
 	    versnum, &call_msg.rm_xid)) {
 	case 1:
 		/* Success; forward_register() will free m_uaddr for us. */
@@ -866,7 +900,7 @@  rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 		goto error;
 	}
 
-	if (sendto(fd, outbuf, outlen, 0, (struct sockaddr *)na->buf, na->len)
+	if (sendto(rmt->fd, outbuf, outlen, 0, (struct sockaddr *)na->buf, na->len)
 	    != outlen) {
 		if (debugging)
 			xlog(LOG_DEBUG,
@@ -875,6 +909,7 @@  rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 			svcerr_systemerr(transp);
 		goto error;
 	}
+	rmt->nr_rqsts++;
 	goto out;
 
 error:
@@ -1098,22 +1133,23 @@  check_rmtcalls(struct pollfd *pfds, int nfds)
 {
 	int j, ncallbacks_found = 0, rmtcalls_pending;
 	SVCXPRT *xprt;
+	struct rmtcallfd_list *rmt;
 
 	if (rpcb_rmtcalls == 0)
 		return (0);
 
 	rmtcalls_pending = rpcb_rmtcalls;
 	for (j = 0; j < nfds; j++) {
-		if ((xprt = find_rmtcallxprt_by_fd(pfds[j].fd)) != NULL) {
+		if ((rmt = find_rmtcallfd_list_by_fd(pfds[j].fd)) != NULL) {
 			if (pfds[j].revents) {
 				ncallbacks_found++;
 #ifdef DEBUG_RMTCALL
 			if (debugging)
 				xlog(LOG_DEBUG,
 "my_svc_run:  polled on forwarding fd %d, netid %s - calling handle_reply\n",
-		pfds[j].fd, xprt->xp_netid);
+		pfds[j].fd, rmt->xprt->xp_netid);
 #endif
-				handle_reply(pfds[j].fd, xprt);
+				handle_reply(pfds[j].fd, rmt);
 				pfds[j].revents = 0;
 				if (ncallbacks_found >= rmtcalls_pending) {
 					break;
@@ -1171,7 +1207,7 @@  send_svcsyserr(SVCXPRT *xprt, struct finfo *fi)
 extern SVCAUTH svc_auth_none;
 
 static void
-handle_reply(int fd, SVCXPRT *xprt)
+handle_reply(int fd, struct rmtcallfd_list *rmt)
 {
 	XDR		reply_xdrs;
 	struct rpc_msg	reply_msg;
@@ -1182,6 +1218,7 @@  handle_reply(int fd, SVCXPRT *xprt)
 	struct r_rmtcall_args a;
 	struct sockaddr_storage ss;
 	socklen_t fromlen;
+	SVCXPRT *xprt = rmt->xprt;
 
 	buffer = malloc(RPC_BUF_MAX);
 	if (buffer == NULL)
@@ -1211,9 +1248,9 @@  handle_reply(int fd, SVCXPRT *xprt)
 		goto done;
 	}
 	fi = forward_find(reply_msg.rm_xid);
-#ifdef	SVC_RUN_DEBUG
+#ifdef	DEBUG_RMTCALL
 	if (debugging) {
-		xlog(LOG_DEBUG, "handle_reply:  reply xid: %d fi addr: %p\n",
+		xlog(LOG_DEBUG, "handle_reply:  reply xid: %x fi addr: %p\n",
 			reply_msg.rm_xid, fi);
 	}
 #endif
@@ -1248,11 +1285,15 @@  handle_reply(int fd, SVCXPRT *xprt)
 #endif
 
 done:
+	rmt->nr_rqsts--;
+	if (rmt->nr_rqsts == 0)
+		delete_rmtcallfd_list(rmt);
+
 	if (buffer)
 		free(buffer);
 
 	if (reply_msg.rm_xid == 0) {
-#ifdef	SVC_RUN_DEBUG
+#ifdef	DEBUG_RMTCALL
 	if (debugging) {
 		xlog(LOG_DEBUG, "handle_reply:  NULL xid on exit!\n");
 	}
diff --git a/src/rpcbind.c b/src/rpcbind.c
index 8db8dfc..97d5fe9 100644
--- a/src/rpcbind.c
+++ b/src/rpcbind.c
@@ -794,25 +794,6 @@  got_socket:
 		}
 	}
 #endif
-	/*
-	 * rmtcall only supported on CLTS transports for now.
-	 */
-	if (nconf->nc_semantics == NC_TPI_CLTS) {
-		status = create_rmtcall_fd(nconf);
-
-#ifdef RPCBIND_DEBUG
-		if (debugging) {
-			if (status < 0) {
-				xlog(LOG_DEBUG,
-				    "Could not create rmtcall fd for %s\n",
-					nconf->nc_netid);
-			} else {
-				xlog(LOG_DEBUG, "rmtcall fd for %s is %d\n",
-					nconf->nc_netid, status);
-			}
-		}
-#endif
-	}
 	return (0);
 error:
 	close(fd);
diff --git a/src/rpcbind.h b/src/rpcbind.h
index 5b1a9bb..e5f8fe0 100644
--- a/src/rpcbind.h
+++ b/src/rpcbind.h
@@ -110,7 +110,7 @@  void *rpcbproc_uaddr2taddr_com(void *, struct svc_req *,
 					     SVCXPRT *, rpcvers_t);
 void *rpcbproc_taddr2uaddr_com(void *, struct svc_req *, SVCXPRT *,
 				    rpcvers_t);
-int create_rmtcall_fd(struct netconfig *);
+struct rmtcallfd_list *create_rmtcall_fd(struct netconfig *);
 void rpcbproc_callit_com(struct svc_req *, SVCXPRT *, rpcvers_t,
 			      rpcvers_t);
 void my_svc_run(void);