-
Couldn't load subscription status.
- Fork 106
sockets: implement WithAdditionalUsersAndGroups for windows #139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
90df6c7 to
91e50f6
Compare
|
Looks like the tests were skipped on Windows; |
91e50f6 to
6e7bde7
Compare
|
951cd84 to
2ea4420
Compare
| socketPath := filepath.Join(os.TempDir(), "test.sock") | ||
| defer func() { _ = os.Remove(socketPath) }() | ||
|
|
||
| l, err := NewUnixSocket(socketPath, []string{group}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Progress" previously we got;
=== RUN TestNewUnixSocket
unix_socket_windows_test.go:32: listen unix /tmp/test.sock: bind: A socket operation encountered a dead network.
I changed the fixed /tmp/test.sock to use os.TempDir(), and now I get;
=== RUN TestNewUnixSocket
unix_socket_windows_test.go:88: The parameter is incorrect.
Possibly the path is too long? Or must it use unix conventions (/somewhere/foo.sock), and not a Windows file path (C:\somewhere\foo.sock)?
2ea4420 to
a465fac
Compare
|
UGH back to the drawing board; some digging (to be verified) that unix sockets on Windows are always virtual, so there's no file, and no permissions to set. So there's no option to control access? |
|
Hm... so AI was dreaming that up; https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
|
| return windows.SetNamedSecurityInfo( | ||
| path, | ||
| windows.SE_FILE_OBJECT, | ||
| windows.DACL_SECURITY_INFORMATION|windows.OWNER_SECURITY_INFORMATION, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From debugging, if we are not specifying an owner then we should exclude OWNER_SECURITY_INFORMATION from security information here.
The thing I was unsure is if this is additive or do we need to apply the owner/group from the security descriptor?
| windows.SE_FILE_OBJECT, | ||
| windows.DACL_SECURITY_INFORMATION|windows.OWNER_SECURITY_INFORMATION, | ||
| nil, // do not change the owner | ||
| nil, // do not change the owner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*group
- Implement a WithAdditionalUsersAndGroups (windows daemon allows specifying multiple additional users and groups for named pipes and unix-sockets). - Implement a WithBasePermissions() option for windows - Implement NewUnixSocket that accepts (optional) additional users and groups. Signed-off-by: Sebastiaan van Stijn <[email protected]>
a465fac to
7c22d0d
Compare
specifying multiple additional users and groups for named pipes
and unix-sockets).
and groups.
Partially based on https://github.com/moby/moby/blob/6b45c76a233b1b8b56465f76c21c09fd7920e82d/daemon/listeners/listeners_windows.go#L53-L80
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)