PersistentLookupTable doesn't surface file system errors consistently |
|
Issue descriptionWhen writing unit tests, I attempted to make on-disk state invalid, but encountered surprising behavior: 1. Doing chmod 0000 $le_state_dir doesn't cause errors but will be seen by the PersistentLookupTable implementation as if no data files existed at all (i.e. it returns PLT_KEY_NOT_FOUND). 2. Clobbering a data file so it can't be parsed causes SignInHashTree to consider the label absent. I think the reason is that SigninHashTree::GetLabelData() can't distinguish between error and absence. We should probably fix error behavior and have a reasonable set of unit tests that make sure errors get detected correctly.
,
Aug 31
> 1. Doing chmod 0000 $le_state_dir doesn't cause errors but will be seen by the PersistentLookupTable implementation as if no data files existed at all (i.e. it returns PLT_KEY_NOT_FOUND). It looks like this appears to be because we use cryptohome::Platform::DirectoryExists, to see if a particular key exists, and that simply returns a boolean false if according to it the directory didn't exist. I'm thinking that if the directory has been marked as 0000, then PLT_KEY_NOT_FOUND is probably the right error code (from the PersistentLookupTable viewpoint, it can't access or even see anything inside that directory). > 2. Clobbering a data file so it can't be parsed causes SignInHashTree to consider the label absent. I think the reason is that SigninHashTree::GetLabelData() can't distinguish between error and absence. Just wanted to clarify: By clobber you mean corrupt the file (but not delete it or replace it with an empty file) right? An empty file has a special meaning for PLT (it means key has been marked for deletion), but for other corruptions yeah it should return the right value (currently only boolean); I'll look into where all it's being called, and where it is being interpreted incorrectly. |
|
►
Sign in to add a comment |
|
Comment 1 by benhenry@chromium.org
, Aug 3