keybase / bot-sshca

A chat bot that can manage your team's SSH accounts
BSD 3-Clause "New" or "Revised" License
222 stars 30 forks source link

AckRequest in wrong channel #86

Closed krezreb closed 4 years ago

krezreb commented 4 years ago

I've followed the procedure to use keybase for ssh auth. https://keybase-ssh-ca-bot.readthedocs.io/en/latest/getting_started.html

It works great! Except that often my colleagues have issues with their AckRequests ending up in the wrong channel.

Our organisation has these channels, the impacted users are members of all

In certain cases, the AckRequests end up in weatherforce.dev, not weatherforce.ssh.dev, so the certbot does not see them. If I kick the impacted user out of weatherforce.dev and they retry to provision, the AckRequests go to the correct channel. So it seems that there's a bug in the regex / filtering that kssh uses to determine where to send the AckRequest.

Impacted users are in Linux, with these versions of keybase

keybase version 5.3.1-20200320154633+3e235215b3 keybase version 5.3.0-20200310205642+4f2689009b

One user is not impacted, he is using MacOS:

keybase version 5.3.0-20200310172631+4f2689009b  All users are using the current version of kssh, 1.1.0-5558800

mmou commented 4 years ago

Weird!

When a user does kssh, kssh searches through the user's KBFS team folders for kssh config files. As a sanity check, can you make sure that there is no kssh config file (probably called kssh-client.config) in /keybase/team/weatherforce.dev?

krezreb commented 4 years ago

Weird!

When a user does kssh, kssh searches through the user's KBFS team folders for kssh config files. As a sanity check, can you make sure that there is no kssh config file (probably called kssh-client.config) in /keybase/team/weatherforce.dev?

Ah, well that is a good point. Initially we used weatherforce.dev for kssh, but then we added the other environments and so changed the groups. There is indeed a kssh-client.config present in that folder.

However, I still have doubts about how this was implemented. Shouldn't kssh have a mechanism by which it can determine, based on the server it's trying to contact, which channel it needs to send the ack to? Or at the least, it could send the acks to all channels for which there's a kssh-client.config, rather than the first one it finds?

mmou commented 4 years ago

Yeah as written, a keybaseca bot can be configured to listen on multiple teams, while kssh is expecting that each keybaseca bot is listening to only one team. cc-ing @ddworken to confirm this, but it seems like we should modify kssh to not expect this: e.g. kssh LoadConfigs() shouldn't dedup by bot name, and GetTeamFromBot should be GetTeamsFromBot.

mmou commented 4 years ago

It also sounds like when you changed the config and restarted keybaseca, the old config files didn't get deleted, so I'll also file a ticket to investigate captureControlCToDeleteClientConfig and try to reproduce the error.

krezreb commented 4 years ago

Yeah as written, a keybaseca bot can be configured to listen on multiple teams, while kssh is expecting that each keybaseca bot is listening to only one team. cc-ing @ddworken to confirm this, but it seems like we should modify kssh to not expect this: e.g. kssh LoadConfigs() shouldn't dedup by bot name, and GetTeamFromBot should be GetTeamsFromBot.

Indeed, being able to have a single bot for all channels is what I wanted to be able to do, this allows for easier configuration and fewer computing resources to be dedicated to singing certs

krezreb commented 4 years ago

It also sounds like when you changed the config and restarted keybaseca, the old config files didn't get deleted, so I'll also file a ticket to investigate captureControlCToDeleteClientConfig and try to reproduce the error.

I can confirm that this is the case

mmou commented 4 years ago

Yeah as written, a keybaseca bot can be configured to listen on multiple teams, while kssh is expecting that each keybaseca bot is listening to only one team. cc-ing @ddworken to confirm this, but it seems like we should modify kssh to not expect this: e.g. kssh LoadConfigs() shouldn't dedup by bot name, and GetTeamFromBot should be GetTeamsFromBot.

Indeed, being able to have a single bot for all channels is what I wanted to be able to do, this allows for easier configuration and fewer computing resources to be dedicated to singing certs

Actually after spending a bit more time in the code, I think something you can do if you wanted to consolidate all of the ssh chat messages in one channel, is set the CHAT_CHANNEL env var for each server that is using your keybaseca bot. See https://github.com/keybase/bot-sshca/blob/master/docs/env.md#chat_channel. In your case you could do something like export CHAT_CHANNEL="weatherforce.dev#ssh-provision" (assuming all devs are in weatherforce.dev.

Consolidating ssh chat messages is just "nice to have", and I wanted to clarify that there is no issue with how kssh currently determines which channel to communicate. To answer your questions here:

However, I still have doubts about how this was implemented. Shouldn't kssh have a mechanism by which it can determine, based on the server it's trying to contact, which channel it needs to send the ack to? Or at the least, it could send the acks to all channels for which there's a kssh-client.config, rather than the first one it finds?

Logically, it is fine that kssh acks any one team#channel that is associated with a given sshca bot, instead of acking all team#channels associated with a given bot. First of all, the bot is listening to all of the teams that the kssh found config files for (except in your case because the old config file wasn't deleted :P that's next on my list to investigate). Then, the certificate that a SSH CA issues contains a list of principals: the sshca bot sets the list of principals to be the list of all the Keybase teams that the requesting user is a member of that it knows about. And the servers are configured to grant access based on the principals. More about principals here: https://github.com/keybase/bot-sshca/blob/master/docs/sshca.md#how-an-ssh-ca-works.

krezreb commented 4 years ago

Thanks @mmou for the feedback, that clarifies a lot of things for me. I will read up on principals :)

mmou commented 4 years ago

It also sounds like when you changed the config and restarted keybaseca, the old config files didn't get deleted, so I'll also file a ticket to investigate captureControlCToDeleteClientConfig and try to reproduce the error.

I can confirm that this is the case

I was able to reproduce this and just merged #96, which fixed the issue for me. If you get a chance to try it out, lmk if you have any feedback! And thanks again for reporting your issues and your quick responses!