syscall: use Windows FILE_FLAG_BACKUP_SEMANTICS in remaining places (Open)

[mappu]

Hi,

FILE_FLAG_BACKUP_SEMANTICS is an optional flag to the Windows CreateFile API. It allows accessing files when privilege would be denied by normal Windows ACLs, except for that the user account has the SE_BACKUP_NAME privilege. If the calling user account does not have this privilege, adding this flag has no impact to behaviour nor performance.

Go already uses FILE_FLAG_BACKUP_SEMANTICS in some CreateFile calls (e.g. Readlink, Utimes, UtimesNano, from issues #8090 and #10804) but not in other cases (notably Open). So it's inconsistent.

I would like to request that FILE_FLAG_BACKUP_SEMANTICS is added to the Open call in src/syscall/syscall_windows.go.

As well as fixing the inconsistency, this would allow Go users to open files on disk, in those cases where their permission to do so comes via SE_BACKUP_NAME instead of windows ACLs.

Thanks
mappu

More information:

• https://github.com/giuliano108/SeBackupPrivilege
• https://msdn.microsoft.com/en-us/library/windows/desktop/bb530716.aspx

What version of Go are you using (go version)?

go version go1.9.2 windows/amd64

Does this issue reproduce with the latest release?

Yes

[bradfitz]

/cc @alexbrainman

[alexbrainman]

Go already uses FILE_FLAG_BACKUP_SEMANTICS in some CreateFile calls (e.g. Readlink, Utimes, UtimesNano, from issues #8090 and #10804) but not in other cases (notably Open). So it's inconsistent.

We only use FILE_FLAG_BACKUP_SEMANTICS to open directories and symlinks. There is no way to open them without FILE_FLAG_BACKUP_SEMANTICS flag. We do not use FILE_FLAG_BACKUP_SEMANTICS to open files, because everything we do with files work just fine without FILE_FLAG_BACKUP_SEMANTICS.

I would like to request that FILE_FLAG_BACKUP_SEMANTICS is added to the Open call in src/syscall/syscall_windows.go.

Why? What is broken if we don't do that?

this would allow Go users to open files on disk, in those cases where their permission to do so comes via SE_BACKUP_NAME instead of windows ACLs.

I did not understand this explanation. Please try again.

Thank you.

Alex

[mappu]

I did not understand this explanation. Please try again.

A backup program should be able to back up all files on the system.

But, a backup program must run as a certain Windows user, and it's possible that there are some files that the backup user account does not have permission to access, according to the normal ACL permission system.

Windows has a solution for it: You can open a file if either (A) you have normal ACL permissions to do so, or (B) your user account has SE_BACKUP_NAME privilege and the FILE_FLAG_BACKUP_SEMANTICS flag was passed to CreateFile.

Passing this flag does not change behaviour in the normal case. It only increases the functionality. So, I don't think there's any harm in passing this flag unconditionally.

What is broken if we don't do that?

Right now, it's difficult to write backup programs in Go that take advantage of SE_BACKUP_NAME to access all files on the system.

[alexbrainman]

A backup program should be able to back up all files on the system.

Thank for explaining.

Passing this flag does not change behaviour in the normal case. It only increases the functionality. So, I don't think there's any harm in passing this flag unconditionally.

I am not security expert, so I would not know if there is a downside to FILE_FLAG_BACKUP_SEMANTICS. So I googled around, and I cannot find anything particular for or against that change. Maybe you could provide some reference explaining why it is safe to use FILE_FLAG_BACKUP_SEMANTICS all the time? Without such reference I cannot support your proposal.

Right now, it's difficult to write backup programs in Go that take advantage of SE_BACKUP_NAME to access all files on the system.

You can call syscall.CreateFile directly. Not many people write backup programs, so people who do, would have to jump through the hoops that Microsoft creates. Sorry.

Alex

[mappu]

I made some experiments with patching syscall to add this feature, but actually it breaks a lot of things.
I withdraw my request.

[qmuntal]

I made some experiments with patching syscall to add this feature, but actually it breaks a lot of things.

Unconditionally setting FILE_FLAG_BACKUP_SEMANTICS shouldn't be a problem after CL 405275. Also, note that after that CL, FILE_FLAG_BACKUP_SEMANTICS is always passed when opening existing files with read only access, as there is no other way to open directories with CreateFile. Making that flag unconditional would make syscall.Open more concistent.

Maybe you could provide some reference explaining why it is safe to use FILE_FLAG_BACKUP_SEMANTICS all the time? Without such reference I cannot support your proposal.

FILE_FLAG_BACKUP_SEMANTICS doesn't really skip any ACL check unless the SE_BACKUP_NAME privilege is enabled, which is not by default. From the CreateFileW docs: Appropriate security checks still apply when this flag is used without SE_BACKUP_NAME and SE_RESTORE_NAME privileges.

I'm interested in setting FILE_FLAG_BACKUP_SEMANTICS unconditionally because I plan to use the Root API (see #67002) in services that perform backup tasks. If syscall.Open doesn't unconditionally set FILE_FLAG_BACKUP_SEMANTICS, then the case for Root unconditionally setting FILE_OPEN_FOR_BACKUP_INTENT wouldn't hold. FYI @neild