Merge pull request #709 from fwininger/remove_unsecure_algs_by_default

Remove unsecure algs by default

4 files changed, 139 insertions, 53 deletions

diff --git a/README.md b/README.md
index 85842d2..1f546ed 100644
--- a/README.md
+++ b/README.md
@@ -26,6 +26,66 @@ It allows you to write programs that invoke and interact with processes on remot
 * Support for SSH subsystems
 * Forward local and remote ports via an SSH connection
 
+## Supported Algorithms
+
+Net::SSH 6.0 remove by default the usage of weak algorithms.
+We strongly recommend that you install a servers's version that supports the latest algorithms.
+
+It is possible to return to the previous behavior by adding the option : `append_all_supported_algorithms: true`
+
+Unsecure algoritms will be definively remove in Net::SSH 7.*.
+
+### Host Keys
+
+| Name                 | Support               | Details  |
+|----------------------|-----------------------|----------|
+| ssh-rsa              | OK                    |          |
+| ssh-ed25519          | OK                    | Require the gem `ed25519` |
+| ecdsa-sha2-nistp521  | OK                    | [using weak elliptic curves](https://safecurves.cr.yp.to/) |
+| ecdsa-sha2-nistp384  | OK                    | [using weak elliptic curves](https://safecurves.cr.yp.to/) |
+| ecdsa-sha2-nistp256  | OK                    | [using weak elliptic curves](https://safecurves.cr.yp.to/) |
+| ssh-dss              | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+
+### Key Exchange
+
+| Name                                 | Support               | Details  |
+|--------------------------------------|-----------------------|----------|
+| ecdh-sha2-nistp521                   | OK                    | [using weak elliptic curves](https://safecurves.cr.yp.to/) |
+| ecdh-sha2-nistp384                   | OK                    | [using weak elliptic curves](https://safecurves.cr.yp.to/) |
+| ecdh-sha2-nistp256                   | OK                    | [using weak elliptic curves](https://safecurves.cr.yp.to/) |
+| diffie-hellman-group1-sha1           | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| diffie-hellman-group14-sha1          | OK                    |          |
+| diffie-hellman-group-exchange-sha1   | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| diffie-hellman-group-exchange-sha256 | OK                    |          |
+
+### Encryption algorithms (ciphers)
+
+| Name                                 | Support               | Details  |
+|--------------------------------------|-----------------------|----------|
+| aes256-ctr / aes192-ctr / aes128-ctr | OK                    | [using weak elliptic curves](https://safecurves.cr.yp.to/) |
+| aes256-cbc / aes192-cbc / aes128-cbc | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| rijndael-cbc@lysator.liu.se          | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| blowfish-ctr blowfish-cbc            | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| cast128-ctr cast128-cbc              | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| 3des-ctr 3des-cbc                    | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| idea-cbc                             | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| none                                 | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+
+### Message Authentication Code algorithms
+
+| Name                 | Support               | Details  |
+|----------------------|-----------------------|----------|
+| hmac-sha2-512        | OK                    |          |
+| hmac-sha2-256        | OK                    |          |
+| hmac-sha2-512-96     | Deprecated in 5.3     | removed from the specification, will be removed in 6.0 |
+| hmac-sha2-256-96     | Deprecated in 5.3     | removed from the specification, will be removed in 6.0 |
+| hmac-sha1            | OK                    | for backward compatibility      |
+| hmac-sha1-96         | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| hmac-ripemd160       | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| hmac-md5             | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| hmac-md5-96          | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+| none                 | Deprecated in 5.3     | unsecure, will be removed in 6.0 |
+
 ## SYNOPSIS:
 
 In a nutshell:
diff --git a/lib/net/ssh/test.rb b/lib/net/ssh/test.rb
index dd8c1bc..7b84cd4 100644
--- a/lib/net/ssh/test.rb
+++ b/lib/net/ssh/test.rb
@@ -74,7 +74,7 @@ module Net
       def transport(options={})
         @transport ||= Net::SSH::Transport::Session.new(
           options[:host] || "localhost",
-          options.merge(kex: "test", host_key: "ssh-rsa", verify_host_key: :never, proxy: socket(options))
+          options.merge(kex: "test", host_key: "ssh-rsa", append_all_supported_algorithms: true, verify_host_key: :never, proxy: socket(options))
         )
       end
 
diff --git a/lib/net/ssh/transport/algorithms.rb b/lib/net/ssh/transport/algorithms.rb
index 6d2a52c..8fca624 100644
--- a/lib/net/ssh/transport/algorithms.rb
+++ b/lib/net/ssh/transport/algorithms.rb
@@ -23,9 +23,8 @@ module Net
         include Loggable
         include Constants
 
-        # Define the default algorithms, in order of preference, supported by
-        # Net::SSH.
-        ALGORITHMS = {
+        # Define the default algorithms, in order of preference, supported by Net::SSH.
+        DEFAULT_ALGORITHMS = {
           host_key: %w[ecdsa-sha2-nistp521-cert-v01@openssh.com
                        ecdsa-sha2-nistp384-cert-v01@openssh.com
                        ecdsa-sha2-nistp256-cert-v01@openssh.com
@@ -34,41 +33,55 @@ module Net
                        ecdsa-sha2-nistp256
                        ssh-rsa-cert-v01@openssh.com
                        ssh-rsa-cert-v00@openssh.com
-                       ssh-rsa ssh-dss],
+                       ssh-rsa],
+
           kex: %w[ecdh-sha2-nistp521
                   ecdh-sha2-nistp384
                   ecdh-sha2-nistp256
                   diffie-hellman-group-exchange-sha256
-                  diffie-hellman-group-exchange-sha1
-                  diffie-hellman-group14-sha1
-                  diffie-hellman-group1-sha1],
-          encryption: %w[aes256-ctr aes192-ctr aes128-ctr
-                         aes256-cbc aes192-cbc aes128-cbc
-                         rijndael-cbc@lysator.liu.se
-                         blowfish-ctr blowfish-cbc
-                         cast128-ctr cast128-cbc
-                         3des-ctr 3des-cbc
-                         idea-cbc
-                         none],
+                  diffie-hellman-group14-sha1],
 
-          hmac: %w[hmac-sha2-512 hmac-sha2-256
-                   hmac-sha2-512-96 hmac-sha2-256-96
-                   hmac-sha1 hmac-sha1-96
-                   hmac-ripemd160 hmac-ripemd160@openssh.com
-                   hmac-md5 hmac-md5-96
-                   none],
+          encryption: %w[aes256-ctr aes192-ctr aes128-ctr],
 
-          compression: %w[none zlib@openssh.com zlib],
-          language: %w[]
+          hmac: %w[hmac-sha2-512 hmac-sha2-256
+                   hmac-sha1]
         }
 
         if Net::SSH::Authentication::ED25519Loader::LOADED
-          ALGORITHMS[:host_key].unshift(
+          DEFAULT_ALGORITHMS[:host_key].unshift(
             'ssh-ed25519-cert-v01@openssh.com',
             'ssh-ed25519'
           )
         end
 
+        # Define all algorithms, with the deprecated, supported by Net::SSH.
+        ALGORITHMS = {
+          host_key: DEFAULT_ALGORITHMS[:host_key] + %w[ssh-dss],
+
+          kex: DEFAULT_ALGORITHMS[:kex] +
+              %w[diffie-hellman-group-exchange-sha1
+                 diffie-hellman-group1-sha1],
+
+          encryption: DEFAULT_ALGORITHMS[:encryption] +
+              %w[aes256-cbc aes192-cbc aes128-cbc
+                 rijndael-cbc@lysator.liu.se
+                 blowfish-ctr blowfish-cbc
+                 cast128-ctr cast128-cbc
+                 3des-ctr 3des-cbc
+                 idea-cbc
+                 none],
+
+          hmac: DEFAULT_ALGORITHMS[:hmac] +
+              %w[hmac-sha2-512-96 hmac-sha2-256-96
+                 hmac-sha1-96
+                 hmac-ripemd160 hmac-ripemd160@openssh.com
+                 hmac-md5 hmac-md5-96
+                 none],
+
+          compression: %w[none zlib@openssh.com zlib],
+          language: %w[]
+        }
+
         # The underlying transport layer session that supports this object
         attr_reader :session
 
@@ -234,7 +247,10 @@ module Net
           options[:compression] = %w[zlib@openssh.com zlib] if options[:compression] == true
 
           ALGORITHMS.each do |algorithm, supported|
-            algorithms[algorithm] = compose_algorithm_list(supported, options[algorithm], options[:append_all_supported_algorithms])
+            algorithms[algorithm] = compose_algorithm_list(
+              supported, options[algorithm] || DEFAULT_ALGORITHMS[algorithm],
+              options[:append_all_supported_algorithms]
+            )
           end
 
           # for convention, make sure our list has the same keys as the server
diff --git a/test/transport/test_algorithms.rb b/test/transport/test_algorithms.rb
index 58bf29a..8ba6a68 100644
--- a/test/transport/test_algorithms.rb
+++ b/test/transport/test_algorithms.rb
@@ -18,14 +18,23 @@ module Transport
     end
 
     def test_constructor_should_build_default_list_of_preferred_algorithms
-      assert_equal ed_ec_host_keys + %w[ssh-rsa-cert-v01@openssh.com ssh-rsa-cert-v00@openssh.com ssh-rsa ssh-dss], algorithms[:host_key]
-      assert_equal ec_kex + %w[diffie-hellman-group-exchange-sha256 diffie-hellman-group-exchange-sha1 diffie-hellman-group14-sha1 diffie-hellman-group1-sha1], algorithms[:kex]
-      assert_equal %w[aes256-ctr aes192-ctr aes128-ctr aes256-cbc aes192-cbc aes128-cbc rijndael-cbc@lysator.liu.se blowfish-ctr blowfish-cbc cast128-ctr cast128-cbc 3des-ctr 3des-cbc idea-cbc none], algorithms[:encryption]
-      assert_equal %w[hmac-sha2-512 hmac-sha2-256 hmac-sha2-512-96 hmac-sha2-256-96 hmac-sha1 hmac-sha1-96 hmac-ripemd160 hmac-ripemd160@openssh.com hmac-md5 hmac-md5-96 none], algorithms[:hmac]
+      assert_equal ed_ec_host_keys + %w[ssh-rsa-cert-v01@openssh.com ssh-rsa-cert-v00@openssh.com ssh-rsa], algorithms[:host_key]
+      assert_equal ec_kex + %w[diffie-hellman-group-exchange-sha256 diffie-hellman-group14-sha1], algorithms[:kex]
+      assert_equal %w[aes256-ctr aes192-ctr aes128-ctr], algorithms[:encryption]
+      assert_equal %w[hmac-sha2-512 hmac-sha2-256 hmac-sha1], algorithms[:hmac]
       assert_equal %w[none zlib@openssh.com zlib], algorithms[:compression]
       assert_equal %w[], algorithms[:language]
     end
 
+    def test_constructor_should_build_complete_list_of_algorithms_with_append_all_supported_algorithms
+      assert_equal ed_ec_host_keys + %w[ssh-rsa-cert-v01@openssh.com ssh-rsa-cert-v00@openssh.com ssh-rsa ssh-dss], algorithms(append_all_supported_algorithms: true)[:host_key]
+      assert_equal ec_kex + %w[diffie-hellman-group-exchange-sha256 diffie-hellman-group14-sha1 diffie-hellman-group-exchange-sha1 diffie-hellman-group1-sha1], algorithms(append_all_supported_algorithms: true)[:kex]
+      assert_equal %w[aes256-ctr aes192-ctr aes128-ctr aes256-cbc aes192-cbc aes128-cbc rijndael-cbc@lysator.liu.se blowfish-ctr blowfish-cbc cast128-ctr cast128-cbc 3des-ctr 3des-cbc idea-cbc none], algorithms(append_all_supported_algorithms: true)[:encryption]
+      assert_equal %w[hmac-sha2-512 hmac-sha2-256 hmac-sha1 hmac-sha2-512-96 hmac-sha2-256-96 hmac-sha1-96 hmac-ripemd160 hmac-ripemd160@openssh.com hmac-md5 hmac-md5-96 none], algorithms(append_all_supported_algorithms: true)[:hmac]
+      assert_equal %w[none zlib@openssh.com zlib], algorithms(append_all_supported_algorithms: true)[:compression]
+      assert_equal %w[], algorithms[:language]
+    end
+
     def test_constructor_should_set_client_and_server_prefs_identically
       %w[encryption hmac compression language].each do |key|
         assert_equal algorithms[key.to_sym], algorithms[:"#{key}_client"], key
@@ -64,7 +73,8 @@ module Transport
     end
 
     def test_constructor_with_unrecognized_host_key_type_should_return_whats_supported
-      assert_equal ed_ec_host_keys + %w[ssh-rsa-cert-v01@openssh.com ssh-rsa-cert-v00@openssh.com ssh-rsa ssh-dss], algorithms(host_key: "bogus ssh-rsa",append_all_supported_algorithms: true)[:host_key]
+      assert_equal ed_ec_host_keys + %w[ssh-rsa-cert-v01@openssh.com ssh-rsa-cert-v00@openssh.com ssh-rsa ssh-dss],
+                   algorithms(host_key: "bogus ssh-rsa", append_all_supported_algorithms: true)[:host_key]
     end
 
     def ec_kex
@@ -72,13 +82,13 @@ module Transport
     end
 
     def test_constructor_with_preferred_kex_should_put_preferred_kex_first
-      assert_equal %w[diffie-hellman-group1-sha1] + ec_kex + %w[diffie-hellman-group-exchange-sha256 diffie-hellman-group-exchange-sha1 diffie-hellman-group14-sha1], algorithms(kex: "diffie-hellman-group1-sha1", append_all_supported_algorithms: true)[:kex]
+      assert_equal %w[diffie-hellman-group1-sha1] + ec_kex + %w[diffie-hellman-group-exchange-sha256 diffie-hellman-group14-sha1 diffie-hellman-group-exchange-sha1],
+                   algorithms(kex: "diffie-hellman-group1-sha1", append_all_supported_algorithms: true)[:kex]
     end
 
     def test_constructor_with_unrecognized_kex_should_not_raise_exception
-      assert_equal %w[diffie-hellman-group1-sha1] + ec_kex + %w[diffie-hellman-group-exchange-sha256 diffie-hellman-group-exchange-sha1 diffie-hellman-group14-sha1], algorithms(
-        kex: %w[bogus diffie-hellman-group1-sha1],append_all_supported_algorithms: true
-      )[:kex]
+      assert_equal %w[diffie-hellman-group1-sha1] + ec_kex + %w[diffie-hellman-group-exchange-sha256 diffie-hellman-group14-sha1 diffie-hellman-group-exchange-sha1],
+                   algorithms(kex: %w[bogus diffie-hellman-group1-sha1], append_all_supported_algorithms: true)[:kex]
     end
 
     def test_constructor_with_preferred_encryption_should_put_preferred_encryption_first
@@ -94,15 +104,15 @@ module Transport
     end
 
     def test_constructor_with_preferred_hmac_should_put_preferred_hmac_first
-      assert_equal %w[hmac-md5-96 hmac-sha2-512 hmac-sha2-256 hmac-sha2-512-96 hmac-sha2-256-96 hmac-sha1 hmac-sha1-96 hmac-ripemd160 hmac-ripemd160@openssh.com hmac-md5 none], algorithms(hmac: "hmac-md5-96", append_all_supported_algorithms: true)[:hmac]
+      assert_equal %w[hmac-md5-96 hmac-sha2-512 hmac-sha2-256 hmac-sha1 hmac-sha2-512-96 hmac-sha2-256-96 hmac-sha1-96 hmac-ripemd160 hmac-ripemd160@openssh.com hmac-md5 none], algorithms(hmac: "hmac-md5-96", append_all_supported_algorithms: true)[:hmac]
     end
 
     def test_constructor_with_multiple_preferred_hmac_should_put_all_preferred_hmac_first
-      assert_equal %w[hmac-md5-96 hmac-sha1-96 hmac-sha2-512 hmac-sha2-256 hmac-sha2-512-96 hmac-sha2-256-96 hmac-sha1 hmac-ripemd160 hmac-ripemd160@openssh.com hmac-md5 none], algorithms(hmac: %w[hmac-md5-96 hmac-sha1-96], append_all_supported_algorithms: true)[:hmac]
+      assert_equal %w[hmac-md5-96 hmac-sha1-96 hmac-sha2-512 hmac-sha2-256 hmac-sha1 hmac-sha2-512-96 hmac-sha2-256-96 hmac-ripemd160 hmac-ripemd160@openssh.com hmac-md5 none], algorithms(hmac: %w[hmac-md5-96 hmac-sha1-96], append_all_supported_algorithms: true)[:hmac]
     end
 
     def test_constructor_with_unrecognized_hmac_should_ignore_those
-      assert_equal %w[hmac-sha2-512 hmac-sha2-256 hmac-sha2-512-96 hmac-sha2-256-96 hmac-sha1 hmac-sha1-96 hmac-ripemd160 hmac-ripemd160@openssh.com hmac-md5 hmac-md5-96 none],
+      assert_equal %w[hmac-sha2-512 hmac-sha2-256 hmac-sha1 hmac-sha2-512-96 hmac-sha2-256-96 hmac-sha1-96 hmac-ripemd160 hmac-ripemd160@openssh.com hmac-md5 hmac-md5-96 none],
         algorithms(hmac: "unknown hmac-md5-96", append_all_supported_algorithms: true)[:hmac]
     end
 
@@ -166,7 +176,7 @@ module Transport
     end
 
     def test_key_exchange_when_server_does_not_support_preferred_kex_should_fallback_to_secondary
-      kexinit kex: "diffie-hellman-group1-sha1"
+      kexinit kex: "diffie-hellman-group14-sha1"
       transport.expect do |t,buffer|
         assert_kexinit(buffer)
         install_mock_key_exchange(buffer, kex: Net::SSH::Transport::Kex::DiffieHellmanGroup1SHA1)
@@ -202,10 +212,10 @@ module Transport
     end
 
     def test_exchange_with_zlib_compression_enabled_sets_compression_to_standard
-      algorithms compression: "zlib", append_all_supported_algorithms: true
+      algorithms compression: 'zlib'
 
       transport.expect do |t, buffer|
-        assert_kexinit(buffer, compression_client: "zlib,none,zlib@openssh.com", compression_server: "zlib,none,zlib@openssh.com")
+        assert_kexinit(buffer, compression_client: 'zlib', compression_server: 'zlib')
         install_mock_key_exchange(buffer)
       end
 
@@ -217,10 +227,10 @@ module Transport
     end
 
     def test_exchange_with_zlib_at_openssh_dot_com_compression_enabled_sets_compression_to_delayed
-      algorithms compression: "zlib@openssh.com", append_all_supported_algorithms: true
+      algorithms compression: 'zlib@openssh.com'
 
       transport.expect do |t, buffer|
-        assert_kexinit(buffer, compression_client: "zlib@openssh.com,none,zlib", compression_server: "zlib@openssh.com,none,zlib")
+        assert_kexinit(buffer, compression_client: 'zlib@openssh.com', compression_server: 'zlib@openssh.com')
         install_mock_key_exchange(buffer)
       end
 
@@ -341,16 +351,16 @@ module Transport
     def assert_kexinit(buffer, options={})
       assert_equal KEXINIT, buffer.type
       assert_equal 16, buffer.read(16).length
-      assert_equal options[:kex] || (ec_kex + %w[diffie-hellman-group-exchange-sha256 diffie-hellman-group-exchange-sha1 diffie-hellman-group14-sha1 diffie-hellman-group1-sha1]).join(','), buffer.read_string
-      assert_equal options[:host_key] || (ed_ec_host_keys + %w[ssh-rsa-cert-v01@openssh.com ssh-rsa-cert-v00@openssh.com ssh-rsa ssh-dss]).join(','), buffer.read_string
-      assert_equal options[:encryption_client] || "aes256-ctr,aes192-ctr,aes128-ctr,aes256-cbc,aes192-cbc,aes128-cbc,rijndael-cbc@lysator.liu.se,blowfish-ctr,blowfish-cbc,cast128-ctr,cast128-cbc,3des-ctr,3des-cbc,idea-cbc,none", buffer.read_string
-      assert_equal options[:encryption_server] || "aes256-ctr,aes192-ctr,aes128-ctr,aes256-cbc,aes192-cbc,aes128-cbc,rijndael-cbc@lysator.liu.se,blowfish-ctr,blowfish-cbc,cast128-ctr,cast128-cbc,3des-ctr,3des-cbc,idea-cbc,none", buffer.read_string
-      assert_equal options[:hmac_client] || "hmac-sha2-512,hmac-sha2-256,hmac-sha2-512-96,hmac-sha2-256-96,hmac-sha1,hmac-sha1-96,hmac-ripemd160,hmac-ripemd160@openssh.com,hmac-md5,hmac-md5-96,none", buffer.read_string
-      assert_equal options[:hmac_server] || "hmac-sha2-512,hmac-sha2-256,hmac-sha2-512-96,hmac-sha2-256-96,hmac-sha1,hmac-sha1-96,hmac-ripemd160,hmac-ripemd160@openssh.com,hmac-md5,hmac-md5-96,none", buffer.read_string
-      assert_equal options[:compression_client] || "none,zlib@openssh.com,zlib", buffer.read_string
-      assert_equal options[:compression_server] || "none,zlib@openssh.com,zlib", buffer.read_string
-      assert_equal options[:language_client] || "", buffer.read_string
-      assert_equal options[:language_server] || "", buffer.read_string
+      assert_equal options[:kex] || (ec_kex + %w[diffie-hellman-group-exchange-sha256 diffie-hellman-group14-sha1]).join(','), buffer.read_string
+      assert_equal options[:host_key] || (ed_ec_host_keys + %w[ssh-rsa-cert-v01@openssh.com ssh-rsa-cert-v00@openssh.com ssh-rsa]).join(','), buffer.read_string
+      assert_equal options[:encryption_client] || 'aes256-ctr,aes192-ctr,aes128-ctr', buffer.read_string
+      assert_equal options[:encryption_server] || 'aes256-ctr,aes192-ctr,aes128-ctr', buffer.read_string
+      assert_equal options[:hmac_client] || 'hmac-sha2-512,hmac-sha2-256,hmac-sha1', buffer.read_string
+      assert_equal options[:hmac_server] || 'hmac-sha2-512,hmac-sha2-256,hmac-sha1', buffer.read_string
+      assert_equal options[:compression_client] || 'none,zlib@openssh.com,zlib', buffer.read_string
+      assert_equal options[:compression_server] || 'none,zlib@openssh.com,zlib', buffer.read_string
+      assert_equal options[:language_client] || '', buffer.read_string
+      assert_equal options[:language_server] || '', buffer.read_string
       assert_equal options[:first_kex_follows] || false, buffer.read_bool
     end