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

[George Dunlap <george.dunlap@eu.citrix.com>]

On 09/29/2014 07:50 AM, Jan Beulich wrote:
>>>> On 26.09.14 at 22:00, <dslutz@verizon.com> wrote:
>> On 09/25/14 06:37, Tim Deegan wrote:
>>> At 17:18 +0100 on 22 Sep (1411402700), Jan Beulich wrote:
>>>> That's indeed what was said so far. I wonder though whether opening
>>>> this up without guest OS consent isn't gong to introduce a security
>>>> issue inside the guest (depending on the exact functionality of these
>>>> hypercalls).
>>> Yes indeed.  VMware seems to have CPL checks on some of the commands
>>> (but not all).  I guess Xen will be no worse than VMware if we do the
>>> same, though I'd like to have an official spec to follow for that.
>> Yes, VMware has CPL checks on some of the commands.  Not at all
>> clear the include file has the correct statement.  I have not do any
>> checking of CPL nor does QEMU.  And the RPC (which is CPL 3) is
>> one of the most likely to have a security issue.
>>
>> I do not know of an official spec to follow.  The best I have the
>> the provided include file and testing on VMware.
>>
>> I do know that BDOOR_CMD_GETHZ is one that is not allowed in
>> ring 3, but this makes no sense to me.  I do not see why tsc_freq
>> and apic bus speed to be things to hide.  And VMware is not
>> consistent.  On newer configs this same info is available via
>> cpuid leaf in ring 3.
>>
>> Also I have not idea if VMware did the CPL checking "correctly".
>> I.E. is a #GP => CPL 3, or do they check CPL?
>>
>> All this leads to I current do not check CPL on any VMware commands.
>>
>> I could look into doing this, but with the xl.cfg flag vmware_port=0
>> turns this all off, I do not see any need for CPL checking.
> Hmm, I think we need to settle on certain things here:
> 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.

> 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.

> 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.

  -George