MFC after: 3 days
Sponsored by: Klara, Inc.
Reviewed by: imp, markj
Differential Revision: https://reviews.freebsd.org/D45129
(cherry picked from commit 816c4d3dcf99adcd40a03d93431237ddbd23bbdf)
tftpd: Drop unneeded includes.
MFC after: 3 days
Sponsored by: Klara, Inc.
Reviewed by: imp, markj
Differential Revision: https://reviews.freebsd.org/D45130
(cherry picked from commit 1111da6b7c612c571453a23a8dd02fd5e7e40b18)
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
(cherry picked from commit ae285a8cbf1212bdc1b3f81219635bc1395fadee)
tftpd: Satisfy clang-analyzer.
* Replace `random()` with `arc4random()`.
* Change some variable types.
* Drop some unused assignments.
MFC after: 3 days
Sponsored by: Klara, Inc.
Reviewed by: imp, markj
Differential Revision: https://reviews.freebsd.org/D45132
(cherry picked from commit 4d09eb87c5d5bec2e2832f50537e2ce6f75f4117)
tftpd: silence gcc overflow warnings
GCC 13 complains that we might be writing too much to an on-stack buffer
when createing a filename.
In practice there is a check that filename isn't too long given the
time format and other static characters so GCC is incorrect, but GCC
isn't wrong that we're potentially trying to put a MAXPATHLEN length
string + some other characters into a MAXPATHLEN buffer (if you ignore
the check GCC can't realistically evaluate at compile time).
Switch to snprintf to populate filename to ensure that future logic
errors don't result in a stack overflow.
Shorten the questionably named yyyymmdd buffer enough to slience the
warning (checking the snprintf return value isn't sufficent) while
preserving maximum flexibility for admins who use the -F option.
MFC after: 3 days
Sponsored by: Klara, Inc.
Reviewed by: brooks
Differential Revision: https://reviews.freebsd.org/D45086
(cherry picked from commit 25945af47e7a1d06c44c8c160045a866e90569ab)
* 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
(cherry picked from commit 1ed44fcc44b2c04db330663589541608135402f4)
tftpd: Clean up the tests.
MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: kevans
Differential Revision: https://reviews.freebsd.org/D44955
(cherry picked from commit 7ab7ecfcfe777f7816e3e01df5f277060b2b609a)
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
(cherry picked from commit 83a6e984ac01657819418746f722163367ec30db)
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
(cherry picked from commit 9f231af307b80eb222d9761bbd81fa4e130bb3d7)
tftpd: Untangle a conditional.
MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D45026
(cherry picked from commit 21b5829d28b1e02e9f30bfa439238c382dd19114)
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
Repeating the default WARNS here makes it slightly more difficult to
experiment with default WARNS changes, e.g. if we did something absolutely
bananas and introduced a WARNS=7 and wanted to try lifting the default to
that.
Drop most of them; there is one in the blake2 kernel module, but I suspect
it should be dropped -- the default WARNS in the rest of the build doesn't
currently apply to kernel modules, and I haven't put too much thought into
whether it makes sense to make it so.
Assume gcc is at least 6.4, the oldest xtoolchain in the ports tree.
Assume clang is at least 6, which was in 11.2-RELEASE. Drop conditions
for older compilers.
Reviewed by: imp (earlier version), emaste, jhb
MFC after: 2 weeks
Sponsored by: Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D24802
r358556 added alignas() use to the functional tests, which isn't defined
until C11. Raise the -std to C11 to fix the build under freebsd-gcc{6,9}.
Reported by: mhorne, Jenkins/CI
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
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
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
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
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
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
