What did you do?

Compiling source and creating executable

Please, provide the steps that we need to do to reproduce your problem. What commands do we run? Assume we know nothing about this "inheritance of NTFS permissions" subject (because we don't). Thank you.

The created executable is currently breaking the inheritance of NTFS permissions. The resulting executable is restricted to the current user and system. In contrast the standard windows behaviour for newly created files is that the file will always inherit its permissions from its parent folder.

Please, provide some references that support your claim. I do not know anything about the subject, so it is hard for me to judge without some authoritive documentation. Thank you.

Alex

With executables running in different and very restricted user accounts the current behaviour of breaking the inheritance is not helpful.

Can you, please, explain this. What is the exact problem?

Alex

Please, provide the steps that we need to do to reproduce your problem. What commands do we run? Assume we know nothing about this "inheritance of NTFS permissions" subject (because we don't). Thank you.

On Windows the command go build x.go will create an executable file. If the file did exist before then this file will be overwritten. This newly created file has NTFS permissions that will be different from its parent folder because permission inheritance is disabled.

Can you, please, explain this. What is the exact problem?
The problem is the file systems with Access Control Lists are still rarely used on the Linux platform therefore the awareness of developers is not very high. NTFS is one variant of ACL based file systems. Here the inheritance of file permissions is an integral part of respecting the security architecture of its users. With security architecture I mean the specific rights that are applied to directories. With new files inheriting the permissions from the parent folder the security architecture is preserved.

In my case the golang executable is executed from a Windows service. This service runs with least necessary privileges to execute the task. This is done by executing the service in a very limited user account. Under normal circumstances I would just modify the privileges of the parent folder of the exe once to allow this restricted user to always read/execute the files in this folder. The go build would produce an executable that would inherit these permissions. Therefore my service would execute the golang application without problems. But with the current go build and its broken inheritance I have to manually modify the NTFS permissions, add the user and allow him read/execute the created executable.

In contrast the Microsoft Compiler does exatly what I expected the go command too: it will inherit the NTFS permissions from the parent folder where the executable has been created. This inheritance is the standard behaviour of Windows: just create a new file and look at its NTFS permissions - all inherited from the parent folder. Copy an existing file to a new directory with different NTFS permissions - the moved file will now inherit from this parent.

This behaviour prevents to create loopholes in the security architecture of the file system. For example on servers where folders are shared with many users. Here the permissions on the folders will control which user group has access. Files moved or newly created there will inherit the permissions from the parent folder. Therefore their permissions are as expected from the security standpoint. Respecting the NTFS permissions instead of replacing them with your own is a standard procedure recommended in "Writing Secure Code" by Microsoft.

On Windows the command go build x.go will create an executable file. If the file did exist before then this file will be overwritten. This newly created file has NTFS permissions that will be different from its parent folder because permission inheritance is disabled.

I need to see it all for myself. What are the steps for me to see this? How do I see that created file has NTFS permissions that are different from its parent folder. You say "permission inheritance is disabled". Where is it disabled? How do I see it? What tools do I use it to see it?

Thank you

Alex

I suggest the following steps:

• take a simple go project as your base project. You put this project in directory A. Then compile an executable in this directory A.

• now visit the directory A with the windows explorer and open the properties of directory A. Right click on directory A and select properties in the menu. In the tab security you see that the directory A has very specific NTFS permissions.

• in the windows explorer you now click on the executable and select right click > properties as before. Now in the tab security you see the NTFS permissions. There you can see that the groups from the directory are not there. Instead just the current user, system and administrators have access. This is because the inheritance of the NTFS permissions has been disabled for this executable. You can verify this by clicking "Advanced" below. In the following dialog you see the button "Enable Inheritance". This shows you that currently the inhertance of NTFS permissions from the parent directory is indeed disabled - and this is the root of the problem.

• now the point is that the Compiler creates the executable very likely with the CreateFile API function. This function has a parameter called lpSecurityAttributes. The structure you have to pass there is rather complex. It controls which NTFS permissions are applied to the created file: which users/groups have access, which rights they have and if inheritance is enabled. One solution is to at least enable inheritance.

However the solution is much easier: instead of passing a complex and error prone structure to lpSecurityAttributes of CreateFile you just pass NULL. This will create an executable file that inherits all NTFS permissions from its parent directory A - and since the current user can already read and write in directory A - otherwise he could not edit files there - the inherited permissions will also allow the created file to be executed. This way NTFS permissions are preserved and respected.

Now you could argue: what if the user is compiling to a directory where the NTFS permissions of the parent folder would not allow the file to be executed? If this is an concern then at least enable inheritance in the more complex lpSecurityAttributes you are already passing. This way the file will inherit from the parent and in addition the rights from the lpSecurityAttributes are applied for the current user, system and administrators.

As far as I can see the compiler does pass nil for the security attributes when opening a file.

I think what is happening is more complicated. The go tool directs the compiler to create the file in a temporary directory. Then the Go tool moves the file into its final location. On Windows this move is done using MoveFileEx. Perhaps that winds up giving the file the wrong permissions.

The relevant code is Builder.moveOrCopyFile, which on tip is in cmd/go/internal/work/exec.go. That function already has some special cases where Unix systems require a copy rather than a rename. Perhaps we need to add a similar special case on Windows, or perhaps on Windows we must always copy rather than rename.

Under Windows the Windows Explorer will adjust the NTFS permissions if files are moved. But the fundamental API function MoveFileEx will just move the file to the destination folder while the NTFS permissions stay the same. In the temp folder the NTFS permissions are very strict. Therefore the executable will inherit these strict permissions and then it will just be moved to the output folder.

I share your conclusion that a special case would be useful. Your approach to Copy from temp to output folder and then delete in the temp folder will solve the issue.

Is there a way that we can whether NTFS permissions apply to the destination directory, comparable to the test of ModeSetgid on Unix systems? Or do these permissions always apply, and we should always copy on Windows?

In the Windows world it is unusual to allow read but not read/execute for users/groups in the folder permissions. Without read/execute granted you can not even list the folder content. Of course from the standpoint of pure NTFS permissions it is possible to do that restriction. However this is a choice by the Admin of the folder and this should be respected. In my opinion it would be wrong to ignore the folder settings and repair the executable to read/execute on your own. Mainly because you would speculate which users you would allow these permissions. You would have to assume that the current user, the system and the admins should be able to execute the file. However you do not know for sure what the folder admin intended here. You could also read the users/groups from the parent folder, enable inheritance and add the NTFS permission read/execute to the executable for all these users/groups. However I would not recommend these NTFS modifications at all.

Better always copy the executable and put it in the responsibility of the NTFS permissions of the target folder to allow this file to be executed. In most cases the executable will run because it has inherited read and read/execute permissions (Standard case in Windows file system).

In the rare case it can not run due to permissions it was the intentional choice of the folder admin. By one adjustment to the folder NTFS permissions the folder admin can then allow read/execute for all files inheriting from this folder. So the fix in this rare case is quite obvious. To have a rare case that needs manual attention is better than disrespecting the existing NTFS permissions of the target folder. Admins often invest a lot of time into their folder structure and their associated NTFS permissions. Same is true for other ACL based file systems like ZFS on linux or solaris.

Thanks. You didn't really answer my question, though. Perhaps the question doesn't make sense.

Is there a way that we can tell whether NTFS permissions apply to the destination directory, comparable to the test of ModeSetgid on Unix systems? Or do these permissions always apply, and we should always copy on Windows?

The copied file will always inherit from the parent that is for sure. However what it will inherit depends on NTFS permissions of the parent folder. An untouched folder will itself inherit from its parent and so forth. Of course the destination folder could also disable inheritance. It could also set permissions read/execute and forbidden read/execute at the same time. In this case the forbidden permissions will overrule the allowed permissions. It is also possible to remove all permissions and leave them empty. But this is all on purpose by the folder admin. If he wanted the permission list to be empty then the executable copied there will inherit this empty permission list. Always copy is at least doing what the owner/admin of the folder intended.

To proof this I did a test with Visual Studio 2017 and created a small console application. I have set the following permissions for the "debug" folder: my user read, write, list folder. In the compilation process Visual Studio warned about missing access rights to the output folder. Still the executable has been created and it inherited the rights from the "debug" folder. The read/execute was not added by the compiler and therefore the resulting executable is not executable. The warning of the compiler about the access rights just informs about these missing read/execute rights. Here Microsoft is serious about not modifying or repairing NTFS permissions. If you copy from temp to a folder with insufficient permissions you will produce the same result. From my standpoint the correct approach.

Change https://golang.org/cl/72910 mentions this issue: cmd/go: always copy files on Windows

I suggest the following steps:

I had some free time, so I tried to follow your description.

take a simple go project as your base project. You put this project in directory A. Then compile an executable in this directory A.

Done. I used "go build" command to build simple "hello world" program.

now visit the directory A with the windows explorer and open the properties of directory A. Right click on directory A and select properties in the menu. In the tab security you see that the directory A has very specific NTFS permissions.

Done. I am not sure why you tell me about "very specific NTFS permissions" and what these are. But I followed your other steps.

now visit the directory A with the windows explorer and open the properties of directory A. Right click on directory A and select properties in the menu. In the tab security you see that the directory A has very specific NTFS permissions.

Same as above. I followed the steps, but I have no idea about "very specific NTFS permissions".

in the windows explorer you now click on the executable and select right click properties as before. Now in the tab security you see the NTFS permissions.

Done.

There you can see that the groups from the directory are not there.

I am not sure what you mean by that. In my directory "group or user names" I see SYSTEM, Alex and Administrators. Same with my executable file, I see SYSTEM, Alex and Administrators.

Instead just the current user, system and administrators have access.

That is what I see for both directory and executable file.

This is because the inheritance of the NTFS permissions has been disabled for this executable. You can verify this by clicking "Advanced" below. In the following dialog you see the button "Enable Inheritance".

I don't see "Enable Inheritance" button, I see "Disable Inheritance" button.

This shows you that currently the inhertance of NTFS permissions from the parent directory is indeed disabled - and this is the root of the problem.

Well, I cannot reproduce your problem here. Why?

However the solution is much easier: instead of passing a complex and error prone structure to lpSecurityAttributes of CreateFile you just pass NULL.

That is what we do already. Except when we call syscall.Open without syscall.O_CLOEXEC flag. But lets try and understand the problem before we look for the solution.

As far as I can see the compiler does pass nil for the security attributes when opening a file.

We don't pass nil for the security attribute for syscall.Open without syscall.O_CLOEXEC.

On Windows this move is done using MoveFileEx. Perhaps that winds up giving the file the wrong permissions.

Quite possible. But I would like to see the problem first.

Thank you

Alex

This is a good observation, Alex. In a way we are both right.

If the folder with the go source belongs to the current user then the temp folder uses the same permissions. Therefore the resulting executable inherits the permissions from the folder - as you have experienced.

However if the folder with the go source belongs to another location with different permissions than the user temp folder then the file created in users temp has different permissions than the output folder. Therefore the resulting executable has different permissions with broken inheritance. Create a folder c:\test, create test.go there and compile with go build -o test.exe test.go. The resulting test.exe will have inheritance disabled. A copy operation from the users temp to the output folder will fix this.

What's the rationale for not building directly into the target directory? This would solve all problems for all operating systems.

In fact, I think the Linux codepath is also broken or rather incomplete. It only checks for setgid, but for instance ignores POSIX ACLs. With setfacl, you can mandate that all files created within a certain directory have some specific ACLs, using the -d/--default option. So if I run setfacl -dm g:www-data:rwx testdir, all new files created within testdir will automatically be granted a ACL that gives www-data rwx access to them. You can test it by entering testdir creating a file with touch foo, and then seeing that there is an ACL added to it automatically with getfacl foo (or even simply ls -l foo and noticing the + next to the mode bits). But if you mv a file into testdir, the ACL will not be there.

$ mkdir testdir
$ sudo setfacl -dm g:www-data:rwx testdir
$ touch testdir/foo
$ touch bar && mv bar testdir
$ getfacl testdir/foo
# file: testdir/foo
# owner: rasky
# group: rasky
user::rw-
group::rwx			#effective:rw-
group:www-data:rwx		#effective:rw-
mask::rw-
other::r--
$ getfacl testdir/bar
# file: testdir/bar
# owner: rasky
# group: rasky
user::rw-
group::rw-
other::r--

Update: starting with go 1.9.2 the created executable will inherit from its parent directory. So my issue has been fixed. In respect to the remarks of @rasky I leave this issue open for further investigation of POSIX ACLs.

starting with go 1.9.2 the created executable will inherit from its parent directory. So my issue has been fixed.

That is strange. These are the changes between go1.9.1 and go1.9.2:

2ea7d34 (tag: go1.9.2) go1.9.2
d93cb46 runtime: use simple, more robust fastrandn
78952c0 cmd/compile: fix sign-extension merging rules
79996e4 cmd/compile: avoid generating large offsets
f36b126 runtime: in cpuProfile.addExtra, set p.lostExtra to 0 after flush
dffc931 cmd/cgo: support large unsigned macro again
33ce168 cmd/cgo: avoid using common names for sniffing
f69668e os: skip TestPipeThreads as flaky for 1.9
9be38a1 runtime: avoid monotonic time zero on systems with low-res timers
8bb333a doc: document Go 1.9.2
0758d2b cmd/go: clean up x.exe properly in TestImportMain
d487b15 cmd/compile: omit ICE diagnostics after normal error messages
fd17253 database/sql: prevent race in driver by locking dc in Next
7e7cb30 internal/poll: only call SetFileCompletionNotificationModes for sockets
f259aed internal/poll: do not call SetFileCompletionNotificationModes if it is broken
39d4bb9 cmd/go: correct directory used in checkNestedVCS test
bfc2231 crypto/x509: reject intermediates with unknown critical extensions.
a1e34ab net/smtp: NewClient: set tls field to true when already using a TLS connection
7dadd8d net: increase expected time to dial a closed port on all Darwin ports
d808893 cmd/compile: fix merge rules for panic calls
87b3a27 net: bump TestDialerDualStackFDLeak timeout on iOS
ebfcdef runtime: make runtime.GC() trigger GC even if GOGC=off
0ab99b3 cmd/compile: fix regression in PPC64.rules move zero
8d4279c internal/poll: be explicit when using runtime netpoller
1ded833 cmd/compile/internal/syntax: fix source buffer refilling
ff8289f reflect: fix pointer past-the-end in Call with zero-sized return value
bd34e74 log: fix data race on log.Output
0b55d8d cmd/compile: replace GOROOT in //line directives
5c48811 cmd/compile: limit the number of simultaneously opened files to avoid EMFILE/ENFILE errors
8c7fa95 expvar: make (*Map).Init clear existing keys
ccd5abc cmd/compile: simplify "missing function body" error message
2e4358c time: fix documentation of Round, Truncate behavior for d <= 0
c6388d3 runtime: capture runtimeInitTime after nanotime is initialized
724638c crypto/x509: skip TestSystemRoots
ed3b0d6 internal/poll: add tests for Windows file and serial ports
93322a5 doc: add missing "Minor revisions" header for 1.9

I don't see which change might fix your problem. @steamonimo could you try and "git bisect" the change that helps? Thank you.

Alex

Hello Alex, I managed to create 3 executables that had permission inheritance enabled. But the following executable had more changes and this time the compiled executable had its permission inheritance disabled again. So my claim that the issue has been fixed with 1.9.2 stands corrected. I am not sure what to make of this.

@rsc See @rasky's comment above (#22343 (comment)). We could change the go tool so that go build and go install link the executables into a temporary file in the target directory, then rename to the final location within the same directory. That would slightly simplify moveOrCopyFile and avoid problems with unusual directory-specific permissions on Windows and some Unix systems.

I can't think of a good reason to not do that. Is there one?

Create a folder c:\test, create test.go there and compile with go build -o test.exe test.go. The resulting test.exe will have inheritance disabled.

I could reproduce that, thank you very much. I suspect the problem is that we use Windows MoveFileEx to move executable file to its destination. I could reproduce the problem just by using Windows

move %TMP%\a c:\alex\a

command. If I replace this with

copy %TMP%\a c:\alex\a

then c:\alex\a inherits all security info from c:\alex.

So Ian's CL 72910 (that replaces moving file with copying on Windows) should do the trick. But I would wait for @rsc to to make decision on a general / all OSes approach.

I also don't like CL 72910 because, I suspect, it will slow thing down - copying many MB file instead of rename.

If we have to go with "windows only" solution, it might still better to keep MoveFileEx, but adjust destination file security info instead. See, for example, https://stackoverflow.com/questions/17536692/resetting-file-security-to-inherit-after-a-movefile-operation

Note for myself: It would also be nice to write some test for this - maybe use icacls to check destination file security info and compare it with the source file. I am not sure if that is possible and how.

Alex

I'm always a little worried about writing to files in directories other than the ones we were requested to write, especially for something like the -o flag.

I don't think it will slow things down a ton to do CL 72910 for Go 1.10, and then maybe we can revisit the "temp files in target directory" in Go 1.11.

I'm always a little worried about writing to files in directories other than the ones we were requested to write, especially for something like the -o flag.

Why? Using ioutil.TempFile looks harmless.

Change https://golang.org/cl/78215 mentions this issue: cmd/go: add TestACL

@rasky, I just feel like if you get told write this one file in a directory, you should not go around writing other files first.

I just feel like if you get told write this one file in a directory, you should not go around writing other files first.

I agree with your sentiment. But what I see at this moment is

c:\Users\Alex\dev\src\a>type main.go
package main

import (
        "fmt"
)

func main() {
        fmt.Println("Starting.")
}

c:\Users\Alex\dev\src\a>go build -o a.exe -x main.go
WORK=C:\Users\Alex\AppData\Local\Temp\go-build755428318
...
"c:\\users\\alex\\dev\\go\\pkg\\tool\\windows_amd64\\link.exe" -o "C:\\Users\\Alex\\AppData\\Local\\Temp\\go-build755428318\\b001\\exe\\a.out.exe" -importcfg "C:\\Users\\Alex\\AppData\\Local\\Temp\\go-build755428318\\b001\\importcfg.link" -buildmode=exe -buildid=8fL-XXA9ln15SCjinNk2/JVZah3nh1jAULktNnEsZ/csN3z7HORSKYrrZX-T3g/8fL-XXA9ln15SCjinNk2 -extld=gcc "C:\\Users\\Alex\\AppData\\Local\\go-build\\81\\81f8e6e20d012e9de2211bfd0271a51f3d57237f2e1db0e2c9d9bc73024b1e83-d"
"c:\\users\\alex\\dev\\go\\pkg\\tool\\windows_amd64\\buildid.exe" -w "C:\\Users\\Alex\\AppData\\Local\\Temp\\go-build755428318\\b001\\exe\\a.out.exe" # internal
mv $WORK\b001\exe\a.out.exe a.exe
rm -r $WORK\b001\

c:\Users\Alex\dev\src\a>

Why cannot we replace that with

...
WORK=C:\Users\Alex\AppData\Local\Temp\go-build755428318
...
"c:\\users\\alex\\dev\\go\\pkg\\tool\\windows_amd64\\link.exe" -o "a.exe" -importcfg "C:\\Users\\Alex\\AppData\\Local\\Temp\\go-build755428318\\b001\\importcfg.link" -buildmode=exe -buildid=8fL-XXA9ln15SCjinNk2/JVZah3nh1jAULktNnEsZ/csN3z7HORSKYrrZX-T3g/8fL-XXA9ln15SCjinNk2 -extld=gcc "C:\\Users\\Alex\\AppData\\Local\\go-build\\81\\81f8e6e20d012e9de2211bfd0271a51f3d57237f2e1db0e2c9d9bc73024b1e83-d"
"c:\\users\\alex\\dev\\go\\pkg\\tool\\windows_amd64\\buildid.exe" -w "a.exe" # internal
rm -r $WORK\b001\

c:\Users\Alex\dev\src\a>

?

Alex

@steamonimo can you test this version of Go https://go-review.googlesource.com/#/c/go/+/72910/ to see if it fixes your problem? Let us know if you need more instructions.

Thank you.

Alex