RegEx engine improvements

General
,
49

OK now I got it.... tricky case

50

Use a negation for the query type, like:

.*;querytype=!A

I just added this.

51

image
image2179×235 9.87 KB

image
image2169×104 4.85 KB

image
image2235×156 6.58 KB

appears to be working, test (time) will tell if the device(s) continue to work without problems...

52

Thanks for testing. Just keep in mind that this is about the new regex feature not about experience exchange about how devices behave under this or that DNS protocol surgery. :wink:

54

quick question

Do the branches new/tre-regex and new/cname_inspection_logging contain all the functionality that pihole v5.1.1 offers?

I installed pihole v5.1.1, but now want to continue testing .*;querytype=!A
So far, it has been tested successfully on chromecast, ps4 and a windows 7 computer that can only access other local devices (no internet connections allowed, blocked on firewall). I'm expecting to have to whitelist PTR queries, some connections on the win 7 to other local devices fail at the first try.

55

Short answer: "no"

Longer answer:

new/tre-regex compared with release/v5.1

new/cname_inspection_logging compared with release/v5.1

FTL 5.1.1 has not been released (yet), only the core is at 5.1.1

57

This summarizes everything. FTL v5.1.1 will come sooner than later (maybe even today, maybe Monday, who knows) and once it is out, I will merge everything into the various feature branches floating around. And, finally, they will be marked as ready for merging into development a few days later. Even if they contain some subtle bugs we haven't seen so far, it is more easy to have everything in one place and fix it there. We need to test their (possible) interaction anyway at some point.

59

Thanks, I'll resume testing, once the new dev branch is up to date, which will allow me to test both new/tre-regex and new/cname_inspection_logging

give us (all testers) a signal when this is done please.

61

Update:

  • new/tre-regex has been updated to the latest version of the code (release/v5.2)
  • new/cname_inspection_logging is already included in here through the merged PR
62

thanks for the heads up, restarted testing, looks OK

image
image1918×637 23.7 KB

image
image2117×222 12.6 KB

and CNAME logging...

Jul 29 18:15:25 dnsmasq[25804]: query[A] fonts.gstatic.com from 192.168.2.228
Jul 29 18:15:25 dnsmasq[25804]: forwarded fonts.gstatic.com to fdaa:bbcc:ddee:2::5552
Jul 29 18:15:25 dnsmasq[25804]: reply fonts.gstatic.com is <CNAME>
Jul 29 18:15:25 dnsmasq[25804]: reply gstaticadssl.l.google.com is blocked during CNAME inspection

edit
As expected (see earlier), need to allow PTR queries: .*;querytype=PTR
/edit

1 Like
63

Ah, right. So it would be a good idea to print a message to the Pi-hole diagnostics are when we compile (at least one) regex that could block PTR requests for (at least) one client.

edit Added a warning

64

I assume you mean this:

[2020-08-09 11:43:34.781 9052M] REGEX WARNING: Invalid regex blacklist filter ".*;querytype=!A": This regex may cause name resolution issues (blocking PTR requests)
[2020-08-09 11:43:34.806 9052M] REGEX WARNING: Invalid regex whitelist filter ".*;querytype=PTR": This regex may cause name resolution issues (blocking PTR requests)

I've tested immediately,
blacklist regex: .*;querytype=!A

image
image749×59 2.01 KB

whitelist regex: .*;querytype=PTR
image
image815×55 4.54 KB

the messages are somewhat confusing:

  • blacklist message: regex isn't invalid, regex might cause network problems...
  • whitelist message: regex isn't invalid, it's a whitelist, so it solves problems...

These warnings are also visible on the tools / Pi-hole diagnosis screen:

Encountered an error when processing blacklist regex filter with ID 0:

image
It's NOT an ERROR, the regex compiles OK, it should be a WARNING...

I know, all of this is cosmetic, everything works as expected, but it may cause other users to report a problem...

edit
the ID on the Pi-hole diagnosis screen is wrong
/edit

65

Hmm, yes, maybe it would be better to not include the warning at all. When regex are compiled, we cannot know if there is another regex coming later which is "fixing" this. I also thought about that users may intentionally want to do this (block PTR requests) and they don't want to see such a warning.

Probably better to undo this, there is also the option ;invert which is not really covered here.

As always, thanks for testing, I'm more and more feeling we should revert this. The complexity of getting all the possible negations does not seem to be justified given that this is unproblematic - users will still see the regex link showing them why the PTRs have been blocked.

66

Now that pihole 5.1.2 (and pihole-FTL 5.2) has been released, I'm again wondering if the new/tre-regex branch has everything on board, that v5.2 has.

Please let us know if (and when) the branch is updated to v5.2 + regex engine improvements, this to allow us to continue testing.

The last update of the branch did not only upgrade FTL but also made core and web changes, a heads up, if this is going to happen would be appreciated.

Thanks.

67

Yes, there were no changes to FTL v5.2, it was sitting still for quite some time before being released. So this branch is already up-to-date.

It picked up the updates for the other branches. There are no changes in core/AdminLTE related to this.

I assume they will soon mark the regex and client-recognition improvements (MAC, etc.) into development. When they have confirmation from us, it may speed this up. I can only say it is working flawlessly for me (both branches). For further changes there is no need to delay this further. Even when I think this is done, any more changes can easily be done separately into development. This will even be better in terms of being reviewable.

68

Yes, this is correct.

69

The new/tre-regex branch has now been merged into development and will not receive any further updates. Please change to development using

pihole checkout dev

(this will also change web and core to development, but this is how this branch is expected to work, no issues expected, so far)

Something else: Due to the merge into development, the branch new/mac_clients picked up the new/tre-regex changes as well now. So if you change to that branch (new/mac_clients), you can already test both changes in combination (plus also the ECS (EDNS Client Subnet) identification if you can use that). I had to fix quite a number of merge conflicts, however, our automated test suite suggests that everything works well.

2 Likes
70

Is it possible to inverse a regex? So \.de$ as inversed regex in the blacklist will only allow ".de" domains.
I tried something like this, but it doesn't work: \.((?!(com|de|org|net|ms|to|eu|at)).)*$

71

Yes.

\.de$;invert
2 Likes
72

Thank you, works great! I missed this in the docu. :wink: