Skip to content
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

Multi Party Authorization #346

Closed
sfc-gh-srhodes opened this issue Oct 11, 2023 · 0 comments · Fixed by #374
Closed

Multi Party Authorization #346

sfc-gh-srhodes opened this issue Oct 11, 2023 · 0 comments · Fixed by #374
Labels
enhancement New feature or request

Comments

@sfc-gh-srhodes
Copy link
Collaborator

Within Snowflake, we're encountering some use cases where we'd benefit from the ability to require one or more human approvals to a Sansshell command at a per-command granularity. This issue tracks any changes related to adding that feature.

@sfc-gh-srhodes sfc-gh-srhodes added the enhancement New feature or request label Oct 11, 2023
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 12, 2023
Many systems have the concept of an entity having an id and a set of groups, so the rpcauth input represents that in a first-class way. Until now, there was no method in this repo to populate the input. For multi-party-authentication, id and group will be an integral part so we'd benefit from having some way to represent them in tests for sansshell.

PeerPrincipalFromCertHook populates principal based on common name and organizational units in a cert. This is meant much more as an example of how principal can be populated and not as an endorsement of how it should be populated. I've regenerated certs to have both fields.

The upstream project we were using for generating certs has been deleted (a cached version lives on at https://pkg.go.dev/github.com/meterup/generate-cert) so I've switched to generating certs with openssl. This always feels a bit crufty to do but it has the benefit of using an extremely common tool that doesn't hide any of the details.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 13, 2023
For multi party authentication support, we plan on storing state in-memory in the sanshell server so the server needs to know the original caller's identity. Right now that gets lost when the caller goes through the proxy.

This PR fixes that by including a json blob of marshalled rpcauth.PrincipalAuthInput information in the gRPC context sent from the proxy to the server. We use json and not base64-encoded proto because the size is unlikely to be significant and there's no existing common proto with the fields we want. I'm unconditionally sending the information when principal is populated so that we don't expose an excessive number of behavioral knobs.

The API design intentionally tries to avoid exposing easy ways to use the gRPC metadata without checking its validity. We assume that in some cases sansshell server will have both proxied clients and direct clients. Most direct clients should be unable to pass in arbitrary proxied identity information.

The interceptor for getting proxied identity is only implemented for unary RPCs because we don't yet have a use case for adding it to streaming RPCs.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 13, 2023
I've prototyped this enough in https://github.com/stvnrhodes/sansshell/tree/mpa to have reasonable confidence that this design is implementable. I'm separating out the design and proto into an initial PR so that we can make sure that we agree on the high-level direction before we deeply review any implementation.

The readme is meant to be used as long-term documentation on how to use MPA.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 13, 2023
I've prototyped this enough in https://github.com/stvnrhodes/sansshell/tree/mpa to have reasonable confidence that this design is implementable. I'm separating out the design and proto into an initial PR so that we can make sure that we agree on the high-level direction before we deeply review any implementation.

The readme is meant to be used as long-term documentation on how to use MPA.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 13, 2023
I've prototyped this enough in https://github.com/stvnrhodes/sansshell/tree/mpa to have reasonable confidence that this design is implementable. I'm separating out the design and proto into an initial PR so that we can make sure that we agree on the high-level direction before we deeply review any implementation.

The readme is meant to be used as long-term documentation on how to use MPA.

Part of Snowflake-Labs#346
sfc-gh-srhodes pushed a commit to stvnrhodes/sansshell that referenced this issue Oct 13, 2023
I've prototyped this enough in https://github.com/stvnrhodes/sansshell/tree/mpa to have reasonable confidence that this design is implementable. I'm separating out the design and proto into an initial PR so that we can make sure that we agree on the high-level direction before we deeply review any implementation.

The readme is meant to be used as long-term documentation on how to use MPA.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 17, 2023
This is heavily based on gRPC interceptors from https://github.com/grpc/grpc-go/blob/master/examples/features/interceptor/README.md and serve the same purpose for proxy-level interactions. These interceptors allow hooking into calls before they get fanned out to many targets. The public interface is new fields on the `proxy.Conn` struct because I'm introducing these in a backwards-compatible way.

I'm planning on using this for MPA requests in sanssh. The interceptor lets us hook into every call and turn it into a series of calls that perform the MPA request flow and then pass on the MPA request as metadata into the call.

It's theoretically possible to avoid adding this interceptor and instead do what I'd like via a grpc.StreamClientInterceptor. Doing so would require writing something tightly coupled to the logic in the proxy client code and more prone to bugs.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 17, 2023
This is heavily based on gRPC interceptors from https://github.com/grpc/grpc-go/blob/master/examples/features/interceptor/README.md and serve the same purpose for proxy-level interactions. These interceptors allow hooking into calls before they get fanned out to many targets. The public interface is new fields on the `proxy.Conn` struct because I'm introducing these in a backwards-compatible way.

I'm planning on using this for MPA requests in sanssh. The interceptor lets us hook into every call and turn it into a series of calls that perform the MPA request flow and then pass on the MPA request as metadata into the call. I'm not completely happy with separate calls for unary and streaming, especially because a message with a single request and a streamed response counts as "streaming", but it matches what gRPC does in its calls.

It's theoretically possible to avoid adding this interceptor and instead do what I'd like via a grpc.StreamClientInterceptor. Doing so would require writing something tightly coupled to the logic in the proxy client code and more prone to bugs. It would involve weird things like lying to the client and claiming that we've connected to targets before we connect to any because the message containing the rpc request doesn't get sent until the proxy client code thinks it has established all connections.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 18, 2023
For multi party authentication support, we plan on storing state in-memory in the sanshell server so the server needs to know the original caller's identity. Right now that gets lost when the caller goes through the proxy.

This PR fixes that by including a json blob of marshalled rpcauth.PrincipalAuthInput information in the gRPC context sent from the proxy to the server. We use json and not base64-encoded proto because the size is unlikely to be significant and there's no existing common proto with the fields we want. I'm unconditionally sending the information when principal is populated so that we don't expose an excessive number of behavioral knobs. I've tweaked rpcauth.PeerInputFromContext so that it'll preserve information about the peer that gets added by rpcauth hooks.

The API design intentionally tries to avoid exposing easy ways to use the gRPC metadata without checking its validity. We assume that in some cases sansshell server will have both proxied clients and direct clients. Most direct clients should be unable to pass in arbitrary proxied identity information.

The interceptor for getting proxied identity is only implemented for unary RPCs because we don't yet have a use case for adding it to streaming RPCs.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 20, 2023
I've prototyped this enough in https://github.com/stvnrhodes/sansshell/tree/mpa to have reasonable confidence that this design is implementable. I'm separating out the design and proto into an initial PR so that we can make sure that we agree on the high-level direction before we deeply review any implementation.

The readme is meant to be used as long-term documentation on how to use MPA.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 20, 2023
Many systems have the concept of an entity having an id and a set of groups, so the rpcauth input represents that in a first-class way. Until now, there was no method in this repo to populate the input. For multi-party-authentication, id and group will be an integral part so we'd benefit from having some way to represent them in tests for sansshell.

PeerPrincipalFromCertHook populates principal based on common name and organizational units in a cert. This is meant much more as an example of how principal can be populated and not as an endorsement of how it should be populated. I've regenerated certs to have both fields.

The upstream project we were using for generating certs has been deleted (a cached version lives on at https://pkg.go.dev/github.com/meterup/generate-cert) so I've switched to generating certs with openssl. This always feels a bit crufty to do but it has the benefit of using an extremely common tool that doesn't hide any of the details.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 20, 2023
For multi party authentication support, we plan on storing state in-memory in the sanshell server so the server needs to know the original caller's identity. Right now that gets lost when the caller goes through the proxy.

This PR fixes that by including a json blob of marshalled rpcauth.PrincipalAuthInput information in the gRPC context sent from the proxy to the server. We use json and not base64-encoded proto because the size is unlikely to be significant and there's no existing common proto with the fields we want. I'm unconditionally sending the information when principal is populated so that we don't expose an excessive number of behavioral knobs. I've tweaked rpcauth.PeerInputFromContext so that it'll preserve information about the peer that gets added by rpcauth hooks.

The API design intentionally tries to avoid exposing easy ways to use the gRPC metadata without checking its validity. We assume that in some cases sansshell server will have both proxied clients and direct clients. Most direct clients should be unable to pass in arbitrary proxied identity information.

The interceptor for getting proxied identity is only implemented for unary RPCs because we don't yet have a use case for adding it to streaming RPCs.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 20, 2023
This is heavily based on gRPC interceptors from https://github.com/grpc/grpc-go/blob/master/examples/features/interceptor/README.md and serve the same purpose for proxy-level interactions. These interceptors allow hooking into calls before they get fanned out to many targets. The public interface is new fields on the `proxy.Conn` struct because I'm introducing these in a backwards-compatible way.

I'm planning on using this for MPA requests in sanssh. The interceptor lets us hook into every call and turn it into a series of calls that perform the MPA request flow and then pass on the MPA request as metadata into the call. I'm not completely happy with separate calls for unary and streaming, especially because a message with a single request and a streamed response counts as "streaming", but it matches what gRPC does in its calls.

It's theoretically possible to avoid adding this interceptor and instead do what I'd like via a grpc.StreamClientInterceptor. Doing so would require writing something tightly coupled to the logic in the proxy client code and more prone to bugs. It would involve weird things like lying to the client and claiming that we've connected to targets before we connect to any because the message containing the rpc request doesn't get sent until the proxy client code thinks it has established all connections.

Part of Snowflake-Labs#346
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 20, 2023
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
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 24, 2023
I've prototyped this enough in https://github.com/stvnrhodes/sansshell/tree/mpa to have reasonable confidence that this design is implementable. I'm separating out the design and proto into an initial PR so that we can make sure that we agree on the high-level direction before we deeply review any implementation.

The readme is meant to be used as long-term documentation on how to use MPA.

Part of Snowflake-Labs#346
sfc-gh-srhodes pushed a commit that referenced this issue Oct 24, 2023
* Add a MPA design and proto definition.

I've prototyped this enough in https://github.com/stvnrhodes/sansshell/tree/mpa to have reasonable confidence that this design is implementable. I'm separating out the design and proto into an initial PR so that we can make sure that we agree on the high-level direction before we deeply review any implementation.

The readme is meant to be used as long-term documentation on how to use MPA.

Part of #346

* Address feedback
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 24, 2023
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
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 24, 2023
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
sfc-gh-srhodes pushed a commit that referenced this issue Oct 25, 2023
Many systems have the concept of an entity having an id and a set of groups, so the rpcauth input represents that in a first-class way. Until now, there was no method in this repo to populate the input. For multi-party-authentication, id and group will be an integral part so we'd benefit from having some way to represent them in tests for sansshell.

PeerPrincipalFromCertHook populates principal based on common name and organizational units in a cert. This is meant much more as an example of how principal can be populated and not as an endorsement of how it should be populated. I've regenerated certs to have both fields.

The upstream project we were using for generating certs has been deleted (a cached version lives on at https://pkg.go.dev/github.com/meterup/generate-cert) so I've switched to generating certs with openssl. This always feels a bit crufty to do but it has the benefit of using an extremely common tool that doesn't hide any of the details.

Part of #346

Co-authored-by: Edbert Linardi <[email protected]>
sfc-gh-srhodes added a commit that referenced this issue Oct 25, 2023
This is heavily based on gRPC interceptors from https://github.com/grpc/grpc-go/blob/master/examples/features/interceptor/README.md and serve the same purpose for proxy-level interactions. These interceptors allow hooking into calls before they get fanned out to many targets. The public interface is new fields on the `proxy.Conn` struct because I'm introducing these in a backwards-compatible way.

I'm planning on using this for MPA requests in sanssh. The interceptor lets us hook into every call and turn it into a series of calls that perform the MPA request flow and then pass on the MPA request as metadata into the call. I'm not completely happy with separate calls for unary and streaming, especially because a message with a single request and a streamed response counts as "streaming", but it matches what gRPC does in its calls.

It's theoretically possible to avoid adding this interceptor and instead do what I'd like via a grpc.StreamClientInterceptor. Doing so would require writing something tightly coupled to the logic in the proxy client code and more prone to bugs. It would involve weird things like lying to the client and claiming that we've connected to targets before we connect to any because the message containing the rpc request doesn't get sent until the proxy client code thinks it has established all connections.

Part of #346

Co-authored-by: Edbert Linardi <[email protected]>
Co-authored-by: Steven Rhodes <[email protected]>
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 26, 2023
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
stvnrhodes added a commit to stvnrhodes/sansshell that referenced this issue Oct 26, 2023
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
sfc-gh-srhodes pushed a commit that referenced this issue Nov 10, 2023
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
sfc-gh-srhodes pushed a commit that referenced this issue Nov 13, 2023
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
sfc-gh-srhodes pushed a commit that referenced this issue Nov 22, 2023
For multi party authentication support, we plan on storing state in-memory in the sanshell server so the server needs to know the original caller's identity. Right now that gets lost when the caller goes through the proxy.

This PR fixes that by including a json blob of marshalled rpcauth.PrincipalAuthInput information in the gRPC context sent from the proxy to the server. We use json and not base64-encoded proto because the size is unlikely to be significant and there's no existing common proto with the fields we want. I'm unconditionally sending the information when principal is populated so that we don't expose an excessive number of behavioral knobs.

The API design intentionally tries to avoid exposing easy ways to use the gRPC metadata without checking its validity. We assume that in some cases sansshell server will have both proxied clients and direct clients. Most direct clients should be unable to pass in arbitrary proxied identity information.

I've updated the MPA service to make use of the proxied information. It's currently the only user.

Part of #346
sfc-gh-srhodes pushed a commit that referenced this issue Nov 27, 2023
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 #346
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant