Re: [Dnsmasq-discuss] Fwd: Fwd: [PATCH] Addressing hostsdir shortcomings

2022-10-16 Thread Simon Kelley

Patches applied with some changes.

Simon.

On 15/10/2022 07:57, Dominik Derigs wrote:

Hey all,

and here comes the third resubmission of my patches. I do still
believe that they are improvements. Even one year after writing
them, them do still apply cleanly on the master branch.

Best,
Dominik

 Forwarded Message 
From: Dominik Derigs 
To: dnsmasq-discuss@lists.thekelleys.org.uk
, Simon Kelley

Subject: Fwd: [PATCH] Addressing hostsdir shortcomings
Date: Sat, 02 Apr 2022 21:32:30 +0200

Dear Simon,

Second resubmission of my patches.
They still apply cleanly to current master.

Best,
Dominik

 Forwarded Message 
From: Dominik Derigs 
To: dnsmasq-discuss@lists.thekelleys.org.uk
, Simon Kelley

Subject: [PATCH] Addressing hostsdir shortcomings
Date: Sat, 08 Jan 2022 11:45:32 +0100

Hey Simon,

dnsmasq v2.73 added --hostsdir which is an efficient way of re-
loading only parts of the cache. When we tried to use hostsdir
yesterday, we identified three problems. They are described
below. Patches addressing them are attached.

--- ISSUE 1 --- Logging imprecision

Assume you have multiple files in hostsdir, dnsmasq can only log
the directory not the file that was the real source:

dnsmasq: read /home/test/hostsdir/hosts1 - 1 addresses
dnsmasq: read /home/test/hostsdir/hosts2 - 1 addresses
dnsmasq: read /home/test/hostsdir/hosts3 - 1 addresses

dnsmasq: 1 127.0.0.1/34170 query[A] aaa from 127.0.0.1
dnsmasq: 1 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 1 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.1
dnsmasq: 1 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2

This happens because the cache entries all use the same index
that is the directory name.

--- ISSUE 2 --- Outdated entries are not removed

When hostsdir re-reads the file, it does not remove outdated
entries. Assume you modify "192.168.1.1 aaa" to "192.168.1.2
aaa", dnsmasq will now serve two A records for "aaa". This may be
considered okay, however, if I add "192.168.1.1 bbb", PTR
requests for this domain will still be replied with "aaa" which
might be completely outdated information.

--- ISSUE 3 --- Ever growing replies under certain situations

When a users uses an editor that creates (temporary) files during
editing (like "sed -i") or uses a script that writes files line
by line (like "echo '' >> file"), they can quickly end up with
strange things like

dnsmasq: 3 127.0.0.1/34170 query[A] aaa from 127.0.0.1
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.1
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2

which is not very meaningful. We check for duplicates before
inserting into the cache, however, duplicate checking can be
foiled here: add_hosts_entry() calls cache_find_by_name() only
once (say it returned "192.168.1.1") so the memcmp() on the
address fails and we can add an arbitrary amount of 192.168.1.2
entries.

For addressing issue 1, I added a new struct *dyndir having a
linked list of struct *hostsfile. With this, cache_insert() can
get the correct index. If a file is newly added, we just add a
new *hostsfile entry to the list (index++).

Issue 2 is an easy one as we can selectively clean the cache when
we know the uid to be removed. This can be called before running
read_hostsfile() to insert new stuff. I added MOVE_FROM and
DELETE to inotify_add_watch() so we catch if a file was removed.
In this case, we only remove old entries.

Issue 3 is fixed by adding a loop over cache_find_by_name() in
add_hosts_entry() to check possible multiple records.

Best,
Dominik

[sent earlier as
https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2021q3/015704.html
,
resubmitting patches rebased on latest master]




___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss


[Dnsmasq-discuss] Fwd: Fwd: [PATCH] Addressing hostsdir shortcomings

2022-10-15 Thread Dominik Derigs
Hey all,

and here comes the third resubmission of my patches. I do still
believe that they are improvements. Even one year after writing
them, them do still apply cleanly on the master branch.

Best,
Dominik

 Forwarded Message 
From: Dominik Derigs 
To: dnsmasq-discuss@lists.thekelleys.org.uk
, Simon Kelley

Subject: Fwd: [PATCH] Addressing hostsdir shortcomings
Date: Sat, 02 Apr 2022 21:32:30 +0200

Dear Simon,

Second resubmission of my patches.
They still apply cleanly to current master.

Best,
Dominik

 Forwarded Message 
From: Dominik Derigs 
To: dnsmasq-discuss@lists.thekelleys.org.uk
, Simon Kelley

Subject: [PATCH] Addressing hostsdir shortcomings
Date: Sat, 08 Jan 2022 11:45:32 +0100

Hey Simon,

dnsmasq v2.73 added --hostsdir which is an efficient way of re-
loading only parts of the cache. When we tried to use hostsdir
yesterday, we identified three problems. They are described
below. Patches addressing them are attached.

--- ISSUE 1 --- Logging imprecision

Assume you have multiple files in hostsdir, dnsmasq can only log
the directory not the file that was the real source:

dnsmasq: read /home/test/hostsdir/hosts1 - 1 addresses
dnsmasq: read /home/test/hostsdir/hosts2 - 1 addresses
dnsmasq: read /home/test/hostsdir/hosts3 - 1 addresses

dnsmasq: 1 127.0.0.1/34170 query[A] aaa from 127.0.0.1
dnsmasq: 1 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 1 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.1
dnsmasq: 1 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2

This happens because the cache entries all use the same index
that is the directory name.

--- ISSUE 2 --- Outdated entries are not removed

When hostsdir re-reads the file, it does not remove outdated
entries. Assume you modify "192.168.1.1 aaa" to "192.168.1.2
aaa", dnsmasq will now serve two A records for "aaa". This may be
considered okay, however, if I add "192.168.1.1 bbb", PTR
requests for this domain will still be replied with "aaa" which
might be completely outdated information.

--- ISSUE 3 --- Ever growing replies under certain situations

When a users uses an editor that creates (temporary) files during
editing (like "sed -i") or uses a script that writes files line
by line (like "echo '' >> file"), they can quickly end up with
strange things like

dnsmasq: 3 127.0.0.1/34170 query[A] aaa from 127.0.0.1
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.1
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2
dnsmasq: 3 127.0.0.1/34170 /home/test/hostsdir aaa is 192.168.1.2

which is not very meaningful. We check for duplicates before
inserting into the cache, however, duplicate checking can be
foiled here: add_hosts_entry() calls cache_find_by_name() only
once (say it returned "192.168.1.1") so the memcmp() on the
address fails and we can add an arbitrary amount of 192.168.1.2
entries.

For addressing issue 1, I added a new struct *dyndir having a
linked list of struct *hostsfile. With this, cache_insert() can
get the correct index. If a file is newly added, we just add a
new *hostsfile entry to the list (index++).

Issue 2 is an easy one as we can selectively clean the cache when
we know the uid to be removed. This can be called before running
read_hostsfile() to insert new stuff. I added MOVE_FROM and
DELETE to inotify_add_watch() so we catch if a file was removed.
In this case, we only remove old entries.

Issue 3 is fixed by adding a loop over cache_find_by_name() in
add_hosts_entry() to check possible multiple records.

Best,
Dominik

[sent earlier as
https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2021q3/015704.html
,
resubmitting patches rebased on latest master]


From 7873cc3dbfce3edeb534bf4d0a0030894aaa152a Mon Sep 17 00:00:00 2001
From: Dominik Derigs 
Date: Wed, 29 Sep 2021 08:22:05 +0200
Subject: [PATCH 1/3] Extend hostsdir to store the individual files as sources
 for loggin

Signed-off-by: DL6ER 
---
 src/cache.c   |   9 +++--
 src/dnsmasq.h |  13 ++-
 src/inotify.c | 103 ++
 src/option.c  |  40 
 4 files changed, 111 insertions(+), 54 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 246c3f2..e86d69b 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -1839,6 +1839,7 @@ void dump_cache(time_t now)
 char *record_source(unsigned int index)
 {
   struct hostsfile *ah;
+  struct dyndir *dd;
 
   if (index == SRC_CONFIG)
 return "config";
@@ -1850,9 +1851,11 @@ char *record_source(unsigned int index)
   return ah->fname;
 
 #ifdef HAVE_INOTIFY
-  for