Skip to content
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

roles: hosted_engine_setup: Adjust files permissions #409

Merged
merged 1 commit into from Dec 27, 2021

Conversation

arachmani
Copy link
Member

@arachmani arachmani commented Dec 15, 2021

Adjusting files permissions for supporting umask 077.

For security purposes we decided to use 770/660 permissions for the localvm directory, this directory contains the appliance image, and for creating the engine VM we must have its user/group configured for qemu, otherwise qemu user won't have search permissions for the appliance image (see also https://libvirt.org/drvqemu.html#posix-users-groups).
Therefore we have decided to use qemu group instead of kvm.

Bug-Url: https://bugzilla.redhat.com/2020620
Signed-off-by: Asaf Rachmani arachman@redhat.com

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

Looks good, although I wonder if we shouldn't try to use 770 or 660 masks

@mwperina
Copy link
Member

@didib

@didib
Copy link
Member

didib commented Dec 16, 2021

I personally think that 0770/0660 are mandatory, and if they do not work, we should find out why and fix that. The image includes sensitive stuff (hashed root password) and might include other sensitive stuff from user customization.

@arachmani
Copy link
Member Author

Verified and opened https://bugzilla.redhat.com/show_bug.cgi?id=2035631

Adjusting files permissions for supporting umask 077

Bug-Url: https://bugzilla.redhat.com/2020620
Signed-off-by: Asaf Rachmani <arachman@redhat.com>
Copy link
Member

@didib didib left a comment

Choose a reason for hiding this comment

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

Code looks fine to me. Perhaps worth documenting our considerations in the commit message or in comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants