-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add server and client implementations for MPA. #364
Merged
sfc-gh-srhodes
merged 8 commits into
Snowflake-Labs:main
from
stvnrhodes:mpa-basic-impl
Nov 10, 2023
Merged
Add server and client implementations for MPA. #364
sfc-gh-srhodes
merged 8 commits into
Snowflake-Labs:main
from
stvnrhodes:mpa-basic-impl
Nov 10, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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-jallie
approved these changes
Nov 9, 2023
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.
LGTM
minor/optional nits
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.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.
ServerMPAAuthzHook()
mpahooks.UnaryClientIntercepter()
andmpahooks.StreamClientIntercepter()
Part of #346