New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 601492 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Jun 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Feature



Sign in to add a comment

Add cryptohome support for fwmp

Project Member Reported by rspangler@chromium.org, Apr 7 2016

Issue description

Firmware management parameters will be stored in a TPM NV space, similar to the lockbox.  Add cryptohome support for getting/setting/removing this space.

Design doc: https://docs.google.com/a/google.com/document/d/1-O1OZxG0j12emPkfKLhSxFK0shffJqAlDB4RrxP78XU/edit?usp=sharing

 
Status: Started (was: Assigned)
In case I get eaten by a shark next week,

cryptohome changes are here (and have unit tests):

https://chromium-review.googlesource.com/339224
https://chromium-review.googlesource.com/339262

vboot_reference changes are being tracked here (functional in hand-testing, but currently lack unit tests):

https://chromium-review.googlesource.com/339234

Project Member

Comment 3 by bugdroid1@chromium.org, May 9 2016

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

commit 946abf1439f378dded6d4c4a82b53af86cdf44a3
Author: Randall Spangler <rspangler@chromium.org>
Date: Fri Apr 15 21:49:40 2016

vboot: Add firmware management parameters

This adds RW firmware support for the optional firmware management
parameters TPM space.

System-level tests require CL:339262 to add cryptohome support.

BUG= chromium:601492 
BRANCH=baytrail and newer platforms
TEST=make -j runtests
     Or better, COV=1 make, and then make sure all new code is covered.

Change-Id: Ifaf644c80809552d5961615be6017c2a332a034b
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/339234

[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/firmware/lib/rollback_index.c
[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/tests/vboot_api_kernel4_tests.c
[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/tests/vboot_kernel_tests.c
[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/firmware/include/vboot_api.h
[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/tests/vboot_api_kernel2_tests.c
[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/firmware/include/tss_constants.h
[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/firmware/include/gbb_header.h
[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/firmware/lib/vboot_api_kernel.c
[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/firmware/lib/vboot_kernel.c
[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/tests/rollback_index2_tests.c
[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/firmware/lib/include/load_kernel_fw.h
[modify] https://crrev.com/946abf1439f378dded6d4c4a82b53af86cdf44a3/firmware/lib/include/rollback_index.h

Project Member

Comment 4 by bugdroid1@chromium.org, May 11 2016

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

commit 5319565988fc5b1862d649fad985859929946a91
Author: Randall Spangler <rspangler@chromium.org>
Date: Tue May 10 16:45:05 2016

vboot: Fix FWMP link error if TPM is mocked

The MOCK_TPM build flag caused link to fail because RollbackFwmpRead()
was missing its mock.

BUG= chromium:601492 
BRANCH=baytrail and newer platforms
TEST=make -j runtests
     Hack makefile to add MOCK_TPM := 1 and make -j; no link errors.

Change-Id: I3885d6b6c627bf475f4da33ef67f31aec2159799
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/343920
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

[modify] https://crrev.com/5319565988fc5b1862d649fad985859929946a91/firmware/lib/mocked_rollback_index.c
[modify] https://crrev.com/5319565988fc5b1862d649fad985859929946a91/firmware/lib/rollback_index.c

Project Member

Comment 5 by bugdroid1@chromium.org, May 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/3fdb33369644d5e6fb688a72b2e589303d7e0a5b

commit 3fdb33369644d5e6fb688a72b2e589303d7e0a5b
Author: Randall Spangler <rspangler@chromium.org>
Date: Fri Apr 15 20:40:13 2016

cryptohome: Add Firmware Management Parameters API

This adds the get, set, and remove methods for the firmware management
parameters.

BUG= chromium:601492 
TEST=as part of subsequent cryptohome change (CL:339262)

Change-Id: Iece4e4a4320fbd5393d8e12f0f11e6b12c36d6f6
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/339224

[modify] https://crrev.com/3fdb33369644d5e6fb688a72b2e589303d7e0a5b/dbus/cryptohome/rpc.proto
[modify] https://crrev.com/3fdb33369644d5e6fb688a72b2e589303d7e0a5b/dbus/cryptohome/dbus-constants.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/79e911f4eb066976dba4b1078dbfa32626bd3244

commit 79e911f4eb066976dba4b1078dbfa32626bd3244
Author: Randall Spangler <rspangler@chromium.org>
Date: Fri Apr 15 20:45:33 2016

cryptohome: Add firmware management parameters

This adds the set_firmware_management_parameters,
get_firmware_management_parameters, and
remove_firmware_management_parameters actions needed to implement the
Firmware Management Parameters (FWMP).

The firmware changes which use the FWMP are in a subsequent CL.  The
presence or absense of the FWMP has no effect on the OS; only the
firmware (and eventually, the recovery initramfs scripts) cares.

See README.firmware_management_parameters for more information.

BUG= chromium:601492 
TEST=cros_workon_make --test --board=samus cryptohome

  And manual testing on samus.  Enable developer mode, then:

    cryptohome --action=tpm_status

  If the owner password is not known:

    crossystem clear_tpm_owner_request=1 clear_tpm_owner_done=0
    reboot
    cryptohome --action=tpm_take_ownership
    cryptohome --action=tpm_wait_ownership
    cryptohome --action=tpm_status

  Password should now be shown.
  Create a FWMP:

    cryptohome --action=set_firmware_management_parameters \
    --flags=0xaa00 \
    --developer_key_hash=\
    cbff4f4a16e694cd87171d7d22a88c8d6be37165c198d813ecaebe1367980812

  Verify TPM space was created:

    stop cryptohomed
    stop tcsd
    tpmc read 0x100a 0x28

  That should dump a 40-byte space:

    10 28 10 0 0 aa 0 0 cb ff 4f 4a 16 e6 94 cd 87 17 1d 7d 22 a8 8c 8d
    6b e3 71 65 c1 98 d8 13 ec ae be 13 67 98 8 12

  Extend PCR0 and verify the space is still readable (not PCR-locked):

    tpmc pcrextend 0 f1d2d2f924e986ac86fdf7b36c94bcdf32beec15
    tpmc read 0x100a 0x28

  Verify cryptohome can read it back:

    start tcsd
    start cryptohomed
    cryptohome --action=get_firmware_management_parameters

  Flags and hash should match what was set.  Then reboot:

    reboot

  Verify that the dev key hash is optional:

    cryptohome --action=set_firmware_management_parameters --flags=0xbb00
    cryptohome --action=get_firmware_management_parameters

  Flags should be 0xbb00, hash should be all 0.

  Verify FWMP can be removed:

    cryptohome --action=remove_firmware_management_parameters
    cryptohome --action=get_firmware_management_parameters
    stop tcsd
    tpmc read 0x100a 0x28
    start tcsd

  get_firmware_management_parameters fails with
  FIRMWARE_MANAGEMENT_PARAMETERS_INVALID, and tpmc read fails with
  TPM_BADINDEX

  Set a FWMP, then clear the TPM owner:

    cryptohome --action=set_firmware_management_parameters --flags=0xcc00
    crossystem clear_tpm_owner_request=1 clear_tpm_owner_done=0
    reboot
    cryptohome --action=tpm_status
    cryptohome --action=get_firmware_management_parameters

  Owner password should no longer be set.
  FWMP should still exist with flags=0xcc00.

  Set and remove should fail with
  FIRMWARE_MANAGEMENT_PARAMETERS_CANNOT_STORE and
  FIRMWARE_MANAGEMENT_PARAMETERS_CANNOT_REMOVE

    cryptohome --action=set_firmware_management_parameters --flags=0xdd00
    cryptohome --action=remove_firmware_management_parameters
    cryptohome --action=get_firmware_management_parameters

  FWMP should still exist with flags=0xcc00.
  Take ownership again, and remove the FWMP:

    cryptohome --action=tpm_take_ownership
    cryptohome --action=tpm_wait_ownership
    cryptohome --action=tpm_status
    cryptohome --action=remove_firmware_management_parameters
    cryptohome --action=get_firmware_management_parameters

  get_firmware_management_parameters fails with
  FIRMWARE_MANAGEMENT_PARAMETERS_INVALID

CQ-DEPEND=CL:339224

Change-Id: I256e36fabc2c90e50eb41c7f228982c5f224aabb
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/339262
Reviewed-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-by: Darren Krahn <dkrahn@chromium.org>

[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/lockbox_unittest.cc
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/tpm2_impl.h
[add] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/firmware_management_parameters.h
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/cryptohome.gyp
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/tpm2_test.cc
[add] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/firmware_management_parameters_unittest.cc
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/tpm_impl.cc
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/cryptohome.cc
[add] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/mock_firmware_management_parameters.cc
[add] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/mock_firmware_management_parameters.h
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/interface.cc
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/interface.h
[add] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/README.firmware_management_parameters
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/tpm2_impl.cc
[add] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/firmware_management_parameters.cc
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/stub_tpm.h
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/tpm_impl.h
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/cryptohome.xml
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/tpm_live_test.cc
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/service_unittest.cc
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/lockbox.cc
[add] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/crc8.h
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/service.h
[add] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/crc8.c
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/mock_tpm.h
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/service.cc
[modify] https://crrev.com/79e911f4eb066976dba4b1078dbfa32626bd3244/cryptohome/tpm.h

Status: Fixed (was: Started)
Cc: cyrusm@chromium.org
I've picked the firmware changes into the Candy firmware branch and tested them:

https://chromium-review.googlesource.com/356790

Labels: VerifyIn-53
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 22 2016

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

commit b3a625f8fef1768d78eab4cfaaea270cb3fbd0c3
Author: Randall Spangler <rspangler@chromium.org>
Date: Thu Jul 21 22:33:31 2016

vboot: Fix potential alignment issue reading FWMP

RollbackFwmpRead() assumed that a uint8[] array on the stack would be
aligned sufficiently for typecasting to struct RollbackSpaceFwmp and
accessing its members.

This was true on x86 (where unaligned accesses work fine) and probably
harmless on other platforms (since RollbackSpaceFwmp is
__attribute__(packed).  But it's cleaner to switch to using a union of
the buffer and struct, since that will provide the proper alignment.

BUG= chromium:601492 
BRANCH=baytrail and newer platforms
TEST=make -j runtests

Change-Id: I97077923ab5809c68510cbd382541bf2827aba6b
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/362087
Commit-Ready: Dan Shi <dshi@google.com>
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>

[modify] https://crrev.com/b3a625f8fef1768d78eab4cfaaea270cb3fbd0c3/firmware/lib/rollback_index.c

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 29 2016

Labels: merge-merged-firmware-candy-5216.310.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vboot_reference/+/b1d75add014b7383e00961156ce51749e6507465

commit b1d75add014b7383e00961156ce51749e6507465
Author: Randall Spangler <rspangler@chromium.org>
Date: Fri Apr 15 21:49:40 2016

vboot: Add firmware management parameters

This adds RW firmware support for the optional firmware management
parameters TPM space.

System-level tests require CL:339262 to add cryptohome support.

BUG= chromium:601492 
BRANCH=baytrail and newer platforms
TEST=make -j runtests
    Or better, COV=1 make, and then make sure all new code is covered.

    Additional manual tests.  MUST use a test image for these, because a
    test image has a root shell even with dev mode disabled:

    Set FWMP:
    crossystem clear_tpm_owner_request=1
    reboot
    cryptohome --action=tpm_take_ownership
    cryptohome --action=tpm_wait_ownership
    cryptohome --action=set_firmware_management_parameters --flags=1
    cryptohome --action=get_firmware_management_parameters

    Reboot system with power+refresh+esc
    Use Ctrl+D then Enter to enable dev mode.
    Goes to the TONORM screen.  
    Pressing Esc doesn't exit it.
    Pressing Enter turns dev mode off.
    Then let it boot.

    Just to make sure FWMP did get set persistently:
    cryptohome --action=get_firmware_management_parameters

    Now remove the FWMP
    crossystem clear_tpm_owner_request=1
    reboot
    cryptohome --action=tpm_take_ownership
    cryptohome --action=tpm_wait_ownership
    cryptohome --action=remove_firmware_management_parameters

    Reboot system with power+refresh+esc
    Use Ctrl+D then Enter to enable dev mode.
    Goes to the DEV screen.

Change-Id: I0ac31bb2c64671ee9c3c810174baaf02b4cce641
Original-Change-Id: Ifaf644c80809552d5961615be6017c2a332a034b
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/339234
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/356790
Reviewed-by: Darren Krahn <dkrahn@chromium.org>
Reviewed-by: Stefan Reinauer <reinauer@google.com>

[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/firmware/lib/rollback_index.c
[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/tests/vboot_api_kernel4_tests.c
[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/tests/vboot_kernel_tests.c
[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/firmware/include/vboot_api.h
[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/tests/vboot_api_kernel2_tests.c
[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/firmware/include/tss_constants.h
[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/firmware/include/gbb_header.h
[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/firmware/lib/vboot_api_kernel.c
[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/firmware/lib/vboot_kernel.c
[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/tests/rollback_index2_tests.c
[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/firmware/lib/include/load_kernel_fw.h
[modify] https://crrev.com/b1d75add014b7383e00961156ce51749e6507465/firmware/lib/include/rollback_index.h

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 29 2016

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

commit 232def8e8a1d1551127aabb3af4a9054a4b217d5
Author: Randall Spangler <rspangler@chromium.org>
Date: Thu Jul 21 22:33:31 2016

vboot: Fix potential alignment issue reading FWMP

RollbackFwmpRead() assumed that a uint8[] array on the stack would be
aligned sufficiently for typecasting to struct RollbackSpaceFwmp and
accessing its members.

This was true on x86 (where unaligned accesses work fine) and probably
harmless on other platforms (since RollbackSpaceFwmp is
__attribute__(packed).  But it's cleaner to switch to using a union of
the buffer and struct, since that will provide the proper alignment.

BUG= chromium:601492 
BRANCH=baytrail and newer platforms
TEST=make -j runtests

Change-Id: I97077923ab5809c68510cbd382541bf2827aba6b
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/362087
Commit-Ready: Dan Shi <dshi@google.com>
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
(cherry picked from commit b3a625f8fef1768d78eab4cfaaea270cb3fbd0c3)
Reviewed-on: https://chromium-review.googlesource.com/364272

[modify] https://crrev.com/232def8e8a1d1551127aabb3af4a9054a4b217d5/firmware/lib/rollback_index.c

Labels: VerifyIn-54
Labels: VerifyIn-55

Comment 15 by dchan@google.com, Nov 19 2016

Labels: VerifyIn-56

Comment 16 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 17 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 18 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 19 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 21 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment