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

Replaces 'Whitelist' with 'Allowlist' #8305

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function totalsAction($maximum)
}
}

$parsed['whitelisted_domains'] = array_keys($nodes['dnsbl']['whitelists']);
$parsed['allowlisted_domains'] = array_keys($nodes['dnsbl']['allowlists']);
Copy link
Member

Choose a reason for hiding this comment

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

return key is ok access key is difficult, GUI changes are probably ok either way

Copy link
Author

Choose a reason for hiding this comment

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

ack, I'll remove.

$parsed['blocklisted_domains'] = array_keys($nodes['dnsbl']['blocklists']);

return $parsed;
Expand Down Expand Up @@ -124,7 +124,7 @@ public function searchQueriesAction()
}

$response = $this->searchRecordsetBase($parsed);
$response['whitelisted_domains'] = array_keys($nodes['dnsbl']['whitelists']);
$response['allowlisted_domains'] = array_keys($nodes['dnsbl']['allowlists']);
$response['blocklisted_domains'] = array_keys($nodes['dnsbl']['blocklists']);

return $response;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public function updateBlocklistAction()

// strip off any trailing dot
$value = rtrim($domain, '.');
$wl = explode(',', (string)$mdl->dnsbl->whitelists);
$wl = explode(',', (string)$mdl->dnsbl->allowlists);
Copy link
Author

Choose a reason for hiding this comment

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

$wl could have been changed to $al too, I guess.

Copy link
Member

Choose a reason for hiding this comment

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

here is the thing I was afraid of: changing model field names forces data loss for everyone using the feature

Copy link
Author

Choose a reason for hiding this comment

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

good catch.

$bl = explode(',', (string)$mdl->dnsbl->blocklists);

$existing_domains = explode(',', (string)$item);
Expand All @@ -71,10 +71,10 @@ public function updateBlocklistAction()
}

// Check if domains should be switched around in the model
if ($type == 'whitelists' && in_array($value, $bl)) {
if ($type == 'allowlists' && in_array($value, $bl)) {
$mdl->dnsbl->blocklists = $remove($bl, $value);
} elseif ($type == 'blocklists' && in_array($value, $wl)) {
$mdl->dnsbl->whitelists = $remove($wl, $value);
$mdl->dnsbl->allowlists = $remove($wl, $value);
}

// update the model
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
<help>List of domains from where blocklist will be downloaded.</help>
</field>
<field>
<id>unbound.dnsbl.whitelists</id>
<label>Whitelist Domains</label>
<id>unbound.dnsbl.allowlists</id>
<label>Allowlist Domains</label>
Copy link
Member

Choose a reason for hiding this comment

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

start with the label only...

<type>select_multiple</type>
<style>tokenize</style>
<allownew>true</allownew>
<help>List of domains to whitelist. You can use regular expressions.</help>
<help>List of domains to allowlist. You can use regular expressions.</help>
Copy link
Member

Choose a reason for hiding this comment

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

... and the help text.

</field>
<field>
<id>unbound.dnsbl.blocklists</id>
Expand Down
46 changes: 23 additions & 23 deletions src/opnsense/mvc/app/views/OPNsense/Unbound/overview.volt
Original file line number Diff line number Diff line change
Expand Up @@ -383,10 +383,10 @@
ajaxGet('/api/unbound/overview/isBlockListEnabled', {}, function(bl_enabled, status) {
/* reverse_domains refers to the domains for which the opposite action should take place,
* e.g. if a domain is presented that has been blocked N amount of times, but has been
* whitelisted at a later point in time, the action should be to block it, not whitelist it.
* allowlisted at a later point in time, the action should be to block it, not allowlist it.
*/
for (let i = 0; i < 10; i++) {
let class_type = type == "pass" ? "block-domain" : "whitelist-domain";
let class_type = type == "pass" ? "block-domain" : "allowlist-domain";
let icon_type = type == "pass" ? "fa fa-ban text-danger" : "fa fa-pencil text-info";
let domain = Object.keys(data)[i];
let statObj = Object.values(data)[i];
Expand All @@ -403,7 +403,7 @@
let stripped = domain.replace(/\.$/, "");
if (reverse_domains.has(stripped)) {
icon_type = type == "pass" ? "fa fa-pencil text-info" : "fa fa-ban text-danger";
class_type = type == "pass" ? "whitelist-domain" : "block-domain";
class_type = type == "pass" ? "allowlist-domain" : "block-domain";
}

let icon = '<button type="button" class="'+ class_type + '" data-value="'+ domain +'" ' +
Expand Down Expand Up @@ -436,7 +436,7 @@
$('#resolvedCounter').html(data.resolved.total + " (" + data.resolved.pcnt + "%)");

createTopList('top', data.top, 'pass', new Set(data.blocklisted_domains));
createTopList('top-blocked', data.top_blocked, 'block', new Set(data.whitelisted_domains));
createTopList('top-blocked', data.top_blocked, 'block', new Set(data.allowlisted_domains));

$('#top li:nth-child(even)').addClass('odd-bg');
$('#top-blocked li:nth-child(even)').addClass('odd-bg');
Expand All @@ -447,7 +447,7 @@

function reset_tooltips() {
$(".block-domain").attr('title', "{{ lang._('Block Domain') }}").tooltip({container: 'body', trigger: 'hover'});
$(".whitelist-domain").attr('title', "{{ lang._('Whitelist Domain') }}").tooltip({container: 'body', trigger: 'hover'});
$(".allowlist-domain").attr('title', "{{ lang._('Allowlist Domain') }}").tooltip({container: 'body', trigger: 'hover'});
}

g_queryChart = null;
Expand Down Expand Up @@ -532,43 +532,43 @@
'domain': $(this).data('value'),
'type': 'blocklists'
}, function(data, status) {
btn.addClass('whitelist-domain').removeClass('block-domain').remove("i").html('<i class="fa fa-pencil text-info"></i>');
btn.addClass('allowlist-domain').removeClass('block-domain').remove("i").html('<i class="fa fa-pencil text-info"></i>');

btn.off('click').on('click', whitelist_cb);
btn.off('click').on('click', allowlist_cb);

// find all possible other elements containing this domain and update their classes
let elements = $("button[data-value='" + btn.data('value') + "']");
$.each(elements, function (key, value) {
let elem = $(value);
if(elem.hasClass("block-domain")) {
elem.addClass('whitelist-domain').removeClass('block-domain').remove("i").html('<i class="fa fa-pencil text-info"></i>');
elem.addClass('allowlist-domain').removeClass('block-domain').remove("i").html('<i class="fa fa-pencil text-info"></i>');

// remove event binding and bind the whitelist_cb
elem.off('click').on('click', whitelist_cb);
// remove event binding and bind the allowlist_cb
elem.off('click').on('click', allowlist_cb);
}
});

reset_tooltips();
});
};

let whitelist_cb = function() {
let allowlist_cb = function() {
$(this).remove("i").html('<i class="fa fa-spinner fa-spin"></i>');
let btn = $(this);
ajaxCall('/api/unbound/settings/updateBlocklist', {
'domain': $(this).data('value'),
'type': 'whitelists'
'type': 'allowlists'
}, function(data, status) {
btn.addClass('block-domain').removeClass('whitelist-domain').remove("i").html('<i class="fa fa-ban text-danger"></i>');
btn.addClass('block-domain').removeClass('allowlist-domain').remove("i").html('<i class="fa fa-ban text-danger"></i>');

btn.off('click').on('click', blocklist_cb);

// find all possible other elements containing this domain and update their classes
let elements = $("button[data-value='" + btn.data('value') + "']");
$.each(elements, function (key, value) {
let elem = $(value);
if(elem.hasClass("whitelist-domain")) {
elem.addClass('block-domain').removeClass('whitelist-domain').remove("i").html('<i class="fa fa-ban text-danger"></i>');
if(elem.hasClass("allowlist-domain")) {
elem.addClass('block-domain').removeClass('allowlist-domain').remove("i").html('<i class="fa fa-ban text-danger"></i>');

// remove event binding and bind the blocklist_cb
elem.off('click').on('click', blocklist_cb);
Expand All @@ -582,7 +582,7 @@
}

$(document).on('click', '.block-domain', blocklist_cb);
$(document).on('click', '.whitelist-domain', whitelist_cb);
$(document).on('click', '.allowlist-domain', allowlist_cb);

do_startup().done(function() {
$('.wrapper').show();
Expand All @@ -595,10 +595,10 @@
if (e.target.id == 'query_details_tab') {
$("#grid-queries").bootgrid('destroy');
ajaxGet('/api/unbound/overview/isBlockListEnabled', {}, function(bl_enabled, status) {
/* Map the command type (block/whitelist) to the current state of the assigned action as determined by the controller,
* except for cases where they are manually overridden in the Blocklist page (Block/Whitelist Domains).
/* Map the command type (block/allowlist) to the current state of the assigned action as determined by the controller,
* except for cases where they are manually overridden in the Blocklist page (Block/Allowlist Domains).
*/
let whitelisted_domains = null;
let allowlisted_domains = null;
let blocklisted_domains = null;
let grid_queries = $("#grid-queries").UIBootgrid({
search:'/api/unbound/overview/searchQueries/',
Expand All @@ -620,7 +620,7 @@
return request;
},
responseHandler: function (response) {
whitelisted_domains = new Set(response.whitelisted_domains);
allowlisted_domains = new Set(response.allowlisted_domains);
blocklisted_domains = new Set(response.blocklisted_domains);
return response;
},
Expand All @@ -642,15 +642,15 @@
let domain = row.domain.replace(/\.$/, "");
let btn = '';
let block = '<button type="button" class="btn-secondary block-domain" data-value=' + row.domain + ' data-toggle="tooltip"><i class="fa fa-ban text-danger"></i></button> ';
let pass = '<button type="button" class="btn-secondary whitelist-domain" data-value=' + row.domain + ' data-toggle="tooltip"><i class="fa fa-pencil text-info"></i></button>';
let pass = '<button type="button" class="btn-secondary allowlist-domain" data-value=' + row.domain + ' data-toggle="tooltip"><i class="fa fa-pencil text-info"></i></button>';

if (row.action == 'Pass') {
btn = block;
} else if (row.action == 'Block') {
btn = pass;
}

if (whitelisted_domains.has(domain)) {
if (allowlisted_domains.has(domain)) {
btn = block;
}

Expand Down Expand Up @@ -701,7 +701,7 @@

grid_queries.find(".block-domain").on('click', blocklist_cb);

grid_queries.find(".whitelist-domain").on('click', whitelist_cb);
grid_queries.find(".allowlist-domain").on('click', allowlist_cb);
});
})

Expand Down
16 changes: 8 additions & 8 deletions src/opnsense/scripts/unbound/blocklists/default_bl.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class DefaultBlocklistHandler(BaseBlocklistHandler):
def __init__(self):
super().__init__('/usr/local/etc/unbound/unbound-blocklists.conf')
self.priority = 100
self._whitelist_pattern = self._get_excludes()
self._allowlist_pattern = self._get_excludes()
Copy link
Member

Choose a reason for hiding this comment

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

the changes in this script are likely ok


def get_config(self):
cfg = {}
Expand All @@ -53,7 +53,7 @@ def get_blocklist(self):
for blocklist, bl_shortcode in self._blocklists_in_config():
per_file_stats = {'uri': blocklist, 'skip': 0, 'blocklist': 0, 'wildcard': 0}
for domain in self._domains_in_blocklist(blocklist):
if self._whitelist_pattern.match(domain):
if self._allowlist_pattern.match(domain):
per_file_stats['skip'] += 1
else:
if self.domain_pattern.match(domain):
Expand Down Expand Up @@ -81,13 +81,13 @@ def get_blocklist(self):
for key, value in self.cnf['include'].items():
if key.startswith('custom'):
entry = value.rstrip().lower()
if not self._whitelist_pattern.match(entry):
if not self._allowlist_pattern.match(entry):
if self.domain_pattern.match(entry):
result[entry] = {'bl': 'Manual', 'wildcard': False}
elif key.startswith('wildcard'):
entry = value.rstrip().lower()
if self.domain_pattern.match(entry):
# do not apply whitelist to wildcard domains
# do not apply allowlist to wildcard domains
result[entry] = {'bl': 'Manual', 'wildcard': True}

return result
Expand Down Expand Up @@ -142,7 +142,7 @@ def _blocklist_reader(self, uri):


def _get_excludes(self):
whitelist_pattern = re.compile('$^') # match nothing
allowlist_pattern = re.compile('$^') # match nothing
if self.cnf.has_section('exclude'):
exclude_list = set()
for exclude_item in self.cnf['exclude']:
Expand All @@ -152,15 +152,15 @@ def _get_excludes(self):
exclude_list.add(pattern)
except re.error:
syslog.syslog(syslog.LOG_ERR,
'blocklist download : skip invalid whitelist exclude pattern "%s" (%s)' % (
'blocklist download : skip invalid allowlist exclude pattern "%s" (%s)' % (
exclude_item, pattern
)
)
if not exclude_list:
exclude_list.add('$^')

wp = '|'.join(exclude_list)
whitelist_pattern = re.compile(wp, re.IGNORECASE)
allowlist_pattern = re.compile(wp, re.IGNORECASE)
syslog.syslog(syslog.LOG_NOTICE, 'blocklist download : exclude domains matching %s' % wp)

return whitelist_pattern
return allowlist_pattern
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ custom_{{loop.index}}={{uri}}
[exclude]
# exclude localhost entries
default_pattern_1=.*localhost$
{% if not helpers.empty('OPNsense.unboundplus.dnsbl.whitelists')%}
{% if not helpers.empty('OPNsense.unboundplus.dnsbl.allowlists')%}
Copy link
Member

Choose a reason for hiding this comment

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

model changes again here and below

Copy link
Author

Choose a reason for hiding this comment

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

ack, will remove.

# user defined
{% for pattern in OPNsense.unboundplus.dnsbl.whitelists.split(',') %}
{% for pattern in OPNsense.unboundplus.dnsbl.allowlists.split(',') %}
custom_pattern_{{loop.index}}={{ pattern }}
{% endfor %}
{% endif %}
Expand Down