linux-kernel - Re: [PATCH v6 6/7] tpm: ensure that the output of PCR read contains the correct digest size

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: <20181205004643.GA26578@linux.intel.com>
Date:   Tue, 4 Dec 2018 16:46:43 -0800
From:   Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
To:     Roberto Sassu <roberto.sassu@...wei.com>
Cc:     zohar@...ux.ibm.com, david.safford@...com, monty.wiseman@...com,
        linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org, silviu.vlasceanu@...wei.com
Subject: Re: [PATCH v6 6/7] tpm: ensure that the output of PCR read contains
 the correct digest size

On Tue, Dec 04, 2018 at 04:09:10PM -0800, Jarkko Sakkinen wrote:
> On Tue, Dec 04, 2018 at 09:21:37AM +0100, Roberto Sassu wrote:
> >  	out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
> >  	digest_size = be16_to_cpu(out->digest_size);
> > -	if (digest_size > sizeof(digest->digest)) {
> > +	if (digest_size > sizeof(digest->digest) ||
> > +	    (!digest_size_ptr && digest_size != expected_digest_size)) {
> >  		rc = -EINVAL;
> >  		goto out;
> >  	}
> 
> Just noticed this but you must squash 4-6 because applying only
> previous commits will result a broken tree. It will be much bigger
> commit but won't be broken.
> 
> I think you should also feed min_rsp_body_length as you should be
> able to precalculate.
> 
> Last time I was asking why this isn't a bug fix. It is even for
> the existing code. The existing code should have a bug fix that
> checks that the received digest size so that it is the expected
> SHA1 size before we can apply this commit.

My bad. This is not the same deal as the code because in the old code we
always copy a constant block. Here we use the variable as parameter for
memcpy() so it is better to check the size. You can ignore the last
paragraph completely. Sorry, had to double check this one.

There is no need to do any type of bug fix for the current tree.

Still 4-6 need to be squashed in order to not put purposely the tree
into broken state.

/Jarko