Re: [tpmdd-devel] [PATCH 0/2] tpm: enhance TPM 2.0 extend function to support multiple PCR banks
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
Re: [tpmdd-devel] [PATCH 0/2] tpm: enhance TPM 2.0 extend function to support multiple PCR banks
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.
/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
Re: [tpmdd-devel] [PATCH 0/2] tpm: enhance TPM 2.0 extend function to support multiple PCR banks
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 :) /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
Re: [tpmdd-devel] [PATCH 0/2] tpm: enhance TPM 2.0 extend function to support multiple PCR banks
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. /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
[tpmdd-devel] [PATCH 0/2] tpm: enhance TPM 2.0 extend function to support multiple PCR banks
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. [1] TPM2.0 Specification referred here is "TCG PC Client Specific Platform Firmware Profile for TPM 2.0" Nayna Jain (2): tpm: implement TPM 2.0 capability to get active PCR banks tpm: enhance TPM 2.0 PCR extend to support multiple banks drivers/char/tpm/tpm-chip.c | 4 ++ drivers/char/tpm/tpm-interface.c | 17 - drivers/char/tpm/tpm.h | 8 ++- drivers/char/tpm/tpm2-cmd.c | 151 --- drivers/char/tpm/tpm2.h | 53 ++ 5 files changed, 205 insertions(+), 28 deletions(-) -- 2.5.0 -- 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
