I would like to use impersonation on the server side of the pipe, but I
currently cannot because all pipes are connected using the anonymous
impersonation level.
This patch adds a new function that provides the ability to explicitly
specify the desired impersonation level.
Fixes#297
Signed-off-by: Aaron Klotz <aaron@tailscale.com>
Where ever possible, use `golang.org/x/sys/windows` instead of `syscall`
(which has been deprecated since go1.11).
Using `windows.LocalFree` requires using `unsafe.Pointer`, which ensures
that the Go garbage collector does not try to free memory pre-maturely
if it was previously declared as a pointer.
Since `syscall.Handle` is part of API for `vhd` package, it was left
unchanged.
For security descriptor functions, switch to using
`windows.SECURITY_DESCRIPTOR` to avoid unnecessary byte manipulation and
panics due to missing input validation and error checking.
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
* 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 <hamzaelsaawy@microsoft.com>
* PR: types, documentation
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
* remove unneded constant groups
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
* 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 <hamzaelsaawy@microsoft.com>
---------
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
* Add lint and go generate stages to CI
Add CI step to verify `go generate` was run on repo.
Add linter stage to CI along with linter config file,
`.golangci.yml`.
Will likely prefer revive over static-check.
Updated README Contributing section on linting requirements.
Added sequence ordering to make sure lint and go generate stages run
before tests and build.
This way, build and tests are not run on code that could potentially:
1. not build due to `gofmt` issues;
2. contain bugs;
3. have to be re-submitted after issues are fixed; or
4. contain outdated Win32 syscall or other auto-generated files.
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
* Fixed linter issues
Code changes to satisfy linters:
- Ran `gofmt -s -w` on repo.
- Broke up long lines.
- When possible, changed names with incorrect initialism formatting
- Added exceptions for exported variables.
- Added exceptions for ALL_CAPS_WITH_UNDERSCORES code.
- Switched to using `windows` or `syscall` definitions if possible;
especially if some constants were unused.
- Added `_ =` to satisfy error linter, and acknowledge that errors are
being ignored.
- Switched to using `errors.Is` and `As` in places, elsewhere added
exceptions if error value was known to be `syscall.Errno`.
- Removed bare returns.
- Prevented variables from being overshadowed in certain places
(ignoring cases of overshadowing `err`).
- Renamed variables and functions (eg, `len`, `eventMetadata.bytes`) to
prevent shadowing pre-built functions and imported pacakges.
- Removed unused method receivers.
- Added exceptions to certain unused (unexported) constants and
functions.
- Deleted unused `once` from `pkg/etw.providerMap`.
- Renamed `noop.go` files to `main_other.go` or `doc.go`, to better fit
style recommendations.
- Added exceptions for non-secure use of SHA1 and weak crypto
libraries.
- Replaced `ioutil` with `io` and `os` (and `t.TempDir` in tests).
- Added fully exhaustive checks for `switch` statements in `pkg/etw`.
- Defined constant strings for `tools/mkwinsyscall`.
- Removed unnecessary conversions.
- Made sure `context.Cancel` was called.
Additionally, added `//go:build windows" constraints on files with
unexported code, since linter will complain about unused code on
non-Windows platforms.
Added a stub `main() {}` for `mkwinsyscall` for non-Windows builds, just in
case `//go:generate` directives are added to OS-agnostic files.
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
* PR: spelling, constants, fuzzing
Moved HVSocket fuzzing tests to separate file with go 1.18 build
constraint.
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
This error used to just be a `errors.New` with the text exactly matching
the "use of closed network connection" that you'd get from the Go stdlib.
Now that that error was exported as `net.ErrClosed` in Go 1.16 we should
be able to swap to this safely, and anyone who relied on checking the
string explicitly should still be fine.
Signed-off-by: Daniel Canter <dcanter@microsoft.com>
1.13 is sufficiently old at this point and net.ErrClosed from 1.16 will be nice
to use.
This change additionally runs go fmt to bring in the new 1.17 build tag syntax
and builds the binaries in our CI with 1.17+.
Signed-off-by: Daniel Canter <dcanter@microsoft.com>
ListenPipe can fail if there is a concurrent DialPipe because there is a
race window where DialPipe can connect to the initial server named pipe
before it is connected to and closed by ListenPipe.
To fix this, use the lower-level NT API for creating the server pipe,
since this API allows for specifying that a server pipe should initially
be in the disconnected state instead of the listening state. This allows
us to avoid the race condition by creating the pipe in the correct state
initially, so there is no longer a need to create a dummy client
connection.
This changes a few things to try to ensure that we never wind up with
a result of ERROR_FILE_NOT_FOUND, due to a race between closing the
last pipe instance and opening the next.
First we keep an "open" client instance (unused) while the listener
is open, so that we are guaranteed to always have an active pipe
instance. This means attempts to open while no other instances exist
result in ERROR_PIPE_BUSY instead of ERROR_FILE_NOT_FOUND.
Second we have changed the loop for dialing to eliminate a race condition
that is more or less inherent in WaitNamedPipe when synchronizing with
CreateFile. The real timeout needs to be some larger value than the
WaitNamedPipe timeout, and furthermore WaitNamedPipe is not very nice
with the Go runtime, since it is a blocking system call. Instead we
just put the goroutine to sleep for 10 milliseconds, and keep retrying
the CreateFile until the maximum timeout is reached. If no timeout is
specified we assume a reasonable and large default of 5 seconds, which is
similar to a TCP connection timeout.
This isn't perfect, as a client attempting to connect to an extremely
busy pipe server can be starved out by other clients coming in while
it is in that brief sleep, but this potential race was already present
with WaitNamedPipe. The numerous retries (by default 500 retries!)
mean its pretty unlikely to occur, and if a single client hits the
race once, it has an excellent chance of getting in the next cycle.
(A real "fix" that is completely race free and fair would require
changes in the underlying Named Pipe implementation, or some other
kind of external coordination.)
This change resolves the issue where Accept() returns ERROR_NO_DATA
because the client closes the connection immediately. It resolves
the race by ignoring that particular connection and waiting on a
new one.
In order to upgrade to go1.8 all IO operations
including previous pending IO must have the deadline
set when SetReadDeadline or SetWriteDeadline is called
Signed-off-by: Darren Stahl <darst@microsoft.com>
This forces any pending data to be sent to the remote party before
emitting the 0-length EOF message, to make sure it appears as a seperate
message.
This fixes the "TestEchoWithMessaging" test
Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>
This change adds support for message mode pipes and uses them to support
CloseWrite() to better match TCP and UNIX sockets. Message mode pipes
support writing and (optionally) reading data in message-sized chunks.
This is useful for us because when in this mode a zero-sized message can
be read. We use this zero-sized message to signal that no more writes will
arrive. This is not standard practice in Windows, but it is a reasonable
compromise.
This ensures that the pipe server cannot impersonate or even identify
the user that has connected to the pipe. This is a good default to
prevent malicious squatting on a named pipe name.
Signed-off-by: John Starks <jostarks@microsoft.com>
This includes Reader, Writer, and Closer interfaces on top of
Windows file handles in a way that does not block the system thread
while the IO is outstanding.
This also includes net.Listener and net.Conn for named pipes.
Aaron Klotz