The current logs are a mishmash of prints to stdout and stderr. This
both presents a bad user experience and doesn't fold well with
integration into syslog or similar systems.
Similar to log_debug(), introduce a log_err() and log_info(). The prior
can be used to indicate errors in the server or events that the
implementer/owner of the system might benefit from paying attention to.
The latter can be used to provide useful insights into the what the
server is doing (e.g. what files does it serve).
The "[TQFTP]" prefix is removed from the strings, as they don't add any
value when routed through any form of logging framework.
Further polishing of the log entries, both in placement and content, is
expected but the baseline is less of a mishmash now.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
There's a few commented out development/debug prints lingering in the
code that can be useful to enable in order to get some basic insights
into what's going on in the server.
While more work is needed to determine what level of debug prints are
useful, and where they should be placed, introduce a '-d' argument to
tqftpserv and translate these to runtime-conditional prints for now.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
While servicing a read, the only valid messages to receive are OP_ACK
and OP_ERROR. The code correctly handles this in terms of cleaning up
resources and closing the socket, but it doesn't inform the client that
the transfer is terminated.
Resolve this by sending out an error message.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The creation of the oath message is mix of strcpy() and sprintf(). While
it won't overflow the fixed size buffer today, it's better to introduce
bounds check for the future, and to avoid the "dangerous" functions.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The server currently accepts any filename from clients without
validation, allowing directory traversal attacks using paths like
'../../../etc/passwd'. This could expose sensitive files outside the
intended serving directory.
Add a sanitize step to validate that the filename does not contain '..'
portions.
Log rejected paths for security auditing.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Replace fstat() with lseek(SEEK_END) for determining file size when
the tsize option is requested. This approach is more portable and
doesn't require struct stat.
Additionally, handle the case where lseek() fails (e.g., on unseekable
files like pipes or special devices). When this occurs, send a TFTP
ERROR with code 0 (Not defined) and a descriptive message to inform
the client that the file size cannot be determined.
After obtaining the size, reset the file position to the beginning
with lseek(SEEK_SET) to ensure subsequent reads start from the correct
offset.
Also fix resource leak: close the socket when file open fails, ensuring
proper cleanup in all error paths.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The current behavior when failing to write out a block of data to the
storage media is to print an error and return, presumably hoping that
once the client retries the "lost" packet there will suddenly be space
in the file system.
Instead communicate the error to the client, as expected.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Most of the error handling in RRQ and WRQ are "handled" by /* XXX */
comments. Implement some actual error handling, to not leak the
resources.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
While we don't actively tear down the client context on failure, when it
happens (as the remote processor or the client socket goes away), the
block buffer object will be freed.
tftp_send_data() freeing the buffer on failure will therefor result in a
double free, so leave it around.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
When pread() fails during an active file transfer, send a TFTP ERROR
packet to inform the client rather than silently aborting. This can
occur due to I/O errors, filesystem issues, or the file being truncated
during transfer.
Per RFC 1350, we use error code 0 (Not defined) with a descriptive
message, as there's no specific error code for read failures during
transfer. The error message now includes strerror(errno) to provide
detailed diagnostic information in the server logs.
This ensures the client receives proper notification of the failure
rather than timing out waiting for data that will never arrive.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Per RFC 1350, when the server cannot create or open a file for writing,
it must send an ERROR packet with error code 2 (Access violation) to
inform the client of the failure.
Previously, when translate_open() failed during a write request, we
would log the error locally but fail to notify the client. This left
the client waiting indefinitely for an acknowledgment that would never
arrive.
Now we properly send an error response using the standard TFTP error
mechanism, allowing the client to handle the failure appropriately
rather than timing out. This could occur due to permission issues,
disk full conditions, or other filesystem errors.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Per RFC 1350, TFTP supports multiple transfer modes (netascii, octet,
mail), but this server only implements octet mode. When a client
requests a different mode, we must send an ERROR packet with error
code 4 (Illegal TFTP operation) rather than silently dropping the
request.
Previously, we would log the rejection but fail to notify the client,
leaving it waiting indefinitely for a response that would never come.
Now we properly inform the client that only octet mode is supported,
allowing it to either retry with the correct mode or report a
meaningful error to the user.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Both handle_rrq() and handle_wrq() functions uses strlen() on the
filename and mode fields from network packets without first verifying
they were NUL-terminated and within the packet bounds. Malformed packets
could cause buffer overruns when these strings were processed.
Replace strlen() with strnlen() and verify both filename and mode
are properly NUL-terminated within the received packet buffer.
Reject malformed packets early with appropriate error logging.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The parse_options() function invokes strlen() without considering that
the incoming options might not be NUL-terminated, uses atoi() without
error checking and does not validate option values against RFC-specified
limits. This allows malicious clients to trick the server to read beyond
the buffer, request invalid values that could cause resource exhaustion
or integer overflow attacks.
Validate that each option is within bounds, then add RFC-compliant
constants for option limits (MIN/MAX_BLKSIZE per RFC 2348, MIN/MAX_WSIZE
per RFC 7440, and reasonable limits for custom options). Replace atoi()
with strtoll() for proper error detection. Validate all option values
and reject requests with out-of-range values, logging the invalid values
for diagnostics.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The tftp_send_error() function is going to be called in many more
places, rather than keeping it in the middle of the code and introduce a
forward declaration, move the function up.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Replace magic numbers with RFC-defined symbolic constants for TFTP
error codes throughout the codebase. This improves code readability
and maintainability by making the error semantics explicit.
Defined constants per RFC 1350 and RFC 2347:
- TFTP_ERROR_ENOENT (1): File not found
- TFTP_ERROR_EACCESS (2): Access violation
- TFTP_ERROR_EBADOP (4): Illegal TFTP operation
- TFTP_ERROR_EOPTNEG (8): Option negotiation failed
The use of symbolic names makes it immediately clear what error
condition is being reported, and ensures consistency across all
error reporting paths in the server.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The buffer type differs in signedness (buffer is unsigned, local
pointer is signed), resulting in build warnings even with default
settings.
Correct the local type to fix the warnings.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
If systemd-unit-prefix is passed, there is no need for the systemd build
dependency.
Allows building in postmarketOS, where the build environment can't
install systemd.
Signed-off-by: Achill Gilgenast <achill@achill.org>
The client sends data packets of blksize size and waits for an ACK
after sending wsize packets. If packets are not received in sequence,
the server should send NACK, prompting the client to resend the packets
starting from beginning of the window.
For example, with wsize = 10 and blksize = 5000, the client sends
blocks [201–210] and waits. If all are received, the server replies
with ACK; otherwise, it sends NACK, and the client restarts from block 201.
Use rw_buf to cache received blocks and track the next expected block
using blk_expected. Reset the expected block to the start of the window
if out-of-sequence packets are detected.
Once all buffers are received, commit the cached buffers to the file.
Signed-off-by: Bhaskar Valaboju <quic_bhaskarv@quicinc.com>
Use the blksize-sized buffer from the client instance for send
and recvfrom operations during TFTP packet handling.
This eliminates repeated memory allocation in handler_reader
and supports receiving full blksize payloads in handler_writer.
Signed-off-by: Bhaskar Valaboju <quic_bhaskarv@quicinc.com>
Allocate memory in the client instance to support transmission and
reception of TFTP data packets sized according to the negotiated
blksize option. This buffer ensures proper handling of variable-length
payloads during transfer operations.
Signed-off-by: Bhaskar Valaboju <quic_bhaskarv@quicinc.com>
