From bbb321c64802642cbed77d78418147ac195de6e7 Mon Sep 17 00:00:00 2001 From: Steven Rhodes Date: Mon, 27 Nov 2023 12:50:59 -0800 Subject: [PATCH] Implement proxied MPA support (#374) This adds support for MPA requests that go through the proxy, building upon the support that we have for direct MPA requests. I've updated documentation with examples and I've added an integration test. Some notes on implementation quirks: - Our telemetry handler passes through the metadata specifying the MPA request id from the proxy to the server, triggering its MPA authz hook if the method matches. This works fine because the server will look at proxied identity information for deciding if the request id matches the stored data. - Our telemetry handler also passes through the justification metadata. If someone forgets to add the handler that does this, justification isn't stored in the MPA request and the proxy MPA authz hook will fail due to the mismatch between the stored action and the requested action. - sanssh will wait for all targets to be approved before running the final command on any target. Fixes https://github.com/Snowflake-Labs/sansshell/issues/346 --- README.md | 55 +++-- cmd/proxy-server/default-policy.rego | 11 + cmd/proxy-server/main.go | 3 + cmd/sanssh/client/client.go | 4 + cmd/sansshell-server/default-policy.rego | 14 +- cmd/sansshell-server/main.go | 3 + services/mpa/README.md | 57 ++++-- services/mpa/mpahooks/mpahooks.go | 147 +++++++++++++- services/mpa/mpahooks/mpahooks_test.go | 243 +++++++++++++++++++++++ services/mpa/server/server.go | 7 + services/mpa/server/server_test.go | 2 +- services/mpa/testdata/README.md | 19 ++ services/mpa/testdata/proxy.key | 28 +++ services/mpa/testdata/proxy.pem | 18 ++ testing/integrate.sh | 9 +- 15 files changed, 579 insertions(+), 41 deletions(-) create mode 100644 services/mpa/testdata/proxy.key create mode 100644 services/mpa/testdata/proxy.pem diff --git a/README.md b/README.md index 6ddc5a07..4db0adb9 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ sanssh is a simple CLI with a friendly API for dumping debugging state and interacting with a remote machine. It also includes a set of convenient but perhaps-less-friendly subcommands to address the raw SansShell API endpoints. -# Getting Started +## Getting Started How to set up, build and run locally for testing. All commands are relative to the project root directory. @@ -66,7 +66,7 @@ the project root directory. Building SansShell requires a recent version of Go (check the go.mod file for the current version). -## Build and run +### Build and run You need to populate ~/.sansshell with certificates before running. @@ -91,7 +91,7 @@ $ go run ./cmd/sanssh --proxy=localhost:50043 --targets=localhost:50042 file rea Minimal debugging UIs are available at http://localhost:50044 for the server and http://localhost:50046 for the proxy by default. -## Environment setup : protoc +### Environment setup : protoc When making any change to the protocol buffers, you'll also need the protocol buffer compiler (`protoc`) (version 3 or above) as well as the protoc plugins @@ -106,7 +106,7 @@ brew install protobuf On Linux, protoc can be installed using either the OS package manager, or by directly installing a release version from the [protocol buffers github][1] -## Environment setup : protoc plugins. +### Environment setup : protoc plugins On any platform, once protoc has been installed, you can install the required code generation plugins using `go install`. @@ -127,12 +127,12 @@ do this for you, as well as re-generating the service proto files. $ go generate tools.go ``` -## Creating your own certificates +### Creating your own certificates As an alternative to copying auth/mtls/testdata, you can create your own example mTLS certs. See the [mtls testdata readme](/auth/mtls/testdata/README.md) for steps. -## Debugging +### Debugging Reflection is included in the RPC servers (proxy and sansshell-server) allowing for the use of [grpc_cli](https://github.com/grpc/grpc/blob/master/doc/command_line_tool.md). @@ -148,7 +148,7 @@ $ GRPC_DEFAULT_SSL_ROOTS_FILE_PATH=$HOME/.sansshell/root.pem grpc_cli \ NOTE: This connects to the proxy. Change to 50042 if you want to connect to the sansshell-server. -# A tour of the codebase +## A tour of the codebase SansShell is composed of 5 primary concepts: @@ -162,7 +162,7 @@ SansShell is composed of 5 primary concepts: 1. A CLI, which serves as the reference implementation of how to use the services via the agent. -## Services +### Services Services implement at least one gRPC API endpoint, and expose it by calling `RegisterSansShellService` from `init()`. The goal is to allow custom @@ -170,7 +170,7 @@ implementations of the SansShell Server to easily import services they wish to use, and have zero overhead or risk from services they do not import at compile time. -### List of available Services: +#### List of available Services 1. Ansible: Run a local ansible playbook and return output 1. Execute: Execute a command @@ -179,31 +179,32 @@ time. and immutable operations (if OS supported). 1. Package operations: Install, Upgrade, List, Repolist 1. Process operations: List, Get stacks (native or Java), Get dumps (core or Java heap) +1. MPA operations: Multi party authorization for commands 1. Service operations: List, Status, Start/stop/restart TODO: Document service/.../client expectations. -## The Server class +### The Server class Most of the logic of instantiating a local SansShell server lives in the `server` directory. This instantiates a gRPC server, registers the imported services with that server, and constraints them with the supplied OPA policy. -## The reference Proxy Server binary +### The reference Proxy Server binary There is a reference implementation of a SansShell Proxy Server in `cmd/proxy-server`, which should be suitable as-written for many use cases. It's intentionally kept relatively short, so that it can be copied to another repository and customized by adjusting only the imported services. -## The reference Server binary +### The reference Server binary There is a reference implementation of a SansShell Server in -`cmd/sansshell-server`, which should be suitable as-written for many use cases. +`cmd/sansshell-server`, which should be suitable as-written for some use cases. It's intentionally kept relatively short, so that it can be copied to another repository and customized by adjusting only the imported services. -## The reference CLI client +### The reference CLI client There is a reference implementation of a SansShell CLI Client in `cmd/sanssh`. It provides raw access to each gRPC endpoint, as well @@ -222,7 +223,31 @@ autoload -U +X bashcompinit && bashcompinit complete -C /path/to/sanssh -o dirnames sanssh ``` -# Extending SansShell +## Multi party authorization + +MPA, or [multi party authorization](https://en.wikipedia.org/wiki/Multi-party_authorization), +allows guarding sensitive commands behind additional approval. SansShell +supports writing authorization policies that only pass when a command is +approved by additional entities beyond the caller. See +[services/mpa/README.md](/services/mpa/README.md) for details on +implementation and usage. + +To try this out in the reference client, run the following commands in parallel +in separate terminals. This will run a server that accepts any command from a +proxy and a proxy that allows MPA requests from the "sanssh" user when approved by the "approver" user. + +```bash +# Start the server +go run ./cmd/sansshell-server -server-cert ./auth/mtls/testdata/leaf.pem -server-key ./auth/mtls/testdata/leaf.key +# Start the proxy +go run ./cmd/proxy-server -client-cert ./services/mpa/testdata/proxy.pem -client-key ./services/mpa/testdata/proxy.key -server-cert ./services/mpa/testdata/proxy.pem -server-key ./services/mpa/testdata/proxy.key +# Run a command gated on MPA +go run ./cmd/sanssh -client-cert ./auth/mtls/testdata/client.pem -client-key ./auth/mtls/testdata/client.key -mpa -proxy localhost -targets localhost exec run /bin/echo hello world +# Approve the command above +go run ./cmd/sanssh -client-cert ./services/mpa/testdata/approver.pem -client-key ./services/mpa/testdata/approver.key -proxy localhost -targets localhost mpa approve 53feec22-5447f403-c0e0a419 +``` + +## Extending SansShell SansShell is built on a principle of "Don't pay for what you don't use". This is advantageous in both minimizing the resources of SansShell server (binary diff --git a/cmd/proxy-server/default-policy.rego b/cmd/proxy-server/default-policy.rego index 97e22648..691494f2 100644 --- a/cmd/proxy-server/default-policy.rego +++ b/cmd/proxy-server/default-policy.rego @@ -52,6 +52,16 @@ allow { input.method = "/SysInfo.SysInfo/Uptime" } +# Allow anything approved by the user "approver", using MPA +allow { + input.approvers[_].id = "approver" +} + +# We need to allow MPA-related requests for MPA to work +allow { + startswith(input.method, "/Mpa.Mpa/") +} + # More complex example: allow stat of any file in /etc/ for # hosts in the 10.0.0.0/8 subnet, for callers in the 'admin' # group. @@ -69,6 +79,7 @@ denial_hints[msg] { input.message.file.filename != "/etc/hosts" msg := "we only proxy /etc/hosts" } + # You can put multiple denial hints and all of them will be included. denial_hints[msg] { msg := "this message always shows up on errors" diff --git a/cmd/proxy-server/main.go b/cmd/proxy-server/main.go index b4f81b3f..f3a58820 100644 --- a/cmd/proxy-server/main.go +++ b/cmd/proxy-server/main.go @@ -40,6 +40,7 @@ import ( "github.com/Snowflake-Labs/sansshell/auth/opa/rpcauth" "github.com/Snowflake-Labs/sansshell/cmd/proxy-server/server" "github.com/Snowflake-Labs/sansshell/cmd/util" + "github.com/Snowflake-Labs/sansshell/services/mpa/mpahooks" ss "github.com/Snowflake-Labs/sansshell/services/sansshell/server" "github.com/Snowflake-Labs/sansshell/telemetry/metrics" @@ -49,6 +50,7 @@ import ( _ "github.com/Snowflake-Labs/sansshell/services/healthcheck" _ "github.com/Snowflake-Labs/sansshell/services/httpoverrpc" _ "github.com/Snowflake-Labs/sansshell/services/localfile" + _ "github.com/Snowflake-Labs/sansshell/services/mpa" _ "github.com/Snowflake-Labs/sansshell/services/packages" _ "github.com/Snowflake-Labs/sansshell/services/process" _ "github.com/Snowflake-Labs/sansshell/services/sansshell" @@ -141,6 +143,7 @@ func main() { server.WithHostPort(*hostport), server.WithJustification(*justification), server.WithAuthzHook(rpcauth.PeerPrincipalFromCertHook()), + server.WithAuthzHook(mpahooks.ProxyMPAAuthzHook()), server.WithRawServerOption(func(s *grpc.Server) { reflection.Register(s) }), server.WithRawServerOption(func(s *grpc.Server) { channelz.RegisterChannelzServiceToServer(s) }), server.WithRawServerOption(srv.Register), diff --git a/cmd/sanssh/client/client.go b/cmd/sanssh/client/client.go index 24c467a1..7b5d2955 100644 --- a/cmd/sanssh/client/client.go +++ b/cmd/sanssh/client/client.go @@ -401,6 +401,10 @@ func Run(ctx context.Context, rs RunState) { fmt.Fprintf(os.Stderr, "Could not connect to proxy %q node(s) in batch %d: %v\n", rs.Proxy, i, err) os.Exit(1) } + if rs.EnableMPA { + conn.UnaryInterceptors = []proxy.UnaryInterceptor{mpahooks.ProxyClientUnaryInterceptor(state)} + conn.StreamInterceptors = []proxy.StreamInterceptor{mpahooks.ProxyClientStreamInterceptor(state)} + } state.Conn = conn state.Out = output[start:end] state.Err = errors[start:end] diff --git a/cmd/sansshell-server/default-policy.rego b/cmd/sansshell-server/default-policy.rego index 9166703d..15070c53 100644 --- a/cmd/sansshell-server/default-policy.rego +++ b/cmd/sansshell-server/default-policy.rego @@ -85,12 +85,24 @@ allow { input.method = "/SysInfo.SysInfo/Dmesg" } +# Allow anything from a proxy +allow { + input.peer.principal.id = "proxy" +} + # Allow anything with MPA allow { input.peer.principal.id = "sanssh" input.approvers[_].id = "approver" } +# Allow MPA listing commands +allow { + input.method = ["/Mpa.Mpa/Get", "/Mpa.Mpa/List", "/Mpa.Mpa/WaitForApproval"][_] +} + +# Allow MPA setting when not sending a proxied identity. The proxy is allowed above. allow { - startswith(input.method, "/Mpa.Mpa/") + not input.metadata["proxied-sansshell-identity"] + input.method = ["/Mpa.Mpa/Store", "/Mpa.Mpa/Approve"][_] } diff --git a/cmd/sansshell-server/main.go b/cmd/sansshell-server/main.go index b3e7edce..99ae08f2 100644 --- a/cmd/sansshell-server/main.go +++ b/cmd/sansshell-server/main.go @@ -46,6 +46,7 @@ import ( "github.com/Snowflake-Labs/sansshell/auth/mtls" mtlsFlags "github.com/Snowflake-Labs/sansshell/auth/mtls/flags" "github.com/Snowflake-Labs/sansshell/auth/opa" + "github.com/Snowflake-Labs/sansshell/auth/opa/proxiedidentity" "github.com/Snowflake-Labs/sansshell/auth/opa/rpcauth" "github.com/Snowflake-Labs/sansshell/cmd/sansshell-server/server" "github.com/Snowflake-Labs/sansshell/cmd/util" @@ -171,6 +172,8 @@ func main() { server.WithHostPort(*hostport), server.WithParsedPolicy(parsed), server.WithJustification(*justification), + server.WithUnaryInterceptor(proxiedidentity.ServerProxiedIdentityUnaryInterceptor()), + server.WithStreamInterceptor(proxiedidentity.ServerProxiedIdentityStreamInterceptor()), server.WithAuthzHook(rpcauth.PeerPrincipalFromCertHook()), server.WithAuthzHook(mpa.ServerMPAAuthzHook()), server.WithRawServerOption(func(s *grpc.Server) { reflection.Register(s) }), diff --git a/services/mpa/README.md b/services/mpa/README.md index 017e6d52..4dd6da09 100644 --- a/services/mpa/README.md +++ b/services/mpa/README.md @@ -1,6 +1,4 @@ -# Multi Party Authentication - -WARNING: This document describes the intended state. https://github.com/Snowflake-Labs/sansshell/issues/346 tracks implementation. +# Multi Party Authorization This module enables [multi-party authorization](https://en.wikipedia.org/wiki/Multi-party_authorization) for any sansshell command. Approval data is stored in-memory in sansshell-server. @@ -12,24 +10,31 @@ MPA must be explicitly requested. When requested, the MPA flow will be used rega ```bash $ sanssh -mpa -targets=1.2.3.4 -justification emergency exec run /bin/echo hi - Waiting for approval for 1-2345-6789. Command for approving: - sanssh -targets=1.2.3.4 mpa approve 1-2345-6789 + Waiting for multi-party approval on all targets, ask an approver to run: + sanssh --targets 1.2.3.4 mpa approve 244407fc-6b9b338a-db0760b8 ``` 2. The approver views the commands and approves it. ```bash $ sanssh -targets=1.2.3.4 mpa list - 1-2345-6789 - $ sanssh -targets=1.2.3.4 mpa get 1-2345-6789 - user: firstuser - justification: emergency - method: /Exec.Exec/Run - message: { - "command": "/bin/echo", - "args": ["hi"] + 244407fc-6b9b338a-db0760b8 /Exec.Exec/Run from sanssh for emergency + $ sanssh -targets=1.2.3.4 mpa get 244407fc-6b9b338a-db0760b8 + { + "action": { + "user": "sanssh", + "justification": "emergency", + "method": "/Exec.Exec/Run", + "message": { + "@type": "type.googleapis.com/Exec.ExecRequest", + "command": "/bin/echo", + "args": [ + "hi" + ] + } + } } - $ sanssh -targets=1.2.3.4 mpa approve 1-2345-6789 + $ sanssh -targets=1.2.3.4 mpa approve 244407fc-6b9b338a-db0760b8 ``` 3. If the user's command is still running, it will complete. If the user had stopped their command, they can rerun it and the approval will still be valid as long as the command's input remains the same and the sansshell-server still has the approval in memory. Approvals are lost if the server restarts, if the server evicts the approval due to age or staleness, or if a user calls `sanssh mpa clear` oon the request id. @@ -73,20 +78,28 @@ SansShell is built on a principle of "Don't pay for what you don't use". MPA is proxy.WithAuthzHook(mpa.ProxyMPAAuthzHook) ``` - You'll also need to set an additional interceptor on the server to make proxied identity information available. + You'll also need to set additional interceptors on the server to make proxied identity information available. ```go - func(ctx context.Context) bool { - peer := rpcauth.PeerInputFromContext(ctx) - if peer == nil { - return false - } - // Custom business logic goes here. - } proxiedidentity.ServerProxiedIdentityUnaryInterceptor() proxiedidentity.ServerProxiedIdentityStreamInterceptor() ``` + When setting these interceptors, make sure to update the server's rego policies if it allows callers other than the proxy to make direct calls. For example, the policy below will reject calls if proxied identity information is in the metadata and the caller is something other than a peer with an identity of `"proxy"`. + + ```rego + package sansshell.authz + default authz = false + authz { + allow + not deny + } + deny { + input.metadata["proxied-sansshell-identity"] + not input.peer.principal.id = "proxy" + } + ``` + 4. Any approvers must be able to call `/Mpa.Mpa/Approve` and any requestor must be able to call `/Mpa.Mpa/Store`. It's highly recommended to additionally let potential approvers call `/Mpa.Mpa/Get` and potential requestors call `/Mpa.Mpa/WaitForApproval` for better user experiences. `/Mpa.Mpa/Clear` can be used for cancelling MPA requests. Approvers will show up in [RPCAuthInput](https://pkg.go.dev/github.com/Snowflake-Labs/sansshell/auth/opa/rpcauth#RPCAuthInput). Match on these in the OPA policies. diff --git a/services/mpa/mpahooks/mpahooks.go b/services/mpa/mpahooks/mpahooks.go index 94f8d1a3..ab7d2468 100644 --- a/services/mpa/mpahooks/mpahooks.go +++ b/services/mpa/mpahooks/mpahooks.go @@ -19,12 +19,17 @@ package mpahooks import ( "context" + "errors" "fmt" "os" + "sort" + "strings" "github.com/Snowflake-Labs/sansshell/auth/opa/proxiedidentity" "github.com/Snowflake-Labs/sansshell/auth/opa/rpcauth" + "github.com/Snowflake-Labs/sansshell/proxy/proxy" "github.com/Snowflake-Labs/sansshell/services/mpa" + "github.com/Snowflake-Labs/sansshell/services/util" "github.com/google/go-cmp/cmp" "google.golang.org/grpc" "google.golang.org/grpc/metadata" @@ -124,7 +129,7 @@ func createAndBlockOnSingleTargetMPA(ctx context.Context, method string, req any } if len(result.Approver) == 0 { fmt.Fprintln(os.Stderr, "Multi party auth requested, ask an approver to run:") - fmt.Fprintf(os.Stderr, " sanssh --targets %v mpa approve %v\n", cc.Target(), result.Id) + fmt.Fprintf(os.Stderr, " sanssh -targets %v mpa approve %v\n", cc.Target(), result.Id) _, err := mpaClient.WaitForApproval(ctx, &mpa.WaitForApprovalRequest{Id: result.Id}) if err != nil { return "", err @@ -226,3 +231,143 @@ func StreamClientIntercepter() grpc.StreamClientInterceptor { }), nil } } + +func createAndBlockOnProxiedMPA(ctx context.Context, method string, args any, conn *proxy.Conn, state *util.ExecuteState) (mpaID string, err error) { + p, ok := args.(proto.Message) + if !ok { + return "", fmt.Errorf("unable to cast args to proto: %v", args) + } + var msg anypb.Any + if err := msg.MarshalFrom(p); err != nil { + return "", fmt.Errorf("unable to marshal into anyproto: %v", err) + } + mpaClient := mpa.NewMpaClientProxy(conn) + ch, err := mpaClient.StoreOneMany(ctx, &mpa.StoreRequest{ + Method: method, + Message: &msg, + }) + if err != nil { + return "", err + } + mpaIdToTargets := make(map[string][]string) + var targetsNeedingApproval []string + for r := range ch { + if r.Error != nil { + fmt.Fprintf(state.Err[r.Index], "Unable to request MPA: %v\n", r.Error) + } + mpaIdToTargets[r.Resp.Id] = append(mpaIdToTargets[r.Resp.Id], r.Target) + if len(r.Resp.Approver) == 0 { + // Only print out messages for not-yet-approved requests + targetsNeedingApproval = append(targetsNeedingApproval, r.Target) + } + mpaID = r.Resp.Id + } + + if len(mpaIdToTargets) > 1 { + var idMsgs []string + for id, targets := range mpaIdToTargets { + sort.Strings(targets) + idMsgs = append(idMsgs, id+": "+strings.Join(targets, ",")) + } + sort.Strings(idMsgs) + for _, m := range idMsgs { + fmt.Fprintln(os.Stderr, m) + } + return "", errors.New("Multiple MPA IDs generated, command needs to be run separately for each id.") + } + + if len(targetsNeedingApproval) > 0 { + fmt.Fprintln(os.Stderr, "Waiting for multi-party approval on all targets, ask an approver to run:") + fmt.Fprintf(os.Stderr, " sanssh -proxy %v -targets %v mpa approve %v\n", conn.Proxy().Target(), strings.Join(targetsNeedingApproval, ","), mpaID) + // We call WaitForApproval on all targets, even ones already approved. This is silly but not harmful. + waitCh, err := mpaClient.WaitForApprovalOneMany(ctx, &mpa.WaitForApprovalRequest{Id: mpaID}) + if err != nil { + return "", err + } + for r := range waitCh { + if r.Error != nil { + fmt.Fprintf(state.Err[r.Index], "Error when waiting for MPA approval: %v\n", r.Error) + } + } + } + return mpaID, nil +} + +// ProxyClientUnaryInterceptor will perform the MPA flow prior to making the desired RPC +// calls through the proxy. +func ProxyClientUnaryInterceptor(state *util.ExecuteState) proxy.UnaryInterceptor { + return func(ctx context.Context, conn *proxy.Conn, method string, args any, invoker proxy.UnaryInvoker, opts ...grpc.CallOption) (<-chan *proxy.Ret, error) { + // Our hook will run for all gRPC calls, including ones used inside the interceptor. + // We need to bail early on MPA-related ones to prevent infinite recursion. + if method == "/Mpa.Mpa/Store" || method == "/Mpa.Mpa/WaitForApproval" { + return invoker(ctx, method, args, opts...) + } + + mpaID, err := createAndBlockOnProxiedMPA(ctx, method, args, conn, state) + if err != nil { + return nil, err + } + + // Now that we have our approvals, make our call. + ctx = WithMPAInMetadata(ctx, mpaID) + return invoker(ctx, method, args, opts...) + } +} + +// ProxyClientStreamInterceptor will perform the MPA flow prior to making the desired streaming +// RPC calls through the proxy. +func ProxyClientStreamInterceptor(state *util.ExecuteState) proxy.StreamInterceptor { + return func(ctx context.Context, desc *grpc.StreamDesc, cc *proxy.Conn, method string, streamer proxy.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) { + return newStreamAfterFirstSend(func(args any) (grpc.ClientStream, error) { + // Figure out the MPA request + mpaID, err := createAndBlockOnProxiedMPA(ctx, method, args, cc, state) + if err != nil { + return nil, err + } + + // Now establish the stream we actually want because we can only do so after + // we put the MPA ID in the metadata. + ctx := WithMPAInMetadata(ctx, mpaID) + return streamer(ctx, desc, method, opts...) + }), nil + } +} + +// ProxyMPAAuthzHook populates MPA information in the input message +func ProxyMPAAuthzHook() rpcauth.RPCAuthzHook { + return rpcauth.RPCAuthzHookFunc(func(ctx context.Context, input *rpcauth.RPCAuthInput) error { + mpaID, ok := MPAFromIncomingContext(ctx) + if !ok { + // No need to call out if MPA wasn't requested + return nil + } + + if input.Environment == nil || !input.Environment.NonHostPolicyCheck { + // Proxies will evaluate OPA polices on both proxy-level calls and host-level + // calls. We want to only gather MPA info for host-level calls. + return nil + } + + if input.TargetConn == nil { + // If there's no host, we can't call out to the host. + return nil + } + + client := mpa.NewMpaClient(input.TargetConn) + resp, err := client.Get(ctx, &mpa.GetRequest{Id: mpaID}) + if err != nil { + return fmt.Errorf("failed getting MPA info: %v", err) + } + + if err := ActionMatchesInput(ctx, resp.Action, input); err != nil { + return err + } + for _, a := range resp.Approver { + input.Approvers = append(input.Approvers, &rpcauth.PrincipalAuthInput{ + ID: a.Id, + Groups: a.Groups, + }) + } + return nil + }) +} diff --git a/services/mpa/mpahooks/mpahooks_test.go b/services/mpa/mpahooks/mpahooks_test.go index 91c901e3..859cde68 100644 --- a/services/mpa/mpahooks/mpahooks_test.go +++ b/services/mpa/mpahooks/mpahooks_test.go @@ -22,16 +22,23 @@ import ( "io" "log" "net" + "os" "testing" "time" "github.com/Snowflake-Labs/sansshell/auth/mtls" + "github.com/Snowflake-Labs/sansshell/auth/opa/proxiedidentity" "github.com/Snowflake-Labs/sansshell/auth/opa/rpcauth" + "github.com/Snowflake-Labs/sansshell/proxy/proxy" + proxyserver "github.com/Snowflake-Labs/sansshell/proxy/server" "github.com/Snowflake-Labs/sansshell/services" "github.com/Snowflake-Labs/sansshell/services/healthcheck" "github.com/Snowflake-Labs/sansshell/services/localfile" "github.com/Snowflake-Labs/sansshell/services/mpa" "github.com/Snowflake-Labs/sansshell/services/mpa/mpahooks" + "github.com/Snowflake-Labs/sansshell/services/util" + "github.com/Snowflake-Labs/sansshell/telemetry" + "github.com/go-logr/logr" "golang.org/x/sync/errgroup" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -139,6 +146,19 @@ func pollForAction(ctx context.Context, m mpa.MpaClient, method string) (*mpa.Ac } } +func clearAll(ctx context.Context, m mpa.MpaClient) error { + l, err := m.List(ctx, &mpa.ListRequest{}) + if err != nil { + return err + } + for _, i := range l.Item { + if _, err := m.Clear(ctx, &mpa.ClearRequest{Action: i.Action}); err != nil { + return err + } + } + return nil +} + var serverPolicy = ` package sansshell.authz @@ -287,4 +307,227 @@ func TestClientInterceptors(t *testing.T) { if err := g.Wait(); err != nil { t.Error(err) } + + // MPA approvals are stored in a global singleton, so let's clear them + // to prevent interference with other tests + if err := clearAll(ctx, mpa.NewMpaClient(noInterceptorConn)); err != nil { + t.Error(err) + } +} + +var serverBehindProxyPolicy = ` +package sansshell.authz + +default allow = false + +allow { + input.peer.principal.id = "proxy" +} +` + +var proxyPolicy = ` +package sansshell.authz + +default allow = false + +allow { + input.method = "/HealthCheck.HealthCheck/Ok" + input.peer.principal.id = "sanssh" + input.approvers[_].id = "approver" +} + + +allow { + input.method = "/LocalFile.LocalFile/Read" + input.peer.principal.id = "sanssh" + input.approvers[_].id = "approver" +} + +allow { + startswith(input.method, "/Mpa.Mpa/") +} + +allow { + input.method = "/Proxy.Proxy/Proxy" +} +` + +func TestProxiedClientInterceptors(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + rot, err := mtls.LoadRootOfTrust("../../../auth/mtls/testdata/root.pem") + if err != nil { + t.Fatal(err) + } + lis, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatal(err) + } + proxyLis, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatal(err) + } + srvAddr := lis.Addr().String() + proxyAddr := proxyLis.Addr().String() + authz, err := rpcauth.NewWithPolicy(ctx, serverBehindProxyPolicy, rpcauth.PeerPrincipalFromCertHook(), mpaserver.ServerMPAAuthzHook()) + if err != nil { + t.Fatal(err) + } + srvCreds, err := mtls.LoadServerTLS("../../../auth/mtls/testdata/leaf.pem", "../../../auth/mtls/testdata/leaf.key", rot) + if err != nil { + t.Fatal(err) + } + s := grpc.NewServer( + grpc.ChainStreamInterceptor( + proxiedidentity.ServerProxiedIdentityStreamInterceptor(), + authz.AuthorizeStream, + ), + grpc.ChainUnaryInterceptor( + proxiedidentity.ServerProxiedIdentityUnaryInterceptor(), + authz.Authorize, + ), + grpc.Creds(srvCreds), + ) + for _, svc := range services.ListServices() { + svc.Register(s) + } + go func() { + if err := s.Serve(lis); err != nil { + log.Fatalf("Server exited with error: %v", err) + } + }() + defer s.GracefulStop() + + proxyAuthz, err := rpcauth.NewWithPolicy(ctx, proxyPolicy, rpcauth.PeerPrincipalFromCertHook(), mpahooks.ProxyMPAAuthzHook()) + if err != nil { + t.Fatal(err) + } + proxyCreds, err := mtls.LoadServerTLS("../testdata/proxy.pem", "../testdata/proxy.key", rot) + if err != nil { + t.Fatal(err) + } + proxyClientCreds, err := mtls.LoadClientTLS("../testdata/proxy.pem", "../testdata/proxy.key", rot) + if err != nil { + t.Fatal(err) + } + proxySrv := grpc.NewServer( + grpc.Creds(proxyCreds), + grpc.ChainUnaryInterceptor(proxyAuthz.Authorize), + grpc.ChainStreamInterceptor(proxyAuthz.AuthorizeStream), + ) + proxyserver := proxyserver.New( + proxyserver.NewDialer( + grpc.WithTransportCredentials(proxyClientCreds), + // Telemetry interceptors will forward justification + grpc.WithChainUnaryInterceptor(telemetry.UnaryClientLogInterceptor(logr.Discard())), + grpc.WithChainStreamInterceptor(telemetry.StreamClientLogInterceptor(logr.Discard())), + ), + proxyAuthz, + ) + proxyserver.Register(proxySrv) + go func() { + if err := proxySrv.Serve(proxyLis); err != nil { + log.Fatalf("Server exited with error: %v", err) + } + }() + defer proxySrv.GracefulStop() + + clientCreds, err := mtls.LoadClientTLS("../../../auth/mtls/testdata/client.pem", "../../../auth/mtls/testdata/client.key", rot) + if err != nil { + t.Fatal(err) + } + approverCreds, err := mtls.LoadClientTLS("../testdata/approver.pem", "../testdata/approver.key", rot) + if err != nil { + t.Fatal(err) + } + + // We test a single target behind the proxy to keep the tests simpler. The underlying + // MPA code treats a single target as a list of size one, so this should still be + // representative of MPA across multiple targets. + var g errgroup.Group + g.Go(func() error { + conn, err := proxy.DialContext(ctx, proxyAddr, []string{srvAddr}, grpc.WithTransportCredentials(approverCreds)) + if err != nil { + return err + } + defer conn.Close() + m := mpa.NewMpaClientProxy(conn) + + healthcheckAction, err := pollForAction(ctx, m, "/HealthCheck.HealthCheck/Ok") + if err != nil { + return err + } + if _, err := m.Approve(ctx, &mpa.ApproveRequest{Action: healthcheckAction}); err != nil { + return fmt.Errorf("unable to approve %v: %v", healthcheckAction, err) + } + + fileReadAction, err := pollForAction(ctx, m, "/LocalFile.LocalFile/Read") + if err != nil { + return err + } + if _, err := m.Approve(ctx, &mpa.ApproveRequest{Action: fileReadAction}); err != nil { + return fmt.Errorf("unable to approve %v: %v", healthcheckAction, err) + } + return nil + }) + + // Make our calls + conn, err := proxy.DialContext(ctx, proxyAddr, []string{srvAddr}, + grpc.WithTransportCredentials(clientCreds), + grpc.WithChainStreamInterceptor(mpahooks.StreamClientIntercepter()), + grpc.WithChainUnaryInterceptor(mpahooks.UnaryClientIntercepter()), + ) + if err != nil { + t.Error(err) + } + defer conn.Close() + // Confirm that we get Permission Denied before we add the MPA interceptors + hc := healthcheck.NewHealthCheckClientProxy(conn) + if _, err := hc.Ok(ctx, &emptypb.Empty{}); status.Code(err) != codes.PermissionDenied { + t.Fatalf("got something other than permission denied: %v", err) + } + + state := &util.ExecuteState{ + Conn: conn, + Out: []io.Writer{os.Stdout}, + Err: []io.Writer{os.Stderr}, + } + conn.StreamInterceptors = []proxy.StreamInterceptor{ + mpahooks.ProxyClientStreamInterceptor(state), + } + conn.UnaryInterceptors = []proxy.UnaryInterceptor{ + mpahooks.ProxyClientUnaryInterceptor(state), + } + if _, err := hc.Ok(ctx, &emptypb.Empty{}); err != nil { + t.Error(err) + } + + file := localfile.NewLocalFileClientProxy(conn) + bytes, err := file.Read(ctx, &localfile.ReadActionRequest{ + Request: &localfile.ReadActionRequest_File{File: &localfile.ReadRequest{Filename: "/etc/passwd"}}, + }) + if err != nil { + t.Error(err) + } else { + for { + _, err := bytes.Recv() + if err != nil { + if err != io.EOF { + t.Error(err) + } + break + } + } + } + + if err := g.Wait(); err != nil { + t.Error(err) + } + // MPA approvals are stored in a global singleton, so let's clear them + // to prevent interference with other tests + conn.StreamInterceptors = nil + conn.UnaryInterceptors = nil + if err := clearAll(ctx, mpa.NewMpaClientProxy(conn)); err != nil { + t.Error(err) + } } diff --git a/services/mpa/server/server.go b/services/mpa/server/server.go index 5ab447a1..5aec3500 100644 --- a/services/mpa/server/server.go +++ b/services/mpa/server/server.go @@ -59,6 +59,13 @@ func ServerMPAAuthzHook() rpcauth.RPCAuthzHook { if err != nil { return err } + if resp.Action.Method != input.Method { + // Proxies may make extra calls to the server as part of authz hooks. If + // we get an MPA id that corresponds to a different method than the one + // being called, it's probably from the proxy and can be ignored. + // The right method but wrong args indicates a bigger issue and checked below. + return nil + } if err := mpahooks.ActionMatchesInput(ctx, resp.Action, input); err != nil { return err diff --git a/services/mpa/server/server_test.go b/services/mpa/server/server_test.go index 0d58fcdb..97ce4bde 100644 --- a/services/mpa/server/server_test.go +++ b/services/mpa/server/server_test.go @@ -101,7 +101,7 @@ func TestAuthzHook(t *testing.T) { } // An action not matching the input should fail - wrongInput, err := rpcauth.NewRPCAuthInput(mpaCtx, "foobaz", &emptypb.Empty{}) + wrongInput, err := rpcauth.NewRPCAuthInput(mpaCtx, "foobar", &mpa.ApproveRequest{}) if err != nil { t.Fatal(err) } diff --git a/services/mpa/testdata/README.md b/services/mpa/testdata/README.md index 10dbe460..d10e4035 100644 --- a/services/mpa/testdata/README.md +++ b/services/mpa/testdata/README.md @@ -11,3 +11,22 @@ basicConstraints = critical, CA:FALSE EOF rm approver.csr ``` + +``` +openssl genrsa -out proxy.key +openssl req -new -key proxy.key -out proxy.csr -subj "/O=Acme Co/CN=proxy" +openssl x509 -req -days 3000 -in proxy.csr -CA ../../../auth/mtls/testdata/root.pem -CAkey ../../../auth/mtls/testdata/root.key -out proxy.pem -extensions req_ext -extfile /dev/stdin <&${LOGS}/proxy.log & +./bin/proxy-server --justification --root-ca=./auth/mtls/testdata/root.pem --server-cert=./auth/mtls/testdata/leaf.pem --server-key=./auth/mtls/testdata/leaf.key --client-cert=./services/mpa/testdata/proxy.pem --client-key=./services/mpa/testdata/proxy.key --policy-file=${LOGS}/policy >&${LOGS}/proxy.log & PROXY_PID=$! # Since we're controlling lifetime the shell can ignore this (avoids useless termination messages). disown %% @@ -924,6 +924,13 @@ ${SANSSH_NOPROXY} ${SINGLE_TARGET} file mv ${LOGS}/testdir/file ${LOGS}/testdir/ check_status $? /dev/null mv failed check_mv +echo "healthcheck with mpa" +# MPA ID is deterministic, so we approve it in parallel +sleep 1 && ./bin/sanssh ${SINGLE_TARGET} --justification 'approving' --root-ca=./auth/mtls/testdata/root.pem --client-cert=./services/mpa/testdata/approver.pem --client-key=./services/mpa/testdata/approver.key mpa approve dc83bd71-8945e78a-ff01a54c & +${SANSSH_NOPROXY} ${SINGLE_TARGET} -mpa healthcheck validate +${SANSSH_PROXY} ${SINGLE_TARGET} -mpa healthcheck validate +check_status $? /dev/null mv failed + echo "parallel work with some bad targets and various timeouts" mkdir -p "${LOGS}/parallel" start=$(date +%s)