From a059ad7ac1d8c0c4b294b46901dd77d648f29db9 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy <84944216+helsaawy@users.noreply.github.com> Date: Mon, 24 Oct 2022 15:53:06 -0400 Subject: [PATCH 1/9] Soften linter (#264) Remove some aggressive linters: `godoc`, `goconst`, `misspel`, `nestif`, and `prealloc. Also remove `stylecheck` since it is subsumed by `revive`, and the later is more configurable. Allow tests and build stages to continue even if linter fails. Signed-off-by: Hamza El-Saawy --- .github/workflows/ci.yml | 1 - .golangci.yml | 12 ++---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 14e43760..5770f932 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,7 +60,6 @@ jobs: test: name: Run Tests needs: - - lint - go-generate runs-on: ${{ matrix.os }} strategy: diff --git a/.golangci.yml b/.golangci.yml index af403bb1..d527ea56 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -8,12 +8,8 @@ linters: - containedctx # struct contains a context - dupl # duplicate code - errname # erorrs are named correctly - - goconst # strings that should be constants - - godot # comments end in a period - - misspell - nolintlint # "//nolint" directives are properly explained - revive # golint replacement - - stylecheck # golint replacement, less configurable than revive - unconvert # unnecessary conversions - wastedassign @@ -23,9 +19,7 @@ linters: - exhaustive # check exhaustiveness of enum switch statements - gofmt # files are gofmt'ed - gosec # security - - nestif # deeply nested ifs - nilerr # returns nil even with non-nil error - - prealloc # slices that can be pre-allocated - structcheck # unused struct fields - unparam # unused function params @@ -56,6 +50,8 @@ issues: linters-settings: + exhaustive: + default-signifies-exhaustive: true govet: enable-all: true disable: @@ -138,7 +134,3 @@ linters-settings: - VPCI - WCOW - WIM - stylecheck: - checks: - - "all" - - "-ST1003" # use revive's var naming From 650a2e464c7458a1d355d7bb5affad1eec1bfbb6 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy <84944216+helsaawy@users.noreply.github.com> Date: Fri, 28 Oct 2022 12:31:56 -0400 Subject: [PATCH 2/9] Update version, remove deprecated linter (#265) `structcheck` is deprecated. Update golanci-lint to 1.50. Signed-off-by: Hamza El-Saawy --- .github/workflows/ci.yml | 2 +- .golangci.yml | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5770f932..bbd32daf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ jobs: - name: Run golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.48 + version: v1.50 args: >- --verbose --timeout=5m diff --git a/.golangci.yml b/.golangci.yml index d527ea56..2bf84e32 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -20,7 +20,6 @@ linters: - gofmt # files are gofmt'ed - gosec # security - nilerr # returns nil even with non-nil error - - structcheck # unused struct fields - unparam # unused function params issues: @@ -94,6 +93,8 @@ linters-settings: disabled: true - name: flag-parameter # excessive, and a common idiom we use disabled: true + - name: unhandled-error # warns over common fmt.Print* and io.Close; rely on errcheck instead + disabled: true # general config - name: line-length-limit arguments: From cbf4dd97820443d8710f3049d6c78a103c1c6874 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 13 Feb 2023 17:25:49 +0100 Subject: [PATCH 3/9] pkg/etw/sample: remove dependency on github.com/sirupsen/logrus (#267) It looks like for the purpose of the example "a" logger was needed, so not strictly logrus (probably `fmt.Println()` would've worked even). This patch removes logrus as dependency for the example. The only remaining use of logrus in this repository is now in the pkg/etc/etwlogrus package, which _does_ need logrus, but (possibly) could become its own module if we want to remove logrus as dependency of go-winio itself. Signed-off-by: Sebastiaan van Stijn --- pkg/etw/sample/main_windows.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/pkg/etw/sample/main_windows.go b/pkg/etw/sample/main_windows.go index c4e4e858..d6b185df 100644 --- a/pkg/etw/sample/main_windows.go +++ b/pkg/etw/sample/main_windows.go @@ -7,12 +7,12 @@ package main import ( "bufio" "fmt" + "log" "os" "runtime" "github.com/Microsoft/go-winio/pkg/etw" "github.com/Microsoft/go-winio/pkg/guid" - "github.com/sirupsen/logrus" ) func callback(sourceID guid.GUID, state etw.ProviderState, level etw.Level, matchAnyKeyword uint64, matchAllKeyword uint64, filterData uintptr) { @@ -24,31 +24,28 @@ func main() { group, err := guid.FromString("12341234-abcd-abcd-abcd-123412341234") if err != nil { - logrus.Error(err) - return + log.Fatal(err) } provider, err := etw.NewProvider("TestProvider", callback) if err != nil { - logrus.Error(err) - return + log.Fatal(err) } defer func() { if err := provider.Close(); err != nil { - logrus.Error(err) + log.Fatal(err) } }() providerWithGroup, err := etw.NewProviderWithOptions("TestProviderWithGroup", etw.WithGroup(group), etw.WithCallback(callback)) if err != nil { - logrus.Error(err) - return + log.Fatal(err) } defer func() { if err := providerWithGroup.Close(); err != nil { - logrus.Error(err) + log.Fatal(err) } }() @@ -80,8 +77,7 @@ func main() { "Item5", })), ); err != nil { - logrus.Error(err) - return + log.Fatal(err) } if err := providerWithGroup.WriteEvent( @@ -104,7 +100,6 @@ func main() { "Item5", })), ); err != nil { - logrus.Error(err) - return + log.Fatal(err) } } From 6a35904fd473650afc0210ff2b93f227db969683 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy <84944216+helsaawy@users.noreply.github.com> Date: Mon, 13 Feb 2023 12:19:14 -0500 Subject: [PATCH 4/9] update dependencies (#277) Signed-off-by: Hamza El-Saawy --- go.mod | 8 ++++---- go.sum | 29 +++++++++++++++++++++-------- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 8cf6b938..ed3a39f7 100644 --- a/go.mod +++ b/go.mod @@ -3,9 +3,9 @@ module github.com/Microsoft/go-winio go 1.17 require ( - github.com/sirupsen/logrus v1.7.0 - golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f - golang.org/x/tools v0.1.12 + github.com/sirupsen/logrus v1.9.0 + golang.org/x/sys v0.5.0 + golang.org/x/tools v0.6.0 ) -require golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect +require golang.org/x/mod v0.8.0 // indirect diff --git a/go.sum b/go.sum index 3583d96f..529f8c59 100644 --- a/go.sum +++ b/go.sum @@ -1,35 +1,48 @@ +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/sirupsen/logrus v1.7.0 h1:ShrD1U9pZB12TX0cVy0DtePoCH97K8EtX+mg7ZARUtM= -github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= -github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w= -github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0= +github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= +golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8= +golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= +golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= +golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f h1:v4INt8xihDGvnrfjMDVXGxw9wrfxYyCjk0KbXjhR55s= +golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU= +golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= +golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= +golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM= +golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From dd5de6900b62f88deb2acc9cb5d14a480f9fb316 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Tue, 28 Feb 2023 18:37:19 +0200 Subject: [PATCH 5/9] Add some basic bind filter functions (#274) * Add some basic bind filter functions This change adds the ability to mount a a single folder or a volume inside another folder, using the bind filter API. While the API allows mounting multiple sources inside a single mount point, acting as an overlay, we disable this functionality in the ApplyFileBinding function. Signed-off-by: Gabriel Adrian Samfira * Add some tests Signed-off-by: Gabriel Adrian Samfira * Move bind filter to different package Signed-off-by: Gabriel Adrian Samfira * Use string in signature and fix getFinalPath * Properly close handle in getFinalPath() * Use string in function signature. mksyscall generates proper code to convert to utf16 * Enable TestRemoveFileBinding on Windows Server 2019 Windows Server 2019 only exposes 2 function in bindfltapi.dll: * BfRemoveMapping * BfSetupFilter Signed-off-by: Gabriel Adrian Samfira * Use windows.UTF16ToString to decode string Signed-off-by: Gabriel Adrian Samfira * Optimize bfGetMappings signature Signed-off-by: Gabriel Adrian Samfira * Skip unsupported tests on ltsc2019 Signed-off-by: Gabriel Adrian Samfira * Fix typo, add testcase * Additionally check if we can write to a read-only mount point, not just delete from it * No need to set FILE_FLAG_OPEN_REPARSE_POINT when opening a file Signed-off-by: Gabriel Adrian Samfira * Remove extra flags Signed-off-by: Gabriel Adrian Samfira * Add test to account for symlinks as sources Signed-off-by: Gabriel Adrian Samfira --------- Signed-off-by: Gabriel Adrian Samfira --- pkg/bindfilter/bind_filter.go | 308 ++++++++++++++++++++++++++++ pkg/bindfilter/bind_filter_test.go | 309 +++++++++++++++++++++++++++++ pkg/bindfilter/zsyscall_windows.go | 116 +++++++++++ 3 files changed, 733 insertions(+) create mode 100644 pkg/bindfilter/bind_filter.go create mode 100644 pkg/bindfilter/bind_filter_test.go create mode 100644 pkg/bindfilter/zsyscall_windows.go diff --git a/pkg/bindfilter/bind_filter.go b/pkg/bindfilter/bind_filter.go new file mode 100644 index 00000000..7ac377ae --- /dev/null +++ b/pkg/bindfilter/bind_filter.go @@ -0,0 +1,308 @@ +//go:build windows +// +build windows + +package bindfilter + +import ( + "bytes" + "encoding/binary" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +//go:generate go run github.com/Microsoft/go-winio/tools/mkwinsyscall -output zsyscall_windows.go ./bind_filter.go +//sys bfSetupFilter(jobHandle windows.Handle, flags uint32, virtRootPath string, virtTargetPath string, virtExceptions **uint16, virtExceptionPathCount uint32) (hr error) = bindfltapi.BfSetupFilter? +//sys bfRemoveMapping(jobHandle windows.Handle, virtRootPath string) (hr error) = bindfltapi.BfRemoveMapping? +//sys bfGetMappings(flags uint32, jobHandle windows.Handle, virtRootPath *uint16, sid *windows.SID, bufferSize *uint32, outBuffer *byte) (hr error) = bindfltapi.BfGetMappings? + +// BfSetupFilter flags. See: +// https://github.com/microsoft/BuildXL/blob/a6dce509f0d4f774255e5fbfb75fa6d5290ed163/Public/Src/Utilities/Native/Processes/Windows/NativeContainerUtilities.cs#L193-L240 +// +//nolint:revive // var-naming: ALL_CAPS +const ( + BINDFLT_FLAG_READ_ONLY_MAPPING uint32 = 0x00000001 + // Tells bindflt to fail mapping with STATUS_INVALID_PARAMETER if a mapping produces + // multiple targets. + BINDFLT_FLAG_NO_MULTIPLE_TARGETS uint32 = 0x00000040 +) + +//nolint:revive // var-naming: ALL_CAPS +const ( + BINDFLT_GET_MAPPINGS_FLAG_VOLUME uint32 = 0x00000001 + BINDFLT_GET_MAPPINGS_FLAG_SILO uint32 = 0x00000002 + BINDFLT_GET_MAPPINGS_FLAG_USER uint32 = 0x00000004 +) + +// ApplyFileBinding creates a global mount of the source in root, with an optional +// read only flag. +// The bind filter allows us to create mounts of directories and volumes. By default it allows +// us to mount multiple sources inside a single root, acting as an overlay. Files from the +// second source will superscede the first source that was mounted. +// This function disables this behavior and sets the BINDFLT_FLAG_NO_MULTIPLE_TARGETS flag +// on the mount. +func ApplyFileBinding(root, source string, readOnly bool) error { + // The parent directory needs to exist for the bind to work. MkdirAll stats and + // returns nil if the directory exists internally so we should be fine to mkdirall + // every time. + if err := os.MkdirAll(filepath.Dir(root), 0); err != nil { + return err + } + + if strings.Contains(source, "Volume{") && !strings.HasSuffix(source, "\\") { + // Add trailing slash to volumes, otherwise we get an error when binding it to + // a folder. + source = source + "\\" + } + + flags := BINDFLT_FLAG_NO_MULTIPLE_TARGETS + if readOnly { + flags |= BINDFLT_FLAG_READ_ONLY_MAPPING + } + + // Set the job handle to 0 to create a global mount. + if err := bfSetupFilter( + 0, + flags, + root, + source, + nil, + 0, + ); err != nil { + return fmt.Errorf("failed to bind target %q to root %q: %w", source, root, err) + } + return nil +} + +// RemoveFileBinding removes a mount from the root path. +func RemoveFileBinding(root string) error { + if err := bfRemoveMapping(0, root); err != nil { + return fmt.Errorf("removing file binding: %w", err) + } + return nil +} + +// GetBindMappings returns a list of bind mappings that have their root on a +// particular volume. The volumePath parameter can be any path that exists on +// a volume. For example, if a number of mappings are created in C:\ProgramData\test, +// to get a list of those mappings, the volumePath parameter would have to be set to +// C:\ or the VOLUME_NAME_GUID notation of C:\ (\\?\Volume{GUID}\), or any child +// path that exists. +func GetBindMappings(volumePath string) ([]BindMapping, error) { + rootPtr, err := windows.UTF16PtrFromString(volumePath) + if err != nil { + return nil, err + } + + flags := BINDFLT_GET_MAPPINGS_FLAG_VOLUME + // allocate a large buffer for results + var outBuffSize uint32 = 256 * 1024 + buf := make([]byte, outBuffSize) + + if err := bfGetMappings(flags, 0, rootPtr, nil, &outBuffSize, &buf[0]); err != nil { + return nil, err + } + + if outBuffSize < 12 { + return nil, fmt.Errorf("invalid buffer returned") + } + + result := buf[:outBuffSize] + + // The first 12 bytes are the three uint32 fields in getMappingsResponseHeader{} + headerBuffer := result[:12] + // The alternative to using unsafe and casting it to the above defined structures, is to manually + // parse the fields. Not too terrible, but not sure it'd worth the trouble. + header := *(*getMappingsResponseHeader)(unsafe.Pointer(&headerBuffer[0])) + + if header.MappingCount == 0 { + // no mappings + return []BindMapping{}, nil + } + + mappingsBuffer := result[12 : int(unsafe.Sizeof(mappingEntry{}))*int(header.MappingCount)] + // Get a pointer to the first mapping in the slice + mappingsPointer := (*mappingEntry)(unsafe.Pointer(&mappingsBuffer[0])) + // Get slice of mappings + mappings := unsafe.Slice(mappingsPointer, header.MappingCount) + + mappingEntries := make([]BindMapping, header.MappingCount) + for i := 0; i < int(header.MappingCount); i++ { + bindMapping, err := getBindMappingFromBuffer(result, mappings[i]) + if err != nil { + return nil, fmt.Errorf("fetching bind mappings: %w", err) + } + mappingEntries[i] = bindMapping + } + + return mappingEntries, nil +} + +// mappingEntry holds information about where in the response buffer we can +// find information about the virtual root (the mount point) and the targets (sources) +// that get mounted, as well as the flags used to bind the targets to the virtual root. +type mappingEntry struct { + VirtRootLength uint32 + VirtRootOffset uint32 + Flags uint32 + NumberOfTargets uint32 + TargetEntriesOffset uint32 +} + +type mappingTargetEntry struct { + TargetRootLength uint32 + TargetRootOffset uint32 +} + +// getMappingsResponseHeader represents the first 12 bytes of the BfGetMappings() response. +// It gives us the size of the buffer, the status of the call and the number of mappings. +// A response +type getMappingsResponseHeader struct { + Size uint32 + Status uint32 + MappingCount uint32 +} + +type BindMapping struct { + MountPoint string + Flags uint32 + Targets []string +} + +func decodeEntry(buffer []byte) (string, error) { + name := make([]uint16, len(buffer)/2) + err := binary.Read(bytes.NewReader(buffer), binary.LittleEndian, &name) + if err != nil { + return "", fmt.Errorf("decoding name: %w", err) + } + return windows.UTF16ToString(name), nil +} + +func getTargetsFromBuffer(buffer []byte, offset, count int) ([]string, error) { + if len(buffer) < offset+count*6 { + return nil, fmt.Errorf("invalid buffer") + } + + targets := make([]string, count) + for i := 0; i < count; i++ { + entryBuf := buffer[offset+i*8 : offset+i*8+8] + tgt := *(*mappingTargetEntry)(unsafe.Pointer(&entryBuf[0])) + if len(buffer) < int(tgt.TargetRootOffset)+int(tgt.TargetRootLength) { + return nil, fmt.Errorf("invalid buffer") + } + decoded, err := decodeEntry(buffer[tgt.TargetRootOffset : tgt.TargetRootOffset+tgt.TargetRootLength]) + if err != nil { + return nil, fmt.Errorf("decoding name: %w", err) + } + decoded, err = getFinalPath(decoded) + if err != nil { + return nil, fmt.Errorf("fetching final path: %w", err) + } + + targets[i] = decoded + } + return targets, nil +} + +func getFinalPath(pth string) (string, error) { + // BfGetMappings returns VOLUME_NAME_NT paths like \Device\HarddiskVolume2\ProgramData. + // These can be accessed by prepending \\.\GLOBALROOT to the path. We use this to get the + // DOS paths for these files. + if strings.HasPrefix(pth, `\Device`) { + pth = `\\.\GLOBALROOT` + pth + } + + han, err := openPath(pth) + if err != nil { + return "", fmt.Errorf("fetching file handle: %w", err) + } + defer func() { + _ = windows.CloseHandle(han) + }() + + buf := make([]uint16, 100) + var flags uint32 = 0x0 + for { + n, err := windows.GetFinalPathNameByHandle(han, &buf[0], uint32(len(buf)), flags) + if err != nil { + // if we mounted a volume that does not also have a drive letter assigned, attempting to + // fetch the VOLUME_NAME_DOS will fail with os.ErrNotExist. Attempt to get the VOLUME_NAME_GUID. + if errors.Is(err, os.ErrNotExist) && flags != 0x1 { + flags = 0x1 + continue + } + return "", fmt.Errorf("getting final path name: %w", err) + } + if n < uint32(len(buf)) { + break + } + buf = make([]uint16, n) + } + finalPath := syscall.UTF16ToString(buf) + // We got VOLUME_NAME_DOS, we need to strip away some leading slashes. + // Leave unchanged if we ended up requesting VOLUME_NAME_GUID + if len(finalPath) > 4 && finalPath[:4] == `\\?\` && flags == 0x0 { + finalPath = finalPath[4:] + if len(finalPath) > 3 && finalPath[:3] == `UNC` { + // return path like \\server\share\... + finalPath = `\` + finalPath[3:] + } + } + + return finalPath, nil +} + +func getBindMappingFromBuffer(buffer []byte, entry mappingEntry) (BindMapping, error) { + if len(buffer) < int(entry.VirtRootOffset)+int(entry.VirtRootLength) { + return BindMapping{}, fmt.Errorf("invalid buffer") + } + + src, err := decodeEntry(buffer[entry.VirtRootOffset : entry.VirtRootOffset+entry.VirtRootLength]) + if err != nil { + return BindMapping{}, fmt.Errorf("decoding entry: %w", err) + } + targets, err := getTargetsFromBuffer(buffer, int(entry.TargetEntriesOffset), int(entry.NumberOfTargets)) + if err != nil { + return BindMapping{}, fmt.Errorf("fetching targets: %w", err) + } + + src, err = getFinalPath(src) + if err != nil { + return BindMapping{}, fmt.Errorf("fetching final path: %w", err) + } + + return BindMapping{ + Flags: entry.Flags, + Targets: targets, + MountPoint: src, + }, nil +} + +func openPath(path string) (windows.Handle, error) { + u16, err := windows.UTF16PtrFromString(path) + if err != nil { + return 0, err + } + h, err := windows.CreateFile( + u16, + 0, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, + windows.OPEN_EXISTING, + windows.FILE_FLAG_BACKUP_SEMANTICS, // Needed to open a directory handle. + 0) + if err != nil { + return 0, &os.PathError{ + Op: "CreateFile", + Path: path, + Err: err, + } + } + return h, nil +} diff --git a/pkg/bindfilter/bind_filter_test.go b/pkg/bindfilter/bind_filter_test.go new file mode 100644 index 00000000..d4450e59 --- /dev/null +++ b/pkg/bindfilter/bind_filter_test.go @@ -0,0 +1,309 @@ +//go:build windows +// +build windows + +package bindfilter + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + + "golang.org/x/sys/windows/registry" +) + +func TestApplyFileBinding(t *testing.T) { + source := t.TempDir() + destination := t.TempDir() + fileName := "testFile.txt" + srcFile := filepath.Join(source, fileName) + dstFile := filepath.Join(destination, fileName) + + err := ApplyFileBinding(destination, source, false) + if err != nil { + t.Fatal(err) + } + defer removeFileBinding(t, destination) + + data := []byte("bind filter test") + + if err := os.WriteFile(srcFile, data, 0600); err != nil { + t.Fatal(err) + } + + readData, err := os.ReadFile(dstFile) + if err != nil { + t.Fatal(err) + } + + if string(readData) != string(data) { + t.Fatalf("source and destination file contents differ. Expected: %s, got: %s", string(data), string(readData)) + } + + // Remove the file on the mount point. The mount is not read-only, this should work. + if err := os.Remove(dstFile); err != nil { + t.Fatalf("failed to remove file from mount point: %s", err) + } + + // Check that it's gone from the source as well. + if _, err := os.Stat(srcFile); err == nil { + t.Fatalf("expected file %s to be gone but is not", srcFile) + } +} + +func removeFileBinding(t *testing.T, mountpoint string) { + if err := RemoveFileBinding(mountpoint); err != nil { + t.Logf("failed to remove file binding from %s: %q", mountpoint, err) + } +} + +func TestApplyFileBindingReadOnly(t *testing.T) { + source := t.TempDir() + destination := t.TempDir() + fileName := "testFile.txt" + srcFile := filepath.Join(source, fileName) + dstFile := filepath.Join(destination, fileName) + + err := ApplyFileBinding(destination, source, true) + if err != nil { + t.Fatal(err) + } + defer removeFileBinding(t, destination) + + data := []byte("bind filter test") + + if err := os.WriteFile(srcFile, data, 0600); err != nil { + t.Fatal(err) + } + + readData, err := os.ReadFile(dstFile) + if err != nil { + t.Fatal(err) + } + + if string(readData) != string(data) { + t.Fatalf("source and destination file contents differ. Expected: %s, got: %s", string(data), string(readData)) + } + + // Attempt to remove the file on the mount point + err = os.Remove(dstFile) + if err == nil { + t.Fatalf("should not be able to remove a file from a read-only mount") + } + if !errors.Is(err, os.ErrPermission) { + t.Fatalf("expected an access denied error, got: %q", err) + } + + // Attempt to write on the read-only mount point. + err = os.WriteFile(dstFile, []byte("something else"), 0600) + if err == nil { + t.Fatalf("should not be able to overwrite a file from a read-only mount") + } + if !errors.Is(err, os.ErrPermission) { + t.Fatalf("expected an access denied error, got: %q", err) + } +} + +func TestEnsureOnlyOneTargetCanBeMounted(t *testing.T) { + version, err := getWindowsBuildNumber() + if err != nil { + t.Fatalf("couldn't get version number: %s", err) + } + + if version <= 17763 { + t.Skip("not supported on RS5 or earlier") + } + source := t.TempDir() + secondarySource := t.TempDir() + destination := t.TempDir() + + err = ApplyFileBinding(destination, source, false) + if err != nil { + t.Fatal(err) + } + + defer removeFileBinding(t, destination) + + err = ApplyFileBinding(destination, secondarySource, false) + if err == nil { + removeFileBinding(t, destination) + t.Fatalf("we should not be able to mount multiple targets in the same destination") + } +} + +func checkSourceIsMountedOnDestination(src, dst string) (bool, error) { + mappings, err := GetBindMappings(dst) + if err != nil { + return false, err + } + + found := false + // There may be pre-existing mappings on the system. + for _, mapping := range mappings { + if mapping.MountPoint == dst { + found = true + if len(mapping.Targets) != 1 { + return false, fmt.Errorf("expected only one target, got: %s", strings.Join(mapping.Targets, ", ")) + } + if mapping.Targets[0] != src { + return false, fmt.Errorf("expected target to be %s, got %s", src, mapping.Targets[0]) + } + break + } + } + + return found, nil +} + +func TestGetBindMappings(t *testing.T) { + version, err := getWindowsBuildNumber() + if err != nil { + t.Fatalf("couldn't get version number: %s", err) + } + + if version <= 17763 { + t.Skip("not supported on RS5 or earlier") + } + // GetBindMappings will expand short paths like ADMINI~1 and PROGRA~1 to their + // full names. In order to properly match the names later, we expand them here. + srcShort := t.TempDir() + source, err := getFinalPath(srcShort) + if err != nil { + t.Fatalf("failed to get long path") + } + + dstShort := t.TempDir() + destination, err := getFinalPath(dstShort) + if err != nil { + t.Fatalf("failed to get long path") + } + + err = ApplyFileBinding(destination, source, false) + if err != nil { + t.Fatal(err) + } + defer removeFileBinding(t, destination) + + hasMapping, err := checkSourceIsMountedOnDestination(source, destination) + if err != nil { + t.Fatal(err) + } + + if !hasMapping { + t.Fatalf("expected to find %s mounted on %s, but could not", source, destination) + } +} + +func TestRemoveFileBinding(t *testing.T) { + srcShort := t.TempDir() + source, err := getFinalPath(srcShort) + if err != nil { + t.Fatalf("failed to get long path") + } + + dstShort := t.TempDir() + destination, err := getFinalPath(dstShort) + if err != nil { + t.Fatalf("failed to get long path") + } + + fileName := "testFile.txt" + srcFile := filepath.Join(source, fileName) + dstFile := filepath.Join(destination, fileName) + data := []byte("bind filter test") + + if err := os.WriteFile(srcFile, data, 0600); err != nil { + t.Fatal(err) + } + + err = ApplyFileBinding(destination, source, false) + if err != nil { + t.Fatal(err) + } + + if _, err := os.Stat(dstFile); err != nil { + removeFileBinding(t, destination) + t.Fatalf("expected to find %s, but could not", dstFile) + } + + if err := RemoveFileBinding(destination); err != nil { + t.Fatal(err) + } + + if _, err := os.Stat(dstFile); err == nil { + t.Fatalf("expected %s to be gone, but it is not", dstFile) + } +} + +func getWindowsBuildNumber() (int, error) { + k, err := registry.OpenKey(registry.LOCAL_MACHINE, `SOFTWARE\Microsoft\Windows NT\CurrentVersion`, registry.QUERY_VALUE) + if err != nil { + return 0, fmt.Errorf("read CurrentVersion reg key: %w", err) + } + defer k.Close() + buildNumStr, _, err := k.GetStringValue("CurrentBuild") + if err != nil { + return 0, fmt.Errorf("read CurrentBuild reg value: %w", err) + } + buildNum, err := strconv.Atoi(buildNumStr) + if err != nil { + return 0, err + } + return buildNum, nil +} + +func TestGetBindMappingsSymlinks(t *testing.T) { + version, err := getWindowsBuildNumber() + if err != nil { + t.Fatalf("couldn't get version number: %s", err) + } + + if version <= 17763 { + t.Skip("not supported on RS5 or earlier") + } + + srcShort := t.TempDir() + sourceNested := filepath.Join(srcShort, "source") + if err := os.MkdirAll(sourceNested, 0600); err != nil { + t.Fatalf("failed to create folder: %s", err) + } + simlinkSource := filepath.Join(srcShort, "symlink") + if err := os.Symlink(sourceNested, simlinkSource); err != nil { + t.Fatalf("failed to create symlink: %s", err) + } + + // We'll need the long form of the source folder, as we expect bfSetupFilter() + // to resolve the symlink and create a mapping to the actual source the symlink + // points to. + source, err := getFinalPath(sourceNested) + if err != nil { + t.Fatalf("failed to get long path") + } + + dstShort := t.TempDir() + destination, err := getFinalPath(dstShort) + if err != nil { + t.Fatalf("failed to get long path") + } + + // Use the symlink as a source for the mapping. + err = ApplyFileBinding(destination, simlinkSource, false) + if err != nil { + t.Fatal(err) + } + defer removeFileBinding(t, destination) + + // We expect the mapping to point to the folder the symlink points to, not to the + // actual symlink. + hasMapping, err := checkSourceIsMountedOnDestination(source, destination) + if err != nil { + t.Fatal(err) + } + + if !hasMapping { + t.Fatalf("expected to find %s mounted on %s, but could not", source, destination) + } +} diff --git a/pkg/bindfilter/zsyscall_windows.go b/pkg/bindfilter/zsyscall_windows.go new file mode 100644 index 00000000..45c45c96 --- /dev/null +++ b/pkg/bindfilter/zsyscall_windows.go @@ -0,0 +1,116 @@ +//go:build windows + +// Code generated by 'go generate' using "github.com/Microsoft/go-winio/tools/mkwinsyscall"; DO NOT EDIT. + +package bindfilter + +import ( + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +var _ unsafe.Pointer + +// Do the interface allocations only once for common +// Errno values. +const ( + errnoERROR_IO_PENDING = 997 +) + +var ( + errERROR_IO_PENDING error = syscall.Errno(errnoERROR_IO_PENDING) + errERROR_EINVAL error = syscall.EINVAL +) + +// errnoErr returns common boxed Errno values, to prevent +// allocations at runtime. +func errnoErr(e syscall.Errno) error { + switch e { + case 0: + return errERROR_EINVAL + case errnoERROR_IO_PENDING: + return errERROR_IO_PENDING + } + // TODO: add more here, after collecting data on the common + // error values see on Windows. (perhaps when running + // all.bat?) + return e +} + +var ( + modbindfltapi = windows.NewLazySystemDLL("bindfltapi.dll") + + procBfGetMappings = modbindfltapi.NewProc("BfGetMappings") + procBfRemoveMapping = modbindfltapi.NewProc("BfRemoveMapping") + procBfSetupFilter = modbindfltapi.NewProc("BfSetupFilter") +) + +func bfGetMappings(flags uint32, jobHandle windows.Handle, virtRootPath *uint16, sid *windows.SID, bufferSize *uint32, outBuffer *byte) (hr error) { + hr = procBfGetMappings.Find() + if hr != nil { + return + } + r0, _, _ := syscall.Syscall6(procBfGetMappings.Addr(), 6, uintptr(flags), uintptr(jobHandle), uintptr(unsafe.Pointer(virtRootPath)), uintptr(unsafe.Pointer(sid)), uintptr(unsafe.Pointer(bufferSize)), uintptr(unsafe.Pointer(outBuffer))) + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + hr = syscall.Errno(r0) + } + return +} + +func bfRemoveMapping(jobHandle windows.Handle, virtRootPath string) (hr error) { + var _p0 *uint16 + _p0, hr = syscall.UTF16PtrFromString(virtRootPath) + if hr != nil { + return + } + return _bfRemoveMapping(jobHandle, _p0) +} + +func _bfRemoveMapping(jobHandle windows.Handle, virtRootPath *uint16) (hr error) { + hr = procBfRemoveMapping.Find() + if hr != nil { + return + } + r0, _, _ := syscall.Syscall(procBfRemoveMapping.Addr(), 2, uintptr(jobHandle), uintptr(unsafe.Pointer(virtRootPath)), 0) + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + hr = syscall.Errno(r0) + } + return +} + +func bfSetupFilter(jobHandle windows.Handle, flags uint32, virtRootPath string, virtTargetPath string, virtExceptions **uint16, virtExceptionPathCount uint32) (hr error) { + var _p0 *uint16 + _p0, hr = syscall.UTF16PtrFromString(virtRootPath) + if hr != nil { + return + } + var _p1 *uint16 + _p1, hr = syscall.UTF16PtrFromString(virtTargetPath) + if hr != nil { + return + } + return _bfSetupFilter(jobHandle, flags, _p0, _p1, virtExceptions, virtExceptionPathCount) +} + +func _bfSetupFilter(jobHandle windows.Handle, flags uint32, virtRootPath *uint16, virtTargetPath *uint16, virtExceptions **uint16, virtExceptionPathCount uint32) (hr error) { + hr = procBfSetupFilter.Find() + if hr != nil { + return + } + r0, _, _ := syscall.Syscall6(procBfSetupFilter.Addr(), 6, uintptr(jobHandle), uintptr(flags), uintptr(unsafe.Pointer(virtRootPath)), uintptr(unsafe.Pointer(virtTargetPath)), uintptr(unsafe.Pointer(virtExceptions)), uintptr(virtExceptionPathCount)) + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + hr = syscall.Errno(r0) + } + return +} From 2a14e68005f1afb8b5cdef5b1196a35759649347 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy <84944216+helsaawy@users.noreply.github.com> Date: Fri, 3 Mar 2023 15:52:45 -0500 Subject: [PATCH 6/9] Apply two upstream CL to mkwinsyscall (#278) * Apply CL463216: write source to temp file if formatting fails This change writes the unformatted Go source code to a temp file if "format.Source" fails. Print the temp file path to the console to make it easy to find. The source code is what causes formatting errors, and it can be difficult to diagnose them without this context. CL link: https://go-review.googlesource.com/c/sys/+/463216 commit: 4112509618ee88519f899be20efc6882496b57c8 Signed-off-by: Hamza El-Saawy * Apply CL463215: support "." and "-" in DLL name This change adds "." and "-" support for DLL filenames in "//sys". Supporting "." requires a change in how mkwinsyscall handles the "= ." syntax. Instead of assuming that only one "." can appear in this string, now mkwinsyscall assumes that any additional "." belongs to the filename. Supporting "." also requires changing how Go identifiers are created for each DLL. This change also allows mkwinsyscall to support "-". When creating a Go identifier, "." and "-" in the DLL filename are replaced with "_". Otherwise, mkwinsyscall would produce invalid Go code, causing "format.Source" to fail. CL link: https://go-review.googlesource.com/c/sys/+/463215 commit: 71da6904945ac440253cb5c132d64712f80ca497 Signed-off-by: Hamza El-Saawy --------- Signed-off-by: Hamza El-Saawy --- tools/mkwinsyscall/mkwinsyscall.go | 85 +++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 20 deletions(-) diff --git a/tools/mkwinsyscall/mkwinsyscall.go b/tools/mkwinsyscall/mkwinsyscall.go index e72be313..20d9e3d2 100644 --- a/tools/mkwinsyscall/mkwinsyscall.go +++ b/tools/mkwinsyscall/mkwinsyscall.go @@ -477,15 +477,14 @@ func newFn(s string) (*Fn, error) { return nil, errors.New("Could not extract dll name from \"" + f.src + "\"") } s = trim(s[1:]) - a := strings.Split(s, ".") - switch len(a) { - case 1: - f.dllfuncname = a[0] - case 2: - f.dllname = a[0] - f.dllfuncname = a[1] - default: - return nil, errors.New("Could not extract dll name from \"" + f.src + "\"") + if i := strings.LastIndex(s, "."); i >= 0 { + f.dllname = s[:i] + f.dllfuncname = s[i+1:] + } else { + f.dllfuncname = s + } + if f.dllfuncname == "" { + return nil, fmt.Errorf("function name is not specified in %q", s) } if n := f.dllfuncname; endsIn(n, '?') { f.dllfuncname = n[:len(n)-1] @@ -502,7 +501,23 @@ func (f *Fn) DLLName() string { return f.dllname } -// DLLName returns DLL function name for function f. +// DLLVar returns a valid Go identifier that represents DLLName. +func (f *Fn) DLLVar() string { + id := strings.Map(func(r rune) rune { + switch r { + case '.', '-': + return '_' + default: + return r + } + }, f.DLLName()) + if !token.IsIdentifier(id) { + panic(fmt.Errorf("could not create Go identifier for DLLName %q", f.DLLName())) + } + return id +} + +// DLLFuncName returns DLL function name for function f. func (f *Fn) DLLFuncName() string { if f.dllfuncname == "" { return f.Name @@ -648,6 +663,13 @@ func (f *Fn) HelperName() string { return "_" + f.Name } +// DLL is a DLL's filename and a string that is valid in a Go identifier that should be used when +// naming a variable that refers to the DLL. +type DLL struct { + Name string + Var string +} + // Source files and functions. type Source struct { Funcs []*Fn @@ -697,18 +719,20 @@ func ParseFiles(fs []string) (*Source, error) { } // DLLs return dll names for a source set src. -func (src *Source) DLLs() []string { +func (src *Source) DLLs() []DLL { uniq := make(map[string]bool) - r := make([]string, 0) + r := make([]DLL, 0) for _, f := range src.Funcs { - name := f.DLLName() - if _, found := uniq[name]; !found { - uniq[name] = true - r = append(r, name) + id := f.DLLVar() + if _, found := uniq[id]; !found { + uniq[id] = true + r = append(r, DLL{f.DLLName(), id}) } } if *sortdecls { - sort.Strings(r) + sort.Slice(r, func(i, j int) bool { + return r[i].Var < r[j].Var + }) } return r } @@ -878,6 +902,22 @@ func (src *Source) Generate(w io.Writer) error { return nil } +func writeTempSourceFile(data []byte) (string, error) { + f, err := os.CreateTemp("", "mkwinsyscall-generated-*.go") + if err != nil { + return "", err + } + _, err = f.Write(data) + if closeErr := f.Close(); err == nil { + err = closeErr + } + if err != nil { + os.Remove(f.Name()) // best effort + return "", err + } + return f.Name(), nil +} + func usage() { fmt.Fprintf(os.Stderr, "usage: mkwinsyscall [flags] [path ...]\n") flag.PrintDefaults() @@ -904,7 +944,12 @@ func main() { data, err := format.Source(buf.Bytes()) if err != nil { - log.Fatal(err) + log.Printf("failed to format source: %v", err) + f, err := writeTempSourceFile(buf.Bytes()) + if err != nil { + log.Fatalf("failed to write unformatted source to file: %v", err) + } + log.Fatalf("for diagnosis, wrote unformatted source to %v", f) } if *filename == "" { _, err = os.Stdout.Write(data) @@ -970,10 +1015,10 @@ var ( {{/* help functions */}} -{{define "dlls"}}{{range .DLLs}} mod{{.}} = {{newlazydll .}} +{{define "dlls"}}{{range .DLLs}} mod{{.Var}} = {{newlazydll .Name}} {{end}}{{end}} -{{define "funcnames"}}{{range .DLLFuncNames}} proc{{.DLLFuncName}} = mod{{.DLLName}}.NewProc("{{.DLLFuncName}}") +{{define "funcnames"}}{{range .DLLFuncNames}} proc{{.DLLFuncName}} = mod{{.DLLVar}}.NewProc("{{.DLLFuncName}}") {{end}}{{end}} {{define "helperbody"}} From 41915dceb0c49ffa85fec28ba831f1102f4b4e80 Mon Sep 17 00:00:00 2001 From: rcarman-r7 <79936748+rcarman-r7@users.noreply.github.com> Date: Thu, 16 Mar 2023 13:30:07 -0400 Subject: [PATCH 7/9] apply ObjectAttributes fix for certain Windows environments (#280) Signed-off-by: Robert Carman --- pipe.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pipe.go b/pipe.go index ca6e38fc..b234cdf8 100644 --- a/pipe.go +++ b/pipe.go @@ -279,6 +279,7 @@ func makeServerPipeHandle(path string, sd []byte, c *PipeConfig, first bool) (sy } defer localFree(ntPath.Buffer) oa.ObjectName = &ntPath + oa.Attributes = windows.OBJ_CASE_INSENSITIVE // The security descriptor is only needed for the first pipe. if first { From b884eb77dbac86a699d1a8ba56f10aa774db3278 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy <84944216+helsaawy@users.noreply.github.com> Date: Fri, 14 Apr 2023 12:58:14 -0400 Subject: [PATCH 8/9] Add `fs.ResolvePath` to resolve symbolic links (#275) * Add `fs.ResolvePath` to resolve symbolic links `filepath.EvalSymlinks` does not work well on Windows, and can enter infinite loops in certain situations and error out. Use Win32 API GetFinalPathNameByHandle to handle path resolution. Implementation based off on: https://github.com/containerd/containerd/pull/5411 Signed-off-by: Hamza El-Saawy * PR: types, documentation Signed-off-by: Hamza El-Saawy * remove unneded constant groups Signed-off-by: Hamza El-Saawy * Attempt normalized path first Update logic to try querying for normalized path initially, then use opened path if access is denied. Signed-off-by: Hamza El-Saawy --------- Signed-off-by: Hamza El-Saawy --- internal/fs/doc.go | 2 + internal/fs/fs.go | 202 ++++++++++++++++++++++++++ internal/fs/fs_test.go | 42 ++++++ internal/fs/security.go | 12 ++ internal/fs/zsyscall_windows.go | 64 ++++++++ internal/stringbuffer/wstring.go | 132 +++++++++++++++++ internal/stringbuffer/wstring_test.go | 45 ++++++ pipe.go | 21 +-- pkg/fs/doc.go | 2 + pkg/fs/fs_windows.go | 17 ++- pkg/fs/resolve.go | 128 ++++++++++++++++ zsyscall_windows.go | 19 --- 12 files changed, 650 insertions(+), 36 deletions(-) create mode 100644 internal/fs/doc.go create mode 100644 internal/fs/fs.go create mode 100644 internal/fs/fs_test.go create mode 100644 internal/fs/security.go create mode 100644 internal/fs/zsyscall_windows.go create mode 100644 internal/stringbuffer/wstring.go create mode 100644 internal/stringbuffer/wstring_test.go create mode 100644 pkg/fs/doc.go create mode 100644 pkg/fs/resolve.go diff --git a/internal/fs/doc.go b/internal/fs/doc.go new file mode 100644 index 00000000..1f653881 --- /dev/null +++ b/internal/fs/doc.go @@ -0,0 +1,2 @@ +// This package contains Win32 filesystem functionality. +package fs diff --git a/internal/fs/fs.go b/internal/fs/fs.go new file mode 100644 index 00000000..509b3ec6 --- /dev/null +++ b/internal/fs/fs.go @@ -0,0 +1,202 @@ +//go:build windows + +package fs + +import ( + "golang.org/x/sys/windows" + + "github.com/Microsoft/go-winio/internal/stringbuffer" +) + +//go:generate go run github.com/Microsoft/go-winio/tools/mkwinsyscall -output zsyscall_windows.go fs.go + +// https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew +//sys CreateFile(name string, access AccessMask, mode FileShareMode, sa *syscall.SecurityAttributes, createmode FileCreationDisposition, attrs FileFlagOrAttribute, templatefile windows.Handle) (handle windows.Handle, err error) [failretval==windows.InvalidHandle] = CreateFileW + +const NullHandle windows.Handle = 0 + +// AccessMask defines standard, specific, and generic rights. +// +// Bitmask: +// 3 3 2 2 2 2 2 2 2 2 2 2 1 1 1 1 1 1 1 1 1 1 +// 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 +// +---------------+---------------+-------------------------------+ +// |G|G|G|G|Resvd|A| StandardRights| SpecificRights | +// |R|W|E|A| |S| | | +// +-+-------------+---------------+-------------------------------+ +// +// GR Generic Read +// GW Generic Write +// GE Generic Exectue +// GA Generic All +// Resvd Reserved +// AS Access Security System +// +// https://learn.microsoft.com/en-us/windows/win32/secauthz/access-mask +// +// https://learn.microsoft.com/en-us/windows/win32/secauthz/generic-access-rights +// +// https://learn.microsoft.com/en-us/windows/win32/fileio/file-access-rights-constants +type AccessMask = windows.ACCESS_MASK + +//nolint:revive // SNAKE_CASE is not idiomatic in Go, but aligned with Win32 API. +const ( + // Not actually any. + // + // For CreateFile: "query certain metadata such as file, directory, or device attributes without accessing that file or device" + // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew#parameters + FILE_ANY_ACCESS AccessMask = 0 + + // Specific Object Access + // from ntioapi.h + + FILE_READ_DATA AccessMask = (0x0001) // file & pipe + FILE_LIST_DIRECTORY AccessMask = (0x0001) // directory + + FILE_WRITE_DATA AccessMask = (0x0002) // file & pipe + FILE_ADD_FILE AccessMask = (0x0002) // directory + + FILE_APPEND_DATA AccessMask = (0x0004) // file + FILE_ADD_SUBDIRECTORY AccessMask = (0x0004) // directory + FILE_CREATE_PIPE_INSTANCE AccessMask = (0x0004) // named pipe + + FILE_READ_EA AccessMask = (0x0008) // file & directory + FILE_READ_PROPERTIES AccessMask = FILE_READ_EA + + FILE_WRITE_EA AccessMask = (0x0010) // file & directory + FILE_WRITE_PROPERTIES AccessMask = FILE_WRITE_EA + + FILE_EXECUTE AccessMask = (0x0020) // file + FILE_TRAVERSE AccessMask = (0x0020) // directory + + FILE_DELETE_CHILD AccessMask = (0x0040) // directory + + FILE_READ_ATTRIBUTES AccessMask = (0x0080) // all + + FILE_WRITE_ATTRIBUTES AccessMask = (0x0100) // all + + FILE_ALL_ACCESS AccessMask = (STANDARD_RIGHTS_REQUIRED | SYNCHRONIZE | 0x1FF) + FILE_GENERIC_READ AccessMask = (STANDARD_RIGHTS_READ | FILE_READ_DATA | FILE_READ_ATTRIBUTES | FILE_READ_EA | SYNCHRONIZE) + FILE_GENERIC_WRITE AccessMask = (STANDARD_RIGHTS_WRITE | FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_APPEND_DATA | SYNCHRONIZE) + FILE_GENERIC_EXECUTE AccessMask = (STANDARD_RIGHTS_EXECUTE | FILE_READ_ATTRIBUTES | FILE_EXECUTE | SYNCHRONIZE) + + SPECIFIC_RIGHTS_ALL AccessMask = 0x0000FFFF + + // Standard Access + // from ntseapi.h + + DELETE AccessMask = 0x0001_0000 + READ_CONTROL AccessMask = 0x0002_0000 + WRITE_DAC AccessMask = 0x0004_0000 + WRITE_OWNER AccessMask = 0x0008_0000 + SYNCHRONIZE AccessMask = 0x0010_0000 + + STANDARD_RIGHTS_REQUIRED AccessMask = 0x000F_0000 + + STANDARD_RIGHTS_READ AccessMask = READ_CONTROL + STANDARD_RIGHTS_WRITE AccessMask = READ_CONTROL + STANDARD_RIGHTS_EXECUTE AccessMask = READ_CONTROL + + STANDARD_RIGHTS_ALL AccessMask = 0x001F_0000 +) + +type FileShareMode uint32 + +//nolint:revive // SNAKE_CASE is not idiomatic in Go, but aligned with Win32 API. +const ( + FILE_SHARE_NONE FileShareMode = 0x00 + FILE_SHARE_READ FileShareMode = 0x01 + FILE_SHARE_WRITE FileShareMode = 0x02 + FILE_SHARE_DELETE FileShareMode = 0x04 + FILE_SHARE_VALID_FLAGS FileShareMode = 0x07 +) + +type FileCreationDisposition uint32 + +//nolint:revive // SNAKE_CASE is not idiomatic in Go, but aligned with Win32 API. +const ( + // from winbase.h + + CREATE_NEW FileCreationDisposition = 0x01 + CREATE_ALWAYS FileCreationDisposition = 0x02 + OPEN_EXISTING FileCreationDisposition = 0x03 + OPEN_ALWAYS FileCreationDisposition = 0x04 + TRUNCATE_EXISTING FileCreationDisposition = 0x05 +) + +// CreateFile and co. take flags or attributes together as one parameter. +// Define alias until we can use generics to allow both + +// https://learn.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants +type FileFlagOrAttribute uint32 + +//nolint:revive // SNAKE_CASE is not idiomatic in Go, but aligned with Win32 API. +const ( // from winnt.h + FILE_FLAG_WRITE_THROUGH FileFlagOrAttribute = 0x8000_0000 + FILE_FLAG_OVERLAPPED FileFlagOrAttribute = 0x4000_0000 + FILE_FLAG_NO_BUFFERING FileFlagOrAttribute = 0x2000_0000 + FILE_FLAG_RANDOM_ACCESS FileFlagOrAttribute = 0x1000_0000 + FILE_FLAG_SEQUENTIAL_SCAN FileFlagOrAttribute = 0x0800_0000 + FILE_FLAG_DELETE_ON_CLOSE FileFlagOrAttribute = 0x0400_0000 + FILE_FLAG_BACKUP_SEMANTICS FileFlagOrAttribute = 0x0200_0000 + FILE_FLAG_POSIX_SEMANTICS FileFlagOrAttribute = 0x0100_0000 + FILE_FLAG_OPEN_REPARSE_POINT FileFlagOrAttribute = 0x0020_0000 + FILE_FLAG_OPEN_NO_RECALL FileFlagOrAttribute = 0x0010_0000 + FILE_FLAG_FIRST_PIPE_INSTANCE FileFlagOrAttribute = 0x0008_0000 +) + +type FileSQSFlag = FileFlagOrAttribute + +//nolint:revive // SNAKE_CASE is not idiomatic in Go, but aligned with Win32 API. +const ( // from winbase.h + SECURITY_ANONYMOUS FileSQSFlag = FileSQSFlag(SecurityAnonymous << 16) + SECURITY_IDENTIFICATION FileSQSFlag = FileSQSFlag(SecurityIdentification << 16) + SECURITY_IMPERSONATION FileSQSFlag = FileSQSFlag(SecurityImpersonation << 16) + SECURITY_DELEGATION FileSQSFlag = FileSQSFlag(SecurityDelegation << 16) + + SECURITY_SQOS_PRESENT FileSQSFlag = 0x00100000 + SECURITY_VALID_SQOS_FLAGS FileSQSFlag = 0x001F0000 +) + +// GetFinalPathNameByHandle flags +// +// https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfinalpathnamebyhandlew#parameters +type GetFinalPathFlag uint32 + +//nolint:revive // SNAKE_CASE is not idiomatic in Go, but aligned with Win32 API. +const ( + GetFinalPathDefaultFlag GetFinalPathFlag = 0x0 + + FILE_NAME_NORMALIZED GetFinalPathFlag = 0x0 + FILE_NAME_OPENED GetFinalPathFlag = 0x8 + + VOLUME_NAME_DOS GetFinalPathFlag = 0x0 + VOLUME_NAME_GUID GetFinalPathFlag = 0x1 + VOLUME_NAME_NT GetFinalPathFlag = 0x2 + VOLUME_NAME_NONE GetFinalPathFlag = 0x4 +) + +// getFinalPathNameByHandle facilitates calling the Windows API GetFinalPathNameByHandle +// with the given handle and flags. It transparently takes care of creating a buffer of the +// correct size for the call. +// +// https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfinalpathnamebyhandlew +func GetFinalPathNameByHandle(h windows.Handle, flags GetFinalPathFlag) (string, error) { + b := stringbuffer.NewWString() + //TODO: can loop infinitely if Win32 keeps returning the same (or a larger) n? + for { + n, err := windows.GetFinalPathNameByHandle(h, b.Pointer(), b.Cap(), uint32(flags)) + if err != nil { + return "", err + } + // If the buffer wasn't large enough, n will be the total size needed (including null terminator). + // Resize and try again. + if n > b.Cap() { + b.ResizeTo(n) + continue + } + // If the buffer is large enough, n will be the size not including the null terminator. + // Convert to a Go string and return. + return b.String(), nil + } +} diff --git a/internal/fs/fs_test.go b/internal/fs/fs_test.go new file mode 100644 index 00000000..277f4eb0 --- /dev/null +++ b/internal/fs/fs_test.go @@ -0,0 +1,42 @@ +//go:build windows + +package fs + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "golang.org/x/sys/windows" +) + +func Test_GetFinalPathNameByHandle(t *testing.T) { + d := t.TempDir() + // open f via a relative path + name := t.Name() + ".txt" + fullPath := filepath.Join(d, name) + + w, err := os.Getwd() + if err != nil { + t.Fatalf("could not get working directory: %v", err) + } + if err := os.Chdir(d); err != nil { + t.Fatalf("could not chdir to %s: %v", d, err) + } + defer os.Chdir(w) //nolint:errcheck + + f, err := os.Create(name) + if err != nil { + t.Fatalf("could not open %s: %v", fullPath, err) + } + defer f.Close() + + path, err := GetFinalPathNameByHandle(windows.Handle(f.Fd()), GetFinalPathDefaultFlag) + if err != nil { + t.Fatalf("could not get final path for %s: %v", fullPath, err) + } + if strings.EqualFold(fullPath, path) { + t.Fatalf("expected %s, got %s", fullPath, path) + } +} diff --git a/internal/fs/security.go b/internal/fs/security.go new file mode 100644 index 00000000..81760ac6 --- /dev/null +++ b/internal/fs/security.go @@ -0,0 +1,12 @@ +package fs + +// https://learn.microsoft.com/en-us/windows/win32/api/winnt/ne-winnt-security_impersonation_level +type SecurityImpersonationLevel int32 // C default enums underlying type is `int`, which is Go `int32` + +// Impersonation levels +const ( + SecurityAnonymous SecurityImpersonationLevel = 0 + SecurityIdentification SecurityImpersonationLevel = 1 + SecurityImpersonation SecurityImpersonationLevel = 2 + SecurityDelegation SecurityImpersonationLevel = 3 +) diff --git a/internal/fs/zsyscall_windows.go b/internal/fs/zsyscall_windows.go new file mode 100644 index 00000000..e2f7bb24 --- /dev/null +++ b/internal/fs/zsyscall_windows.go @@ -0,0 +1,64 @@ +//go:build windows + +// Code generated by 'go generate' using "github.com/Microsoft/go-winio/tools/mkwinsyscall"; DO NOT EDIT. + +package fs + +import ( + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +var _ unsafe.Pointer + +// Do the interface allocations only once for common +// Errno values. +const ( + errnoERROR_IO_PENDING = 997 +) + +var ( + errERROR_IO_PENDING error = syscall.Errno(errnoERROR_IO_PENDING) + errERROR_EINVAL error = syscall.EINVAL +) + +// errnoErr returns common boxed Errno values, to prevent +// allocations at runtime. +func errnoErr(e syscall.Errno) error { + switch e { + case 0: + return errERROR_EINVAL + case errnoERROR_IO_PENDING: + return errERROR_IO_PENDING + } + // TODO: add more here, after collecting data on the common + // error values see on Windows. (perhaps when running + // all.bat?) + return e +} + +var ( + modkernel32 = windows.NewLazySystemDLL("kernel32.dll") + + procCreateFileW = modkernel32.NewProc("CreateFileW") +) + +func CreateFile(name string, access AccessMask, mode FileShareMode, sa *syscall.SecurityAttributes, createmode FileCreationDisposition, attrs FileFlagOrAttribute, templatefile windows.Handle) (handle windows.Handle, err error) { + var _p0 *uint16 + _p0, err = syscall.UTF16PtrFromString(name) + if err != nil { + return + } + return _CreateFile(_p0, access, mode, sa, createmode, attrs, templatefile) +} + +func _CreateFile(name *uint16, access AccessMask, mode FileShareMode, sa *syscall.SecurityAttributes, createmode FileCreationDisposition, attrs FileFlagOrAttribute, templatefile windows.Handle) (handle windows.Handle, err error) { + r0, _, e1 := syscall.Syscall9(procCreateFileW.Addr(), 7, uintptr(unsafe.Pointer(name)), uintptr(access), uintptr(mode), uintptr(unsafe.Pointer(sa)), uintptr(createmode), uintptr(attrs), uintptr(templatefile), 0, 0) + handle = windows.Handle(r0) + if handle == windows.InvalidHandle { + err = errnoErr(e1) + } + return +} diff --git a/internal/stringbuffer/wstring.go b/internal/stringbuffer/wstring.go new file mode 100644 index 00000000..c6045708 --- /dev/null +++ b/internal/stringbuffer/wstring.go @@ -0,0 +1,132 @@ +package stringbuffer + +import ( + "sync" + "unicode/utf16" +) + +// TODO: worth exporting and using in mkwinsyscall? + +// Uint16BufferSize is the buffer size in the pool, chosen somewhat arbitrarily to accommodate +// large path strings: +// MAX_PATH (260) + size of volume GUID prefix (49) + null terminator = 310. +const MinWStringCap = 310 + +// use *[]uint16 since []uint16 creates an extra allocation where the slice header +// is copied to heap and then referenced via pointer in the interface header that sync.Pool +// stores. +var pathPool = sync.Pool{ //! if go1.18+ adds Pool[T], use that to store []uint16 directly + New: func() interface{} { + b := make([]uint16, MinWStringCap) + return &b + }, +} + +func newBuffer() []uint16 { return *(pathPool.Get().(*[]uint16)) } + +// freeBuffer copies the slice header data, and puts a pointer to that in the pool. +// This avoids taking a pointer to the slice header in WString, which can be set to nil. +func freeBuffer(b []uint16) { pathPool.Put(&b) } + +// WString is a wide string buffer ([]uint16) meant for storing UTF-16 encoded strings +// for interacting with Win32 APIs. +// Sizes are specified as uint32 and not int. +// +// It is not thread safe. +type WString struct { + // type-def allows casting to []uint16 directly, use struct to prevent that and allow adding fields in the future. + + // raw buffer + b []uint16 +} + +// NewWString returns a [WString] allocated from a shared pool with an +// initial capacity of at least [MinWStringCap]. +// Since the buffer may have been previously used, its contents are not guaranteed to be empty. +// +// The buffer should be freed via [WString.Free] +func NewWString() *WString { + return &WString{ + b: newBuffer(), + } +} + +func (b *WString) Free() { + if b.empty() { + return + } + freeBuffer(b.b) + b.b = nil +} + +// ResizeTo grows the buffer to at least c and returns the new capacity, freeing the +// previous buffer back into pool. +func (b *WString) ResizeTo(c uint32) uint32 { + // allready sufficient (or n is 0) + if c <= b.Cap() { + return b.Cap() + } + + if c <= MinWStringCap { + c = MinWStringCap + } + // allocate at-least double buffer size, as is done in [bytes.Buffer] and other places + if c <= 2*b.Cap() { + c = 2 * b.Cap() + } + + b2 := make([]uint16, c) + if !b.empty() { + copy(b2, b.b) + freeBuffer(b.b) + } + b.b = b2 + return c +} + +// Buffer returns the underlying []uint16 buffer. +func (b *WString) Buffer() []uint16 { + if b.empty() { + return nil + } + return b.b +} + +// Pointer returns a pointer to the first uint16 in the buffer. +// If the [WString.Free] has already been called, the pointer will be nil. +func (b *WString) Pointer() *uint16 { + if b.empty() { + return nil + } + return &b.b[0] +} + +// String returns the returns the UTF-8 encoding of the UTF-16 string in the buffer. +// +// It assumes that the data is null-terminated. +func (b *WString) String() string { + // Using [windows.UTF16ToString] would require importing "golang.org/x/sys/windows" + // and would make this code Windows-only, which makes no sense. + // So copy UTF16ToString code into here. + //! If other windows-specific code is added, switch to [windows.UTF16ToString] + + s := b.b + for i, v := range s { + if v == 0 { + s = s[:i] + break + } + } + return string(utf16.Decode(s)) +} + +// Cap returns the underlying buffer capacity. +func (b *WString) Cap() uint32 { + if b.empty() { + return 0 + } + return b.cap() +} + +func (b *WString) cap() uint32 { return uint32(cap(b.b)) } +func (b *WString) empty() bool { return b == nil || b.cap() == 0 } diff --git a/internal/stringbuffer/wstring_test.go b/internal/stringbuffer/wstring_test.go new file mode 100644 index 00000000..7772f628 --- /dev/null +++ b/internal/stringbuffer/wstring_test.go @@ -0,0 +1,45 @@ +//go:build windows + +package stringbuffer + +import "testing" + +func Test_BufferCapacity(t *testing.T) { + b := NewWString() + + c := b.Cap() + if c < MinWStringCap { + t.Fatalf("expected capacity >= %d, got %d", MinWStringCap, c) + } + + if l := len(b.b); l != int(c) { + t.Fatalf("buffer length (%d) and capacity (%d) mismatch", l, c) + } + + n := uint32(1.5 * MinWStringCap) + nn := b.ResizeTo(n) + if len(b.b) != int(nn) { + t.Fatalf("resized buffer should be %d, was %d", nn, len(b.b)) + } + if n > nn { + t.Fatalf("resized to a value smaller than requested") + } +} + +func Test_BufferFree(t *testing.T) { + // make sure free-ing doesn't set pooled buffer to nil as well + for i := 0; i < 256; i++ { + // try allocating and freeing repeatedly since pool does not guarantee item reuse + b := NewWString() + b.Free() + if b.b != nil { + t.Fatalf("freed buffer is not nil") + } + + b = NewWString() + c := b.Cap() + if c < MinWStringCap { + t.Fatalf("expected capacity >= %d, got %d", MinWStringCap, c) + } + } +} diff --git a/pipe.go b/pipe.go index b234cdf8..ee9fc1ea 100644 --- a/pipe.go +++ b/pipe.go @@ -16,11 +16,12 @@ import ( "unsafe" "golang.org/x/sys/windows" + + "github.com/Microsoft/go-winio/internal/fs" ) //sys connectNamedPipe(pipe syscall.Handle, o *syscall.Overlapped) (err error) = ConnectNamedPipe //sys createNamedPipe(name string, flags uint32, pipeMode uint32, maxInstances uint32, outSize uint32, inSize uint32, defaultTimeout uint32, sa *syscall.SecurityAttributes) (handle syscall.Handle, err error) [failretval==syscall.InvalidHandle] = CreateNamedPipeW -//sys createFile(name string, access uint32, mode uint32, sa *syscall.SecurityAttributes, createmode uint32, attrs uint32, templatefile syscall.Handle) (handle syscall.Handle, err error) [failretval==syscall.InvalidHandle] = CreateFileW //sys getNamedPipeInfo(pipe syscall.Handle, flags *uint32, outSize *uint32, inSize *uint32, maxInstances *uint32) (err error) = GetNamedPipeInfo //sys getNamedPipeHandleState(pipe syscall.Handle, state *uint32, curInstances *uint32, maxCollectionCount *uint32, collectDataTimeout *uint32, userName *uint16, maxUserNameSize uint32) (err error) = GetNamedPipeHandleStateW //sys localAlloc(uFlags uint32, length uint32) (ptr uintptr) = LocalAlloc @@ -163,19 +164,21 @@ func (s pipeAddress) String() string { } // tryDialPipe attempts to dial the pipe at `path` until `ctx` cancellation or timeout. -func tryDialPipe(ctx context.Context, path *string, access uint32) (syscall.Handle, error) { +func tryDialPipe(ctx context.Context, path *string, access fs.AccessMask) (syscall.Handle, error) { for { select { case <-ctx.Done(): return syscall.Handle(0), ctx.Err() default: - h, err := createFile(*path, + wh, err := fs.CreateFile(*path, access, - 0, - nil, - syscall.OPEN_EXISTING, - windows.FILE_FLAG_OVERLAPPED|windows.SECURITY_SQOS_PRESENT|windows.SECURITY_ANONYMOUS, - 0) + 0, // mode + nil, // security attributes + fs.OPEN_EXISTING, + fs.FILE_FLAG_OVERLAPPED|fs.SECURITY_SQOS_PRESENT|fs.SECURITY_ANONYMOUS, + 0, //template file handle + ) + h := syscall.Handle(wh) if err == nil { return h, nil } @@ -219,7 +222,7 @@ func DialPipeContext(ctx context.Context, path string) (net.Conn, error) { func DialPipeAccess(ctx context.Context, path string, access uint32) (net.Conn, error) { var err error var h syscall.Handle - h, err = tryDialPipe(ctx, &path, access) + h, err = tryDialPipe(ctx, &path, fs.AccessMask(access)) if err != nil { return nil, err } diff --git a/pkg/fs/doc.go b/pkg/fs/doc.go new file mode 100644 index 00000000..1f653881 --- /dev/null +++ b/pkg/fs/doc.go @@ -0,0 +1,2 @@ +// This package contains Win32 filesystem functionality. +package fs diff --git a/pkg/fs/fs_windows.go b/pkg/fs/fs_windows.go index 7a73aafb..92cec8c2 100644 --- a/pkg/fs/fs_windows.go +++ b/pkg/fs/fs_windows.go @@ -5,6 +5,8 @@ import ( "path/filepath" "golang.org/x/sys/windows" + + "github.com/Microsoft/go-winio/internal/stringbuffer" ) var ( @@ -13,19 +15,18 @@ var ( ) // GetFileSystemType obtains the type of a file system through GetVolumeInformation. -// https://msdn.microsoft.com/en-us/library/windows/desktop/aa364993(v=vs.85).aspx +// +// https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getvolumeinformationw func GetFileSystemType(path string) (fsType string, err error) { drive := filepath.VolumeName(path) if len(drive) != 2 { return "", ErrInvalidPath } - var ( - buf = make([]uint16, 255) - size = uint32(windows.MAX_PATH + 1) - ) + buf := stringbuffer.NewWString() + defer buf.Free() + drive += `\` - err = windows.GetVolumeInformation(windows.StringToUTF16Ptr(drive), nil, 0, nil, nil, nil, &buf[0], size) - fsType = windows.UTF16ToString(buf) - return fsType, err + err = windows.GetVolumeInformation(windows.StringToUTF16Ptr(drive), nil, 0, nil, nil, nil, buf.Pointer(), buf.Cap()) + return buf.String(), err } diff --git a/pkg/fs/resolve.go b/pkg/fs/resolve.go new file mode 100644 index 00000000..b876c4c0 --- /dev/null +++ b/pkg/fs/resolve.go @@ -0,0 +1,128 @@ +//go:build windows + +package fs + +import ( + "errors" + "os" + "strings" + + "golang.org/x/sys/windows" + + "github.com/Microsoft/go-winio/internal/fs" +) + +// ResolvePath returns the final path to a file or directory represented, resolving symlinks, +// handling mount points, etc. +// The resolution works by using the Windows API GetFinalPathNameByHandle, which takes a +// handle and returns the final path to that file. +// +// It is intended to address short-comings of [filepath.EvalSymlinks], which does not work +// well on Windows. +func ResolvePath(path string) (string, error) { + // We are not able to use builtin Go functionality for opening a directory path: + // - os.Open on a directory returns a os.File where Fd() is a search handle from FindFirstFile. + // - syscall.Open does not provide a way to specify FILE_FLAG_BACKUP_SEMANTICS, which is needed to + // open a directory. + // + // We could use os.Open if the path is a file, but it's easier to just use the same code for both. + // Therefore, we call windows.CreateFile directly. + h, err := fs.CreateFile( + path, + fs.FILE_ANY_ACCESS, // access + fs.FILE_SHARE_READ|fs.FILE_SHARE_WRITE|fs.FILE_SHARE_DELETE, + nil, // security attributes + fs.OPEN_EXISTING, + fs.FILE_FLAG_BACKUP_SEMANTICS, // Needed to open a directory handle. + fs.NullHandle, // template file + ) + if err != nil { + return "", &os.PathError{ + Op: "CreateFile", + Path: path, + Err: err, + } + } + defer windows.CloseHandle(h) //nolint:errcheck + + // We use the Windows API GetFinalPathNameByHandle to handle path resolution. GetFinalPathNameByHandle + // returns a resolved path name for a file or directory. The returned path can be in several different + // formats, based on the flags passed. There are several goals behind the design here: + // - Do as little manual path manipulation as possible. Since Windows path formatting can be quite + // complex, we try to just let the Windows APIs handle that for us. + // - Retain as much compatibility with existing Go path functions as we can. In particular, we try to + // ensure paths returned from resolvePath can be passed to EvalSymlinks. + // + // First, we query for the VOLUME_NAME_GUID path of the file. This will return a path in the form + // "\\?\Volume{8a25748f-cf34-4ac6-9ee2-c89400e886db}\dir\file.txt". If the path is a UNC share + // (e.g. "\\server\share\dir\file.txt"), then the VOLUME_NAME_GUID query will fail with ERROR_PATH_NOT_FOUND. + // In this case, we will next try a VOLUME_NAME_DOS query. This query will return a path for a UNC share + // in the form "\\?\UNC\server\share\dir\file.txt". This path will work with most functions, but EvalSymlinks + // fails on it. Therefore, we rewrite the path to the form "\\server\share\dir\file.txt" before returning it. + // This path rewrite may not be valid in all cases (see the notes in the next paragraph), but those should + // be very rare edge cases, and this case wouldn't have worked with EvalSymlinks anyways. + // + // The "\\?\" prefix indicates that no path parsing or normalization should be performed by Windows. + // Instead the path is passed directly to the object manager. The lack of parsing means that "." and ".." are + // interpreted literally and "\"" must be used as a path separator. Additionally, because normalization is + // not done, certain paths can only be represented in this format. For instance, "\\?\C:\foo." (with a trailing .) + // cannot be written as "C:\foo.", because path normalization will remove the trailing ".". + // + // FILE_NAME_NORMALIZED can fail on some UNC paths based on access restrictions. + // Attempt to query with FILE_NAME_NORMALIZED, and then fall back on FILE_NAME_OPENED if access is denied. + // + // Querying for VOLUME_NAME_DOS first instead of VOLUME_NAME_GUID would yield a "nicer looking" path in some cases. + // For instance, it could return "\\?\C:\dir\file.txt" instead of "\\?\Volume{8a25748f-cf34-4ac6-9ee2-c89400e886db}\dir\file.txt". + // However, we query for VOLUME_NAME_GUID first for two reasons: + // - The volume GUID path is more stable. A volume's mount point can change when it is remounted, but its + // volume GUID should not change. + // - If the volume is mounted at a non-drive letter path (e.g. mounted to "C:\mnt"), then VOLUME_NAME_DOS + // will return the mount path. EvalSymlinks fails on a path like this due to a bug. + // + // References: + // - GetFinalPathNameByHandle: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfinalpathnamebyhandlea + // - Naming Files, Paths, and Namespaces: https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file + // - Naming a Volume: https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-volume + + normalize := true + guid := true + rPath := "" + for i := 1; i <= 4; i++ { // maximum of 4 different cases to try + var flags fs.GetFinalPathFlag + if normalize { + flags |= fs.FILE_NAME_NORMALIZED // nop; for clarity + } else { + flags |= fs.FILE_NAME_OPENED + } + + if guid { + flags |= fs.VOLUME_NAME_GUID + } else { + flags |= fs.VOLUME_NAME_DOS // nop; for clarity + } + + rPath, err = fs.GetFinalPathNameByHandle(h, flags) + switch { + case guid && errors.Is(err, windows.ERROR_PATH_NOT_FOUND): + // ERROR_PATH_NOT_FOUND is returned from the VOLUME_NAME_GUID query if the path is a + // network share (UNC path). In this case, query for the DOS name instead. + guid = false + continue + case normalize && errors.Is(err, windows.ERROR_ACCESS_DENIED): + // normalization failed when accessing individual components along path for SMB share + normalize = false + continue + default: + } + break + } + + if err == nil && strings.HasPrefix(rPath, `\\?\UNC\`) { + // Convert \\?\UNC\server\share -> \\server\share. The \\?\UNC syntax does not work with + // some Go filepath functions such as EvalSymlinks. In the future if other components + // move away from EvalSymlinks and use GetFinalPathNameByHandle instead, we could remove + // this path munging. + rPath = `\\` + rPath[len(`\\?\UNC\`):] + } + return rPath, err +} diff --git a/zsyscall_windows.go b/zsyscall_windows.go index 83f45a13..469b16f6 100644 --- a/zsyscall_windows.go +++ b/zsyscall_windows.go @@ -63,7 +63,6 @@ var ( procBackupWrite = modkernel32.NewProc("BackupWrite") procCancelIoEx = modkernel32.NewProc("CancelIoEx") procConnectNamedPipe = modkernel32.NewProc("ConnectNamedPipe") - procCreateFileW = modkernel32.NewProc("CreateFileW") procCreateIoCompletionPort = modkernel32.NewProc("CreateIoCompletionPort") procCreateNamedPipeW = modkernel32.NewProc("CreateNamedPipeW") procGetCurrentThread = modkernel32.NewProc("GetCurrentThread") @@ -305,24 +304,6 @@ func connectNamedPipe(pipe syscall.Handle, o *syscall.Overlapped) (err error) { return } -func createFile(name string, access uint32, mode uint32, sa *syscall.SecurityAttributes, createmode uint32, attrs uint32, templatefile syscall.Handle) (handle syscall.Handle, err error) { - var _p0 *uint16 - _p0, err = syscall.UTF16PtrFromString(name) - if err != nil { - return - } - return _createFile(_p0, access, mode, sa, createmode, attrs, templatefile) -} - -func _createFile(name *uint16, access uint32, mode uint32, sa *syscall.SecurityAttributes, createmode uint32, attrs uint32, templatefile syscall.Handle) (handle syscall.Handle, err error) { - r0, _, e1 := syscall.Syscall9(procCreateFileW.Addr(), 7, uintptr(unsafe.Pointer(name)), uintptr(access), uintptr(mode), uintptr(unsafe.Pointer(sa)), uintptr(createmode), uintptr(attrs), uintptr(templatefile), 0, 0) - handle = syscall.Handle(r0) - if handle == syscall.InvalidHandle { - err = errnoErr(e1) - } - return -} - func createIoCompletionPort(file syscall.Handle, port syscall.Handle, key uintptr, threadCount uint32) (newport syscall.Handle, err error) { r0, _, e1 := syscall.Syscall6(procCreateIoCompletionPort.Addr(), 4, uintptr(file), uintptr(port), uintptr(key), uintptr(threadCount), 0, 0) newport = syscall.Handle(r0) From 070c828abb873da9e71c7247740253b50f7cf049 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy <84944216+helsaawy@users.noreply.github.com> Date: Fri, 14 Apr 2023 15:24:28 -0400 Subject: [PATCH 9/9] update linter and fix lints (#284) Signed-off-by: Hamza El-Saawy --- .github/workflows/ci.yml | 44 ++++++++++++++++++++++-------- .golangci.yml | 12 ++++++++ hvsock.go | 6 ++-- internal/socket/socket.go | 4 +-- internal/stringbuffer/wstring.go | 4 +-- pipe.go | 2 +- pkg/etw/eventdescriptor.go | 2 -- pkg/security/grantvmgroupaccess.go | 2 -- wim/lzx/lzx.go | 11 ++++---- 9 files changed, 58 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bbd32daf..c6b529fa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,21 +4,26 @@ on: - pull_request env: - GO_VERSION: "1.17" + GO_VERSION: "oldstable" + GOTESTSUM_VERSION: "latest" jobs: lint: name: Lint runs-on: windows-2019 steps: - - uses: actions/checkout@v3 - - uses: actions/setup-go@v3 + - name: Checkout + uses: actions/checkout@v3 + + - name: Install go + uses: actions/setup-go@v4 with: go-version: ${{ env.GO_VERSION }} + - name: Run golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.50 + version: v1.52 args: >- --verbose --timeout=5m @@ -31,10 +36,14 @@ jobs: name: Go Generate runs-on: windows-2019 steps: - - uses: actions/checkout@v3 - - uses: actions/setup-go@v3 + - name: Checkout + uses: actions/checkout@v3 + + - name: Install go + uses: actions/setup-go@v4 with: go-version: ${{ env.GO_VERSION }} + - name: Run go generate shell: pwsh run: | @@ -45,6 +54,7 @@ jobs: Write-Output "::error title=Go Generate::Error running go generate." exit $LASTEXITCODE } + - name: Diff shell: pwsh run: | @@ -66,11 +76,19 @@ jobs: matrix: os: [windows-2019, windows-2022, ubuntu-latest] steps: - - uses: actions/checkout@v3 - - uses: actions/setup-go@v3 + - name: Checkout + uses: actions/checkout@v3 + + - name: Install go + uses: actions/setup-go@v4 with: go-version: ${{ env.GO_VERSION }} - - run: go test -gcflags=all=-d=checkptr -v ./... + + - name: Install gotestsum + run: go install gotest.tools/gotestsum@${{ env.GOTESTSUM_VERSION }} + + - name: Test repo + run: gotestsum --format standard-verbose --debug -- -gcflags=all=-d=checkptr -v ./... build: name: Build Repo @@ -78,10 +96,14 @@ jobs: - test runs-on: "windows-2019" steps: - - uses: actions/checkout@v3 - - uses: actions/setup-go@v3 + - name: Checkout + uses: actions/checkout@v3 + + - name: Install go + uses: actions/setup-go@v4 with: go-version: ${{ env.GO_VERSION }} + - run: go build ./pkg/etw/sample/ - run: go build ./tools/etw-provider-gen/ - run: go build ./tools/mkwinsyscall/ diff --git a/.golangci.yml b/.golangci.yml index 2bf84e32..7b503d26 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -35,6 +35,18 @@ issues: text: "^line-length-limit: " source: "^//(go:generate|sys) " + #TODO: remove after upgrading to go1.18 + # ignore comment spacing for nolint and sys directives + - linters: + - revive + text: "^comment-spacings: no space between comment delimiter and comment text" + source: "//(cspell:|nolint:|sys |todo)" + + # not on go 1.18 yet, so no any + - linters: + - revive + text: "^use-any: since GO 1.18 'interface{}' can be replaced by 'any'" + # allow unjustified ignores of error checks in defer statements - linters: - nolintlint diff --git a/hvsock.go b/hvsock.go index 52f1c280..c8819165 100644 --- a/hvsock.go +++ b/hvsock.go @@ -23,7 +23,7 @@ import ( const afHVSock = 34 // AF_HYPERV // Well known Service and VM IDs -//https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/user-guide/make-integration-service#vmid-wildcards +// https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/user-guide/make-integration-service#vmid-wildcards // HvsockGUIDWildcard is the wildcard VmId for accepting connections from all partitions. func HvsockGUIDWildcard() guid.GUID { // 00000000-0000-0000-0000-000000000000 @@ -31,7 +31,7 @@ func HvsockGUIDWildcard() guid.GUID { // 00000000-0000-0000-0000-000000000000 } // HvsockGUIDBroadcast is the wildcard VmId for broadcasting sends to all partitions. -func HvsockGUIDBroadcast() guid.GUID { //ffffffff-ffff-ffff-ffff-ffffffffffff +func HvsockGUIDBroadcast() guid.GUID { // ffffffff-ffff-ffff-ffff-ffffffffffff return guid.GUID{ Data1: 0xffffffff, Data2: 0xffff, @@ -246,7 +246,7 @@ func (l *HvsockListener) Accept() (_ net.Conn, err error) { var addrbuf [addrlen * 2]byte var bytes uint32 - err = syscall.AcceptEx(l.sock.handle, sock.handle, &addrbuf[0], 0 /*rxdatalen*/, addrlen, addrlen, &bytes, &c.o) + err = syscall.AcceptEx(l.sock.handle, sock.handle, &addrbuf[0], 0 /* rxdatalen */, addrlen, addrlen, &bytes, &c.o) if _, err = l.sock.asyncIO(c, nil, bytes, err); err != nil { return nil, l.opErr("accept", os.NewSyscallError("acceptex", err)) } diff --git a/internal/socket/socket.go b/internal/socket/socket.go index 39e8c05f..aeb7b725 100644 --- a/internal/socket/socket.go +++ b/internal/socket/socket.go @@ -100,8 +100,8 @@ func (f *runtimeFunc) Load() error { (*byte)(unsafe.Pointer(&f.addr)), uint32(unsafe.Sizeof(f.addr)), &n, - nil, //overlapped - 0, //completionRoutine + nil, // overlapped + 0, // completionRoutine ) }) return f.err diff --git a/internal/stringbuffer/wstring.go b/internal/stringbuffer/wstring.go index c6045708..7ad50570 100644 --- a/internal/stringbuffer/wstring.go +++ b/internal/stringbuffer/wstring.go @@ -15,7 +15,7 @@ const MinWStringCap = 310 // use *[]uint16 since []uint16 creates an extra allocation where the slice header // is copied to heap and then referenced via pointer in the interface header that sync.Pool // stores. -var pathPool = sync.Pool{ //! if go1.18+ adds Pool[T], use that to store []uint16 directly +var pathPool = sync.Pool{ // if go1.18+ adds Pool[T], use that to store []uint16 directly New: func() interface{} { b := make([]uint16, MinWStringCap) return &b @@ -108,7 +108,7 @@ func (b *WString) String() string { // Using [windows.UTF16ToString] would require importing "golang.org/x/sys/windows" // and would make this code Windows-only, which makes no sense. // So copy UTF16ToString code into here. - //! If other windows-specific code is added, switch to [windows.UTF16ToString] + // If other windows-specific code is added, switch to [windows.UTF16ToString] s := b.b for i, v := range s { diff --git a/pipe.go b/pipe.go index ee9fc1ea..25cc8110 100644 --- a/pipe.go +++ b/pipe.go @@ -176,7 +176,7 @@ func tryDialPipe(ctx context.Context, path *string, access fs.AccessMask) (sysca nil, // security attributes fs.OPEN_EXISTING, fs.FILE_FLAG_OVERLAPPED|fs.SECURITY_SQOS_PRESENT|fs.SECURITY_ANONYMOUS, - 0, //template file handle + 0, // template file handle ) h := syscall.Handle(wh) if err == nil { diff --git a/pkg/etw/eventdescriptor.go b/pkg/etw/eventdescriptor.go index 0dd11b45..ef29ca36 100644 --- a/pkg/etw/eventdescriptor.go +++ b/pkg/etw/eventdescriptor.go @@ -47,8 +47,6 @@ const ( ) // EventDescriptor represents various metadata for an ETW event. -// -//nolint:structcheck // task is currently unused type eventDescriptor struct { id uint16 version uint8 diff --git a/pkg/security/grantvmgroupaccess.go b/pkg/security/grantvmgroupaccess.go index 6df87b74..bb276fff 100644 --- a/pkg/security/grantvmgroupaccess.go +++ b/pkg/security/grantvmgroupaccess.go @@ -21,7 +21,6 @@ type ( trusteeForm uint32 trusteeType uint32 - //nolint:structcheck // structcheck thinks fields are unused, but the are used to pass data to OS explicitAccess struct { accessPermissions accessMask accessMode accessMode @@ -29,7 +28,6 @@ type ( trustee trustee } - //nolint:structcheck,unused // structcheck thinks fields are unused, but the are used to pass data to OS trustee struct { multipleTrustee *trustee multipleTrusteeOperation int32 diff --git a/wim/lzx/lzx.go b/wim/lzx/lzx.go index f7cf69a9..e5db8260 100644 --- a/wim/lzx/lzx.go +++ b/wim/lzx/lzx.go @@ -489,15 +489,14 @@ func (f *decompressor) readCompressedBlock(start, end uint16, hmain, hlength, ha f.lru[0] = matchoffset } - if matchoffset <= i && matchlen <= end-i { - copyend := i + matchlen - for ; i < copyend; i++ { - f.window[i] = f.window[i-matchoffset] - } - } else { + if !(matchoffset <= i && matchlen <= end-i) { f.fail(errCorrupt) break } + copyend := i + matchlen + for ; i < copyend; i++ { + f.window[i] = f.window[i-matchoffset] + } } return int(i - start), f.err }