Storing and deleting public keys in .ssh/authorized_keys

[Louis Bertrand]

Hello Mads,

For issue 1, removing the leftover SSH public keys, thank you.

For issue 2, simpler is gooder 😊 Just leave out the comment entirely as you do now. The only reason we noticed was because we were inspecting the file to confirm that the SSH key was removed when a user account was deleted.

Thanks
 --Louis


-----Original Message-----
From: Mads Kiilerich <mads at kiilerich.com>
Sent: July 30, 2020 5:44 PM
To: Louis Bertrand <Louis.Bertrand at durhamcollege.ca>; Kallithea <kallithea-general at sfconservancy.org>
Subject: Re: Storing and deleting public keys in .ssh/authorized_keys

[EXTERNAL EMAIL]

On 7/30/20 3:03 PM, Louis Bertrand wrote:
> Hi,
> While reviewing the installation of Kallithea on a test server, one of
> our college Unix admins pointed out two issues with the way SSH public
> keys are saved in .ssh/authorized_keys
>
> 1) When a user is deleted from the Kallithea system, the public key is not removed from the file. The only thing stopping access through SSH is when Kallithea does not find an active user and denies the request. It seems to me that removing the public key would greatly reduce the processing done before access is refused.


Yes - that seems like an oversight. We will fix that.


> 2) When a user submits a public key through the Web interface, the comment at the end of the key line is not copied into the authorized_keys file. The comment should be retained to help manually manage the file, or at least identify the users at a glance. Yes, I agree that the file is managed by the system, but sometimes you need to look at it.


If I remember correctly, it was a deliberate design decision to not include anything user controlled (except the base64 encoded public key) in the authorized_keys file - just to make it 100% obvious that no user in any way could inject anything that could have unintended side effects (or add noise that could confuse the sysadmin). Since some systems might have self registration, we don't even put usernames in there.

I guess we could define a (somewhat arbitrary) safe subset of comment characters and a max length ... but it would be a bit arbitrary and of limited value.

We could perhaps also put the username in the file. Would that help enough for your use case?

Any thoughts on how we should balance the concerns?


> If you'd like, I can enter the issue in bitbucket, but at the moment it seems to be undergoing maintenance.


I guess that's bitbucket's way of saying that they no longer support Mercurial. We should remove all bitbucket references from our documentation. For now, it is fine (and preferred) to use mails to the mailing list for bug reports.

/Mads

________________________________

________________________________
This message is intended only for the named recipients. This message may contain information that is confidential or exempt from disclosure under applicable law. Any dissemination or copying of this message by anyone other than a named recipient is strictly prohibited. If you are not a named recipient or an employee or agent responsible for delivering this message to a named recipient, please notify us immediately, and permanently destroy this message and any copies you may have. Warning: Email may not be secure unless properly encrypted.