Re: [PATCH] schemas: Add Google Widevine initialization parameters

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



On Fri, Nov 10, 2023 at 10:37 PM Rob Herring <robh@xxxxxxxxxx> wrote:
>
> On Fri, Nov 10, 2023 at 1:51 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >
> > On 10/11/2023 05:32, Yi Chou wrote:
> > > The necessary fields to initialize the widevine related functions in
> > > OP-TEE.
> > >
> > > Signed-off-by: Yi Chou <yich@xxxxxxxxxxxx>
> > > Reviewed-by: Simon Glass <sjg@xxxxxxxxxxxx>
> > >
> > > ---
> >
> > Please provide version log and proper background for this patch. This
> > was already sent to Linux kernel, so we should not spend time to figure
> > it out.
> >

Sent out a new patch that has the version log and background for this patch.
https://lore.kernel.org/all/20231113075249.3807225-1-yich@xxxxxxxxxxxx

> > > There's no plan to add more optee stuff now.
> > > This is it.
> > >
> > >  dtschema/schemas/options/op-tee/widevine.yaml | 73 +++++++++++++++++++
> > >  1 file changed, 73 insertions(+)
> > >  create mode 100644 dtschema/schemas/options/op-tee/widevine.yaml
> > >
> > > diff --git a/dtschema/schemas/options/op-tee/widevine.yaml b/dtschema/schemas/options/op-tee/widevine.yaml
> > > new file mode 100644
> > > index 0000000..925c214
> > > --- /dev/null
> > > +++ b/dtschema/schemas/options/op-tee/widevine.yaml
> >
> > Missing vendor prefix - google,
> >
> > > @@ -0,0 +1,73 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/options/op-tee/widevine.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Google Widevine initialization parameters
> >
> > What is Widevine? You write it once "widevine", once "Widevine" but
> > never explain what it is.
> >
> > > +
> > > +maintainers:
> > > +  - Jeffrey Kardatzke <jkardatzke@xxxxxxxxxxxx>
> > > +  - Yi Chou <yich@xxxxxxxxxxxx>
> > > +
> > > +description:
> > > +  The necessary fields to initialize the widevine related functions in
> > > +  OP-TEE. This node does not represent a real device, but serves as a
> > > +  place for passing data between firmware and OP-TEE.
> > > +
> > > +properties:
> > > +  google,optee-hardware-unique-key:
> > > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > +    maxItems: 32
> > > +    description: |
> > > +      The hardware-unique key of the Widevine OP-TEE. It will be used
> > > +      to derive the secure storage key.
> > > +      For more information, please reference:
> > > +      https://optee.readthedocs.io/en/latest/architecture/porting_guidelines.html#hardware-unique-key
>
> Reading this, it says: "in the best case the HUK should never ever be
> readable directly from software, not even from the secure side"
>
> Putting it in DT is certainly "readable directly from software".
>
> Also, wouldn't widevine be calling tee_otp_get_hw_unique_key(...)? So
> if you do put this in DT, it should be consumed by optee rather than
> widevine?
>
> I'd like to see an ack from Jens or other optee maintainer on this
> before merging.

Yes, it will be consumed by the OP-TEE.
The OP-TEE side PR is this: https://github.com/OP-TEE/optee_os/pull/6379
There is a new flag in OP-TEE called "CFG_WIDEVINE_HUK", and it will
let the OP-TEE consume the HUK in DT.

And in our use case, it is not "readable directly from software",
because the DT will be generated at the runtime in TF-A, and we will
only pass it into the OP-TEE.
https://github.com/OP-TEE/optee_os/pull/6379#discussion_r1368844405

> >
> > Blank line
> >
> > > +  google,tcg-tpm-auth-public-key:
> >
> > Missing tcg prefix as asked by Rob.
> >
> > > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > +    maxItems: 1024
> > > +    description: |
> > > +      The TPM auth public key. Used to communicate the TPM from OP-TEE.
> > > +      The format of data should be TPM2B_PUBLIC.
> > > +      For more information, please reference the 12.2.5 section:
> > > +      https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p59_Part2_Structures_pub.pdf
> >
> > Blank line
> >
> >
> > > +  google,widevine-root-of-trust-ecc-p256:
> > > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > +    maxItems: 32
> > > +    description: |
> > > +      The Widevine root of trust secret. Used to sign the widevine
> > > +      request in OP-TEE. The value is an ECC NIST P-256 scalar.
> > > +      For more information, please reference the G.1.2 section:
> > > +      https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-186.pdf
> >
> > Blank line
> >
> >
> > > +required:
> > > +  - google,optee-hardware-unique-key
> > > +  - google,tcg-tpm-auth-public-key
> > > +  - google,widevine-root-of-trust-ecc-p256
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    options {
> > > +      widevine {
> >
> > Node names should be generic. See also an explanation and list of
> > examples (not exhaustive) in DT specification:
> > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
> Yes, but 'options' nodes are for software components and named for the
> component. So I think 'widevine' is okay here.
>
> However, I think the schema path/filename should match the DT path. So
> either need to add 'optee' to the DT path or remove it from the schema
> path. It's really up to the optee community to decide. They are the
> ones who will be stuck with whatever is created here.
>

The previous discussion in the optee:
https://github.com/OP-TEE/optee_os/pull/6418#pullrequestreview-1714399006
I believe they prefer the "/options/op-tee/widevine" path.

> Rob

Thanks,
Yi





[Index of Archives]     [Device Tree]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Photos]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]

  Powered by Linux