Skip to content

sockets: implement WithAdditionalUsersAndGroups for windows#139

Closed
thaJeztah wants to merge 1 commit intodocker:mainfrom
thaJeztah:windows_unix_socket_permissions
Closed

sockets: implement WithAdditionalUsersAndGroups for windows#139
thaJeztah wants to merge 1 commit intodocker:mainfrom
thaJeztah:windows_unix_socket_permissions

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Aug 4, 2025 •
edited
Loading

  • Implement a WithAdditionalUsersAndGroups (windows daemon allows
    specifying multiple additional users and groups for named pipes
    and unix-sockets).
  • Implement NewUnixSocket that accepts (optional) additional users
    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)

@thaJeztah thaJeztah force-pushed the windows_unix_socket_permissions branch from 90df6c7 to 91e50f6 Compare August 4, 2025 21:59
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Aug 4, 2025

Looks like the tests were skipped on Windows;

--- PASS: TestInmemSocket (0.00s)
=== RUN   TestUnixSocketWithOpts
--- PASS: TestUnixSocketWithOpts (0.00s)
=== RUN   TestNewUnixSocket
    unix_socket_windows_test.go:20: requires root
--- SKIP: TestNewUnixSocket (0.00s)
=== RUN   TestNewUnixSocketUnknownGroup
    unix_socket_windows_test.go:36: requires root
--- SKIP: TestNewUnixSocketUnknownGroup (0.00s)
PASS
ok  	github.com/docker/go-connections/sockets	0.026s
=== RUN   TestConfigServerTLSFailsIfUnableToLoadCerts

@thaJeztah thaJeztah force-pushed the windows_unix_socket_permissions branch from 91e50f6 to 6e7bde7 Compare August 5, 2025 09:20
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Aug 5, 2025 

=== RUN   TestNewUnixSocket
    unix_socket_windows_test.go:32: listen unix /tmp/test.sock: bind: A socket operation encountered a dead network.

@thaJeztah thaJeztah force-pushed the windows_unix_socket_permissions branch 3 times, most recently from 951cd84 to 2ea4420 Compare August 5, 2025 09:48
thaJeztah
thaJeztah commented Aug 5, 2025
View reviewed changes
Comment thread sockets/unix_socket_windows_test.go
Comment on lines +83 to +86
socketPath := filepath.Join(os.TempDir(), "test.sock")
defer func() { _ = os.Remove(socketPath) }()

l, err := NewUnixSocket(socketPath, []string{group})
Copy link
Copy Markdown
Member Author

@thaJeztah thaJeztah Aug 5, 2025 •
edited
Loading

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)?

@thaJeztah thaJeztah force-pushed the windows_unix_socket_permissions branch from 2ea4420 to a465fac Compare August 5, 2025 10:09
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Aug 5, 2025

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?

@thaJeztah thaJeztah mentioned this pull request Aug 5, 2025
Merged
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Aug 5, 2025

Hm... so AI was dreaming that up; https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/

Unix sockets provide a mechanism for secure communication. Communication over unix sockets can be secured by controlling the file (or directory) permissions on the pathname sockets (or the parent directory). For example, the bind socket API creates a ‘socket’ file with the given pathname. The creation of the new socket file will fail if the calling process does not has write permission on the directory where the file is being created. Similarly, for connecting to a stream socket, the connecting process should have write permission on the socket. The same level of security is available and enforced on the Windows unix socket implementation. See the man page on AF_UNIX for more details on the security.

austinvazquez
austinvazquez reviewed Aug 6, 2025
View reviewed changes
Comment thread sockets/unix_socket_windows.go
return windows.SetNamedSecurityInfo(
path,
windows.SE_FILE_OBJECT,
windows.DACL_SECURITY_INFORMATION|windows.OWNER_SECURITY_INFORMATION,
Copy link
Copy Markdown
Contributor

@austinvazquez austinvazquez Aug 6, 2025 •
edited
Loading

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?

austinvazquez
austinvazquez reviewed Aug 6, 2025
View reviewed changes
Comment thread sockets/unix_socket_windows.go
windows.SE_FILE_OBJECT,
windows.DACL_SECURITY_INFORMATION|windows.OWNER_SECURITY_INFORMATION,
nil, // do not change the owner
nil, // do not change the owner
Copy link
Copy Markdown
Contributor

@austinvazquez austinvazquez Aug 6, 2025

Choose a reason for hiding this comment

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

*group

@thaJeztah
sockets: implement WithAdditionalUsersAndGroups for windows
7c22d0d 
- 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 <github@gone.nl>
@thaJeztah thaJeztah force-pushed the windows_unix_socket_permissions branch from a465fac to 7c22d0d Compare October 28, 2025 07:49
@chelnak chelnak mentioned this pull request Oct 28, 2025
Merged
@vvoland vvoland closed this in #149 Dec 1, 2025
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Dec 1, 2025

superseded by #149

@thaJeztah thaJeztah deleted the windows_unix_socket_permissions branch December 1, 2025 13:30
@thaJeztah thaJeztah mentioned this pull request Feb 19, 2026
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@austinvazquez austinvazquez austinvazquez left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@thaJeztah @austinvazquez