snow/wireguard-go
1
0
Fork 0
Code Issues Pull requests Releases Wiki Activity

conn: windows: do not error out when receiving UDP jumbogram

Browse source 
If we receive a large UDP packet, don't return an error to receive.go,
which then terminates the receive loop. Instead, simply retry.

Considering Winsock's general finickiness, we might consider other
places where an attacker on the wire can generate error conditions like
this.

Reported-by: Sascha Dierberg <sascha.dierberg@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
This commit is contained in:
Jason A. Donenfeld 2021-04-26 22:07:03 -04:00
parent c9db4b7aaa
commit 8246d251ea
1 changed files with 14 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
conn/bind_windows.go
View file

@ -352,8 +352,11 @@ func (bind *afWinRingBind) Receive(buf []byte, isOpen *uint32) (int, Endpoint, e
} }
bind.rx.mu.Lock() bind.rx.mu.Lock()
defer bind.rx.mu.Unlock() defer bind.rx.mu.Unlock()
var err error
var count uint32 var count uint32
var results [1]winrio.Result var results [1]winrio.Result
retry:
for tries := 0; count == 0 && tries < receiveSpins; tries++ { for tries := 0; count == 0 && tries < receiveSpins; tries++ {
if tries > 0 { if tries > 0 {
if atomic.LoadUint32(isOpen) != 1 { if atomic.LoadUint32(isOpen) != 1 {
@ -364,7 +367,7 @@ func (bind *afWinRingBind) Receive(buf []byte, isOpen *uint32) (int, Endpoint, e
count = winrio.DequeueCompletion(bind.rx.cq, results[:]) count = winrio.DequeueCompletion(bind.rx.cq, results[:])
} }
if count == 0 { if count == 0 {
err := winrio.Notify(bind.rx.cq) err = winrio.Notify(bind.rx.cq)
if err != nil { if err != nil {
return 0, nil, err return 0, nil, err
} }
@ -385,10 +388,19 @@ func (bind *afWinRingBind) Receive(buf []byte, isOpen *uint32) (int, Endpoint, e
} }
} }
bind.rx.Return(1) bind.rx.Return(1)
err := bind.InsertReceiveRequest() err = bind.InsertReceiveRequest()
if err != nil { if err != nil {
return 0, nil, err return 0, nil, err
} }
// We limit the MTU well below the 65k max for practicality, but this means a remote host can still send us
// huge packets. Just try again when this happens. The infinite loop this could cause is still limited to
// attacker bandwidth, just like the rest of the receive path.
if windows.Errno(results[0].Status) == windows.WSAEMSGSIZE {
if atomic.LoadUint32(isOpen) != 1 {
return 0, nil, net.ErrClosed
}
goto retry
}
if results[0].Status != 0 { if results[0].Status != 0 {
return 0, nil, windows.Errno(results[0].Status) return 0, nil, windows.Errno(results[0].Status)
} }