Skip to content

also provide credentials in ExecStartPre#24734

Merged
poettering merged 1 commit intosystemd:mainfrom
Mic92:make-systemd-great
Sep 19, 2022
Merged

also provide credentials in ExecStartPre#24734
poettering merged 1 commit intosystemd:mainfrom
Mic92:make-systemd-great

Conversation

@Mic92
Copy link
Copy Markdown
Contributor

@Mic92 Mic92 commented Sep 18, 2022

Systemd's credential interface is not yet natively supported by all programs yet. Hence it's often required to run scripts to massage secrets in the way the programs expect it.

This commit allows the ExecStartPre commands to access credentials.

Fixes #19604

Systemd's credential interface is not yet natively supported by all
programs yet. Hence it's often required to run scripts to massage
secrets in the way the programs expect it.

This commit allows the ExecStartPre commands to access credentials.

Fixes systemd#19604
@flokli
Copy link
Copy Markdown
Contributor

flokli commented Sep 19, 2022

cc @keszybz (#19604 (comment))

@poettering poettering added pid1 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 Sep 19, 2022
@poettering poettering merged commit e7f64b8 into systemd:main Sep 19, 2022
@Mic92 Mic92 deleted the make-systemd-great branch September 20, 2022 17:59
@ElvishJerricco
Copy link
Copy Markdown
Contributor

Using EXEC_WRITE_CREDENTIALS in both ExecStartPre and ExecStart will technically create the file twice, so they'll have different inode numbers. Not really a big deal but I can imagine some applications making mistakes that make that relevant.

@Mic92
Copy link
Copy Markdown
Contributor Author

Mic92 commented Sep 28, 2022

In some cases ExecStartPre runs with different privileges than ExecStart, so this works around this issue without adding too much new code and complexity.

@flokli
Copy link
Copy Markdown
Contributor

flokli commented Oct 3, 2022

Would it be possible to backport this to systemd-stable, or does this need to wait for a new release?

@wahjava
Copy link
Copy Markdown

wahjava commented Feb 3, 2024

Using EXEC_WRITE_CREDENTIALS in both ExecStartPre and ExecStart will technically create the file twice, so they'll have different inode numbers. Not really a big deal but I can imagine some applications making mistakes that make that relevant.

And it's causing the brokenness in this issue. Following is a minimal reproduction case:

In one terminal:

❯ sudo journalctl --follow | fgrep -i space
Feb 03 20:40:14 chateau (sd-[42723]: Failed to write credential 'credsone': No space left on device

In another terminal:

❯ credone=$(mktemp); credtwo=$(mktemp); dd if=/dev/random of=$credone bs=512 count=1024; dd if=/dev/random of=$credtwo bs=512 count=12; sudo systemctl log-level debug; sudo systemd-run -pLoadCredential=credsone:$credone -pLoadCredential=credstwo:$credtwo -pExecStartPre=$(which uptime) $(which id); sudo systemctl log-level info; rm -vf $credone $credtwo
1024+0 records in
1024+0 records out
524288 bytes (524 kB, 512 KiB) copied, 0.00246026 s, 213 MB/s
12+0 records in
12+0 records out
6144 bytes (6.1 kB, 6.0 KiB) copied, 5.4994e-05 s, 112 MB/s
Running as unit: run-r097ec0cdb6b343f3a4c9413e6d672499.service; invocation ID: 641c066824cf439d85603e1159e7f9c0
removed '/tmp/tmp.KjAiNqAXZ7'
removed '/tmp/tmp.ScBd6zdbc8'

This issue happens because which creates the credentials are created twice (courtesy: ExecStartPre), and one of the credential is unfortunatetly half of the maximum allowed credential size (1M)

YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 4, 2024
Currently, exec_setup_credential always rewrite
all credentials on exec_invoke, i.e. each ExecCommand
invocation. This is problematic though:

* When writing tmp cred files, we essentially double
  the size of each credential. Therefore, if one cred
  is bigger than half of CREDENTIALS_TOTAL_SIZE_MAX,
  confusing error occurs (see also
  systemd#24734 (comment))

* Credential is a unit thing and should not change during
  the whole lifetime of the unit. However, if e.g. a
  on-disk credential or unit file (SetCredential=)
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten and the main process
  is suddenly seeing completely different creds
  when ExecStartPost= gets executed.

So, let's always reuse final cred dir if mounted and
not empty, so that the creds used is stable and probably
more efficient.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 4, 2024
Currently, exec_setup_credential always rewrite
all credentials on exec_invoke, i.e. each ExecCommand
invocation. This is problematic though:

* When writing tmp cred files, we essentially double
  the size of each credential. Therefore, if one cred
  is bigger than half of CREDENTIALS_TOTAL_SIZE_MAX,
  confusing error occurs (see also
  systemd#24734 (comment))

* Credential is a unit thing and should not change during
  the whole lifetime of the unit. However, if e.g. a
  on-disk credential or unit file (SetCredential=)
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten and the main process
  is suddenly seeing completely different creds
  when ExecStartPost= gets executed.

So, let's always reuse final cred dir if mounted and
not empty, so that the creds used is stable and probably
more efficient.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 4, 2024
Currently, exec_setup_credential always rewrite
all credentials on exec_invoke, i.e. each ExecCommand
invocation. This is problematic though:

* When writing tmp cred files, we essentially double
  the size of each credential. Therefore, if one cred
  is bigger than half of CREDENTIALS_TOTAL_SIZE_MAX,
  confusing error occurs (see also
  systemd#24734 (comment))

* Credential is a unit thing and should not change during
  the whole lifetime of the unit. However, if e.g. a
  on-disk credential or unit file (SetCredential=)
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten and the main process
  is suddenly seeing completely different creds
  when ExecStartPost= gets executed.

So, let's always reuse final cred dir if mounted and
not empty, so that the creds used is stable and probably
more efficient.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 4, 2024
Currently, exec_setup_credential always rewrite
all credentials on exec_invoke, i.e. each ExecCommand
invocation. This is problematic though:

* When writing tmp cred files, we essentially double
  the size of each credential. Therefore, if one cred
  is bigger than half of CREDENTIALS_TOTAL_SIZE_MAX,
  confusing error occurs (see also
  systemd#24734 (comment))

* Credential is a unit thing and should not change during
  the whole lifetime of the unit. However, if e.g. a
  on-disk credential or unit file (SetCredential=)
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten and the main process
  is suddenly seeing completely different creds
  when ExecStartPost= gets executed.

So, let's always reuse final cred dir if mounted and
not empty, so that the creds used is stable and probably
more efficient.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 4, 2024
Currently, exec_setup_credential always rewrite
all credentials on exec_invoke, i.e. each ExecCommand
invocation. This is problematic though:

* When writing tmp cred files, we essentially double
  the size of each credential. Therefore, if one cred
  is bigger than half of CREDENTIALS_TOTAL_SIZE_MAX,
  confusing error occurs (see also
  systemd#24734 (comment))

* Credential is a unit thing and should not change during
  the whole lifetime of the unit. However, if e.g. a
  on-disk credential or unit file (SetCredential=)
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten and the main process
  is suddenly seeing completely different creds
  when ExecStartPost= gets executed.

So, let's always reuse final cred dir if mounted and
not empty, so that the creds used is stable and probably
more efficient.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 4, 2024
Currently, exec_setup_credential always rewrite
all credentials on exec_invoke, i.e. each ExecCommand
invocation. This is problematic though:

* When writing tmp cred files, we essentially double
  the size of each credential. Therefore, if one cred
  is bigger than half of CREDENTIALS_TOTAL_SIZE_MAX,
  confusing error occurs (see also
  systemd#24734 (comment))

* Credential is a unit thing and should not change during
  the whole lifetime of the unit. However, if e.g. a
  on-disk credential or unit file (SetCredential=)
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten and the main process
  is suddenly seeing completely different creds
  when ExecStartPost= gets executed.

So, let's always reuse final cred dir if mounted and
not empty, so that the creds used is stable and probably
more efficient.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 6, 2024
Currently, exec_setup_credential always rewrite
all credentials on exec_invoke, i.e. each ExecCommand
invocation. This is problematic though:

* When writing tmp cred files, we essentially double
  the size of each credential. Therefore, if one cred
  is bigger than half of CREDENTIALS_TOTAL_SIZE_MAX,
  confusing error occurs (see also
  systemd#24734 (comment))

* Credential is a unit thing and should not change during
  the whole lifetime of the unit. However, if e.g. a
  on-disk credential or unit file (SetCredential=)
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten and the main process
  is suddenly seeing completely different creds
  when ExecStartPost= gets executed.

So, let's always reuse final cred dir if mounted and
not empty, so that the creds used is stable and probably
more efficient.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 6, 2024
Currently, exec_setup_credential() always rewrite all credentials
upon exec_invoke(), i.e. invocation of each ExecCommand. This is
problematic though:

* When writing each tmp cred file, we essentially double the size
  of the credential. Therefore, if one cred is bigger than half
  of CREDENTIALS_TOTAL_SIZE_MAX, confusing ENOSPC occurs (see also
  systemd#24734 (comment))

* Credential is a unit-wide thing and thus should not change
  during the whole lifetime of the unit. However, if e.g.
  a on-disk credential or SetCredential= in unit file
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten when the latter gets to run,
  and the already-running main process is suddenly seeing
  completely different creds. This one is especially awkward IMO,
  say if people use a timer to regularly update local creds.

So, let's always reuse final cred dir if mounted and not empty,
so that the creds used is stable and each exec_invoke() is probably
more efficient.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 6, 2024
fresh otherwise

Currently, exec_setup_credential() always rewrite all credentials
upon exec_invoke(), i.e. invocation of each ExecCommand, and within
a single tmpfs instance. This is problematic though:

* When writing each tmp cred file, we essentially double the size
  of the credential. Therefore, if one cred is bigger than half
  of CREDENTIALS_TOTAL_SIZE_MAX, confusing ENOSPC occurs (see also
  systemd#24734 (comment))

* Credential is a unit-wide thing and thus should not change
  during the whole lifetime of main process. However, if e.g.
  a on-disk credential or SetCredential= in unit file
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten when the latter gets to run,
  and the already-running main process is suddenly seeing
  completely different creds.

So, let's try to reuse final cred dir if the main process has started
and the tmpfs has been populated, so that the creds used is stable
across all ExecStart= and ExecStartPost=-s. We still want to retain
the ability of updating creds through ExecStartPre= though, therefore
we forcibly use a fresh cred dir for those. 'Fresh' means to actually
unmount the old tmpfs first, so the first problem goes away, too.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 6, 2024
fresh otherwise

Currently, exec_setup_credential() always rewrite all credentials
upon exec_invoke(), i.e. invocation of each ExecCommand, and within
a single tmpfs instance. This is problematic though:

* When writing each tmp cred file, we essentially double the size
  of the credential. Therefore, if one cred is bigger than half
  of CREDENTIALS_TOTAL_SIZE_MAX, confusing ENOSPC occurs (see also
  systemd#24734 (comment))

* Credential is a unit-wide thing and thus should not change
  during the whole lifetime of main process. However, if e.g.
  a on-disk credential or SetCredential= in unit file
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten when the latter gets to run,
  and the already-running main process is suddenly seeing
  completely different creds.

So, let's try to reuse final cred dir if the main process has started
and the tmpfs has been populated, so that the creds used is stable
across all ExecStart= and ExecStartPost=-s. We still want to retain
the ability of updating creds through ExecStartPre= though, therefore
we forcibly use a fresh cred dir for those. 'Fresh' means to actually
unmount the old tmpfs first, so the first problem goes away, too.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 6, 2024
fresh otherwise

Currently, exec_setup_credential() always rewrite all credentials
upon exec_invoke(), i.e. invocation of each ExecCommand, and within
a single tmpfs instance. This is problematic though:

* When writing each tmp cred file, we essentially double the size
  of the credential. Therefore, if one cred is bigger than half
  of CREDENTIALS_TOTAL_SIZE_MAX, confusing ENOSPC occurs (see also
  systemd#24734 (comment))

* Credential is a unit-wide thing and thus should not change
  during the whole lifetime of main process. However, if e.g.
  a on-disk credential or SetCredential= in unit file
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten when the latter gets to run,
  and the already-running main process is suddenly seeing
  completely different creds.

So, let's try to reuse final cred dir if the main process has started
and the tmpfs has been populated, so that the creds used is stable
across all ExecStart= and ExecStartPost=-s. We still want to retain
the ability of updating creds through ExecStartPre= though, therefore
we forcibly use a fresh cred dir for those. 'Fresh' means to actually
unmount the old tmpfs first, so the first problem goes away, too.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 6, 2024
fresh otherwise

Currently, exec_setup_credential() always rewrite all credentials
upon exec_invoke(), i.e. invocation of each ExecCommand, and within
a single tmpfs instance. This is problematic though:

* When writing each tmp cred file, we essentially double the size
  of the credential. Therefore, if one cred is bigger than half
  of CREDENTIALS_TOTAL_SIZE_MAX, confusing ENOSPC occurs (see also
  systemd#24734 (comment))

* Credential is a unit-wide thing and thus should not change
  during the whole lifetime of main process. However, if e.g.
  a on-disk credential or SetCredential= in unit file
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten when the latter gets to run,
  and the already-running main process is suddenly seeing
  completely different creds.

So, let's try to reuse final cred dir if the main process has started
and the tmpfs has been populated, so that the creds used is stable
across all ExecStart= and ExecStartPost=-s. We still want to retain
the ability of updating creds through ExecStartPre= though, therefore
we forcibly use a fresh cred dir for those. 'Fresh' means to actually
unmount the old tmpfs first, so the first problem goes away, too.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 6, 2024
fresh otherwise

Currently, exec_setup_credential() always rewrite all credentials
upon exec_invoke(), i.e. invocation of each ExecCommand, and within
a single tmpfs instance. This is problematic though:

* When writing each tmp cred file, we essentially double the size
  of the credential. Therefore, if one cred is bigger than half
  of CREDENTIALS_TOTAL_SIZE_MAX, confusing ENOSPC occurs (see also
  systemd#24734 (comment))

* Credential is a unit-wide thing and thus should not change
  during the whole lifetime of main process. However, if e.g.
  a on-disk credential or SetCredential= in unit file
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten when the latter gets to run,
  and the already-running main process is suddenly seeing
  completely different creds.

So, let's try to reuse final cred dir if the main process has started
and the tmpfs has been populated, so that the creds used is stable
across all ExecStart= and ExecStartPost=-s. We still want to retain
the ability of updating creds through ExecStartPre= though, therefore
we forcibly use a fresh cred dir for those. 'Fresh' means to actually
unmount the old tmpfs first, so the first problem goes away, too.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 6, 2024
fresh otherwise

Currently, exec_setup_credential() always rewrite all credentials
upon exec_invoke(), i.e. invocation of each ExecCommand, and within
a single tmpfs instance. This is problematic though:

* When writing each tmp cred file, we essentially double the size
  of the credential. Therefore, if one cred is bigger than half
  of CREDENTIALS_TOTAL_SIZE_MAX, confusing ENOSPC occurs (see also
  systemd#24734 (comment))

* Credential is a unit-wide thing and thus should not change
  during the whole lifetime of main process. However, if e.g.
  a on-disk credential or SetCredential= in unit file
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten when the latter gets to run,
  and the already-running main process is suddenly seeing
  completely different creds.

So, let's try to reuse final cred dir if the main process has started
and the tmpfs has been populated, so that the creds used is stable
across all ExecStart= and ExecStartPost=-s. We still want to retain
the ability of updating creds through ExecStartPre= though, therefore
we forcibly use a fresh cred dir for those. 'Fresh' means to actually
unmount the old tmpfs first, so the first problem goes away, too.
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Feb 6, 2024
fresh otherwise

Currently, exec_setup_credential() always rewrite all credentials
upon exec_invoke(), i.e. invocation of each ExecCommand, and within
a single tmpfs instance. This is problematic though:

* When writing each tmp cred file, we essentially double the size
  of the credential. Therefore, if one cred is bigger than half
  of CREDENTIALS_TOTAL_SIZE_MAX, confusing ENOSPC occurs (see also
  systemd#24734 (comment))

* Credential is a unit-wide thing and thus should not change
  during the whole lifetime of main process. However, if e.g.
  a on-disk credential or SetCredential= in unit file
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten when the latter gets to run,
  and the already-running main process is suddenly seeing
  completely different creds.

So, let's try to reuse final cred dir if the main process has started
and the tmpfs has been populated, so that the creds used is stable
across all ExecStart= and ExecStartPost=-s. We still want to retain
the ability of updating creds through ExecStartPre= though, therefore
we forcibly use a fresh cred dir for those. 'Fresh' means to actually
unmount the old tmpfs first, so the first problem goes away, too.
ayhamthemayhem pushed a commit to neighbourhoodie/nh-systemd that referenced this pull request Mar 25, 2024
fresh otherwise

Currently, exec_setup_credential() always rewrite all credentials
upon exec_invoke(), i.e. invocation of each ExecCommand, and within
a single tmpfs instance. This is problematic though:

* When writing each tmp cred file, we essentially double the size
  of the credential. Therefore, if one cred is bigger than half
  of CREDENTIALS_TOTAL_SIZE_MAX, confusing ENOSPC occurs (see also
  systemd#24734 (comment))

* Credential is a unit-wide thing and thus should not change
  during the whole lifetime of main process. However, if e.g.
  a on-disk credential or SetCredential= in unit file
  changes between ExecStart= and ExecStartPost=,
  the credentials are overwritten when the latter gets to run,
  and the already-running main process is suddenly seeing
  completely different creds.

So, let's try to reuse final cred dir if the main process has started
and the tmpfs has been populated, so that the creds used is stable
across all ExecStart= and ExecStartPost=-s. We still want to retain
the ability of updating creds through ExecStartPre= though, therefore
we forcibly use a fresh cred dir for those. 'Fresh' means to actually
unmount the old tmpfs first, so the first problem goes away, too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

RFE: credentials loaded with LoadCredential should be available to ExecStartPre too

5 participants