feature: add notpm command & keep tpm devices in private-dev #6390

qdii · 2024-06-22T11:42:22Z

An ssh private key may be stored in a Trusted Platform Module (TPM)
device and private-dev in ssh.profile currently breaks this use-case,
as it does not keep tpm devices (see #6379).

So add a new notpm command and keep tpm devices in /dev by default
with private-dev unless notpm is used.

Tested locally with:

❯ /usr/local/bin/firejail --private-dev --noprofile stat /dev/tpm0
firejail version 0.9.73

Parent pid 34059, child pid 34060
Base filesystem installed in 0.03 ms
Child process initialized in 13.23 ms
  File: /dev/tpm0
  Size: 0         	Blocks: 0          IO Block: 4096   character special file
Device: 0,6	Inode: 150         Links: 1     Device type: 10,224
Access: (0600/crw-------)  Uid: (  973/     tss)   Gid: (  973/     tss)
Access: 2024-06-22 11:20:44.017230197 +0200
Modify: 2024-06-22 11:20:44.017230197 +0200
Change: 2024-06-22 11:20:44.017230197 +0200
 Birth: 2024-06-22 11:20:10.406666659 +0200

Parent is shutting down, bye...

Fixes #6379.

rusty-snake

LGTM

There has to be an follow-up PR that adds notpm to all profiles with private-dev.

qdii · 2024-06-22T13:12:30Z

@rusty-snake Added a commit that adds notpm statement to all the profiles which contain private-dev.

glitsj16

LGTM

qdii · 2024-06-23T06:22:35Z

After this is committed, we should also update the Wiki: https://github.com/netblue30/firejail/wiki/Comparison-of-firejail-and-systemd's-hardening-options

etc/templates/profile.template

src/man/firejail.1.in

glitsj16

LGTM. Some ordering nitpicks...

qdii · 2024-06-23T06:43:29Z

Amended and force-pushed with alphabetical ordering.

kmk3

Sorting.

src/fbuilder/build_profile.c

src/firejail/firejail.h

src/firejail/fs_dev.c

src/firejail/main.c

src/firejail/sandbox.c

src/firejail/usage.c

kmk3 · 2024-06-23T14:07:30Z

There are some other missing changes:

Add notpm to profile.c
Add notpm to firejail-profile.5.in
Add #notpm to default.profile (keep it commented like #nou2f for
consistency)
Run make syntax and commit the resulting changes

See commit 760f50f ("landlock: move commands into profile and add
landlock.enforce", 2023-11-17) / PR #6125 for a recent example of a new
command.

qdii · 2024-06-24T21:10:33Z

It looks like force-pushing is making other updates obsolete.
I'm not used to working on github just yet, sorry.

kmk3

This PR already makes substantial changes by adding the new command; leave the
profile changes (other than the ones for default.profile and profile.template)
for after this PR to make reviewing easier.

In the parts of the code that are mostly alphabetically sorted (such as in the
man pages), it makes sense to put notpm before notv.

But in some parts of the code, for whatever reason the multimedia-related
options (nosound, noautopulse, no3d, notv, nodvd) are sorted
together, so for consistency put notpḿ before nou2f, as was suggested in
the previous review.

That is:

notv, nou2f -> notpm, notv, nou2f
nodvd, nou2f -> nodvd, notpm, nou2f

Also, avoid sorting things in this PR (especially in the same commit).

After this PR I might try to make these parts more consistent.

kmk3

Misc

src/firejail/fs_dev.c

src/man/firejail-profile.5.in

src/man/firejail.1.in

qdii · 2024-07-03T19:31:12Z

This PR already makes substantial changes by adding the new command; leave the profile changes (other than the ones for default.profile and profile.template) for after this PR to make reviewing easier.

OK, reverted the long list of profile changes. I stored it in another branch to propose later.

In the parts of the code that are mostly alphabetically sorted (such as in the man pages), it makes sense to put notpm before notv.

But in some parts of the code, for whatever reason the multimedia-related options (nosound, noautopulse, no3d, notv, nodvd) are sorted together, so for consistency put notpḿ before nou2f, as was suggested in the previous review.

That is:

notv, nou2f -> notpm, notv, nou2f

nodvd, nou2f -> nodvd, notpm, nou2f

OK, I see.

Also, avoid sorting things in this PR (especially in the same commit).

This sentence confuses me, because I interpreted the two previous paragraphs as a ask to actually sort these options in a certain way.

Or do you want to merge this PR, and do the sorting in a different one? I'm happy either way.

After this PR I might try to make these parts more consistent.

kmk3 · 2024-07-04T07:30:15Z

Also, avoid sorting things in this PR (especially in the same commit).

This sentence confuses me, because I interpreted the two previous paragraphs
as a ask to actually sort these options in a certain way.

Sorry, I meant avoid sorting/moving existing lines while adding new lines at
the same time, such as in these cases:

In general it seems better to avoid refactoring in PRs that just add a new
thing, as they will mostly contain hunks that add new lines, which are easier
to review. Hunks that sort lines might be wrong/inconsistent with how the
sorting is done elsewhere (this applies to the first link above).

Or do you want to merge this PR, and do the sorting in a different one? I'm
happy either way.

I fixed the sorting, squashed the commits, edited the commit message and
force-pushed.

Let me know if there are any issues.

An ssh private key may be stored in a Trusted Platform Module (TPM) device and `private-dev` in ssh.profile currently breaks this use-case, as it does not keep tpm devices (see netblue30#6379). So add a new `notpm` command and keep tpm devices in /dev by default with `private-dev` unless `notpm` is used.

Relates to #6372 #6379 #6380 #6382 #6387 #6390 #6398.

This command is deprecated and may be confused for a hardening option. This amends commit 5a61202 ("rename noautopulse to keep-config-pulse", 2021-05-13) / PR netblue30#4278. This is a follow-up to netblue30#6390.

This command is deprecated and may be confused for a hardening option. This amends commit 5a61202 ("rename noautopulse to keep-config-pulse", 2021-05-13) / PR #4278. This is a follow-up to #6390.

Remove the newer #6390 item as it is already on the list, remove the older #6307 item (modif) and sort the new #6307 item (bugfix). This amends commit 9ebecd0 ("readme/relnotes update", 2024-07-13).

The `notpm` command will be deprecated. Relates to netblue30#6390.

Instead of having a `notpm` command and potentially adding it to almost all profiles (as few programs should need direct access to TPM devices), add a `keep-dev-tpm` command and use it only in profiles that need access to TPM devices. Changes: * Turn `notpm` command into `keep-dev-tpm` command * Warn and ignore if `notpm` is used * Block `/dev/tpm*` devices by default * Allow `/dev/tpm*` devices with `keep-dev-tpm` (even if `private-dev` is used) Added on commit 0013202 ("feature: add notpm command & keep tpm devices in private-dev (netblue30#6390)", 2024-07-09). See also commit ee1c264 ("feature: block /dev/ntsync & add keep-dev-ntsync command (netblue30#6660)", 2025-03-06) and the discussion at PR netblue30#6660. This is a follow-up to netblue30#6687.

The `notpm` command will be deprecated. Relates to netblue30#6390.

Instead of having a `notpm` command and potentially adding it to almost all profiles (as few programs should need direct access to TPM devices), add a `keep-dev-tpm` command and use it only in profiles that need access to TPM devices. Changes: * Turn `notpm` command into `keep-dev-tpm` command * Warn and ignore if `notpm` is used * Block `/dev/tpm*` devices by default * Allow `/dev/tpm*` devices with `keep-dev-tpm` (even if `private-dev` is used) Added on commit 0013202 ("feature: add notpm command & keep tpm devices in private-dev (netblue30#6390)", 2024-07-09). See also commit ee1c264 ("feature: block /dev/ntsync & add keep-dev-ntsync command (netblue30#6660)", 2025-03-06) and the discussion at PR netblue30#6660. This is a follow-up to netblue30#6687.

The `notpm` command will be deprecated. Relates to netblue30#6390.

Instead of having a `notpm` command and potentially adding it to almost all profiles (as few programs should need direct access to TPM devices), add a `keep-dev-tpm` command and use it only in profiles that need access to TPM devices. Changes: * Turn `notpm` command into `keep-dev-tpm` command * Warn and ignore if `notpm` is used * Block `/dev/tpm*` devices by default * Allow `/dev/tpm*` devices with `keep-dev-tpm` (even if `private-dev` is used) Added on commit 0013202 ("feature: add notpm command & keep tpm devices in private-dev (netblue30#6390)", 2024-07-09). See also commit ee1c264 ("feature: block /dev/ntsync & add keep-dev-ntsync command (netblue30#6660)", 2025-03-06) and the discussion at PR netblue30#6660. This is a follow-up to netblue30#6687.

This group is apparently used by tpm2-tss for accessing TPM devices. udev rules from tpm2-tss 4.1.3[1]: # tpm devices can only be accessed by the tss user but the tss # group members can access tpmrm devices KERNEL=="tpm[0-9]*", TAG+="systemd", MODE="0660", OWNER="tss" KERNEL=="tpmrm[0-9]*", TAG+="systemd", MODE="0660", GROUP="tss" Misc: This was noticed on netblue30#6700. Relates to netblue30#6390. [1] https://github.com/tpm2-software/tpm2-tss/blob/4.1.3/dist/tpm-udev.rules

This group is apparently used by tpm2-tss for accessing TPM devices. udev rules from tpm2-tss 4.1.3[1]: # tpm devices can only be accessed by the tss user but the tss # group members can access tpmrm devices KERNEL=="tpm[0-9]*", TAG+="systemd", MODE="0660", OWNER="tss" KERNEL=="tpmrm[0-9]*", TAG+="systemd", MODE="0660", GROUP="tss" Misc: This was noticed on #6700. Relates to #6390. [1] https://github.com/tpm2-software/tpm2-tss/blob/4.1.3/dist/tpm-udev.rules

Treat them like `/dev/tpm[0-9]*` devices. It seems that `/dev/tpm[0-9]*` allows direct access to the TPM device while `/dev/tpmrm[0-9]*` mediates access through a "resource manager" inside of the kernel (for example, to facilitate concurrent access). Alternatively, it looks like the resource management can be done in userspace through tpm2-abrmd, the "TPM2 Access Broker & Resource Management Daemon", which also supports older kernels (Linux 3.x vs 4.12) [1] [2]. udev rules from tpm2-tss 4.1.3[3]: # tpm devices can only be accessed by the tss user but the tss # group members can access tpmrm devices KERNEL=="tpm[0-9]*", TAG+="systemd", MODE="0660", OWNER="tss" KERNEL=="tpmrm[0-9]*", TAG+="systemd", MODE="0660", GROUP="tss" This is a follow-up to netblue30#6718. Misc: This was noticed on netblue30#6700. Relates to netblue30#6390. [1] https://github.com/tpm2-software/tpm2-abrmd [2] tpm2-software/tpm2-tss-engine#149 [3] https://github.com/tpm2-software/tpm2-tss/blob/4.1.3/dist/tpm-udev.rules

Treat them like `/dev/tpm[0-9]*` devices. It seems that `/dev/tpm[0-9]*` allows direct access to the TPM device while `/dev/tpmrm[0-9]*` mediates access through a "resource manager" inside of the kernel (for example, to facilitate concurrent access). Alternatively, it looks like the resource management can be done in userspace through tpm2-abrmd, the "TPM2 Access Broker & Resource Management Daemon", which also supports older kernels (Linux 3.x vs 4.12) [1] [2] [3]. udev rules from tpm2-tss 4.1.3[4]: # tpm devices can only be accessed by the tss user but the tss # group members can access tpmrm devices KERNEL=="tpm[0-9]*", TAG+="systemd", MODE="0660", OWNER="tss" KERNEL=="tpmrm[0-9]*", TAG+="systemd", MODE="0660", GROUP="tss" This is a follow-up to netblue30#6718. Misc: This was noticed on netblue30#6700. Relates to netblue30#6390. [1] https://github.com/tpm2-software/tpm2-abrmd [2] tpm2-software/tpm2-abrmd#830 [3] tpm2-software/tpm2-tss-engine#149 [4] https://github.com/tpm2-software/tpm2-tss/blob/4.1.3/dist/tpm-udev.rules

Treat them like `/dev/tpm[0-9]*` devices. It seems that `/dev/tpm[0-9]*` allows direct access to the TPM device while `/dev/tpmrm[0-9]*` mediates access through a "resource manager" inside of the kernel (for example, to facilitate concurrent access). Alternatively, it looks like the resource management can be done in userspace through tpm2-abrmd, the "TPM2 Access Broker & Resource Management Daemon", which also supports older kernels (Linux 3.x vs 4.12) [1] [2] [3]. udev rules from tpm2-tss 4.1.3[4]: # tpm devices can only be accessed by the tss user but the tss # group members can access tpmrm devices KERNEL=="tpm[0-9]*", TAG+="systemd", MODE="0660", OWNER="tss" KERNEL=="tpmrm[0-9]*", TAG+="systemd", MODE="0660", GROUP="tss" This is a follow-up to #6718. Misc: This was noticed on #6700. Relates to #6390. [1] https://github.com/tpm2-software/tpm2-abrmd [2] tpm2-software/tpm2-abrmd#830 [3] tpm2-software/tpm2-tss-engine#149 [4] https://github.com/tpm2-software/tpm2-tss/blob/4.1.3/dist/tpm-udev.rules

Paths: * `/dev/tcm[0-9]*` * `/dev/tcmrm[0-9]*` Apparently Trusted Cryptography Module (TCM) is a standard from China that is an alternative to the TCG Trusted Compute Module (TPM) standard[1]. udev rules from tpm2-tss master[2]: # tpm devices can only be accessed by the tss user but the tss # group members can access tpmrm devices KERNEL=="tpm[0-9]*", TAG+="systemd", MODE="0660", OWNER="tss" KERNEL=="tpmrm[0-9]*", TAG+="systemd", MODE="0660", GROUP="tss" KERNEL=="tcm[0-9]*", TAG+="systemd", MODE="0660", OWNER="tss" KERNEL=="tcmrm[0-9]*", TAG+="systemd", MODE="0660", GROUP="tss" This is a follow-up to netblue30#6719. Relates to netblue30#6390. Misc: This was noticed on netblue30#6700. [1] tpm2-software/tpm2-tss#2905 [2] https://github.com/tpm2-software/tpm2-tss/blob/b2ab12f860598afb759ecef81bf662a70811557c/dist/tpm-udev.rules

rusty-snake reviewed Jun 22, 2024

View reviewed changes

glitsj16 approved these changes Jun 22, 2024

View reviewed changes

qdii force-pushed the master branch from 596cea3 to fe6bcde Compare June 23, 2024 06:12

glitsj16 reviewed Jun 23, 2024

View reviewed changes

etc/templates/profile.template Show resolved Hide resolved

glitsj16 reviewed Jun 23, 2024

View reviewed changes

src/man/firejail.1.in Show resolved Hide resolved

glitsj16 reviewed Jun 23, 2024

View reviewed changes

src/man/firejail.1.in Outdated Show resolved Hide resolved

glitsj16 approved these changes Jun 23, 2024

View reviewed changes

qdii force-pushed the master branch from fe6bcde to e643453 Compare June 23, 2024 06:43

kmk3 requested changes Jun 23, 2024

View reviewed changes

kmk3 changed the title ~~Add TPM devices to private-dev~~ feature: add notpm command & keep tpm devices in private-dev Jun 23, 2024

qdii force-pushed the master branch 3 times, most recently from 0a57213 to 6d50555 Compare June 24, 2024 21:09

kmk3 requested changes Jun 29, 2024

View reviewed changes

src/firejail/fs_dev.c Outdated Show resolved Hide resolved

src/man/firejail-profile.5.in Outdated Show resolved Hide resolved

src/man/firejail-profile.5.in Outdated Show resolved Hide resolved

src/man/firejail.1.in Outdated Show resolved Hide resolved

kmk3 force-pushed the master branch 2 times, most recently from 4da75ef to 1d80642 Compare July 4, 2024 07:10

kmk3 approved these changes Jul 4, 2024

View reviewed changes

This was referenced Jul 4, 2024

ssh: cannot access private key stored in TPM (private-dev) #6379

Closed

docs: man: format and sort some private- items #6398

Merged

kmk3 force-pushed the master branch 2 times, most recently from 6b97aad to 8bfc334 Compare July 7, 2024 20:07

kmk3 merged commit 0013202 into netblue30:master Jul 9, 2024
14 checks passed

kmk3 added a commit that referenced this pull request Jul 9, 2024

RELNOTES: add feature, modif and docs items

aed94a3

Relates to #6372 #6379 #6380 #6382 #6387 #6390 #6398.

kmk3 added the enhancement New feature request label Jul 9, 2024

kmk3 mentioned this pull request Jul 9, 2024

bugfix: remove --noautopulse from --help and zsh comp #6401

Merged

kmk3 added a commit to kmk3/firejail that referenced this pull request Apr 1, 2025

profiles: remove notpm entries

216af58

The `notpm` command will be deprecated. Relates to netblue30#6390.

kmk3 mentioned this pull request Apr 1, 2025

modif: block TPM devices & turn notpm command into keep-dev-tpm #6698

Merged

kmk3 added a commit to kmk3/firejail that referenced this pull request Apr 5, 2025

profiles: remove notpm entries

8348050

The `notpm` command will be deprecated. Relates to netblue30#6390.

kmk3 added a commit to kmk3/firejail that referenced this pull request Apr 5, 2025

profiles: remove notpm entries

844f307

The `notpm` command will be deprecated. Relates to netblue30#6390.

kmk3 mentioned this pull request Apr 25, 2025

modif: keep tss group if keep-dev-tpm is used #6718

Merged

kmk3 mentioned this pull request Apr 26, 2025

modif: keep /dev/tpmrm devices if keep-dev-tpm is used #6719

Merged

kmk3 mentioned this pull request Apr 29, 2025

modif: keep tcm/tcmrm devices if keep-dev-tpm is used #6724

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: add notpm command & keep tpm devices in private-dev #6390

feature: add notpm command & keep tpm devices in private-dev #6390

qdii commented Jun 22, 2024 •

edited by kmk3

Loading

rusty-snake left a comment

qdii commented Jun 22, 2024

glitsj16 left a comment

qdii commented Jun 23, 2024

glitsj16 left a comment

qdii commented Jun 23, 2024

kmk3 left a comment

kmk3 commented Jun 23, 2024 •

edited

Loading

qdii commented Jun 24, 2024

kmk3 left a comment

kmk3 left a comment

qdii commented Jul 3, 2024

kmk3 commented Jul 4, 2024

feature: add notpm command & keep tpm devices in private-dev #6390

feature: add notpm command & keep tpm devices in private-dev #6390

Conversation

qdii commented Jun 22, 2024 • edited by kmk3 Loading

rusty-snake left a comment

Choose a reason for hiding this comment

qdii commented Jun 22, 2024

glitsj16 left a comment

Choose a reason for hiding this comment

qdii commented Jun 23, 2024

glitsj16 left a comment

Choose a reason for hiding this comment

qdii commented Jun 23, 2024

kmk3 left a comment

Choose a reason for hiding this comment

kmk3 commented Jun 23, 2024 • edited Loading

qdii commented Jun 24, 2024

kmk3 left a comment

Choose a reason for hiding this comment

kmk3 left a comment

Choose a reason for hiding this comment

qdii commented Jul 3, 2024

kmk3 commented Jul 4, 2024

qdii commented Jun 22, 2024 •

edited by kmk3

Loading

kmk3 commented Jun 23, 2024 •

edited

Loading