Re: [PATCH for-4.5 v6 00/16] Xen VMware tools support

["Jan Beulich" <JBeulich@suse.com>]

>>> On 30.09.14 at 01:13, <dslutz@verizon.com> wrote:
> On 09/29/14 09:27, George Dunlap wrote:
>> On 09/29/2014 07:50 AM, Jan Beulich wrote:
>>> a) I don't think it is okay to base our emulation layer entirely
>>> on observed behavior. At least some form of specification should
>>> be there to follow. This is both for reviewing the code you want
>>> committed and maintainability.
>>
>> While that would be nice, I think that's unlikely; and overall I think 
>> it would be better to have a reverse-engineered implementation than no 
>> implementation at all.  Having a reverse-engineered spec might be a 
>> good idea though.
>>
> 
> I could work on a reverse-engineered spec.  Is having this on the wiki
> good enough or does it need to be in the code?

I don't think the place it's at matters that much. All that does matter
is if it's something outside of our control, it should be a place that
reasonably certainly won't go away any time soon, so that a link
placed somewhere in our tree won't become stale.

>>> b) I don't think it is okay to introduce security issues into a guest
>>> even if that is something that isn't enabled by default.
>>
>> I agree with this; in particular, it's quite possible that someone 
>> will decide to enable VMWare functionality by default, "just in case", 
>> and then forget that they've done so.
>>
> 
> I am assuming that the phrase "security issues" is used as a
> reference to things like http://xenbits.xen.org/xsa/ or
> http://wiki.xen.org/wiki/Securing_Xen.
> 
> Or as it might be stated -- A way to cause a guest to crash or have
> a DoS (/Denial of Service) or a way in from outside (like "/SMASH the
> Bash bug".
> 
> 
> But not the area of
> http://en.wikipedia.org/wiki/Rainbow_Series or
> http://en.wikipedia.org/wiki/Trusted_Computer_System_Evaluation_Criteria 
> 
> Which talks about "Covert Channel Analysis" and other complex
> security issues. (like *"Evaluation Assurance Level", **"Trusted 
> Computer System Evaluation Criteria", etc.)*

Covert channels are consider security issues too when applying
strict criteria. But the main concern here are indeed ways for guest
user mode to badly affect the guest as a whole (or the host, but I
think that should really go without saying).

> I feel it is "safe" to run all guests with vmware_port=1 and
> vmware_hw=7.  However I am not stating that all guests function
> the same with just this.  I do know that xen_platform_pci=0
> may also need to be specified to get expected results.
> 
> I also do not understand the statement "enable VMWare functionality by
> default".  I must be missing something because as far as I know each
> guest (domU) has it's own config.  Is this a xl tool stack feature (some
> common config for guests)? Or is it some other tool stack feature?

Higher layer management tools may choose to create guest configs
that have certain settings always enabled (like at least used to be
the case in XenServer for the Viridian flag - not sure if that got
changed -, i.e. enabling this even for non-Windows guests, which
caused issues with Linux).

>>> c) Apparent or real flaws with VMware's native implementation
>>> should be brought up with VMware. While mimicking their behavior
>>> as closely as possible is certainly a desirable goal, reproducing
>>> flaws their code has should imo be avoided if at all possible.
>>
>> If our goal is compatibility with exiting tools, is there really such 
>> a thing as "reproducing flaws"?  Obviously we shouldn't reproduce a 
>> real security flaw, but for everything else, if the feature is "Looks 
>> just like VMWare", then being as close as possible in behavior is the 
>> ideal.
>>
> 
> I can agree that is it ideal.  However getting to this ideal place can have
> a high cost.  Case in point "BDOOR_CMD_GETHZ".  The VMware provided
> include file has no CPL comments.  The same include file in "open vm
> tools" does, but not for BDOOR_CMD_GETHZ.  However a VMware
> test system does different things for ring 0 (aka a Linux kernel module)
> and ring 3.  Neither one report a fault, but the ring 3 one does not
> return any data.  I do not know that the result is if I enable ring 3 I/O
> via the TSS (since I do not know of a simple way to do that).  If I change
> IOPL then it still hides information.
> 
> Now there is also the "BDOOR_CMD_GETMHZ".  It also has no CPL
> statement but does return the tsc_freq in MHZ.  So why is tsc_freq
> in HZ considered sensitive information, but in MHZ it is not?
> 
> Just to confuse this more, for vmware_hw=7, cpuid leave 0x40000010
> has tsc_freq in KHZ.  So again why is tsc_freq in HZ special?

I think this was just an example. All of the command need to be
considered wrt the (guest) privilege they require.

> How about the comment on other commands "/* CPL 0 only. */"
> which appears to apply here, but the comment is missing.  So
> do I check the segment register SS's DPL (what I know of as CPL),
> or is it the segment register CS's DPL (which has been mistakenly called
> CPL by some programs)?  Or is it something else which VMware has
> decided to mean "ring 3"?

SS DPL is the canonical way for determining CPL - you can find a
number of examples throughout the code base.

Jan