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

dns/unbound-plus: new plugin #1495

Merged
merged 13 commits into from
Sep 13, 2019
Merged

dns/unbound-plus: new plugin #1495

merged 13 commits into from
Sep 13, 2019

Conversation

mimugmail
Copy link
Member

Initial release for unbound-plus, add DNSBL to native Unbound DNS without the need for bind or dnscrypt-proxy.

Future version will also offer DoT or whatever may come.

dnsbl.sh need chmod as usual.

Copy link
Member

@fichtner fichtner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a bit jammed in although it would probably be what core support would look like.

What worries me a little is generating include files loosely which can’t be deleted when plugin is uninstalled and Unbound can’t trigger their generation either. Web Proxy has a bit of plugin support that tries to correct the call flow. We have to look if the same is feasible here for Unbound.

@@ -0,0 +1,37 @@
<?php
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can’t we use Unbound namespace?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, would better fit when it moves to core

cat $(find ${WORKDIR} -type f -name "*.inc") /dev/null > ${DESTDIR}/dnsbl.conf
chown unbound:unbound ${DESTDIR}/dnsbl.conf
rm -rf ${WORKDIR}
pluginctl -s unbound restart
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

People will hate you for purging their caches ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a smarter way? :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not at the moment, no. but we'll work this out later :)

@mimugmail
Copy link
Member Author

As there is no rc.conf I used a stupid include, will have a look. I thought we agreed to start as plugin?


hbbtv() {
# HBBTV List
${FETCH} https://raw.githubusercontent.com/Akamaru/Pi-Hole-Lists/master/hbbtv.txt -o ${WORKDIR}/hbbtv-raw

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I checked the link here I got a 404 page error

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mimugmail
Copy link
Member Author

@fichtner I renamed it to Unbound but the ACL complains since it wants to overwrite the existing one in core. Any idea?:

Deinstallation has been requested for the following 1 packages (of 0 packages in the universe):

Installed packages to be REMOVED:
        os-unbound-plus-devel-0.1

Number of packages to be removed: 1
[1/1] Deinstalling os-unbound-plus-devel-0.1...
[1/1] Deleting files for os-unbound-plus-devel-0.1: 100%
Reloading plugin configuration
Installing os-unbound-plus-devel-0.1...
pkg: os-unbound-plus-devel-0.1 conflicts with opnsense-devel-20.1.a_120 (installs files into the same place).  Problematic file: /usr/local/opnsense/mvc/app/models/OPNsense/Unbound/ACL/ACL.xml

Failed to install the following 1 package(s): /root/plugins/dns/unbound-plus/work/pkg/os-unbound-plus-devel-0.1.txz
*** Error code 70

Stop.
make: stopped in /root/plugins/dns/unbound-plus

type:script
message:fetching DNSBLs

[dnsblcron]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's really no difference here in dnsbl and dnsblcron as %s is optional I think so you can merge the two

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I remeber we did it with %s in the beginning and now both is possible

@fichtner
Copy link
Member

I would like to reengineer some of this.. with the file and the template and stuff... I do believe template dirs can have subdirectories for plugins and that would help us to start a full reconfigure from core unbound :) but for now this is ready to merge... when you're ready let me know

@fichtner fichtner self-assigned this Sep 13, 2019
@mimugmail
Copy link
Member Author

For a 0.1 release it's good to merge :) Thanks for your time to review

@fichtner fichtner merged commit 28a001c into opnsense:master Sep 13, 2019
@fichtner
Copy link
Member

Merged, thanks!

@marjohn56
Copy link
Member

This is so cool... not had any time to play for a while but have a few days of peace so trying to catch up... just installed 20.* dev and installed this plugin. Well done, well done indeed.

@mimugmail
Copy link
Member Author

So thisi s the first user confirmation that it works? :)

@marjohn56
Copy link
Member

It passed the add blocker tests on a couple of sites. I've only added a few lists, but using some test sites it does seem to work...

@mimugmail
Copy link
Member Author

Great! Then I'll start adding some other features to it :)

@marjohn56
Copy link
Member

I've not tested the whitelist domains.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants