Skip to content

Commit 8d166f7

Browse files
LaurenceJJonesoschwartz10612
authored andcommitted
fix(windows): Use ACL instead of chmod
Golang chmod function on windows does not alter ACL's to be 'user only read' it simply changes the read permissions, instead we must use specific windows callouts to set the permissions to be user only preventing ssh from complaining about weak permissions
1 parent ebf9046 commit 8d166f7

1 file changed

Lines changed: 59 additions & 3 deletions

File tree

cmd/ssh/runner_exec_windows.go

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"os"
99
"os/exec"
1010
"strconv"
11+
12+
"golang.org/x/sys/windows"
1113
)
1214

1315
// execSSHSearchPaths are fallback locations for the ssh executable on Windows.
@@ -77,6 +79,59 @@ func RunExec(opts RunOpts) (int, error) {
7779
return 0, nil
7880
}
7981

82+
// setWindowsFileOwnerOnly sets the file's ACL so that only the current user has access.
83+
// This is required for SSH private keys on Windows, as OpenSSH checks that the key
84+
// is not accessible by other users.
85+
func setWindowsFileOwnerOnly(path string) error {
86+
// Get the current process token to find the user's SID
87+
var token windows.Token
88+
proc := windows.CurrentProcess()
89+
err := windows.OpenProcessToken(proc, windows.TOKEN_QUERY, &token)
90+
if err != nil {
91+
return err
92+
}
93+
defer token.Close()
94+
95+
// Get the token user (contains the SID)
96+
tokenUser, err := token.GetTokenUser()
97+
if err != nil {
98+
return err
99+
}
100+
userSID := tokenUser.User.Sid
101+
102+
// Build an explicit access entry for the current user only (full control)
103+
access := []windows.EXPLICIT_ACCESS{
104+
{
105+
AccessPermissions: windows.GENERIC_ALL,
106+
AccessMode: windows.SET_ACCESS,
107+
Inheritance: windows.NO_INHERITANCE,
108+
Trustee: windows.TRUSTEE{
109+
TrusteeForm: windows.TRUSTEE_IS_SID,
110+
TrusteeType: windows.TRUSTEE_IS_USER,
111+
TrusteeValue: windows.TrusteeValueFromSID(userSID),
112+
},
113+
},
114+
}
115+
116+
// Create a new ACL with only our access entry using the public API
117+
acl, err := windows.ACLFromEntries(access, nil)
118+
if err != nil {
119+
return err
120+
}
121+
122+
// Set the security info: owner + DACL, with PROTECTED_DACL to block inheritance
123+
secInfo := windows.SECURITY_INFORMATION(windows.OWNER_SECURITY_INFORMATION | windows.DACL_SECURITY_INFORMATION | windows.PROTECTED_DACL_SECURITY_INFORMATION)
124+
return windows.SetNamedSecurityInfo(
125+
path,
126+
windows.SE_FILE_OBJECT,
127+
secInfo,
128+
userSID,
129+
nil,
130+
acl,
131+
nil,
132+
)
133+
}
134+
80135
func writeExecKeyFilesWindows(opts RunOpts) (keyPath, certPath string, cleanup func(), err error) {
81136
if opts.PrivateKeyPEM == "" {
82137
return "", "", nil, errors.New("private key required (JIT flow)")
@@ -90,12 +145,13 @@ func writeExecKeyFilesWindows(opts RunOpts) (keyPath, certPath string, cleanup f
90145
os.Remove(keyFile.Name())
91146
return "", "", nil, err
92147
}
93-
if err := keyFile.Chmod(0o600); err != nil {
94-
keyFile.Close()
148+
if err := keyFile.Close(); err != nil {
95149
os.Remove(keyFile.Name())
96150
return "", "", nil, err
97151
}
98-
if err := keyFile.Close(); err != nil {
152+
153+
// Set Windows ACL to restrict access to only the current user
154+
if err := setWindowsFileOwnerOnly(keyFile.Name()); err != nil {
99155
os.Remove(keyFile.Name())
100156
return "", "", nil, err
101157
}

0 commit comments

Comments
 (0)