Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise errors returned from TcpSocket::process() #23

Closed
9 tasks done
whitequark opened this issue Jun 26, 2017 · 1 comment
Closed
9 tasks done

Revise errors returned from TcpSocket::process() #23

whitequark opened this issue Jun 26, 2017 · 1 comment

Comments

@whitequark
Copy link
Contributor

whitequark commented Jun 26, 2017

To summarize:

  • We should never return Err(Error::Malformed) for errors that depend on socket state (since it might simply be that we crashed and lost the state); only return that for packets that are invalid on their own, i.e. would not be accepted no matter what state the socket has.
  • Returning Err(Error::Rejected) from TcpSocket::process() is only really appropriate while we're determining which endpoint they're addressed to. Once we know it's addressed to us (having two open sockets with the same local and remote endpoints, and the remote endpoint is not unspecified) we should not go on iterating through the sockets and send RST immediately.

In detail:

  • Protocol check should be a debug_assert!() now.
  • If state is CLOSED or the destination does not match, we return Err(Error::Rejected); it might be a half-open connection but it also might simply be addressed to another socket.
  • If state is LISTEN and we get an ACK then we're probably a socket in backlog and it's actually destined to someone else, so return Err(Error::Rejected).
  • If state is SYN-SENT and we get an RST with an unacceptable ACK we return Err(Error::Dropped); it's probably a half-open connection or other anomaly, and if the remote end doesn't want to accept our packets we'll get a proper RST when our SYN is retransmitted.
  • RSTs in any other state need only have an acceptable SEQ.
  • Any packet except SYNs is malformed if it's not also an ACK, so return Err(Error::Malformed).
  • Unacceptable SEQs (so long as we're synchronized) should always trigger a challenge ACK.
  • Unacceptable ACKs should always trigger a challenge ACK.
  • Consider returning an Option<TcpRepr> from TcpSocket::process() so that we can bring reset generation together with the rest of the TCP code (and test it there too). This will still need to be exported so that EthernetInterface can respond with a reset e.g. even if we have no TCP sockets at all. It can also be used for generating challenge ACKs immediately and not on the next iteration of emission.
@whitequark whitequark modified the milestone: v0.4 Aug 29, 2017
@whitequark
Copy link
Contributor Author

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

1 participant