-
Notifications
You must be signed in to change notification settings - Fork 227
src/nbd: set groups of child process #1695
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
src/nbd: set groups of child process #1695
Conversation
In contrast to the uid and gid values of the child-process, the groups require allocation of dynamic memory (to avoid allocating the maximum value, which is unreasonably high). This means that the groups values of child_setup_args need to be freed. Currently, this is not done very elegantly. |
You could define a G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC function for |
3b1313b
to
e9618b0
Compare
Thanks, I didn't know about this option, I've implemented it accordingly now. |
Originally, the struct of child_setup_args did not have any typedef, and was always used as "struct child_setup_args". In the future, this struct will be surrounded by gmacro macros, to handle memory management of the fields in this struct. The gmacro macros are using the type name of the given object when creating their own objects through these macros. To make it possible for an object to have these macros, the type of the object must be constructed of a single word, otherwise, the gmacros will break by the space in the type name. The current commit adds the typedef to the struct definition, and updates all the usage in the code from "struct child_setup_args" to be just "child_setup_args". Signed-off-by: Omri Sarig <omri.sarig@prevas.dk>
When rauc is streaming a bundle during an installation operation, it creates a sub-process, and switches the user of this subprocess, to create e sandbox environment where this user have access only to the required resources in the system. Until now, when this subprocess received the UID and GID of the streaming user, however, the groups of this user were not set accordingly. This can cause problems when resources needed for the installation (e.g., installation keys) are available only through group permissions of the sandbox user. This commit adds relevant logic to the child preparation and setup functions, where the code finds out which groups the user is a part of and set them accordingly. Setting the groups of the process must happen before changing the UID of the process, otherwise, the group setting operation will fail with permissions errors. Signed-off-by: Omri Sarig <omri.sarig@prevas.dk>
e9618b0
to
109dfa6
Compare
Force-pushed to align the commit titles with our convention (no capitalization). |
In the commit message, you write:
With installation keys, you mean private keys for TLS client authentication, right? The subprocess shouldn't really need to access much else... |
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (63.63%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff
8000
@@
## master #1695 +/- ##
==========================================
- Coverage 84.47% 84.45% -0.02%
==========================================
Files 76 76
Lines 22199 22219 +20
==========================================
+ Hits 18752 18766 +14
- Misses 3447 3453 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Yes. In my specific case, I have a private key (for TLS client authentication) that is stored in a TPM, so the user needs to be part of the 'tss' group to be able to access the relevant devices, so it's a bit more than just a file, but in essence, it is only for the TLS client private-key access. |
Thanks for your contribution! |
When rauc is streaming a bundle during an installation operation, it creates a sub-process, and switches the user of this subprocess, to create e sandbox environment where this user have access only to the required resources in the system.
Until now, when this subprocess received the UID and GID of the streaming user, however, the groups of this user were not set accordingly.
This can cause problems when resources needed for the installation (e.g., installation keys) are available only through group permissions of the sandbox user.
This commit adds relevant logic to the child preparation and setup functions, where the code finds out which groups the user is a part of and sets them accordingly.
Setting the groups of the process must happen before changing the UID of the process, otherwise, the group setting operation will fail with permissions errors.