Re: [libvirt] [PATCH v3 1/2] security: also parse user/group names instead of just IDs for DAC labels

On Thu, Oct 04, 2012 at 05:46:31PM -0600, Eric Blake wrote: > On 10/02/2012 11:57 AM, Marcelo Cerri wrote: > > The DAC driver is missing parsing of group and user names for DAC labels > > and currently just parses uid and gid. This patch extends it to support > > names, so the following security label definition is now valid: > > > > <seclabel type='static' model='dac' relabel='yes'> > > <label>qemu:qemu</label> > > <imagelabel>qemu:qemu</imagelabel> > > </seclabel> > > > > When it tries to parse an owner or a group, it first tries to resolve it as > > a name, if it fails or it's an invalid user/group name then it tries to > > parse it as an UID or GID. A leading '+' can also be used for both owner and > > group to force it to be parsed as IDs, so the following example is also > > valid: > > > > <seclabel type='static' model='dac' relabel='yes'> > > <label>+101:+101</label> > > <imagelabel>+101:+101</imagelabel> > > </seclabel> > > > > Yuck. With this patch, I'm seeing lots of ugly error messages in the log: > > 2012-10-04 22:59:52.584+0000: 9225: error : virGetUserID:2535 : Failed > to find user record for name '0': Success > > I think the correct fix is to move this logic... > > > + /* Parse owner */ > > + if (*owner == '+') { > > + if (virStrToLong_ui(++owner, NULL, 10, &theuid) < 0) { > > + virReportError(VIR_ERR_INVALID_ARG, > > + _("Invalid uid \"%s\" in DAC label \"%s\""), > > + owner, label); > > + goto cleanup; > > + } > > + } else { > > + if (virGetUserID(owner, &theuid) < 0 && > > + virStrToLong_ui(owner, NULL, 10, &theuid) < 0) { > > + virReportError(VIR_ERR_INVALID_ARG, > > + _("Invalid owner \"%s\" in DAC label \"%s\""), > > + owner, label); > > + goto cleanup; > > + } > > } > > ...out of security_dac.c and into src/util/util.c:virGetUserID(), so > that we are consistently parsing in this manner for ALL places where we > convert a string into a user id, and also so that virGetUserID will quit > logging such a bogus error message when it fails to find a given id > string that happens to be a valid number. Ok. I'll provide a patch for this. I made a search in the code for virGetUserID and, other than in security_dac.c, it seems to be used just for parsing user name in qemu.conf. > > Likewise for virGetGroupID. Same for virGetGroupID. > > -- > Eric Blake eblake(a)redhat.com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list