#8 Server ignores windowsize option when files are written

[Frank Kuehndel]

I believe I discovered a bug in the atftp implementation, when writing a file to the server with the windowsize option enabled. It looks like the server sends an ACK packet after each DATA packet send from the client (i.e. the server ignores the windowsize option when a file is sent to the server). (When reading a file from the server, everything works fine.)

I compiled atftp from the git with HEAD c68a89b10dc9130766420ddd218564dcc99583cf. My machine is an OpenSUSE leap 15.3 GNU/Linux.

To reproduce the error, I use a simple setup using atftpd as server and atftp as client.

I start the server as root:

# atftpd --user tftp --group tftp --daemon --verbose=7 --logfile /var/log/atftpd/atftp.log /srv/tftpboot

To log the packages transferred, I use this command as root:

# tcpdump -vXn#i lo udp |& tee /var/log/atftpd/tcpdump.log

I create a file with 272 bytes length to be written as unprivileged user:

$ head -c 272 /dev/zero >file.bin

Next as unprivileged user, I start the atftp client:

$ atftp
tftp> option blksize 32
Option blksize = 32
tftp> option windowsize 4
Option windowsize = 4
tftp> status
Not connected
Mode:       octet
Verbose:    off
Trace:      off
Options
 tsize:     disabled
 blksize:   enabled
 timeout:   disabled
 multicast: disabled
mtftp variables
 client-port:   76
 mcast-ip:      0.0.0.0
 listen-delay:  2
 timeout-delay: 2
tftp> connect 127.0.0.1
tftp> put file.bin
tftp> quit

Looking at the tcpdump output:

• The client send the write request with the options:

    127.0.0.1.43777 > 127.0.0.1.69:  41 WRQ "file.bin" octet blksize 32 windowsize 4
    0x0000:  4500 0045 f21e 4000 4011 4a87 7f00 0001  E..E..@.@.J.....
    0x0010:  7f00 0001 ab01 0045 0031 fe44 0002 6669  .......E.1.D..fi
    0x0020:  6c65 2e62 696e 006f 6374 6574 0062 6c6b  le.bin.octet.blk
    0x0030:  7369 7a65 0033 3200 7769 6e64 6f77 7369  size.32.windowsi
    0x0040:  7a65 0034 00                             ze.4.

• The server accepts blksize and windowsize options:

127.0.0.1.58975 > 127.0.0.1.43777: UDP, length 26
    0x0000:  4500 0036 f21f 4000 4011 4a95 7f00 0001  E..6..@.@.J.....
    0x0010:  7f00 0001 e65f ab01 0022 fe35 0006 626c  ....._...".5..bl
    0x0020:  6b73 697a 6500 3332 0077 696e 646f 7773  ksize.32.windows
    0x0030:  697a 6500 3400                           ize.4.

• Next the client sends 4 DATA packets (block numbers 1, 2, 3, 4). (See at the end of the 0x0010 line the "0003 0001". 0003 means DATA packet, 0001 is the block number.)

127.0.0.1.43777 > 127.0.0.1.58975: UDP, length 36
    0x0000:  4500 0040 f220 4000 4011 4a8a 7f00 0001  E..@..@.@.J.....
    0x0010:  7f00 0001 ab01 e65f 002c fe3f 0003 0001  ......._.,.?....
    0x0020:  0000 0000 0000 0000 0000 0000 0000 0000  ................
    0x0030:  0000 0000 0000 0000 0000 0000 0000 0000  ................

    127.0.0.1.43777 > 127.0.0.1.58975: UDP, length 36
    0x0000:  4500 0040 f221 4000 4011 4a89 7f00 0001  E..@.!@.@.J.....
    0x0010:  7f00 0001 ab01 e65f 002c fe3f 0003 0002  ......._.,.?....
    0x0020:  0000 0000 0000 0000 0000 0000 0000 0000  ................
    0x0030:  0000 0000 0000 0000 0000 0000 0000 0000  ................

    127.0.0.1.43777 > 127.0.0.1.58975: UDP, length 36
    0x0000:  4500 0040 f222 4000 4011 4a88 7f00 0001  E..@."@.@.J.....
    0x0010:  7f00 0001 ab01 e65f 002c fe3f 0003 0003  ......._.,.?....
    0x0020:  0000 0000 0000 0000 0000 0000 0000 0000  ................
    0x0030:  0000 0000 0000 0000 0000 0000 0000 0000  ................

    127.0.0.1.43777 > 127.0.0.1.58975: UDP, length 36
    0x0000:  4500 0040 f223 4000 4011 4a87 7f00 0001  E..@.#@.@.J.....
    0x0010:  7f00 0001 ab01 e65f 002c fe3f 0003 0004  ......._.,.?....
    0x0020:  0000 0000 0000 0000 0000 0000 0000 0000  ................
    0x0030:  0000 0000 0000 0000 0000 0000 0000 0000  ................

• Next one sees the server response. 0004 is an ACK packet. I would expect a single ACK packet for block 0004 but the server responses with four ACK packets, one for each DATA packet received:

    127.0.0.1.58975 > 127.0.0.1.43777: UDP, length 4
    0x0000:  4500 0020 f224 4000 4011 4aa6 7f00 0001  E....$@.@.J.....
    0x0010:  7f00 0001 e65f ab01 000c fe1f 0004 0001  ....._..........

    127.0.0.1.58975 > 127.0.0.1.43777: UDP, length 4
    0x0000:  4500 0020 f225 4000 4011 4aa5 7f00 0001  E....%@.@.J.....
    0x0010:  7f00 0001 e65f ab01 000c fe1f 0004 0002  ....._..........

    127.0.0.1.58975 > 127.0.0.1.43777: UDP, length 4
    0x0000:  4500 0020 f226 4000 4011 4aa4 7f00 0001  E....&@.@.J.....
    0x0010:  7f00 0001 e65f ab01 000c fe1f 0004 0003  ....._..........

    127.0.0.1.58975 > 127.0.0.1.43777: UDP, length 4
    0x0000:  4500 0020 f227 4000 4011 4aa3 7f00 0001  E....'@.@.J.....
    0x0010:  7f00 0001 e65f ab01 000c fe1f 0004 0004  ....._..........

From this point on, the remaining of the communication logged by tcpdump is a mess. The reason is that the client when it sees the ACK for packets 1, 2, 3 "believes" packages got lost or out-of-order and starts sending windows with the allegedly lost packets as starting points - and the server keeps sending ACk packets to these.

[Andi]

I can confirm this bug. Write requests have not been tested as much as read requests. I'll investigate. Thanks for testing and reporting!

[Andi]

Patch is available:
https://salsa.debian.org/debian/atftp/-/blob/master/debian/patches/fix-windowsize-WRQ.patch
I'll add some tests to test/test.sh soon.

[Andi]

Hi all,

attached is the final patch (fix-windowsize-WRQ.patch) addressing this issue. The problem was, that the server did not use the windowsize value sent from the client when receiving a file.

A second patch (minor-fixes.patch) addresses an update to RFC1782 → RFC2347 and adds the windowsize option to the client's status output.

I updated the test script in patch tests.patch. It checks different windowsize options. It detects and reports the bug fixed nicely now.

Finally, test-pcre.patch implements tests for the PCREs.

Apply in the following order:

fix-windowsize-WRQ.patch
minor-fixes.patch
tests.patch
test-pcre.patch

Best regards,
Andi

[Andi]

I guess this bug is fixed and can be closed.