Re: [PATCH 04/15] device core: Introduce "device evidence" over netlink

[Donald Hunter <donald.hunter@gmail.com>]

Dan Williams <djbw@kernel.org> writes:

> Multiple device security mechanisms are built on top of the SPDM protocol
> from the DMTF. The protocol arranges for devices to verify their identity
> and establish secure sessions between requesters and responders. In turn,
> device security protocols like PCIe IDE use SPDM sessions to program link
> encryption keys, and protocols like PCIe TDISP use exclusive secure
> sessions for managing the device's security state.
>
> All of these uses share a need to retrieve the device's installed
> certificates, its measurements, and other protocol specific objects like
> interface reports. Unify all of those object retrieval and regeneration
> cases into a device-scoped ABI that buses can leverage.
>
> When PCI CMA was presented at Linux Plumbers the consensus reaction was
> that it was not suitable to be driven via sysfs. Given the varied use
> cases of the same fundamental objects the consensus converged on
> netlink. With the new 'blob' extensions to netlink, it can support up to
> the 16MB signed transcripts that SPDM generates.
>
> A bus opts in to evidence gathering via device_evidence_register() and
> passing device_evidence_ops. The ops resolve a device handle to its
> corresponding 'struct device_evidence' context.
>
> See inline documentation in
> Documentation/netlink/specs/device-evidence.yaml for the object types and
> options. This includes objects like TDISP interface reports and options
> like digests instead of full blobs.

Can you include sample pyynl output; it helps immensely during review.

>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Donald Hunter <donald.hunter@gmail.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Xu Yilun <yilun.xu@linux.intel.com>
> Cc: "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>

You probably need to cc the netdev list so that the series gets picked
up by patchwork.

> ---
>  drivers/base/Kconfig                          |   4 +
>  drivers/base/Makefile                         |   1 +
>  .../netlink/specs/device-evidence.yaml        | 186 +++++++
>  drivers/base/device-evidence-netlink.h        |  23 +
>  include/linux/device/evidence.h               |  85 ++++
>  include/uapi/linux/device-evidence.h          | 110 +++++
>  drivers/base/device-evidence-netlink.c        |  44 ++
>  drivers/base/evidence.c                       | 453 ++++++++++++++++++
>  MAINTAINERS                                   |   4 +
>  9 files changed, 910 insertions(+)
>  create mode 100644 Documentation/netlink/specs/device-evidence.yaml
>  create mode 100644 drivers/base/device-evidence-netlink.h
>  create mode 100644 include/linux/device/evidence.h
>  create mode 100644 include/uapi/linux/device-evidence.h
>  create mode 100644 drivers/base/device-evidence-netlink.c
>  create mode 100644 drivers/base/evidence.c
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index f7d385cbd3ba..e3929fe6b240 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -190,6 +190,10 @@ config HMEM_REPORTING
>  	  Enable reporting for heterogeneous memory access attributes under
>  	  their non-uniform memory nodes.
>  
> +config DEVICE_EVIDENCE
> +	bool
> +	depends on NET
> +
>  source "drivers/base/test/Kconfig"
>  
>  config SYS_HYPERVISOR
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 8074a10183dc..02bdc4f74019 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
>  obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o
>  obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
>  obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
> +obj-$(CONFIG_DEVICE_EVIDENCE) += evidence.o device-evidence-netlink.o
>  obj-$(CONFIG_ACPI) += physical_location.o
>  
>  obj-y			+= test/
> diff --git a/Documentation/netlink/specs/device-evidence.yaml b/Documentation/netlink/specs/device-evidence.yaml
> new file mode 100644
> index 000000000000..44b278bc20e1
> --- /dev/null
> +++ b/Documentation/netlink/specs/device-evidence.yaml
> @@ -0,0 +1,186 @@
> +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
> +#
> +---
> +name: device-evidence

The naming seems to be too narrow - if the scope expands at all then the
name would need to change. A subsystem name like spdm, tsm or tsm-device
seems better?

> +protocol: genetlink
> +uapi-header: linux/device-evidence.h
> +doc: Device evidence retrieval over generic netlink
> +
> +definitions:
> +  -
> +    type: const
> +    name: max-object-size
> +    value: 0x01000000
> +  -
> +    type: const
> +    name: max-nonce-size
> +    value: 32
> +  -
> +    name: type
> +    type: enum
> +    doc: Device security evidence objects
> +    render-max: true
> +    entries:
> +      -
> +        name: cert0
> +        doc: SPDM certificate chain from device slot0
> +      -
> +        name: cert1
> +        doc: SPDM certificate chain from device slot1
> +      -
> +        name: cert2
> +        doc: SPDM certificate chain from device slot2
> +      -
> +        name: cert3
> +        doc: SPDM certificate chain from device slot3
> +      -
> +        name: cert4
> +        doc: SPDM certificate chain from device slot4
> +      -
> +        name: cert5
> +        doc: SPDM certificate chain from device slot5
> +      -
> +        name: cert6
> +        doc: SPDM certificate chain from device slot6
> +      -
> +        name: cert7
> +        doc: SPDM certificate chain from device slot7
> +      -
> +        name: vca
> +        doc: |
> +          SPDM version, capabilities, and algorithms transcript
> +          negotiated at session establishment. An implementation may not
> +          provide this separately and instead include it in the
> +          measurements transcript.
> +      -
> +        name: measurements
> +        doc: SPDM GET_MEASUREMENTS response
> +      -
> +        name: report
> +        doc: |
> +          A bus that implements a device interface security protocol
> +          like TDISP may convey an interface report that details
> +          interface settings and capabilities.
> +  -
> +    name: type-flag
> +    type: flags
> +    doc: Device security evidence request flags
> +    render-max: true
> +    # NOTE! these values must match the type enum name and order
> +    entries:
> +      -
> +        name: cert0
> +      -
> +        name: cert1
> +      -
> +        name: cert2
> +      -
> +        name: cert3
> +      -
> +        name: cert4
> +      -
> +        name: cert5
> +      -
> +        name: cert6
> +      -
> +        name: cert7
> +      -
> +        name: vca
> +      -
> +        name: measurements
> +      -
> +        name: report
> +  -
> +    name: flag
> +    type: flags
> +    render-max: true
> +    doc: Flags to control evidence retrieval
> +    entries:
> +      -
> +        name: digest
> +        doc: |
> +          Request a secure hash of objects like vca and measurements.
> +          The expectation is that this digest is produced by a responder
> +          within the TCB like a platform TSM. It validates a blob that
> +          may have traversed a transport without integrity protections.
> +
> +attribute-sets:
> +  -
> +    name: object
> +    attributes:
> +      -
> +        name: type
> +        type: u32

Is this an instance of the type enum? If so then:

           enum: type

> +        doc: evidence type-id
> +      -
> +        name: type-mask
> +        type: u32

Likewise:
           enum: type-flag

> +        doc: evidence types as a flag mask
> +      -
> +        name: flags
> +        type: u32

And:
           enum: flags

> +        doc: evidence modifier flags like 'request object digest'
> +      -
> +        name: subsys
> +        type: string
> +        doc: device subsystem name (e.g. bus or class name)
> +      -
> +        name: dev-name
> +        type: string
> +        doc: device name
> +      -
> +        name: nonce
> +        type: binary
> +        checks:
> +          max-len: max-nonce-size
> +      -
> +        name: generation
> +        type: u32
> +        doc: detect local threads racing evidence refresh
> +      -
> +        name: count
> +        type: u64
> +        doc: TSM, if present, observed count of evidence refresh events
> +      -
> +        name: length
> +        type: u32
> +        checks:
> +          max: max-object-size
> +        doc: |
> +          Final size of 'val' blob after all messages received.
> +      -
> +        name: val

Any reason for the abbreviated name?

> +        type: binary
> +        multi-attr: true
> +        bloblen: length
> +        doc: |
> +          Ordered evidence object value chunk. Large evidence objects
> +          may be split over several dump reply messages up to 'length'.
> +          'length' is transmitted before the first chunk.
> +
> +operations:
> +  list:
> +    -
> +      name: read
> +      doc: |
> +        Read device evidence objects of a given type mask. The dump reply
> +        reports the total value length before val attributes fill the evidence
> +        object, possibly across multiple messages.
> +      attribute-set: object
> +      flags: [admin-perm]
> +      dump:
> +        pre: device-evidence-nl-read-pre
> +        post: device-evidence-nl-read-post
> +        request:
> +          attributes:
> +            - type-mask
> +            - flags
> +            - subsys
> +            - dev-name
> +            - nonce
> +        reply:
> +          attributes:
> +            - type
> +            - generation
> +            - length
> +            - val

Is the intention that the dump filter must narrow to a single device? Is
the user expected to enumerate the devices in subsystem then read device
evidence for each in turn?