Ensure system.data is initialized when the TPM is already owned. (issue3318011)
f...@chromium.org
Description:
Ensure system.data is initialized when the TPM is already owned.
This CL addresses a problem where trousers requires an initialized
system.data,
but that only happens fully when a successful call to
Tspi_TPM_TakeOwnership is
made. If the TPM has been owned by a previous install, and isn't cleared,
then
trousers will not work. To address this, the tcsd.conf file has been
modified
to copy a stock system.data file when it detects that the TPM is owned, but
the
system.data file is missing or zero-length.
Testing consists of owning the TPM, and then deleting or zero-sizing the
system.data file, rebooting and running `cryptohome --action=status`,
which, on
success, will be able to test the TPM functionality for cryptohome.
Change-Id: I0a97fa7cd74b4dff6cdf74e8ca2d85842ff85fc3
BUG=NONE
TEST=see above
Please review this at http://codereview.chromium.org/3318011/show
SVN Base: http://git.chromium.org/git/init.git
Affected files:
M tcsd.conf
Index: tcsd.conf
diff --git a/tcsd.conf b/tcsd.conf
index
59199f446b2853b130981f2edd2d8185f9d42420..4dee158e8e53e8878f9d79a1dfe4fcd4d814708f
100644
--- a/tcsd.conf
+++ b/tcsd.conf
@@ -16,9 +16,23 @@ pre-start script
/usr/bin/logger "tpm fix status: $status"
# end of temporary fix
- if [ "0" = $(cat /sys/class/misc/tpm0/device/owned) ]; then
- # clean up any existing tcsd state.
- rm -rf /var/lib/tpm/*
+ if [ -e /sys/class/misc/tpm0/device/owned ]; then
+ if [ "0" = $(cat /sys/class/misc/tpm0/device/owned) ]; then
+ # Clean up any existing tcsd state.
+ rm -rf /var/lib/tpm/*
+ else
+ # Already owned.
+ # Check if trousers' system.data is size zero. If so, then the TPM
has
+ # been owned already and we need to copy over an empty system.data
to be
+ # able to use it in trousers.
+ if [ ! -f /var/lib/tpm/system.data ] || \
+ [ ! -s /var/lib/tpm/system.data ]; then
+ if [ ! -e /var/lib/tpm ]; then
+ mkdir -p /var/lib/tpm
+ fi
+ cp /etc/trousers/system.data.auth /var/lib/tpm/system.data
+ fi
+ fi
fi
end script
Luigi Semenzato
look at how trousers uses system.data, and it seems that it may be
writing some keys to it. Are you comfortable that replacing a
possibly customized system.data with a stock one will not cause weird
problems (possibly in the future)?
f...@chromium.org
> So there is one thing that worries me a little... I was taking a quick
> look at how trousers uses system.data, and it seems that it may be
> writing some keys to it. Are you comfortable that replacing a
> possibly customized system.data with a stock one will not cause weird
> problems (possibly in the future)?
So this will only copy the stock one if the TPM says it is owned and we
have no
existing system.data, or the system.data is zero-length. The stock one
that we
copy from the distro is actually the same as the one that Trousers creates
when
we go through the ownership steps. Trousers will add keys to system.data
when
you call Tspi_Context_RegisterKey, but that means it won't be zero-length,
so
the fix should be fine with that. Cryptohome doesn't use that API, and I
believe that opencryptoki also stores keys outside of the TSS system
persistent
storage.
seme...@chromium.org
http://codereview.chromium.org/3318011/diff/3001/4001
File tcsd.conf (right):
http://codereview.chromium.org/3318011/diff/3001/4001#newcode19
tcsd.conf:19: if [ -e /sys/class/misc/tpm0/device/owned ]; then
Suggestion: replace /sys/class/misc/tpm0/device with a shell variable.
Or better:
owned=$(cat /sys/class/misc/tpm0/device/owned || echo "")
if [ "$owned" != "" ] etc. etc.
http://codereview.chromium.org/3318011/diff/3001/4001#newcode31
tcsd.conf:31: mkdir -p /var/lib/tpm
The thing to worry about here is that you're making the directory as
root and maybe trousers runs as tss?
f...@chromium.org
http://codereview.chromium.org/3318011/diff/3001/4001
File tcsd.conf (right):
http://codereview.chromium.org/3318011/diff/3001/4001#newcode19
tcsd.conf:19: if [ -e /sys/class/misc/tpm0/device/owned ]; then
On 2010/09/03 21:05:15, Luigi Semenzato wrote:
> Suggestion: replace /sys/class/misc/tpm0/device with a shell variable.
> Or better:
> owned=$(cat /sys/class/misc/tpm0/device/owned || echo "")
> if [ "$owned" != "" ] etc. etc.
Done.
http://codereview.chromium.org/3318011/diff/3001/4001#newcode31
tcsd.conf:31: mkdir -p /var/lib/tpm
On 2010/09/03 21:05:15, Luigi Semenzato wrote:
> The thing to worry about here is that you're making the directory as
root and
> maybe trousers runs as tss?
tcsd runs as root on our install, and the normal ownership code path
creates a directory and system.data that are owned by root.
seme...@chromium.org
changing the code or commenting).
http://codereview.chromium.org/3318011/diff/9002/21001
File tcsd.conf (right):
http://codereview.chromium.org/3318011/diff/9002/21001#newcode40
tcsd.conf:40: umask $(current_mask)
I am afraid to ask. Why go through all this trouble with umask instead
of just chmoding system.data to something reasonable?
f...@chromium.org
necessary
changes.
http://codereview.chromium.org/3318011/diff/9002/21001
File tcsd.conf (right):
http://codereview.chromium.org/3318011/diff/9002/21001#newcode40
tcsd.conf:40: umask $(current_mask)
On 2010/09/08 21:06:08, Luigi Semenzato wrote:
> I am afraid to ask. Why go through all this trouble with umask
instead of just
> chmoding system.data to something reasonable?
As far as doing it here, it's merely a matter of dislike for
create-then-chmod in favor of having the file mode correct at the time
of creation or copy, which I can do with umask.
The other part to this is the original file mode of the stock
system.data.auth file. In a separate CL I can change the ebuild for
trousers to set the file mode using insopts, but to keep this from being
dependent, I added the file mode bits here.
seme...@chromium.org
Also, feel free to ignore the pickiness.
http://codereview.chromium.org/3318011/diff/9002/21001
File tcsd.conf (right):
http://codereview.chromium.org/3318011/diff/9002/21001#newcode40
tcsd.conf:40: umask $(current_mask)
Sorry, I meant "LGTM unless the question seems reasonable to you". You
decide.
In any case you're right: create + chmod has a race. But it's a little
confusing with "touch .tpm_owned" because the casual reader won't know
that the file doesn't exist. Maybe "echo > .tpm_owned" is clearer? But
this is good enough.