Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Feature: Two Man Realms #68

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

New Feature: Two Man Realms #68

wants to merge 16 commits into from

Conversation

ddworken
Copy link
Contributor

@ddworken ddworken commented Oct 3, 2019

This PR adds a new feature to the SSH CA project: The ability to define realms of servers that are only available with approval from someone else on your team. An example config for this feature would be:

export TEAMS="team.ssh.staging,team.ssh.prod,team.ssh.root_everywhere"
export TWO_MAN_TEAMS="team.ssh.root_everywhere"
export TWO_MAN_APPROVERS="dworken, username1, username2" 

Approval is done via reacting with a 👍 to a message posted by the bot. All approvers are logged to the SSH CA audit log.

An example interaction for this mode looks like:

image

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)
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.
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.
@ddworken ddworken marked this pull request as ready for review October 20, 2019 02:49
@ddworken ddworken requested a review from xgess October 20, 2019 04:35
docs/env.md Outdated
```
export TWO_MAN_TEAMS="team.ssh.root_everywhere, team.ssh.ci"
export TWO_MAN_APPROVERS="username0, username1, username2"
export TWO_MAN_APPROVER_COUNT="2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TWO_MAN_ is a little confusing since the number of approvers is variable. it sounds to me like this is an M of N control policy. can we call it something like

CTRL_POLICY_1_MOFN_TEAMS="team.ssh.root_everywhere, team.ssh.ci"
CTRL_POLICY_1_M=2
CTRL_POLICY_1_N="username0, username1, username2"

is it possible we could want multiple control policies managed by a single bot? if so, this might need to be more complex. that's why i shoved a 1 in there. maybe not necessary right now. i dunno.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! I think for now it isn't necessary to have the 1 in there since that seems like a pretty advanced feature and probably beyond the scope of this project.

As a user, this would be done by running:

```
kssh --request-realm team.ssh.root_everywhere root@production
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is kind of a mouthful. any chance it can work more like:

> kssh root@production
ERROR: awaiting approval in team.ssh.root_everywhere
...
> kssh root@production

OR

> kssh root@production
ERROR: you need to request approval first like: `kssh --request root@production`
> kssh --request root@production
...
> kssh root@production

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, there isn't really a way to make that work :/

In your first example, there is no way for kssh to know which servers need which principals. This becomes especially hard when talking about ssh configs which can define custom names like "production" for a hostname.

The second example runs into essentially the same issue. While kssh could attempt to go first and then alert the user if the connection fails, there is no way for kssh --request root@production to know which principals to request.

A feature that could be built out is allowing the user to configure this sort of thing on the client side. Eg:

kssh root@production   # permission denied
kssh --configure-realm team.ssh.root_everywhere root@production  # configures kssh to always request that principal when accessing the given host
kssh root@production  # works 

The difficulty with the local config strategy is that it will break in weird ways when using proxy commands or any more advanced SSH features. Eg kssh -J root@production david@other. kssh wouldn't see the root@production connection and wouldn't know to provision a certificate with that principal. So IMO it doesn't seem worth it to build this feature given how brittle it would end up being, but certainly open to it if you think it would still be useful.

Copy link
Contributor

@xgess xgess Nov 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems really sub-optimal to me to push the onus of knowing how everything is set-up down to the kssh user.

In your first example, there is no way for kssh to know which servers need which principals

but doesn't the bot know? or if not, can we make it know with additional environment variables?
it looks like the message requesting approval is coming from the kssh user. can it come from the bot instead? this will make it much easier for an operations team to iterate on how they want this thing to work for them without having to push an update to all kssh users, which i imagine would be much more difficult.

@ddworken ddworken requested a review from xgess October 30, 2019 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants