[Rseding] [1.1.30] Deadlock on loss of ConnectionAcceptOrDeny message

This subforum contains all the issues which we already resolved.
Post Reply
Hornwitser
Fast Inserter
Fast Inserter
Posts: 183
Joined: Fri Oct 05, 2018 4:34 pm
Contact:

[Rseding] [1.1.30] Deadlock on loss of ConnectionAcceptOrDeny message

Post by Hornwitser »

When the ConnectionAcceptOrDeny message from the server is dropped on the way to the client it's possible for the server to end up deadlocked waiting for the client to continue the joining process. See the annotated image of the packet capture I've attached.

The server starts sending SynchronizerActions to the client immediately after it has sent the ConnectionAcceptOrDeny message, but if the client doesn't receive the ConnectionAcceptOrDeny message it waits half a second and then resends the ConnectioRequestReplyConfirm message it sent to the server.

In response to the second ConnectionRequestReplyConfirm message the server sends a modified ConnectionAcceptOrDeny message back with a higher first sequence number, if the client continues from this sequence then it will miss all the synchronizer actions sent by the server between the first and second ConnectionAcceptOrDeny message.

If the client is waiting for one of those synchronizer actions then the client and server deadlocks, the client is waiting for the server to send a message it has already sent, and the server is waiting for the client to signal it's finished downloading the auxiliary data. For players connected to the server this shows up as a progress bar at 100% saying "Server (<server>) is saving the map for player.", for the player connecting it's a progress bar at 100% saying "Waiting for the server to save the map.". The server remains frozen until the player connecting aborts the connection.
Attachments
annotated-packet-dump.png
annotated-packet-dump.png (179.24 KiB) Viewed 2745 times

oof2win2
Long Handed Inserter
Long Handed Inserter
Posts: 53
Joined: Wed Nov 18, 2020 8:13 am
Contact:

Re: [Rseding] [1.1.30] Deadlock on loss of ConnectionAcceptOrDeny message

Post by oof2win2 »

Please fix this. It still occurs sometimes, and it forces the server to be restarted.

Here is a bit of code to replicate the issue. Doesn't support passworded servers (yet), but replicates the issue successfully. Existing data (such as serverKey and serverKeyTimestamp) was taken from running a Factorio instance and connecting it, with Wireshark logging the packets. The fake client will be dropped after some time as it doesn't change it's nextToRecieveServerTickClosure value, but for that time the server will be stuck saving.
https://github.com/oof2win2/factorio-connect-client

Rseding91
Factorio Staff
Factorio Staff
Posts: 12341
Joined: Wed Jun 11, 2014 5:23 am
Contact:

Re: [Rseding] [1.1.30] Deadlock on loss of ConnectionAcceptOrDeny message

Post by Rseding91 »

After looking into this (again) I think I found at least a solution that prevents the deadlock and it will be in the next 1.1. release.
If you want to get ahold of me I'm almost always on Discord.

oof2win2
Long Handed Inserter
Long Handed Inserter
Posts: 53
Joined: Wed Nov 18, 2020 8:13 am
Contact:

Re: [Rseding] [1.1.30] Deadlock on loss of ConnectionAcceptOrDeny message

Post by oof2win2 »

The issue still persists in 1.1.60, which is the latest 1.1. release. See the github repo provided above for a working implementation that deadlocks the server 100% of the time. You only need to change the instance and request IDs - I do it by Wireshark and then I increment the request ID by an amount, which usually works.

Image

Rseding91
Factorio Staff
Factorio Staff
Posts: 12341
Joined: Wed Jun 11, 2014 5:23 am
Contact:

Re: [Rseding] [1.1.30] Deadlock on loss of ConnectionAcceptOrDeny message

Post by Rseding91 »

oof2win2 wrote:
Mon Jun 06, 2022 7:14 pm
The issue still persists in 1.1.60, which is the latest 1.1. release. See the github repo provided above for a working implementation that deadlocks the server 100% of the time. You only need to change the instance and request IDs - I do it by Wireshark and then I increment the request ID by an amount, which usually works.
That's packet manipulation not packet loss. Feel free to make a different report about packet manipulation.
If you want to get ahold of me I'm almost always on Discord.

User avatar
Windsinger
Inserter
Inserter
Posts: 41
Joined: Fri Mar 20, 2020 7:33 pm
Contact:

Re: [Rseding] [1.1.30] Deadlock on loss of ConnectionAcceptOrDeny message

Post by Windsinger »

Respectfully, you just ignored a perfectly worded PoC that allow malicious users to take down a server running with your software and you tell them to make a new bug report out of it. Are you serious?

Hornwitser
Fast Inserter
Fast Inserter
Posts: 183
Joined: Fri Oct 05, 2018 4:34 pm
Contact:

Re: [Rseding] [1.1.30] Deadlock on loss of ConnectionAcceptOrDeny message

Post by Hornwitser »

Windsinger wrote:
Thu Jun 09, 2022 1:19 pm
Respectfully, you just ignored a perfectly worded PoC that allow malicious users to take down a server running with your software and you tell them to make a new bug report out of it. Are you serious?
To be fair the issue this report is about is that the server sends an incorrect sequence number when the client retries a connection request and as a result of that the client holds up the server. This issue is not about the ability of a client to hold up the server during saving, that is technically a different issue concerning a different part of the network code. Which by the way has now been fixed for the next release.

Post Reply

Return to “Resolved Problems and Bugs”