rpc.mountd: fix some vulnerabilities

PATCH In review [email protected] 4 participants 7 messages Nov 5, 2014 – Nov 12, 2014

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

[email protected]

author

2014-11-05 20:21 UTC #0 lore raw

cover rpc.mountd: fix some vulnerabilities

SGVsbG8sCgpJJ20gc2VuZGluZyBhIHNtYWxsIHNldCBvZiAzIHBhdGNoZXMgZm9yIGEgcHJv YmxlbSwgdGhhdCBJIGhhdmUKcmVwb3J0ZWQgYSBmZXcgd2Vla3MgYWdvLgpycGMubW91bnRk IGNhbiBiZSBibG9ja2VkIGJ5IGEgYmFkIGNsaWVudCwgdGhhdCBzZW5kcyBsb3RzIG9mClJQ QyByZXF1ZXN0cywgYnV0IG5ldmVyIHJlYWRzIHRoZSByZXBsaWVzIGZyb20gdGhlIHNvY2tl dCBlaXRoZXIKaW50ZW50aW9uYWxseSBvciBlLmcuIGNhdXNlZCBieSBhIHdyb25nIGNvbmZp Z3VyZWQgTVRVLgoKV2hpbGUgbG9va2luZyBmb3IgYSBwb3NzaWJsZSBzb2x1dGlvbiwgSSBm b3VuZCBhbm90aGVyIHdlYWtuZXNzCmluIHJwYy5tb3VudGQgaWYgaXQgaXMgdXNlZCAibXVs dGl0aHJlYWRlZCIgKC10IG5uKS4KClRoZSBmaXJzdCB0d28gcGF0Y2hlcyBmaXggdGhhdCB3 ZWFrbmVzcyBpbiB0aGUgY2FzZSBvZiAhSEFWRV9MSUJUSVJQQwphbmQgSEFWRV9MSUJUSVJQ Qy4KVGhlIHRoaXJkIHBhdGNoIG1vcmUgYSBraW5kIG9mIHN1Z2dlc3Rpb24gaG93IHRoZSBt YWluIHByb2JsZW0gY291bGQKYmUgZml4ZWQuIEkgZG9uJ3Qga25vdyB3aGV0aGVyIHdlIGNh biBzZXQgTUFYUkVDIHdpdGhvdXQgY2F1c2luZwpuZXcgdHJvdWJsZXMuIFdoZW4gdGhpcyBw YXRjaCBpcyB1c2VkLCBhICBmdXJ0aGVyIHBhdGNoIGZvciBsaWJ0aXJwYwphbHNvIHNob3Vs ZCBiZSB1c2VkLiBZb3UgY2FuIGZpbmQgaXQgaGVyZToKICAgIGh0dHA6Ly9zb3VyY2Vmb3Jn ZS5uZXQvcC9saWJ0aXJwYy9tYWlsbWFuL2xpYnRpcnBjLWRldmVsLz92aWV3bW9udGg9MjAx NDA5CgpCZXN0IHJlZ2FyZHMsCkJvZG8K

Steve Dickson

2014-11-11 18:24 UTC #1 lore raw

Hello,

Quoted 11 lines · On 11/05/2014 03:21 PM, [email protected] wrote:

Hello,
I'm sending a small set of 3 patches for a problem, that I have reported a few weeks ago.
rpc.mountd can be blocked by a bad client, that sends lots of RPC requests, but never reads the replies from the socket either intentionally or e.g. caused by a wrong configured MTU.
While looking for a possible solution, I found another weakness in rpc.mountd if it is used "multithreaded" (-t nn).

Quoted 7 lines

The first two patches fix that weakness in the case of !HAVE_LIBTIRPC and HAVE_LIBTIRPC.
The third patch more a kind of suggestion how the main problem could be fixed. I don't know whether we can set MAXREC without causing new troubles. When this patch is used, a further patch for libtirpc also should be used. You can find it here:
http://sourceforge.net/p/libtirpc/mailman/libtirpc-devel/?viewmonth=201409

Over the weekend I took a good hard look at these 3 patches and the one for libtirpc with the reproducer Bodo supplied me. (Bodo, thanks all the help getting things set up!).

I agree with the libtirpc patch so its been committed, but other three made me nervous so I wanted to take a good hard look at them. (Neil's question, "do we really what to make these non-block" haunted me ;-) ).

It turns out, at least in my setup, these patches do not and can not stop the mountd DOS that Bodo's reproducer creates.

Here is why they do not work:

The fd that the write() is getting hung on, is not be created by the 3 nfs_svc_create() calls in mountd:main(). Its being created by the accept() when the tcp connection is created (via SVC_RECV()), so making those fds coming out of the nfs_svc_create() non-blocking does nothing.

Here is the hang can't be stop (for now):

Here is the stack:
Stack trace of thread 4150:

#0  0x00000036816f0e90 write (libc.so.6)
#1  0x00007fd53837da6d write_vc (libtirpc.so.1)
#2  0x00007fd53838103f flush_out (libtirpc.so.1)
#3  0x00007fd53837dd81 svc_vc_reply (libtirpc.so.1)
#4  0x00007fd53837b096 svcerr_noprog (libtirpc.so.1)
#5  0x00007fd53837b360 svc_getreq_common (libtirpc.so.1)
#6  0x00000000004086d9 my_svc_getreqset (mountd)
#7  0x0000000000403d80 main (mountd)
#8  0x000000368161ffe0 __libc_start_main (libc.so.6)
#9  0x00000000004040f5 _start (mountd)

The fd that created by the SVC_RECV() is never
given back to the app (in case mountd) to make
it non-blocking. Its used in the error path
where it get hung up in the write(). The
app has no control over that.

Now I definitely see a problem, but these
patches (with the exception of the libtirpc
patch) don't address the problem I am seeing.

So unless I'm not seeing something, I'm not in
favor of taking these 3 patches.

thoughts?

steved.

Strösser, Bodo

2014-11-11 19:01 UTC #2 lore raw

Quoted 42 lines

-----Original Message-----
From: Steve Dickson [mailto:[email protected]]
Sent: Tuesday, November 11, 2014 7:25 PM
To: Strösser, Bodo; [email protected]; [email protected]
Cc: [email protected]
Subject: Re: [nfs-utils] [PATCH 0/3] rpc.mountd: fix some vulnerabilities
Hello,
Quoted 11 lines · On 11/05/2014 03:21 PM, [email protected] wrote:
Hello,
I'm sending a small set of 3 patches for a problem, that I have reported a few weeks ago.
rpc.mountd can be blocked by a bad client, that sends lots of RPC requests, but never reads the replies from the socket either intentionally or e.g. caused by a wrong configured MTU.
While looking for a possible solution, I found another weakness in rpc.mountd if it is used "multithreaded" (-t nn).
Quoted 7 lines
The first two patches fix that weakness in the case of !HAVE_LIBTIRPC and HAVE_LIBTIRPC.
The third patch more a kind of suggestion how the main problem could be fixed. I don't know whether we can set MAXREC without causing new troubles. When this patch is used, a further patch for libtirpc also should be used. You can find it here:
http://sourceforge.net/p/libtirpc/mailman/libtirpc-devel/?viewmonth=201409
Over the weekend I took a good hard look at these 3 patches and the one for libtirpc with the reproducer Bodo supplied me. (Bodo, thanks all the help getting things set up!).
I agree with the libtirpc patch so its been committed, but other three made me nervous so I wanted to take a good hard look at them. (Neil's question, "do we really what to make these non-block" haunted me ;-) ).
It turns out, at least in my setup, these patches do not and can not stop the mountd DOS that Bodo's reproducer creates.

Quoted 7 lines

Here is why they do not work:
The fd that the write() is getting hung on, is not be created by the 3 nfs_svc_create() calls in mountd:main(). Its being created by the accept() when the tcp connection is created (via SVC_RECV()), so making those fds coming out of the nfs_svc_create() non-blocking does nothing.

Sorry, Steve, unfortunately I have to disagree.

The hung we tried to reproduce on your config yesterday is fixed by patch 3/3 only. It simply uses

            rpc_control(RPC_SVC_CONNMAXREC_SET, &maxrec);

to make libtirpc set O_NONBLOCK on the tcp connections for us. That _definitely_ works well to avoid the hung, but maybe triggers the problem in libtirpc which is fixed by the patch you committed.
I for myself was uncertain regarding this patch, but meanwhile Neil pointed out, that setting MAXREC only affects the receive buffer size. For receive the used RPC_MAXDATASIZE (= 9000) always will be enough.

The two other patches (1/3 and 2/3) are related to the use of "-t nn" with rpc.mountd. They change the listening socket (TCP) and the reading socket (UDP) to nonblocking mode. See the following comment from source support/nfs/svc_socket.c, which is an old comment that doesn't come from me:

/* This socket might be shared among multiple processes
* if mountd is run multi-threaded. So it is safest to
* make it non-blocking, else all threads might wake
* one will get the data, and the others will block
* indefinitely.
* In all cases, transaction on this socket are atomic
* (accept for TCP, packet-read and packet-write for UDP)
* so O_NONBLOCK will not confuse unprepared code causing
* it to corrupt messages.
* It generally safest to have O_NONBLOCK when doing an accept
* as if we get a RST after the SYN and before accept runs,
* we can block despite being told there was an acceptable
* connection.
*/

This comment describs precisely, why

"We really do want non-blocking sockets here!"

as NeilBrown said. I don't think, that this was a question (Neil, please correct me if I'm wrong).

The problem is, that the code switches to nonblocking mode only if rpc.mountd was built without libtirpc and the "-p <portno>" option is not used.

If you use the option or rpc.mountd is built with libtirpc, the listening sockets are blocking. I have tested it simply by using the socket features of bash and strace'ing rpc.mountd.

When connecting to a multithreaded rpc.mountd, all threads wake up and call accept(), but of course only one thread gets the connection, all others sleep on the accept().

So, we talk about switching to nonblocking mode for the listeners (Patch 1/3 and 2/3) and the TCP connections (Patch 3/3).

Bodo

Quoted 32 lines

Here is the hang can't be stop (for now):
Here is the stack:
Stack trace of thread 4150:
#0  0x00000036816f0e90 write (libc.so.6)
#1  0x00007fd53837da6d write_vc (libtirpc.so.1)
#2  0x00007fd53838103f flush_out (libtirpc.so.1)
#3  0x00007fd53837dd81 svc_vc_reply (libtirpc.so.1)
#4  0x00007fd53837b096 svcerr_noprog (libtirpc.so.1)
#5  0x00007fd53837b360 svc_getreq_common (libtirpc.so.1)
#6  0x00000000004086d9 my_svc_getreqset (mountd)
#7  0x0000000000403d80 main (mountd)
#8  0x000000368161ffe0 __libc_start_main (libc.so.6)
#9  0x00000000004040f5 _start (mountd)
The fd that created by the SVC_RECV() is never
given back to the app (in case mountd) to make
it non-blocking. Its used in the error path
where it get hung up in the write(). The
app has no control over that.
Now I definitely see a problem, but these
patches (with the exception of the libtirpc
patch) don't address the problem I am seeing.
So unless I'm not seeing something, I'm not in
favor of taking these 3 patches.
thoughts?
steved.

NeilBrown

2014-11-11 23:00 UTC #3 lore raw

In reply to Steve Dickson

Quoted 35 lines · On Tue, 11 Nov 2014 13:24:31 -0500 Steve Dickson <[email protected]> wrote:

Hello,
Quoted 11 lines · On 11/05/2014 03:21 PM, [email protected] wrote:
Hello,
I'm sending a small set of 3 patches for a problem, that I have reported a few weeks ago.
rpc.mountd can be blocked by a bad client, that sends lots of RPC requests, but never reads the replies from the socket either intentionally or e.g. caused by a wrong configured MTU.
While looking for a possible solution, I found another weakness in rpc.mountd if it is used "multithreaded" (-t nn).
Quoted 7 lines
The first two patches fix that weakness in the case of !HAVE_LIBTIRPC and HAVE_LIBTIRPC.
The third patch more a kind of suggestion how the main problem could be fixed. I don't know whether we can set MAXREC without causing new troubles. When this patch is used, a further patch for libtirpc also should be used. You can find it here:
http://sourceforge.net/p/libtirpc/mailman/libtirpc-devel/?viewmonth=201409
Over the weekend I took a good hard look at these 3 patches and the one for libtirpc with the reproducer Bodo supplied me. (Bodo, thanks all the help getting things set up!).
I agree with the libtirpc patch so its been committed, but other three made me nervous so I wanted to take a good hard look at them. (Neil's question, "do we really what to make these non-block" haunted me ;-) ).
It turns out, at least in my setup, these patches do not and can not stop the mountd DOS that Bodo's reproducer creates.

Quoted 7 lines

Here is why they do not work:
The fd that the write() is getting hung on, is not be created by the 3 nfs_svc_create() calls in mountd:main(). Its being created by the accept() when the tcp connection is created (via SVC_RECV()), so making those fds coming out of the nfs_svc_create() non-blocking does nothing.

Just to add to what Bodo replied:

SVC_RECV is a macro which calls .xp_recv, which in the TCP listen case is rendezvous_request.

rendezvous_request does the accept:

        if ((sock = accept(xprt->xp_fd, (struct sockaddr *)(void *)&addr,
            &len)) < 0) {

and then

        if (cd->maxrec != 0) {
                flags = fcntl(sock, F_GETFL, 0);
                if (flags  == -1)
                        return (FALSE);
                if (fcntl(sock, F_SETFL, flags | O_NONBLOCK) == -1)
                        return (FALSE);

so if cd->maxrec is not zero, O_NONBLOCK get set.

It gets set through an SVC_CONTROL -> xp_control -> svc_vc_rendezvous_control

call, which no-one ever makes, and it initialised to __svc_maxrec, which is set by the rpc_control() call that Bodo mentions.
So providing rpc_control is called before the service is created, it should work fine.

I wonder why it didn't work for you...

Thanks,
NeilBrown

Quoted 32 lines

Here is the hang can't be stop (for now):
Here is the stack:
Stack trace of thread 4150:
#0  0x00000036816f0e90 write (libc.so.6)
#1  0x00007fd53837da6d write_vc (libtirpc.so.1)
#2  0x00007fd53838103f flush_out (libtirpc.so.1)
#3  0x00007fd53837dd81 svc_vc_reply (libtirpc.so.1)
#4  0x00007fd53837b096 svcerr_noprog (libtirpc.so.1)
#5  0x00007fd53837b360 svc_getreq_common (libtirpc.so.1)
#6  0x00000000004086d9 my_svc_getreqset (mountd)
#7  0x0000000000403d80 main (mountd)
#8  0x000000368161ffe0 __libc_start_main (libc.so.6)
#9  0x00000000004040f5 _start (mountd)
The fd that created by the SVC_RECV() is never
given back to the app (in case mountd) to make
it non-blocking. Its used in the error path
where it get hung up in the write(). The
app has no control over that.
Now I definitely see a problem, but these
patches (with the exception of the libtirpc
patch) don't address the problem I am seeing.
So unless I'm not seeing something, I'm not in
favor of taking these 3 patches.
thoughts?
steved.

Steve Dickson

2014-11-12 15:58 UTC #4 lore raw

Quoted 27 lines · NeilBrown

Quoted 7 lines
Here is why they do not work:
Quoted 6 lines
The fd that the write() is getting hung on, is not be created by the 3 nfs_svc_create() calls in mountd:main(). Its being created by the accept() when the tcp connection is created (via SVC_RECV()), so making those fds coming out of the nfs_svc_create() non-blocking does nothing.
Just to add to what Bodo replied:
SVC_RECV is a macro which calls .xp_recv, which in the TCP listen case is rendezvous_request.
rendezvous_request does the accept:
        if ((sock = accept(xprt->xp_fd, (struct sockaddr *)(void *)&addr,
            &len)) < 0) {
and then
        if (cd->maxrec != 0) {
                flags = fcntl(sock, F_GETFL, 0);
                if (flags  == -1)
                        return (FALSE);
                if (fcntl(sock, F_SETFL, flags | O_NONBLOCK) == -1)
                        return (FALSE);
so if cd->maxrec is not zero, O_NONBLOCK get set.

Quoted 8 lines

It gets set through an SVC_CONTROL -> xp_control -> svc_vc_rendezvous_control
call, which no-one ever makes, and it initialised to __svc_maxrec, which is set by the rpc_control() call that Bodo mentions.
So providing rpc_control is called before the service is created, it should work fine.
I wonder why it didn't work for you...

Because I didn't apply the 3rd patch because it was marked as experimental. I figured I would get the first two patches working and deal with it later...

Plus although I did look at this code, I didn't notice the setting of the O_NONBLOCK on the fd from the accept. Thanks for pointing it out....

steved.

Steve Dickson

2014-11-12 18:57 UTC #5 lore raw

In reply to [email protected]

Quoted 11 lines · On 11/05/2014 03:21 PM, [email protected] wrote:

Hello,
I'm sending a small set of 3 patches for a problem, that I have reported a few weeks ago.
rpc.mountd can be blocked by a bad client, that sends lots of RPC requests, but never reads the replies from the socket either intentionally or e.g. caused by a wrong configured MTU.
While looking for a possible solution, I found another weakness in rpc.mountd if it is used "multithreaded" (-t nn).

Quoted 7 lines

The first two patches fix that weakness in the case of !HAVE_LIBTIRPC and HAVE_LIBTIRPC.
The third patch more a kind of suggestion how the main problem could be fixed. I don't know whether we can set MAXREC without causing new troubles. When this patch is used, a further patch for libtirpc also should be used. You can find it here:
http://sourceforge.net/p/libtirpc/mailman/libtirpc-devel/?viewmonth=201409

After applying all three patches, the DOS does stop... All three committed! Nice work! Thank you... very much!!

steved.

Quoted 5 lines

Best regards,
Bodo N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±�û"žØ^n‡r¡ö¦zË�ëh™¨èÚ&¢ø®G«�éh®(éšŽŠÝ¢j"�ú¶m§ÿï�êäz¹Þ–Šàþf£¢·hšˆ§~ˆmml==

Strösser, Bodo

2014-11-12 19:26 UTC #6 lore raw

Thank you for your thorough work.

Bodo

Quoted 40 lines

-----Original Message-----
From: Steve Dickson [mailto:[email protected]]
Sent: Wednesday, November 12, 2014 7:57 PM
To: Strösser, Bodo; [email protected]; [email protected]
Cc: [email protected]
Subject: Re: [nfs-utils] [PATCH 0/3] rpc.mountd: fix some vulnerabilities
Quoted 11 lines · On 11/05/2014 03:21 PM, [email protected] wrote:
Hello,
I'm sending a small set of 3 patches for a problem, that I have reported a few weeks ago.
rpc.mountd can be blocked by a bad client, that sends lots of RPC requests, but never reads the replies from the socket either intentionally or e.g. caused by a wrong configured MTU.
While looking for a possible solution, I found another weakness in rpc.mountd if it is used "multithreaded" (-t nn).
Quoted 7 lines
The first two patches fix that weakness in the case of !HAVE_LIBTIRPC and HAVE_LIBTIRPC.
The third patch more a kind of suggestion how the main problem could be fixed. I don't know whether we can set MAXREC without causing new troubles. When this patch is used, a further patch for libtirpc also should be used. You can find it here:
http://sourceforge.net/p/libtirpc/mailman/libtirpc-devel/?viewmonth=201409
After applying all three patches, the DOS does stop... All three committed! Nice work! Thank you... very much!!
steved.
Quoted 4 lines
Best regards,
Bodo
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–
)Þº{.nÇ+‰·¥Š{±�û"žØ^n‡r¡ö¦zË�ëh™¨èÚ&¢ø®G«�éh®(éšŽŠÝ¢j"�ú¶m§ÿï�êäz¹Þ– Šàþf£¢·hšˆ§~ˆmml==
Quoted 1 line