tftpd: Address flaky tests

The tftpd tests all follow the same pattern:
1. open a UDP socket,
2. fork a child to exec tftpd, which subsequently handles requests on
the socket,
3. use a client soc

tftpd: Address flaky tests

The tftpd tests all follow the same pattern:
1. open a UDP socket,
2. fork a child to exec tftpd, which subsequently handles requests on
the socket,
3. use a client socket to send some message to the tftpd daemon.

However, tftpd's first action is to mark its socket as non-blocking and
then read a request from it. If no data is present in the socket, tftpd
exits immediately with an error. So, there is a race; we often see
tftpd test timeouts when running tests in parallel. These timeouts also
arise periodically in CI runs.

One solution is to restructure each test to create the server socket,
then write the request to the client socket, then fork tftpd. This
closes the race. However, this involves a lot of churn.

This patch fixes the problem a different way, by adding a new -b flag to
tftpd which makes it block to read the initial request. Each test is
modified to use -b, closing the race.

Reviewed by: imp, asomers
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D47404

show more ...

tftpd: Add missing include.

This went unnoticed due to namespace pollution in our headers.

MFC after: 3 days
Sponsored by: Klara, Inc.
Reviewed by: imp
Differential Revision: https://reviews.freebs

tftpd: Add missing include.

This went unnoticed due to namespace pollution in our headers.

MFC after: 3 days
Sponsored by: Klara, Inc.
Reviewed by: imp
Differential Revision: https://reviews.freebsd.org/D45131

show more ...

tftpd: Immediately reject any request shorter than 4 bytes.

MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: kevans
Differential Revision: https://reviews.freebsd.org/D44957

tftpd: Check the server status after each test.

* In the setup phase, wait for the server to start (or fail to start)
before proceeding with the test. This makes it possible to write test
cases

tftpd: Check the server status after each test.

* In the setup phase, wait for the server to start (or fail to start)
before proceeding with the test. This makes it possible to write test
cases that don't expect a response from the server without ending up
in a race over the server PID file.
* After running each test, wait up to 30 seconds for the server to exit
and check that the exit status matches what the test case says to
expect (usually 0).
* We still kill and collect the server in the cleanup phase, in case the
test ended early.

MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: kevans
Differential Revision: https://reviews.freebsd.org/D44956

show more ...

tftpd: Clean up the tests.

MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: kevans
Differential Revision: https://reviews.freebsd.org/D44955

tftpd: Use `size_t` where appropriate.

* Limit the use of `ssize_t` to only where it's needed.
* Correct one case of `int` being used for a length.

MFC after: 1 week
Sponsored by: Klara, Inc.
Revie

tftpd: Use `size_t` where appropriate.

* Limit the use of `ssize_t` to only where it's needed.
* Correct one case of `int` being used for a length.

MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: kevans
Differential Revision: https://reviews.freebsd.org/D44954

show more ...

libexec: Automated cleanup of cdefs and other formatting

Apply the following automated changes to try to eliminate
no-longer-needed sys/cdefs.h includes as well as now-empty
blank lines in a row.

R

libexec: Automated cleanup of cdefs and other formatting

Apply the following automated changes to try to eliminate
no-longer-needed sys/cdefs.h includes as well as now-empty
blank lines in a row.

Remove /^#if.*\n#endif.*\n#include\s+<sys/cdefs.h>.*\n/
Remove /\n+#include\s+<sys/cdefs.h>.*\n+#if.*\n#endif.*\n+/
Remove /\n+#if.*\n#endif.*\n+/
Remove /^#if.*\n#endif.*\n/
Remove /\n+#include\s+<sys/cdefs.h>\n#include\s+<sys/types.h>/
Remove /\n+#include\s+<sys/cdefs.h>\n#include\s+<sys/param.h>/
Remove /\n+#include\s+<sys/cdefs.h>\n#include\s+<sys/capsicum.h>/

Sponsored by: Netflix

show more ...

Remove $FreeBSD$: one-line .c pattern

Remove /^[\s*]*__FBSDID\("\$FreeBSD\$"\);?\s*\n/

spdx: The BSD-2-Clause-FreeBSD identifier is obsolete, drop -FreeBSD

The SPDX folks have obsoleted the BSD-2-Clause-FreeBSD identifier. Catch
up to that fact and revert to their recommended match of

spdx: The BSD-2-Clause-FreeBSD identifier is obsolete, drop -FreeBSD

The SPDX folks have obsoleted the BSD-2-Clause-FreeBSD identifier. Catch
up to that fact and revert to their recommended match of BSD-2-Clause.

Discussed with: pfg
MFC After: 3 days
Sponsored by: Netflix

show more ...

tftpd: Gracefully skip tests if networking is not enabled.

Sponsored by: Klara, Inc.
Reviewed by: asomers
Differential Revision: https://reviews.freebsd.org/D39012

Merge ^/head r358466 through r358677.

Add support for the TFTP windowsize option described in RFC 7440.

The windowsize option permits multiple blocks to be transmitted
before the receiver sends an ACK improving throughput for larger
fil

Add support for the TFTP windowsize option described in RFC 7440.

The windowsize option permits multiple blocks to be transmitted
before the receiver sends an ACK improving throughput for larger
files.

Reviewed by: asomers
MFC after: 2 weeks
Sponsored by: DARPA
Differential Revision: https://reviews.freebsd.org/D23836

show more ...

MFHead@r345275

Drop "All rights reserved" from the files I own

Also, add SPDX tags where needed.

MFC after: 2 weeks

tftpd: misc Coverity cleanup in the tests

A bunch of unchecked return values from open(2) and read(2)

Reported by: Coverity
CID: 1386900, 1386911, 1386926, 1386928, 1386932, 1386942
CID: 1386961,

tftpd: misc Coverity cleanup in the tests

A bunch of unchecked return values from open(2) and read(2)

Reported by: Coverity
CID: 1386900, 1386911, 1386926, 1386928, 1386932, 1386942
CID: 1386961, 1386979
MFC after: 8 days
X-MFC-With: 330696

show more ...

tftpd: fix the build of tests on i386 after 330696

It's those darn printf format specifiers again

Reported by: cy, kibab
MFC after: 20 days
X-MFC-With: 330696

tftpd: reject unknown opcodes

If tftpd receives a command with an unknown opcode, it simply exits 1. It
doesn't send an ERROR packet, and the client will hang waiting for one. Fix
it.

PR: 226005

tftpd: reject unknown opcodes

If tftpd receives a command with an unknown opcode, it simply exits 1. It
doesn't send an ERROR packet, and the client will hang waiting for one. Fix
it.

PR: 226005
MFC after: 3 weeks

show more ...

tftpd: Abort on an WRQ access violation

On a WRQ (write request) tftpd checks whether the client has access
permission for the file in question. If not, then the write is prevented.
However, tftpd

tftpd: Abort on an WRQ access violation

On a WRQ (write request) tftpd checks whether the client has access
permission for the file in question. If not, then the write is prevented.
However, tftpd doesn't reply with an ERROR packet, nor does it abort.
Instead, it tries to receive the packet anyway.

The symptom is slightly different depending on the nature of the error. If
the target file is nonexistent and tftpd lacks permission to create it, then
tftpd will willingly receive the file, but not write it anywhere. If the
file exists but is not writable, then tftpd will fail to ACK to WRQ.

PR: 225996
MFC after: 3 weeks

show more ...

tftpd: Verify world-writability for WRQ when using relative paths

tftpd(8) says that files may only be written if they already exist and are
publicly writable. tftpd.c verifies that a file is publi

tftpd: Verify world-writability for WRQ when using relative paths

tftpd(8) says that files may only be written if they already exist and are
publicly writable. tftpd.c verifies that a file is publicly writable if it
uses an absolute pathname. However, if the pathname is relative, that check
is skipped. Fix it.

Note that this is not a security vulnerability, because the transfer
ultimately doesn't work unless the file already exists and is owned by user
nobody. Also, this bug does not affect the default configuration, because
the default uses the "-s" option which makes all pathnames absolute.

PR: 226004
MFC after: 3 weeks

show more ...

tftpd: Flush files as soon as they are fully received

On an RRQ, tftpd doesn't exit as soon as it's finished receiving a file.
Instead, it waits five seconds just in case the client didn't receive t

tftpd: Flush files as soon as they are fully received

On an RRQ, tftpd doesn't exit as soon as it's finished receiving a file.
Instead, it waits five seconds just in case the client didn't receive the
server's last ACK and decides to resend the final DATA packet.
Unfortunately, this created a 5 second delay from when the client thinks
it's done sending the file, and when the file is available for other
processes.

Fix this bug by closing the file as soon as receipt is finished.

PR: 157700
Reported by: Barry Mishler <barry_mishler@yahoo.com>
MFC after: 3 weeks

show more ...

Add some functional tests for tftpd(8)

tftpd(8) is difficult to test in isolation due to its relationship with
inetd. Create a test program that mimics the behavior of tftp(1) and
inetd(8) and veri

Add some functional tests for tftpd(8)

tftpd(8) is difficult to test in isolation due to its relationship with
inetd. Create a test program that mimics the behavior of tftp(1) and
inetd(8) and verifies tftpd's response in several different scenarios.

These test cases cover all of the basic TFTP protocol, but not the optional
parts.

PR: 157700
PR: 225996
PR: 226004
PR: 226005
MFC after: 3 weeks
Differential Revision: https://reviews.freebsd.org/D14310

show more ...