On 1/12/20 6:26 PM, Doug Goldstein wrote: > On 1/11/20 3:02 AM, George Dunlap wrote: >> >> >>> On Jan 11, 2020, at 4:02 AM, Doug Goldstein <car...@cardoe.com> wrote: >>> >>> >>> >>> On 1/10/20 4:37 AM, Sergey Dyasli wrote: >>>> Hide the following information that can help identify the running Xen >>>> binary version: XENVER_extraversion, XENVER_compile_info, >>>> XENVER_changeset. >>>> Add explicit cases for XENVER_commandline and XENVER_build_id as well. >>>> Introduce xsm_filter_denied() to hvmloader to remove "<denied>" string >>>> from guest's DMI tables that otherwise would be shown in tools like >>>> dmidecode. >>>> Signed-off-by: Sergey Dyasli <sergey.dya...@citrix.com> >>>> --- >>>> v1 --> v2: >>>> - Added xsm_filter_denied() to hvmloader instead of modifying >>>> xen_deny() >>> >>> So 100% this version of the patch won't fly with the various >>> downstreams that run the v1 of this patch. Those various consumers >>> will stick with v1. >>> >>> If the goal of this is to reduce the burden of the downstreams and >>> their customers to carry a patch against Xen then I wouldn't even >>> bother with this version. >> >> If the goal is to come up with a solution that works for everyone, it >> would be helpful if you said *why* “various consumers” would find this >> patch unacceptable; and also what they might think about the alternate >> solutions proposed (and why). >> >> -George >> >
[snip] > Now I know someone is going to read this and say "Look at Doug and him > advocating for security through obscurity". FWIW I'd be the first person to contradict them, and say you were practicing "defense in depth". :-) > Ultimately my point is if the goal of this patch is to upstream a patch > that's carried by various downstreams, why not actually listen to what > caused them to write the patch? Right, that's what I'm trying to do; but I don't seem to be making much progress. Here's my summary of the situation and arguments so far: 1. The xen_version hypercall can return strings for a number of different values, including XENVER_extraversion, which gives the point release and build id. 2. The XSM dummy module has code to filter which of these are allowed for unprivileged guests. When access to a given value is filtered, no error is returned; rather, the string "<denied>" is returned. 3. Knowledge about the specific instantiation of Xen on which they are running makes it easier for attackers to know how to attack t he system; the XENVER_extraversion provides little value to legitimate users, but a lot of value to attackers. As a defense-in-depth measure, it's important to be able to hide this information. 4. There's currently a patch carried by many downstreams, which changes the XSM dummy module to deny XENVER_extraversion to unprivileged guests. 5. However, this caused "<denied>" to show up in various user-visible places, which caused customer support headaches. So this out-of-tree patch also replaced the string returned when denying access to "" instead. Note that this is not *only* for XENVER_extraversion; with that patch, *any* time the value requested in xen_version is denied by policy, "" will be returned. 6. Silently returning an empty string is considered bad interface design by several developers. So Sergey's second patch: - Still denies XENVER_extraversion at the hypervisor level - Leaves the value returned by the hypervisor as "<denied>" - Filters the "<denied>" string at the hvmloader level, to prevent it leaking into a GUI and scaring customers. Now we get to Andy's objection on the 10th: --- The reason for this (which ought to be obvious, but I guess only to those who actually do customer support) is basic human physiology. "denied" means something has gone wrong. It scares people, and causes them to seek help to change fix whatever is broken. It is not appropriate for it to find its way into the guest in the first place, and that includes turning up in `dmesg` and other logs, and expecting guest runtime to filter for it is complete nonsense. --- Basically, Andy says that *anywhere* it might show up is way too scary, even a guest dmesg log. Well, I disagree; I look in "dmesg" and I see loads of "scary" things. But if "<denied>" is too scary, then we can try "<hidden>". Then we come to your mail. You spend two paragraphs justifying why we need to do #4 (hide the value from unprivileged guests), basically reiterating point #3 and dealing with potential objections. But nobody objects to #4, or disagrees with #3. You then have a paragraph arguing why it's important that information be stripped at the hypervisor rather than in the toolstack. But Sergey's v2 patch *does* strip the information at the hypervisor. His patch makes it so that XENVER_extraversion returns "<denied>". The code which converts "<denied>" to "" in hvmloader is purely a UI thing, so that people looking in their Windows System Info don't get scary messages. > I'd be happy if we had a Kconfig option behind what the string is. Give > me a blank as an option but default it to whatever string like > "<hidden>" that you'd like. Every shipping Xen distro I've worked on has > had its own v1 variant of the patch and none of them authored by me. OK, so with this we have four proposed options: 1. Block XENVER_extraversion at the hypervisor level. Change the xen_deny() string to "". (This is v1 of sergey's patch.) 2. Block XENVER_extraversion at the hypervisor level. Leave xen_deny() as returning "<denied>", but replace "<denied>" with "" in hvmloader so it doesn't show up in the System Info and scare users. 3. Block XENVER_extraversion at the hypervisor level. Change xen_deny() to return a more benign string like "<hidden>". (Perhaps also filter it in hvmloader, just for good measure.) 4. Block XENVER_extraversion at the hypervisor level. Make the xen_deny() string configurable in KConfig. Fundamentally I have no objection to #4. But I still don't know what your objections are to #2 and #3. -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
