Bug 241068 (IPV4, dnsmasq, tftp)

Summary: dns/dnsmasq: IPv4 TFTP broken on dnsmasq after FreeBSD 11.2
Product: Ports & Packages Reporter: gpw928 <phil.chadwick>
Component: Individual Port(s)Assignee: Matthias Andree <mandree>
Status: Closed FIXED    
Severity: Affects Some People CC: dave.mueller, mandree
Priority: --- Flags: mandree: maintainer-feedback+
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
upstream fix attempt to get TFTP working none

Description gpw928 2019-10-05 00:04:51 UTC 
See: https://forums.freebsd.org/threads/dnsmasq-tftp-service-not-working-any-longer.72457/#post-440976

To reproduce the problem:

cd /usr/ports/dns/dnsmasq
make install

cat - >/usr/local/etc/dnsmasq.conf <<EOF
enable-tftp
tftp-root=/tmp
log-facility=/var/log/dnsmasq.log
EOF

echo "something" >/tmp/test.file

# Start in debug mode
/usr/local/sbin/dnsmasq -d -C /usr/local/etc/dnsmasq.conf

# Start a client transfer from another session on the same host:
tftp localhost
get test.file

It fails, repeatedly, on FreeBSD 11.3 and FreeBSD 12.0.  Works on FreeBSD 11.2.

The problem arises in tftp.c at line 195, when the return is taken because if_index is zero, and can not be translated to a name.  This results in dnsmasq refusing to service a client tftp request:

if (!indextoname(listen->tftpfd, if_index, namebuff))
    return;

The failure to set if_index correctly for BSD hosts occurs just prior to this  in the block of code commencing at line 159:

 #elif defined(IP_RECVDSTADDR) && defined(IP_RECVIF)
      if (listen->family == AF_INET)
        for (cmptr = CMSG_FIRSTHDR(&msg); cmptr; cmptr = CMSG_NXTHDR(&msg, cmptr))
          {
            union {
              unsigned char *c;
              struct in_addr *a;
              struct sockaddr_dl *s;
            } p;
            p.c = CMSG_DATA(cmptr);
            if (cmptr->cmsg_level == IPPROTO_IP && cmptr->cmsg_type == IP_RECVDSTADDR)
              addr.in.sin_addr = *(p.a);
            else if (cmptr->cmsg_level == IPPROTO_IP && cmptr->cmsg_type == IP_RECVIF)
              if_index = p.s->sdl_index;
          }

#endif

The value of if_index should be set by the above code, and it is not.
Comment 1 Matthias Andree freebsd_committer freebsd_triage 2019-10-05 06:55:13 UTC 
I'll be forwarding this upstream.
Comment 2 dave.mueller 2019-10-07 13:25:51 UTC 
Not sure if this is really the right fix, but I think the "control" array (around line 79 of tftp.c) should be declared like this:

#elif defined(IP_RECVDSTADDR) && defined(IP_RECVIF)
    char control[CMSG_SPACE(sizeof(struct in_addr)) +      /* IP_RECVDSTADDR */
                 CMSG_SPACE(sizeof(struct sockaddr_dl))];  /* IP_RECVIF */
#endif
Comment 3 Matthias Andree freebsd_committer freebsd_triage 2019-10-13 15:10:03 UTC 
Created attachment 208279 [details]
upstream fix attempt to get TFTP working

This patch is to apply against the ports tree in /usr/ports/dns/dnsmasq.

Please apply and see if it fixes the problem.

Reference:
http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2019q4/013412.html
Comment 4 commit-hook freebsd_committer freebsd_triage 2019-10-13 15:26:10 UTC 
A commit references this bug:

Author: mandree
Date: Sun Oct 13 15:25:56 UTC 2019
New revision: 514404
URL: https://svnweb.freebsd.org/changeset/ports/514404

Log:
  dns/dnsmasq: Fix TFTP regression on recent FreeBSD versions.

  Add a patch taken from upstream. Tested successfully on 11.3-RELEASE amd64.

  The upstream fix is an extended version of a fix proposed by
  Dave Mueller in the PR.

  See also:
  http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2019q4/013412.html

  PR:		241068
  Reported by:	Phil Chadwick
  Obtained from:	Simon Kelley, http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=936bd82755e8f75fc09c1e9a67fb390175b157d4
  MFH:		2019Q4

Changes:
  head/dns/dnsmasq/Makefile
  head/dns/dnsmasq/files/patch-src_tftp.c
Comment 5 commit-hook freebsd_committer freebsd_triage 2019-10-13 15:30:12 UTC 
A commit references this bug:

Author: mandree
Date: Sun Oct 13 15:29:17 UTC 2019
New revision: 514405
URL: https://svnweb.freebsd.org/changeset/ports/514405

Log:
  MFH: r514404 (blanket reliability fix)

  dns/dnsmasq: Fix TFTP regression on recent FreeBSD versions.

  Add a patch taken from upstream. Tested successfully on 11.3-RELEASE amd64.

  The upstream fix is an extended version of a fix proposed by
  Dave Mueller in the PR.

  See also:
  http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2019q4/013412.html

  PR:		241068
  Reported by:	Phil Chadwick
  Obtained from:	Simon Kelley, http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=936bd82755e8f75fc09c1e9a67fb390175b157d4

  Approved by:	ports-secteam (blanket)

Changes:
_U  branches/2019Q4/
  branches/2019Q4/dns/dnsmasq/Makefile
  branches/2019Q4/dns/dnsmasq/files/patch-src_tftp.c
Comment 6 Matthias Andree freebsd_committer freebsd_triage 2019-10-13 15:30:30 UTC 
Committed and MFH'd. Dnsmasq 2.80_4 should fix things. 
If it does not, please reopen the PR.