Comment 1 Pierre Ossman 2020-12-23 12:14:11 CET

Hmm... Something is off with the documentation. The 8.4 release notes state that the next release will enable UpdateHostKeys by default. However the code indicates it was enabled (except some special cases) in 8.2.

Perhaps what they intend to do in 8.5 is enable it even for those special cases?

Comment 2 Pierre Ossman 2020-12-28 16:21:43 CET

Some behaviours we'll try to match from OpenSSH:

 * Don't update any keys if we authenticated the host using the system known_hosts file (I guess this makes sense in that this is now a sysadmin setting, not a user one)

 * Remove the complete line if the line contains an old key, even if that line also matches other hosts (seems a bit unsafe, but it's what OpenSSH does and such lines should be extremely rare in the wild)

Comment 7 Pierre Ossman 2020-12-29 16:28:17 CET

Seems to work the way we want now:

>  * When getting a list of all host keys, all keys received should be stored for the host

Yup.

>  * If a list of host keys is received, then any other keys for the same should be removed, otherwise they should be left in place

Yup.

>  * If the server is authenticated using a global host key (i.e. .../etc/ssh_known_hosts) then no keys should be removed or added

Yup.

Comment 8 Niko Lehto 2021-01-07 09:07:37 CET

Found out during testing that tlclient can't handle updating keys with the same type like this.

Steps to reproduce:
Given
  - Server with following host keys:
      ssh_host_rsa_key, ssh_host_ecdsa_key and ssh_host_ed25519_key
And
  - Client with these keys in their .thinlinc/known_hosts

When
  1 Server adds another host_rsa_key
  2 client logs in (and therefore updates their known_hosts with the new key)
  3 Server removes the first host_rsa_key
  3 client tries to login again

Then
  - A warning is shown ("Warning, security breach") and login is prevented
And
  - Client host keys are updated with same keys as on server

The same scenario works using ssh client instead of tlclient though.
Should we support this also?

Comment 9 Pierre Ossman 2021-01-07 12:31:18 CET

Given the original description for this feature, multiple keys of the same type do indeed need to be handled properly:

http://blog.djm.net.au/2015/02/key-rotation-in-openssh-68.html

Comment 11 Pierre Ossman 2021-01-07 16:48:11 CET

As noted on bug 4716:

> Redone and now all platforms support saving as many keys as you'd like.
> 
> The format used in the Windows registry was only slightly tweaked in order to
> maintain backward compatibility. Older clients (or current one) don't care
> about the details of the name, just as long as the host bit matches. So all
> client versions should pick up all entries and do the right thing.
> 
> (of course, older clients can't write multiple keys of the same type properly
> though)

Comment 12 Frida Flodin 2021-01-08 10:05:42 CET

Found this scenario when testing bug 7618.

Given:
  1. We have connected to the standard port 22 before and save the host keys.

When: 
  2. When connecting to a non-standard port the keys for port 22 should be used 
  if there are no keys for the non-standard port.

Then:
  3. After the connection we will add keys in known_hosts for the non-standard 
  port. OpenSSH on the other hand will not add any entries for the non-standard 
  port.


Tested on Ubuntu20.04, with OpenSSH_8.2p1 and tlclient nightly-build 6711.

Comment 13 Pierre Ossman 2021-01-08 15:00:43 CET

Hmm... given how the code in OpenSSH is structured it's difficult to tell if this is intentional or an oversight.

For now it's probably best to play it safe and try to mimic their actual behaviour.

Comment 15 Linn 2021-01-14 09:41:30 CET

(In reply to Niko Lehto from comment #8)
> Found out during testing that tlclient can't handle updating keys with the
> same type like this.
I tested the scenario from this comment with client build #1764 on Ubuntu 20.04, and the warning about security breach is still present.

The content of known_hosts is described below:

> Given
>   - Client with these keys in their .thinlinc/known_hosts
known_hosts contains ssh_host_rsa_key (rsa_key_1), ssh_host_ecdsa_key and ssh_host_ed25519_key


> When
>   2 client logs in (and therefore updates their known_hosts with the new key)
rsa_key_2 gets added after rsa_key_1 in known_hosts


> When
>   3 client tries to login again
rsa_key_1 is removed, and the warning is shown

Comment 16 Pierre Ossman 2021-01-14 10:41:54 CET

False alarm. The issue was actually bug 7625. The host keys were properly handled in the first connection to the master. After restarting the vsmserver service the second connection also worked without error.

Comment 17 Linn 2021-01-14 15:07:26 CET

Tested the following on Ubuntu 20.04 and Windows 10, and everything seems to be working as expected.

 Linux | Win
   ✓      ✓   Save all keys¹
   ✓      ✓   Only keep recieved keys¹
   ✓      ✓   Don't update keys if using global key¹
   ✓      ✓   Save multiple keys of same type²
   ✓      ✓   Don't add keys for non-standard port³

¹ From acceptance criteria
² From comment 8
³ From comment 12