Bug 1327537

Hi László,

should a domain using this option be marked as tainted, similar to how we taint domains using -dtb?

If I understand it correctly, a SLIC table should only contain OEM IDs, so tainting them should not be required. Can the -acpitable option can be used to provide tables that would taint the domain?

Thanks,
Ján

Good question!

I think that <qemu:commandline> should continue to taint the domain, in general. The point of this BZ is, IIUC, to provide the admin with a supportable way (a dedicated XML element) to pass his "slic.dat" to the guest.

Originally I thought "just let him pass in any kind of ACPI table he might want to". But, you are making a good point: "any kind of ACPI table" can easily break the guest, so maybe we shouldn't aim at blanket coverage.

It would be nice to find a middle way: generic syntax (so we can extend it to future, individual ACPI table types, if necessary), but limit the semantics to SLIC for the moment. How about (just brainstorming):

<acpitables>
  <acpitable type='SLIC'>pathname/to/slic.dat</acpitable>
  <acpitable type='generic'>...</acpitable>
<acpitables>

- generic syntax: check
- @type is "generic": domain gets tainted
- @type is "SLIC": domain does not get tainted
- @type is "SLIC" but the user passes in a non-SLIC ACPI table:
  - libvirt may or may not want to enforce a type match. Checking the signature
    of the ACPI table is very easy, if libvirt wants to do it. On the other
    hand, an argument can be made that this is not libvirt's business.
    I think if the user intentionally circumvents the tainting, we shouldn't
    try to catch it. After all, they can always point the <emulator> element
    to a wrapper shell script, and put whatever they want in it.

So, I guess: generic syntax with extensible semantics would be nice. SLIC shouldn't taint. Everything else (at this point) should, IMO.

(In reply to Laszlo Ersek from comment #2)
> <acpitables>
>   <acpitable type='SLIC'>pathname/to/slic.dat</acpitable>
>   <acpitable type='generic'>...</acpitable>
> <acpitables>
> 
> - generic syntax: check
> - @type is "generic": domain gets tainted
> - @type is "SLIC": domain does not get tainted
> - @type is "SLIC" but the user passes in a non-SLIC ACPI table:
>   - libvirt may or may not want to enforce a type match. Checking the
> signature
>     of the ACPI table is very easy, if libvirt wants to do it. On the other
>     hand, an argument can be made that this is not libvirt's business.
>     I think if the user intentionally circumvents the tainting, we shouldn't
>     try to catch it. After all, they can always point the <emulator> element
>     to a wrapper shell script, and put whatever they want in it.
> 
> So, I guess: generic syntax with extensible semantics would be nice. SLIC
> shouldn't taint. Everything else (at this point) should, IMO.

I don't think we should support --acpitable at all. Only developers would use this and the possibilities to break the VM are far greater than the gain of doing it. For thesting of course the command passthrough is okay.

On the other hand, passing the 'oem_id/oem_table_id' is the desired functionality here, so I think this should be discussed separately.

Passing the platform IDs does make sense but should be separated from doing it via an external file. If a user wishes to pass some custom IDs we should pass them on the commandline or via the file that will be managed by libvirt.

Allowing to use external files controlled by users rather than libvirt is another pain point in this case since they then can change across migration or suspend resume with a new fun matrix of possible outcomes.

TL;DR: Libvirt should not add --acpitable as a workaround to specify the SLIC oem_id fields.

(In reply to Peter Krempa from comment #3)

> On the other hand, passing the 'oem_id/oem_table_id' is the desired
> functionality here, so I think this should be discussed separately.
>
> Passing the platform IDs does make sense but should be separated from
> doing it via an external file. If a user wishes to pass some custom IDs we
> should pass them on the commandline or via the file that will be managed
> by libvirt.

Unfortunately, the title of the clone-origin bug 1248758 is not intuitive
enough. It says

    match the OEM ID and OEM Table ID fields of the FADT and the RSDT to
    those of the SLIC

but there's a lot more to it in the background, actually. Please let me
elaborate.

The SLIC ACPI table contains cryptographic (sensitive) material. The example
SLIC table that I attached (for testing purposes only) to bug 1248758
comment 42 only contains the fixed initial fields, required by the SLIC
specification from Microsoft.

If you look at the Microsoft spec:

  Microsoft Software Licensing Tables (SLIC and MSDM)
  http://go.microsoft.com/fwlink/p/?LinkId=234834

you will see that the actual meaning of the SLIC table is in the "Software
Licensing Structure" field, the one that starts at offset 36. The internal
structure of this ACPI table field is nowhere documented publicly, but it is
key to actually keep the OEM Windows activation work.

(You will notice that the SLIC spec says, about the "Software Licensing
Structure" field:

    Proprietary data structure that contains all the licensing data
    necessary to enable Windows activation. Details can be found in the
    appropriate Microsoft OEM licensing kit by first visiting the Microsoft
    OEM website (http://www.microsoft.com/oem/pages/index.aspx).

except the link doesn't work, and if you try to google the internals, you
will find nothing.)

The ACPICA suite (that provides the "acpidump" and "iasl" utilities) does
provide parsing routines for the internals of the SLIC table:

  https://github.com/acpica/acpica/commit/62230afb8f526

but it is unknown where that information comes from. It could be the result
of reverse engineering, or a private agreement between Intel and Microsoft,
or whatever else.

(You can test it too: if you boot a Fedora LiveCD on a physical machine that
was originally sold with OEM Windows on it, you can run

  acpidump -b
  iasl -d slic.dat
  cat slic.dsl

and you will see the SLIC table in all of its glory. Of course, you would
never share that file with anyone at all, as this SLIC table is specific to
the exact physical machine you are dumping it from.)

So, in order to move a physical OEM Windows installation into a guest, and
preserve its activation in working shape, two things are necessary:

(1) The VM must present the exact same SLIC table to the guest as the
    physical machine used to.

(2) The OEM ID and OEM Table ID fields between the SLIC, the FACP (a.k.a.
    FADT) and the RSDT must match.

    (This requirement emerges as a chain of sub-requirements. It originates
    from the SLIC spec (see the link above), which spells out the
    RSDT<->SLIC match, and is continued by the ACPI spec, which requires the
    RSDT<->FACP match in general. So that's how you end up with
    SLIC<->RSDT<->FACP.)

Now, an end-user can always satisfy the first condition: just dump the SLIC
on the physical machine, and pass it in to QEMU with the -acpitable switch.

The second condition is what the clone-origin bug 1248758 is about. The RSDT
and the FACP are generated by QEMU automatically (with their OEM fields
matching each other, as required by the ACPI spec in general). However, when
the user passes in a SLIC table manually, the match between the SLIC and
RSDT+FACP was not ensured, thereby breaking the SLIC spec from MS, and
consequently Windows activation in the guest as well.

Since the SLIC table from the user is unmodifiable, we can only adapt the
auto-generated RSDT and FACP to it; which is what the fix for bug 1248758
does (condition (2)). However, Windows activation in the guest still depends
on the *rest* of the table (condition (1)).

> Allowing to use external files controlled by users rather than libvirt is
> another pain point in this case since they then can change across
> migration or suspend resume with a new fun matrix of possible outcomes.

In this scenario, the user would be responsible for distributing (and for
keeping confidential) his "slic.dat" files. Please see bug 1248758 comment
36.

> TL;DR: Libvirt should not add --acpitable as a workaround to specify the
> SLIC oem_id fields.

As I described above, setting the OEM ID and OEM Table ID fields in the
auto-generated RSDT and FACP tables to user-specified values doesn't
suffice. That would only satisfy condition (2). For Windows activation to
continue working in the guest, condition (1) is also necessary -- the exact
same SLIC table must be presented to the guest.

For this,

- libvirt can either allow the user to dump the SLIC on the physical host
  (with acpidump), and to pass the dumped table to QEMU as a file,

- or libvirt can require the user to disassemble the dumped SLIC table,
  manually converting it to some (to be designed) XML fragment -- note again
  that the SLIC internals are not publicly specified. Then libvirt would
  synthesize a temporary "slic.dat" file from the XML, and pass that to
  QEMU.

- A third option would be to ask the user to paste the SLIC table, encoded
  as a multi-line base64 string, into a dedicated domain XML element.
  Libvirtd could decode this and pass the result to QEMU, as a temporary
  regular file. (I believe libvirt could also pass the data through an
  anonymous pipe, using "-acpitable file=/dev/fd/NNN".)

I don't know if option #3 would satisfy confidentiality requirements (I'm
uncertain if the domain XML is supposed to contain secrets), but it would
take care of the migration & external file issues.

(In reply to Laszlo Ersek from comment #2)
> Good question!
> 
> I think that <qemu:commandline> should continue to taint the domain, in
> general. The point of this BZ is, IIUC, to provide the admin with a
> supportable way (a dedicated XML element) to pass his "slic.dat" to the
> guest.
> 
> Originally I thought "just let him pass in any kind of ACPI table he might
> want to". But, you are making a good point: "any kind of ACPI table" can
> easily break the guest, so maybe we shouldn't aim at blanket coverage.
> 
> It would be nice to find a middle way: generic syntax (so we can extend it
> to future, individual ACPI table types, if necessary), but limit the
> semantics to SLIC for the moment. How about (just brainstorming):
> 
> <acpitables>
>   <acpitable type='SLIC'>pathname/to/slic.dat</acpitable>
>   <acpitable type='generic'>...</acpitable>
> <acpitables>
> 
> - generic syntax: check
> - @type is "generic": domain gets tainted
> - @type is "SLIC": domain does not get tainted
> - @type is "SLIC" but the user passes in a non-SLIC ACPI table:
>   - libvirt may or may not want to enforce a type match. Checking the
> signature
>     of the ACPI table is very easy, if libvirt wants to do it. On the other
>     hand, an argument can be made that this is not libvirt's business.

Yes, libvirt definitely does not want to get in the business of identifying blobs. Can this be done by QEMU? Something like:
-acpitable slic=pathname/to/slic.dat

>     I think if the user intentionally circumvents the tainting, we shouldn't
>     try to catch it. After all, they can always point the <emulator> element
>     to a wrapper shell script, and put whatever they want in it.
> 
> So, I guess: generic syntax with extensible semantics would be nice. SLIC
> shouldn't taint. Everything else (at this point) should, IMO.

The problem is libvirt would have to probe the file to tell between the two cases and it cannot both taint and not taint the domain at the same time.

(In reply to Ján Tomko from comment #5)
> (In reply to Laszlo Ersek from comment #2)

> > - @type is "SLIC" but the user passes in a non-SLIC ACPI table:
> >   - libvirt may or may not want to enforce a type match. Checking the
> >     signature of the ACPI table is very easy, if libvirt wants to do it.
> >     On the other hand, an argument can be made that this is not
> >     libvirt's business.
>
> Yes, libvirt definitely does not want to get in the business of
> identifying blobs. Can this be done by QEMU? Something like: -acpitable
> slic=pathname/to/slic.dat

No, QEMU only detects whether the table has SLIC signature for the purpose
of OEM matching; it doesn't try to validate what a user-provided ACPI table
"should be" (according to another parameter that originates from the exact
same user).

> >     I think if the user intentionally circumvents the tainting, we
> >     shouldn't try to catch it. After all, they can always point the
> >     <emulator> element to a wrapper shell script, and put whatever they
> >     want in it.
> >
> > So, I guess: generic syntax with extensible semantics would be nice.
> > SLIC shouldn't taint. Everything else (at this point) should, IMO.
>
> The problem is libvirt would have to probe the file to tell between the
> two cases and it cannot both taint and not taint the domain at the same
> time.

I agree.

My point is that we shouldn't try to determine the exact case when to taint
the domain. As I said, the user can replace the emulator binary with
whatever he wants (a QEMU wrapper shell script, for example, that appends
various command line options), and libvirt will not taint the domain just
because of that either. If a user really wants to circumvent the tainting,
he definitely will. He can even recompile QEMU and put it under the original
binary's pathname, and we'll be none the wiser, just from looking at the
domain XML and the libvirtd log.

So, I'd suggest to provide an element that clearly communicates that its
purpose is passing in the SLIC, and if the user circumvents it (passes in a
non-SLIC table with it), that's his problem.

The goal of tainting is to heuristically identify cases where it's
reasonable for us to deny support for a use case. Since the entire tainting
logic executes at the user's side, it is impossible to enforce 100%. The
user can doctor the logs before sending them to us, doctor the domain XML,
doctor QEMU, and so on. What we assume is that a user does not intend to lie
to us; with the tainting we just want to catch honest mistakes. To my
knowledge anyway.

If the user inadvertently passes in a non-SLIC table with the SLIC element,
that can be determined on our side in the following way:

- analyze the domain XML, notice the SLIC element,
- ask for the first 36 bytes (at most) of "slic.dat",
- check it with "iasl -d".

------*------

Actually, there might be one way for libvirt to cut this Gordian Knot.
Namely, the -acpitable switch provides some sub-options that *overwrite*
data in the ACPI table, after it is read from the file:

       -acpitable [sig=str][,rev=n][,oem_id=str][,oem_table_id=str]
       [,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n]
       [,data=file1[:file2]...]
           Add ACPI table with specified header fields and context from
           specified files.  For file=, take whole ACPI table from the
           specified files, including all ACPI headers (possible overridden
           by other options). For data=, only data portion of the table is
           used, all header information is specified in the command line.
           If a SLIC table is supplied to QEMU, then the SLIC's oem_id and
           oem_table_id fields will override the same in the RSDT and the
           FADT (a.k.a. FACP), in order to ensure the field matches required
           by the Microsoft SLIC spec and the ACPI spec.

So libvirt could pass

  -acpitable,sig=SLIC,file=/full/path/to/slic.dat

In this case, if the user passes in a genuine SLIC table, then nothing will
change, the original SLIC signature will be replaced with SLIC, which has no
effect. Otherwise, a different signature will be replaced with SLIC, with
the following consequences, as far as the guest OS is concerned:

- The ACPI table checksum will likely not match.

- Even if the ACPI table checksum happens to match, the OS will attempt to
  interpret the crypto stuff in the rest of the table as appropriate for
  SLIC, and it will likely be malformed, hence rejected.

- Even if it happens to parse as a well-formed SLIC table, since the content
  is meant to be cryptographically secure, it will *definitely* be rejected
  (it will appear to be tampered-with).

Proposed upstream patches:
https://www.redhat.com/archives/libvir-list/2016-May/msg00972.html

<os>
  <acpi table="slic">/path/to/acpi/table/file</acpi>
</os>

will result in:

-acpitable sig=SLIC,file=/path/to/acpi/table/file

v2 using a slightly different XML format:
<os>
  <acpi>
    <table type="slic">/path/to/acpi/table/file</table>
  </acpi>
</os>
https://www.redhat.com/archives/libvir-list/2016-May/msg01730.html

Pushed upstream as:
commit 72f652da63255c7f1a9914625cce617dde9128d0
Author:     Ján Tomko <jtomko>
CommitDate: 2016-05-25 17:15:21 +0200

    conf: add <acpi><table> to <os>
    
    Add a new element to <domain> XML:
    <os>
      <acpi>
        <table type="slic">/path/to/acpi/table/file</table>
      </acpi>
    </os>
    
    To supply a path to a SLIC (Software Licensing) ACPI
    table blob.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1327537

commit ea04d1a659209416b191c6b4b5803f4db3b69d81
Author:     Ján Tomko <jtomko>
CommitDate: 2016-05-25 17:15:21 +0200

    qemu: format SLIC ACPI table command line
    
    <os>
      <acpi>
        <table type="slic">/path/to/acpi/table/file</table>
      </acpi>
    </os>
    
    will result in:
    
    -acpitable sig=SLIC,file=/path/to/acpi/table/file
    
    This option was introduced by QEMU commit 8a92ea2 in 2009.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1327537

commit 5da23bbedf66a028b6bff6bcd5f3f453b4bbb3a9
Author:     Ján Tomko <jtomko>
CommitDate: 2016-05-25 17:15:21 +0200

    security: label the slic_table
    
    Add support for the slic_table to the security drivers.

git describe: v1.3.4-493-g5da23bb

verify the bug

version:
libvirt-1.3.5-1.el7.x86_64
qemu-kvm-rhev-2.6.0-7.el7.x86_64
kernel-3.10.0-443.el7.x86_64
 
steps:
1.in the host,look at the slic info
[root@localhost ~]# hexdump /root/slic.dat  -C
00000000  53 4c 49 43 24 00 00 00  01 3a 41 42 52 41 43 41  |SLIC$....:ABRACA|
00000010  44 41 42 52 41 58 59 5a  04 03 02 01 51 55 55 58  |DABRAXYZ....QUUX|
00000020  08 07 06 05                                       |....|
00000024

2.start a guest with the xml:
...
 <os>
    <type arch='x86_64' machine='pc-i440fx-rhel7.2.0'>hvm</type>
    <acpi>
      <table type='slic'>/root/slic.dat</table>
    </acpi>
    <boot dev='hd'/>
  </os>
...

3.In the guest,run "acpidump"
#acpidump
...
SLIC @ 0x0000000000000000
  0000: 53 4C 49 43 24 00 00 00 01 3A 41 42 52 41 43 41  SLIC$....:ABRACA
  0010: 44 41 42 52 41 58 59 5A 04 03 02 01 51 55 55 58  DABRAXYZ....QUUX
  0020: 08 07 06 05                                      ....

s/PATH/PATHNAME in the doc text -- Jirka (Herrmann), can you please review it? Thanks.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHSA-2016-2577.html