[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv3] security: Don't ignore errors when parsing DAC security labels

On 09/20/12 16:04, Martin Kletzander wrote:
On 09/19/2012 02:42 PM, Peter Krempa wrote:
The DAC security driver silently ignored errors when parsing the DAC
label and used default values instead.

With a domain containing the following label definition:

<seclabel type='static' model='dac' relabel='yes'>

the domain would start normaly but the disk images would be still owned
by root and no error was displayed.

This patch changes the behavior if the parsing of the label fails (note
that a not present label is not a failure and in this case the default
label should be used) the error isn't masked but is raised that causes
the domain start to fail with a descriptive error message:

virsh #  start tr
error: Failed to start domain tr
error: internal error invalid argument: failed to parse DAC label
'sdfklsdjlfjklsdjkl' for domain 'tr'

I also changed the error code to "invalid argument" from "internal
error" and tweaked the various error messages to contain correct and
useful information.
Diff to v2:
- Fixed all error reporting paths to contain useful messages
- Tweaked error messages to contain more information
- Fixed printing of seclabel->label instead of seclabel->imagelabel in the imagelabel parsing func.

  src/security/security_dac.c | 95 +++++++++++++++++++++++++++++----------------
  1 file changed, 61 insertions(+), 34 deletions(-)


Code seems fine, though, so ACK with unifying those messages. I suggest
either "seclabel" and "imagelabel" OR 'security label" and "security
imagelabel", whatever you think suits the error and XML better, but one
for all errors with the same label.

I've gone with the first option and pushed the patch. Thanks for the review.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]