Option to send a RST when closing sockets

[bmadhavan]

Description: There are some use cases where we would want to close the socket by sending a RST rather than a 3 way handshake ( FIN,FIN+ACK,ACK ).

Reason: For certain security use cases if we detect clients are trying to attack, we would want to close the socket by sending a RST and free up our socket immediately. Currently we can only send a FIN for which the client might not reply back with a FIN+ACK and socket will be left hanging for a while.

This can be determined/used by any application or filters when trying to close a socket.

[ggreenway]

Also, for tcp_proxy, if we can detect that one side received a RST, it is more correct to send a RST to the other side than a FIN.

[mattklein123]

How does one do this using the socket API? I found this article. Is that the way? http://deepix.github.io/2016/10/21/tcprst.html

[bmadhavan]

Yes. Setting SO_LINGER using setsockopt() before close() is the way I was thinking of.

[alyssawilk]

When we do this, can we make it extensible enough to allow for TCP silent close?

It turns out to be super important when your proxies are talking heavily to cell networks https://www.cs.indiana.edu/~fengqian/paper/stc_conext13.pdf

[ggreenway]

@alyssawilk The two approaches I've thought of are:

1) Add a new value to ConnectionCloseType for Reset (and then an additional one for SilentClose). 2) Add a function to Connection to allow setting the close type when closed with NoFlush (I think FlushWrite should always be a clean/normal close).

I think what we choose will depend on where we want to configure the close type; I don't have a good sense for where that will be cleanest.

[alyssawilk]

Yeah, I'm not sure which is cleaner offhand, I just want to make sure we don't plumb through a boolean when I believe we'll want an enum or some such :-)

[ggreenway]

Duly noted.

[mattklein123]

Most likely we are going to end up doing this from HTTP level filters for security reasons, so also keep that in mind. I don't think we will want to expose the full connection object up to the HTTP filter level.

[bmadhavan]

@ggreenway I am thinking of implementing this by adding a new type in ConnectionCloseType (very much in sync with your approach (1) ). Following that I plan to add callers from conn_manager_impl.cc that way it can be called/utilized by any HTTP filters. Let me know if that sounds good. Once we have this I am sure it can be called from tcp_proxy too, whenever the logic to detect a RST gets added. sounds ok ?

[ggreenway]

I think that sounds reasonable. @mattklein123 @alyssawilk any objections?

[mattklein123]

Sounds reasonable to me.

[bmadhavan]

Great. Thanks guys !!

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[botengyao]

This seems a very old issue, is this implemented? Thanks!

[ggreenway]

No, I don't think this was ever implemented. I don't think a PR was ever written.

[botengyao]

Thanks for the quick response! So even for now, the Abort type recently added is following the normal FIN step IIUC. Envoy will send RST to downstream only in some specific conditions like there is data in socket recv buffer kernel and we close(fd), and there is no way to actively send RST right now. Probably we cannot distinguish FIN and RST from peer as well? I am looking at the code, not sure.

[ggreenway]

Yes, Abort just means to close the connection without trying to send any data buffered by Envoy. Adding a mode to send a RST is feasible if someone wanted to do it. I'm not sure there's a reliable way to detect receiving a RST with the socket API.

[botengyao]

Hi @ggreenway, @yanavlasov, @mattklein123

Above is a draft PR to implement this feature, and it still needs some clean-up, but it is good to be reviewed in high level. I put a detailed description in that PR, feel free to comment and review. Thanks!