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

Added missing 0x20 in DCS EncodeDisconnectPacket #223

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dimant
Copy link

@dimant dimant commented Sep 4, 2022

As is, EncodeDisconnectPacket will fail IsValidDisconnectPacket check. The encoded packet is only has 18 bytes instead of 19 because it's missing a 0x20.

As is, EncodeDisconnectPacket will fail IsValidDisconnectPacket check. The encoded packet is only has 18 bytes instead of 19 because it's missing a 0x20.
@narspt
Copy link
Contributor

narspt commented Sep 10, 2022

@dimant: It was actually 19 bytes, with your change it is now 20 bytes! I think your added 0x20 is correct but you should also remove the very last 0x00, then it will match the disconnect packets generated by ircddbgateway (I did just sniff them to compare).

This is from XLX (without your change):

19:50:38.500223 IP 185.11.166.22.30051 > 192.168.1.74.30052: UDP, length 19
        0x0000:  4500 002f a910 4000 3711 7999 b90b a616  E../[email protected].....
        0x0010:  c0a8 014a 7563 7564 001b dc0e 4354 3248  ...Jucud....CT2H
        0x0020:  5242 2020 4200 4443 5332 3638 2020 00    RB..B.DCS268...

This is from ircddbgateway:

20:08:40.617847 IP 192.168.1.74.30052 > 185.11.166.22.30051: UDP, length 19
        0x0000:  45b8 002f b7e7 4000 4011 610a c0a8 014a  E../..@[email protected]
        0x0010:  b90b a616 7564 7563 001b 2141 4354 3248  ....uduc..!ACT2H
        0x0020:  5242 2020 4220 0044 4353 3236 3820 20    RB..B..DCS268..

Curiously "real" DCS reflectors doesn't even send these disconnect packets on timeout, they just drop client.

@yo2loj
Copy link
Contributor

yo2loj commented Sep 10, 2022

It makes no real sense to send a disconnect packet on client timeout, since the client is the one that has failed and it is reasonable to assume it is not there anymore. It would be just a courtesy action just in case, but could be useful for debug purposes.

Removed last byte of disconnect packet to ensure equivalence with ircdbgw.
@dimant
Copy link
Author

dimant commented Sep 22, 2022

@dimant: It was actually 19 bytes, with your change it is now 20 bytes! I think your added 0x20 is correct but you should also remove the very last 0x00, then it will match the disconnect packets generated by ircddbgateway (I did just sniff them to compare).
...
Curiously "real" DCS reflectors doesn't even send these disconnect packets on timeout, they just drop client.

My bad, thanks for the catch. Also I didn't know "real" reflectors don't send these packets, thanks for the info!

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

Successfully merging this pull request may close these issues.

3 participants