Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v6 placeholder #2153

Open
wants to merge 2,627 commits into
base: master
Choose a base branch
from
Open

v6 placeholder #2153

wants to merge 2,627 commits into from

Conversation

PromoFaux
Copy link
Member

Getting ducks in a row...

simonkelley and others added 30 commits December 2, 2024 19:35
Signed-off-by: DL6ER <[email protected]>
Timeouts for TCP connections to non-responive servers are very long.
This in not appropriate for DNS connections.

Set timeouts for connection setup, sending data and recieving data.
The timeouts for connection setup and sending data are set at 5 seconds.
For recieving the reply this is doubled, to take into account the
time for usptream to actually get the answer.

Thanks to Petr Menšík for pointing out this problem, and finding a better
and more portable solution than the one in place heretofore.

Signed-off-by: DL6ER <[email protected]>
mkfifo /tmp/dnsmasq.pipe
dnsmasq --dumpfile=/tmp/dnsmasq.pipe ....
wireshark -i /tmp/dnsmasq.pipe

gives real-time display of network traffic in Wireshark.

Signed-off-by: DL6ER <[email protected]>
When deriving a domain name from an IPv6 address, an address
such as 1234:: would become 1234--.example.com, which is
not legal in IDNA2008. Stop using the :: compression method,
so 1234:: becomes
1234-0000-0000-0000-0000-0000-0000-0000.example.com

Signed-off-by: DL6ER <[email protected]>
A bug in gentoo linux https://bugs.gentoo.org/945183 reported that dnsmasq 2.90 fails to compile with GCC 15.

The issue is that while previous versions of GCC defaulted to the C17 standard and C23 could be selected with
"-std=c23" or "-std=gnu23", GCC 15 defaults to C23. In C23 incompatible pointer types are an error instead of
a warning, so the "int (*callback)()" incomplete prototypes cause errors.

For example, compiling dnsmasq 2.90 with gcc 14.2.1 and "-std=gnu23" fails with errors such as:
    lease.c: In function `lease_find_interfaces':
    lease.c:467:34: warning: passing argument 3 of `iface_enumerate' from incompatible pointer type [-Wincompatible-pointer-types[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wincompatible-pointer-types]]
      467 |   iface_enumerate(AF_INET, &now, find_interface_v4);
          |                                  ^~~~~~~~~~~~~~~~~
          |                                  |
          |                                  int (*)(struct in_addr,  int,  char *, struct in_addr,  struct in_addr,  void *)
    In file included from lease.c:17:
    dnsmasq.h:1662:50: note: expected `int (*)(void)' but argument is of type `int (*)(struct in_addr,  int,  char *, struct in_addr,  struct in_addr,  void *)'
     1662 | int iface_enumerate(int family, void *parm, int (callback)());
          |                                             ~~~~~^~~~~~~~~~~

This patch uses a typedef'ed union of pointer types to get type checking of the pointers. If that's too complicated,
another way might be to use (void *) casts to disable type checking.

Also, some of the IPv6 callbacks had "int preferred, int valid" and some had
"unsigned int preferred, unsigned int valid". This patch changes them all to "unsigned int"
so they're the same and to avoid casting "u32" to "int", eg:
    u32 preferred = 0xffffffff;
    callback(..., (int)preferred, ...)
Even if those cast values aren't used in the callback, casting u32 to "int" feels bad, especially if "int" is 32 bits.

Signed-off-by: DL6ER <[email protected]>
This is not supported, and doesn't behave as one might expect.

Signed-off-by: DL6ER <[email protected]>
Signed-off-by: DL6ER <[email protected]>
…ing of dnsmasq ("extra" + protocol information)

Signed-off-by: DL6ER <[email protected]>
…all runtime of the dhcp-discover tool

Signed-off-by: DL6ER <[email protected]>
Signed-off-by: DL6ER <[email protected]>
Ommision broke DHCP relay on *BSD.

Signed-off-by: DL6ER <[email protected]>
This acts almost exactly like --dhcp-option except that the defined option
is only sent when replying to PXE clients. More importantly, these
options are sent in reply PXE clients when dnsmasq in acting in PXE
proxy mode. In PXE proxy mode, the set of options sent is defined by
the PXE standard and the normal set of options is not sent. This config
allows arbitrary options in PXE-proxy replies. A typical use-case is
to send option 175 to iPXE. Thanks to Jason Berry for finding the
requirement for this.

Signed-off-by: DL6ER <[email protected]>
When using PXE proxy-DHCP, dnsmasq supplies PXE information to
the client, which also talks to another "normal" DHCP server
for address allocation and similar. The normal DHCP server may
be on the local network, but it may also be remote, and accessed via
a DHCP relay. This change allows dnsmasq to act as both a
PXE proxy-DHCP server AND a DHCP relay for the same network.

Signed-off-by: DL6ER <[email protected]>
Remove duplicated code and silence a harmless warning
…. The current implementation mixed it in a harmless way, however, it is generating a warning in GCC 14 and up. Also, reduce code duplication

Signed-off-by: DL6ER <[email protected]>
DL6ER and others added 30 commits February 6, 2025 06:01
We must only compare case when mapping an answer from upstream
to a forwarding record, not when checking a query to see if it's a
duplicate. Since the saved query name is scrambled, that ensures
that almost all such checks will wrongly fail.

Thanks to Peter Tirsek for an exemplary bug report for this.

Signed-off-by: DL6ER <[email protected]>
Fix a case sensitivity problem which has been lurking for a long while.
When we get example.com and Example.com and combine them, we send whichever
query arrives first upstream and then later answer it, and we also
answer the second with the same answer. That means that if example.com
arrives first, it will get the answer example.com - good - but Example.com
will _also_ get the answer example.com - not so good.

In theory, fixing this is simple without having to keep seperate
copies of all the queries: Just use the bit-vector representation
of case flipping that we have for 0x20-encoding to keep the
differences in case. The complication comes from the fact that
the existing bit-vector code only holds data on the first 32 alpha
letters, because we only flip that up to many for 0x20 encoding.

In practise, the delta between combined queries can almost always
be represented with that data, since almost all queries are
all lower case and we only purturb the first 32 letters with
0x20 encoding. It's therefore worth keeping the existing,
efficient data structure for the 99.9% of the time it works.
For the 0.1% it doesn't, however, one needs an arbitrary-length data
structure with the resource implications of that.

Thanks to Peter Tirsek for the well researched bug report which set me
on to these problems.

Signed-off-by: DL6ER <[email protected]>
Some of my PA-RISC UNIX machines boot remotely via tftp, but dnsmasq
randomly fails to deliver (the identical file) to some of the machines.

I traced the issue and basically dnsmasq fails with error "unsupported
request from IP.x.y.z" (line 366 in tftp.c).

Here is an example package which is sent (516 hex bytes):
76 6d 6c 69 6e 75 78 00 6f 63 74 65 74 00 12 74 10 3c 00 00 00 00 00 01
a9 24 00 00 00 00 00 00 1e 38 00 00 00 00 00 00 1c a0 00 00 00 00 00 00
1d 08 00 00 00 00 00 00 1d 28 00 00 00 00 00 00 08 00 00 00 00 00 00 00
03 d8 00 00 00 00 00 00 00 00 00 00 00 00 00 00 1d 30 00 00 00 02 ff e0
00 00 00 00 03 60 a8 49 55 93 00 00 00 01 f0 d4 21 e4 00 00 00 00 00 00
1d 78 00 00 00 f0 f0 d8 51 38 00 00 00 f0 f0 d4 21 c0 00 00 00 00 00 00
00 00 00 00 00 00 00 01 aa b8 00 00 00 f0 f0 e9 62 7c 00 00 00 00 00 00
03 01 ff ff ff ff ff ff 03 00 ff ff ff ff ff ff ff ff 00 00 00 00 00 00
00 03 00 00 00 00 00 00 00 05 00 00 00 00 00 00 00 04 ff ff ff ff ff ff
ff ff 00 00 00 00 00 00 00 00 ff ff ff ff ff ff ff ff 00 00 00 00 00 00
00 05 00 00 00 00 00 00 1e 38 00 00 00 00 00 00 00 60 00 00 00 00 00 01
a6 68 00 00 00 00 00 00 00 03 00 00 00 00 00 00 00 ff 00 00 00 00 00 00
00 03 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 02 00 00 00 00 00 00
00 00 00 00 00 f0 f0 d8 4f 30 00 00 00 00 00 00 00 01 00 00 00 00 00 00
00 00 00 00 00 00 00 01 ae ec 00 00 00 00 00 00 1f 70 00 00 00 00 00 00
1e b8 00 00 03 60 a8 49 55 93 00 00 00 02 18 71 1a 00 00 00 00 00 00 00
00 03 00 00 00 00 00 00 00 03 00 00 00 00 00 00 1e 38 00 00 00 00 00 00
00 07 00 00 00 00 00 00 00 00 00 00 00 f0 f0 d2 f0 70 00 00 00 00 00 00
1f c0 00 00 00 f0 f0 d4 0b e8 00 00 00 00 00 00 00 01 00 00 00 00 00 00
00 60 ff ff ff fc 00 60 18 00 00 00 00 00 00 00 00 00 00 00 00 f0 f0 d8
8f d0 00 00 00 00 00 00 1f f8 00 00 00 00 00 00 00 00 00 00 00 f0 f0 d8
8d b8 00 00 00 00 00 00 1e e8 00 00

Please note the last 3 bytes: "e8 00 00".
If the 3rd last byte is "00", then dnsmasq works and it fails it it's "e8".

So, the bug is in line 366 of tftp.c:
   filename = next(&p, end)
Here filename gets the value NULL from next(), because the "end" variable is off-by-2.
The fix is to change line 363 to add an offset of 2:
  end = packet + 2 + len;

Signed-off-by: Helge Deller <[email protected]>
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2293793
Signed-off-by: DL6ER <[email protected]>
A retry to upstream DNS servers triggered by the following conditions

1) A query asking for the same data as a previous query which has not yet been answered.
2) The second query arrives more than two seconds after the first.
3) Either the source of the second query or the id field differs from the first.

fails to set the case of the retry to the same pattern as the first attempt.

However dnsmasq expects the reply from upstream to have the case
pattern of the first attempt.

If the answer to the retry arrives before the answer to the first
query, dnsmasq will notice the case mismatch, log an error, and
ignore the answer.

The worst case scenario would be the first upstream query or reply is
lost and there would follow a short period where all queries for that
particular domain would fail.

This is a 2.91 development issue, it doesn't apply to previous stable releases.

Signed-off-by: DL6ER <[email protected]>
…possible detection (and startup prevention) of legit long-lived other processes like "pihole-FTL sqlite3", etc.

Signed-off-by: DL6ER <[email protected]>
…running duplicates don't interfere with each other. This can be seen as the fallback solution in case the PID file-based duplicate detection did not work due to security restrictions concerning process deetection on the system (see comment in function daemon.c:another_FTL() for further context)

Signed-off-by: DL6ER <[email protected]>
To complement the previous one, which fixed the retry path
when the query is retried from a different id/source address, this
fixes retries from the same id/source address.

Signed-off-by: DL6ER <[email protected]>
… processes's state as the latter may not be allowed if the other process is running as another user and we don't have CAP_KILL (or am root)

Signed-off-by: DL6ER <[email protected]>
…dhcp-discover routine to prevent hanging threads from being able to cause the tool to stall forever

Signed-off-by: DL6ER <[email protected]>
Add proper timeout handling to dhcp-dicsover feature
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants