On Apple hosts we can read AppleSMC OSK key directly from host's
SMC and forward this value to QEMU Guest.

New 'hostosk' property is added:
* `-device isa-applesmc,hostosk=on`
The property is set to 'on' by default for machine version > 7.0

Apple licence allows use and run up to two additional copies
or instances of macOS operating system 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>
---
 hw/core/machine.c  |   4 +-
 hw/misc/applesmc.c | 125 +++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 125 insertions(+), 4 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index cb9bbc844d..7f4a27406a 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -37,7 +37,9 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-pci.h"
 
-GlobalProperty hw_compat_7_0[] = {};
+GlobalProperty hw_compat_7_0[] = {
+    { "isa-applesmc", "hostosk", "off" }
+};
 const size_t hw_compat_7_0_len = G_N_ELEMENTS(hw_compat_7_0);
 
 GlobalProperty hw_compat_6_2[] = {
diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index 81cd6b6423..8672c9d56e 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -37,6 +37,11 @@
 #include "qemu/module.h"
 #include "qemu/timer.h"
 #include "qom/object.h"
+#include "qapi/error.h"
+
+#if defined(__APPLE__) && defined(__MACH__)
+#include <IOKit/IOKitLib.h>
+#endif
 
 /* #define DEBUG_SMC */
 
@@ -80,7 +85,7 @@ enum {
 #define smc_debug(...) do { } while (0)
 #endif
 
-static char default_osk[64] = "This is a dummy key. Enter the real key "
+static char default_osk[65] = "This is a dummy key. Enter the real key "
                               "using the -osk parameter";
 
 struct AppleSMCData {
@@ -109,6 +114,7 @@ struct AppleSMCState {
     uint8_t data_pos;
     uint8_t data[255];
     char *osk;
+    bool hostosk;
     QLIST_HEAD(, AppleSMCData) data_def;
 };
 
@@ -312,6 +318,101 @@ static const MemoryRegionOps applesmc_err_io_ops = {
     },
 };
 
+#if defined(__APPLE__) && defined(__MACH__)
+/*
+ * Based on
+ * https://web.archive.org/web/20200103161737/osxbook.com/book/bonus/chapter7/tpmdrmmyth/
+ */
+enum {
+    SMC_HANDLE_EVENT     = 2,
+    SMC_READ_KEY         = 5
+};
+
+struct AppleSMCParam {
+    uint32_t key;
+    uint8_t pad0[22];
+    IOByteCount data_size;
+    uint8_t pad1[10];
+    uint8_t command;
+    uint32_t pad2;
+    uint8_t bytes[32];
+};
+
+static bool applesmc_read_host_osk(char *host_osk, Error **errp)
+{
+    assert(host_osk != NULL);
+
+    io_service_t hostsmc_service = IO_OBJECT_NULL;
+    io_connect_t hostsmc_connect = IO_OBJECT_NULL;
+    size_t smc_param_size = sizeof(struct AppleSMCParam);
+    IOReturn status = kIOReturnError;
+    int i;
+
+    struct AppleSMCParam smc_param[2] = {
+         {
+             .key = ('OSK0'),
+             .data_size = sizeof(smc_param[0].bytes),
+             .command = SMC_READ_KEY,
+         }, {
+             .key = ('OSK1'),
+             .data_size = sizeof(smc_param[0].bytes),
+             .command = SMC_READ_KEY,
+         },
+    };
+
+    hostsmc_service = IOServiceGetMatchingService(
+        kIOMasterPortDefault,
+        IOServiceMatching("AppleSMC"));
+    if (hostsmc_service == IO_OBJECT_NULL) {
+        error_setg(errp, "Unable to get host-AppleSMC service");
+        goto error;
+    }
+
+    status = IOServiceOpen(hostsmc_service,
+                           mach_task_self(),
+                           0,
+                           &hostsmc_connect);
+    if (status != kIOReturnSuccess || hostsmc_connect == IO_OBJECT_NULL) {
+        error_setg(errp, "Unable to open host-AppleSMC service");
+        goto error;
+    }
+
+    for (i = 0; i < ARRAY_SIZE(smc_param); ++i) {
+        status = IOConnectCallStructMethod(
+                hostsmc_connect,
+                SMC_HANDLE_EVENT,
+                &smc_param[i],
+                sizeof(struct AppleSMCParam),
+                &smc_param[i],
+                &smc_param_size
+            );
+
+        if (status != kIOReturnSuccess) {
+            error_setg(errp, "Unable to read OSK from host-AppleSMC");
+            goto error;
+        }
+    }
+
+    memcpy(host_osk, smc_param[0].bytes, 32);
+    memcpy(host_osk + 32, smc_param[1].bytes, 32);
+
+    IOServiceClose(hostsmc_connect);
+    IOObjectRelease(hostsmc_service);
+    return true;
+
+error:
+    IOServiceClose(hostsmc_connect);
+    IOObjectRelease(hostsmc_service);
+    return false;
+}
+#else
+static bool applesmc_read_host_osk(char *host_osk, Error **errp)
+{
+    error_setg(errp, "OSK read is not supported on this host");
+    return false;
+}
+#endif
+
 static void applesmc_isa_realize(DeviceState *dev, Error **errp)
 {
     AppleSMCState *s = APPLE_SMC(dev);
@@ -331,9 +432,26 @@ 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->osk || (strlen(s->osk) != 64)) {
-        warn_report("Using AppleSMC with invalid key");
+    if (s->osk && s->hostosk) {
+        error_setg(errp, "-osk property cannot be used with -hostosk=on");
+    }
+
+    if (!s->osk && !s->hostosk) {
         s->osk = default_osk;
+        warn_report("Using AppleSMC with default (dummy) OSK");
+    }
+
+    if (s->hostosk) {
+        s->osk = g_malloc0(65);
+        /* Fail hard if we cannot read requested host OSK */
+        if (!applesmc_read_host_osk(s->osk, errp)) {
+            g_assert_not_reached();
+        }
+    }
+
+    if (s->osk && strlen(s->osk) != 64) {
+        /* Only valid OSK is accepted within 'osk' property */
+        error_setg(errp, "Using AppleSMC with key of invalid length");
     }
 
     QLIST_INIT(&s->data_def);
@@ -344,6 +462,7 @@ static Property applesmc_isa_properties[] = {
     DEFINE_PROP_UINT32(APPLESMC_PROP_IO_BASE, AppleSMCState, iobase,
                        APPLESMC_DEFAULT_IOBASE),
     DEFINE_PROP_STRING("osk", AppleSMCState, osk),
+    DEFINE_PROP_BOOL("hostosk", AppleSMCState, hostosk, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.34.1.vfs.0.0

> Am 29.04.2022 um 21:18 schrieb Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>:
> 
> On Apple hosts we can read AppleSMC OSK key directly from host's
> SMC and forward this value to QEMU Guest.
> 
> New 'hostosk' property is added:
> * `-device isa-applesmc,hostosk=on`
> The property is set to 'on' by default for machine version > 7.0
> 
> Apple licence allows use and run up to two additional copies
> or instances of macOS operating system 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>
> ---
> hw/core/machine.c  |   4 +-
> hw/misc/applesmc.c | 125 +++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 125 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index cb9bbc844d..7f4a27406a 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -37,7 +37,9 @@
> #include "hw/virtio/virtio.h"
> #include "hw/virtio/virtio-pci.h"
> 
> -GlobalProperty hw_compat_7_0[] = {};
> +GlobalProperty hw_compat_7_0[] = {
> +    { "isa-applesmc", "hostosk", "off" }
> +};
> const size_t hw_compat_7_0_len = G_N_ELEMENTS(hw_compat_7_0);
> 
> GlobalProperty hw_compat_6_2[] = {
> diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
> index 81cd6b6423..8672c9d56e 100644
> --- a/hw/misc/applesmc.c
> +++ b/hw/misc/applesmc.c
> @@ -37,6 +37,11 @@
> #include "qemu/module.h"
> #include "qemu/timer.h"
> #include "qom/object.h"
> +#include "qapi/error.h"
> +
> +#if defined(__APPLE__) && defined(__MACH__)
> +#include <IOKit/IOKitLib.h>
> +#endif
> 
> /* #define DEBUG_SMC */
> 
> @@ -80,7 +85,7 @@ enum {
> #define smc_debug(...) do { } while (0)
> #endif
> 
> -static char default_osk[64] = "This is a dummy key. Enter the real key "
> +static char default_osk[65] = "This is a dummy key. Enter the real key "
>                               "using the -osk parameter";
> 
> struct AppleSMCData {
> @@ -109,6 +114,7 @@ struct AppleSMCState {
>     uint8_t data_pos;
>     uint8_t data[255];
>     char *osk;
> +    bool hostosk;
>     QLIST_HEAD(, AppleSMCData) data_def;
> };
> 
> @@ -312,6 +318,101 @@ static const MemoryRegionOps applesmc_err_io_ops = {
>     },
> };
> 
> +#if defined(__APPLE__) && defined(__MACH__)
> +/*
> + * Based on
> + * https://web.archive.org/web/20200103161737/osxbook.com/book/bonus/chapter7/tpmdrmmyth/
> + */
> +enum {
> +    SMC_HANDLE_EVENT     = 2,
> +    SMC_READ_KEY         = 5
> +};
> +
> +struct AppleSMCParam {
> +    uint32_t key;
> +    uint8_t pad0[22];
> +    IOByteCount data_size;
> +    uint8_t pad1[10];
> +    uint8_t command;
> +    uint32_t pad2;
> +    uint8_t bytes[32];
> +};
> +
> +static bool applesmc_read_host_osk(char *host_osk, Error **errp)
> +{
> +    assert(host_osk != NULL);
> +
> +    io_service_t hostsmc_service = IO_OBJECT_NULL;
> +    io_connect_t hostsmc_connect = IO_OBJECT_NULL;
> +    size_t smc_param_size = sizeof(struct AppleSMCParam);
> +    IOReturn status = kIOReturnError;
> +    int i;
> +
> +    struct AppleSMCParam smc_param[2] = {
> +         {
> +             .key = ('OSK0'),
> +             .data_size = sizeof(smc_param[0].bytes),
> +             .command = SMC_READ_KEY,
> +         }, {
> +             .key = ('OSK1'),
> +             .data_size = sizeof(smc_param[0].bytes),
> +             .command = SMC_READ_KEY,
> +         },
> +    };
> +
> +    hostsmc_service = IOServiceGetMatchingService(
> +        kIOMasterPortDefault,
> +        IOServiceMatching("AppleSMC"));
> +    if (hostsmc_service == IO_OBJECT_NULL) {
> +        error_setg(errp, "Unable to get host-AppleSMC service");
> +        goto error;
> +    }
> +
> +    status = IOServiceOpen(hostsmc_service,
> +                           mach_task_self(),
> +                           0,
> +                           &hostsmc_connect);
> +    if (status != kIOReturnSuccess || hostsmc_connect == IO_OBJECT_NULL) {
> +        error_setg(errp, "Unable to open host-AppleSMC service");
> +        goto error;
> +    }
> +
> +    for (i = 0; i < ARRAY_SIZE(smc_param); ++i) {
> +        status = IOConnectCallStructMethod(
> +                hostsmc_connect,
> +                SMC_HANDLE_EVENT,
> +                &smc_param[i],
> +                sizeof(struct AppleSMCParam),
> +                &smc_param[i],
> +                &smc_param_size
> +            );
> +
> +        if (status != kIOReturnSuccess) {
> +            error_setg(errp, "Unable to read OSK from host-AppleSMC");
> +            goto error;
> +        }
> +    }
> +
> +    memcpy(host_osk, smc_param[0].bytes, 32);
> +    memcpy(host_osk + 32, smc_param[1].bytes, 32);
> +
> +    IOServiceClose(hostsmc_connect);
> +    IOObjectRelease(hostsmc_service);
> +    return true;
> +
> +error:
> +    IOServiceClose(hostsmc_connect);
> +    IOObjectRelease(hostsmc_service);
> +    return false;
> +}
> +#else
> +static bool applesmc_read_host_osk(char *host_osk, Error **errp)
> +{
> +    error_setg(errp, "OSK read is not supported on this host");
> +    return false;
> +}
> +#endif
> +
> static void applesmc_isa_realize(DeviceState *dev, Error **errp)
> {
>     AppleSMCState *s = APPLE_SMC(dev);
> @@ -331,9 +432,26 @@ 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->osk || (strlen(s->osk) != 64)) {
> -        warn_report("Using AppleSMC with invalid key");
> +    if (s->osk && s->hostosk) {
> +        error_setg(errp, "-osk property cannot be used with -hostosk=on");
> +    }
> +
> +    if (!s->osk && !s->hostosk) {
>         s->osk = default_osk;
> +        warn_report("Using AppleSMC with default (dummy) OSK");
> +    }
> +
> +    if (s->hostosk) {
> +        s->osk = g_malloc0(65);
> +        /* Fail hard if we cannot read requested host OSK */
> +        if (!applesmc_read_host_osk(s->osk, errp)) {
> +            g_assert_not_reached();
> +        }
> +    }
> +
> +    if (s->osk && strlen(s->osk) != 64) {
> +        /* Only valid OSK is accepted within 'osk' property */
> +        error_setg(errp, "Using AppleSMC with key of invalid length");
>     }
> 
>     QLIST_INIT(&s->data_def);
> @@ -344,6 +462,7 @@ static Property applesmc_isa_properties[] = {
>     DEFINE_PROP_UINT32(APPLESMC_PROP_IO_BASE, AppleSMCState, iobase,
>                        APPLESMC_DEFAULT_IOBASE),
>     DEFINE_PROP_STRING("osk", AppleSMCState, osk),
> +    DEFINE_PROP_BOOL("hostosk", AppleSMCState, hostosk, true),

I think this should only default to true on x86 macOS. On Linux hosts or M1, we want to go through the normal 3rd party provided osk logic by default. Otherwise in 15 years when x86 Macs are fully deprecated, we'd still need to pass hostosk=false on the cmdline for archeology QEMU macOS VMs ;).

Alex

>     DEFINE_PROP_END_OF_LIST(),
> };
> 
> -- 
> 2.34.1.vfs.0.0
> 

Hey Vladislav,

On 29.04.22 21:18, Vladislav Yaroshchuk wrote:
> On Apple hosts we can read AppleSMC OSK key directly from host's
> SMC and forward this value to QEMU Guest.
>
> New 'hostosk' property is added:
> * `-device isa-applesmc,hostosk=on`
> The property is set to 'on' by default for machine version > 7.0
>
> Apple licence allows use and run up to two additional copies
> or instances of macOS operating system 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>
> ---
>   hw/core/machine.c  |   4 +-
>   hw/misc/applesmc.c | 125 +++++++++++++++++++++++++++++++++++++++++++--
>   2 files changed, 125 insertions(+), 4 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index cb9bbc844d..7f4a27406a 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -37,7 +37,9 @@
>   #include "hw/virtio/virtio.h"
>   #include "hw/virtio/virtio-pci.h"
>   
> -GlobalProperty hw_compat_7_0[] = {};
> +GlobalProperty hw_compat_7_0[] = {
> +    { "isa-applesmc", "hostosk", "off" }
> +};
>   const size_t hw_compat_7_0_len = G_N_ELEMENTS(hw_compat_7_0);
>   
>   GlobalProperty hw_compat_6_2[] = {
> diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
> index 81cd6b6423..8672c9d56e 100644
> --- a/hw/misc/applesmc.c
> +++ b/hw/misc/applesmc.c
> @@ -37,6 +37,11 @@
>   #include "qemu/module.h"
>   #include "qemu/timer.h"
>   #include "qom/object.h"
> +#include "qapi/error.h"
> +
> +#if defined(__APPLE__) && defined(__MACH__)
> +#include <IOKit/IOKitLib.h>
> +#endif
>   
>   /* #define DEBUG_SMC */
>   
> @@ -80,7 +85,7 @@ enum {
>   #define smc_debug(...) do { } while (0)
>   #endif
>   
> -static char default_osk[64] = "This is a dummy key. Enter the real key "
> +static char default_osk[65] = "This is a dummy key. Enter the real key "


This is only necessary if we run strlen() on the default_osk. Do we have to?


>                                 "using the -osk parameter";
>   
>   struct AppleSMCData {
> @@ -109,6 +114,7 @@ struct AppleSMCState {
>       uint8_t data_pos;
>       uint8_t data[255];
>       char *osk;
> +    bool hostosk;
>       QLIST_HEAD(, AppleSMCData) data_def;
>   };
>   
> @@ -312,6 +318,101 @@ static const MemoryRegionOps applesmc_err_io_ops = {
>       },
>   };
>   
> +#if defined(__APPLE__) && defined(__MACH__)


This needs to check for x86 as well, no?


> +/*
> + * Based on
> + * https://web.archive.org/web/20200103161737/osxbook.com/book/bonus/chapter7/tpmdrmmyth/
> + */
> +enum {
> +    SMC_HANDLE_EVENT     = 2,
> +    SMC_READ_KEY         = 5
> +};
> +
> +struct AppleSMCParam {
> +    uint32_t key;
> +    uint8_t pad0[22];
> +    IOByteCount data_size;
> +    uint8_t pad1[10];
> +    uint8_t command;


This looks odd. You have 10 bytes padding + 1 byte command between a u64 
and a u32. Please make sure that there the struct would be identical if 
you add __attribute__((packed)).


> +    uint32_t pad2;
> +    uint8_t bytes[32];
> +};
> +
> +static bool applesmc_read_host_osk(char *host_osk, Error **errp)
> +{
> +    assert(host_osk != NULL);


That assert is pretty useless here, as the function is only used 
internally on s->osk. I'd just remove it. If you want to keep it, please 
move it below the variable declarations.


> +
> +    io_service_t hostsmc_service = IO_OBJECT_NULL;
> +    io_connect_t hostsmc_connect = IO_OBJECT_NULL;
> +    size_t smc_param_size = sizeof(struct AppleSMCParam);
> +    IOReturn status = kIOReturnError;
> +    int i;
> +
> +    struct AppleSMCParam smc_param[2] = {
> +         {
> +             .key = ('OSK0'),
> +             .data_size = sizeof(smc_param[0].bytes),
> +             .command = SMC_READ_KEY,
> +         }, {
> +             .key = ('OSK1'),
> +             .data_size = sizeof(smc_param[0].bytes),
> +             .command = SMC_READ_KEY,
> +         },
> +    };
> +
> +    hostsmc_service = IOServiceGetMatchingService(
> +        kIOMasterPortDefault,
> +        IOServiceMatching("AppleSMC"));
> +    if (hostsmc_service == IO_OBJECT_NULL) {
> +        error_setg(errp, "Unable to get host-AppleSMC service");
> +        goto error;
> +    }
> +
> +    status = IOServiceOpen(hostsmc_service,
> +                           mach_task_self(),
> +                           0,
> +                           &hostsmc_connect);
> +    if (status != kIOReturnSuccess || hostsmc_connect == IO_OBJECT_NULL) {
> +        error_setg(errp, "Unable to open host-AppleSMC service");
> +        goto error;
> +    }
> +
> +    for (i = 0; i < ARRAY_SIZE(smc_param); ++i) {
> +        status = IOConnectCallStructMethod(
> +                hostsmc_connect,
> +                SMC_HANDLE_EVENT,
> +                &smc_param[i],
> +                sizeof(struct AppleSMCParam),
> +                &smc_param[i],
> +                &smc_param_size


I think it's better to pull smc_param_size's declaration (or at least 
its initialization) into the loop. That way you know you always pass the 
correct size and IOConnectCallStructMethod() didn't modify it after the 
first call.


> +            );
> +
> +        if (status != kIOReturnSuccess) {
> +            error_setg(errp, "Unable to read OSK from host-AppleSMC");
> +            goto error;
> +        }
> +    }
> +
> +    memcpy(host_osk, smc_param[0].bytes, 32);
> +    memcpy(host_osk + 32, smc_param[1].bytes, 32);
> +
> +    IOServiceClose(hostsmc_connect);
> +    IOObjectRelease(hostsmc_service);
> +    return true;
> +
> +error:
> +    IOServiceClose(hostsmc_connect);
> +    IOObjectRelease(hostsmc_service);
> +    return false;
> +}
> +#else
> +static bool applesmc_read_host_osk(char *host_osk, Error **errp)
> +{
> +    error_setg(errp, "OSK read is not supported on this host");
> +    return false;
> +}
> +#endif
> +
>   static void applesmc_isa_realize(DeviceState *dev, Error **errp)
>   {
>       AppleSMCState *s = APPLE_SMC(dev);
> @@ -331,9 +432,26 @@ 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->osk || (strlen(s->osk) != 64)) {
> -        warn_report("Using AppleSMC with invalid key");
> +    if (s->osk && s->hostosk) {
> +        error_setg(errp, "-osk property cannot be used with -hostosk=on");


This isn't -hostosk, it's just "hostosk=on". I think it's also better to 
refer to osk as osk rather than -osk. -osk is the legacy command line 
syntax.


> +    }
> +
> +    if (!s->osk && !s->hostosk) {
>           s->osk = default_osk;
> +        warn_report("Using AppleSMC with default (dummy) OSK");
> +    }
> +
> +    if (s->hostosk) {
> +        s->osk = g_malloc0(65);
> +        /* Fail hard if we cannot read requested host OSK */
> +        if (!applesmc_read_host_osk(s->osk, errp)) {
> +            g_assert_not_reached();
> +        }
> +    }
> +
> +    if (s->osk && strlen(s->osk) != 64) {
> +        /* Only valid OSK is accepted within 'osk' property */
> +        error_setg(errp, "Using AppleSMC with key of invalid length");


Do you envision that the hostosk function will return an invalid key in 
the future? If not, I would just only do the strlen() check if we have 
s->osk set. That way, you can also limit your alloc to 64 bytes.


The rest looks great to me! I'll give v11 a spin on a few machines I 
have lying around when it comes :)


Thanks!

Alex


>       }
>   
>       QLIST_INIT(&s->data_def);
> @@ -344,6 +462,7 @@ static Property applesmc_isa_properties[] = {
>       DEFINE_PROP_UINT32(APPLESMC_PROP_IO_BASE, AppleSMCState, iobase,
>                          APPLESMC_DEFAULT_IOBASE),
>       DEFINE_PROP_STRING("osk", AppleSMCState, osk),
> +    DEFINE_PROP_BOOL("hostosk", AppleSMCState, hostosk, true),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>

On 01.05.22 19:06, Alexander Graf wrote:
> Hey Vladislav,
>
> On 29.04.22 21:18, Vladislav Yaroshchuk wrote:
>> On Apple hosts we can read AppleSMC OSK key directly from host's
>> SMC and forward this value to QEMU Guest.
>>
>> New 'hostosk' property is added:
>> * `-device isa-applesmc,hostosk=on`
>> The property is set to 'on' by default for machine version > 7.0
>>
>> Apple licence allows use and run up to two additional copies
>> or instances of macOS operating system 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>
>> ---
>>   hw/core/machine.c  |   4 +-
>>   hw/misc/applesmc.c | 125 +++++++++++++++++++++++++++++++++++++++++++--
>>   2 files changed, 125 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index cb9bbc844d..7f4a27406a 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -37,7 +37,9 @@
>>   #include "hw/virtio/virtio.h"
>>   #include "hw/virtio/virtio-pci.h"
>>   -GlobalProperty hw_compat_7_0[] = {};
>> +GlobalProperty hw_compat_7_0[] = {
>> +    { "isa-applesmc", "hostosk", "off" }
>> +};
>>   const size_t hw_compat_7_0_len = G_N_ELEMENTS(hw_compat_7_0);
>>     GlobalProperty hw_compat_6_2[] = {
>> diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
>> index 81cd6b6423..8672c9d56e 100644
>> --- a/hw/misc/applesmc.c
>> +++ b/hw/misc/applesmc.c
>> @@ -37,6 +37,11 @@
>>   #include "qemu/module.h"
>>   #include "qemu/timer.h"
>>   #include "qom/object.h"
>> +#include "qapi/error.h"
>> +
>> +#if defined(__APPLE__) && defined(__MACH__)
>> +#include <IOKit/IOKitLib.h>
>> +#endif
>>     /* #define DEBUG_SMC */
>>   @@ -80,7 +85,7 @@ enum {
>>   #define smc_debug(...) do { } while (0)
>>   #endif
>>   -static char default_osk[64] = "This is a dummy key. Enter the real 
>> key "
>> +static char default_osk[65] = "This is a dummy key. Enter the real 
>> key "
>
>
> This is only necessary if we run strlen() on the default_osk. Do we 
> have to?


Ugh, just after sending reply I realized why we need to: We make osk 
available as object property, which means anyone who would read it from 
outside would run out of bounds.

Can you please make the bit that bumps default_osk to 65 bytes a 
separate patch that we can apply immediately? It's a long standing, 
existing bug.


Thanks!

Alex