linux-kernel - Re: [PATCH v2 2/5] tsm: Introduce a shared ABI for attestation reports

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <64da54713e2fb_2138e294d@dwillia2-xfh.jf.intel.com.notmuch>
Date:   Mon, 14 Aug 2023 09:21:05 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Jeremi Piotrowski <jpiotrowski@...ux.microsoft.com>,
        Dan Williams <dan.j.williams@...el.com>,
        <linux-coco@...ts.linux.dev>
CC:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Dionna Amalie Glaze <dionnaglaze@...gle.com>,
        James Bottomley <James.Bottomley@...senpartnership.com>,
        Peter Gonda <pgonda@...gle.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Samuel Ortiz <sameo@...osinc.com>, <peterz@...radead.org>,
        <x86@...nel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/5] tsm: Introduce a shared ABI for attestation
 reports

Jeremi Piotrowski wrote:
> On 8/14/2023 9:43 AM, Dan Williams wrote:
> > One of the common operations of a TSM (Trusted Security Module) is to
> > provide a way for a TVM (confidential computing guest execution
> > environment) to take a measurement of its launch state, sign it and
> > submit it to a verifying party. Upon successful attestation that
> > verifies the integrity of the TVM additional secrets may be deployed.
> > The concept is common across TSMs, but the implementations are
> > unfortunately vendor specific. While the industry grapples with a common
> > definition of this attestation format [1], Linux need not make this
> > problem worse by defining a new ABI per TSM that wants to perform a
> > similar operation. The current momentum has been to invent new ioctl-ABI
> > per TSM per function which at best is an abdication of the kernel's
> > responsibility to make common infrastructure concepts share common ABI.
> > 
> > The proposal, targeted to conceptually work with TDX, SEV, COVE if not
> > more, is to define a sysfs interface to retrieve the TSM-specific blob.
> > 
> >     echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
> >     hexdump /sys/class/tsm/tsm0/outblob
> > 
> > This approach later allows for the standardization of the attestation
> > blob format without needing to change the Linux ABI. Until then, the
> > format of 'outblob' is determined by the parent device for 'tsm0'.
> > 
> > The expectation is that this is a boot time exchange that need not be
> > regenerated, making it amenable to a sysfs interface. In case userspace
> > does try to generate multiple attestation reports it includes conflict
> > detection so userspace can be sure no other thread changed the
> > parameters from its last configuration step to the blob retrieval.
> > 
> > TSM specific options are encoded as 'extra' attributes on the TSM device
> > with the expectation that vendors reuse the same options for similar
> > concepts. The current options are defined by SEV-SNP's need for a
> > 'privilege level' concept (VMPL), and the option to retrieve a
> > certificate chain in addition to the attestation report ("extended"
> > format).
> > 
> > Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> > Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
> > Cc: Dionna Amalie Glaze <dionnaglaze@...gle.com>
> > Cc: James Bottomley <James.Bottomley@...senPartnership.com>
> > Cc: Peter Gonda <pgonda@...gle.com>
> > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > Cc: Samuel Ortiz <sameo@...osinc.com>
> > Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> > ---
[..]
> > +static ssize_t outblob_read(struct file *f, struct kobject *kobj,
> > +			    struct bin_attribute *bin_attr, char *buf,
> > +			    loff_t offset, size_t count)
> > +{
> > +	guard(rwsem_read)(&tsm_rwsem);
> 
> This is unfortunate but it would need to be a rwsem_write otherwise two
> processes can race to reach the kvfree and both call report_new at the
> same time (unlikely as it may be).

Ugh, yup, good eye, will fix.