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

Issue 688234 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

cryptohome: can't login if trunksd restarts

Project Member Reported by apronin@chromium.org, Feb 3 2017

Issue description

If trunksd restarts (e.g. crashes and is automatically re-launched) while the user is logged out, the user can't log back in.

The problem is that the virtual handle that cryptohomed used for cryptohome key is valid only while trunksd is running. When attempting to use this handle with the new instance of trunksd, trunksd returns an error.

There is already logic in cryptohomed that reloads the key if it gets an "invalid handle" error when reading the key public area. However, trunksd sets its own non-zero error base code in the upper 20 bits of the returned response code (which is always set to 0 by the actual tpm), and thus the returned code is not recognized as "invalid handle" by cryptohomed.  
 
Status: Started (was: Assigned)
To fix, base the response-code-specific decisions inside cryptohomed only on the lower 12 bits of the returned code.
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 6 2017

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

commit de1f344564d3e034caa3f3d56e5df222d9db039e
Author: Andrey Pronin <apronin@chromium.org>
Date: Mon Feb 06 22:50:31 2017

cryptohome: use only lower 12 bits of error code for tpm2

For TPM2, trunksd sets its own non-zero base error code in the upper
20 bits of the returned code, when the error is generated inside
trunksd. That may prevent cryptohome from correctly recognizing
the right retry action.
With this fix, selection of the retry action is based only on the
lower 12 bits of the returned code.

BUG= chromium:688234 
TEST=1) Boot, login, logout.
     2) restart trunksd.
     3) Login. The system should let the user back in.

Change-Id: I6fe1a80b2d2000242734dc7bbbd1fa80dcc36f2d
Reviewed-on: https://chromium-review.googlesource.com/436767
Commit-Ready: Andrey Pronin <apronin@chromium.org>
Tested-by: Andrey Pronin <apronin@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/de1f344564d3e034caa3f3d56e5df222d9db039e/cryptohome/tpm2_impl.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 6 2017

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

commit de1f344564d3e034caa3f3d56e5df222d9db039e
Author: Andrey Pronin <apronin@chromium.org>
Date: Mon Feb 06 22:50:31 2017

cryptohome: use only lower 12 bits of error code for tpm2

For TPM2, trunksd sets its own non-zero base error code in the upper
20 bits of the returned code, when the error is generated inside
trunksd. That may prevent cryptohome from correctly recognizing
the right retry action.
With this fix, selection of the retry action is based only on the
lower 12 bits of the returned code.

BUG= chromium:688234 
TEST=1) Boot, login, logout.
     2) restart trunksd.
     3) Login. The system should let the user back in.

Change-Id: I6fe1a80b2d2000242734dc7bbbd1fa80dcc36f2d
Reviewed-on: https://chromium-review.googlesource.com/436767
Commit-Ready: Andrey Pronin <apronin@chromium.org>
Tested-by: Andrey Pronin <apronin@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/de1f344564d3e034caa3f3d56e5df222d9db039e/cryptohome/tpm2_impl.cc

Cc: keta...@chromium.org hennessywill@chromium.org
Labels: Merge-Request-57
The last piece of the puzzle per http://crosbug.com/p/62370#c35.
Improves robustness of reloading cryptohome key after suspend or other situations when it is not known to trunksd on tpm2.
Which OSs does this change affect?
Labels: OS-Chrome
Re #5: Chrome OS, in specific cases when trunksd daemon is running.
Project Member

Comment 7 by sheriffbot@chromium.org, Feb 8 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 8 2017

Labels: merge-merged-release-R57-9202.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/7da3b1c592bd90f377693df3f3f38bf3ba18ec7e

commit 7da3b1c592bd90f377693df3f3f38bf3ba18ec7e
Author: Andrey Pronin <apronin@chromium.org>
Date: Wed Feb 08 01:09:21 2017

cryptohome: use only lower 12 bits of error code for tpm2

For TPM2, trunksd sets its own non-zero base error code in the upper
20 bits of the returned code, when the error is generated inside
trunksd. That may prevent cryptohome from correctly recognizing
the right retry action.
With this fix, selection of the retry action is based only on the
lower 12 bits of the returned code.

BUG= chromium:688234 
TEST=1) Boot, login, logout.
     2) restart trunksd.
     3) Login. The system should let the user back in.

Change-Id: I6fe1a80b2d2000242734dc7bbbd1fa80dcc36f2d
Reviewed-on: https://chromium-review.googlesource.com/436767
Commit-Ready: Andrey Pronin <apronin@chromium.org>
Tested-by: Andrey Pronin <apronin@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>
(cherry picked from commit de1f344564d3e034caa3f3d56e5df222d9db039e)
Reviewed-on: https://chromium-review.googlesource.com/438933
Commit-Queue: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/7da3b1c592bd90f377693df3f3f38bf3ba18ec7e/cryptohome/tpm2_impl.cc

Labels: -Merge-Approved-57 Merge-Merged
Status: Fixed (was: Started)

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

Labels: VerifyIn-59

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

Labels: VerifyIn-60
Labels: VerifyIn-61

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

Status: Archived (was: Fixed)

Sign in to add a comment