New issue
Advanced search Search tips

Issue 847438 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 16
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Switch cryptohomed lockbox verification to using the install attributes cache file

Project Member Reported by mnissler@chromium.org, May 29 2018

Issue description

cryptohomed currently does duplicate lockbox verification. We should switch it over to just use the result from lockbox-cache
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 5

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

commit d9bf5e89344daaf0af001a7dfda07586b08759ce
Author: Mattias Nissler <mnissler@chromium.org>
Date: Wed Dec 05 22:09:25 2018

cryptohome: Break out Lockbox::ErrorId

This replaces the existing enum that is nested in the lockbox class
with an enum class LockboxError in the cryptohome namespace. This is a
mechanical refactoring in preparation to break up the Lockbox class in
subsequent changes.

BUG= chromium:847438 
TEST=Compiles and passes tests.

Change-Id: I2fd4f05b13a277d52bf3982d2e556504147c133a
Reviewed-on: https://chromium-review.googlesource.com/1163674
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/d9bf5e89344daaf0af001a7dfda07586b08759ce/cryptohome/install_attributes_unittest.cc
[modify] https://crrev.com/d9bf5e89344daaf0af001a7dfda07586b08759ce/cryptohome/lockbox.cc
[modify] https://crrev.com/d9bf5e89344daaf0af001a7dfda07586b08759ce/cryptohome/lockbox_unittest.cc
[modify] https://crrev.com/d9bf5e89344daaf0af001a7dfda07586b08759ce/cryptohome/lockbox-cache.cc
[modify] https://crrev.com/d9bf5e89344daaf0af001a7dfda07586b08759ce/cryptohome/mock_lockbox.h
[modify] https://crrev.com/d9bf5e89344daaf0af001a7dfda07586b08759ce/cryptohome/lockbox.h
[modify] https://crrev.com/d9bf5e89344daaf0af001a7dfda07586b08759ce/cryptohome/install_attributes.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 5

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

commit 93e8b623bb376d9c3e0020e14c8b07a265034046
Author: Mattias Nissler <mnissler@chromium.org>
Date: Wed Dec 05 22:09:25 2018

cryptohome: Modularize lockbox code

Move all verification logic into the LockboxContents class. This will
allow lockbox verification without a dependency to TPM. As a result,
the unit testing code becomes cleaner, and we'll be able to simplify
lockbox-cache in a subsequent change.

BUG= chromium:847438 
TEST=Compiles and passes tests.

Change-Id: If745644739aa27a080458ff67414a4fa90d50375
Reviewed-on: https://chromium-review.googlesource.com/1163675
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/93e8b623bb376d9c3e0020e14c8b07a265034046/cryptohome/install_attributes_unittest.cc
[modify] https://crrev.com/93e8b623bb376d9c3e0020e14c8b07a265034046/cryptohome/lockbox.h
[modify] https://crrev.com/93e8b623bb376d9c3e0020e14c8b07a265034046/cryptohome/install_attributes.cc
[modify] https://crrev.com/93e8b623bb376d9c3e0020e14c8b07a265034046/cryptohome/lockbox.cc
[modify] https://crrev.com/93e8b623bb376d9c3e0020e14c8b07a265034046/cryptohome/lockbox_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 5

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

commit 6a48b4abdf1ec193064d7bb0feb8e15b92224d88
Author: Mattias Nissler <mnissler@chromium.org>
Date: Wed Dec 05 22:09:26 2018

cryptohome: Remove Tpm dependency from LockboxCache

Use LockboxContents directly and avoid the heavy-weight TPM
dependency, which was stubbed out anyways. While at it, get rid of the
LockboxCache class in favor of a simple function to simplify the code.

BUG= chromium:847438 
TEST=Unit tests.

Change-Id: Ie28e25b6d3713fcfb5d3ab13426dfe596479e5e9
Reviewed-on: https://chromium-review.googlesource.com/1202423
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[delete] https://crrev.com/d454150c9969c4f13cfc36be884005ea723960e6/cryptohome/lockbox-cache-tpm.cc
[modify] https://crrev.com/6a48b4abdf1ec193064d7bb0feb8e15b92224d88/cryptohome/libs/BUILD.gn
[modify] https://crrev.com/6a48b4abdf1ec193064d7bb0feb8e15b92224d88/cryptohome/lockbox-cache.h
[modify] https://crrev.com/6a48b4abdf1ec193064d7bb0feb8e15b92224d88/cryptohome/BUILD.gn
[modify] https://crrev.com/6a48b4abdf1ec193064d7bb0feb8e15b92224d88/cryptohome/lockbox-cache.cc
[delete] https://crrev.com/d454150c9969c4f13cfc36be884005ea723960e6/cryptohome/lockbox-cache-tpm.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 16 (6 days ago)

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

commit eb2d207b7f29f7500091d903d1d3ad15e14c28c7
Author: Mattias Nissler <mnissler@chromium.org>
Date: Wed Jan 16 13:25:39 2019

cryptohome: Load install attributes from cache file

Remove the install attributes verification code path from the
InstallAttributes class in cryptohomed. The code now loads the install
attributes from the cache file generated by lockbox-cache during early
boot.

BUG= chromium:847438 
TEST=Unit tests.

Change-Id: I1b77e41967f0e3b0bedee4f644d47597e1696c65
Reviewed-on: https://chromium-review.googlesource.com/1206076
Commit-Ready: Mattias Nissler <mnissler@chromium.org>
Tested-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/eb2d207b7f29f7500091d903d1d3ad15e14c28c7/cryptohome/install_attributes_unittest.cc
[modify] https://crrev.com/eb2d207b7f29f7500091d903d1d3ad15e14c28c7/cryptohome/service.h
[modify] https://crrev.com/eb2d207b7f29f7500091d903d1d3ad15e14c28c7/cryptohome/tpm_manager_v1.cc
[modify] https://crrev.com/eb2d207b7f29f7500091d903d1d3ad15e14c28c7/cryptohome/install_attributes.h
[modify] https://crrev.com/eb2d207b7f29f7500091d903d1d3ad15e14c28c7/cryptohome/mock_tpm_init.h
[modify] https://crrev.com/eb2d207b7f29f7500091d903d1d3ad15e14c28c7/cryptohome/mock_install_attributes.h
[modify] https://crrev.com/eb2d207b7f29f7500091d903d1d3ad15e14c28c7/cryptohome/install_attributes.cc
[modify] https://crrev.com/eb2d207b7f29f7500091d903d1d3ad15e14c28c7/cryptohome/service.cc
[modify] https://crrev.com/eb2d207b7f29f7500091d903d1d3ad15e14c28c7/cryptohome/attestation_unittest.cc

Comment 6 by mnissler@chromium.org, Jan 16 (6 days ago)

Status: Fixed (was: Assigned)
This is finally done.

Sign in to add a comment