Have some question about this command requirement here.

Andrey,

1. I read the 2.0 spec. They define this command that should work under owner authorization and platform authorization. Do we need to support it under the platform hierarchy? or owner authorization is enough.

2. What's the use case of tpmc? Given we already have tpm_manager_client that can destroy a space with owner authorization.

3. Do we need to support for TPM 1.2? I means write some code that make `tpmc undef` is work when it is TPM1.2 build.
If the answer is yes, then how to setup a machine with TPM1.2 then I test it?

> 1. I read the 2.0 spec. They define this command that should work under owner authorization and platform authorization. Do we need to support it under the platform hierarchy? or owner authorization is enough.

Both should be supported.
We can start with platform authorization since that's the only actively requested use case for now: do it for OOBE-auto-config space defined with PLATFORMCREATE set.
But the next step is using it in places like clobber_lockbox_space() in platform/initramfs/recovery/recovery_init.sh to finally support TPM 2.0 case there. And, for example, that lockbox space doesn't have PLATFORMCREATE set.

> 2. What's the use case of tpmc? Given we already have tpm_manager_client that can destroy a space with owner authorization.

tpmc is used in workflows, where tcsd/trunksd (and all the dependent daemons like tpm_managerd) don't run: (a) in the scripts early in boot sequence, before tcsd/trunksd are started, (b) in recovery and factory scripts.

> 3. Do we need to support for TPM 1.2? I means write some code that make `tpmc undef` is work when it is TPM1.2 build.

Yes, we need to support TPM 1.2. There tpmc undef is just an equivalent of doing tpmc def with size 0 (not so for TPM 2.0). But it still makes sense to do it (implement Undefine) at Tlcl level (not at tpmc utility level) to have the same API as in 2.0 case. Inside Tlcl just call Define with size 0 from Undefine.

> If the answer is yes, then how to setup a machine with TPM1.2 then I test it?

You need a device that has TPM 1.2 onboard instead of H1 to test.

Forgot to mention that I already implemented 'tpm undef' in TPM2.0. I reference the implementation of the define_space() so it should support both owner auth and platform auth. It is tested with owner auth for now. Here's the CL: http://crrev.com/c/1291131

Perhaps you can start the review if you have the time, and I will be working on testing for platform hierarchy and on TPM1.2 parallelly. (or is it better to have separate CL for TPM1.2?)

For the platform hierarchy, how to setup a environment that enable platform hierarchy and test tpmc? I tried the recovery mode and it seems the only thing I can do is inserting a recovery image. I also asked Louis but he also doesn't know how to do that.
Can you provide a brief step about this? or point me some document to reference.

I set up a TPM1.2 device today. But keep failing on executing the `tpmc def` to setup a test space. I am not familiar with the permission design of TPM1.2. Which state I can execute the `tpmc def`?


Here's some execution result:
localhost ~ # tpmc def 0x1050 0x1 0x1
command "def" failed with code 0x2d
TPM_BAD_PRESENCE
Either the physicalPresence or physicalPresenceLock bits
have the wrong value

I tried in either unowned and pre-own cases. The error message seems like I have to be in PP state to define a space, but it seems it is locked. Also, the tlcl doesn't use any owner authorization in TlclDefineSpace.


More TPM status information:
Unowned:
localhost ~ # tpmc getpf
disable 0
ownership 1
deactivated 0
readPubek 1
disableOwnerClear 0
allowMaintenance 0
physicalPresenceLifetimeLock 1
physicalPresenceHWEnable 0
physicalPresenceCMDEnable 1
CEKPUsed 0
TPMpost 0
TPMpostLock 0
FIPS 0
Operator 0
enableRevokeEK 0
nvLocked 1
readSRKPub 0
tpmEstablished 0
maintenanceDone 0
disableFullDALogicInfo 0

Pre-owned:
localhost ~ # tpmc getpf
disable 0
ownership 1
deactivated 0
readPubek 0
disableOwnerClear 0
allowMaintenance 0
physicalPresenceLifetimeLock 1
physicalPresenceHWEnable 0
physicalPresenceCMDEnable 1
CEKPUsed 0
TPMpost 0
TPMpostLock 0
FIPS 0
Operator 0
enableRevokeEK 0
nvLocked 1
readSRKPub 1
tpmEstablished 0
maintenanceDone 0
disableFullDALogicInfo 0

> I set up a TPM1.2 device today. But keep failing on executing the `tpmc def` to setup a test space. I am not familiar with the permission design of TPM1.2. Which state I can execute the `tpmc def`?

Note that TlclDefineSpaceEx() that is called internally in tpm 1.2 case sends command without authentication (starts with 00 C1) if owner_auth == NULL. And that's exactly what TlclDefineSpace() called by 'tpmc def' does. In this case, it is expected that physical presence is used, what's only available when firmware runs or in recovery mode (that's where 'tpmc def' is normally caleld by the scripts - in recovery mode). So, to test you need to build a custom recovery image, or also  support a version of tpmc def that accepts owner password as authentication and calls TlclDefineSpaceEx() with it.

For TPM2.0 with platform hierarchy, I had some tried that flash my custom AP firmware that disabled the platform hierarchy disablement in coreboot at in third_party/coreboot/src/vendorcode/google/chromeos/tpm2.c
and also in trunks::TpmUtilityImpl::InitializeTpm 

But it seems even I can modify the change, the platform hierarchy is still be disabled. I have not looked into the detail yet. Is there any protection in Cr50 for custom firmware such as it will disable the platform hierarchy when loading custom AP firmware?

I guess that if this direction is possible, then I can have similar change for enabling the pp in TPM1.2, and using make PP be enabled after booting into OS.

> But it seems even I can modify the change, the platform hierarchy is still be disabled. I have not looked into the detail yet. Is there any protection in Cr50 for custom firmware such as it will disable the platform hierarchy when loading custom AP firmware?

No, cr50 doesn't contain any logic that disables PH w/o requests from AP.
I believe, atm neither trunksd not tpm_managerd do anything to disable PH when they start.


> I had some tried that flash my custom AP firmware that disabled the platform hierarchy disablement in coreboot at in third_party/coreboot/src/vendorcode/google/chromeos/tpm2.c
and also in trunks::TpmUtilityImpl::InitializeTpm

First, I don't think trunks::TpmUtilityImpl::InitializeTpm is ever called during boot. Only if somebody does it through trunks_client CLI.

Second, third_party/coreboot/src/vendorcode/google/chromeos/tpm2.c maybe not the right location. Are you building coreboot & depthcharge from ToT - you want to build from the right firmware-* branch to get something that is known to work on a particular board. And in many of those branches the layout of the code is different. Plus, it should be depthcharge that disables PH iirc.

TlclLockPhysicalPresence() in RollbackKernelLock() in platform/vboot_reference/firmware/lib/rollback_index.c called from vb2_kernel_phase4() in platform/vboot_reference/firmware/lib/vboot_api_kernel.c is what does it iiuc.

If in doubt, add debug printouts to see if a certain function is called. But changing TlclLockPhysicalPresence() in Tlcl seems to be the right way to do it.


In any case, you care about TPM 1.2 case, not 2.0 case for comment #7.

Thanks. You point me a direction to take a look a depthcharge. It it surprised that depthcharge located in vboot_reference

> Are you building coreboot & depthcharge from ToT
BTW, what's the ToT means here? 

I already tested with TPM 2.0 and now are working on use recover image to test on TPM 1.2 since it doesn't have CCD there.

For TPM 1.2, I am able to write some code in src/platform/initramfs/recovery/init and flash to a recovery image. It seems it's able to run my code, but got a black screen there. I am working on figuring out whether my code is executed successfully and also finding out a way to have more convenient way to test without re-build the image and re-fresh the USB every time.
 

Some update, already got a shell in the environment that pp is on. But it seems the undefine operation in TPM 1.2 is only allowed with TPM Owner authentication.

Here's the some paragraph of the TPM 1.2 spec,
> If tag != TPM_TAG_RQU_AUTH1_COMMAND then
>   ...
>   If pubInfo -> dataSize is 0 then return TPM_BAD_DATASIZE. Setting the size to 0 represents an attempt to delete the value without TPM Owner authentication.
>   ...

Do I misunderstand anything? If not, the 'undef' need owner authentication which need the SRK is set. It also means it is not possible to use it when physical presence is enabled. Then if it is not used in firmware, what the 'tpm undef' use case for TPM 1.2 here?

> BTW, what's the ToT means here? 
Top of trunk for the source code repository. In this case I mean chromeos-2016.05 or master branch.

> Do I misunderstand anything? 
Looks like you are right. Though see the following text snippet:

2. If TPM_PERMANENT_FLAGS -> nvLocked is FALSE then all authorization checks except for the Max NV writes are ignored
a. Ignored checks include [...] pubInfo -> dataSize is 0 in Action 5.c. [...]

Though, for an owned TPM we may still stumble here:
ii. If ownerAuth is present, the TPM MAY check the authorization HMAC


So, if nvLocked is FALSE then this check is not enforced, so an area may be undefined with PP auth (depends on what TPM implementation decides to do with owner auth). So, adding support for undefining still makes sense.

However, from the practical point of view, looks like we need to have support for 2 cases:
 1) the caller wants to define a new space only if it is not yet defined [this is what the current 2.0 implementation does]
 2) the caller wants to delete the old space if already existed (assuming the provided auth method works), and define a new one with new parameters [this is what the current 1.2 implementation does]

Since tpmc is normally called from recovery/factory scripts, which are pretty blunt and attempt to put the device in the right state even if something was wrong before, (2) is what they should need. The OOBE-auto-config case that triggered this change now also needs behavior (2).

So, having 'undef'/TlclUndefineSpaceEx (note, in Ex mode you can still provide owner auth, though tpmc undef can be quite simple and rely on having PP/PH, also passing NULL to UndefineEx as ownerAuth) still makes sense. Even if it will not work in 1.2 case in the majority of the situations.

But what we also need is to change the "tpmc def" behavior for 2.0 to check if the space already exists and call Undefine (or always call Undefine and ignore errors resulting from no space defined), and then call Define. This way, scripts will just continue to always call 'tpmc def' assuming behavior (2). It makes sense to update the help text for tpmc accordingly.

If somebody ever requests behavior (1), we can add a new call to tpmc to do just that.

Modified the bug title and description to match comments #12 and #13.

Do you think which is a better place to put the implementation of behavior that 'tpmc def' will call Undefine first? In TlclDefineSpace or in tpmc.c?
a. If we put that in TlclDefineSpace we can just put the code only in TPM 2.0 part, but it will make Tlcl much complex and will change the function behavior.
b. If we put the that in tpmc, then we will rely on the TPM2_MODE to decide whether to delete which may code much complex.

I choose b. right now and use (2) by default and give a option in TPM 2.0 for (1).
In TPM 2.0, it seems the owner authorization with customized owner password is unimplemented in DefineSpaceEx, so I leave the UndefineSpaceEx also unimplemented here.

The CLs are ready for review.

BTW, do you know when the NVLocked is set? I searched "tpmc setnv" in code search and didn't find anything. It seems it is set before enter the recovery mode.

+1 for going with option (b) == call Define + Undefine on tpmc side.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vboot_reference/+/737e34e377dfb547e64c278470a224bfcde3b97c

commit 737e34e377dfb547e64c278470a224bfcde3b97c
Author: Meng-Huan Yu <menghuan@google.com>
Date: Thu Nov 01 09:17:42 2018

tpmc: Add TlclUndefineSpace/Ex for TPM 1.2/2.0

For TPM 1.2, to undefine the space is just define a size 0 space.
And all operation should be done under physical presence is set
if NvLocked is set. Iirc, NvLocked is usually set before boot.

For TPM 2.0, support to undefine space regardless platform hierarchy
state. We will use platform authorization when TPMA_NV_PLATFORMCREATE
of that space is set. Otherwise, we will try to use owner
authorization with NULL password.
For owner authorization with customized password is still not
supported in UndefineSpace since it is also not support in
DefineSpaceEx.

BUG= chromium:895549 
BRANCH=None
TEST=vboot_reference unit test passed and added new link test for TPM 1.2.
For TPM 2.0, there is no unit test, but passed manually test
with tpmc in the following commit.
Also passed depthcharge unit test for TPM 2.0 and TPM 1.2 board.

Change-Id: I06dcc70c63a88a04d19f3b248666ff2492a1d2b0
Reviewed-on: https://chromium-review.googlesource.com/1291131
Commit-Ready: Meng-Huan Yu <menghuan@chromium.org>
Tested-by: Meng-Huan Yu <menghuan@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/737e34e377dfb547e64c278470a224bfcde3b97c/firmware/lib/tpm2_lite/tlcl.c
[modify] https://crrev.com/737e34e377dfb547e64c278470a224bfcde3b97c/firmware/linktest/main.c
[modify] https://crrev.com/737e34e377dfb547e64c278470a224bfcde3b97c/firmware/lib/tpm_lite/tlcl.c
[modify] https://crrev.com/737e34e377dfb547e64c278470a224bfcde3b97c/firmware/include/tpm2_tss_constants.h
[modify] https://crrev.com/737e34e377dfb547e64c278470a224bfcde3b97c/firmware/lib/tpm2_lite/marshaling.c
[modify] https://crrev.com/737e34e377dfb547e64c278470a224bfcde3b97c/tests/tlcl_tests.c
[modify] https://crrev.com/737e34e377dfb547e64c278470a224bfcde3b97c/firmware/include/tlcl.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vboot_reference/+/e05cdbc1d3f10df0301fe0b4ec7a7ff7bc502097

commit e05cdbc1d3f10df0301fe0b4ec7a7ff7bc502097
Author: Meng-Huan Yu <menghuan@google.com>
Date: Thu Nov 01 09:17:43 2018

tpmc: Add 'undef' command support to undefine NV space

For TPM 1.2, the undef command only works when NvLocked is not set
which is usually set before boot, even for recovery mode.

For TPM 2.0, it will automaticly choose the correct authorization
according to the TPMA_NV_PLATFORMCREATE attribute of that index.

BUG= chromium:895549 
BRANCH=None
TEST=No test for TPM 1.2

Manually test for TPM 2.0:
1. Boot with platform hierarchy is disabled, then
  # perm: TPMA_NV_AUTHREAD | TPMA_NV_AUTHWRITE
  tpmc def 0x1020 0x10 0x40004
  tpmc getp 0x1020 # check the space exists, expect success
  tpmc undef 0x1020
2. Boot with platform hierarchy is enabled, then run
  # perm: TPMA_NV_AUTHREAD | TPMA_NV_AUTHWRITE |
  #       TPMA_NV_PLATFORMCREATE
  tpmc def 0x1020 0x1 0x40040004
  tpmc getp 0x1020 # check the space exists, expect success
  tpmc undef 0x1020

Change-Id: I1d814287fda3e7c11933eca7334fdc3ab1ebf895
Reviewed-on: https://chromium-review.googlesource.com/1298097
Commit-Ready: Meng-Huan Yu <menghuan@chromium.org>
Tested-by: Meng-Huan Yu <menghuan@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/e05cdbc1d3f10df0301fe0b4ec7a7ff7bc502097/utility/tpmc.c

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vboot_reference/+/62eec262de829a641833f9509231f099f0661dad

commit 62eec262de829a641833f9509231f099f0661dad
Author: Meng-Huan Yu <menghuan@google.com>
Date: Thu Nov 01 09:17:43 2018

tpmc: Make 'tpmc def' replace the existing space by default

In chromium:895549, we want to have consistent behavior of
'tpmc def' between TPM 1.2 and TPM 2.0.
In TPM 1.2, define space command will undefine the existing space,
and create a new one. So we make the 'tpmc def' act as this by
default.

Also, provide a option for whom may want to define a new space
only if it is not defined yet. It will return TPM error code
at that case.

BUG= chromium:895549 
BRANCH=None
TEST=unit test; manually test:

 # For TPM 2.0 use AUTHREAD|AUTHWRITE
 tpmc tpmversion | grep 2.0 && export PERM=0x40004
 tpmc tpmversion | grep 1.2 && export PERM=0x1

 # Define the space
 tpmc def 0x1020 0x1 "$PERM"

 # Redefine the space, default will overwrite
 tpmc def 0x1020 0x1 "$PERM"
 # Expected: Success

 tpmc def 0x1020 0x1 "$PERM" --no-overwrite
 # Expected: output error for the space is already defined.
 # For TPM 2.0, it should output:
 #  command "def" failed with code 0x14c
 #  the TPM error code is unknown to this program
 # For TPM 1.2, it should output:
 #  The space is existing but --no-overwrite is set.

Change-Id: I9b4e742f2935578443ebcc69e91d0aebc84deed8
Reviewed-on: https://chromium-review.googlesource.com/1298098
Commit-Ready: Meng-Huan Yu <menghuan@chromium.org>
Tested-by: Meng-Huan Yu <menghuan@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/62eec262de829a641833f9509231f099f0661dad/utility/tpmc.c

All CL landed. Mark as fixed.