On Mon, Jun 20, 2016 at 08:10:10 -0400, John Ferlan wrote:
On 06/17/2016 09:44 AM, Peter Krempa wrote:
Since introduction of the DAC security driver we've documented that seclabels with a leading + can be used with numerical uid. This would not work though with the rest of libvirt if the uid was not actually used in the system as we'd fail when trying to get a list of supplementary groups for the given uid. Since a uid without entry in /etc/passwd (or other user database) will not have any supplementary groups we can treat the failure to obtain them as such.
This patch modifies virGetGroupList to not report the error for missing users and makes it return an empty list or just the group specified in @gid.
All callers will grand less permissions in case of failure and thus this change is safe.
"grand less"? did you mean "gain less"?
I've explained it a bit more: All callers will grant less permissions to a user in case of failure of this function and thus this change is safe.
ACK series (just typed out loud below)
John
--- src/util/virutil.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/src/util/virutil.c b/src/util/virutil.c index 392091b..60da17b 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c
[...]
+ } }
Playing the "what if" game here. If for some reason uid == -1 OR virGetUserEnt fails, then
gid - still to be determined ret = 0
if (gid != (gid_t)-1) {
If we enter this "if", then the for loop will exit immediately and the VIR_APPEND_ELEMENT will append that gid onto *list which is IIRC what we want. Then ret will be set to 1 (i) and we return. Otherwise if gid == -1, then we return 0, which is still fine.
Yep I wanted to achieve exactly this logic as it makes most sense IMO. I've pushed this now. Thanks. Peter