Windows EmptyDir volumes are not created with 0777

[benmoss]

What happened:
Running a Windows pod with an EmptyDir volume attached should allow a non-Administrator user to read, write, and execute files inside that directory. Instead, it appears that this currently only works if the host's filesystem has ACLs set on a parent directory that set these permissions on all subdirectories. Basically the equivalent of setting 0777 on all of /var/lib/kubelet or something like that.

What you expected to happen:
EmptyDir directories should be created with 0777 permissions, just like on Linux.

How to reproduce it (as minimally and precisely as possible):

• On a Windows host where BUILTIN\Users do not have write permission over /var/lib/kubelet/, run this spec: https://gist.github.com/benmoss/cfea81267982d5d17c4c7ed2a58bf541
• See that the touch /cache/foo line does not emit touch: /cache/foo: Permission denied in the logs

/sig windows

[benmoss]

Relevant context:

• Support emptydir volumes for containers running as non-root #9384
• Remove ContainerUser from busybox kubernetes-sigs/windows-testing#35

[benmoss]

Seems like at least part of the problem is that Mkdir on Windows ignores the file mode, since Windows permissions are a bit different: https://github.com/golang/sys/blob/70f52985063808f21fd6dd197895cdb9729443a5/windows/syscall_windows.go#L433-L439

[yujuhong]

Seems like at least part of the problem is that Mkdir on Windows ignores the file mode, since Windows permissions are a bit different: https://github.com/golang/sys/blob/70f52985063808f21fd6dd197895cdb9729443a5/windows/syscall_windows.go#L433-L439

:-(

Ref: code to set permission to 777 in emptydir

kubernetes/pkg/volume/emptydir/empty_dir.go

Line 41 in 4706389

const perm os.FileMode = 0777

/cc @jingxu97, who ran into similar problems writing tests.

[benmoss]

Started looking at this today, it's more difficult than I imagined. What I mean by "are not created with 0777" is that on Linux 0777 means all users have read/write/execute access. The code in

kubernetes/pkg/volume/emptydir/empty_dir.go

Lines 326 to 361 in f8024ab

	func (ed *emptyDir) setupDir(dir string) error {
	// Create the directory if it doesn't already exist.
	if err := os.MkdirAll(dir, perm); err != nil {
	return err
	}
	// stat the directory to read permission bits
	fileinfo, err := os.Lstat(dir)
	if err != nil {
	return err
	}
	if fileinfo.Mode().Perm() != perm.Perm() {
	// If the permissions on the created directory are wrong, the
	// kubelet is probably running with a umask set. In order to
	// avoid clearing the umask for the entire process or locking
	// the thread, clearing the umask, creating the dir, restoring
	// the umask, and unlocking the thread, we do a chmod to set
	// the specific bits we need.
	err := os.Chmod(dir, perm)
	if err != nil {
	return err
	}
	fileinfo, err = os.Lstat(dir)
	if err != nil {
	return err
	}
	if fileinfo.Mode().Perm() != perm.Perm() {
	klog.Errorf("Expected directory %q permissions to be: %s; got: %s", dir, perm.Perm(), fileinfo.Mode().Perm())
	}
	}
	return nil
	}

is running on Windows and isn't erroring, but the problem is that Go's attempt to map Unix permissions to Windows ends up not really matching the intention of this code.

I don't know that there is a super straightforward way to translate something like 0777 to Windows, except maybe putting ACLs on the directory giving BUILTIN\Users the windows.GENERIC_ALL permission.

[yujuhong]

I don't know that there is a super straightforward way to translate something like 0777 to Windows, except maybe putting ACLs on the directory giving BUILTIN\Users the windows.GENERIC_ALL permission.

This is what was done in the stackoverflow answer: https://stackoverflow.com/questions/33445727/how-to-control-file-access-in-windows

[jingxu97]

cc @andyzhangx

Could we fix this issue this the package mentioned by @yujuhong? https://github.com/hectane/go-acl

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[yujuhong]

@benmoss @PatrickLang do you know of a better way to handle this, or would we have to change the ACLs as the comments above suggest?

[benmoss]

I think we would have to use ACLs. I ultimately added these lines just to make these tests pass: https://github.com/cloudfoundry-incubator/kubo-release-windows/blob/0831c11effeafb16f56e347daa229adb10757eeb/jobs/kubelet-windows/templates/bin/kubelet_ctl.ps1.erb#L133-L138

[liyanhui1228]

Tried reproducing this by deploying that pod using the yaml file, and this seems like already been fixed in someway. We could mark this as fixed if it cannot be reproduced any more. cc @yujuhong

Path Owner                       Access
---- -----                       ------
C:\  NT SERVICE\TrustedInstaller CREATOR OWNER Allow  268435456
                                NT AUTHORITY\SYSTEM Allow  FullControl
                                BUILTIN\Administrators Allow  FullControl
                                BUILTIN\Users Allow  AppendData
                                BUILTIN\Users Allow  CreateFiles
                                BUILTIN\Users Allow  ReadAndExecute, Synchronize

PS C:\busybox> get-acl /cache | format-table -wrap

Path  Owner                  Access
----  -----                  ------
cache BUILTIN\Administrators NT AUTHORITY\SYSTEM Allow  FullControl
                             BUILTIN\Administrators Allow  FullControl
                             BUILTIN\Users Allow  ReadAndExecute, Synchronize
                             BUILTIN\Users Allow  AppendData
                             BUILTIN\Users Allow  CreateFiles
                             CREATOR OWNER Allow  268435456

[yujuhong]

@benmoss could you still reproduce the issue?

[benmoss]

No, I'm not sure this is still an issue
/close

[k8s-ci-robot]

@benmoss: Closing this issue.

In response to this:

No, I'm not sure this is still an issue
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.