google / netstack

IPv4 and IPv6 userland network stack
Apache License 2.0
3.09k stars 279 forks source link

tcpip/seqnum/LessThan has a math calculation bug. #14

Closed fanpei91 closed 4 years ago

fanpei91 commented 5 years ago

seqnum.LessThan :

func (v Value) LessThan(w Value) bool {
    return int32(v-w) < 0
}

It will be false at this case:

func main() {
       var tsRecent uint32
       var tsVal  uint32 = 3744464773
       fmt.Println(int32(tsRecent-tsVal) < 0)  //  false
}

This version works correctly.

func (v Value) LessThan(w Value) bool {
    return int64(v)- int64(w) < 0
}
hbhasker commented 5 years ago

I believe this is correct. The reason is sequence numbers wrap around and TCP window size is capped at 1/2 the sequence space (i.e 2^32/2). Which means that 0 is in fact > 3744465773 and not less as otherwise the permitted window will be > 2 ^32/2 and that would break down the strict ordering logic of sequence numbers.

fanpei91 commented 5 years ago

I see, thank you for your explanation. I understand now why the window scale factor is up to 14, not 255.

fanpei91 commented 5 years ago

One question: Why there is no implementation of PAWS?

hbhasker commented 5 years ago

PAWS has been on my todo list forever. It required first implementing support for Timestamps which was done but then we had other things like SACK that took priority. PAWS is something we intend to implement in the future. Current uses of netstack ran little risk of wrap around as it's not used in situations where the sequence number could wrap around that fast for PAWS to be required.

-bhasker

On Thu, Dec 27, 2018 at 6:58 PM fanpei91 notifications@github.com wrote:

One question: Why there is no implementation of PAWS https://tools.ietf.org/html/rfc1323#page-17?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/google/netstack/issues/14#issuecomment-450278440, or mute the thread https://github.com/notifications/unsubscribe-auth/AAssNUOLyuxPWjJg7cAtJcRLoxiy48yIks5u9YjkgaJpZM4ZgOVV .

fanpei91 commented 5 years ago

Thank you. I am forwarding SACK and PAWS.

tamird commented 4 years ago

Can this issue be closed?