On 10/09/2016 04:07 PM, Jarkko Sakkinen wrote: > On Sun, Oct 09, 2016 at 03:30:18PM +0530, Nayna wrote: >> >> >> On 10/09/2016 02:59 PM, Jarkko Sakkinen wrote: >>> On Sun, Oct 09, 2016 at 12:08:27PM +0300, Jarkko Sakkinen wrote: >>>> On Sat, Oct 08, 2016 at 10:15:55PM -0400, Nayna Jain wrote: >>>>> The existing in-kernel interface for extending a TPM PCR extends >>>>> the SHA1 PCR bank. For TPM 1.2, that is the one and only PCR bank >>>>> defined. TPM 2.0 adds support for multiple PCR banks, to support >>>>> different hash algorithms. The TPM 2.0 Specification[1] >>>>> recommends extending all active PCR banks. This patch set enhances >>>>> the existing TPM 2.0 extend function and corresponding in-kernel >>>>> interface to support extending all active PCR banks. >>>>> >>>>> The first patch implements the TPM 2.0 capability to retrieve >>>>> the list of active PCR banks. >>>>> >>>>> The second patch modifies the TPM 2.0 device driver extend function >>>>> to support extending multiple PCR banks. The existing in-kernel >>>>> interface expects only a SHA1 digest. Hence, to extend all active >>>>> PCR banks with differing digest sizes for TPM 2.0, the SHA1 digest >>>>> is padded with 0's as needed. >>>>> >>>>> This approach is taken to maintain backwards compatibility for the >>>>> existing users (i.e. IMA) in order to continue working with both >>>>> TPM 1.2 and TPM 2.0 without any changes and still comply with the >>>>> TPM 2.0 Specification[1] requirement of extending all active PCR >>>>> banks. >>>>> >>>>> This patch series has a prerequisite(header file tpm2.h) of TPM 2.0 >>>>> event log patch series. >>>> >>>> This is an unacceptable requirement. I don't even like the idea >>>> of having tpm2.h (rather would keep stuff in tpm2-cmd.c). >>>> >>>> Also I seriously cannot accept patch sets that add code without >>>> giving value. >>> >>> I would propose that you work on a PoC for IMA with TPM 2.0 that >>> includes these patches. Then we can try it out. Depending on half >>> finished patch sets is not just right way to do it. I'm happy to >>> test if you have someting runnable :) >> >> I actually created tpm2.h in eventlog patch series thinking just like tpm.h >> and tpm_eventlog.h is meant for TPM 1.2 specific structs, there can be >> tpm2.h specific to TPM 2.0 structs. It was just my thought to segregate the >> headers, but if it doesn't look good idea, I can change it to more >> recommended way. > > The structures that are in tpm2-cmd.c are there because they are and > should not be exposed to anywhere else. > > But this is essentially a meta-discussion. If you send a series it > should always apply to upstream. There was not commit that creates > tpm2.h. > >> Also, struct tpml_digest_values are used by both eventlog and extend >> function as shown below: >> >> struct tcg_pcr_event2 { >> u32 pcr_idx; >> u32 event_type; >> struct tpml_digest_values digests; >> struct tcg_event_field event; >> } __packed; >> >> /* Crypto agile extend format. */ >> struct tpm2_pcr_extend_in { >> __be32 pcr_idx; >> __be32 auth_area_size; >> struct tpm2_null_auth_area auth_area; >> struct tpml_digest_values digests; >> } __packed; >> >> So, I continued using tpm2.h for this patch series and created a >> pre-requisite on eventlog patch series. > > One thing that I would see useful would be to move TPM 1.x command > functions and headers to tpm1-cmd.c and enable conditional compilation > for TPM 1.x and TPM 2.0 protocols. > >> I have applied to upstream and tested on top of eventlog patch series, so >> yes, it doesn't apply directly to upstream without eventlog patches because >> of tpm2.h file. > > You should always try to send series in a form that applies to > upstream. Mistakes happen and that's OK but as general rule.... > >> If this doesn't look an acceptable approach, I would be happy to redo it in >> new way which is more acceptable. > > OK, here's what you could do (just a proposal): > > 1. Take the commits I posted and apply them to your upstream tree. > 2. Rewrite code that gets active PCR banks with tpm2_get_cap > 3. Rewrite PCR extend code with tpm_buf > 4. git format-patch --subject-prefix="PATCH RFC v2" > > Please carry the RFC tag if this is not something directly usable (user > visible functionality). I won't apply these before they are used but I'm > glad to help reviewing RFC-tagged series. > > Hope these help. >
Sure Jarkko. This is helpful. Thanks for reviewing and all your inputs. I will include your suggestions and post the next version. Thanks & Regards, - Nayna > /Jarkko > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel