linux-kernel - Re: [PATCH v3 2/5] configfs-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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <64f231b8ddde2_4c1f32941f@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Date:   Fri, 1 Sep 2023 11:47:20 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Dan Williams <dan.j.williams@...el.com>,
        Dionna Amalie Glaze <dionnaglaze@...gle.com>
CC:     <linux-coco@...ts.linux.dev>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.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>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/5] configfs-tsm: Introduce a shared ABI for
 attestation reports

Thomas Gleixner wrote:
> On Thu, Aug 31 2023 at 15:13, Dan Williams wrote:
> > Dionna Amalie Glaze wrote:
> >> This is clean and approachable. Thanks for your implementation.
> >> 
> >> > +static int try_advance_write_generation(struct tsm_report *report)
> >> > +{
> >> > +       lockdep_assert_held_write(&tsm_rwsem);
> >> > +
> >> > +       /*
> >> > +        * malicious or broken userspace is attempting to wrap read_generation,
> >> > +        * stop accepting updates until current report configuration is read.
> >> > +        */
> >> > +       if (report->write_generation == report->read_generation - 1)
> >> > +               return -EBUSY;
> >> > +       report->write_generation++;
> >> > +       return 0;
> >> > +}
> >> > +
> >> 
> >> This took me a while to wrap my head around.
> >> The property we want is that when we read outblob, it is the result of
> >> the attribute changes since the last read. If we write to an attribute
> >> 2^64 times, we could get write_generation to wrap around to equal
> >> read_generation without actually reading outblob. So we could end up
> >> given a badly cached result when we read outblob? Is that what this is
> >> preventing?
> >
> > Correct. The criticism of kernfs based interfaces like sysfs and
> > configfs is that there is no facility to atomically modify a set of
> > attributes at once. The expectated mitigation is simply that userspace
> > is well behaved. For example, 2 invocations of fdisk can confuse each
> > other, so userspace is expected to run them serially and the kernel
> > otherwise lets userspace do silly things.
> >
> > If a tool has any concern that it has exclusive ownership of the
> > interface this 'generation' attribute allows for a flow like:
> >
> > gen=$(cat $report/generation)
> > dd if=userdata > $report/inblob
> > cat $report/outblob > report
> > gen2=$(cat $report/generation)
> >
> > ...and if $gen2 is not $((gen + 1)) then tooling can detect that the
> > "userspace is well behaved" expectation was violated.
> >
> > Now configs is slightly better in this regard because objects can be
> > atomically created. So if 2 threads happen to pick the same name for the
> > report object then 'mkdir' will only succeed for one while the other
> > gets an EEXIST error. So for containers each can get their own
> > submission interface without worrying about other containers.
> >
> >> I think I would word this to say,
> >> 
> >> "Malicious or broken userspace has written enough times for
> >> read_generation == write_generation by modular arithmetic without an
> >> interim read. Stop accepting updates until the current report
> >> configuration is read."
> >
> > That works for me.
> 
> That's a pretty theoretical problem. Under the assumption that one
> syscall takes 50ns the wraparound on a 64bit variable will take ~29247
> years to complete.
> 
> I think the important point is that the generation check there ensures
> that the expected sequence takes place and can't be overwritten by
> accident or malice, no?

Exactly. The "attack" / "bug" is hard to carry out, so this is more for
theoretical completeness than practical protection.