Provide an alternative implementation of Net::SSH::KnownHost in ssh_options

[byroot]

As discussed in #326 (comment)
Net::SSH re-parse the known_hosts files every time it needs to lookup for a known key.
This alternative implementation parse it once and for all, and cache the result.

NB: This is not complete yet, I need to also make test_known_hosts_for_when_an_host_hash_is_recognized pass.

cc @mattbrictson @TiagoCardoso1983

[mattbrictson]

@byroot I haven't had a chance to review yet, but in the meantime can you look into this test failure reported by Travis? Thanks.

SSHKit::Backend::TestNetssh
[]
  test_known_hosts_for_when_an_host_hash_is_recognized            FAIL (0.00s)
Minitest::Assertion:         Expected: 1
          Actual: 0
        /home/travis/build/capistrano/sshkit/test/unit/backends/test_netssh.rb:72:in `perform_known_hosts_test'
        /home/travis/build/capistrano/sshkit/test/unit/backends/test_netssh.rb:62:in `test_known_hosts_for_when_an_host_hash_is_recognized'

[byroot]

That's what I've put in my NB. I haven't finished yet :)

[mattbrictson]

Oops, sorry! 🤦

[byroot]

@mattbrictson I finally got around to finish the feature.

[mattbrictson]

Not sure if you saw this already, but RuboCop has flagged something that is breaking the build:

lib/sshkit/backends/netssh.rb:38:25: W: Assignment in condition - you probably meant to use ==.
            if key_list = keys[host]

https://travis-ci.org/capistrano/sshkit/jobs/109975382

[byroot]

Nope. But i'll fix it

[byroot]

Done 👌

[mattbrictson]

@byroot I think perhaps some documentation is in order to explain how this is supposed to be used?

I did my best to test this as follows:

1. Checked out net-ssh@master
2. Used your branch of sshkit
3. In my deploy.rb, added to my ssh_options: :known_hosts => SSHKit::Backend::Netssh::KnownHosts.new
4. Tried a cap production deploy:check

But:

NoMethodError: undefined method `each' for nil:NilClass
sshkit/backends/netssh.rb:41:in `block in keys_for'
sshkit/backends/netssh.rb:37:in `each'
sshkit/backends/netssh.rb:37:in `keys_for'
sshkit/backends/netssh.rb:132:in `block in search_for'
sshkit/backends/netssh.rb:131:in `map'
sshkit/backends/netssh.rb:131:in `search_for'
net/ssh/transport/algorithms.rb:246:in `prepare_preferred_algorithms!'
net/ssh/transport/algorithms.rb:124:in `initialize'
net/ssh/transport/session.rb:86:in `new'
net/ssh/transport/session.rb:86:in `initialize'
net/ssh.rb:227:in `new'
net/ssh.rb:227:in `start'

Is there more I have to do to use this correctly?

[byroot]

@mattbrictson apologies, I forgot to test in realistic conditions :/ and then got little time to fix it.

I just fixed the issue you had, and tested that branch against our capistrano recipe.

I think perhaps some documentation is in order to explain how this is supposed to be used?

It's exactly what you figured out:

In my deploy.rb, added to my ssh_options: :known_hosts => SSHKit::Backend::Netssh::KnownHosts.new

What kind of documentation are you thinking about? A README section?

I was seeing this as a beta feature for now that you have to explicitly and then maybe it could become the default in a future release?

Let me know.

[mattbrictson]

What kind of documentation are you thinking about? A README section?

Yes, I think a small mention in the README near where we talk about connection pooling would be good.

I was seeing this as a beta feature for now that you have to explicitly and then maybe it could become the default in a future release?

Sounds good. Please open a separate issue for making it the default, so that we don't lose track of that task.

👍

[byroot]

@mattbrictson I had to change the PR a bit with 25510e4 to react to net-ssh/net-ssh#320

Basically there is 2 possibilities:

• net-ssh < 3.1.0: Even if you pass known_hosts: SSHKit::Backends::Netssh::KnownHosts.new it won't be used by net-ssh
• net-ssh >- 3.1.0: Then it will be used.

I can setup a Travis build matrix with 2 versions of net-ssh if you want.

I also updated the README, let me know.

[byroot]

Oh and the profiles:

Without known_hosts caching:

==================================
  Mode: wall(1000)
  Samples: 3397 (6.44% miss rate)
  GC: 264 (7.77%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
      1325  (39.0%)        1321  (38.9%)     SSHKit::Runner::Parallel#execute
       234   (6.9%)         234   (6.9%)     block (3 levels) in Net::SSH::KnownHosts#keys_for
       418  (12.3%)         164   (4.8%)     block (2 levels) in Net::SSH::KnownHosts#keys_for
       145   (4.3%)         107   (3.1%)     Net::SSH::Transport::Kex::DiffieHellmanGroup1SHA1#send_kexinit
       175   (5.2%)          99   (2.9%)     OpenSSL::PKey::DH#valid?
        92   (2.7%)          92   (2.7%)     Net::SSH::Compat.io_select
        76   (2.2%)          76   (2.2%)     block in OpenSSL::PKey::DH#valid?
        63   (1.9%)          63   (1.9%)     block (2 levels) in Net::SSH::Transport::ServerVersion#negotiate!
        49   (1.4%)          49   (1.4%)     block (2 levels) in Net::SSH::Connection::Channel#forward_local_env
        73   (2.1%)          44   (1.3%)     block in Net::SSH::Config.load
        46   (1.4%)          40   (1.2%)     Net::SSH::Buffer#read
       105   (3.1%)          32   (0.9%)     Net::SSH::Config.load
        37   (1.1%)          29   (0.9%)     Net::SSH::Transport::State#update_next_iv
        54   (1.6%)          27   (0.8%)     Net::SSH::BufferedIo#fill
        27   (0.8%)          27   (0.8%)     Net::SSH::Buffer#write_long
        23   (0.7%)          23   (0.7%)     Net::SSH::Config.pattern2regex
        23   (0.7%)          23   (0.7%)     Logger#debug?
        23   (0.7%)          23   (0.7%)     Net::SSH::Transport::HMAC::Abstract.mac_length
        22   (0.6%)          22   (0.6%)     Net::SSH::Transport::HMAC::Abstract.digest_class
        21   (0.6%)          21   (0.6%)     Net::SSH::BufferedIo#input
       448  (13.2%)          20   (0.6%)     Net::SSH::KnownHosts#keys_for
        46   (1.4%)          19   (0.6%)     Net::SSH::Buffer#read_long
       775  (22.8%)          18   (0.5%)     block in Net::SSH::Transport::Session#poll_message
        18   (0.5%)          18   (0.5%)     Net::SSH::Buffer#initialize
        65   (1.9%)          17   (0.5%)     block in Net::SSH::Buffer.from
        16   (0.5%)          16   (0.5%)     Net::SSH::KnownHosts#known_host_hash?
        82   (2.4%)          15   (0.4%)     Net::SSH::Buffer.from
       175   (5.2%)          14   (0.4%)     Net::SSH::Transport::PacketStream#poll_next_packet
        14   (0.4%)          14   (0.4%)     Net::SSH::Buffer#to_s
        14   (0.4%)          14   (0.4%)     block (3 levels) in <class:Digest>

With known_hosts caching:

==================================
  Mode: wall(1000)
  Samples: 2845 (9.39% miss rate)
  GC: 198 (6.96%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
      1004  (35.3%)        1001  (35.2%)     SSHKit::Runner::Parallel#execute
       259   (9.1%)         241   (8.5%)     SSHKit::Backend::ConnectionPool#find_cache
       153   (5.4%)         153   (5.4%)     Net::SSH::Compat.io_select
       151   (5.3%)         116   (4.1%)     Net::SSH::Transport::Kex::DiffieHellmanGroup1SHA1#send_kexinit
       173   (6.1%)         114   (4.0%)     OpenSSL::PKey::DH#valid?
        60   (2.1%)          60   (2.1%)     block (2 levels) in Net::SSH::Transport::ServerVersion#negotiate!
        59   (2.1%)          59   (2.1%)     block in OpenSSL::PKey::DH#valid?
        61   (2.1%)          41   (1.4%)     block in Net::SSH::Config.load
        95   (3.3%)          34   (1.2%)     Net::SSH::Config.load
        34   (1.2%)          34   (1.2%)     block (2 levels) in Net::SSH::Connection::Channel#forward_local_env
        64   (2.2%)          31   (1.1%)     Net::SSH::BufferedIo#fill
        37   (1.3%)          30   (1.1%)     Net::SSH::Buffer#read
       157   (5.5%)          27   (0.9%)     Net::SSH::Transport::PacketStream#poll_next_packet
        27   (0.9%)          27   (0.9%)     Net::SSH::Buffer#initialize
        26   (0.9%)          26   (0.9%)     Logger#debug?
        24   (0.8%)          24   (0.8%)     Net::SSH::Buffer#length
        21   (0.7%)          21   (0.7%)     Net::SSH::Buffer#write_long
        19   (0.7%)          19   (0.7%)     Net::SSH::Transport::HMAC::Abstract.mac_length
        43   (1.5%)          19   (0.7%)     Net::SSH::BufferedIo#send_pending
        19   (0.7%)          19   (0.7%)     block in Net::SSH::Transport::PacketStream#enqueue_packet
        22   (0.8%)          18   (0.6%)     Net::SSH::Transport::State#update_next_iv
        18   (0.6%)          18   (0.6%)     Net::SSH::BufferedIo#input
        25   (0.9%)          17   (0.6%)     Net::SSH::Authentication::Agent#read_packet
        16   (0.6%)          16   (0.6%)     Net::SSH::Buffer#to_s
        15   (0.5%)          14   (0.5%)     OpenSSL::PKey::RSA#ssh_do_verify
        14   (0.5%)          14   (0.5%)     Net::SSH::Config.pattern2regex
        14   (0.5%)          14   (0.5%)     Net::SSH::Buffer#append
        13   (0.5%)          13   (0.5%)     block (3 levels) in <class:Digest>
        23   (0.8%)          13   (0.5%)     Net::SSH::KeyFactory.load_public_key
        11   (0.4%)          11   (0.4%)     Net::SSH::Transport::HMAC::Abstract.digest_class

[mattbrictson]

@byroot I am just now coming back to this PR. Let's try to get this merged in.

1. Can you fix the merge conflicts?
2. netssh.rb is getting pretty huge. What do you think about moving the new KnownHosts and KnownHostsKeys classes into separate files?
3. Otherwise, in your estimation, is this PR ready to go?

Thanks!

[byroot]

Make dense, i'll do all that in a few minutes

[mattbrictson]

Great. I'm planning on releasing SSHKit 1.10 today, and it would be nice to get this in.

[byroot]

Can you fix the merge conflicts?

Done

moving the new KnownHosts and KnownHostsKeys classes into separate files?

Done as well. NB: I moved them both in a single file. Since they interact a lot together I think it makes sense.

Otherwise, in your estimation, is this PR ready to go?

It's been more than a month we are running it in production now. I just checked, we deployed a bit more than a thousand time with it, so 400k SSH connections over a month. IMO we're good :)

[leehambley]

It's been more than a month we are running it in production now. I just checked, we deployed a bit more than a thousand time with it, so 400k SSH connections over a month. IMO we're good :)

😆 🚀

[mattbrictson]

Merged. Thanks! 🙌