-
Notifications
You must be signed in to change notification settings - Fork 554
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
Replace "whitelist" and "blacklist" terms by "allowlist" and "blocklist"/"denylist" #4379
Comments
Looks like a duplicate of #3447. |
@glitsj16 Well spotted. I did search before posting but it didn't turn up. |
@ned64 That does happen sometimes. Especially with 'keywords' like whitelist and blacklist being all over our code base it can help to run a search through the web search engine of your choosing instead of through the GitHub search. |
Tentative move to allow/deny under way. I put a message on the main README.md, and redirected the old #3447 issue here. |
I'm done for now. Everything should be working as before. blacklist/whitelist are set as aliases, nothing should break. In src/tools/profcleaner.c I have a small tool you can use to convert your local profiles - details in the .c file. Any suggestions in man pages just put them in git. Probably there will be several iterations until we get them right. Hold on on changes throughout the wiki until we release it. |
Phase 2 "deny" seems to be working, but "allow" sucks! I was thinking... In Firefox we have the home directory, which is not a real directory, but kind of a virtual one. So instead of "allow" let's put "virtual". How does it sound? Any other ideas? Also, do we change file names such as whitelist-common.inc? Git blame/history might get confusing. |
The thing that makes me worry more it that renaming can break user overrides. We can leave redirect-aliases so
Other notes:
|
strict-allow is fine, the best so far! Let's say we wait until Monday morning in case somebody comes with another idea, and I'll modify the code and the profiles. Then you go in for --help, vim syntax, profile template (we missed it in the first round) and whatever else. |
@netblue30 commented 12 hours ago:
@netblue30 commented 11 hours ago:
Hello, I have a proposal for improving the syntax/semantics of the I haven't submitted it before because the write-up was a little convoluted and Anyway, I'll try to do so by Monday (probably on a new issue), so please don't |
FWIW: There was already a discussion about improved semantic in #3447. |
Let's keep track of them: from #4388
|
Sorry for not delivering on time. The problems and solutions themselves are Here's the current status:
So please bear with me for a few more days (I'm aiming for Friday). |
(Offtopic)
If you also think that this automatic expansion from |
Thanks @kmk3, absolutely no rush! Let's wait until August 1st, and if necessary we move it again. |
I have made great and steady progress on the text; this is the current status:
Additionally, today, after copying the quotes and thinking a long time about The 2 main problems remain solved either way, but this curve ball means that So, to avoid further delaying what I think is the most urgent step, I have (Offtopic) @kmk3 commented 3 days ago:
By the way, I have found a workaround for this: manually linking the issues: Issue [#3447][3447].
[3447]: https://github.com/netblue30/firejail/issues/3447 So funnily enough, the new troublesome non-standard markdown extension has led |
This reverts commit fe0f975. Note: This only reverts the changes from etc. The 4 aliases introduced on commit 45f2ba5 are mere, well, aliases. That is, they fail to address the different usability problems discussed on [netblue30#3447][3447] and in fact only make things more confusing (as has already been mentioned on [this][4379] and later comments). The main reason is that the aliases do not meaningfully map to the original commands. For example, the commands from each pair below seem like they would do the exact same thing: * `allow` and `nodeny` * `deny` and `noallow` Additionally, if these aliases are not the final commands, but only a test/work-in-progress, then keeping the wide-scale search/replace changes made on commit fe0f975 would only serve to cause confusion, as users of firejail-git, contributors and downstream projects might start changing the commands used on their profiles, only to later have to change them again, potentially to completely different commands. The sooner this is undone the better, as (besides the above reasons) the more profile changes there are between the original commit and the revert, the harder it is to e.g.: `git diff` versions of files across the following revision ranges: before the commit, after the commit but before the revert and after the revert. Note: This is still the case even if a commit is [ignored by `git blame`][4390]. So let us revert fe0f975 and only reapply similar large-scale changes once we have discussed and settled on better commands. How the revert was applied: Despite using the auto-generated message from `git revert`, to ensure correctness and to avoid conflicts the changes were reverted in different steps: Firstly, revert the files which can be safely reverted directly ("filestorevert"): # Find out which files have been changed on fe0f975, but have not # been changed afterwards and list them on "filestorevert" git show --pretty='' --name-only fe0f975 -- etc | LC_ALL=C sort >allfiles git diff --name-only fe0f975..master -- etc | LC_ALL=C sort >filestoignore comm -2 -3 allfiles filestoignore >filestorevert # Note: There are 3 extra files on filestoignore because they were # added after commit fe0f975 wc -l allfiles filestoignore filestorevert | head -n 3 # 797 allfiles # 8 filestoignore # 792 filestorevert # Automatically revert files in "filestorevert" # See https://stackoverflow.com/a/23401018/10095231 tr '\n' '\000' <filestorevert | xargs -0 git show fe0f975 -- | git apply --reverse printf 'Total files reverted:\n' git diff --name-only | wc -l # 792 Secondly, do some search/replace on the rest: tr '\n' '\000' <filestoignore | xargs -0 sed -i.bak \ -e 's/allow /whitelist /' -e 's/noallow /nowhitelist /' \ -e 's/deny /blacklist /' -e 's/nodeny /noblacklist /' \ -e 's/deny-nolog /blacklist-nolog /' find etc -name '*.bak' -print0 | xargs -0 rm Thirdly, verify the result. The following command shows the difference between all the changes in etc from before fe0f975 and this commit (inclusive): git diff fe0f975~1 -- etc From the output, it looks like all alias changes are fully reverted and that the other changes to etc (from after fe0f975) remain, so the revert seems to be done correctly. [3447]: netblue30#3447 [4379]: netblue30#4379 (comment) [4390]: netblue30#4390
So yesterday I remembered that Status:
A few days were spent on #4410 and the rest is basically the same. There are @kmk3 commented 11 days ago:
I was mistaken about this; while I did not find it explicitly described, there |
The solution section had many disjoint paragraphs and after trying to also I have spent a large portion of the time trying out the new commands on This helped a lot to notice redundancies/inconsistencies, so the Some questions came up regarding how whitelisting-related commands, running
While the former is still true, the latter was not quite correct and requires On the upside, there are fewer commands proposed now, as many of them were due Summary of the changes that I remember:
Also, there are some discussions that I intend to open separately:
I'd like to open some of them beforehand (even if no answers are posted), to be I know it's already past the mentioned August 1st date, but there is still work |
There has been a debate in the IT sec community for a while now about the terms "whitelist" and "blacklist", see e.g. here
dice.com article: ‘Whitelist,’ ‘Blacklist’: The New Debate Over Security Terminology
While certainly not meant by firejail or others in any racial or other negative sense there is at least a potential problem of connotation. "Black" items are to excluded/blocked, while "white" ones will be included/accepted. In a world where there are racism problems in most societies - many of which are about dark and light skin colour - this can be experienced by some as just one more implicit bias of white/black being good/bad.
I therefore suggest that
--blacklist
options and--whitelist
options (including all derivatives) be renamed--blocklist
and--allowlist
(etc.) as some companies have already started doing.Alternatively one could use derivatives of
allow
anddeny
- terms known from Apache (httpd.conf
is where I first came across them decades ago).In whatever way the change is implemented, the old options would need to be kept (but deprecated a.s.a.p.) as alternative names for existing user configurations not to break, perhaps for quite some time. In all own releases the replacement should be complete, though, including contributed profiles and documentation.
The text was updated successfully, but these errors were encountered: