nixos/github-runner: refactor tokens handling #148164
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit changes how we deal with the current token, i.e., the token which may exist from a previous runner registration, and the configured token, i.e., the path set for the respective NixOS configuration option. Until now, we copied the configured and the current token (if any) to the runtime directory to compare them. The path of the current token may reference a file which is only accessible to specific users (even only root). Therefore, we ran the copying of credentials with elevated privileges by prefixing the `ExecStartPre=` script with a `+` (see systemd.service(5)). In this script, we also changed the owner of the files to the service user. Apparently, however, the user/group pair sometimes did not exist because we use `DynamicUser=`. To address this issue, we no longer change the owner of the file. Instead, we change the file permissions to 0666 to allow the runner configuration script (runs with full sandboxing) to read-write the file. Due to the current permissions of the runtime directory (0755), this would expose the token. Therefore, we process the tokens in the state directory, which is only accessible to the service user. If a new token file exists in the state directory, the configuration script should trigger a new runner registration. Afterward, it deletes the new token file. The token is still available using the path of the current token which is inaccessible within the service's sandbox.
github-actions
bot
added
6.topic: nixos
ofborg
bot
added
10.rebuild-darwin: 0
yayayayaka
approved these changes
Dec 11, 2021
1 task
|
Successfully created backport PR #153511 for |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
This commit changes how we deal with the current token, i.e., the token
which may exist from a previous runner registration, and the configured
token, i.e., the path set for the respective NixOS configuration option.
Until now, we copied the configured and the current token (if any) to
the runtime directory to compare them. The path of the current token may
reference a file which is only accessible to specific users (even only
root). Therefore, we ran the copying of credentials with elevated
privileges by prefixing the
ExecStartPre=script with a+(seesystemd.service(5)). In this script, we also changed the owner of the
files to the service user. Apparently, however, the user/group pair
sometimes did not exist because we use
DynamicUser=.To address this issue, we no longer change the owner of the file.
Instead, we change the file permissions to 0666 to allow the runner
configuration script (runs with full sandboxing) to read-write the file.
Due to the current permissions of the runtime directory (0755), this
would expose the token. Therefore, we process the tokens in the state
directory, which is only accessible to the service user.
If a new token file exists in the state directory, the configuration
script should trigger a new runner registration. Afterward, it deletes
the new token file. The token is still available using the path of the
current token which is inaccessible within the service's sandbox.
Fixes #148024
Supersedes #148047
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes