[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts |
Date: |
Tue, 26 Oct 2021 09:42:05 +0100 |
User-agent: |
Mutt/2.0.7 (2021-05-04) |
On Mon, Oct 25, 2021 at 09:45:35PM +0200, Alexander Graf wrote:
>
> On 25.10.21 17:10, Daniel P. Berrangé wrote:
> > On Mon, Oct 25, 2021 at 04:53:57PM +0200, Alexander Graf wrote:
> > > On 25.10.21 16:47, Daniel P. Berrangé wrote:
> > > > On Mon, Oct 25, 2021 at 04:42:22PM +0200, Alexander Graf wrote:
> > > > > On 25.10.21 16:22, Daniel P. Berrangé wrote:
> > > > > > On Mon, Oct 25, 2021 at 12:13:32PM +0200, Alexander Graf wrote:
> > > > > > > On 22.10.21 18:14, Vladislav Yaroshchuk wrote:
> > > > > > > > On Apple hosts we can read AppleSMC OSK key directly from host's
> > > > > > > > SMC and forward this value to QEMU Guest.
> > > > > > > >
> > > > > > > > Usage:
> > > > > > > > `-device isa-applesmc,hostosk=on`
> > > > > > > >
> > > > > > > > Apple licence allows use and run up to two additional copies
> > > > > > > > or instances of macOS operating within virtual operating system
> > > > > > > > environments on each Apple-branded computer that is already
> > > > > > > > running
> > > > > > > > the Apple Software, for purposes of:
> > > > > > > > - software development
> > > > > > > > - testing during software development
> > > > > > > > - using macOS Server
> > > > > > > > - personal, non-commercial use
> > > > > > > >
> > > > > > > > Guest macOS requires AppleSMC with correct OSK. The most legal
> > > > > > > > way to pass it to the Guest is to forward the key from host SMC
> > > > > > > > without any value exposion.
> > > > > > > >
> > > > > > > > Based on
> > > > > > > > https://web.archive.org/web/20200103161737/osxbook.com/book/bonus/chapter7/tpmdrmmyth/
> > > > > > > >
> > > > > > > > Signed-off-by: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>
> > > > > > > > @@ -331,6 +464,25 @@ static void
> > > > > > > > applesmc_isa_realize(DeviceState *dev, Error **errp)
> > > > > > > > isa_register_ioport(&s->parent_obj, &s->io_err,
> > > > > > > > s->iobase + APPLESMC_ERR_PORT);
> > > > > > > > + if (s->hostosk_flag) {
> > > > > > > > + /*
> > > > > > > > + * Property 'hostosk' has higher priority than 'osk'
> > > > > > > > + * and shadows it.
> > > > > > > > + * Free user-provided 'osk' property value
> > > > > > > > + */
> > > > > > > > + if (s->osk) {
> > > > > > > > + warn_report("isa-applesmc.osk is shadowed "
> > > > > > > > + "by isa-applesmc.hostosk");
> > > > > > > > + g_free(s->osk);
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + if (!applesmc_read_host_osk(&s->osk, &err)) {
> > > > > > > > + /* On host OSK retrieval error report a warning */
> > > > > > > > + error_report_err(err);
> > > > > > > > + s->osk = default_osk;
> > > > > > > > + }
> > > > > > > > + }
> > > > > > > This part is yucky. A few things:
> > > > > > >
> > > > > > > 1) QEMU in general does not fail user requested operations
> > > > > > > silently. If the
> > > > > > > user explicitly asked to read the host OSK and we couldn't, it
> > > > > > > must
> > > > > > > propagate that error.
> > > > > > > 2) In tandem to the above, I think the only consistent CX is to
> > > > > > > make both
> > > > > > > options mutually exclusive. The easiest way to achieve that IMHO
> > > > > > > would be to
> > > > > > > overload the "osk" property. If it is "host", then use the host
> > > > > > > one.
> > > > > > > 3) Should we make "osk"="host" the default on macOS as well then?
> > > > > > > Of course,
> > > > > > > that one should *not* fail hard when it can't read the key,
> > > > > > > because it's an
> > > > > > > implicit request rather than an explicit one.
> > > > > > The problem with using a magic string value for the existing "osk"
> > > > > > parameter is that this is not introspectable by management apps.
> > > > > What introspectability would you like to have?
> > > > Essentially to answer the question
> > > >
> > > > "Does this QEMU support OSK passthrough from the host"
> > > >
> > > > Mgmt apps like libvirt introspect using various query-XXX QMP commands.
> > > > For devices, the typical approach is to ask for the list of properties
> > > > the device supports. If we're just accepting a new magic value on an
> > > > existing property there is no way to query for existance of that
> > > > feature.
> > > > If we add a "host-osk=bool" parameter introspectability is trivially
> > > > satisfied.
> > >
> > > Ok, the only flow that remains sensible in that case to me sounds like the
> > > following:
> > Just need an extra check upfront:
> >
> > if (s->osk && s->use_hoist_osk)
> > error_setg(errp, ...)
> > else
> > > if (s->osk) {
> > > /* Use osk */
> > This should fail hard if the provided value is the wrong length - currently
> > it falls back with a warning IIUC.
> >
> > > } else if (s->use_host_osk) {
> > > /* Use host OSK. Fail hard if we can't find it */
> > > } else if (can_use_host_osk) {
> > > /* See if we can extract the key from the host. If not, fall back to
> > > old
> > > behavior */
> > > } else {
> > > /* Old fallback behavior */
> > Was this old fallback behaviour actually useful ? IIUC it means it is using
> >
> >
> > static char default_osk[64] = "This is a dummy key. Enter the real key "
> > "using the -osk parameter";
> >
> > which obviously isn't a valid key that will work with any gust OS that
> > cares. I guess it at least let QEMU startup, but any the guest OS that
> > checks the key will be unhappy.
> >
> > If if don't think default_osk is actually useful, then we could simplify
> > further to
> >
> > if (s->osk && s->use_host_osk) {
> > error_setg(errp, ...)
> > } else if (s->osk) {
> > /* Use osk. Fail hard if invalid (ie wrong length) */
> > } else if (s->use_host_osk) {
> > /* Use host OSK. Fail hard if we can't find it */
> > } else {
> > /* try to use host OSK, fail hard if we can't find it or non-OS-X
> > build */
> > }
>
>
> In the example above, use_host_osk=on and use_host_osk=off yield the exact
> same behavior, so we don't need the switch, no?
Hmm, I forgot about machine type back compat. From a strict pov, if we
change the default it should only be for the 6.2.0 machine type, with
old machines keeping the current 'default_osk' behaviour.
IOW, for 6.2.0 machine use_host_osk=on as default, and for < 6.2.0,
use_host_osk=off as default .
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts, Vladislav Yaroshchuk, 2021/10/22
- Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts, Alexander Graf, 2021/10/25
- Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts, Vladislav Yaroshchuk, 2021/10/25
- Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts, Daniel P . Berrangé, 2021/10/25
- Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts, Alexander Graf, 2021/10/25
- Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts, Daniel P . Berrangé, 2021/10/25
- Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts, Alexander Graf, 2021/10/25
- Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts, Daniel P . Berrangé, 2021/10/25
- Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts, Alexander Graf, 2021/10/25
- Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts,
Daniel P . Berrangé <=
- Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts, Alexander Graf, 2021/10/26