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

Re: [Xen-devel] [PATCH] xsm: hide detailed Xen version from unprivileged guests


  • To: Jan Beulich <jbeulich@xxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
  • From: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
  • Date: Thu, 19 Dec 2019 11:23:58 +0000
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=sergey.dyasli@xxxxxxxxxx; spf=Pass smtp.mailfrom=sergey.dyasli@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx
  • Autocrypt: addr=sergey.dyasli@xxxxxxxxxx; keydata= mQINBFtMVHEBEADc/hZcLexrB6vGTdGqEUsYZkFGQh6Z1OO7bCtM1go1RugSMeq9tkFHQSOc 9c7W9NVQqLgn8eefikIHxgic6tGgKoIQKcPuSsnqGao2YabsTSSoeatvmO5HkR0xGaUd+M6j iqv3cD7/WL602NhphT4ucKXCz93w0TeoJ3gleLuILxmzg1gDhKtMdkZv6TngWpKgIMRfoyHQ jsVzPbTTjJl/a9Cw99vuhFuEJfzbLA80hCwhoPM+ZQGFDcG4c25GQGQFFatpbQUhNirWW5b1 r2yVOziSJsvfTLnyzEizCvU+r/Ek2Kh0eAsRFr35m2X+X3CfxKrZcePxzAf273p4nc3YIK9h cwa4ZpDksun0E2l0pIxg/pPBXTNbH+OX1I+BfWDZWlPiPxgkiKdgYPS2qv53dJ+k9x6HkuCy i61IcjXRtVgL5nPGakyOFQ+07S4HIJlw98a6NrptWOFkxDt38x87mSM7aSWp1kjyGqQTGoKB VEx5BdRS5gFdYGCQFc8KVGEWPPGdeYx9Pj2wTaweKV0qZT69lmf/P5149Pc81SRhuc0hUX9K DnYBa1iSHaDjifMsNXKzj8Y8zVm+J6DZo/D10IUxMuExvbPa/8nsertWxoDSbWcF1cyvZp9X tUEukuPoTKO4Vzg7xVNj9pbK9GPxSYcafJUgDeKEIlkn3iVIPwARAQABtChTZXJnZXkgRHlh c2xpIDxzZXJnZXkuZHlhc2xpQGNpdHJpeC5jb20+iQJOBBMBCgA4FiEEkI7HMI5EbM2FLA1L Aa+w5JvbyusFAltMVHECGwMFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQAa+w5JvbyuuQ JBAAry/oRK6m0I+ck1Tarz9a1RrF73r1YoJUk5Bw+PSxsBJOPp3vDeAz3Kqw58qmBXeNlMU4 1cqAxFxCCKMtER1gpmrKWBA1/H1ZoBRtzhaHgPTQLyR7LB1OgdpgwEOjN1Q5gME8Pk21y/3N cG5YBgD/ZHbq8nWS/G3r001Ie3nX55uacGk/Ry175cS48+asrerShKMDNMT1cwimo9zH/3Lm RTpWloh2dG4jjwtCXqB7s+FEE5wQVCpPp9p55+9pPd+3DXmsQEcJ/28XHo/UJW663WjRlRc4 wgPwiC9Co1HqaMKSzdPpZmI5D4HizWH8jF7ppUjWoPapwk4dEA7Al0vx1Bz3gbJAL8DaRgQp H4j/16ifletfGUNbHJR2vWljZ5SEf2vMVcdubf9eFUfBF/9OOR1Kcj1PISP8sPhcP7oCfFtH RcxXh1OStrRFtltJt2VlloKXAUggdewwyyD4xl9UHCfI4lSexOK37wNSQYPQcVcOS1bl4NhQ em6pw2AC32NsnQE5PmczFADDIpWhO/+WtkTFeE2HHfAn++y3YDtKQd7xes9UJjQNiGziArST l6Zrx4/nShVLeYRVW76l27gI5a8BZLWwBVRsWniGM50OOJULvSag7kh+cjsrXXpNuA4rfEoB Bxr7pso9e5YghupDc8XftsYd7mlAgOTCAC8uZme5Ag0EW0xUcQEQAMKi97v3DwwPgYVPYIbQ JAvoMgubJllC9RcE0PQsE6nEKSrfOT6Gh5/LHOXLbQI9nzU/xdr6kMfwbYVTnZIY/SwsLrJa gSKm64t11MjC1Vf03/sncx1tgI7nwqMMIAYLsXnQ9X/Up5L/gLO2YDIPxrQ6g4glgRYPT53i r6/hTz3dlpqyPCorpuF+WY7P2ujhlFlXCAaD6btPPM/9LZSmI0xS4aCBLH+pZeCr0UGSMhsX JYN0QRLjfsIDGyqaXVH9gwV2Hgsq6z8fNPQlBc3IpDvfXa1rYtgldYBfG521L3wnsMcKoFSr R5dpH7Jtvv5YBuAk8r571qlMhyAmVKiEnc+RonWl503D5bAHqNmFNjV248J5scyRD/+BcYLI 2CFG28XZrCvjxq3ux5hpmg2fCu+y98h6/yuwB/JhbFlDOSoluEpysiEL3R5GTKbxOF664q5W fiSObxNONxs86UtghqNDRUJgyS0W6TfykGOnZDVYAC9Gg8SbQDta1ymA0q76S/NG2MrJEOIr 1GtOr/UjNv2x4vW56dzX/3yuhK1ilpgzh1q504ETC6EKXMaFT8cNgsMlk9dOvWPwlsIJ249+ PizMDFGITxGTIrQAaUBO+HRLSBYdHNrHJtytkBoTjykCt7M6pl7l+jFYjGSw4fwexVy0MqsD AZ2coH82RTPb6Q7JABEBAAGJAjYEGAEKACAWIQSQjscwjkRszYUsDUsBr7Dkm9vK6wUCW0xU cQIbDAAKCRABr7Dkm9vK6+9uD/9Ld3X5cvnrwrkFMddpjFKoJ4yphtX2s+EQfKT6vMq3A1dJ tI7zHTFm60uBhX6eRbQow8fkHPcjXGJEoCSJf8ktwx/HYcBcnUK/aulHpvHIIYEma7BHry4x L+Ap7oBbBNiraS3Wu1k+MaX07BWhYYkpu7akUEtaYsCceVc4vpYNITUzPYCHeMwc5pLICA+7 VdI1rrTSAwlCtLGBt7ttbvaAKN4dysiN+/66Hlxnn8n952lZdG4ThPPzafG50EgcTa+dASgm tc6HaQAmJiwb4iWUOoUoM+udLRHcN6cE0bQivyH1bqF4ROeFBRz00MUJKvzUynR9E50F9hmd DOBJkyM3Z5imQ0RayEkRHhlhj7uECaojnUeewq4zjpAg2HTSMkdEzKRbdMEyXCdQXFnSCmUB 5yMIULuDbOODWo3EufExLjAKzIRWEKQ/JidLzO6hrhlQffsJ7MPTU+Hg7WxqWfn4zhuUcIQB SlkiRMalSiJITC2jG7oQRRh9tyNaDMkKzTbeFtHKRmUUAuhE0LBXP8Wc+5W7b3WOf2SO8JMR 4TqDZ0K06s66S5fOTW0h56iCCxTsAnRvM/tA4SERyRoFs/iTqJzboskZY0yKeWV4/IQxfOyC YwdU3//zANM1ZpqeE/8lnW/kx+fyzVyEioLSwkjDvdG++4GQ5r6PHQ7BbdEWhA==
  • Cc: "sergey.dyasli@xxxxxxxxxx >> Sergey Dyasli" <sergey.dyasli@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxx, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
  • Delivery-date: Thu, 19 Dec 2019 11:24:17 +0000
  • Ironport-sdr: k16zD4NNSObNWCn8p73A824NIFCArc2WW6SnQCz08WcrBM84HCD6zek+jlKt4kPOA9BSF1Im+p owsgfEpcco84hruN2Dk2EiOslDkj6NCWujaJnDpSJoc+doJEOPzD6R8gdK0DPMmk0E8hJ1mShH ztlXN1NcH19316QYutkFGVQo0es0uV3jQ60YyxbDnW+IOVne8CQYSgDBe/8pdw6VWVlpmFU2Ke GiuQ09OeYV6JaHUucaaxcYDc6UeEsteaXDIcTjx8IqXplDwZHf0GlIDJieCxowiovzYZ18quXE ufY=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Openpgp: preference=signencrypt

On 18/12/2019 11:06, Jan Beulich wrote:
> On 17.12.2019 16:46, Sergey Dyasli wrote:
>> Hide the following information that can help identify the running Xen
>> binary version:
>>
>>     XENVER_extraversion
>>     XENVER_compile_info
>>     XENVER_capabilities
> 
> What's wrong with exposing this one?

extraversion can help identify the exact running Xen binary (and I must
say that at Citrix we add some additional version information there).
compile_info will identify compiler and many speculative mitigations
are provided by compilers these days. Not sure if it's worth hiding
capabilities, but OTOH I don't see how guests could meaningfully use it.

> 
>>     XENVER_changeset
>>     XENVER_commandline
>>     XENVER_build_id
>>
>> Return a more customer friendly empty string instead of "<denied>"
>> which would be shown in tools like dmidecode.>
> I think "<denied>" is quite fine for many of the original purposes.
> Maybe it would be better to filter for this when populating guest
> DMI tables?

I don't know how DMI tables are populated, but nothing stops a guest
from using these hypercalls directly.

> 
>> But allow guests to see this information in Debug builds of Xen.
> 
> Behavior like this would imo better not differ between debug and
> release builds, or else guest software may behave entirely
> differently once you put it on a production build.

I agree on this one, it's not much worth providing this information in
debug builds.

> 
>> --- a/xen/include/xsm/dummy.h
>> +++ b/xen/include/xsm/dummy.h
>> @@ -750,16 +750,21 @@ static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG 
>> uint32_t op)
>>      case XENVER_get_features:
>>          /* These sub-ops ignore the permission checks and return data. */
>>          return 0;
>> -    case XENVER_extraversion:
>> -    case XENVER_compile_info:
>> -    case XENVER_capabilities:
>> -    case XENVER_changeset:
>>      case XENVER_pagesize:
>>      case XENVER_guest_handle:
>>          /* These MUST always be accessible to any guest by default. */
> 
> This comment, not the least because of its use of capitals,
> suggests to me that there's further justification needed for
> your change, including discussion of why there's no risk of
> breaking existing guests.

Not sure about this comment, maybe the author (Konrad) remembers?
We had this change in production for very long time now and haven't
seen any guest regressions.

> 
>>          return xsm_default_action(XSM_HOOK, current->domain, NULL);
>> +
>> +    case XENVER_extraversion:
>> +    case XENVER_compile_info:
>> +    case XENVER_capabilities:
>> +    case XENVER_changeset:
>> +    case XENVER_commandline:
>> +    case XENVER_build_id:
>>      default:
> 
> There's no need to add all of these next to the default case.
> Note how commandline and build_id have been coming here already
> (and there would need to be further justification for exposing
> them on debug builds, should this questionable behavior - see
> above - be retained).

I find that explicitly listing all the cases is better for code
readability, but I don't have a strong opinion here.

--
Thanks,
Sergey

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.