From 76a1f7efa9bda32e662e1f0fa19c0b0eba0715a6 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Wed, 11 Sep 2019 16:03:52 -0400 Subject: [PATCH 01/16] Working integration tests for two man mode! keybaseca now supports signature requests that specify a requested realm. If the requested realm is one of a configured list of two man realms, it requires confirmation from another party. A list of approvers is defined in the keybaseca config. The approvers are tagged with a request for their approval. They approve a request by reacting with a thumbs-up emoji. kssh now has a `kssh --request-realm foo` flag. Integration tests are done with a bit of python code that reacts to messages with a thumbs-up emoji. TODO: * All the TODOs marked in the code * More integration tests * Have kssh track the current principals in a key and decide whether or not to renew (also good for debug logging) --- buildAll.sh | 12 +- integrationTest.sh | 2 +- src/cmd/kssh/kssh.go | 71 +++-- src/keybaseca/bot/bot.go | 158 ++++++++-- src/keybaseca/config/config.go | 67 +++- src/keybaseca/sshutils/sshutils.go | 29 +- src/kssh/bot.go | 10 +- src/shared/chat_types.go | 9 +- tests/Dockerfile-autoapprover | 37 +++ tests/bot-entrypoint.py | 2 +- tests/configure_accounts.py | 2 + tests/docker-compose.yml | 17 + tests/envFiles/test_env_5_two_man | 9 + tests/tester-entrypoint.sh | 2 +- tests/tests/autoapprover.py | 68 ++++ tests/tests/test_env_1.py | 291 +++++++++--------- tests/tests/test_env_2_local_audit_log.py | 34 +- .../test_env_3_user_not_in_first_team.py | 44 +-- ...test_env_4_user_not_in_configured_teams.py | 76 ++--- tests/tests/test_env_5_two_man.py | 24 ++ 20 files changed, 656 insertions(+), 308 deletions(-) create mode 100644 tests/Dockerfile-autoapprover create mode 100644 tests/envFiles/test_env_5_two_man create mode 100644 tests/tests/autoapprover.py create mode 100644 tests/tests/test_env_5_two_man.py diff --git a/buildAll.sh b/buildAll.sh index c1da84b..bee631e 100755 --- a/buildAll.sh +++ b/buildAll.sh @@ -7,9 +7,9 @@ go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/kssh-linux src/cmd/kss go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/keybaseca-linux src/cmd/keybaseca/keybaseca.go # Mac -GOOS=darwin GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/kssh-mac src/cmd/kssh/kssh.go -GOOS=darwin GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/keybaseca-mac src/cmd/keybaseca/keybaseca.go - -# Windows -GOOS=windows GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/kssh-windows src/cmd/kssh/kssh.go -GOOS=windows GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/keybaseca-windows src/cmd/keybaseca/keybaseca.go +#GOOS=darwin GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/kssh-mac src/cmd/kssh/kssh.go +#GOOS=darwin GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/keybaseca-mac src/cmd/keybaseca/keybaseca.go +# +## Windows +#GOOS=windows GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/kssh-windows src/cmd/kssh/kssh.go +#GOOS=windows GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/keybaseca-windows src/cmd/keybaseca/keybaseca.go diff --git a/integrationTest.sh b/integrationTest.sh index e062e72..e0f0259 100755 --- a/integrationTest.sh +++ b/integrationTest.sh @@ -50,7 +50,7 @@ echo "Building containers..." cd ../docker/ && make && cd ../tests/ docker-compose build echo "Running integration tests..." -docker-compose up -d +docker-compose up docker logs kssh -f | indent TEST_EXIT_CODE=`docker wait kssh` diff --git a/src/cmd/kssh/kssh.go b/src/cmd/kssh/kssh.go index 0c45006..fb65784 100644 --- a/src/cmd/kssh/kssh.go +++ b/src/cmd/kssh/kssh.go @@ -20,27 +20,27 @@ import ( func main() { kssh.InitLogging() - team, remainingArgs, action, err := handleArgs(os.Args[1:]) + botname, requestedPrincipal, remainingArgs, action, err := handleArgs(os.Args[1:]) if err != nil { fmt.Printf("Failed to parse arguments: %v\n", err) os.Exit(1) } - keyPath, err := getSignedKeyLocation(team) + keyPath, err := getSignedKeyLocation(botname) if err != nil { fmt.Printf("Failed to retrieve location to store SSH keys: %v\n", err) os.Exit(1) } - if isValidCert(keyPath) { + if isValidCert(keyPath) && requestedPrincipal == "" { log.WithField("keyPath", keyPath).Debug("Reusing unexpired certificate") doAction(action, keyPath, remainingArgs) os.Exit(0) } - config, err := getConfig(team) + config, err := getConfig(botname) if err != nil { fmt.Printf("%v\n", err) os.Exit(1) } - err = provisionNewKey(config, keyPath) + err = provisionNewKey(config, keyPath, requestedPrincipal) if err != nil { fmt.Printf("%v\n", err) os.Exit(1) @@ -104,6 +104,7 @@ var cliArguments = []kssh.CLIArgument{ {Name: "--help", HasArgument: false}, {Name: "-v", HasArgument: false, Preserve: true}, {Name: "--set-keybase-binary", HasArgument: true}, + {Name: "--request-realm", HasArgument: true}, } var VersionNumber = "master" @@ -131,7 +132,8 @@ GLOBAL OPTIONS: --set-default-user Set the default SSH user to be used for kssh. Useful if you use ssh configs that do not set a default SSH user --clear-default-user Clear the default SSH user - --set-keybase-binary Run kssh with a specific keybase binary rather than resolving via $PATH `, VersionNumber) + --set-keybase-binary Advanced feature: Run kssh with a specific keybase binary rather than resolving via $PATH + --request-realm Advanced feature: Request a specific two-man realm in your provisioned certificate `, VersionNumber) } type Action int @@ -141,55 +143,56 @@ const ( SSH ) -// Returns botname, remaining arguments, action, error +// Returns botname, requestedPrincipal, remaining arguments, action, error // If the argument requires exiting after processing, it will call os.Exit -func handleArgs(args []string) (string, []string, Action, error) { +func handleArgs(args []string) (string, string, []string, Action, error) { remaining, found, err := kssh.ParseArgs(args, cliArguments) if err != nil { - return "", nil, 0, fmt.Errorf("Failed to parse provided arguments: %v", err) + return "", "", nil, 0, fmt.Errorf("Failed to parse provided arguments: %v", err) } - team := "" + requestedPrincipal := "" + botname := "" action := SSH for _, arg := range found { if arg.Argument.Name == "--bot" { - team = arg.Value + botname = arg.Value } - if arg.Argument.Name == "--set-default-user" { - err := kssh.SetDefaultSSHUser(arg.Value) + if arg.Argument.Name == "--set-default-bot" { + // We exit immediately after setting the default bot + err := kssh.SetDefaultBot(arg.Value) if err != nil { - fmt.Printf("Failed to set the default ssh user: %v\n", err) + fmt.Printf("Failed to set the default bot: %v\n", err) os.Exit(1) } - fmt.Println("Set default ssh user, exiting...") + fmt.Println("Set default bot, exiting...") os.Exit(0) } - if arg.Argument.Name == "--clear-default-user" { - err := kssh.SetDefaultSSHUser("") + if arg.Argument.Name == "--clear-default-bot" { + err := kssh.SetDefaultBot("") if err != nil { - fmt.Printf("Failed to clear the default ssh user: %v\n", err) + fmt.Printf("Failed to clear the default bot: %v\n", err) os.Exit(1) } - fmt.Println("Cleared default ssh user, exiting...") + fmt.Println("Cleared default bot, exiting...") os.Exit(0) } - if arg.Argument.Name == "--set-default-bot" { - // We exit immediately after setting the default bot - err := kssh.SetDefaultBot(arg.Value) + if arg.Argument.Name == "--set-default-user" { + err := kssh.SetDefaultSSHUser(arg.Value) if err != nil { - fmt.Printf("Failed to set the default bot: %v\n", err) + fmt.Printf("Failed to set the default ssh user: %v\n", err) os.Exit(1) } - fmt.Println("Set default bot, exiting...") + fmt.Println("Set default ssh user, exiting...") os.Exit(0) } - if arg.Argument.Name == "--clear-default-bot" { - err := kssh.SetDefaultBot("") + if arg.Argument.Name == "--clear-default-user" { + err := kssh.SetDefaultSSHUser("") if err != nil { - fmt.Printf("Failed to clear the default bot: %v\n", err) + fmt.Printf("Failed to clear the default ssh user: %v\n", err) os.Exit(1) } - fmt.Println("Cleared default bot, exiting...") + fmt.Println("Cleared default ssh user, exiting...") os.Exit(0) } if arg.Argument.Name == "--set-keybase-binary" { @@ -211,8 +214,11 @@ func handleArgs(args []string) (string, []string, Action, error) { if arg.Argument.Name == "-v" { log.SetLevel(log.DebugLevel) } + if arg.Argument.Name == "--request-realm" { + requestedPrincipal = arg.Value + } } - return team, remaining, action, nil + return botname, requestedPrincipal, remaining, action, nil } // Get the kssh.ConfigFile. botname is the team specified via --bot if one was specified, otherwise the empty string @@ -289,7 +295,7 @@ func isValidCert(keyPath string) bool { } // Provision a new signed SSH key with the given config -func provisionNewKey(config kssh.ConfigFile, keyPath string) error { +func provisionNewKey(config kssh.ConfigFile, keyPath string, requestedPrincipal string) error { log.Debug("Generating a new SSH key...") // Make ~/.ssh/ in case it doesn't exist @@ -316,8 +322,9 @@ func provisionNewKey(config kssh.ConfigFile, keyPath string) error { log.Debug("Requesting signature from the CA....") resp, err := kssh.GetSignedKey(config, shared.SignatureRequest{ - UUID: randomUUID.String(), - SSHPublicKey: string(pubKey), + UUID: randomUUID.String(), + SSHPublicKey: string(pubKey), + RequestedPrincipal: requestedPrincipal, }) if err != nil { return fmt.Errorf("Failed to get a signed key from the CA: %v", err) diff --git a/src/keybaseca/bot/bot.go b/src/keybaseca/bot/bot.go index 02daaaf..54f712d 100644 --- a/src/keybaseca/bot/bot.go +++ b/src/keybaseca/bot/bot.go @@ -5,6 +5,8 @@ import ( "fmt" "strings" + "github.com/keybase/go-keybase-chat-bot/kbchat/types/chat1" + "github.com/keybase/bot-sshca/src/keybaseca/botwrapper" auditlog "github.com/keybase/bot-sshca/src/keybaseca/log" @@ -36,8 +38,18 @@ func GetUsername(conf config.Config) (string, error) { return username, nil } +type OutstandingTwoManSignatureRequest struct { + SignatureRequest shared.SignatureRequest + RequestMessageID chat1.MessageID + ApprovalCount int + ConvID string +} + // Start the keybaseca bot in an infinite loop. Does not return unless it encounters an unrecoverable error. func StartBot(conf config.Config) error { + // Initialize a list for the outstanding two-man signature requests + outstandingTwoManRequests := []OutstandingTwoManSignatureRequest{} + kbc, err := GetKBChat(conf) if err != nil { return fmt.Errorf("error starting Keybase chat: %v", err) @@ -60,23 +72,49 @@ func StartBot(conf config.Config) error { return fmt.Errorf("failed to read message: %v", err) } - if msg.Message.Content.TypeName != "text" { + if msg.Message.Content.TypeName != "text" && msg.Message.Content.TypeName != "reaction" { continue } - messageBody := msg.Message.Content.Text.Body + if msg.Message.Content.TypeName == "reaction" { + // TODO: Do I care about the fact that the bot user could react to messages? + // TODO: SECURITY VULN: Someone can approve a message multiple times by reacting and then unreacting + emoji := msg.Message.Content.Reaction.Body + responseTo := msg.Message.Content.Reaction.MessageID - log.Debugf("Received message in %s#%s: %s", msg.Message.Channel.Name, msg.Message.Channel.TopicName, messageBody) - - if msg.Message.Sender.Username == kbc.GetUsername() { - log.Debug("Skipping message since it comes from the bot user") - if strings.Contains(messageBody, shared.AckRequestPrefix) || strings.Contains(messageBody, shared.SignatureRequestPreamble) { - log.Warn("Ignoring AckRequest/SignatureRequest coming from the bot user! Are you trying to run the bot " + - "and kssh as the same user?") + log.Debug("Examining reaction...") + for _, outstanding := range outstandingTwoManRequests { + if outstanding.RequestMessageID == responseTo { + log.Debug("Message is a reply to an outstanding two-man request") + if emoji == ":+1:" && isValidApprover(conf, msg.Message.Sender.Username, outstanding.SignatureRequest) { + outstanding.ApprovalCount++ + log.WithField("requester", outstanding.SignatureRequest.Username).WithField("approver", msg.Message.Sender.Username).WithField("approval_count", outstanding.ApprovalCount).Debugf("Message approved request") + threshold := conf.GetNumberRequiredApprovers() + if outstanding.ApprovalCount >= threshold { + respondToSignatureRequest(conf, kbc, outstanding.SignatureRequest, outstanding.SignatureRequest.Username, outstanding.RequestMessageID, outstanding.ConvID) + } + } else { + log.Debug("Message did not approve request") + } + continue + } } + log.Debug("Ignoring reaction since it is not a reaction on an outstanding two-man request") continue } + messageBody := msg.Message.Content.Text.Body + log.Debugf("Received message in %s#%s: %s", msg.Message.Channel.Name, msg.Message.Channel.TopicName, messageBody) + + //if msg.Message.Sender.Username == kbc.GetUsername() { + // log.Debug("Skipping message since it comes from the bot user") + // if strings.Contains(messageBody, shared.AckRequestPrefix) || strings.Contains(messageBody, shared.SignatureRequestPreamble) { + // log.Warn("Ignoring AckRequest/SignatureRequest coming from the bot user! Are you trying to run the bot " + + // "and kssh as the same user?") + // } + // continue + //} + // Note that this line is one of the main security barriers around the SSH bot. If this line were removed // or had a bug, it would cause the SSH bot to respond to any SignatureRequest messages in any channels. This // would allow an attacker to provision SSH keys even though they are not in the listed channels. @@ -90,33 +128,38 @@ func StartBot(conf config.Config) error { // Ack any AckRequests so that kssh can determine whether it has fully connected _, err = kbc.SendMessageByConvID(msg.Message.ConvID, shared.GenerateAckResponse(messageBody)) if err != nil { - LogError(conf, kbc, msg, err) + LogError(conf, kbc, msg.Message.Sender.Username, msg.Message.Id, msg.Message.ConvID, err) continue } } else if strings.HasPrefix(messageBody, shared.SignatureRequestPreamble) { log.Debug("Responding to SignatureRequest") signatureRequest, err := shared.ParseSignatureRequest(messageBody) if err != nil { - LogError(conf, kbc, msg, err) + LogError(conf, kbc, msg.Message.Sender.Username, msg.Message.Id, msg.Message.ConvID, err) continue } signatureRequest.Username = msg.Message.Sender.Username signatureRequest.DeviceName = msg.Message.Sender.DeviceName - signatureResponse, err := sshutils.ProcessSignatureRequest(conf, signatureRequest) - if err != nil { - LogError(conf, kbc, msg, err) - continue - } - response, err := json.Marshal(signatureResponse) - if err != nil { - LogError(conf, kbc, msg, err) - continue - } - _, err = kbc.SendMessageByConvID(msg.Message.ConvID, shared.SignatureResponsePreamble+string(response)) - if err != nil { - LogError(conf, kbc, msg, err) - continue + // Process the signature request depending on whether they requested two-man only principals or not + if signatureRequest.RequestedPrincipal == "" { + // If they didn't request a principal just respond immediately + respondToSignatureRequest(conf, kbc, signatureRequest, msg.Message.Sender.Username, msg.Message.Id, msg.Message.ConvID) + } else { + if isTwoManPrincipal(conf, signatureRequest.RequestedPrincipal) { + // If they requested a principal that doesn't require two-man authorization, respond immediately + respondToSignatureRequest(conf, kbc, signatureRequest, msg.Message.Sender.Username, msg.Message.Id, msg.Message.ConvID) + } else { + // If the principal requires two-man authorization, treat it as such + resp, err := kbc.SendMessageByConvID(msg.Message.ConvID, buildTwoManApprovalRequestMessage(conf, msg.Message.Sender.Username, signatureRequest.RequestedPrincipal)) + if err != nil { + LogError(conf, kbc, msg.Message.Sender.Username, msg.Message.Id, msg.Message.ConvID, err) + continue + } + + outstandingTwoManRequests = append(outstandingTwoManRequests, + OutstandingTwoManSignatureRequest{SignatureRequest: signatureRequest, ApprovalCount: 0, RequestMessageID: *resp.Result.MessageID, ConvID: msg.Message.ConvID}) + } } } else { log.Debug("Ignoring unparsed message") @@ -124,12 +167,71 @@ func StartBot(conf config.Config) error { } } +func buildTwoManApprovalRequestMessage(conf config.Config, sender string, requestedPrincipal string) string { + approvers := []string{} + for _, approver := range conf.GetTwoManApprovers() { + approvers = append(approvers, "@"+approver) + } + + return fmt.Sprintf("@%s has requested access to the two-man realm %s! In order to approve this access, "+ + "reply with a thumbs-up to this message. (Configured approvers: %s)", sender, requestedPrincipal, strings.Join(approvers, ", ")) +} + +func isTwoManPrincipal(conf config.Config, requestedPrincipal string) bool { + for _, team := range conf.GetTwoManApprovers() { + if team == requestedPrincipal { + return true + } + } + return false +} + +// Note that this function is a key security barrier for the two-man feature. This function checks that only people +// in the define list of approvers can approve a request and that people cannot approve their own request. +func isValidApprover(conf config.Config, senderUsername string, signatureRequest shared.SignatureRequest) bool { + validApprover := false + for _, knownApprover := range conf.GetTwoManApprovers() { + if knownApprover == senderUsername { + validApprover = true + } + } + if !validApprover { + log.Debug("Reply came from someone who isn't a valid two man approver, rejecting!") + return false + } + //if senderUsername == signatureRequest.Username { + // log.Debug("Reply came from the sender of the signature request, rejecting!") + // return false + //} + return true +} + +// Respond to the given SignatureRequest and log any errors that are produced. This function does not return any error. +func respondToSignatureRequest(conf config.Config, kbc *kbchat.API, signatureRequest shared.SignatureRequest, Username string, MessageID chat1.MessageID, conversationID string) { + signatureResponse, err := sshutils.ProcessSignatureRequest(conf, signatureRequest) + if err != nil { + LogError(conf, kbc, Username, MessageID, conversationID, err) + return + } + + response, err := json.Marshal(signatureResponse) + if err != nil { + LogError(conf, kbc, Username, MessageID, conversationID, err) + return + } + _, err = kbc.SendMessageByConvID(conversationID, shared.SignatureResponsePreamble+string(response)) + if err != nil { + LogError(conf, kbc, Username, MessageID, conversationID, err) + return + } +} + // Log the given error to Keybase chat and to the configured log file. Used so that the chatbot does not crash // due to an error caused by a malformed message. -func LogError(conf config.Config, kbc *kbchat.API, msg kbchat.SubscriptionMessage, err error) { - message := fmt.Sprintf("Encountered error while processing message from %s (messageID:%d): %v", msg.Message.Sender.Username, msg.Message.Id, err) +func LogError(conf config.Config, kbc *kbchat.API, Username string, MessageID chat1.MessageID, conversationID string, err error) { + message := fmt.Sprintf("Encountered error while processing message from %s (messageID:%d): %v", Username, MessageID, err) auditlog.Log(conf, message) - _, e := kbc.SendMessageByConvID(msg.Message.ConvID, message) + _, e := kbc.SendMessageByConvID(conversationID, message) if e != nil { auditlog.Log(conf, fmt.Sprintf("failed to log an error to chat (something is probably very wrong): %v", err)) } diff --git a/src/keybaseca/config/config.go b/src/keybaseca/config/config.go index d044a6e..6cc6bdb 100644 --- a/src/keybaseca/config/config.go +++ b/src/keybaseca/config/config.go @@ -4,6 +4,7 @@ import ( "fmt" "io/ioutil" "os" + "strconv" "strings" "github.com/keybase/bot-sshca/src/keybaseca/constants" @@ -29,6 +30,9 @@ type Config interface { GetStrictLogging() bool GetAnnouncement() string DebugString() string + GetTwoManTeams() []string + GetTwoManApprovers() []string + GetNumberRequiredApprovers() int } // Validate the given config file. If offline, do so without connecting to keybase (used in code that is meant @@ -76,6 +80,18 @@ func ValidateConfig(conf EnvConfig, offline bool) error { } } } + if len(conf.GetTwoManTeams()) != 0 || len(conf.GetTwoManApprovers()) != 0 { + if len(conf.GetTwoManTeams()) != 0 && len(conf.GetTwoManApprovers()) == 0 { + return fmt.Errorf("cannot specify TWO_MAN_TEAMS without setting TWO_MAN_APPROVERS") + } + if len(conf.GetTwoManTeams()) == 0 && len(conf.GetTwoManApprovers()) != 0 { + return fmt.Errorf("cannot specify TWO_MAN_APPROVERS without setting TWO_MAN_TEAMS") + } + _, err := conf.getNumberRequiredApprovers() + if err != nil { + return fmt.Errorf("failed to parse NUMBER_REQUIRED_APPROVERS value as a intenger: %v", err) + } + } log.Debugf("Validated config: %s", conf.DebugString()) return nil } @@ -196,15 +212,7 @@ func (ef *EnvConfig) GetKeyExpiration() string { // Get the list of keybase teams configured to be used with the bot. func (ef *EnvConfig) GetTeams() []string { - split := strings.Split(os.Getenv("TEAMS"), ",") - var teams []string - for _, item := range split { - trimmed := strings.TrimSpace(item) - if trimmed != "" { - teams = append(teams, trimmed) - } - } - return teams + return commaSeparatedStringToList(os.Getenv("TEAMS")) } // Get the location for the bot's audit logs. May be empty. @@ -272,3 +280,44 @@ func splitTeamChannel(teamChannel string) (string, string, error) { } return split[0], split[1], nil } + +func (ef *EnvConfig) GetTwoManTeams() []string { + twoManTeams := os.Getenv("TWO_MAN_TEAMS") + if twoManTeams == "" { + return []string{} + } + return commaSeparatedStringToList(twoManTeams) +} + +func (ef *EnvConfig) GetTwoManApprovers() []string { + twoManApprovers := os.Getenv("TWO_MAN_APPROVERS") + if twoManApprovers == "" { + return []string{} + } + return commaSeparatedStringToList(twoManApprovers) +} + +func (ef *EnvConfig) getNumberRequiredApprovers() (int, error) { + val := os.Getenv("NUMBER_REQUIRED_APPROVERS") + if val == "" { + return 1, nil + } + return strconv.Atoi(val) +} + +func (ef *EnvConfig) GetNumberRequiredApprovers() int { + num, _ := ef.getNumberRequiredApprovers() + return num +} + +func commaSeparatedStringToList(val string) []string { + split := strings.Split(val, ",") + var teams []string + for _, item := range split { + trimmed := strings.TrimSpace(item) + if trimmed != "" { + teams = append(teams, trimmed) + } + } + return teams +} diff --git a/src/keybaseca/sshutils/sshutils.go b/src/keybaseca/sshutils/sshutils.go index b0ae033..6f3f8c9 100644 --- a/src/keybaseca/sshutils/sshutils.go +++ b/src/keybaseca/sshutils/sshutils.go @@ -80,6 +80,7 @@ func ProcessSignatureRequest(conf config.Config, sr shared.SignatureRequest) (re if err != nil { return } + principals, err := getPrincipals(conf, sr) if err != nil { return @@ -174,14 +175,36 @@ func getPrincipals(conf config.Config, sr shared.SignatureRequest) (string, erro } } + // Map from a team to whether or not it is a two-man only team + // Note that this is a key security barrier in the two-man feature. This ensures that signature requests that do + // not specify a principal are not given any two-man required principals. + teamToTwoManRequired := make(map[string]bool) + for _, team := range conf.GetTeams() { + teamToTwoManRequired[team] = false + } + for _, team := range conf.GetTwoManTeams() { + teamToTwoManRequired[team] = true + } + // Iterate through the teams in the config file and use the subteam as the principal - // if the user is in that subteam + // if the user is in that subteam and the subteam doesn't require the two-man rule var principals []string for _, team := range conf.GetTeams() { - result, ok := teamToMembership[team] - if ok && result { + isMember, ok1 := teamToMembership[team] + requiresTwoMan, ok2 := teamToTwoManRequired[team] + if ok1 && isMember && ok2 && !requiresTwoMan { principals = append(principals, team) } } + + // Add the specific principals that they requested. Note that getPrincipals() is only called if the signature + // request has been validated and the two-man request been approved. + requestedTeam := sr.RequestedPrincipal + isMember, ok := teamToMembership[requestedTeam] + _, isConfiguredTeam := teamToTwoManRequired[requestedTeam] + if ok && isMember && isConfiguredTeam { + principals = append(principals, requestedTeam) + } + return strings.Join(principals, ","), nil } diff --git a/src/kssh/bot.go b/src/kssh/bot.go index 66d1f3f..432b276 100644 --- a/src/kssh/bot.go +++ b/src/kssh/bot.go @@ -6,6 +6,8 @@ import ( "strings" "time" + "github.com/sirupsen/logrus" + "github.com/keybase/bot-sshca/src/shared" "github.com/keybase/go-keybase-chat-bot/kbchat" ) @@ -14,6 +16,12 @@ import ( func GetSignedKey(config ConfigFile, request shared.SignatureRequest) (shared.SignatureResponse, error) { empty := shared.SignatureResponse{} + timeout := 5 * time.Second + if request.RequestedPrincipal != "" { + timeout = time.Hour + logrus.Debug("Requesting a two-man enabled certificate. Setting time out to one hour...") + } + // Start communicating with the Keybase chat API runOptions := kbchat.RunOptions{KeybaseLocation: GetKeybaseBinaryPath()} kbc, err := kbchat.Start(runOptions) @@ -60,7 +68,7 @@ func GetSignedKey(config ConfigFile, request shared.SignatureRequest) (shared.Si hasBeenAcked := false startTime := time.Now() for { - if time.Since(startTime) > 5*time.Second { + if time.Since(startTime) > timeout { return empty, fmt.Errorf("timed out while waiting for a response from the CA") } msg, err := sub.Read() diff --git a/src/shared/chat_types.go b/src/shared/chat_types.go index 466a135..68702ad 100644 --- a/src/shared/chat_types.go +++ b/src/shared/chat_types.go @@ -17,10 +17,11 @@ import ( // The body of signature request messages sent over KB chat type SignatureRequest struct { - SSHPublicKey string `json:"ssh_public_key"` - UUID string `json:"uuid"` - Username string `json:"-"` - DeviceName string `json:"-"` + SSHPublicKey string `json:"ssh_public_key"` + UUID string `json:"uuid"` + RequestedPrincipal string `json:"requested_principal,omitempty"` + Username string `json:"-"` + DeviceName string `json:"-"` } // The preamble used at the start of signature request messages diff --git a/tests/Dockerfile-autoapprover b/tests/Dockerfile-autoapprover new file mode 100644 index 0000000..bb3fa0a --- /dev/null +++ b/tests/Dockerfile-autoapprover @@ -0,0 +1,37 @@ +# This dockerfile builds a container capable of running kssh. Note that a lot of this code is duplicated +# between this file and Dockerfile-ca. # TODO: be less shitty +FROM ubuntu:18.04 + +# Dependencies +RUN apt-get -qq update +RUN apt-get -qq install curl software-properties-common ca-certificates gnupg -y +RUN useradd -ms /bin/bash keybase +USER keybase +WORKDIR /home/keybase + +# Download and verify the deb +RUN curl --remote-name https://prerelease.keybase.io/keybase_amd64.deb +RUN curl --remote-name https://prerelease.keybase.io/keybase_amd64.deb.sig +# Import our gpg key from our website. Pulling from key servers caused a flakey build so +# we get the key from the Keybase website instead. +RUN curl -sSL https://keybase.io/docs/server_security/code_signing_key.asc | gpg --import +# This line will error if the fingerprint of the key in the file does not match the +# known fingerprint of the our PGP key +RUN gpg --fingerprint 222B85B0F90BE2D24CFEB93F47484E50656D16C7 +# And then verify the signature now that we have the key +RUN gpg --verify keybase_amd64.deb.sig keybase_amd64.deb + +# Silence the error from dpkg about failing to configure keybase since `apt-get install -f` fixes it +USER root +RUN dpkg -i keybase_amd64.deb || true +RUN apt-get install -fy +USER keybase + +# Install python +USER root +RUN apt-get update +RUN apt-get install python3.7 python3-pip gettext -y +RUN python3.7 -m pip install pykeybasebot flask +USER keybase + +COPY tests/autoapprover.py . \ No newline at end of file diff --git a/tests/bot-entrypoint.py b/tests/bot-entrypoint.py index 5344d2d..c2e5e6b 100644 --- a/tests/bot-entrypoint.py +++ b/tests/bot-entrypoint.py @@ -28,7 +28,7 @@ def load_env(): "echo yes | bin/keybaseca backup > /shared/cakey.backup\n" # The output from this sign operation is tested in test_env_1.py "ssh-keygen -t ed25519 -f /shared/userkey -N '' && bin/keybaseca sign --public-key /shared/userkey.pub > /shared/keybaseca-sign.out\n" - "bin/keybaseca service &" + "bin/keybaseca --debug service &" ) % (shlex.quote(path))) # Sleep so keybaseca has time to start time.sleep(5) diff --git a/tests/configure_accounts.py b/tests/configure_accounts.py index 266aa65..aa59646 100644 --- a/tests/configure_accounts.py +++ b/tests/configure_accounts.py @@ -123,6 +123,8 @@ def make_user(purpose): paperkey = input("TEST SETUP - What is the paper key: ") return User(username, paperkey) +# TODO: Make testing user + if __name__ == '__main__': print("TEST SETUP - We need to do some one-time test setup.") ca_user = make_user("the CA bot") diff --git a/tests/docker-compose.yml b/tests/docker-compose.yml index a3c87cd..6225985 100644 --- a/tests/docker-compose.yml +++ b/tests/docker-compose.yml @@ -1,5 +1,6 @@ version: '3' services: + # TODO ca-bot: image: ca-bot container_name: ca-bot @@ -10,6 +11,7 @@ services: - BOT_PAPERKEY - BOT_USERNAME - SUBTEAM + - TESTER_USERNAME volumes: - app-volume:/shared/ user: root @@ -19,6 +21,7 @@ services: depends_on: - sshd-prod - sshd-staging + # TODO kssh: image: kssh container_name: kssh @@ -39,6 +42,7 @@ services: - sshd-staging - sshd-prod - ca-bot + - autoapprover # An ssh server that will accept signed requests with the principal "staging" sshd-staging: image: sshd-staging @@ -63,5 +67,18 @@ services: root_principal: ${SUBTEAM}.ssh.root_everywhere volumes: - app-volume:/shared/ + # TODO + autoapprover: + image: autoapprover + container_name: autoapprover + build: + context: ./ + dockerfile: "Dockerfile-autoapprover" + environment: + - TESTER_USERNAME + - TESTER_PAPERKEY + ports: + - 8080 + command: "python3.7 autoapprover.py" volumes: app-volume: diff --git a/tests/envFiles/test_env_5_two_man b/tests/envFiles/test_env_5_two_man new file mode 100644 index 0000000..beaffdc --- /dev/null +++ b/tests/envFiles/test_env_5_two_man @@ -0,0 +1,9 @@ +# Used to test the two man functionality whereby an approval is required for certain realms of servers +export KEY_EXPIRATION="+1h" +export LOG_LOCATION="/shared/ca.log" +export TEAMS="$SUBTEAM.ssh.staging,$SUBTEAM.ssh.prod,$SUBTEAM.ssh.root_everywhere" +export KEYBASE_PAPERKEY="$BOT_PAPERKEY" +export KEYBASE_USERNAME="$BOT_USERNAME" +export CA_KEY_LOCATION="/shared/keybase-ca-key" +export TWO_MAN_TEAMS="$SUBTEAM.ssh.root_everywhere" +export TWO_MAN_APPROVERS="$TESTER_USERNAME" # TODO: Need to make this user diff --git a/tests/tester-entrypoint.sh b/tests/tester-entrypoint.sh index d277fe5..6ed0ac2 100755 --- a/tests/tester-entrypoint.sh +++ b/tests/tester-entrypoint.sh @@ -10,7 +10,7 @@ do sleep 1 done echo "" -sleep 2 +sleep 5 keybase oneshot --username $KSSH_USERNAME --paperkey "$KSSH_PAPERKEY" diff --git a/tests/tests/autoapprover.py b/tests/tests/autoapprover.py new file mode 100644 index 0000000..bc00e1c --- /dev/null +++ b/tests/tests/autoapprover.py @@ -0,0 +1,68 @@ +#!/usr/bin/env python3.7 + +import asyncio +import logging +import os +import sys +import time +from multiprocessing import Process, Value + +from flask import Flask, request + + +import pykeybasebot.types.chat1 as chat1 +from pykeybasebot import Bot + +logging.basicConfig(level=logging.DEBUG) + + +class Handler: + def __init__(self, shared_running_val: Value): + self.shared_running_val = shared_running_val + + async def __call__(self, bot, event): + print("HANDLER CALLED") + if self.shared_running_val.value: + print("RUNNING") + if event.msg.content.type_name != chat1.MessageTypeStrings.TEXT.value: + return + channel = event.msg.channel + msg_id = event.msg.id + body = event.msg.content.text.body + if "has requested access to the two-man realm" in body: + await bot.chat.react(channel, msg_id, ":+1:") + else: + print("NOT RUNNING") + +shared_running_val = Value('i', 0) + +def start_bot_event_loop(): + username = os.environ["TESTER_USERNAME"] + paperkey = os.environ["TESTER_PAPERKEY"] + bot = Bot( + username=username, paperkey=paperkey, + handler=Handler(shared_running_val) + ) + p = Process(target=lambda: asyncio.run(bot.start({}))) + p.start() + +app = Flask(__name__) + +@app.route('/start') +def start_autoresponder(): + global shared_running_val + shared_running_val.value = 1 + time.sleep(1) + return "OK" + +@app.route('/stop') +def stop_autoresponder(): + global shared_running_val + shared_running_val.value = 0 + time.sleep(1) + return "OK" + + +if __name__ == '__main__': + start_bot_event_loop() + app.run(host='0.0.0.0', port='8080') diff --git a/tests/tests/test_env_1.py b/tests/tests/test_env_1.py index 7ce1a35..6cae15a 100644 --- a/tests/tests/test_env_1.py +++ b/tests/tests/test_env_1.py @@ -21,149 +21,150 @@ def test_kssh_staging_user(self, test_config): with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging "sha1sum /etc/unique" """)) - def test_kssh_staging_root(self, test_config): - # Test ksshing into staging as user - with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-staging "sha1sum /etc/unique" """)) - - def test_kssh_prod_root(self, test_config): - # Test ksshing into prod as root - with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) - - def test_kssh_reject_prod_user(self, test_config): - # Test that we can't kssh into prod as user since we aren't in the correct team for that - with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - try: - run_command_with_agent("""bin/kssh -o StrictHostKeyChecking=no user@sshd-prod "sha1sum /etc/unique" 2>&1 """) - assert False - except subprocess.CalledProcessError as e: - assert b"Permission denied" in e.output - assert test_config.expected_hash not in e.output - - def test_kssh_reuse(self, test_config): - # Test that kssh reuses unexpired keys - with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) - start = time.time() - assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) - elapsed = time.time() - start - assert elapsed < 0.5 - - def test_kssh_regenerate_expired_keys(self, test_config): - # Test that kssh reprovisions a key when the stored keys are expired - with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - run_command_with_agent("mv ~/tests/testFiles/expired ~/.ssh/keybase-signed-key-- && mv ~/tests/testFiles/expired.pub ~/.ssh/keybase-signed-key--.pub && mv ~/tests/testFiles/expired-cert.pub ~/.ssh/keybase-signed-key---cert.pub") - assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) - - def test_kssh_provision(self, test_config): - # Test the `kssh --provision` flag - # we have to run all of the below commands in one run_command call so that environment variables are shared - # so ssh-agent can work - with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - output = run_command_with_agent(""" - bin/kssh --provision - ssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" - echo -n foo > /tmp/foo - scp /tmp/foo root@sshd-prod:/tmp/foo - ssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /tmp/foo" - """) - assert_contains_hash(test_config.expected_hash, output) - assert hashlib.sha1(b"foo").hexdigest().encode('utf-8') in output - assert get_principals("~/.ssh/keybase-signed-key---cert.pub") == set([test_config.subteam + ".ssh.staging", test_config.subteam + ".ssh.root_everywhere"]) - - def test_kssh_errors_on_two_bots(self, test_config): - # Test that kssh does not run if there are multiple bots, no kssh config, and no --bot flag - with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=0): - try: - run_command_with_agent("bin/kssh root@sshd-prod") - assert False - except subprocess.CalledProcessError as e: - assert b"Found 2 config files" in e.output - - def test_kssh_bot_flag(self, test_config): - # Test that kssh works with the --bot flag - with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --bot {test_config.bot_username} -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) - - def test_kssh_set_default_bot(self, test_config): - # Test that kssh works with the --set-default-bot flag - with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - run_command_with_agent(f"bin/kssh --set-default-bot {test_config.bot_username}") - assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) - - def test_kssh_override_default_bot(self, test_config): - # Test that the --bot flag overrides the local config file - with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - run_command_with_agent(f"bin/kssh --set-default-bot otherbotname") - assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --bot {test_config.bot_username} -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) - - def test_kssh_clear_default_bot(self, test_config): - # Test that kssh --clear-default-bot clears the default bot - with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=0): - run_command_with_agent(f"bin/kssh --set-default-bot {test_config.bot_username}") - run_command_with_agent("bin/kssh --clear-default-bot") - try: - # No default set and no bot specified so it will error out - run_command_with_agent("bin/kssh root@sshd-prod") - assert False - except subprocess.CalledProcessError as e: - assert b"Found 2 config files" in e.output - - def test_keybaseca_backup(self): - # Test the keybaseca backup command by reading and verifying the private key stored in /shared/cakey.backup - run_command("mkdir -p /tmp/ssh/") - run_command("chown -R keybase:keybase /tmp/ssh/") - with open('/shared/cakey.backup') as f: - keyLines = [] - add = False - for line in f: - if "----" in line and "PRIVATE" in line and "BEGIN" in line: - add = True - if add: - keyLines.append(line.strip()) - if "----" in line and "PRIVATE" in line and "END" in line: - add = False - key = '\n'.join(keyLines) + '\n' - with open('/tmp/ssh/cakey', 'w+') as f: - f.write(key) - run_command("chmod 0600 /tmp/ssh/cakey") - output = run_command("ssh-keygen -y -e -f /tmp/ssh/cakey") - assert b"BEGIN SSH2 PUBLIC KEY" in output - - def test_keybaseca_sign(self, test_config): - # Stdout contains a useful message - with open('/shared/keybaseca-sign.out') as f: - assert "Provisioned new certificate" in f.read() - - # SSH with that certificate should just work for every team - assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey user@sshd-prod 'sha1sum /etc/unique'")) - assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey root@sshd-prod 'sha1sum /etc/unique'")) - assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey user@sshd-staging 'sha1sum /etc/unique'")) - assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey root@sshd-prod 'sha1sum /etc/unique'")) - - # Checking that it actually contains the correct principals - assert get_principals("/shared/userkey-cert.pub") == set(test_config.subteams) - - def test_kssh_default_user(self, test_config): - # Set the default user to root - run_command_with_agent("bin/kssh --set-default-user root") - # A normal SSH connection - assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no sshd-prod 'sha1sum /etc/unique'")) - assert b"root" in run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no sshd-prod 'whoami'") - # A proxy jump (relies on the ssh agent) - assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no -J sshd-staging sshd-prod 'sha1sum /etc/unique'")) - # Reset the default user - run_command_with_agent("bin/kssh --clear-default-user") + # def test_kssh_staging_root(self, test_config): + # # Test ksshing into staging as user + # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + # assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-staging "sha1sum /etc/unique" """)) + # + # def test_kssh_prod_root(self, test_config): + # # Test ksshing into prod as root + # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + # assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) + # + # def test_kssh_reject_prod_user(self, test_config): + # # Test that we can't kssh into prod as user since we aren't in the correct team for that + # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + # try: + # run_command_with_agent("""bin/kssh -o StrictHostKeyChecking=no user@sshd-prod "sha1sum /etc/unique" 2>&1 """) + # assert False + # except subprocess.CalledProcessError as e: + # assert b"Permission denied" in e.output + # assert test_config.expected_hash not in e.output + # + # def test_kssh_reuse(self, test_config): + # # Test that kssh reuses unexpired keys + # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + # assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) + # start = time.time() + # assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) + # elapsed = time.time() - start + # assert elapsed < 0.5 + # + # def test_kssh_regenerate_expired_keys(self, test_config): + # # Test that kssh reprovisions a key when the stored keys are expired + # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + # run_command_with_agent("mv ~/tests/testFiles/expired ~/.ssh/keybase-signed-key-- && mv ~/tests/testFiles/expired.pub ~/.ssh/keybase-signed-key--.pub && mv ~/tests/testFiles/expired-cert.pub ~/.ssh/keybase-signed-key---cert.pub") + # assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) + # + # def test_kssh_provision(self, test_config): + # # Test the `kssh --provision` flag + # # we have to run all of the below commands in one run_command call so that environment variables are shared + # # so ssh-agent can work + # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + # output = run_command_with_agent(""" + # bin/kssh --provision + # ssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" + # echo -n foo > /tmp/foo + # scp /tmp/foo root@sshd-prod:/tmp/foo + # ssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /tmp/foo" + # """) + # assert_contains_hash(test_config.expected_hash, output) + # assert hashlib.sha1(b"foo").hexdigest().encode('utf-8') in output + # assert get_principals("~/.ssh/keybase-signed-key---cert.pub") == set([test_config.subteam + ".ssh.staging", test_config.subteam + ".ssh.root_everywhere"]) + # + # def test_kssh_errors_on_two_bots(self, test_config): + # # Test that kssh does not run if there are multiple bots, no kssh config, and no --bot flag + # with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=0): + # try: + # run_command_with_agent("bin/kssh root@sshd-prod") + # assert False + # except subprocess.CalledProcessError as e: + # assert b"Found 2 config files" in e.output + # + # def test_kssh_bot_flag(self, test_config): + # # Test that kssh works with the --bot flag + # with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + # assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --bot {test_config.bot_username} -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) + # + # def test_kssh_set_default_bot(self, test_config): + # # Test that kssh works with the --set-default-bot flag + # with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + # run_command_with_agent(f"bin/kssh --set-default-bot {test_config.bot_username}") + # assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) + # + # def test_kssh_override_default_bot(self, test_config): + # # Test that the --bot flag overrides the local config file + # with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + # run_command_with_agent(f"bin/kssh --set-default-bot otherbotname") + # assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --bot {test_config.bot_username} -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) + # + # def test_kssh_clear_default_bot(self, test_config): + # # Test that kssh --clear-default-bot clears the default bot + # with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=0): + # run_command_with_agent(f"bin/kssh --set-default-bot {test_config.bot_username}") + # run_command_with_agent("bin/kssh --clear-default-bot") + # try: + # # No default set and no bot specified so it will error out + # run_command_with_agent("bin/kssh root@sshd-prod") + # assert False + # except subprocess.CalledProcessError as e: + # assert b"Found 2 config files" in e.output + # + # def test_keybaseca_backup(self): + # # Test the keybaseca backup command by reading and verifying the private key stored in /shared/cakey.backup + # run_command("mkdir -p /tmp/ssh/") + # run_command("chown -R keybase:keybase /tmp/ssh/") + # with open('/shared/cakey.backup') as f: + # keyLines = [] + # add = False + # for line in f: + # if "----" in line and "PRIVATE" in line and "BEGIN" in line: + # add = True + # if add: + # keyLines.append(line.strip()) + # if "----" in line and "PRIVATE" in line and "END" in line: + # add = False + # key = '\n'.join(keyLines) + '\n' + # with open('/tmp/ssh/cakey', 'w+') as f: + # f.write(key) + # run_command("chmod 0600 /tmp/ssh/cakey") + # output = run_command("ssh-keygen -y -e -f /tmp/ssh/cakey") + # assert b"BEGIN SSH2 PUBLIC KEY" in output + # + # def test_keybaseca_sign(self, test_config): + # # Stdout contains a useful message + # with open('/shared/keybaseca-sign.out') as f: + # assert "Provisioned new certificate" in f.read() + # + # # SSH with that certificate should just work for every team + # assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey user@sshd-prod 'sha1sum /etc/unique'")) + # assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey root@sshd-prod 'sha1sum /etc/unique'")) + # assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey user@sshd-staging 'sha1sum /etc/unique'")) + # assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey root@sshd-prod 'sha1sum /etc/unique'")) + # + # # Checking that it actually contains the correct principals + # assert get_principals("/shared/userkey-cert.pub") == set(test_config.subteams) + # + # def test_kssh_default_user(self, test_config): + # # Set the default user to root + # run_command_with_agent("bin/kssh --set-default-user root") + # # A normal SSH connection + # assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no sshd-prod 'sha1sum /etc/unique'")) + # assert b"root" in run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no sshd-prod 'whoami'") + # # A proxy jump (relies on the ssh agent) + # assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no -J sshd-staging sshd-prod 'sha1sum /etc/unique'")) + # # Reset the default user + # run_command_with_agent("bin/kssh --clear-default-user") + # + # def test_kssh_alternate_binary(self, test_config): + # # Test it by creating another keybase binary earlier in the path and running kssh. This isn't a perfect test but it is + # # enough to smoketest it + # run_command("echo '#!/bin/bash' | sudo tee /usr/local/bin/keybase") + # run_command("sudo chmod +x /usr/local/bin/keybase") + # try: + # run_command("bin/kssh --set-keybase-binary /usr/bin/keybase") + # assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging 'sha1sum /etc/unique'")) + # run_command("bin/kssh --set-keybase-binary ''") + # finally: + # run_command("sudo rm /usr/local/bin/keybase") - def test_kssh_alternate_binary(self, test_config): - # Test it by creating another keybase binary earlier in the path and running kssh. This isn't a perfect test but it is - # enough to smoketest it - run_command("echo '#!/bin/bash' | sudo tee /usr/local/bin/keybase") - run_command("sudo chmod +x /usr/local/bin/keybase") - try: - run_command("bin/kssh --set-keybase-binary /usr/bin/keybase") - assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging 'sha1sum /etc/unique'")) - run_command("bin/kssh --set-keybase-binary ''") - finally: - run_command("sudo rm /usr/local/bin/keybase") \ No newline at end of file diff --git a/tests/tests/test_env_2_local_audit_log.py b/tests/tests/test_env_2_local_audit_log.py index bc3da8e..d98678c 100644 --- a/tests/tests/test_env_2_local_audit_log.py +++ b/tests/tests/test_env_2_local_audit_log.py @@ -1,17 +1,17 @@ -import pytest - -from lib import TestConfig, load_env, outputs_audit_log, assert_contains_hash, run_command_with_agent - -class TestEnv2LocalAuditLog: - @pytest.fixture(autouse=True, scope='class') - def configure_env(self): - assert load_env(__file__) - - @pytest.fixture(autouse=True, scope='class') - def test_config(self): - return TestConfig.getDefaultTestConfig() - - def test_kssh(self, test_config): - # Test ksshing into staging as user - with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=1): - assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging "sha1sum /etc/unique" """)) +# import pytest +# +# from lib import TestConfig, load_env, outputs_audit_log, assert_contains_hash, run_command_with_agent +# +# class TestEnv2LocalAuditLog: +# @pytest.fixture(autouse=True, scope='class') +# def configure_env(self): +# assert load_env(__file__) +# +# @pytest.fixture(autouse=True, scope='class') +# def test_config(self): +# return TestConfig.getDefaultTestConfig() +# +# def test_kssh(self, test_config): +# # Test ksshing into staging as user +# with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=1): +# assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging "sha1sum /etc/unique" """)) diff --git a/tests/tests/test_env_3_user_not_in_first_team.py b/tests/tests/test_env_3_user_not_in_first_team.py index be8633e..db25cf8 100644 --- a/tests/tests/test_env_3_user_not_in_first_team.py +++ b/tests/tests/test_env_3_user_not_in_first_team.py @@ -1,22 +1,22 @@ -import pytest - -from lib import TestConfig, load_env, outputs_audit_log, clear_keys, outputs_audit_log, assert_contains_hash, run_command_with_agent - -class TestEnv3UserNotInFirstTeam: - @pytest.fixture(autouse=True, scope='class') - def configure_env(self): - assert load_env(__file__) - - @pytest.fixture(autouse=True, scope='class') - def test_config(self): - return TestConfig.getDefaultTestConfig() - - def test_kssh(self, test_config): - # Test ksshing which tests that it is correctly finding a client config - with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=3): - clear_keys() - assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging "sha1sum /etc/unique" """)) - clear_keys() - assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-staging "sha1sum /etc/unique" """)) - clear_keys() - assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) +# import pytest +# +# from lib import TestConfig, load_env, outputs_audit_log, clear_keys, outputs_audit_log, assert_contains_hash, run_command_with_agent +# +# class TestEnv3UserNotInFirstTeam: +# @pytest.fixture(autouse=True, scope='class') +# def configure_env(self): +# assert load_env(__file__) +# +# @pytest.fixture(autouse=True, scope='class') +# def test_config(self): +# return TestConfig.getDefaultTestConfig() +# +# def test_kssh(self, test_config): +# # Test ksshing which tests that it is correctly finding a client config +# with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=3): +# clear_keys() +# assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging "sha1sum /etc/unique" """)) +# clear_keys() +# assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-staging "sha1sum /etc/unique" """)) +# clear_keys() +# assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) diff --git a/tests/tests/test_env_4_user_not_in_configured_teams.py b/tests/tests/test_env_4_user_not_in_configured_teams.py index 8c0a2eb..b1714d0 100644 --- a/tests/tests/test_env_4_user_not_in_configured_teams.py +++ b/tests/tests/test_env_4_user_not_in_configured_teams.py @@ -1,38 +1,38 @@ -import json -import subprocess - -import pytest - -from lib import TestConfig, load_env, outputs_audit_log, run_command, run_command_with_agent - -class TestEnv4UserNotInConfiguredTeams: - @pytest.fixture(autouse=True, scope='class') - def configure_env(self): - assert load_env(__file__) - - @pytest.fixture(autouse=True, scope='class') - def test_config(self): - return TestConfig.getDefaultTestConfig() - - def test_kssh_no_config_files(self, test_config): - # Test that it can't find any config files - with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0): - for s in ['user@sshd-staging', 'root@sshd-staging', 'user@sshd-prod', 'root@sshd-prod']: - try: - run_command_with_agent(f"""bin/kssh -q -o StrictHostKeyChecking=no {s} "sha1sum /etc/unique" """) - assert False - except subprocess.CalledProcessError as e: - assert b"Did not find any config files in KBFS" in e.output - - def test_kssh_spoofed_config(self, test_config): - # Test that even when kssh is forced to run by a spoofed config, the CA bot ignores messages that are in the - # wrong channel - with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0): - client_config = json.dumps({'teamname': f"{test_config.subteam}.ssh", "channelname": "", "botname": test_config.bot_username}) - run_command(f"echo '{client_config}' | keybase fs write /keybase/team/{test_config.subteam}.ssh/kssh-client.config") - for s in ['user@sshd-staging', 'root@sshd-staging', 'user@sshd-prod', 'root@sshd-prod']: - try: - run_command_with_agent(f"""bin/kssh -q -o StrictHostKeyChecking=no {s} "sha1sum /etc/unique" """) - assert False - except subprocess.CalledProcessError as e: - assert b"Failed to get a signed key from the CA: timed out while waiting for a response from the CA" in e.output +# import json +# import subprocess +# +# import pytest +# +# from lib import TestConfig, load_env, outputs_audit_log, run_command, run_command_with_agent +# +# class TestEnv4UserNotInConfiguredTeams: +# @pytest.fixture(autouse=True, scope='class') +# def configure_env(self): +# assert load_env(__file__) +# +# @pytest.fixture(autouse=True, scope='class') +# def test_config(self): +# return TestConfig.getDefaultTestConfig() +# +# def test_kssh_no_config_files(self, test_config): +# # Test that it can't find any config files +# with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0): +# for s in ['user@sshd-staging', 'root@sshd-staging', 'user@sshd-prod', 'root@sshd-prod']: +# try: +# run_command_with_agent(f"""bin/kssh -q -o StrictHostKeyChecking=no {s} "sha1sum /etc/unique" """) +# assert False +# except subprocess.CalledProcessError as e: +# assert b"Did not find any config files in KBFS" in e.output +# +# def test_kssh_spoofed_config(self, test_config): +# # Test that even when kssh is forced to run by a spoofed config, the CA bot ignores messages that are in the +# # wrong channel +# with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0): +# client_config = json.dumps({'teamname': f"{test_config.subteam}.ssh", "channelname": "", "botname": test_config.bot_username}) +# run_command(f"echo '{client_config}' | keybase fs write /keybase/team/{test_config.subteam}.ssh/kssh-client.config") +# for s in ['user@sshd-staging', 'root@sshd-staging', 'user@sshd-prod', 'root@sshd-prod']: +# try: +# run_command_with_agent(f"""bin/kssh -q -o StrictHostKeyChecking=no {s} "sha1sum /etc/unique" """) +# assert False +# except subprocess.CalledProcessError as e: +# assert b"Failed to get a signed key from the CA: timed out while waiting for a response from the CA" in e.output diff --git a/tests/tests/test_env_5_two_man.py b/tests/tests/test_env_5_two_man.py new file mode 100644 index 0000000..06fbc23 --- /dev/null +++ b/tests/tests/test_env_5_two_man.py @@ -0,0 +1,24 @@ +import json +import subprocess + +import pytest +import requests + +from lib import TestConfig, load_env, assert_contains_hash, run_command_with_agent + +class TestEnv5TwoMan: + @pytest.fixture(autouse=True, scope='class') + def configure_env(self): + assert load_env(__file__) + + @pytest.fixture(autouse=True, scope='class') + def test_config(self): + return TestConfig.getDefaultTestConfig() + + def test_kssh_with_two_man(self, test_config): + assert requests.get('http://autoapprover:8080/start').content == b"OK" + + try: + assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --request-realm {test_config.subteam}.ssh.root_everywhere -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) + finally: + assert requests.get('http://autoapprover:8080/stop').content == b"OK" From 8e3a92e5b2f457db936fc0dd696a4ed21b22509a Mon Sep 17 00:00:00 2001 From: David Dworken Date: Mon, 30 Sep 2019 10:10:36 -0400 Subject: [PATCH 02/16] Fix security vulnerability in WIP two-man mode This makes it so it is not possible for the same person to double-approve a message. It used to be possible to double-approve a message by reacting, unreacting, and reacting again. This is now prevented by keeping a list of approvers and checking whether someone is double approving. Also re-enables all the other integration tests that were disabled for testing purposes. --- src/keybaseca/bot/bot.go | 53 +++- tests/tests/test_env_1.py | 292 +++++++++--------- tests/tests/test_env_2_local_audit_log.py | 34 +- .../test_env_3_user_not_in_first_team.py | 44 +-- ...test_env_4_user_not_in_configured_teams.py | 76 ++--- tests/tests/test_env_5_two_man.py | 2 + 6 files changed, 262 insertions(+), 239 deletions(-) diff --git a/src/keybaseca/bot/bot.go b/src/keybaseca/bot/bot.go index 54f712d..f3fdfb5 100644 --- a/src/keybaseca/bot/bot.go +++ b/src/keybaseca/bot/bot.go @@ -41,7 +41,7 @@ func GetUsername(conf config.Config) (string, error) { type OutstandingTwoManSignatureRequest struct { SignatureRequest shared.SignatureRequest RequestMessageID chat1.MessageID - ApprovalCount int + Approvers []string ConvID string } @@ -77,8 +77,6 @@ func StartBot(conf config.Config) error { } if msg.Message.Content.TypeName == "reaction" { - // TODO: Do I care about the fact that the bot user could react to messages? - // TODO: SECURITY VULN: Someone can approve a message multiple times by reacting and then unreacting emoji := msg.Message.Content.Reaction.Body responseTo := msg.Message.Content.Reaction.MessageID @@ -86,12 +84,22 @@ func StartBot(conf config.Config) error { for _, outstanding := range outstandingTwoManRequests { if outstanding.RequestMessageID == responseTo { log.Debug("Message is a reply to an outstanding two-man request") - if emoji == ":+1:" && isValidApprover(conf, msg.Message.Sender.Username, outstanding.SignatureRequest) { - outstanding.ApprovalCount++ - log.WithField("requester", outstanding.SignatureRequest.Username).WithField("approver", msg.Message.Sender.Username).WithField("approval_count", outstanding.ApprovalCount).Debugf("Message approved request") + approver := msg.Message.Sender.Username + if emoji == ":+1:" && isValidApprover(conf, approver, outstanding.SignatureRequest) { + isDuplicateApprover := addApprover(&outstanding, approver) + if isDuplicateApprover { + // TODO: Test this code + log.Debugf("Rejecting duplicate approver %s since they already approved the two-man request with ID=%s", approver, outstanding.SignatureRequest.UUID) + continue + } + log.WithField("requester", outstanding.SignatureRequest.Username). + WithField("current_approver", approver). + WithField("all_approvers", outstanding.Approvers). + Debugf("Message approved request") threshold := conf.GetNumberRequiredApprovers() - if outstanding.ApprovalCount >= threshold { + if len(outstanding.Approvers) >= threshold { respondToSignatureRequest(conf, kbc, outstanding.SignatureRequest, outstanding.SignatureRequest.Username, outstanding.RequestMessageID, outstanding.ConvID) + auditlog.Log(conf, fmt.Sprintf("Two-man SignatureRequest id=%s approved by %v", outstanding.SignatureRequest.UUID, outstanding.Approvers)) } } else { log.Debug("Message did not approve request") @@ -106,14 +114,14 @@ func StartBot(conf config.Config) error { messageBody := msg.Message.Content.Text.Body log.Debugf("Received message in %s#%s: %s", msg.Message.Channel.Name, msg.Message.Channel.TopicName, messageBody) - //if msg.Message.Sender.Username == kbc.GetUsername() { - // log.Debug("Skipping message since it comes from the bot user") - // if strings.Contains(messageBody, shared.AckRequestPrefix) || strings.Contains(messageBody, shared.SignatureRequestPreamble) { - // log.Warn("Ignoring AckRequest/SignatureRequest coming from the bot user! Are you trying to run the bot " + - // "and kssh as the same user?") - // } - // continue - //} + if msg.Message.Sender.Username == kbc.GetUsername() { + log.Debug("Skipping message since it comes from the bot user") + if strings.Contains(messageBody, shared.AckRequestPrefix) || strings.Contains(messageBody, shared.SignatureRequestPreamble) { + log.Warn("Ignoring AckRequest/SignatureRequest coming from the bot user! Are you trying to run the bot " + + "and kssh as the same user?") + } + continue + } // Note that this line is one of the main security barriers around the SSH bot. If this line were removed // or had a bug, it would cause the SSH bot to respond to any SignatureRequest messages in any channels. This @@ -158,7 +166,7 @@ func StartBot(conf config.Config) error { } outstandingTwoManRequests = append(outstandingTwoManRequests, - OutstandingTwoManSignatureRequest{SignatureRequest: signatureRequest, ApprovalCount: 0, RequestMessageID: *resp.Result.MessageID, ConvID: msg.Message.ConvID}) + OutstandingTwoManSignatureRequest{SignatureRequest: signatureRequest, Approvers: []string{}, RequestMessageID: *resp.Result.MessageID, ConvID: msg.Message.ConvID}) } } } else { @@ -167,6 +175,19 @@ func StartBot(conf config.Config) error { } } +// Add the given approver to the list of approvers in the given outstanding two man request if the +// given user has not already approved the request. Returns whether the given approver has already +// approved the request. +func addApprover(request *OutstandingTwoManSignatureRequest, approver string) bool { + for _, curApprover := range request.Approvers { + if curApprover == approver { + return true + } + } + request.Approvers = append(request.Approvers, approver) + return false +} + func buildTwoManApprovalRequestMessage(conf config.Config, sender string, requestedPrincipal string) string { approvers := []string{} for _, approver := range conf.GetTwoManApprovers() { diff --git a/tests/tests/test_env_1.py b/tests/tests/test_env_1.py index 6cae15a..313b45e 100644 --- a/tests/tests/test_env_1.py +++ b/tests/tests/test_env_1.py @@ -21,150 +21,150 @@ def test_kssh_staging_user(self, test_config): with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging "sha1sum /etc/unique" """)) - # def test_kssh_staging_root(self, test_config): - # # Test ksshing into staging as user - # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - # assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-staging "sha1sum /etc/unique" """)) - # - # def test_kssh_prod_root(self, test_config): - # # Test ksshing into prod as root - # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - # assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) - # - # def test_kssh_reject_prod_user(self, test_config): - # # Test that we can't kssh into prod as user since we aren't in the correct team for that - # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - # try: - # run_command_with_agent("""bin/kssh -o StrictHostKeyChecking=no user@sshd-prod "sha1sum /etc/unique" 2>&1 """) - # assert False - # except subprocess.CalledProcessError as e: - # assert b"Permission denied" in e.output - # assert test_config.expected_hash not in e.output - # - # def test_kssh_reuse(self, test_config): - # # Test that kssh reuses unexpired keys - # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - # assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) - # start = time.time() - # assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) - # elapsed = time.time() - start - # assert elapsed < 0.5 - # - # def test_kssh_regenerate_expired_keys(self, test_config): - # # Test that kssh reprovisions a key when the stored keys are expired - # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - # run_command_with_agent("mv ~/tests/testFiles/expired ~/.ssh/keybase-signed-key-- && mv ~/tests/testFiles/expired.pub ~/.ssh/keybase-signed-key--.pub && mv ~/tests/testFiles/expired-cert.pub ~/.ssh/keybase-signed-key---cert.pub") - # assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) - # - # def test_kssh_provision(self, test_config): - # # Test the `kssh --provision` flag - # # we have to run all of the below commands in one run_command call so that environment variables are shared - # # so ssh-agent can work - # with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - # output = run_command_with_agent(""" - # bin/kssh --provision - # ssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" - # echo -n foo > /tmp/foo - # scp /tmp/foo root@sshd-prod:/tmp/foo - # ssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /tmp/foo" - # """) - # assert_contains_hash(test_config.expected_hash, output) - # assert hashlib.sha1(b"foo").hexdigest().encode('utf-8') in output - # assert get_principals("~/.ssh/keybase-signed-key---cert.pub") == set([test_config.subteam + ".ssh.staging", test_config.subteam + ".ssh.root_everywhere"]) - # - # def test_kssh_errors_on_two_bots(self, test_config): - # # Test that kssh does not run if there are multiple bots, no kssh config, and no --bot flag - # with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=0): - # try: - # run_command_with_agent("bin/kssh root@sshd-prod") - # assert False - # except subprocess.CalledProcessError as e: - # assert b"Found 2 config files" in e.output - # - # def test_kssh_bot_flag(self, test_config): - # # Test that kssh works with the --bot flag - # with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - # assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --bot {test_config.bot_username} -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) - # - # def test_kssh_set_default_bot(self, test_config): - # # Test that kssh works with the --set-default-bot flag - # with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - # run_command_with_agent(f"bin/kssh --set-default-bot {test_config.bot_username}") - # assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) - # - # def test_kssh_override_default_bot(self, test_config): - # # Test that the --bot flag overrides the local config file - # with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): - # run_command_with_agent(f"bin/kssh --set-default-bot otherbotname") - # assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --bot {test_config.bot_username} -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) - # - # def test_kssh_clear_default_bot(self, test_config): - # # Test that kssh --clear-default-bot clears the default bot - # with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=0): - # run_command_with_agent(f"bin/kssh --set-default-bot {test_config.bot_username}") - # run_command_with_agent("bin/kssh --clear-default-bot") - # try: - # # No default set and no bot specified so it will error out - # run_command_with_agent("bin/kssh root@sshd-prod") - # assert False - # except subprocess.CalledProcessError as e: - # assert b"Found 2 config files" in e.output - # - # def test_keybaseca_backup(self): - # # Test the keybaseca backup command by reading and verifying the private key stored in /shared/cakey.backup - # run_command("mkdir -p /tmp/ssh/") - # run_command("chown -R keybase:keybase /tmp/ssh/") - # with open('/shared/cakey.backup') as f: - # keyLines = [] - # add = False - # for line in f: - # if "----" in line and "PRIVATE" in line and "BEGIN" in line: - # add = True - # if add: - # keyLines.append(line.strip()) - # if "----" in line and "PRIVATE" in line and "END" in line: - # add = False - # key = '\n'.join(keyLines) + '\n' - # with open('/tmp/ssh/cakey', 'w+') as f: - # f.write(key) - # run_command("chmod 0600 /tmp/ssh/cakey") - # output = run_command("ssh-keygen -y -e -f /tmp/ssh/cakey") - # assert b"BEGIN SSH2 PUBLIC KEY" in output - # - # def test_keybaseca_sign(self, test_config): - # # Stdout contains a useful message - # with open('/shared/keybaseca-sign.out') as f: - # assert "Provisioned new certificate" in f.read() - # - # # SSH with that certificate should just work for every team - # assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey user@sshd-prod 'sha1sum /etc/unique'")) - # assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey root@sshd-prod 'sha1sum /etc/unique'")) - # assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey user@sshd-staging 'sha1sum /etc/unique'")) - # assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey root@sshd-prod 'sha1sum /etc/unique'")) - # - # # Checking that it actually contains the correct principals - # assert get_principals("/shared/userkey-cert.pub") == set(test_config.subteams) - # - # def test_kssh_default_user(self, test_config): - # # Set the default user to root - # run_command_with_agent("bin/kssh --set-default-user root") - # # A normal SSH connection - # assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no sshd-prod 'sha1sum /etc/unique'")) - # assert b"root" in run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no sshd-prod 'whoami'") - # # A proxy jump (relies on the ssh agent) - # assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no -J sshd-staging sshd-prod 'sha1sum /etc/unique'")) - # # Reset the default user - # run_command_with_agent("bin/kssh --clear-default-user") - # - # def test_kssh_alternate_binary(self, test_config): - # # Test it by creating another keybase binary earlier in the path and running kssh. This isn't a perfect test but it is - # # enough to smoketest it - # run_command("echo '#!/bin/bash' | sudo tee /usr/local/bin/keybase") - # run_command("sudo chmod +x /usr/local/bin/keybase") - # try: - # run_command("bin/kssh --set-keybase-binary /usr/bin/keybase") - # assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging 'sha1sum /etc/unique'")) - # run_command("bin/kssh --set-keybase-binary ''") - # finally: - # run_command("sudo rm /usr/local/bin/keybase") + def test_kssh_staging_root(self, test_config): + # Test ksshing into staging as user + with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-staging "sha1sum /etc/unique" """)) + + def test_kssh_prod_root(self, test_config): + # Test ksshing into prod as root + with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) + + def test_kssh_reject_prod_user(self, test_config): + # Test that we can't kssh into prod as user since we aren't in the correct team for that + with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + try: + run_command_with_agent("""bin/kssh -o StrictHostKeyChecking=no user@sshd-prod "sha1sum /etc/unique" 2>&1 """) + assert False + except subprocess.CalledProcessError as e: + assert b"Permission denied" in e.output + assert test_config.expected_hash not in e.output + + def test_kssh_reuse(self, test_config): + # Test that kssh reuses unexpired keys + with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) + start = time.time() + assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) + elapsed = time.time() - start + assert elapsed < 0.5 + + def test_kssh_regenerate_expired_keys(self, test_config): + # Test that kssh reprovisions a key when the stored keys are expired + with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + run_command_with_agent("mv ~/tests/testFiles/expired ~/.ssh/keybase-signed-key-- && mv ~/tests/testFiles/expired.pub ~/.ssh/keybase-signed-key--.pub && mv ~/tests/testFiles/expired-cert.pub ~/.ssh/keybase-signed-key---cert.pub") + assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) + + def test_kssh_provision(self, test_config): + # Test the `kssh --provision` flag + # we have to run all of the below commands in one run_command call so that environment variables are shared + # so ssh-agent can work + with outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + output = run_command_with_agent(""" + bin/kssh --provision + ssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" + echo -n foo > /tmp/foo + scp /tmp/foo root@sshd-prod:/tmp/foo + ssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /tmp/foo" + """) + assert_contains_hash(test_config.expected_hash, output) + assert hashlib.sha1(b"foo").hexdigest().encode('utf-8') in output + assert get_principals("~/.ssh/keybase-signed-key---cert.pub") == set([test_config.subteam + ".ssh.staging", test_config.subteam + ".ssh.root_everywhere"]) + + def test_kssh_errors_on_two_bots(self, test_config): + # Test that kssh does not run if there are multiple bots, no kssh config, and no --bot flag + with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=0): + try: + run_command_with_agent("bin/kssh root@sshd-prod") + assert False + except subprocess.CalledProcessError as e: + assert b"Found 2 config files" in e.output + + def test_kssh_bot_flag(self, test_config): + # Test that kssh works with the --bot flag + with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --bot {test_config.bot_username} -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) + + def test_kssh_set_default_bot(self, test_config): + # Test that kssh works with the --set-default-bot flag + with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + run_command_with_agent(f"bin/kssh --set-default-bot {test_config.bot_username}") + assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) + + def test_kssh_override_default_bot(self, test_config): + # Test that the --bot flag overrides the local config file + with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=1): + run_command_with_agent(f"bin/kssh --set-default-bot otherbotname") + assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --bot {test_config.bot_username} -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) + + def test_kssh_clear_default_bot(self, test_config): + # Test that kssh --clear-default-bot clears the default bot + with simulate_two_teams(test_config), outputs_audit_log(test_config, filename=test_env_1_log_filename, expected_number=0): + run_command_with_agent(f"bin/kssh --set-default-bot {test_config.bot_username}") + run_command_with_agent("bin/kssh --clear-default-bot") + try: + # No default set and no bot specified so it will error out + run_command_with_agent("bin/kssh root@sshd-prod") + assert False + except subprocess.CalledProcessError as e: + assert b"Found 2 config files" in e.output + + def test_keybaseca_backup(self): + # Test the keybaseca backup command by reading and verifying the private key stored in /shared/cakey.backup + run_command("mkdir -p /tmp/ssh/") + run_command("chown -R keybase:keybase /tmp/ssh/") + with open('/shared/cakey.backup') as f: + keyLines = [] + add = False + for line in f: + if "----" in line and "PRIVATE" in line and "BEGIN" in line: + add = True + if add: + keyLines.append(line.strip()) + if "----" in line and "PRIVATE" in line and "END" in line: + add = False + key = '\n'.join(keyLines) + '\n' + with open('/tmp/ssh/cakey', 'w+') as f: + f.write(key) + run_command("chmod 0600 /tmp/ssh/cakey") + output = run_command("ssh-keygen -y -e -f /tmp/ssh/cakey") + assert b"BEGIN SSH2 PUBLIC KEY" in output + + def test_keybaseca_sign(self, test_config): + # Stdout contains a useful message + with open('/shared/keybaseca-sign.out') as f: + assert "Provisioned new certificate" in f.read() + + # SSH with that certificate should just work for every team + assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey user@sshd-prod 'sha1sum /etc/unique'")) + assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey root@sshd-prod 'sha1sum /etc/unique'")) + assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey user@sshd-staging 'sha1sum /etc/unique'")) + assert_contains_hash(test_config.expected_hash, run_command(f"ssh -q -o StrictHostKeyChecking=no -i /shared/userkey root@sshd-prod 'sha1sum /etc/unique'")) + + # Checking that it actually contains the correct principals + assert get_principals("/shared/userkey-cert.pub") == set(test_config.subteams) + + def test_kssh_default_user(self, test_config): + # Set the default user to root + run_command_with_agent("bin/kssh --set-default-user root") + # A normal SSH connection + assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no sshd-prod 'sha1sum /etc/unique'")) + assert b"root" in run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no sshd-prod 'whoami'") + # A proxy jump (relies on the ssh agent) + assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no -J sshd-staging sshd-prod 'sha1sum /etc/unique'")) + # Reset the default user + run_command_with_agent("bin/kssh --clear-default-user") + + def test_kssh_alternate_binary(self, test_config): + # Test it by creating another keybase binary earlier in the path and running kssh. This isn't a perfect test but it is + # enough to smoketest it + run_command("echo '#!/bin/bash' | sudo tee /usr/local/bin/keybase") + run_command("sudo chmod +x /usr/local/bin/keybase") + try: + run_command("bin/kssh --set-keybase-binary /usr/bin/keybase") + assert_contains_hash(test_config.expected_hash, run_command_with_agent("bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging 'sha1sum /etc/unique'")) + run_command("bin/kssh --set-keybase-binary ''") + finally: + run_command("sudo rm /usr/local/bin/keybase") diff --git a/tests/tests/test_env_2_local_audit_log.py b/tests/tests/test_env_2_local_audit_log.py index d98678c..bc3da8e 100644 --- a/tests/tests/test_env_2_local_audit_log.py +++ b/tests/tests/test_env_2_local_audit_log.py @@ -1,17 +1,17 @@ -# import pytest -# -# from lib import TestConfig, load_env, outputs_audit_log, assert_contains_hash, run_command_with_agent -# -# class TestEnv2LocalAuditLog: -# @pytest.fixture(autouse=True, scope='class') -# def configure_env(self): -# assert load_env(__file__) -# -# @pytest.fixture(autouse=True, scope='class') -# def test_config(self): -# return TestConfig.getDefaultTestConfig() -# -# def test_kssh(self, test_config): -# # Test ksshing into staging as user -# with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=1): -# assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging "sha1sum /etc/unique" """)) +import pytest + +from lib import TestConfig, load_env, outputs_audit_log, assert_contains_hash, run_command_with_agent + +class TestEnv2LocalAuditLog: + @pytest.fixture(autouse=True, scope='class') + def configure_env(self): + assert load_env(__file__) + + @pytest.fixture(autouse=True, scope='class') + def test_config(self): + return TestConfig.getDefaultTestConfig() + + def test_kssh(self, test_config): + # Test ksshing into staging as user + with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=1): + assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging "sha1sum /etc/unique" """)) diff --git a/tests/tests/test_env_3_user_not_in_first_team.py b/tests/tests/test_env_3_user_not_in_first_team.py index db25cf8..be8633e 100644 --- a/tests/tests/test_env_3_user_not_in_first_team.py +++ b/tests/tests/test_env_3_user_not_in_first_team.py @@ -1,22 +1,22 @@ -# import pytest -# -# from lib import TestConfig, load_env, outputs_audit_log, clear_keys, outputs_audit_log, assert_contains_hash, run_command_with_agent -# -# class TestEnv3UserNotInFirstTeam: -# @pytest.fixture(autouse=True, scope='class') -# def configure_env(self): -# assert load_env(__file__) -# -# @pytest.fixture(autouse=True, scope='class') -# def test_config(self): -# return TestConfig.getDefaultTestConfig() -# -# def test_kssh(self, test_config): -# # Test ksshing which tests that it is correctly finding a client config -# with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=3): -# clear_keys() -# assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging "sha1sum /etc/unique" """)) -# clear_keys() -# assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-staging "sha1sum /etc/unique" """)) -# clear_keys() -# assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) +import pytest + +from lib import TestConfig, load_env, outputs_audit_log, clear_keys, outputs_audit_log, assert_contains_hash, run_command_with_agent + +class TestEnv3UserNotInFirstTeam: + @pytest.fixture(autouse=True, scope='class') + def configure_env(self): + assert load_env(__file__) + + @pytest.fixture(autouse=True, scope='class') + def test_config(self): + return TestConfig.getDefaultTestConfig() + + def test_kssh(self, test_config): + # Test ksshing which tests that it is correctly finding a client config + with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=3): + clear_keys() + assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no user@sshd-staging "sha1sum /etc/unique" """)) + clear_keys() + assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-staging "sha1sum /etc/unique" """)) + clear_keys() + assert_contains_hash(test_config.expected_hash, run_command_with_agent("""bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod "sha1sum /etc/unique" """)) diff --git a/tests/tests/test_env_4_user_not_in_configured_teams.py b/tests/tests/test_env_4_user_not_in_configured_teams.py index b1714d0..8c0a2eb 100644 --- a/tests/tests/test_env_4_user_not_in_configured_teams.py +++ b/tests/tests/test_env_4_user_not_in_configured_teams.py @@ -1,38 +1,38 @@ -# import json -# import subprocess -# -# import pytest -# -# from lib import TestConfig, load_env, outputs_audit_log, run_command, run_command_with_agent -# -# class TestEnv4UserNotInConfiguredTeams: -# @pytest.fixture(autouse=True, scope='class') -# def configure_env(self): -# assert load_env(__file__) -# -# @pytest.fixture(autouse=True, scope='class') -# def test_config(self): -# return TestConfig.getDefaultTestConfig() -# -# def test_kssh_no_config_files(self, test_config): -# # Test that it can't find any config files -# with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0): -# for s in ['user@sshd-staging', 'root@sshd-staging', 'user@sshd-prod', 'root@sshd-prod']: -# try: -# run_command_with_agent(f"""bin/kssh -q -o StrictHostKeyChecking=no {s} "sha1sum /etc/unique" """) -# assert False -# except subprocess.CalledProcessError as e: -# assert b"Did not find any config files in KBFS" in e.output -# -# def test_kssh_spoofed_config(self, test_config): -# # Test that even when kssh is forced to run by a spoofed config, the CA bot ignores messages that are in the -# # wrong channel -# with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0): -# client_config = json.dumps({'teamname': f"{test_config.subteam}.ssh", "channelname": "", "botname": test_config.bot_username}) -# run_command(f"echo '{client_config}' | keybase fs write /keybase/team/{test_config.subteam}.ssh/kssh-client.config") -# for s in ['user@sshd-staging', 'root@sshd-staging', 'user@sshd-prod', 'root@sshd-prod']: -# try: -# run_command_with_agent(f"""bin/kssh -q -o StrictHostKeyChecking=no {s} "sha1sum /etc/unique" """) -# assert False -# except subprocess.CalledProcessError as e: -# assert b"Failed to get a signed key from the CA: timed out while waiting for a response from the CA" in e.output +import json +import subprocess + +import pytest + +from lib import TestConfig, load_env, outputs_audit_log, run_command, run_command_with_agent + +class TestEnv4UserNotInConfiguredTeams: + @pytest.fixture(autouse=True, scope='class') + def configure_env(self): + assert load_env(__file__) + + @pytest.fixture(autouse=True, scope='class') + def test_config(self): + return TestConfig.getDefaultTestConfig() + + def test_kssh_no_config_files(self, test_config): + # Test that it can't find any config files + with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0): + for s in ['user@sshd-staging', 'root@sshd-staging', 'user@sshd-prod', 'root@sshd-prod']: + try: + run_command_with_agent(f"""bin/kssh -q -o StrictHostKeyChecking=no {s} "sha1sum /etc/unique" """) + assert False + except subprocess.CalledProcessError as e: + assert b"Did not find any config files in KBFS" in e.output + + def test_kssh_spoofed_config(self, test_config): + # Test that even when kssh is forced to run by a spoofed config, the CA bot ignores messages that are in the + # wrong channel + with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0): + client_config = json.dumps({'teamname': f"{test_config.subteam}.ssh", "channelname": "", "botname": test_config.bot_username}) + run_command(f"echo '{client_config}' | keybase fs write /keybase/team/{test_config.subteam}.ssh/kssh-client.config") + for s in ['user@sshd-staging', 'root@sshd-staging', 'user@sshd-prod', 'root@sshd-prod']: + try: + run_command_with_agent(f"""bin/kssh -q -o StrictHostKeyChecking=no {s} "sha1sum /etc/unique" """) + assert False + except subprocess.CalledProcessError as e: + assert b"Failed to get a signed key from the CA: timed out while waiting for a response from the CA" in e.output diff --git a/tests/tests/test_env_5_two_man.py b/tests/tests/test_env_5_two_man.py index 06fbc23..4454526 100644 --- a/tests/tests/test_env_5_two_man.py +++ b/tests/tests/test_env_5_two_man.py @@ -22,3 +22,5 @@ def test_kssh_with_two_man(self, test_config): assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --request-realm {test_config.subteam}.ssh.root_everywhere -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) finally: assert requests.get('http://autoapprover:8080/stop').content == b"OK" + +# TODO: Have the above check for old audit log and new audit log that contains approver info \ No newline at end of file From 1c64f9b263a47460a43b5ed67be6d3d58a02c9d4 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Mon, 30 Sep 2019 10:18:28 -0400 Subject: [PATCH 03/16] Uncomment buildAll.sh --- buildAll.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/buildAll.sh b/buildAll.sh index bee631e..c1da84b 100755 --- a/buildAll.sh +++ b/buildAll.sh @@ -7,9 +7,9 @@ go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/kssh-linux src/cmd/kss go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/keybaseca-linux src/cmd/keybaseca/keybaseca.go # Mac -#GOOS=darwin GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/kssh-mac src/cmd/kssh/kssh.go -#GOOS=darwin GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/keybaseca-mac src/cmd/keybaseca/keybaseca.go -# -## Windows -#GOOS=windows GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/kssh-windows src/cmd/kssh/kssh.go -#GOOS=windows GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/keybaseca-windows src/cmd/keybaseca/keybaseca.go +GOOS=darwin GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/kssh-mac src/cmd/kssh/kssh.go +GOOS=darwin GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/keybaseca-mac src/cmd/keybaseca/keybaseca.go + +# Windows +GOOS=windows GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/kssh-windows src/cmd/kssh/kssh.go +GOOS=windows GOARCH=amd64 go build -ldflags "-X main.VersionNumber=$VERSION" -o bin/keybaseca-windows src/cmd/keybaseca/keybaseca.go From 78e9fe23a01f2014429a41c3c7ecb390f757cdd1 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Wed, 2 Oct 2019 22:57:37 -0400 Subject: [PATCH 04/16] t --- integrationTest.sh | 2 +- src/keybaseca/log/log.go | 4 ++-- tests/docker-compose.yml | 7 ++++--- tests/tests/lib.py | 17 +++++++++++++++-- tests/tests/test_env_5_two_man.py | 29 ++++++++++++++++++++++------- 5 files changed, 44 insertions(+), 15 deletions(-) diff --git a/integrationTest.sh b/integrationTest.sh index e0f0259..e062e72 100755 --- a/integrationTest.sh +++ b/integrationTest.sh @@ -50,7 +50,7 @@ echo "Building containers..." cd ../docker/ && make && cd ../tests/ docker-compose build echo "Running integration tests..." -docker-compose up +docker-compose up -d docker logs kssh -f | indent TEST_EXIT_CODE=`docker wait kssh` diff --git a/src/keybaseca/log/log.go b/src/keybaseca/log/log.go index a35c864..82c46e3 100644 --- a/src/keybaseca/log/log.go +++ b/src/keybaseca/log/log.go @@ -14,10 +14,10 @@ import ( // Log attempts to log the given string to a file. If conf.GetStrictLogging() it will panic if it fails // to log to the file. If conf.GetStrictLogging() is false, it may silently fail func Log(conf config.Config, str string) { - strWithTs := fmt.Sprintf("[%s] %s", time.Now().String(), str) + strWithTs := strings.TrimSpace(fmt.Sprintf("[%s] %s", time.Now().String(), str)) + "\n" if conf.GetLogLocation() == "" { - fmt.Print(strWithTs + "\n") + fmt.Print(strWithTs) } else { err := appendToFile(conf.GetLogLocation(), strWithTs) if err != nil { diff --git a/tests/docker-compose.yml b/tests/docker-compose.yml index 6225985..74a1e94 100644 --- a/tests/docker-compose.yml +++ b/tests/docker-compose.yml @@ -1,6 +1,6 @@ version: '3' services: - # TODO + # The container running the CA bot ca-bot: image: ca-bot container_name: ca-bot @@ -21,7 +21,7 @@ services: depends_on: - sshd-prod - sshd-staging - # TODO + # The test container where kssh is tested via pytest kssh: image: kssh container_name: kssh @@ -67,7 +67,8 @@ services: root_principal: ${SUBTEAM}.ssh.root_everywhere volumes: - app-volume:/shared/ - # TODO + # A container containing a simple python program that can be configured to autonmatically react to all messages + # with a thumbs-up message autoapprover: image: autoapprover container_name: autoapprover diff --git a/tests/tests/lib.py b/tests/tests/lib.py index bd1f565..20d59d9 100644 --- a/tests/tests/lib.py +++ b/tests/tests/lib.py @@ -1,10 +1,11 @@ from contextlib import contextmanager import hashlib import os +import re import signal import subprocess import time -from typing import List, Set +from typing import List, Set, Mapping import requests @@ -116,9 +117,11 @@ def simulate_two_teams(tc: TestConfig): run_command(f"keybase fs rm /keybase/team/{tc.subteam_secondary}/kssh-client.config") @contextmanager -def outputs_audit_log(tc: TestConfig, filename: str, expected_number: int): +def outputs_audit_log(tc: TestConfig, filename: str, expected_number: int, additional_regexes: Mapping[str, int] = None): # A context manager that asserts that the given function triggers expected_number of audit logs to be added to the # log at the given filename + if additional_regexes is None: + additional_regexes = {} # Make a set of the lines in the audit log before we ran before_lines = set(read_file(filename)) @@ -143,6 +146,16 @@ def outputs_audit_log(tc: TestConfig, filename: str, expected_number: int): if cnt != expected_number: assert False, f"Found {cnt} audit log entries, expected {expected_number}! New audit logs: {new_lines}" + additional_regexes_cnt = {k: 0 for k, v in additional_regexes.items()} + for line in new_lines: + line = line.decode('utf-8') + for regex in additional_regexes: + if re.search(regex, line): + additional_regexes_cnt[regex] += 1 + for regex, actual_cnt in additional_regexes_cnt.items(): + if additional_regexes[regex] != actual_cnt: + assert False, f"Found {cnt} audit log entries matching the regex {repr(regex)}. Expected={additional_regexes[regex]}. New audit logs: {new_lines}" + def get_principals(certificateFilename: str) -> Set[str]: inPrincipalsSection = False principalsIndentationLevel = 16 diff --git a/tests/tests/test_env_5_two_man.py b/tests/tests/test_env_5_two_man.py index 4454526..ca55114 100644 --- a/tests/tests/test_env_5_two_man.py +++ b/tests/tests/test_env_5_two_man.py @@ -4,7 +4,15 @@ import pytest import requests -from lib import TestConfig, load_env, assert_contains_hash, run_command_with_agent +from lib import TestConfig, load_env, assert_contains_hash, run_command_with_agent, outputs_audit_log + +from contextlib import contextmanager + +@contextmanager +def autoapprover(): + assert requests.get('http://autoapprover:8080/start').content == b"OK" + yield + assert requests.get('http://autoapprover:8080/stop').content == b"OK" class TestEnv5TwoMan: @pytest.fixture(autouse=True, scope='class') @@ -16,11 +24,18 @@ def test_config(self): return TestConfig.getDefaultTestConfig() def test_kssh_with_two_man(self, test_config): - assert requests.get('http://autoapprover:8080/start').content == b"OK" - - try: + with autoapprover(), outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=2, additional_regexes={f"Two-man SignatureRequest id=.* approved by ": 2}): assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --request-realm {test_config.subteam}.ssh.root_everywhere -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) - finally: - assert requests.get('http://autoapprover:8080/stop').content == b"OK" + assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --request-realm {test_config.subteam}.ssh.root_everywhere -q -o StrictHostKeyChecking=no root@sshd-staging 'sha1sum /etc/unique'")) + + def test_kssh_with_two_man_no_approval(self, test_config): + with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0): + with pytest.raises(subprocess.CalledProcessError): + run_command_with_agent(f"bin/kssh --request-realm {test_config.subteam}.ssh.root_everywhere -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'") -# TODO: Have the above check for old audit log and new audit log that contains approver info \ No newline at end of file + def test_kssh_without_requested_realm(self, test_config): + with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=2): + with pytest.raises(subprocess.CalledProcessError): + run_command_with_agent(f"bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'") + with pytest.raises(subprocess.CalledProcessError): + run_command_with_agent(f"bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'") From 558f44307f2543fff658a8de7d42cc98a0934371 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Thu, 3 Oct 2019 15:04:46 -0400 Subject: [PATCH 05/16] Working decently comprehensive integration tests for two-man realms --- tests/tests/lib.py | 10 ++++++++-- tests/tests/test_env_5_two_man.py | 17 ++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/tests/tests/lib.py b/tests/tests/lib.py index 20d59d9..45b819c 100644 --- a/tests/tests/lib.py +++ b/tests/tests/lib.py @@ -117,11 +117,17 @@ def simulate_two_teams(tc: TestConfig): run_command(f"keybase fs rm /keybase/team/{tc.subteam_secondary}/kssh-client.config") @contextmanager -def outputs_audit_log(tc: TestConfig, filename: str, expected_number: int, additional_regexes: Mapping[str, int] = None): +def outputs_audit_log(tc: TestConfig, filename: str, expected_number: int, additional_regexes: Mapping[str, int] = None, expected_principals: str = None): # A context manager that asserts that the given function triggers expected_number of audit logs to be added to the # log at the given filename if additional_regexes is None: additional_regexes = {} + if expected_principals is None: + expected_principals = f"principals:{tc.subteam}.ssh.staging,{tc.subteam}.ssh.root_everywhere" + elif expected_principals == ".*": + expected_principals = "" + else: + expected_principals = f"principals:{expected_principals}" # Make a set of the lines in the audit log before we ran before_lines = set(read_file(filename)) @@ -140,7 +146,7 @@ def outputs_audit_log(tc: TestConfig, filename: str, expected_number: int, addit cnt = 0 for line in new_lines: line = line.decode('utf-8') - if line and f"Processing SignatureRequest from user={tc.username}" in line and f"principals:{tc.subteam}.ssh.staging,{tc.subteam}.ssh.root_everywhere, expiration:+1h, pubkey:" in line: + if line and f"Processing SignatureRequest from user={tc.username}" in line and f"{expected_principals}, expiration:+1h, pubkey:" in line: cnt += 1 if cnt != expected_number: diff --git a/tests/tests/test_env_5_two_man.py b/tests/tests/test_env_5_two_man.py index ca55114..b7a8fd4 100644 --- a/tests/tests/test_env_5_two_man.py +++ b/tests/tests/test_env_5_two_man.py @@ -4,7 +4,7 @@ import pytest import requests -from lib import TestConfig, load_env, assert_contains_hash, run_command_with_agent, outputs_audit_log +from lib import TestConfig, load_env, assert_contains_hash, run_command_with_agent, outputs_audit_log, clear_keys from contextlib import contextmanager @@ -24,18 +24,25 @@ def test_config(self): return TestConfig.getDefaultTestConfig() def test_kssh_with_two_man(self, test_config): - with autoapprover(), outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=2, additional_regexes={f"Two-man SignatureRequest id=.* approved by ": 2}): + with autoapprover(), \ + outputs_audit_log(test_config, + filename="/shared/ca.log", + expected_number=2, + additional_regexes={f"Two-man SignatureRequest id=.* approved by ": 2}, + expected_principals=f"{test_config.subteam}.ssh.staging,{test_config.subteam}.ssh.root_everywhere"): assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --request-realm {test_config.subteam}.ssh.root_everywhere -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) + clear_keys() assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --request-realm {test_config.subteam}.ssh.root_everywhere -q -o StrictHostKeyChecking=no root@sshd-staging 'sha1sum /etc/unique'")) def test_kssh_with_two_man_no_approval(self, test_config): - with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0): - with pytest.raises(subprocess.CalledProcessError): + with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0, expected_principals=".*"): + with pytest.raises(subprocess.TimeoutExpired): run_command_with_agent(f"bin/kssh --request-realm {test_config.subteam}.ssh.root_everywhere -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'") def test_kssh_without_requested_realm(self, test_config): - with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=2): + with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=2, expected_principals=f"{test_config.subteam}.ssh.staging"): with pytest.raises(subprocess.CalledProcessError): run_command_with_agent(f"bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'") + clear_keys() with pytest.raises(subprocess.CalledProcessError): run_command_with_agent(f"bin/kssh -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'") From 0e0648a1913b278f301dae850409b58589207a4b Mon Sep 17 00:00:00 2001 From: David Dworken Date: Thu, 3 Oct 2019 15:30:35 -0400 Subject: [PATCH 06/16] Updated account creation script to create the auto approval user --- tests/configure_accounts.py | 31 ++++++++++++++++++++----------- tests/docker-compose.yml | 6 +++--- tests/envFiles/test_env_5_two_man | 2 +- tests/tests/autoapprover.py | 4 ++-- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/tests/configure_accounts.py b/tests/configure_accounts.py index aa59646..cfb00cc 100644 --- a/tests/configure_accounts.py +++ b/tests/configure_accounts.py @@ -58,7 +58,7 @@ def invite_to_team(self, team, username): """ assert b"Success!" in self._run_command("team", "add-member", team, "--user=%s" % username, "--role=writer") - def create_team_and_invite(self, team, username): + def create_team_and_invite(self, team, username, *usernames): """ Create a team and invite the given user to it as an admin. Team can be a subteam but the parent teams must exist :param team: A team name. Eg "foo", "foo.bar" @@ -67,6 +67,8 @@ def create_team_and_invite(self, team, username): """ self.create_team(team) self.invite_to_team(team, username) + for u in usernames: + self.invite_to_team(team, u) def create_channel(self, team, channel): """ @@ -86,7 +88,7 @@ def join_channel(self, team, channel): """ assert b"" == self._run_command("chat", "join-channel", team, channel, "--topic-type", "chat") -def write_env_files(kssh_user, ca_user, parent_team): +def write_env_files(kssh_user, ca_user, two_man_approver, parent_team): with open("tests/env.sh", "w+") as f: f.write("#!/bin/bash\n" "# File automatically generated by configure_accounts.py\n" @@ -94,6 +96,8 @@ def write_env_files(kssh_user, ca_user, parent_team): f"export BOT_PAPERKEY='{ca_user.paperkey}'\n" f"export KSSH_USERNAME='{kssh_user.username}'\n" f"export KSSH_PAPERKEY='{kssh_user.paperkey}'\n" + f"export TWO_MAN_APPROVER_USERNAME='{two_man_approver.username}'\n" + f"export TWO_MAN_APPROVER_PAPERKEY='{two_man_approver.paperkey}'\n" f"export SUBTEAM='{parent_team}'\n" f"export SUBTEAM_SECONDARY='{parent_team}.secondary'\n") @@ -127,19 +131,24 @@ def make_user(purpose): if __name__ == '__main__': print("TEST SETUP - We need to do some one-time test setup.") - ca_user = make_user("the CA bot") - kssh_user = make_user("the kssh tester") + # ca_user = make_user("the CA bot") + # kssh_user = make_user("the kssh tester") + # two_man_approver = make_user("the two-man request approver") + ca_user = User('ssh_ca_circle', 'journey sphere cart tiny salt talk athlete gun observe march stock exhaust stand') + kssh_user = User('ssh_kssh_circle', 'man nest merge embody marble exclude first sheriff expire average notice embody joke') + two_man_approver = User('ssh_app_circle', 'heavy knife green tennis useful ranch whale evidence vessel spare open wait series') parent_team = secure_random_str(16) - ca_user.create_team_and_invite(parent_team, kssh_user.username) + ca_user.create_team_and_invite(parent_team, kssh_user.username, two_man_approver.username) - ca_user.create_team_and_invite(parent_team + ".ssh", kssh_user.username) + ca_user.create_team_and_invite(parent_team + ".ssh", kssh_user.username, two_man_approver.username) ca_user.create_channel(parent_team + ".ssh", "ssh-provision") kssh_user.join_channel(parent_team + ".ssh", "ssh-provision") - ca_user.create_team_and_invite(parent_team + ".ssh.staging", kssh_user.username) - ca_user.create_team(parent_team + ".ssh.prod") - ca_user.create_team_and_invite(parent_team + ".ssh.root_everywhere", kssh_user.username) + two_man_approver.join_channel(parent_team + ".ssh", "ssh-provision") + ca_user.create_team_and_invite(parent_team + ".ssh.staging", kssh_user.username, two_man_approver.username) + ca_user.create_team_and_invite(parent_team + ".ssh.prod", two_man_approver.username) + ca_user.create_team_and_invite(parent_team + ".ssh.root_everywhere", kssh_user.username, two_man_approver.username) - ca_user.create_team_and_invite(parent_team + ".secondary", kssh_user.username) + ca_user.create_team_and_invite(parent_team + ".secondary", kssh_user.username, two_man_approver.username) - write_env_files(kssh_user, ca_user, parent_team) + write_env_files(kssh_user, ca_user, two_man_approver, parent_team) diff --git a/tests/docker-compose.yml b/tests/docker-compose.yml index 74a1e94..fa956c1 100644 --- a/tests/docker-compose.yml +++ b/tests/docker-compose.yml @@ -11,7 +11,7 @@ services: - BOT_PAPERKEY - BOT_USERNAME - SUBTEAM - - TESTER_USERNAME + - TWO_MAN_APPROVER_USERNAME volumes: - app-volume:/shared/ user: root @@ -76,8 +76,8 @@ services: context: ./ dockerfile: "Dockerfile-autoapprover" environment: - - TESTER_USERNAME - - TESTER_PAPERKEY + - TWO_MAN_APPROVER_USERNAME + - TWO_MAN_APPROVER_PAPERKEY ports: - 8080 command: "python3.7 autoapprover.py" diff --git a/tests/envFiles/test_env_5_two_man b/tests/envFiles/test_env_5_two_man index beaffdc..c1adbf1 100644 --- a/tests/envFiles/test_env_5_two_man +++ b/tests/envFiles/test_env_5_two_man @@ -6,4 +6,4 @@ export KEYBASE_PAPERKEY="$BOT_PAPERKEY" export KEYBASE_USERNAME="$BOT_USERNAME" export CA_KEY_LOCATION="/shared/keybase-ca-key" export TWO_MAN_TEAMS="$SUBTEAM.ssh.root_everywhere" -export TWO_MAN_APPROVERS="$TESTER_USERNAME" # TODO: Need to make this user +export TWO_MAN_APPROVERS="$TWO_MAN_APPROVER_USERNAME" # TODO: Need to make this user diff --git a/tests/tests/autoapprover.py b/tests/tests/autoapprover.py index bc00e1c..fd49bd7 100644 --- a/tests/tests/autoapprover.py +++ b/tests/tests/autoapprover.py @@ -37,8 +37,8 @@ async def __call__(self, bot, event): shared_running_val = Value('i', 0) def start_bot_event_loop(): - username = os.environ["TESTER_USERNAME"] - paperkey = os.environ["TESTER_PAPERKEY"] + username = os.environ["TWO_MAN_APPROVER_USERNAME"] + paperkey = os.environ["TWO_MAN_APPROVER_PAPERKEY"] bot = Bot( username=username, paperkey=paperkey, handler=Handler(shared_running_val) From 2924e62e33aa25981e8b246952eac1a7e263c26f Mon Sep 17 00:00:00 2001 From: David Dworken Date: Thu, 3 Oct 2019 18:59:56 -0400 Subject: [PATCH 07/16] Add additional validation to the two man config --- src/keybaseca/config/config.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/keybaseca/config/config.go b/src/keybaseca/config/config.go index 6cc6bdb..85a58a4 100644 --- a/src/keybaseca/config/config.go +++ b/src/keybaseca/config/config.go @@ -91,6 +91,15 @@ func ValidateConfig(conf EnvConfig, offline bool) error { if err != nil { return fmt.Errorf("failed to parse NUMBER_REQUIRED_APPROVERS value as a intenger: %v", err) } + for _, twoManTeam := range conf.GetTwoManTeams() { + found := false + for _, team := range conf.GetTeams() { + found = found || team == twoManTeam + } + if !found { + return fmt.Errorf("two man team %s is not in the list of configured teams %v (all two man teams must be listed in the TEAMS environment variable)", twoManTeam, conf.GetTeams()) + } + } } log.Debugf("Validated config: %s", conf.DebugString()) return nil From e132a111993cbc888762aa6c7c8802a179d91d6b Mon Sep 17 00:00:00 2001 From: David Dworken Date: Thu, 3 Oct 2019 19:04:15 -0400 Subject: [PATCH 08/16] Uncomments code and adds docs --- src/keybaseca/bot/bot.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/keybaseca/bot/bot.go b/src/keybaseca/bot/bot.go index f3fdfb5..244145d 100644 --- a/src/keybaseca/bot/bot.go +++ b/src/keybaseca/bot/bot.go @@ -188,6 +188,7 @@ func addApprover(request *OutstandingTwoManSignatureRequest, approver string) bo return false } +// Build an announcement message to be sent in Keybase chat about the sender requesting access to the requested principal func buildTwoManApprovalRequestMessage(conf config.Config, sender string, requestedPrincipal string) string { approvers := []string{} for _, approver := range conf.GetTwoManApprovers() { @@ -198,6 +199,7 @@ func buildTwoManApprovalRequestMessage(conf config.Config, sender string, reques "reply with a thumbs-up to this message. (Configured approvers: %s)", sender, requestedPrincipal, strings.Join(approvers, ", ")) } +// Returns whether the given principal is a principal that requires two man approval func isTwoManPrincipal(conf config.Config, requestedPrincipal string) bool { for _, team := range conf.GetTwoManApprovers() { if team == requestedPrincipal { @@ -220,10 +222,10 @@ func isValidApprover(conf config.Config, senderUsername string, signatureRequest log.Debug("Reply came from someone who isn't a valid two man approver, rejecting!") return false } - //if senderUsername == signatureRequest.Username { - // log.Debug("Reply came from the sender of the signature request, rejecting!") - // return false - //} + if senderUsername == signatureRequest.Username { + log.Debug("Reply came from the sender of the signature request, rejecting!") + return false + } return true } From 8c5bd38b68e049434f151f4573d9c2c6e087e29a Mon Sep 17 00:00:00 2001 From: David Dworken Date: Thu, 3 Oct 2019 19:21:18 -0400 Subject: [PATCH 09/16] Add preliminary docs on two man mode --- docs/env.md | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/docs/env.md b/docs/env.md index e2e928c..b143d54 100644 --- a/docs/env.md +++ b/docs/env.md @@ -106,6 +106,45 @@ export ANNOUNCEMENT="Hello! I'm {USERNAME} and I'm an SSH bot! I'm currently lis export ANNOUNCEMENT="Hello! I'm {USERNAME} and I'm an SSH bot! Being in {CURRENT_TEAM} will grant you SSH access to certain servers. Reach out to @dworken for more information." ``` +## Two Man Realms + +It is possible to configure the SSH CA bot to require approval prior to granting access. For example, one could require +that in order to use kssh with servers in the `team.ssh.root_everywhere` realm two other people must approve the request. +Approvals are done by reacting with a :+1: emoji to the message inside of Keybase chat. + +In order to configure this, there are 3 environment variables that can be used: + +``` +export TWO_MAN_TEAMS="team.ssh.root_everywhere, team.ssh.ci" +export TWO_MAN_APPROVERS="username0, username1, username2" +export TWO_MAN_APPROVER_COUNT="2" +``` + +The above will require that anyone who wants to use kssh with `team.ssh.root_everywhere` or `team.ssh.ci` gets approval +from two other people. Only three people are configured to approve requests: `username0, username1, username2`. + +A few other example configurations are: + +``` +export TWO_MAN_TEAMS="team.ssh.root_everywhere" +export TWO_MAN_APPROVERS="username" +``` + +This would require that in order to use kssh with `team.ssh.root_everywhere` it must be approved by `username`. + +As a user, this would be done by running: + +``` +kssh --request-realm team.ssh.root_everywhere root@production +``` + +For a user, the standard commands would work for all other configured teams: + +``` +kssh dev@staging +kssh dev@prod +``` + ## Developer Options These environment variables are mainly useful for dev work. For security reasons, it is recommended always to run a From 1c35692975b0d1bbb69aca0f5235554d5f73def7 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Thu, 3 Oct 2019 22:12:37 -0400 Subject: [PATCH 10/16] Refactored keybase docker installation to a single Dockerfile and renamed environment variable --- docker/Dockerfile-ca | 31 ++----------------------------- docker/Dockerfile-keybase | 27 +++++++++++++++++++++++++++ docker/Makefile | 1 + integrationTest.sh | 3 ++- src/keybaseca/config/config.go | 4 ++-- src/kssh/bot.go | 1 + tests/Dockerfile-autoapprover | 30 ++---------------------------- tests/Dockerfile-kssh | 30 ++---------------------------- tests/configure_accounts.py | 2 -- tests/envFiles/test_env_5_two_man | 2 +- 10 files changed, 40 insertions(+), 91 deletions(-) create mode 100644 docker/Dockerfile-keybase diff --git a/docker/Dockerfile-ca b/docker/Dockerfile-ca index 749fcb5..e448f18 100644 --- a/docker/Dockerfile-ca +++ b/docker/Dockerfile-ca @@ -1,32 +1,5 @@ -# This dockerfile builds a container capable of running the SSH CA bot. Note that a lot of this code is duplicated -# between this file and Dockerfile-kssh. -FROM ubuntu:18.04 - -# Dependencies -RUN apt-get -qq update -RUN apt-get -qq install curl software-properties-common ca-certificates gnupg -y -RUN useradd -ms /bin/bash keybase -USER keybase -WORKDIR /home/keybase - -# Download and verify the deb -# Key fingerprint from https://keybase.io/docs/server_security/our_code_signing_key -RUN curl --remote-name https://prerelease.keybase.io/keybase_amd64.deb -RUN curl --remote-name https://prerelease.keybase.io/keybase_amd64.deb.sig -# Import our gpg key from our website. Pulling from key servers caused a flakey build so -# we get the key from the Keybase website instead. -RUN curl -sSL https://keybase.io/docs/server_security/code_signing_key.asc | gpg --import -# This line will error if the fingerprint of the key in the file does not match the -# known fingerprint of the our PGP key -RUN gpg --fingerprint 222B85B0F90BE2D24CFEB93F47484E50656D16C7 -# And then verify the signature now that we have the key -RUN gpg --verify keybase_amd64.deb.sig keybase_amd64.deb - -# Silence the error from dpkg about failing to configure keybase since `apt-get install -f` fixes it -USER root -RUN dpkg -i keybase_amd64.deb || true -RUN apt-get install -fy -USER keybase +# See docker/Dockerfile-keybase +FROM keybase:latest # Install go USER root diff --git a/docker/Dockerfile-keybase b/docker/Dockerfile-keybase new file mode 100644 index 0000000..63dc68f --- /dev/null +++ b/docker/Dockerfile-keybase @@ -0,0 +1,27 @@ +# This dockerfile builds an Ubuntu container with Keybase installed +FROM ubuntu:18.04 + +# Dependencies +RUN apt-get -qq update +RUN apt-get -qq install curl software-properties-common ca-certificates gnupg -y +RUN useradd -ms /bin/bash keybase +USER keybase +WORKDIR /home/keybase + +# Download and verify the deb +RUN curl --remote-name https://prerelease.keybase.io/keybase_amd64.deb +RUN curl --remote-name https://prerelease.keybase.io/keybase_amd64.deb.sig +# Import our gpg key from our website. Pulling from key servers caused a flakey build so +# we get the key from the Keybase website instead. +RUN curl -sSL https://keybase.io/docs/server_security/code_signing_key.asc | gpg --import +# This line will error if the fingerprint of the key in the file does not match the +# known fingerprint of the our PGP key +RUN gpg --fingerprint 222B85B0F90BE2D24CFEB93F47484E50656D16C7 +# And then verify the signature now that we have the key +RUN gpg --verify keybase_amd64.deb.sig keybase_amd64.deb + +# Silence the error from dpkg about failing to configure keybase since `apt-get install -f` fixes it +USER root +RUN dpkg -i keybase_amd64.deb || true +RUN apt-get install -fy +USER keybase diff --git a/docker/Makefile b/docker/Makefile index 28d169d..1695dd1 100644 --- a/docker/Makefile +++ b/docker/Makefile @@ -11,6 +11,7 @@ SHELL := /bin/bash # Build a new docker image for the CA bot build: reset-permissions + docker build -t keybase -f Dockerfile-keybase . docker build -t ca -f Dockerfile-ca .. # Generate a new CA key diff --git a/integrationTest.sh b/integrationTest.sh index e062e72..29fdf50 100755 --- a/integrationTest.sh +++ b/integrationTest.sh @@ -47,7 +47,8 @@ cd tests/ reset_docker echo "Building containers..." -cd ../docker/ && make && cd ../tests/ +cd ../docker/ && make build && cd ../tests/ +pwd docker-compose build echo "Running integration tests..." docker-compose up -d diff --git a/src/keybaseca/config/config.go b/src/keybaseca/config/config.go index 85a58a4..0fe4bea 100644 --- a/src/keybaseca/config/config.go +++ b/src/keybaseca/config/config.go @@ -89,7 +89,7 @@ func ValidateConfig(conf EnvConfig, offline bool) error { } _, err := conf.getNumberRequiredApprovers() if err != nil { - return fmt.Errorf("failed to parse NUMBER_REQUIRED_APPROVERS value as a intenger: %v", err) + return fmt.Errorf("failed to parse TWO_MAN_APPROVER_COUNT value as a intenger: %v", err) } for _, twoManTeam := range conf.GetTwoManTeams() { found := false @@ -307,7 +307,7 @@ func (ef *EnvConfig) GetTwoManApprovers() []string { } func (ef *EnvConfig) getNumberRequiredApprovers() (int, error) { - val := os.Getenv("NUMBER_REQUIRED_APPROVERS") + val := os.Getenv("TWO_MAN_APPROVER_COUNT") if val == "" { return 1, nil } diff --git a/src/kssh/bot.go b/src/kssh/bot.go index 432b276..f57369c 100644 --- a/src/kssh/bot.go +++ b/src/kssh/bot.go @@ -71,6 +71,7 @@ func GetSignedKey(config ConfigFile, request shared.SignatureRequest) (shared.Si if time.Since(startTime) > timeout { return empty, fmt.Errorf("timed out while waiting for a response from the CA") } + // TODO: Add feedback here to people that they need to wait for approval msg, err := sub.Read() if err != nil { return empty, fmt.Errorf("failed to read message: %v", err) diff --git a/tests/Dockerfile-autoapprover b/tests/Dockerfile-autoapprover index bb3fa0a..faa5a3e 100644 --- a/tests/Dockerfile-autoapprover +++ b/tests/Dockerfile-autoapprover @@ -1,31 +1,5 @@ -# This dockerfile builds a container capable of running kssh. Note that a lot of this code is duplicated -# between this file and Dockerfile-ca. # TODO: be less shitty -FROM ubuntu:18.04 - -# Dependencies -RUN apt-get -qq update -RUN apt-get -qq install curl software-properties-common ca-certificates gnupg -y -RUN useradd -ms /bin/bash keybase -USER keybase -WORKDIR /home/keybase - -# Download and verify the deb -RUN curl --remote-name https://prerelease.keybase.io/keybase_amd64.deb -RUN curl --remote-name https://prerelease.keybase.io/keybase_amd64.deb.sig -# Import our gpg key from our website. Pulling from key servers caused a flakey build so -# we get the key from the Keybase website instead. -RUN curl -sSL https://keybase.io/docs/server_security/code_signing_key.asc | gpg --import -# This line will error if the fingerprint of the key in the file does not match the -# known fingerprint of the our PGP key -RUN gpg --fingerprint 222B85B0F90BE2D24CFEB93F47484E50656D16C7 -# And then verify the signature now that we have the key -RUN gpg --verify keybase_amd64.deb.sig keybase_amd64.deb - -# Silence the error from dpkg about failing to configure keybase since `apt-get install -f` fixes it -USER root -RUN dpkg -i keybase_amd64.deb || true -RUN apt-get install -fy -USER keybase +# See docker/Dockerfile-keybase +FROM keybase:latest # Install python USER root diff --git a/tests/Dockerfile-kssh b/tests/Dockerfile-kssh index 41a6493..9609f9a 100644 --- a/tests/Dockerfile-kssh +++ b/tests/Dockerfile-kssh @@ -1,31 +1,5 @@ -# This dockerfile builds a container capable of running kssh. Note that a lot of this code is duplicated -# between this file and Dockerfile-ca. -FROM ubuntu:18.04 - -# Dependencies -RUN apt-get -qq update -RUN apt-get -qq install curl software-properties-common ca-certificates gnupg -y -RUN useradd -ms /bin/bash keybase -USER keybase -WORKDIR /home/keybase - -# Download and verify the deb -RUN curl --remote-name https://prerelease.keybase.io/keybase_amd64.deb -RUN curl --remote-name https://prerelease.keybase.io/keybase_amd64.deb.sig -# Import our gpg key from our website. Pulling from key servers caused a flakey build so -# we get the key from the Keybase website instead. -RUN curl -sSL https://keybase.io/docs/server_security/code_signing_key.asc | gpg --import -# This line will error if the fingerprint of the key in the file does not match the -# known fingerprint of the our PGP key -RUN gpg --fingerprint 222B85B0F90BE2D24CFEB93F47484E50656D16C7 -# And then verify the signature now that we have the key -RUN gpg --verify keybase_amd64.deb.sig keybase_amd64.deb - -# Silence the error from dpkg about failing to configure keybase since `apt-get install -f` fixes it -USER root -RUN dpkg -i keybase_amd64.deb || true -RUN apt-get install -fy -USER keybase +# See docker/Dockerfile-keybase +FROM keybase:latest # Install go USER root diff --git a/tests/configure_accounts.py b/tests/configure_accounts.py index cfb00cc..843b884 100644 --- a/tests/configure_accounts.py +++ b/tests/configure_accounts.py @@ -127,8 +127,6 @@ def make_user(purpose): paperkey = input("TEST SETUP - What is the paper key: ") return User(username, paperkey) -# TODO: Make testing user - if __name__ == '__main__': print("TEST SETUP - We need to do some one-time test setup.") # ca_user = make_user("the CA bot") diff --git a/tests/envFiles/test_env_5_two_man b/tests/envFiles/test_env_5_two_man index c1adbf1..af1a04a 100644 --- a/tests/envFiles/test_env_5_two_man +++ b/tests/envFiles/test_env_5_two_man @@ -6,4 +6,4 @@ export KEYBASE_PAPERKEY="$BOT_PAPERKEY" export KEYBASE_USERNAME="$BOT_USERNAME" export CA_KEY_LOCATION="/shared/keybase-ca-key" export TWO_MAN_TEAMS="$SUBTEAM.ssh.root_everywhere" -export TWO_MAN_APPROVERS="$TWO_MAN_APPROVER_USERNAME" # TODO: Need to make this user +export TWO_MAN_APPROVERS="$TWO_MAN_APPROVER_USERNAME" From c3866898b4b85b319aa472e2a1b29ff25ca0f714 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Thu, 3 Oct 2019 22:29:11 -0400 Subject: [PATCH 11/16] Fix golangci-lint errors --- src/keybaseca/bot/bot.go | 12 ++++++------ src/keybaseca/config/config.go | 10 +++++++++- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/keybaseca/bot/bot.go b/src/keybaseca/bot/bot.go index 244145d..bbbbd7b 100644 --- a/src/keybaseca/bot/bot.go +++ b/src/keybaseca/bot/bot.go @@ -230,29 +230,29 @@ func isValidApprover(conf config.Config, senderUsername string, signatureRequest } // Respond to the given SignatureRequest and log any errors that are produced. This function does not return any error. -func respondToSignatureRequest(conf config.Config, kbc *kbchat.API, signatureRequest shared.SignatureRequest, Username string, MessageID chat1.MessageID, conversationID string) { +func respondToSignatureRequest(conf config.Config, kbc *kbchat.API, signatureRequest shared.SignatureRequest, username string, messageID chat1.MessageID, conversationID string) { signatureResponse, err := sshutils.ProcessSignatureRequest(conf, signatureRequest) if err != nil { - LogError(conf, kbc, Username, MessageID, conversationID, err) + LogError(conf, kbc, username, messageID, conversationID, err) return } response, err := json.Marshal(signatureResponse) if err != nil { - LogError(conf, kbc, Username, MessageID, conversationID, err) + LogError(conf, kbc, username, messageID, conversationID, err) return } _, err = kbc.SendMessageByConvID(conversationID, shared.SignatureResponsePreamble+string(response)) if err != nil { - LogError(conf, kbc, Username, MessageID, conversationID, err) + LogError(conf, kbc, username, messageID, conversationID, err) return } } // Log the given error to Keybase chat and to the configured log file. Used so that the chatbot does not crash // due to an error caused by a malformed message. -func LogError(conf config.Config, kbc *kbchat.API, Username string, MessageID chat1.MessageID, conversationID string, err error) { - message := fmt.Sprintf("Encountered error while processing message from %s (messageID:%d): %v", Username, MessageID, err) +func LogError(conf config.Config, kbc *kbchat.API, username string, messageID chat1.MessageID, conversationID string, err error) { + message := fmt.Sprintf("Encountered error while processing message from %s (messageID:%d): %v", username, messageID, err) auditlog.Log(conf, message) _, e := kbc.SendMessageByConvID(conversationID, message) if e != nil { diff --git a/src/keybaseca/config/config.go b/src/keybaseca/config/config.go index 0fe4bea..e40534f 100644 --- a/src/keybaseca/config/config.go +++ b/src/keybaseca/config/config.go @@ -80,6 +80,15 @@ func ValidateConfig(conf EnvConfig, offline bool) error { } } } + err := validateTwoManConfig(conf) + if err != nil { + return err + } + log.Debugf("Validated config: %s", conf.DebugString()) + return nil +} + +func validateTwoManConfig(conf EnvConfig) error { if len(conf.GetTwoManTeams()) != 0 || len(conf.GetTwoManApprovers()) != 0 { if len(conf.GetTwoManTeams()) != 0 && len(conf.GetTwoManApprovers()) == 0 { return fmt.Errorf("cannot specify TWO_MAN_TEAMS without setting TWO_MAN_APPROVERS") @@ -101,7 +110,6 @@ func ValidateConfig(conf EnvConfig, offline bool) error { } } } - log.Debugf("Validated config: %s", conf.DebugString()) return nil } From 7c069679721e5522d21b30b9d2d037e352b0863e Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 19 Oct 2019 22:19:00 -0400 Subject: [PATCH 12/16] Update log string and variable names --- src/keybaseca/bot/bot.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/keybaseca/bot/bot.go b/src/keybaseca/bot/bot.go index bbbbd7b..cef6065 100644 --- a/src/keybaseca/bot/bot.go +++ b/src/keybaseca/bot/bot.go @@ -78,17 +78,16 @@ func StartBot(conf config.Config) error { if msg.Message.Content.TypeName == "reaction" { emoji := msg.Message.Content.Reaction.Body - responseTo := msg.Message.Content.Reaction.MessageID + reactionTo := msg.Message.Content.Reaction.MessageID log.Debug("Examining reaction...") for _, outstanding := range outstandingTwoManRequests { - if outstanding.RequestMessageID == responseTo { - log.Debug("Message is a reply to an outstanding two-man request") + if outstanding.RequestMessageID == reactionTo { + log.Debug("Message is a reaction to an outstanding two-man request") approver := msg.Message.Sender.Username if emoji == ":+1:" && isValidApprover(conf, approver, outstanding.SignatureRequest) { isDuplicateApprover := addApprover(&outstanding, approver) if isDuplicateApprover { - // TODO: Test this code log.Debugf("Rejecting duplicate approver %s since they already approved the two-man request with ID=%s", approver, outstanding.SignatureRequest.UUID) continue } From 5d915e6427f76634663baef4df78768c1a42ad20 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 19 Oct 2019 22:39:54 -0400 Subject: [PATCH 13/16] Add feedback to let people know kssh is still running and to be patient --- src/kssh/bot.go | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/kssh/bot.go b/src/kssh/bot.go index f57369c..273aa83 100644 --- a/src/kssh/bot.go +++ b/src/kssh/bot.go @@ -20,6 +20,7 @@ func GetSignedKey(config ConfigFile, request shared.SignatureRequest) (shared.Si if request.RequestedPrincipal != "" { timeout = time.Hour logrus.Debug("Requesting a two-man enabled certificate. Setting time out to one hour...") + fmt.Println("Requesting a two-man enabled SSH certificate. See Keybase Chat to find an approver. ") } // Start communicating with the Keybase chat API @@ -39,19 +40,23 @@ func GetSignedKey(config ConfigFile, request shared.SignatureRequest) (shared.Si return empty, fmt.Errorf("error subscribing to messages: %v", err) } + terminateAllCh := make(chan struct{}) + defer close(terminateAllCh) + // If we just send our signature request to chat, we hit a race condition where if the CA responds fast enough // we will miss the response from the CA. We fix this with a simple ACKing algorithm: // 1. Send an AckRequest every 100ms until an Ack is received. // 2. Once an Ack is received, we know we are correctly receiving messages // 3. Send the signature request payload and get back a signed cert // We implement this with a terminatable goroutine that just sends acks and a while(true) loop that looks for responses - terminateRoutineCh := make(chan interface{}) + terminateAckRequestCh := make(chan interface{}) go func() { // Make the AckRequests send less often over time by tracking how many we've sent numberSent := 0 for { select { - case <-terminateRoutineCh: + case <-terminateAckRequestCh: + case <-terminateAllCh: return default: @@ -65,13 +70,28 @@ func GetSignedKey(config ConfigFile, request shared.SignatureRequest) (shared.Si } }() + // A goroutine that simply prints a dot every 500ms until terminated. Used to show progress to users while they + // wait for a response from the bot. + go func() { + for { + select { + case <-terminateAllCh: + return + default: + + } + + fmt.Print(".") + time.Sleep(500 * time.Millisecond) + } + }() + hasBeenAcked := false startTime := time.Now() for { if time.Since(startTime) > timeout { return empty, fmt.Errorf("timed out while waiting for a response from the CA") } - // TODO: Add feedback here to people that they need to wait for approval msg, err := sub.Read() if err != nil { return empty, fmt.Errorf("failed to read message: %v", err) @@ -90,7 +110,7 @@ func GetSignedKey(config ConfigFile, request shared.SignatureRequest) (shared.Si if shared.IsAckResponse(messageBody) && !hasBeenAcked { // We got an Ack so we terminate our AckRequests and send the real payload hasBeenAcked = true - terminateRoutineCh <- true + terminateAckRequestCh <- true marshaledRequest, err := json.Marshal(request) if err != nil { return empty, err From c9724fa0980d89efb046729f2afd517282593363 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 19 Oct 2019 22:41:49 -0400 Subject: [PATCH 14/16] Always run apt-get install on the same line as apt-get update Fixes an issue where the docker build cache will cache the update but not the install which can lead to a cache containing invalid entries and thus failed builds. --- docker/Dockerfile-ca | 3 +-- docker/Dockerfile-keybase | 3 +-- tests/Dockerfile-autoapprover | 3 +-- tests/Dockerfile-cabot | 2 +- tests/Dockerfile-kssh | 3 +-- 5 files changed, 5 insertions(+), 9 deletions(-) diff --git a/docker/Dockerfile-ca b/docker/Dockerfile-ca index e448f18..406a420 100644 --- a/docker/Dockerfile-ca +++ b/docker/Dockerfile-ca @@ -4,8 +4,7 @@ FROM keybase:latest # Install go USER root RUN add-apt-repository ppa:gophers/archive -y -RUN apt-get update -RUN apt-get install golang-1.11-go git sudo -y +RUN apt-get update && apt-get install golang-1.11-go git sudo -y USER keybase # Install go dependencies (speeds up future builds) diff --git a/docker/Dockerfile-keybase b/docker/Dockerfile-keybase index 63dc68f..8101259 100644 --- a/docker/Dockerfile-keybase +++ b/docker/Dockerfile-keybase @@ -2,8 +2,7 @@ FROM ubuntu:18.04 # Dependencies -RUN apt-get -qq update -RUN apt-get -qq install curl software-properties-common ca-certificates gnupg -y +RUN apt-get -qq update && apt-get -qq install curl software-properties-common ca-certificates gnupg -y RUN useradd -ms /bin/bash keybase USER keybase WORKDIR /home/keybase diff --git a/tests/Dockerfile-autoapprover b/tests/Dockerfile-autoapprover index faa5a3e..10eb305 100644 --- a/tests/Dockerfile-autoapprover +++ b/tests/Dockerfile-autoapprover @@ -3,8 +3,7 @@ FROM keybase:latest # Install python USER root -RUN apt-get update -RUN apt-get install python3.7 python3-pip gettext -y +RUN apt-get update && apt-get install python3.7 python3-pip gettext -y RUN python3.7 -m pip install pykeybasebot flask USER keybase diff --git a/tests/Dockerfile-cabot b/tests/Dockerfile-cabot index d7f328d..7e7c181 100644 --- a/tests/Dockerfile-cabot +++ b/tests/Dockerfile-cabot @@ -2,5 +2,5 @@ FROM ca:latest USER root -RUN apt-get install python3 python3-pip gettext -y +RUN apt-get update && apt-get install python3 python3-pip gettext -y RUN pip3 install flask diff --git a/tests/Dockerfile-kssh b/tests/Dockerfile-kssh index 9609f9a..b424f6e 100644 --- a/tests/Dockerfile-kssh +++ b/tests/Dockerfile-kssh @@ -4,8 +4,7 @@ FROM keybase:latest # Install go USER root RUN add-apt-repository ppa:gophers/archive -y -RUN apt-get update -y -RUN apt-get install golang-1.11-go git sudo python3 python3-pip sudo -y +RUN apt-get update -y && apt-get install golang-1.11-go git sudo python3 python3-pip sudo -y RUN pip3 install pytest requests # Make it so the keybase user has passwordless sudo so it can move the keybase binary around From 14bfca800219aa6711dd704c90ffdcbe77e8f128 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 19 Oct 2019 22:49:01 -0400 Subject: [PATCH 15/16] Cleanup code to open the PR --- src/keybaseca/bot/bot.go | 1 + tests/configure_accounts.py | 9 +++------ tests/docker-compose.yml | 2 +- tests/tests/autoapprover.py | 11 ++++++----- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/keybaseca/bot/bot.go b/src/keybaseca/bot/bot.go index cef6065..1a84608 100644 --- a/src/keybaseca/bot/bot.go +++ b/src/keybaseca/bot/bot.go @@ -38,6 +38,7 @@ func GetUsername(conf config.Config) (string, error) { return username, nil } +// A struct used for the bot to keep track of any outstanding two man requests and the data associated with the requests type OutstandingTwoManSignatureRequest struct { SignatureRequest shared.SignatureRequest RequestMessageID chat1.MessageID diff --git a/tests/configure_accounts.py b/tests/configure_accounts.py index 843b884..ebcfcfe 100644 --- a/tests/configure_accounts.py +++ b/tests/configure_accounts.py @@ -129,12 +129,9 @@ def make_user(purpose): if __name__ == '__main__': print("TEST SETUP - We need to do some one-time test setup.") - # ca_user = make_user("the CA bot") - # kssh_user = make_user("the kssh tester") - # two_man_approver = make_user("the two-man request approver") - ca_user = User('ssh_ca_circle', 'journey sphere cart tiny salt talk athlete gun observe march stock exhaust stand') - kssh_user = User('ssh_kssh_circle', 'man nest merge embody marble exclude first sheriff expire average notice embody joke') - two_man_approver = User('ssh_app_circle', 'heavy knife green tennis useful ranch whale evidence vessel spare open wait series') + ca_user = make_user("the CA bot") + kssh_user = make_user("the kssh tester") + two_man_approver = make_user("the two-man request approver") parent_team = secure_random_str(16) ca_user.create_team_and_invite(parent_team, kssh_user.username, two_man_approver.username) diff --git a/tests/docker-compose.yml b/tests/docker-compose.yml index fa956c1..a7e8dea 100644 --- a/tests/docker-compose.yml +++ b/tests/docker-compose.yml @@ -68,7 +68,7 @@ services: volumes: - app-volume:/shared/ # A container containing a simple python program that can be configured to autonmatically react to all messages - # with a thumbs-up message + # with a thumbs-up message. Used to test two-man mode. autoapprover: image: autoapprover container_name: autoapprover diff --git a/tests/tests/autoapprover.py b/tests/tests/autoapprover.py index fd49bd7..78bbbdd 100644 --- a/tests/tests/autoapprover.py +++ b/tests/tests/autoapprover.py @@ -3,7 +3,6 @@ import asyncio import logging import os -import sys import time from multiprocessing import Process, Value @@ -17,13 +16,14 @@ class Handler: + """ + A Keybase chatbot handler that reacts to two-man requests with a thumbs up + """ def __init__(self, shared_running_val: Value): self.shared_running_val = shared_running_val async def __call__(self, bot, event): - print("HANDLER CALLED") if self.shared_running_val.value: - print("RUNNING") if event.msg.content.type_name != chat1.MessageTypeStrings.TEXT.value: return channel = event.msg.channel @@ -31,12 +31,13 @@ async def __call__(self, bot, event): body = event.msg.content.text.body if "has requested access to the two-man realm" in body: await bot.chat.react(channel, msg_id, ":+1:") - else: - print("NOT RUNNING") +# A shared boolean flag that tracks whether the auto-reacter is currently running shared_running_val = Value('i', 0) def start_bot_event_loop(): + # Start the bot running in a separate process so that it doesn't block the main process that hosts the flask + # webserver username = os.environ["TWO_MAN_APPROVER_USERNAME"] paperkey = os.environ["TWO_MAN_APPROVER_PAPERKEY"] bot = Bot( From 11d3e073d22f83ed6938a2e3e09b014a56b0664b Mon Sep 17 00:00:00 2001 From: David Dworken Date: Wed, 30 Oct 2019 10:08:59 -0400 Subject: [PATCH 16/16] Rename two man rule to M of N control policy --- docs/env.md | 12 ++-- src/cmd/kssh/kssh.go | 2 +- src/keybaseca/bot/bot.go | 56 +++++++++---------- src/keybaseca/config/config.go | 54 +++++++++--------- src/keybaseca/sshutils/sshutils.go | 24 ++++---- src/kssh/bot.go | 4 +- tests/configure_accounts.py | 24 ++++---- tests/docker-compose.yml | 8 +-- .../{test_env_5_two_man => test_env_5_mofn} | 6 +- tests/tests/autoapprover.py | 8 +-- ...st_env_5_two_man.py => test_env_5_mofn.py} | 8 +-- 11 files changed, 103 insertions(+), 103 deletions(-) rename tests/envFiles/{test_env_5_two_man => test_env_5_mofn} (55%) rename tests/tests/{test_env_5_two_man.py => test_env_5_mofn.py} (90%) diff --git a/docs/env.md b/docs/env.md index b143d54..884c134 100644 --- a/docs/env.md +++ b/docs/env.md @@ -106,7 +106,7 @@ export ANNOUNCEMENT="Hello! I'm {USERNAME} and I'm an SSH bot! I'm currently lis export ANNOUNCEMENT="Hello! I'm {USERNAME} and I'm an SSH bot! Being in {CURRENT_TEAM} will grant you SSH access to certain servers. Reach out to @dworken for more information." ``` -## Two Man Realms +## M of N Realms It is possible to configure the SSH CA bot to require approval prior to granting access. For example, one could require that in order to use kssh with servers in the `team.ssh.root_everywhere` realm two other people must approve the request. @@ -115,9 +115,9 @@ Approvals are done by reacting with a :+1: emoji to the message inside of Keybas In order to configure this, there are 3 environment variables that can be used: ``` -export TWO_MAN_TEAMS="team.ssh.root_everywhere, team.ssh.ci" -export TWO_MAN_APPROVERS="username0, username1, username2" -export TWO_MAN_APPROVER_COUNT="2" +export CTRL_POLICY_MOFN_TEAMS="team.ssh.root_everywhere, team.ssh.ci" +export CTRL_POLICY_MOFN_APPROVERS="username0, username1, username2" +export CTRL_POLICY_MOFN_REQUIRED_COUNT="2" ``` The above will require that anyone who wants to use kssh with `team.ssh.root_everywhere` or `team.ssh.ci` gets approval @@ -126,8 +126,8 @@ from two other people. Only three people are configured to approve requests: `us A few other example configurations are: ``` -export TWO_MAN_TEAMS="team.ssh.root_everywhere" -export TWO_MAN_APPROVERS="username" +export CTRL_POLICY_MOFN_TEAMS="team.ssh.root_everywhere" +export CTRL_POLICY_MOFN_APPROVERS="username" ``` This would require that in order to use kssh with `team.ssh.root_everywhere` it must be approved by `username`. diff --git a/src/cmd/kssh/kssh.go b/src/cmd/kssh/kssh.go index fb65784..c4c8bbc 100644 --- a/src/cmd/kssh/kssh.go +++ b/src/cmd/kssh/kssh.go @@ -133,7 +133,7 @@ GLOBAL OPTIONS: a default SSH user --clear-default-user Clear the default SSH user --set-keybase-binary Advanced feature: Run kssh with a specific keybase binary rather than resolving via $PATH - --request-realm Advanced feature: Request a specific two-man realm in your provisioned certificate `, VersionNumber) + --request-realm Advanced feature: Request a specific M of N enabled realm in your provisioned certificate `, VersionNumber) } type Action int diff --git a/src/keybaseca/bot/bot.go b/src/keybaseca/bot/bot.go index 1a84608..aa3dcb1 100644 --- a/src/keybaseca/bot/bot.go +++ b/src/keybaseca/bot/bot.go @@ -38,8 +38,8 @@ func GetUsername(conf config.Config) (string, error) { return username, nil } -// A struct used for the bot to keep track of any outstanding two man requests and the data associated with the requests -type OutstandingTwoManSignatureRequest struct { +// A struct used for the bot to keep track of any outstanding M of N requests and the data associated with the requests +type OutstandingMOfNSignatureRequest struct { SignatureRequest shared.SignatureRequest RequestMessageID chat1.MessageID Approvers []string @@ -48,8 +48,8 @@ type OutstandingTwoManSignatureRequest struct { // Start the keybaseca bot in an infinite loop. Does not return unless it encounters an unrecoverable error. func StartBot(conf config.Config) error { - // Initialize a list for the outstanding two-man signature requests - outstandingTwoManRequests := []OutstandingTwoManSignatureRequest{} + // Initialize a list for the outstanding M of N signature requests + outstandingMOfNRequests := []OutstandingMOfNSignatureRequest{} kbc, err := GetKBChat(conf) if err != nil { @@ -82,24 +82,24 @@ func StartBot(conf config.Config) error { reactionTo := msg.Message.Content.Reaction.MessageID log.Debug("Examining reaction...") - for _, outstanding := range outstandingTwoManRequests { + for _, outstanding := range outstandingMOfNRequests { if outstanding.RequestMessageID == reactionTo { - log.Debug("Message is a reaction to an outstanding two-man request") + log.Debug("Message is a reaction to an outstanding M of N request") approver := msg.Message.Sender.Username if emoji == ":+1:" && isValidApprover(conf, approver, outstanding.SignatureRequest) { isDuplicateApprover := addApprover(&outstanding, approver) if isDuplicateApprover { - log.Debugf("Rejecting duplicate approver %s since they already approved the two-man request with ID=%s", approver, outstanding.SignatureRequest.UUID) + log.Debugf("Rejecting duplicate approver %s since they already approved the M of N request with ID=%s", approver, outstanding.SignatureRequest.UUID) continue } log.WithField("requester", outstanding.SignatureRequest.Username). WithField("current_approver", approver). WithField("all_approvers", outstanding.Approvers). Debugf("Message approved request") - threshold := conf.GetNumberRequiredApprovers() + threshold := conf.GetMOfNRequiredApproversCount() if len(outstanding.Approvers) >= threshold { respondToSignatureRequest(conf, kbc, outstanding.SignatureRequest, outstanding.SignatureRequest.Username, outstanding.RequestMessageID, outstanding.ConvID) - auditlog.Log(conf, fmt.Sprintf("Two-man SignatureRequest id=%s approved by %v", outstanding.SignatureRequest.UUID, outstanding.Approvers)) + auditlog.Log(conf, fmt.Sprintf("M of N SignatureRequest id=%s approved by %v", outstanding.SignatureRequest.UUID, outstanding.Approvers)) } } else { log.Debug("Message did not approve request") @@ -107,7 +107,7 @@ func StartBot(conf config.Config) error { continue } } - log.Debug("Ignoring reaction since it is not a reaction on an outstanding two-man request") + log.Debug("Ignoring reaction since it is not a reaction on an outstanding M of N request") continue } @@ -149,24 +149,24 @@ func StartBot(conf config.Config) error { signatureRequest.Username = msg.Message.Sender.Username signatureRequest.DeviceName = msg.Message.Sender.DeviceName - // Process the signature request depending on whether they requested two-man only principals or not + // Process the signature request depending on whether they requested M of N enabled principals or not if signatureRequest.RequestedPrincipal == "" { // If they didn't request a principal just respond immediately respondToSignatureRequest(conf, kbc, signatureRequest, msg.Message.Sender.Username, msg.Message.Id, msg.Message.ConvID) } else { - if isTwoManPrincipal(conf, signatureRequest.RequestedPrincipal) { - // If they requested a principal that doesn't require two-man authorization, respond immediately + if isMOfNPrincipal(conf, signatureRequest.RequestedPrincipal) { + // If they requested a principal that doesn't require M of N authorization, respond immediately respondToSignatureRequest(conf, kbc, signatureRequest, msg.Message.Sender.Username, msg.Message.Id, msg.Message.ConvID) } else { - // If the principal requires two-man authorization, treat it as such - resp, err := kbc.SendMessageByConvID(msg.Message.ConvID, buildTwoManApprovalRequestMessage(conf, msg.Message.Sender.Username, signatureRequest.RequestedPrincipal)) + // If the principal requires M of N authorization, treat it as such + resp, err := kbc.SendMessageByConvID(msg.Message.ConvID, buildMOfNApprovalRequestMessage(conf, msg.Message.Sender.Username, signatureRequest.RequestedPrincipal)) if err != nil { LogError(conf, kbc, msg.Message.Sender.Username, msg.Message.Id, msg.Message.ConvID, err) continue } - outstandingTwoManRequests = append(outstandingTwoManRequests, - OutstandingTwoManSignatureRequest{SignatureRequest: signatureRequest, Approvers: []string{}, RequestMessageID: *resp.Result.MessageID, ConvID: msg.Message.ConvID}) + outstandingMOfNRequests = append(outstandingMOfNRequests, + OutstandingMOfNSignatureRequest{SignatureRequest: signatureRequest, Approvers: []string{}, RequestMessageID: *resp.Result.MessageID, ConvID: msg.Message.ConvID}) } } } else { @@ -175,10 +175,10 @@ func StartBot(conf config.Config) error { } } -// Add the given approver to the list of approvers in the given outstanding two man request if the +// Add the given approver to the list of approvers in the given outstanding m of n signature request if the // given user has not already approved the request. Returns whether the given approver has already // approved the request. -func addApprover(request *OutstandingTwoManSignatureRequest, approver string) bool { +func addApprover(request *OutstandingMOfNSignatureRequest, approver string) bool { for _, curApprover := range request.Approvers { if curApprover == approver { return true @@ -189,19 +189,19 @@ func addApprover(request *OutstandingTwoManSignatureRequest, approver string) bo } // Build an announcement message to be sent in Keybase chat about the sender requesting access to the requested principal -func buildTwoManApprovalRequestMessage(conf config.Config, sender string, requestedPrincipal string) string { +func buildMOfNApprovalRequestMessage(conf config.Config, sender string, requestedPrincipal string) string { approvers := []string{} - for _, approver := range conf.GetTwoManApprovers() { + for _, approver := range conf.GetMOfNApprovers() { approvers = append(approvers, "@"+approver) } - return fmt.Sprintf("@%s has requested access to the two-man realm %s! In order to approve this access, "+ + return fmt.Sprintf("@%s has requested access to the M of N realm %s! In order to approve this access, "+ "reply with a thumbs-up to this message. (Configured approvers: %s)", sender, requestedPrincipal, strings.Join(approvers, ", ")) } -// Returns whether the given principal is a principal that requires two man approval -func isTwoManPrincipal(conf config.Config, requestedPrincipal string) bool { - for _, team := range conf.GetTwoManApprovers() { +// Returns whether the given principal is a principal that requires M of N approval +func isMOfNPrincipal(conf config.Config, requestedPrincipal string) bool { + for _, team := range conf.GetMOfNApprovers() { if team == requestedPrincipal { return true } @@ -209,17 +209,17 @@ func isTwoManPrincipal(conf config.Config, requestedPrincipal string) bool { return false } -// Note that this function is a key security barrier for the two-man feature. This function checks that only people +// Note that this function is a key security barrier for the M of N feature. This function checks that only people // in the define list of approvers can approve a request and that people cannot approve their own request. func isValidApprover(conf config.Config, senderUsername string, signatureRequest shared.SignatureRequest) bool { validApprover := false - for _, knownApprover := range conf.GetTwoManApprovers() { + for _, knownApprover := range conf.GetMOfNApprovers() { if knownApprover == senderUsername { validApprover = true } } if !validApprover { - log.Debug("Reply came from someone who isn't a valid two man approver, rejecting!") + log.Debug("Reply came from someone who isn't a valid M of N approver, rejecting!") return false } if senderUsername == signatureRequest.Username { diff --git a/src/keybaseca/config/config.go b/src/keybaseca/config/config.go index e40534f..29bbd25 100644 --- a/src/keybaseca/config/config.go +++ b/src/keybaseca/config/config.go @@ -30,9 +30,9 @@ type Config interface { GetStrictLogging() bool GetAnnouncement() string DebugString() string - GetTwoManTeams() []string - GetTwoManApprovers() []string - GetNumberRequiredApprovers() int + GetMOfNTeams() []string + GetMOfNApprovers() []string + GetMOfNRequiredApproversCount() int } // Validate the given config file. If offline, do so without connecting to keybase (used in code that is meant @@ -80,7 +80,7 @@ func ValidateConfig(conf EnvConfig, offline bool) error { } } } - err := validateTwoManConfig(conf) + err := validateMOfNConfig(conf) if err != nil { return err } @@ -88,25 +88,25 @@ func ValidateConfig(conf EnvConfig, offline bool) error { return nil } -func validateTwoManConfig(conf EnvConfig) error { - if len(conf.GetTwoManTeams()) != 0 || len(conf.GetTwoManApprovers()) != 0 { - if len(conf.GetTwoManTeams()) != 0 && len(conf.GetTwoManApprovers()) == 0 { - return fmt.Errorf("cannot specify TWO_MAN_TEAMS without setting TWO_MAN_APPROVERS") +func validateMOfNConfig(conf EnvConfig) error { + if len(conf.GetMOfNTeams()) != 0 || len(conf.GetMOfNApprovers()) != 0 { + if len(conf.GetMOfNTeams()) != 0 && len(conf.GetMOfNApprovers()) == 0 { + return fmt.Errorf("cannot specify CTRL_POLICY_MOFN_TEAMS without setting CTRL_POLICY_MOFN_APPROVERS") } - if len(conf.GetTwoManTeams()) == 0 && len(conf.GetTwoManApprovers()) != 0 { - return fmt.Errorf("cannot specify TWO_MAN_APPROVERS without setting TWO_MAN_TEAMS") + if len(conf.GetMOfNTeams()) == 0 && len(conf.GetMOfNApprovers()) != 0 { + return fmt.Errorf("cannot specify CTRL_POLICY_MOFN_APPROVERS without setting CTRL_POLICY_MOFN_TEAMS") } - _, err := conf.getNumberRequiredApprovers() + _, err := conf.getMOfNRequiredApproversCount() if err != nil { - return fmt.Errorf("failed to parse TWO_MAN_APPROVER_COUNT value as a intenger: %v", err) + return fmt.Errorf("failed to parse CTRL_POLICY_MOFN_REQUIRED_COUNT value as a intenger: %v", err) } - for _, twoManTeam := range conf.GetTwoManTeams() { + for _, mofnTeam := range conf.GetMOfNTeams() { found := false for _, team := range conf.GetTeams() { - found = found || team == twoManTeam + found = found || team == mofnTeam } if !found { - return fmt.Errorf("two man team %s is not in the list of configured teams %v (all two man teams must be listed in the TEAMS environment variable)", twoManTeam, conf.GetTeams()) + return fmt.Errorf("M of N configured team %s is not in the list of configured teams %v (all M of N teams must be listed in the TEAMS environment variable)", mofnTeam, conf.GetTeams()) } } } @@ -298,32 +298,32 @@ func splitTeamChannel(teamChannel string) (string, string, error) { return split[0], split[1], nil } -func (ef *EnvConfig) GetTwoManTeams() []string { - twoManTeams := os.Getenv("TWO_MAN_TEAMS") - if twoManTeams == "" { +func (ef *EnvConfig) GetMOfNTeams() []string { + mofnTeams := os.Getenv("CTRL_POLICY_MOFN_TEAMS") + if mofnTeams == "" { return []string{} } - return commaSeparatedStringToList(twoManTeams) + return commaSeparatedStringToList(mofnTeams) } -func (ef *EnvConfig) GetTwoManApprovers() []string { - twoManApprovers := os.Getenv("TWO_MAN_APPROVERS") - if twoManApprovers == "" { +func (ef *EnvConfig) GetMOfNApprovers() []string { + mofnApprovers := os.Getenv("CTRL_POLICY_MOFN_APPROVERS") + if mofnApprovers == "" { return []string{} } - return commaSeparatedStringToList(twoManApprovers) + return commaSeparatedStringToList(mofnApprovers) } -func (ef *EnvConfig) getNumberRequiredApprovers() (int, error) { - val := os.Getenv("TWO_MAN_APPROVER_COUNT") +func (ef *EnvConfig) getMOfNRequiredApproversCount() (int, error) { + val := os.Getenv("CTRL_POLICY_MOFN_REQUIRED_COUNT") if val == "" { return 1, nil } return strconv.Atoi(val) } -func (ef *EnvConfig) GetNumberRequiredApprovers() int { - num, _ := ef.getNumberRequiredApprovers() +func (ef *EnvConfig) GetMOfNRequiredApproversCount() int { + num, _ := ef.getMOfNRequiredApproversCount() return num } diff --git a/src/keybaseca/sshutils/sshutils.go b/src/keybaseca/sshutils/sshutils.go index 6f3f8c9..7911b00 100644 --- a/src/keybaseca/sshutils/sshutils.go +++ b/src/keybaseca/sshutils/sshutils.go @@ -175,33 +175,33 @@ func getPrincipals(conf config.Config, sr shared.SignatureRequest) (string, erro } } - // Map from a team to whether or not it is a two-man only team - // Note that this is a key security barrier in the two-man feature. This ensures that signature requests that do - // not specify a principal are not given any two-man required principals. - teamToTwoManRequired := make(map[string]bool) + // Map from a team to whether or not it is an M of N enabled team + // Note that this is a key security barrier in the M of N feature. This ensures that signature requests that do + // not specify a principal are not given any M of N enabled principals. + teamToMOfNRequired := make(map[string]bool) for _, team := range conf.GetTeams() { - teamToTwoManRequired[team] = false + teamToMOfNRequired[team] = false } - for _, team := range conf.GetTwoManTeams() { - teamToTwoManRequired[team] = true + for _, team := range conf.GetMOfNTeams() { + teamToMOfNRequired[team] = true } // Iterate through the teams in the config file and use the subteam as the principal - // if the user is in that subteam and the subteam doesn't require the two-man rule + // if the user is in that subteam and the subteam doesn't require M of N approval var principals []string for _, team := range conf.GetTeams() { isMember, ok1 := teamToMembership[team] - requiresTwoMan, ok2 := teamToTwoManRequired[team] - if ok1 && isMember && ok2 && !requiresTwoMan { + requiresMOfNApproval, ok2 := teamToMOfNRequired[team] + if ok1 && isMember && ok2 && !requiresMOfNApproval { principals = append(principals, team) } } // Add the specific principals that they requested. Note that getPrincipals() is only called if the signature - // request has been validated and the two-man request been approved. + // request has been validated and the M of N request been approved. requestedTeam := sr.RequestedPrincipal isMember, ok := teamToMembership[requestedTeam] - _, isConfiguredTeam := teamToTwoManRequired[requestedTeam] + _, isConfiguredTeam := teamToMOfNRequired[requestedTeam] if ok && isMember && isConfiguredTeam { principals = append(principals, requestedTeam) } diff --git a/src/kssh/bot.go b/src/kssh/bot.go index 273aa83..d5d62db 100644 --- a/src/kssh/bot.go +++ b/src/kssh/bot.go @@ -19,8 +19,8 @@ func GetSignedKey(config ConfigFile, request shared.SignatureRequest) (shared.Si timeout := 5 * time.Second if request.RequestedPrincipal != "" { timeout = time.Hour - logrus.Debug("Requesting a two-man enabled certificate. Setting time out to one hour...") - fmt.Println("Requesting a two-man enabled SSH certificate. See Keybase Chat to find an approver. ") + logrus.Debug("Requesting an M of N enabled certificate. Setting time out to one hour...") + fmt.Println("Requesting an M of N enabled SSH certificate. See Keybase Chat to find an approver. ") } // Start communicating with the Keybase chat API diff --git a/tests/configure_accounts.py b/tests/configure_accounts.py index ebcfcfe..edeb064 100644 --- a/tests/configure_accounts.py +++ b/tests/configure_accounts.py @@ -88,7 +88,7 @@ def join_channel(self, team, channel): """ assert b"" == self._run_command("chat", "join-channel", team, channel, "--topic-type", "chat") -def write_env_files(kssh_user, ca_user, two_man_approver, parent_team): +def write_env_files(kssh_user, ca_user, mofn_approver, parent_team): with open("tests/env.sh", "w+") as f: f.write("#!/bin/bash\n" "# File automatically generated by configure_accounts.py\n" @@ -96,8 +96,8 @@ def write_env_files(kssh_user, ca_user, two_man_approver, parent_team): f"export BOT_PAPERKEY='{ca_user.paperkey}'\n" f"export KSSH_USERNAME='{kssh_user.username}'\n" f"export KSSH_PAPERKEY='{kssh_user.paperkey}'\n" - f"export TWO_MAN_APPROVER_USERNAME='{two_man_approver.username}'\n" - f"export TWO_MAN_APPROVER_PAPERKEY='{two_man_approver.paperkey}'\n" + f"export MOFN_APPROVER_USERNAME='{mofn_approver.username}'\n" + f"export MOFN_APPROVER_PAPERKEY='{mofn_approver.paperkey}'\n" f"export SUBTEAM='{parent_team}'\n" f"export SUBTEAM_SECONDARY='{parent_team}.secondary'\n") @@ -131,19 +131,19 @@ def make_user(purpose): print("TEST SETUP - We need to do some one-time test setup.") ca_user = make_user("the CA bot") kssh_user = make_user("the kssh tester") - two_man_approver = make_user("the two-man request approver") + mofn_approver = make_user("the M of N request approver") parent_team = secure_random_str(16) - ca_user.create_team_and_invite(parent_team, kssh_user.username, two_man_approver.username) + ca_user.create_team_and_invite(parent_team, kssh_user.username, mofn_approver.username) - ca_user.create_team_and_invite(parent_team + ".ssh", kssh_user.username, two_man_approver.username) + ca_user.create_team_and_invite(parent_team + ".ssh", kssh_user.username, mofn_approver.username) ca_user.create_channel(parent_team + ".ssh", "ssh-provision") kssh_user.join_channel(parent_team + ".ssh", "ssh-provision") - two_man_approver.join_channel(parent_team + ".ssh", "ssh-provision") - ca_user.create_team_and_invite(parent_team + ".ssh.staging", kssh_user.username, two_man_approver.username) - ca_user.create_team_and_invite(parent_team + ".ssh.prod", two_man_approver.username) - ca_user.create_team_and_invite(parent_team + ".ssh.root_everywhere", kssh_user.username, two_man_approver.username) + mofn_approver.join_channel(parent_team + ".ssh", "ssh-provision") + ca_user.create_team_and_invite(parent_team + ".ssh.staging", kssh_user.username, mofn_approver.username) + ca_user.create_team_and_invite(parent_team + ".ssh.prod", mofn_approver.username) + ca_user.create_team_and_invite(parent_team + ".ssh.root_everywhere", kssh_user.username, mofn_approver.username) - ca_user.create_team_and_invite(parent_team + ".secondary", kssh_user.username, two_man_approver.username) + ca_user.create_team_and_invite(parent_team + ".secondary", kssh_user.username, mofn_approver.username) - write_env_files(kssh_user, ca_user, two_man_approver, parent_team) + write_env_files(kssh_user, ca_user, mofn_approver, parent_team) diff --git a/tests/docker-compose.yml b/tests/docker-compose.yml index a7e8dea..0b86b21 100644 --- a/tests/docker-compose.yml +++ b/tests/docker-compose.yml @@ -11,7 +11,7 @@ services: - BOT_PAPERKEY - BOT_USERNAME - SUBTEAM - - TWO_MAN_APPROVER_USERNAME + - MOFN_APPROVER_USERNAME volumes: - app-volume:/shared/ user: root @@ -68,7 +68,7 @@ services: volumes: - app-volume:/shared/ # A container containing a simple python program that can be configured to autonmatically react to all messages - # with a thumbs-up message. Used to test two-man mode. + # with a thumbs-up message. Used to test the M of N approval feature. autoapprover: image: autoapprover container_name: autoapprover @@ -76,8 +76,8 @@ services: context: ./ dockerfile: "Dockerfile-autoapprover" environment: - - TWO_MAN_APPROVER_USERNAME - - TWO_MAN_APPROVER_PAPERKEY + - MOFN_APPROVER_USERNAME + - MOFN_APPROVER_PAPERKEY ports: - 8080 command: "python3.7 autoapprover.py" diff --git a/tests/envFiles/test_env_5_two_man b/tests/envFiles/test_env_5_mofn similarity index 55% rename from tests/envFiles/test_env_5_two_man rename to tests/envFiles/test_env_5_mofn index af1a04a..7448cab 100644 --- a/tests/envFiles/test_env_5_two_man +++ b/tests/envFiles/test_env_5_mofn @@ -1,9 +1,9 @@ -# Used to test the two man functionality whereby an approval is required for certain realms of servers +# Used to test the M of N functionality whereby an approval is required for certain realms of servers export KEY_EXPIRATION="+1h" export LOG_LOCATION="/shared/ca.log" export TEAMS="$SUBTEAM.ssh.staging,$SUBTEAM.ssh.prod,$SUBTEAM.ssh.root_everywhere" export KEYBASE_PAPERKEY="$BOT_PAPERKEY" export KEYBASE_USERNAME="$BOT_USERNAME" export CA_KEY_LOCATION="/shared/keybase-ca-key" -export TWO_MAN_TEAMS="$SUBTEAM.ssh.root_everywhere" -export TWO_MAN_APPROVERS="$TWO_MAN_APPROVER_USERNAME" +export CTRL_POLICY_MOFN_TEAMS="$SUBTEAM.ssh.root_everywhere" +export CTRL_POLICY_MOFN_APPROVERS="$MOFN_APPROVER_USERNAME" diff --git a/tests/tests/autoapprover.py b/tests/tests/autoapprover.py index 78bbbdd..148ef0c 100644 --- a/tests/tests/autoapprover.py +++ b/tests/tests/autoapprover.py @@ -17,7 +17,7 @@ class Handler: """ - A Keybase chatbot handler that reacts to two-man requests with a thumbs up + A Keybase chatbot handler that reacts to M of N requests with a thumbs up """ def __init__(self, shared_running_val: Value): self.shared_running_val = shared_running_val @@ -29,7 +29,7 @@ async def __call__(self, bot, event): channel = event.msg.channel msg_id = event.msg.id body = event.msg.content.text.body - if "has requested access to the two-man realm" in body: + if "has requested access to the M of N realm" in body: await bot.chat.react(channel, msg_id, ":+1:") # A shared boolean flag that tracks whether the auto-reacter is currently running @@ -38,8 +38,8 @@ async def __call__(self, bot, event): def start_bot_event_loop(): # Start the bot running in a separate process so that it doesn't block the main process that hosts the flask # webserver - username = os.environ["TWO_MAN_APPROVER_USERNAME"] - paperkey = os.environ["TWO_MAN_APPROVER_PAPERKEY"] + username = os.environ["MOFN_APPROVER_USERNAME"] + paperkey = os.environ["MOFN_APPROVER_PAPERKEY"] bot = Bot( username=username, paperkey=paperkey, handler=Handler(shared_running_val) diff --git a/tests/tests/test_env_5_two_man.py b/tests/tests/test_env_5_mofn.py similarity index 90% rename from tests/tests/test_env_5_two_man.py rename to tests/tests/test_env_5_mofn.py index b7a8fd4..5a9b2b1 100644 --- a/tests/tests/test_env_5_two_man.py +++ b/tests/tests/test_env_5_mofn.py @@ -14,7 +14,7 @@ def autoapprover(): yield assert requests.get('http://autoapprover:8080/stop').content == b"OK" -class TestEnv5TwoMan: +class TestEnv5MOfN: @pytest.fixture(autouse=True, scope='class') def configure_env(self): assert load_env(__file__) @@ -23,18 +23,18 @@ def configure_env(self): def test_config(self): return TestConfig.getDefaultTestConfig() - def test_kssh_with_two_man(self, test_config): + def test_kssh_with_requested_realm(self, test_config): with autoapprover(), \ outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=2, - additional_regexes={f"Two-man SignatureRequest id=.* approved by ": 2}, + additional_regexes={f"M of N SignatureRequest id=.* approved by ": 2}, expected_principals=f"{test_config.subteam}.ssh.staging,{test_config.subteam}.ssh.root_everywhere"): assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --request-realm {test_config.subteam}.ssh.root_everywhere -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")) clear_keys() assert_contains_hash(test_config.expected_hash, run_command_with_agent(f"bin/kssh --request-realm {test_config.subteam}.ssh.root_everywhere -q -o StrictHostKeyChecking=no root@sshd-staging 'sha1sum /etc/unique'")) - def test_kssh_with_two_man_no_approval(self, test_config): + def test_kssh_with_requested_realm_no_approval(self, test_config): with outputs_audit_log(test_config, filename="/shared/ca.log", expected_number=0, expected_principals=".*"): with pytest.raises(subprocess.TimeoutExpired): run_command_with_agent(f"bin/kssh --request-realm {test_config.subteam}.ssh.root_everywhere -q -o StrictHostKeyChecking=no root@sshd-prod 'sha1sum /etc/unique'")