Add an optional TargetConn to RPCAuthInput #361

stvnrhodes · 2023-10-20T03:54:40Z

We're finding cases where we want the sansshell proxy to make gRPC calls to the target sansshell server as part of gathering input for policy evaluation. For example, for MPA we want the proxy to call out to the server to check if there's any MPA approval for a request.

The ways of doing this in RPCAuthzHook without adding it to RPCAuthInput get very hacky. It's possible to pull out the hosts's address and dial it, but then you're establishing a separate redundant connection and you need to replicate dial options around things like authentication and metrics.

We can't change the hook interface, so our two options are adding a new field to RPCAuthInput or putting the connection into the context. Putting it into the context is possible but we should be putting "request-scoped data that transits processes and API boundaries" in there and a grpc connection doesn't fit that model. We're left with the new RPCAuthInput field as the best option. This PR introduces the first field in RPCAuthInput that's meant purely for use by RPCAuthzHooks and not for policy evaluation.

Part of #346

We're finding cases where we want the sansshell proxy to make gRPC calls to the target sansshell server as part of gathering input for policy evaluation. For example, for MPA we want the proxy to call out to the server to check if there's any MPA approval for a request. The ways of doing this in RPCAuthzHook without adding it to RPCAuthInput get very hacky. It's possible to pull out the hosts's address and dial it, but then you're establishing a separate redundant connection and you need to replicate dial options around things like authentication and metrics. We can't change the hook interface, so our two options are adding a new field to RPCAuthInput or putting the connection into the context. Putting it into the context is possible but we should be putting "request-scoped data that transits processes and API boundaries" in there and a grpc connection doesn't fit that model. We're left with the new RPCAuthInput field as the best option. This PR introduces the first field in RPCAuthInput that's meant purely for use by RPCAuthzHooks and not for policy evaluation. Part of Snowflake-Labs#346

These changes alongside Snowflake-Labs#351 are sufficient for MPA when using a direct connection to the server. Here's a few sample commands to try it out. ``` go run ./cmd/sansshell-server go run ./cmd/sanssh -mpa -targets localhost healthcheck validate go run ./cmd/sanssh -targets localhost mpa approve 12345 ``` This implements the client and server portion, but not the proxy portion. The proxy needs some additional features in core sansshell code before we can implement it. - Snowflake-Labs#361 for implementing the proxy equivalent of `ServerMPAAuthzHook()` - Snowflake-Labs#358 for implementing the proxy equivalents of `mpahooks.UnaryClientIntercepter()` and `mpahooks.StreamClientIntercepter()` - Snowflake-Labs#359 so that MPA can use the identity of the caller to the proxy instead of the identity of the proxy. Part of Snowflake-Labs#346

These changes are sufficient for MPA when using a direct connection to the server. Here's a few sample commands you can run in parallel to try it out. ``` go run ./cmd/sansshell-server go run ./cmd/sanssh -client-cert ./auth/mtls/testdata/client.pem -client-key ./auth/mtls/testdata/client.key -mpa -targets localhost healthcheck validate go run ./cmd/sanssh -client-cert ./services/mpa/testdata/approver.pem -client-key ./services/mpa/testdata/approver.key -targets localhost mpa approve a59c2fef-748944da-336c9d35 ``` This implements the client and server portion, but not the proxy portion. The proxy part mostly builds on top of what I have here and will take advantage of some other features I'm implementing. - Snowflake-Labs#361 for implementing the proxy equivalent of `ServerMPAAuthzHook()` - Snowflake-Labs#358 for implementing the proxy equivalents of `mpahooks.UnaryClientIntercepter()` and `mpahooks.StreamClientIntercepter()` - Snowflake-Labs#359 so that MPA can use the identity of the caller to the proxy instead of the identity of the proxy. I'm going to wait to mention this in the readme until I've implemented the proxy part. Part of Snowflake-Labs#346

These changes are sufficient for MPA when using a direct connection to the server. Here's a few sample commands you can run in parallel to try it out. ``` go run ./cmd/sansshell-server go run ./cmd/sanssh -client-cert ./auth/mtls/testdata/client.pem -client-key ./auth/mtls/testdata/client.key -mpa -targets localhost healthcheck validate go run ./cmd/sanssh -client-cert ./services/mpa/testdata/approver.pem -client-key ./services/mpa/testdata/approver.key -targets localhost mpa approve a59c2fef-748944da-336c9d35 ``` I've added some new testdata certs because I'm forbidding cases where approver == requester. I've updated the sansshell server code to allow any request if it's requested by our "normal" client cert and approved by our "approver" client cert. The output of `-mpa` prints a nonconfigurable help message to stderr while waiting on approval. If the command is already approved, the message won't show up. ``` $ sanssh -mpa -targets localhost healthcheck validate Multi party auth requested, ask an approver to run: sanssh --targets localhost:50042 mpa approve a59c2fef-748944da-336c9d35 Target localhost:50042 (0) healthy` ``` This implements the client and server portion, but not the proxy portion. The proxy part mostly builds on top of what I have here and will take advantage of some other features I'm implementing. - Snowflake-Labs#361 for implementing the proxy equivalent of `ServerMPAAuthzHook()` - Snowflake-Labs#358 for implementing the proxy equivalents of `mpahooks.UnaryClientIntercepter()` and `mpahooks.StreamClientIntercepter()` - Snowflake-Labs#359 so that MPA can use the identity of the caller to the proxy instead of the identity of the proxy. I'm going to wait to mention this in the readme until I've implemented the proxy part. Part of Snowflake-Labs#346

These changes are sufficient for MPA when using a direct connection to the server. Here's a few sample commands you can run in parallel to try it out. ``` go run ./cmd/sansshell-server go run ./cmd/sanssh -client-cert ./auth/mtls/testdata/client.pem -client-key ./auth/mtls/testdata/client.key -mpa -targets localhost healthcheck validate go run ./cmd/sanssh -client-cert ./services/mpa/testdata/approver.pem -client-key ./services/mpa/testdata/approver.key -targets localhost mpa approve a59c2fef-748944da-336c9d35 ``` I've added some new testdata certs because I'm forbidding cases where approver == requester. I've updated the sansshell server code to allow any request if it's requested by our "normal" client cert and approved by our "approver" client cert. The output of `-mpa` prints a nonconfigurable help message to stderr while waiting on approval. If the command is already approved, the message won't show up. ``` $ sanssh -mpa -targets localhost healthcheck validate Multi party auth requested, ask an approver to run: sanssh --targets localhost:50042 mpa approve a59c2fef-748944da-336c9d35 Target localhost:50042 (0) healthy` ``` This implements the client and server portion, but not the proxy portion. The proxy part mostly builds on top of what I have here and will take advantage of some other features I'm implementing. - #361 for implementing the proxy equivalent of `ServerMPAAuthzHook()` - #358 for implementing the proxy equivalents of `mpahooks.UnaryClientIntercepter()` and `mpahooks.StreamClientIntercepter()` - #359 so that MPA can use the identity of the caller to the proxy instead of the identity of the proxy. Part of #346

stvnrhodes · 2023-11-10T17:27:03Z

I left this in draft for a couple weeks as I thought through the consequences of sticking data not meant for OPA evaluation into rpcauthinput. This still seems like the best option for giving a way for proxy to talk to the host.

sfc-gh-elinardi · 2023-11-10T20:30:49Z

@stvnrhodes RPCAuthInput is logged on places like

sansshell/auth/opa/rpcauth/rpcauth.go

Line 107 in 5f1ff80

logger.V(2).Info("evaluating authz policy", "input", redactedInput)

. How would the connection look like in the logs?

stvnrhodes · 2023-11-13T16:28:22Z

The json:"-" struct tag makes the logging library skip the new field, https://github.com/go-logr/logr/blob/10d92fc5ec4e26005e978dd3da7f9bd10f63bf1e/funcr/funcr.go#L465

Here's a log message to confirm:

2023/11/11 21:26:05 rpcauth.go:147: sanshell-proxy: "level"=0 "msg"="authz policy evaluation result" "method"="/Proxy.Proxy/Proxy" "peer"={"net":{"network":"tcp","address":"127.0.0.1","port":"38064"},"cert":{"subject":"CN=sanssh,OU=group1+OU=group2,O=Acme Co","issuer":"O=Acme Co","dnsnames":[],"spiffeid":""},"principal":null} "authorizationResult"=true "input"={"method":"/LocalFile.LocalFile/Read","message":[123,34,102,105,108,101,34,58,123,34,102,105,108,101,110,97,109,101,34,58,34,47,101,116,99,47,104,111,115,116,115,34,125,125],"type":"LocalFile.ReadActionRequest","metadata":{":authority":["localhost:50043"],"content-type":["application/grpc"],"user-agent":["grpc-go/1.59.0"]},"peer":{"net":{"network":"tcp","address":"127.0.0.1","port":"38064"},"cert":{"subject":"CN=sanssh,OU=group1+OU=group2,O=Acme Co","issuer":"O=Acme Co","dnsnames":[],"spiffeid":""},"principal":{"id":"sanssh","groups":["group1","group2"]}},"host":{"net":{"network":"tcp","address":"127.0.0.1","port":"50042"},"cert":{"subject":"CN=sansshell-server,OU=group2+OU=group3,O=Acme Co","issuer":"O=Acme Co","dnsnames":["localhost","bufnet"],"spiffeid":""},"principal":null},"approvers":[],"environment":{"non_host_policy_check":true},"extensions":[]} "denialHints"=[]

sfc-gh-elinardi · 2023-11-13T17:07:04Z

The json:"-" struct tag makes the logging library skip the new field, go-logr/logr@10d92fc/funcr/funcr.go#L465

Here's a log message to confirm:

2023/11/11 21:26:05 rpcauth.go:147: sanshell-proxy: "level"=0 "msg"="authz policy evaluation result" "method"="/Proxy.Proxy/Proxy" "peer"={"net":{"network":"tcp","address":"127.0.0.1","port":"38064"},"cert":{"subject":"CN=sanssh,OU=group1+OU=group2,O=Acme Co","issuer":"O=Acme Co","dnsnames":[],"spiffeid":""},"principal":null} "authorizationResult"=true "input"={"method":"/LocalFile.LocalFile/Read","message":[123,34,102,105,108,101,34,58,123,34,102,105,108,101,110,97,109,101,34,58,34,47,101,116,99,47,104,111,115,116,115,34,125,125],"type":"LocalFile.ReadActionRequest","metadata":{":authority":["localhost:50043"],"content-type":["application/grpc"],"user-agent":["grpc-go/1.59.0"]},"peer":{"net":{"network":"tcp","address":"127.0.0.1","port":"38064"},"cert":{"subject":"CN=sanssh,OU=group1+OU=group2,O=Acme Co","issuer":"O=Acme Co","dnsnames":[],"spiffeid":""},"principal":{"id":"sanssh","groups":["group1","group2"]}},"host":{"net":{"network":"tcp","address":"127.0.0.1","port":"50042"},"cert":{"subject":"CN=sansshell-server,OU=group2+OU=group3,O=Acme Co","issuer":"O=Acme Co","dnsnames":["localhost","bufnet"],"spiffeid":""},"principal":null},"approvers":[],"environment":{"non_host_policy_check":true},"extensions":[]} "denialHints"=[]

Ah my bad I overlooked the json tag.

stvnrhodes mentioned this pull request Oct 26, 2023

Add server and client implementations for MPA. #364

Merged

Merge branch 'main' into conn-in-rpcauth

3627f6e

stvnrhodes marked this pull request as ready for review November 10, 2023 17:23

sfc-gh-srhodes requested a review from sfc-gh-elinardi November 10, 2023 17:27

sfc-gh-elinardi approved these changes Nov 13, 2023

View reviewed changes

sfc-gh-srhodes merged commit a751446 into Snowflake-Labs:main Nov 13, 2023
4 checks passed

stvnrhodes deleted the conn-in-rpcauth branch November 14, 2023 02:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an optional TargetConn to RPCAuthInput #361

Add an optional TargetConn to RPCAuthInput #361

stvnrhodes commented Oct 20, 2023

stvnrhodes commented Nov 10, 2023

sfc-gh-elinardi commented Nov 10, 2023

stvnrhodes commented Nov 13, 2023

sfc-gh-elinardi commented Nov 13, 2023

Add an optional TargetConn to RPCAuthInput #361

Add an optional TargetConn to RPCAuthInput #361

Conversation

stvnrhodes commented Oct 20, 2023

stvnrhodes commented Nov 10, 2023

sfc-gh-elinardi commented Nov 10, 2023

stvnrhodes commented Nov 13, 2023

sfc-gh-elinardi commented Nov 13, 2023