From 5f04517c483203e50e78b5d70d1c31cb0f912a43 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 18 Jun 2021 09:53:50 +0200 Subject: [PATCH 1/3] pkg/system: remove deprecated GetOSVersion(), consts, SecurityInfo utils. This removes the deprecated wrappers, so that the package no longer has hcsshim as a dependency. These wrappers were no longer used in our code, and were deprecated in the 20.10 release (giving external consumers to replace the deprecated ones). Note that there are two consts which were unused, but for which there is no replacement in golang.org/x/sys; const ( PROCESS_TRUST_LABEL_SECURITY_INFORMATION = 0x00000080 ACCESS_FILTER_SECURITY_INFORMATION = 0x00000100 ) PROCESS_TRUST_LABEL_SECURITY_INFORMATION is documented as "reserved", and I could not find clear documentation about ACCESS_FILTER_SECURITY_INFORMATION, so not sure if they must be included in golang.org/x/sys: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-dtyp/23e75ca3-98fd-4396-84e5-86cd9d40d343 Signed-off-by: Sebastiaan van Stijn --- pkg/system/syscall_windows.go | 86 ++++------------------------------- 1 file changed, 9 insertions(+), 77 deletions(-) diff --git a/pkg/system/syscall_windows.go b/pkg/system/syscall_windows.go index 1588aa3ef9..6fca733b18 100644 --- a/pkg/system/syscall_windows.go +++ b/pkg/system/syscall_windows.go @@ -1,47 +1,12 @@ package system // import "github.com/docker/docker/pkg/system" import ( - "syscall" "unsafe" - "github.com/Microsoft/hcsshim/osversion" "github.com/sirupsen/logrus" "golang.org/x/sys/windows" ) -const ( - OWNER_SECURITY_INFORMATION = windows.OWNER_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.OWNER_SECURITY_INFORMATION - GROUP_SECURITY_INFORMATION = windows.GROUP_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.GROUP_SECURITY_INFORMATION - DACL_SECURITY_INFORMATION = windows.DACL_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.DACL_SECURITY_INFORMATION - SACL_SECURITY_INFORMATION = windows.SACL_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.SACL_SECURITY_INFORMATION - LABEL_SECURITY_INFORMATION = windows.LABEL_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.LABEL_SECURITY_INFORMATION - ATTRIBUTE_SECURITY_INFORMATION = windows.ATTRIBUTE_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.ATTRIBUTE_SECURITY_INFORMATION - SCOPE_SECURITY_INFORMATION = windows.SCOPE_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.SCOPE_SECURITY_INFORMATION - PROCESS_TRUST_LABEL_SECURITY_INFORMATION = 0x00000080 - ACCESS_FILTER_SECURITY_INFORMATION = 0x00000100 - BACKUP_SECURITY_INFORMATION = windows.BACKUP_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.BACKUP_SECURITY_INFORMATION - PROTECTED_DACL_SECURITY_INFORMATION = windows.PROTECTED_DACL_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.PROTECTED_DACL_SECURITY_INFORMATION - PROTECTED_SACL_SECURITY_INFORMATION = windows.PROTECTED_SACL_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.PROTECTED_SACL_SECURITY_INFORMATION - UNPROTECTED_DACL_SECURITY_INFORMATION = windows.UNPROTECTED_DACL_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.UNPROTECTED_DACL_SECURITY_INFORMATION - UNPROTECTED_SACL_SECURITY_INFORMATION = windows.UNPROTECTED_SACL_SECURITY_INFORMATION // Deprecated: use golang.org/x/sys/windows.UNPROTECTED_SACL_SECURITY_INFORMATION -) - -const ( - SE_UNKNOWN_OBJECT_TYPE = windows.SE_UNKNOWN_OBJECT_TYPE // Deprecated: use golang.org/x/sys/windows.SE_UNKNOWN_OBJECT_TYPE - SE_FILE_OBJECT = windows.SE_FILE_OBJECT // Deprecated: use golang.org/x/sys/windows.SE_FILE_OBJECT - SE_SERVICE = windows.SE_SERVICE // Deprecated: use golang.org/x/sys/windows.SE_SERVICE - SE_PRINTER = windows.SE_PRINTER // Deprecated: use golang.org/x/sys/windows.SE_PRINTER - SE_REGISTRY_KEY = windows.SE_REGISTRY_KEY // Deprecated: use golang.org/x/sys/windows.SE_REGISTRY_KEY - SE_LMSHARE = windows.SE_LMSHARE // Deprecated: use golang.org/x/sys/windows.SE_LMSHARE - SE_KERNEL_OBJECT = windows.SE_KERNEL_OBJECT // Deprecated: use golang.org/x/sys/windows.SE_KERNEL_OBJECT - SE_WINDOW_OBJECT = windows.SE_WINDOW_OBJECT // Deprecated: use golang.org/x/sys/windows.SE_WINDOW_OBJECT - SE_DS_OBJECT = windows.SE_DS_OBJECT // Deprecated: use golang.org/x/sys/windows.SE_DS_OBJECT - SE_DS_OBJECT_ALL = windows.SE_DS_OBJECT_ALL // Deprecated: use golang.org/x/sys/windows.SE_DS_OBJECT_ALL - SE_PROVIDER_DEFINED_OBJECT = windows.SE_PROVIDER_DEFINED_OBJECT // Deprecated: use golang.org/x/sys/windows.SE_PROVIDER_DEFINED_OBJECT - SE_WMIGUID_OBJECT = windows.SE_WMIGUID_OBJECT // Deprecated: use golang.org/x/sys/windows.SE_WMIGUID_OBJECT - SE_REGISTRY_WOW64_32KEY = windows.SE_REGISTRY_WOW64_32KEY // Deprecated: use golang.org/x/sys/windows.SE_REGISTRY_WOW64_32KEY -) - const ( SeTakeOwnershipPrivilege = "SeTakeOwnershipPrivilege" ) @@ -52,18 +17,11 @@ const ( ) var ( - ntuserApiset = windows.NewLazyDLL("ext-ms-win-ntuser-window-l1-1-0") - modadvapi32 = windows.NewLazySystemDLL("advapi32.dll") - procGetVersionExW = modkernel32.NewProc("GetVersionExW") - procSetNamedSecurityInfo = modadvapi32.NewProc("SetNamedSecurityInfoW") - procGetSecurityDescriptorDacl = modadvapi32.NewProc("GetSecurityDescriptorDacl") + ntuserApiset = windows.NewLazyDLL("ext-ms-win-ntuser-window-l1-1-0") + procGetVersionExW = modkernel32.NewProc("GetVersionExW") ) -// OSVersion is a wrapper for Windows version information -// https://msdn.microsoft.com/en-us/library/windows/desktop/ms724439(v=vs.85).aspx -type OSVersion = osversion.OSVersion - -// https://msdn.microsoft.com/en-us/library/windows/desktop/ms724833(v=vs.85).aspx +// https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-osversioninfoexa // TODO: use golang.org/x/sys/windows.OsVersionInfoEx (needs OSVersionInfoSize to be exported) type osVersionInfoEx struct { OSVersionInfoSize uint32 @@ -79,22 +37,18 @@ type osVersionInfoEx struct { Reserve byte } -// GetOSVersion gets the operating system version on Windows. Note that -// dockerd.exe must be manifested to get the correct version information. -// Deprecated: use github.com/Microsoft/hcsshim/osversion.Get() instead -func GetOSVersion() OSVersion { - return osversion.Get() -} - -// IsWindowsClient returns true if the SKU is client +// IsWindowsClient returns true if the SKU is client. It returns false on +// Windows server, or if an error occurred when making the GetVersionExW +// syscall. func IsWindowsClient() bool { osviex := &osVersionInfoEx{OSVersionInfoSize: 284} r1, _, err := procGetVersionExW.Call(uintptr(unsafe.Pointer(osviex))) if r1 == 0 { - logrus.Warnf("GetVersionExW failed - assuming server SKU: %v", err) + logrus.WithError(err).Warn("GetVersionExW failed - assuming server SKU") return false } - const verNTWorkstation = 0x00000001 + // VER_NT_WORKSTATION, see https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-osversioninfoexa + const verNTWorkstation = 0x00000001 // VER_NT_WORKSTATION return osviex.ProductType == verNTWorkstation } @@ -112,25 +66,3 @@ func HasWin32KSupport() bool { // APIs. return ntuserApiset.Load() == nil } - -// Deprecated: use golang.org/x/sys/windows.SetNamedSecurityInfo() -func SetNamedSecurityInfo(objectName *uint16, objectType uint32, securityInformation uint32, sidOwner *windows.SID, sidGroup *windows.SID, dacl *byte, sacl *byte) (result error) { - r0, _, _ := syscall.Syscall9(procSetNamedSecurityInfo.Addr(), 7, uintptr(unsafe.Pointer(objectName)), uintptr(objectType), uintptr(securityInformation), uintptr(unsafe.Pointer(sidOwner)), uintptr(unsafe.Pointer(sidGroup)), uintptr(unsafe.Pointer(dacl)), uintptr(unsafe.Pointer(sacl)), 0, 0) - if r0 != 0 { - result = syscall.Errno(r0) - } - return -} - -// Deprecated: uses golang.org/x/sys/windows.SecurityDescriptorFromString() and golang.org/x/sys/windows.SECURITY_DESCRIPTOR.DACL() -func GetSecurityDescriptorDacl(securityDescriptor *byte, daclPresent *uint32, dacl **byte, daclDefaulted *uint32) (result error) { - r1, _, e1 := syscall.Syscall6(procGetSecurityDescriptorDacl.Addr(), 4, uintptr(unsafe.Pointer(securityDescriptor)), uintptr(unsafe.Pointer(daclPresent)), uintptr(unsafe.Pointer(dacl)), uintptr(unsafe.Pointer(daclDefaulted)), 0, 0) - if r1 == 0 { - if e1 != 0 { - result = e1 - } else { - result = syscall.EINVAL - } - } - return -} From 26f5db7a1da0223c7584492d6e9bdf1c7b81278b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 18 Jun 2021 10:13:24 +0200 Subject: [PATCH 2/3] pkg/system: remove unused system.Unmount() utility On Linux/Unix it was just a thin wrapper for unix.Unmount(), and a no-op on Windows. This function was not used anywhere (also not externally), so removing this without deprecating first. Signed-off-by: Sebastiaan van Stijn --- pkg/system/syscall_unix.go | 11 ----------- pkg/system/syscall_windows.go | 6 ------ 2 files changed, 17 deletions(-) delete mode 100644 pkg/system/syscall_unix.go diff --git a/pkg/system/syscall_unix.go b/pkg/system/syscall_unix.go deleted file mode 100644 index 905d10f153..0000000000 --- a/pkg/system/syscall_unix.go +++ /dev/null @@ -1,11 +0,0 @@ -// +build linux freebsd - -package system // import "github.com/docker/docker/pkg/system" - -import "golang.org/x/sys/unix" - -// Unmount is a platform-specific helper function to call -// the unmount syscall. -func Unmount(dest string) error { - return unix.Unmount(dest, 0) -} diff --git a/pkg/system/syscall_windows.go b/pkg/system/syscall_windows.go index 6fca733b18..0708e32595 100644 --- a/pkg/system/syscall_windows.go +++ b/pkg/system/syscall_windows.go @@ -52,12 +52,6 @@ func IsWindowsClient() bool { return osviex.ProductType == verNTWorkstation } -// Unmount is a platform-specific helper function to call -// the unmount syscall. Not supported on Windows -func Unmount(_ string) error { - return nil -} - // HasWin32KSupport determines whether containers that depend on win32k can // run on this machine. Win32k is the driver used to implement windowing. func HasWin32KSupport() bool { From 46c591b045a77b7f0f96f96a51d8304264685929 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 18 Jun 2021 10:37:04 +0200 Subject: [PATCH 3/3] pkg/system: deprecate some consts and move them to pkg/idtools These consts were used in combination with idtools utilities, which makes it a more logical location for these consts to live. Signed-off-by: Sebastiaan van Stijn --- builder/dockerfile/copy_windows.go | 3 +-- builder/dockerfile/internals_windows.go | 5 ++--- pkg/idtools/idtools_windows.go | 9 +++++++++ pkg/system/syscall_windows.go | 5 ++++- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/builder/dockerfile/copy_windows.go b/builder/dockerfile/copy_windows.go index 77cd866437..83640ebf42 100644 --- a/builder/dockerfile/copy_windows.go +++ b/builder/dockerfile/copy_windows.go @@ -43,8 +43,7 @@ func fixPermissionsReexec() { } func fixPermissionsWindows(source, destination, SID string) error { - - privileges := []string{winio.SeRestorePrivilege, system.SeTakeOwnershipPrivilege} + privileges := []string{winio.SeRestorePrivilege, idtools.SeTakeOwnershipPrivilege} err := winio.EnableProcessPrivileges(privileges) if err != nil { diff --git a/builder/dockerfile/internals_windows.go b/builder/dockerfile/internals_windows.go index f4e18c9a4e..0d26e69ef3 100644 --- a/builder/dockerfile/internals_windows.go +++ b/builder/dockerfile/internals_windows.go @@ -11,7 +11,6 @@ import ( "github.com/docker/docker/api/types/mount" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/jsonmessage" - "github.com/docker/docker/pkg/system" "golang.org/x/sys/windows" ) @@ -44,10 +43,10 @@ func getAccountIdentity(builder *Builder, accountName string, ctrRootPath string // Check if the account name is one unique to containers. if strings.EqualFold(accountName, "ContainerAdministrator") { - return idtools.Identity{SID: system.ContainerAdministratorSidString}, nil + return idtools.Identity{SID: idtools.ContainerAdministratorSidString}, nil } else if strings.EqualFold(accountName, "ContainerUser") { - return idtools.Identity{SID: system.ContainerUserSidString}, nil + return idtools.Identity{SID: idtools.ContainerUserSidString}, nil } // All other lookups failed, so therefore determine if the account in diff --git a/pkg/idtools/idtools_windows.go b/pkg/idtools/idtools_windows.go index 35ede0fffa..0f5aadd496 100644 --- a/pkg/idtools/idtools_windows.go +++ b/pkg/idtools/idtools_windows.go @@ -6,6 +6,15 @@ import ( "github.com/docker/docker/pkg/system" ) +const ( + SeTakeOwnershipPrivilege = "SeTakeOwnershipPrivilege" +) + +const ( + ContainerAdministratorSidString = "S-1-5-93-2-1" + ContainerUserSidString = "S-1-5-93-2-2" +) + // This is currently a wrapper around MkdirAll, however, since currently // permissions aren't set through this path, the identity isn't utilized. // Ownership is handled elsewhere, but in the future could be support here diff --git a/pkg/system/syscall_windows.go b/pkg/system/syscall_windows.go index 0708e32595..afebed74d7 100644 --- a/pkg/system/syscall_windows.go +++ b/pkg/system/syscall_windows.go @@ -8,12 +8,15 @@ import ( ) const ( + // Deprecated: use github.com/docker/pkg/idtools.SeTakeOwnershipPrivilege SeTakeOwnershipPrivilege = "SeTakeOwnershipPrivilege" ) const ( + // Deprecated: use github.com/docker/pkg/idtools.ContainerAdministratorSidString ContainerAdministratorSidString = "S-1-5-93-2-1" - ContainerUserSidString = "S-1-5-93-2-2" + // Deprecated: use github.com/docker/pkg/idtools.ContainerUserSidString + ContainerUserSidString = "S-1-5-93-2-2" ) var (