Skip to content
This repository has been archived by the owner on Feb 12, 2019. It is now read-only.

libdokan: Provide fake root directory for other SIDs #1016

Merged
merged 4 commits into from
Jun 7, 2017

Conversation

taruti
Copy link
Contributor

@taruti taruti commented Jun 5, 2017

Provides a fake root directory for different SIDs. Should help with:
keybase/client#5490 and more recent issues.

The basic issue is with corporate AV products doing:

  1. try to access K:
  2. access denied because AV running with different user/permissions
  3. goto 1 in a hot loop

To test this patch using -mount-flags 0 when mounting kbfsdokan + switch user is the simplest way.

@zanderz is the error folder name ok for you?

@taruti taruti requested review from zanderz and strib June 5, 2017 11:48
Copy link
Contributor

@zanderz zanderz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

As an aside, I've wondered if there is a way to quiet down all the Explorer access it seems to do for previews and recently used files listing, etc. I think there may be a way which would require using an extension, which seems heavy-handed. Do you have thoughts on that?

libdokan/fs.go Outdated
@@ -538,6 +545,18 @@ func (f *FS) MoveFile(ctx context.Context, source *dokan.FileInfo, targetPath st
return nil
}

// isPotentialRenamePath filters out some special paths
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you please move this up to above the use?

return nil
}
var ns dokan.NamedStat
ns.FileAttributes = dokan.FileAttributeDirectory
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a directory and not a file? Not a big deal, just curious. I'd slightly prefer a solution that had a text file with explanatory text inside, but this is good enough for now I think.

f.log.Errorf("Refusing access: SID match error")
return nil, false, dokan.ErrAccessDenied
f.log.Errorf("FS CreateFile - Refusing real access: SID match error")
return openFakeRoot(ctx, f, fi)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're always guaranteed on Windows that the root will get a CreateFile call first on an empty string, before file listing is allowed or any other specific files can be looked up?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only ops not needing an opened file are CreateFile, GetDiskFreeSpace and GetVolumeInformation. MoveFile (rename) is also interesting from a security pov, but it needs an open file. Fakeroot takes care that only \ and \errorfilename succeed at CreateFile.

@taruti
Copy link
Contributor Author

taruti commented Jun 7, 2017

Also changed from a folder to a file as per CR and moved the one comment around.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants