TFTP tweak.

Check sender of all received packets, as specified in RFC 1350 para 4.

My understanding of the example in the RFC is that it in fact only
applies to server-to-client packets, and packet loss or duplication
cannot result in a client sending from more than one port to a server.
This check is not, therefore, strictly needed on the server side.
It's still useful, and adds a little security against packet
spoofing. (though if you're running TFTP on a public network with
bad actors, nothing can really save you.)
This commit is contained in:
Simon Kelley
2021-03-30 21:21:09 +01:00
parent b5d1b20727
commit dfb1f7ccf1
2 changed files with 24 additions and 3 deletions

View File

@@ -65,6 +65,9 @@ version 2.85
Scale the size of the DNS random-port pool based on the Scale the size of the DNS random-port pool based on the
value of the --dns-forward-max configuration. value of the --dns-forward-max configuration.
Tweak TFTP code to check sender of all recieved packets, as
specified in RFC 1350 para 4.
version 2.84 version 2.84
Fix a problem, introduced in 2.83, which could see DNS replies Fix a problem, introduced in 2.83, which could see DNS replies

View File

@@ -39,6 +39,7 @@ static void sanitise(char *buf);
#define ERR_PERM 2 #define ERR_PERM 2
#define ERR_FULL 3 #define ERR_FULL 3
#define ERR_ILL 4 #define ERR_ILL 4
#define ERR_TID 5
void tftp_request(struct listener *listen, time_t now) void tftp_request(struct listener *listen, time_t now)
{ {
@@ -583,11 +584,27 @@ void check_tftp_listeners(time_t now)
for (transfer = daemon->tftp_trans; transfer; transfer = transfer->next) for (transfer = daemon->tftp_trans; transfer; transfer = transfer->next)
if (poll_check(transfer->sockfd, POLLIN)) if (poll_check(transfer->sockfd, POLLIN))
{ {
union mysockaddr peer;
socklen_t addr_len = sizeof(union mysockaddr);
ssize_t len;
/* we overwrote the buffer... */ /* we overwrote the buffer... */
daemon->srv_save = NULL; daemon->srv_save = NULL;
handle_tftp(now, transfer, recv(transfer->sockfd, daemon->packet, daemon->packet_buff_sz, 0));
if ((len = recvfrom(transfer->sockfd, daemon->packet, daemon->packet_buff_sz, 0, &peer.sa, &addr_len)) > 0)
{
if (sockaddr_isequal(&peer, &transfer->peer))
handle_tftp(now, transfer, len);
else
{
/* Wrong source address. See rfc1350 para 4. */
prettyprint_addr(&peer, daemon->addrbuff);
len = tftp_err(ERR_TID, daemon->packet, _("ignoring packet from %s (TID mismatch)"), daemon->addrbuff);
sendto(transfer->sockfd, daemon->packet, len, 0, &peer.sa, sa_len(&peer));
}
}
} }
for (transfer = daemon->tftp_trans, up = &daemon->tftp_trans; transfer; transfer = tmp) for (transfer = daemon->tftp_trans, up = &daemon->tftp_trans; transfer; transfer = tmp)
{ {
tmp = transfer->next; tmp = transfer->next;
@@ -736,7 +753,8 @@ static ssize_t tftp_err(int err, char *packet, char *message, char *file)
char *errstr = strerror(errno); char *errstr = strerror(errno);
memset(packet, 0, daemon->packet_buff_sz); memset(packet, 0, daemon->packet_buff_sz);
sanitise(file); if (file)
sanitise(file);
mess->op = htons(OP_ERR); mess->op = htons(OP_ERR);
mess->err = htons(err); mess->err = htons(err);