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

Replace whitelist and blacklist commands with better terms #3447

Closed
ClemMakesApps opened this issue Jun 2, 2020 · 28 comments
Closed

Replace whitelist and blacklist commands with better terms #3447

ClemMakesApps opened this issue Jun 2, 2020 · 28 comments
Labels
enhancement New feature request
Milestone

Comments

@ClemMakesApps
Copy link

Since Whitelist/Blacklist have connotations about value and map to racial terms, we should replace those commands in this project with Allowlist and Denylist or something similar.

@netblue30
Copy link
Owner

Are you kidding?

@Fred-Barclay
Copy link
Collaborator

Fred-Barclay commented Jun 4, 2020

@ClemMakesApps I appreciate the concern but let's don't go down this path. Our terminology is based upon the behavior/functionality of the code, nothing more, and reflects accepted terminology. There's no need to change code which is intrinsically neutral based upon non-existent connotations to the ratio of melanin in people's skin.

@NicoleSchwartz
Copy link

@Fred-Barclay It doesn't matter what the original intent was. People do have a negative reaction to these terms. There is no justification for keeping it.

Over time, although you may intend them neutrally they do have negative impact. Many projects and groups have moved to the equally descriptive terms that avoid making people uncomfortable but are still equally descriptive. see rails/rails#33677

"To compound the issue, it is also striking how often the term “whitelist” is used for a supposedly good, respectable, or safe list of publishers [20, 22, 27]. The racism in such “black is bad, white is good” metaphors is inappropriate and needs to cease. The black-white dualism explicit in these binary terms is often associated with Western thinking that is usually traced back to the work of Rene Descartes. Although the epistemological dualism of Descartes may be seen in earlier works by Plato and Aristotle, this way of thinking is often associated with the Enlightenment and the subsequent scientific revolution and industrial development [28–30]. Thus, a foundational ontological dualism accepted by many people in Western cultures includes the supposedly “natural” divides between subject-object, body-spirit, human-nature, and self-other. Such dualism extends into our conceptions of good-evil, sacred/divine-profane, and civilized-heathen/barbarian [31]." https://www.ncbi.nlm.nih.gov/pmc/articles/PMC6148600/

@glitsj16
Copy link
Collaborator

glitsj16 commented Jun 5, 2020

People do have a negative reaction to these terms. There is no justification for keeping it.

Some do and some don't, it's all in the 'Eye Of The Beholder'. There's no justification for not keeping it either, if this is nothing more than a plea to change a few words and call it quits. The conclusions drawn, and the subsequent (disputable) history lesson on ontological dualism feel out of place. Quoting research examining the emerging literature surrounding predatory publishing might buy one some 'social capital' (to quote another famous French male theorist), but what good does that bring besides a few likes?

Regardless of the philosophy one subscribes to (if any), I wonder if anyone here actually believes that changing terminology would make supported applications run any more secure? This project is open to PR's, which are always reviewed on their merit in relation to the goal: reducing potential privacy/security risks when using Linux applications on a daily basis. Feel free to open. Another option is forking, like glimpse-editor did for example.

@rusty-snake
Copy link
Collaborator

noblacklist/blacklist/whitelist are the biggest challenge when learning to customize firejail-profiles. Regardless of the original idea, this is also a reasons to change them.

What is currently difficult?

blacklist ${HOME}/foo
whitelist ${HOME}/foo

~/foo is still unaccessible. (Adding whitelist ${HOME}/foo breaks everything in non-whitelisting profiles.)

include disable-baz.inc
whitelist ${HOME}/bar

In order to allow baz in this profile, you need to add noblacklist and whitelist.

My suggestion for simplifying:

blacklist -> deny (or block)
noblacklist/whitelist -> allow
whitelist -> enable-whitelisting (or require-explicit-allow)

Examples:

Old:

noblacklist ${HOME}/.cache/mozilla
noblacklist ${HOME}/.mozilla

mkdir ${HOME}/.cache/mozilla/firefox
mkdir ${HOME}/.mozilla
whitelist ${HOME}/.cache/mozilla/firefox
whitelist ${HOME}/.mozilla

New:

enable-whitelisting

mkdir ${HOME}/.cache/mozilla/firefox
mkdir ${HOME}/.mozilla
allow ${HOME}/.cache/mozilla/firefox
allow ${HOME}/.mozilla

Still uncertain

Maybe enable-whitelisting-in ${HOME}, enable-whitelisting-in /var, ... would be better.

Reasons against it

It's a breaking change.

@topimiettinen
Copy link
Collaborator

Usually in security terminology whitelist lists stuff that is only allowed, the rest is denied. This means that there's an implicit deny all directive, or it could be explicit. Likewise, with blacklist there's implicit or explicit allow all and then the list specifies denied items. In Firejail the directives doesn't match this (like @rusty-snake showed), so if there would be a change (I wouldn't be bothered to change anything just for the reasons OP gave), this could be an opportunity to improve.

@kris7t
Copy link
Collaborator

kris7t commented Jun 7, 2020

@rusty-snake I for one would support the blacklist -> block, noblacklist/whitelist -> allow, whitelist -> require-explicit-allow-in terminology change, so that we can clean up the confusion about noblacklist/whitelist in profiles (enable-whitelisting-in wouldn't make much sense, if there is no whitelist keyword, though).

If I understand correctly, this wouldn't really be a breaking change, since we can just interpret the original commands and emit warnings to help users with the migration. Eventually, we could deprecate blacklist/noblacklist/whitelist and achieve whitelisting profile bliss.

There could also be arguments having "whitelisting" / deny all profiles as a default (and something like implicit-allow-in to disable it), but I guess that would be a breaking change indeed.

@rusty-snake
Copy link
Collaborator

Eventually, we could deprecate blacklist/noblacklist/whitelist and achieve whitelisting profile bliss.

👍

We can the remove nodbus and these in 0.9.66.

There could also be arguments having "whitelisting" / deny all profiles as a default (and something like implicit-allow-in to disable it), but I guess that would be a breaking change indeed.

I would favour ignore require-explicit-allow ${HOME}, using ignore seems to be the firejail-way in the most places.

BTW: such a whitelist on/off switch will be very usefull


Related issues: (mostly affected through require-explicit-allow /...)

#504: whitelist in ~/.local/share and other (mostly dotfiles) subdirs of $HOME
#3189 & #2041: whitelist in /run and others
#3041: This issue would be simplyfied
#2882: nowhitelist -> block or ignore allow ...;; This issue can be closed then, because whitelist on/off has now its own switch.

@Fred-Barclay
Copy link
Collaborator

Fred-Barclay commented Jun 7, 2020

Sounds like some solid technical reasons for a change 😄
I would suggest we don't touch this until 0.9.64 is tagged, make the changes for 0.9.65/0.9.66, and remove in 0.9.68.

EDIT: assuming we made these changes

EDIT 2: should we reopen this issue?

@ClemMakesApps
Copy link
Author

I wouldn't be bothered to change anything just for the reasons OP gave

It's a shame that the original premise wasn't deemed sufficient enough for a change but I'm glad this change is still happening even if it is for another reason.

@rusty-snake
Copy link
Collaborator

EDIT 2: should we reopen this issue?

I would say yes.

@rusty-snake rusty-snake reopened this Jun 8, 2020
@Fred-Barclay
Copy link
Collaborator

@ClemMakesApps Thanks for bringing it to our attention! Even if we disagree on the premises it seems a change was needed 😄

@netblue30
Copy link
Owner

Let's wait for a while to see in what direction the big guys are going, so we don't have to change it again in a few months. Whitelist/blacklist are heavily used in various places:

  • networking industry: email infrastructure and firewall people. If you tell them allow/deny they have no idea what you are talking about
  • mandatory access control (SELinux, AppArmor) - the terms have been there for more than 20 years
  • mass media: a search on news google.com shows CNN, ABC News, vox.com and a lot more.
  • the advertising industry: apparently adblockers are "blacklisting" them!

@topimiettinen
Copy link
Collaborator

With allow/deny I was thinking about Apache config (Order Allow, Deny; Deny from x; Allow from y) but it seems that modern 2.4 syntax uses Require.

Examples from similar, rather old (1990s) software using allow and deny:

  • OpenSSH uses AllowGroup, AllowUser, DenyGroup, DenyUser.
  • TCP Wrappers have two files, /etc/hosts.allow and /etc/hosts.deny.

MAC systems:

  • In AppArmor, the rules list allowed conditions without a keyword. There's an explicit deny keyword.
  • SELinux has allow keyword (everything not allowed is forbidden). There's also neverallow but it's more like assert() function in C.
  • Smack does not use English keywords.
  • With TOMOYO Linux there are no explicit allow/deny keywords, the rules just list all allowed conditions.

Firewalls:

IMHO white and black are used in white/blacklists without much of a preference, both are "good" and the "evil" is not using either. Whitelisting is preferred in many cases, but there are also situations where blacklisting is better.

@glitsj16
Copy link
Collaborator

Now this. It's going to be a hell of a summer this time around...

@rusty-snake
Copy link
Collaborator

@laniakea64
Copy link
Contributor

noblacklist/blacklist/whitelist are the biggest challenge when learning to customize firejail-profiles.

As someone who has written firejail profiles since firejail 0.9.38 and STILL sometimes gets tripped up by this, if I could expand on @rusty-snake suggestion above?

I'm glad something will be done about that. Though I have few technical concerns about the current suggestion as-is. For one, I'm not sure require-explicit-allow-in accurately describes that functionality: require-explicit-allow-in seems to say that the files and directories will be there-but-blocked by default. But in reality they are completely absent and their paths are not banned by default.

And IIUC the suggestion above would take away the possibility provided by current blacklist + whitelist: to deliberately expose a file or directory in the sandbox while denying access to it. I had this as part of my setup when I had multiple profiles for SeaMonkey, with a different firejail profile for each SeaMonkey profile.

So here's a tweak of @rusty-snake suggestion that would

  • alleviate the confusion in writing profiles, as already proposed;
  • not reduce possibilities.

blacklist -> deny
noblacklist + whitelist -> allow
noblacklist alone -> ignore deny
whitelist alone -> expose

And the new directive to enable the functionality currently called "whitelisting": default-mask.

(I would think expose would have no effect where no default-mask directive is applied. Not sure what's best for allow in that case, should it be equivalent to ignore deny?)


Taking the example above:

Old:

noblacklist ${HOME}/.cache/mozilla
noblacklist ${HOME}/.mozilla

mkdir ${HOME}/.cache/mozilla/firefox
mkdir ${HOME}/.mozilla
whitelist ${HOME}/.cache/mozilla/firefox
whitelist ${HOME}/.mozilla

Suggested: there would be two ways to do this -

default-mask ${HOME}

mkdir ${HOME}/.cache/mozilla/firefox
mkdir ${HOME}/.mozilla
allow ${HOME}/.cache/mozilla/firefox
allow ${HOME}/.mozilla

or

default-mask ${HOME}

ignore deny ${HOME}/.cache/mozilla
ignore deny ${HOME}/.mozilla

mkdir ${HOME}/.cache/mozilla/firefox
mkdir ${HOME}/.mozilla
expose ${HOME}/.cache/mozilla/firefox
expose ${HOME}/.mozilla

@Boruch-Baum
Copy link

Add VMware to the list of major players seeing fit to step up.

@rusty-snake
Copy link
Collaborator

I recently thought about @laniakea64 ideas in #3447 (comment). Based on this I extend my #3447 (comment) draft with

  • deny-expose/deny-exposed with makes a file present but inaccessible in whitelisting-profiles.
  • enable-whitelisting/require-explicit-allow -> mask ${HOME} it is much shorter, also talking about masked-profiles is clearer.
  • blacklist-nolog->deny-nolog

About the implementation version. firejail 0.9.64 should not have it, it should be released in the next week (but not before the appimage bug is fixed).

Then we can implement it. I have no problem when we replace it without backward-compatibility. And provide a transition-script/flag. We can then move from a felt 0.9.VERSION[.PATCH] version-shema to firejail 1.0 🎆 .

@topimiettinen
Copy link
Collaborator

+1 for mask. How about unmask/deny-unmask instead of expose/deny-expose then?

@rusty-snake
Copy link
Collaborator

So where are we?

allow ${HOME}/foo: access to $HOME/foo is always allowed
deny ${HOME}/bar: access to $HOME/bar is always denied

If a file has a allow and a deny rule, the first will win (i.e. later rules are ignored). This is necessary for .local-overrides to work.

mask ${HOME}: mount a tmpfs over $HOME and bind-mount allowed files.
TODO: Does mask need restrictions?

deny-exposed ${HOME}/foobar: access to $HOME/foobar is denied, but $HOME is visible if mask is used.
I don't care if we call it deny-expose(d) or deny-unmask. unmask sound more as it is related to mask and not deny.

In addition:

Why do we need mkdir + whitelist all the time?
Proposal: allow implies mkdir if mask is used. A allow-nocreate can be used to opt-out.

Examples:

Old:

noblacklist ${HOME}/.config/mpv
blacklist /tmp/.X11-unix
blacklist ${HOME}/.config/mpv -- include disable-programs.inc

mkdir ${HOME}/.config/mpv
whitelist ${HOME}/.config/mpv
whitelist ${DOWNLOADS}

New:

mask ${HOME}
allow ${HOME}/.config/mpv
allow-nocreate ${DOWNLOADS}
deny /tmp/.X11-unix
deny ${HOME}/.config/mpv -- include disable-programs.inc

@laniakea64
Copy link
Contributor

Why do we need mkdir + whitelist all the time?
Proposal: allow implies mkdir if mask is used. A allow-nocreate can be used to opt-out.

I see this could be helpful for the typical case. But wouldn't it be confusing if someone wanted to allow a file that does not yet exist?

With the current proposal, that would look like -

mkfile ~/.hgrc
allow-nocreate ~/.hgrc

That confusion could be alleviated by, in addition to allow and allow-nocreate, also having an allow-file directive to imply mkfile instead of mkdir on non-existant where mask is used.

@reinerh reinerh added this to the 0.9.64 milestone Sep 1, 2020
@rusty-snake
Copy link
Collaborator

or we use allow/allow-dir and allow-file and allow-nocreate (we need this for e.g. socktes).

Now 0.9.64 is out. Let's go.

@rusty-snake rusty-snake added the enhancement New feature request label Oct 25, 2020
@smitsohu
Copy link
Collaborator

There is now an industry wide effort

https://inclusivenaming.org/

@netblue30
Copy link
Owner

I'll add this one from Linux kernel:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=49decddd39e5f6132ccd7d9fdc3d7c470b0061bb

+For symbol names and documentation, avoid introducing new usage of
+'master / slave' (or 'slave' independent of 'master') and 'blacklist /
+whitelist'.
+
+Recommended replacements for 'master / slave' are:
+    '{primary,main} / {secondary,replica,subordinate}'
+    '{initiator,requester} / {target,responder}'
+    '{controller,host} / {device,worker,proxy}'
+    'leader / follower'
+    'director / performer'
+
+Recommended replacements for 'blacklist/whitelist' are:
+    'denylist / allowlist'
+    'blocklist / passlist'
+
+Exceptions for introducing new usage is to maintain a userspace ABI/API,
+or when updating code for an existing (as of 2020) hardware or protocol
+specification that mandates those terms. For new specifications
+translate specification usage of the terminology to the kernel coding
+standard where possible.

@rusty-snake
Copy link
Collaborator

Before I forget it again to comment. The noblacklist|whitelist -> allow change must work like this.

old:

# thunderbird.profile
noblacklist ${HOME}/.mozilla
whitelist ${HOME}/.mozilla/firefox/profiles.ini
read-only ${HOME}/.mozilla/firefox/profiles.ini
# disable-programs.inc
blacklist ${HOME}/.mozilla

new:

# thunderbird.profile
mask ${HOME}
allow ${HOME}/.mozilla/firefox/profiles.ini
read-only ${HOME}/.mozilla/firefox/profiles.ini
# disable-programs.inc
deny ${HOME}/.mozilla

@netblue30
Copy link
Owner

Moving the discussion here: #4379

kmk3 added a commit to kmk3/firejail that referenced this issue Jul 18, 2021
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
@samuelfmlourenco

This comment was marked as spam.

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

No branches or pull requests