adding ssh public keys with lf in it

[Mads Kiilerich]

(Note: please post to kallithea-general at sfconservancy.org - not to the 
-bounces address.)


Thanks for the report and patch.

Can you clarify exactly which problem you saw? Did you get a "nice" 
"Incorrect SSH key - failed to decode base64 part" message when adding 
the key? A correct and correctly shown message ... but not helpful? And 
you missed the error message and thought the key had been added 
successfully?


Some comments to the proposed fix:

Note that public keys can be added both by admin 
kallithea/controllers/admin/users.py and by users 
kallithea/controllers/admin/my_account.py . A fix of this kind would 
thus probably be better to have in shared code, for example in 
kallithea/model/ssh_key.py .

It seems like the added test will pass, also without the fix. The 
kallithea/model/db.py public_key setter will just decode using 
base64.b64decode which happens to happily ignore any kind of whitespace. 
I guess it would be better to test this change like we test ssh key 
adding in kallithea/tests/functional/test_admin_users.py and 
kallithea/tests/functional/test_my_account.py .


But back to the core of the problem:

The format of these ssh public keys is that they are one line. First the 
key type, then space, then the base64 encoded key, then optional space 
followed by anything that is a comment and ignored. I thus have some 
concerns of adding partial support for a non-standard format. Especially 
as this is security sensitive and we thus try to be paranoid. When 
parsing the base64 encoded part, we even have a "Incorrect SSH key - 
unexpected characters in base64 part" check to explicitly avoid newlines.

But we could perhaps do it anyway... Would 
https://kallithea-scm.org/repos/kallithea-incoming/changeset/d8ec9261cead78bb6f4768ebf7f82bd21c0d74fd 
work for you and fix your problem?

/Mads



On 9/25/20 9:50 AM, Ed Wong wrote:
> Hi, .
>
> I created a new user in a local Kallithea instance and added a public
> key to it and saved.
>
> Then I proceeded to try to clone from a different system using that
> user account's key.  No matter what I did, it kept on asking for
> a password.
> just
> Completely stumped me as I could clone off a different account,
> so I went to insspect the public key and realized that when
> I copy and pasted off a mingw terminal, it had included
> a bunch of \n.  I removed the \n and repasted the public_key.
>
> Then it allowed me to clone.
>
> I'm currently setting up a dev env to test if my fix is gonna
> work though I suspect it's a bit of a hack.  I've included
> it in this post for comment.
>
>
> Edmund
>
> diff -r c819a1e9103b kallithea/controllers/admin/users.py
> --- a/kallithea/controllers/admin/users.py      Mon Aug 24 15:02:16 2020
> +0200
> +++ b/kallithea/controllers/admin/users.py      Fri Sep 25 15:33:19 2020
> +0800
> @@ -446,10 +446,13 @@
>
>       @IfSshEnabled
>       def ssh_keys_add(self, id):
> +        def rem_newline(in_pub_key):
> +            return in_pub_key.replace("\r\n", "").replace("\n", "")
> +
>           c.user = self._get_user_or_raise_if_default(id)
>
>           description = request.POST.get('description')
> -        public_key = request.POST.get('public_key')
> +        public_key = rem_newline(request.POST.get('public_key'))
>           try:
>               new_ssh_key = SshKeyModel().create(c.user.user_id,
>                                                  description, public_key)
> diff -r c819a1e9103b kallithea/tests/models/test_user_ssh_keys.py
> --- a/kallithea/tests/models/test_user_ssh_keys.py      Mon Aug 24
> 15:02:16 2020 +0200
> +++ b/kallithea/tests/models/test_user_ssh_keys.py      Fri Sep 25
> 15:33:19 2020 +0800
> @@ -7,6 +7,10 @@
>
>   public_key = 'ssh-rsa
> AAAAB3NzaC1yc2EAAAADAQABAAAAgQC6Ycnc2oUZHQnQwuqgZqTTdMDZD7ataf3JM7oG2Fw8JR6cdmz4QZLe5mfDwaFwG2pWHLRpVqzfrD/Pn3rIO++bgCJH5ydczrl1WScfryV1hYMJ/4EzLGM657J1/q5EI+b9SntKjf4ax+KP322L0TNQGbZUHLbfG2MwHMrYBQpHUQ==
> kallithea at localhost'
> +public_key_with_lf = 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAA\n' + \
> +                   'AAgQC6Ycnc2oUZHQnQwuqgZqTTdMDZD7ataf3\n' +
> +
> 'JM7oG2Fw8JR6cdmz4QZLe5mfDwaFwG2pWHLRpVqzfrD/Pn3rIO++bgCJH5ydczrl1WScfryV1hYMJ/4EzLGM657J1/q5EI+b9SntKjf4ax+KP322L0TNQGbZUHLbfG2MwHMrYBQpHUQ==
> kallithea at localhost'
> +
>
>   class TestUserSshKeys(TestController):
>
> @@ -15,3 +19,9 @@
>           key_model.public_key = public_key
>           expected = 'Ke3oUCNJM87P0jJTb3D+e3shjceP2CqMpQKVd75E9I8'
>           assert expected == key_model.fingerprint
> +
> +    def test_line_feed_public_key(self):
> +        key_model = UserSshKeys()
> +        key_model.public_key = public_key_with_lf
> +        expected = 'Ke3oUCNJM87P0jJTb3D+e3shjceP2CqMpQKVd75E9I8'
> +        assert expected == key_model.fingerprint
>