Skip to content

smack: Add DefaultSmackProcessLabel to user.conf and system.conf #23921

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 12, 2022
Merged

smack: Add DefaultSmackProcessLabel to user.conf and system.conf #23921

merged 1 commit into from
Jul 12, 2022

Conversation

steelman
Copy link
Contributor

@steelman steelman commented Jul 6, 2022

DefaultSmackProcessLabel tells systemd what label to assign to its child
process in case SmackProcessLabel is not set in the service file. By
default, when DefaultSmackProcessLabel is not set child processes inherit
label from systemd.

@steelman
Copy link
Contributor Author

steelman commented Jul 6, 2022

Mailing list discussion

poettering
poettering reviewed Jul 6, 2022
View reviewed changes
poettering
poettering reviewed Jul 6, 2022
View reviewed changes
poettering
poettering reviewed Jul 6, 2022
View reviewed changes
poettering
poettering reviewed Jul 6, 2022
View reviewed changes
@poettering poettering added pid1 reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks smack labels Jul 6, 2022
@steelman steelman force-pushed the default-smack-process-label branch from b9ba61e to 03eadf5 Compare July 6, 2022 16:36
poettering
poettering reviewed Jul 8, 2022
View reviewed changes
poettering
poettering reviewed Jul 8, 2022
View reviewed changes
man/systemd-system.conf.xml Outdated
for the details.</para>

<para>If the value is <literal>/</literal> only labels labels specified with <varname>SmackProcessLabel=</para>
are assigned and the compile-time default is ignored.</listitem>
Copy link
Member

Choose a reason for hiding this comment

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

is this common in SMACK land, to use / as marker for "unset"? or did you invent this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is my invention. According to SMACK documentation

[…] Single character labels using special characters, that being anything other than a letter or digit, are reserved for use by the Smack development team.

but

[…] Smack labels cannot contain unprintable characters, the "/" (slash), the "" (backslash), the "'" (quote) and '"' (double-quote) characters. Smack labels cannot begin with a '-'. This is reserved for special options. […]

Which means we can be sure / won't be used anywhere in SMACK.

Every task on a Smack system is assigned a label. The Smack label
of a process will usually be assigned by the system initialization
mechanism.

Therefore, there is no notion of unset in SMACK. This is going to by purely systemd logic, to use kernel default instead of the compiled in systemd default.

Copy link
Member

Choose a reason for hiding this comment

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

but "-" sounds like a safer option then, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The documentation says.

Smack labels cannot begin with a '-'. This is reserved for special options.

I'd rather not use something that is marked as reserved.

poettering
poettering reviewed Jul 8, 2022
View reviewed changes
@steelman steelman force-pushed the default-smack-process-label branch 2 times, most recently from b34b354 to 59c6910 Compare July 8, 2022 13:30
@bluca bluca added please-review and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jul 8, 2022
poettering
poettering reviewed Jul 8, 2022
View reviewed changes
poettering
poettering reviewed Jul 8, 2022
View reviewed changes
poettering
poettering reviewed Jul 8, 2022
View reviewed changes
@steelman steelman force-pushed the default-smack-process-label branch from 59c6910 to e6c8911 Compare July 8, 2022 14:51
poettering
poettering reviewed Jul 11, 2022
View reviewed changes
@steelman steelman force-pushed the default-smack-process-label branch from e6c8911 to 83b6cfd Compare July 11, 2022 09:40
@poettering poettering added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review labels Jul 11, 2022
yuwata
yuwata requested changes Jul 11, 2022
View reviewed changes
man/systemd-system.conf.xml Outdated
unit. See <citerefentry><refentrytitle>systemd.exec</refentrytitle><manvolnum>5</manvolnum></citerefentry>
for the details.</para>

<para>If the value is <literal>/</literal> only labels labels specified with <varname>SmackProcessLabel=</varname>
Copy link
Member

Choose a reason for hiding this comment

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

"labels labels"

Also, I'd like to add a comma after <literal>/</literal>.

BTW, as already discussed at #23921 (comment), still I'd lie to use - for disabling the compile-time defaults. As you already commented, normal labels cannot start with '-'. That means we can safely use '-'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

'-' is actually used in a special way (see -CIPSO and -DELETE) and I'd like to stay away from it, even if only to avoid confusion (LSM/MAC stuff is complicated enough) between the ways the kernel and systemd use '-'.

Copy link
Member

Choose a reason for hiding this comment

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

hmm, can we use ~?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am afraid we can't

Single character labels using special characters, that being anything other than a letter or digit, are reserved for use by the Smack development team.

@yuwata yuwata added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Jul 11, 2022
smack: Add DefaultSmackProcessLabel to user.conf and system.conf
ca1f6e0 
DefaultSmackProcessLabel tells systemd what label to assign to its child
process in case SmackProcessLabel is not set in the service file. By
default, when DefaultSmackProcessLabel is not set child processes inherit
label from systemd.

If DefaultSmackProcessLabel is set to "/" (which is an invalid character
for a SMACK label) the DEFAULT_SMACK_PROCESS_LABEL set during compilation
is ignored and systemd act as if the option was unset.
@steelman steelman force-pushed the default-smack-process-label branch from 83b6cfd to ca1f6e0 Compare July 12, 2022 09:43
@yuwata yuwata removed the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Jul 12, 2022
@yuwata yuwata added the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Jul 12, 2022
@bluca bluca merged commit aa5ae97 into systemd:main Jul 12, 2022
@steelman steelman deleted the default-smack-process-label branch July 13, 2022 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poettering poettering poettering left review comments

@yuwata yuwata yuwata requested changes

@bluca bluca bluca approved these changes

Assignees
No one assigned
Labels
good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed pid1 smack
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@steelman @bluca @poettering @yuwata