-
Notifications
You must be signed in to change notification settings - Fork 455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Windows support: Explicitly deny access to remote callers in exposed named pipes endpoints #3236
Windows support: Explicitly deny access to remote callers in exposed named pipes endpoints #3236
Conversation
…ints Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
…ints Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
_, err = util.GRPCDialContext(ctx, targetAsRemote, grpc.WithBlock(), grpc.FailOnNonTempDialError(true)) | ||
|
||
// Remote calls must be denied | ||
require.True(t, errors.Is(err, windows.ERROR_ACCESS_DENIED)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe you can use require.ErrorIs()
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohhh, TIL that exists!
_, err = util.GRPCDialContext(ctx, targetAsRemote, grpc.WithBlock(), grpc.FailOnNonTempDialError(true)) | ||
|
||
// Remote calls must be denied | ||
require.True(t, errors.Is(err, windows.ERROR_ACCESS_DENIED)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as before
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\o/
…named pipes endpoints (spiffe#3236) * Explicitly deny access to remote callers in exposed named pipes endpoints Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Our named pipes endpoints are designed to be accessed locally only, and shouldn't be exposed to remote callers.
We are currently relying on the implementation of
winio.ListenPipe
to deny access to remote callers of the exposed named pipes endpoints. This is done through the FILE_PIPE_REJECT_REMOTE_CLIENTS option when calling the NtCreateNamedPipeFile function.Ideally, we shouldn't rely on the behavior of an external library to have this security property. Microsoft recommends to deny access to NT AUTHORITY\NETWORK if the pipe is intended to be accessed only locally.
This changes updates the security descriptors used to establish the security properties of the named pipes to explicitly deny access to
NT AUTHORITY\NETWORK
. This is a group identifier added to the token of a process when it was logged on across a network.Additional testing was added to make sure that if the endpoints are exposed to remote callers, we catch that through our tests.