Re: [PATCH] DCCP: Fix to handle invalid packets in REQUEST state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Gerrit Renker wrote:
| >> 2. If sequence-invalid Dccp-Response is received in the REQUEST state, | >> kernel will panic. This is because that after send reset when received | >> sequence-invalid Dccp-Response, the state of socket not be changed. The | >> procedure is like this:
| >>
| >> Endpoint A                   Endpint B
| >>         <-----------------  Request
| >> Response ----------------->  (sequence-invalid)
| >>         <-----------------  Reset (invalid packet)
| >> Response ----------------->  kernel panic
| >>
| >> Pid: 0, comm: swapper Not tainted (2.6.26 #1)
| >> EIP: 0060:[<c05b426d>] EFLAGS: 00010282 CPU: 0
| >> EIP is at skb_release_all+0x6/0x7e
| >> EAX: 00000000 EBX: 00000000 ECX: 00000046 EDX: c4fdd000
| >> ESI: c79aad80 EDI: c4fdd000 EBP: c077ee0c ESP: c077ee08
| >> DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
| >> Process swapper (pid: 0, ti=c077e000 task=c0700340 task.ti=c0734000)
| >> Stack: 00000000 c077ee18 c05b3bf3 c79a9234 c077ee58 c8ab1db5 c79aad80 c4fdd000
| >>       02000015 c79aada0 00000c3c 00000854 c79aad80 c077ee50 c04260db c8ac176e
| >>       0059b20e c4fdd000 c79aad80 c4fdd000 c077ee78 c8ac06e3 0000001c c79a9234
| >> Call Trace:
| >> [<c05b3bf3>] ? __kfree_skb+0xb/0x66
| >> | > <snip>
| > This is clearly a bug. From your analysis above and from looking at the code
| > the cause is in all likelihood:
| >  1. sequence-invalid Response arrives;
| >  2. retransmit timer is stopped and sk->sk_send_head is freed;
| >  3. Reset (Packet Error) is sent as per the above diagram;
| >  4. Next Response (valid or invalid) arrives
| >  5. dccp_rcv_request_sent_state_process tries to __kfree_skb() the
| >     sk_send_head (which had already been freed in (2))
| >     ==> panic
| >
| > In your approach the socket is closed for each error case. This seems to
| > work, but there is a problem with it. | >
| > First, invalid or unexpected packets may arrive (as in the first
| > scenario). It is even possible that for example a DataAck stems from a
| > previous incarnation of the same connection. Closing the socket here
| > will kill the new connection.
| >
| > Secondly, it also opens the door to an easy denial-of-service attack:
| > all an attacker needs to do is to send an unexpected packet type to the
| > client in state REQUEST, whereupon the client closes its socket.
| > | | The problem still exists if we not close the client's socket. See the
| following:
| | The Client The Server
|                                   (OPEN)
| Request        ---------------->
|                <---------------   Sync, DataAck etc.
| Reset          ---------------->  enter TIMEWAIT state (8.5, Step 9)
| Request ----------------> | <---------------- Reset (no connection) | | | So I think if you'd like to retransmit Request and let Server to send
| Response, you should change the server not to enter TIMEWAIT state, just
| enter CLOSED state, this break the 8.5, Step 9.
| >From looking through the code, a sequence-valide Reset packet will close
the Client socket, via the following steps:
 * packet is received via dccp_v{4,6}_do_rcv (dccp_child_process is not
   relevant here, since we are interested in the client);
 * since the state is not LISTEN, control proceeds to
 * dccp_rcv_state_process and from there to
 * dccp_rcv_reset, which
   - sets the state to TIMEWAIT,
   - shuts the socket for both sending and receiving,
   - sets the SOCK_DEAD flag.

In accordance with the note marked with the asterisk underneath the
table in 7.5.3, no sequence number check is performed for the Reset
packet, and an acknowledgment-window check is not applicable.

>From this analysis, I think that the socket will be closed, with the
client entering TIMEWAIT state as the last step.

However, this is by just looking through, I have not tested if this
holds in practice as expected (it should).


Yes, it does, the last Reset (no connection) will cause client entering TIMEWAIT state. Your patch is correct.

BTW: The other interesting thing is that maybe the following testcase can be used for attack the sequence num:

The Client                        The Server
                                 (OPEN) (GSS=10, GSR=1)
Request        ---------------->
(SEQ=10000)
              <---------------   Sync
                                 (SEQ=11, ACK=10000)
Reset ----------------> (SEQ=10001, ACK=11)
              <---------------   Sync
                                 (SEQ=12, ACK=1)
DATA ----------------> (SEQ=2, ACK=12)
             <---------------   ACK
                ..........


In this time, the client will know both the sever and the client's sequence num, this maybe used for attack I think. If it is a attack, so the best way to avoid this is not sent Dccp-Sync after received the sequence-invalid reset.

Regards.

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

[Index of Archives]     [Linux Kernel]     [IETF DCCP]     [Linux Networking]     [Git]     [Security]     [Linux Assembly]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]

  Powered by Linux