Validate SSH keys through the sshkey gemrd-40552-gitlab-should-check-if-keys-are-valid-before-saving

6 files changed, 119 insertions, 45 deletions

diff --git a/Gemfile b/Gemfile
index 880ed483c34..a05ca23f5eb 100644
--- a/Gemfile
+++ b/Gemfile
@@ -401,6 +401,7 @@ gem 'sys-filesystem', '~> 1.1.6'
 
 # SSH host key support
 gem 'net-ssh', '~> 4.1.0'
+gem 'sshkey', '~> 1.9.0'
 
 # Required for ED25519 SSH host key support
 group :ed25519 do
diff --git a/Gemfile.lock b/Gemfile.lock
index 22c4fc0ef28..8de6c8d80a8 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -895,6 +895,7 @@ GEM
       activesupport (>= 4.0)
       sprockets (>= 3.0.0)
     sqlite3 (1.3.13)
+    sshkey (1.9.0)
     stackprof (0.2.10)
     state_machines (0.4.0)
     state_machines-activemodel (0.4.0)
@@ -1192,6 +1193,7 @@ DEPENDENCIES
   spring-commands-rspec (~> 1.0.4)
   spring-commands-spinach (~> 1.1.0)
   sprockets (~> 3.7.0)
+  sshkey (~> 1.9.0)
   stackprof (~> 0.2.10)
   state_machines-activerecord (~> 0.4.0)
   sys-filesystem (~> 1.1.6)
diff --git a/lib/gitlab/ssh_public_key.rb b/lib/gitlab/ssh_public_key.rb
index 545e7c74f7e..6f63ea91ae8 100644
--- a/lib/gitlab/ssh_public_key.rb
+++ b/lib/gitlab/ssh_public_key.rb
@@ -53,7 +53,7 @@ module Gitlab
     end
 
     def valid?
-      key.present? && bits && technology.supported_sizes.include?(bits)
+      SSHKey.valid_ssh_public_key?(key_text)
     end
 
     def type
diff --git a/spec/factories/keys.rb b/spec/factories/keys.rb
index 23a98a899f1..3f0c60f32b7 100644
--- a/spec/factories/keys.rb
+++ b/spec/factories/keys.rb
@@ -22,38 +22,104 @@ FactoryBot.define do
     factory :rsa_key_2048 do
       key do
         <<~KEY.delete("\n")
-          ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDFf6RYK3qu/RKF/3ndJmL5xgMLp3O9
-          6x8lTay+QGZ0+9FnnAXMdUqBq/ZU6d/gyMB4IaW3nHzM1w049++yAB6UPCzMB8Uo27K5
-          /jyZCtj7Vm9PFNjF/8am1kp46c/SeYicQgQaSBdzIW3UDEa1Ef68qroOlvpi9PYZ/tA7
-          M0YP0K5PXX+E36zaIRnJVMPT3f2k+GnrxtjafZrwFdpOP/Fol5BQLBgcsyiU+LM1SuaC
-          rzd8c9vyaTA1CxrkxaZh+buAi0PmdDtaDrHd42gqZkXCKavyvgM5o2CkQ5LJHCgzpXy0
-          5qNFzmThBSkb+XtoxbyagBiGbVZtSVow6Xa7qewz= dummy@gitlab.com
+          ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC98dbu7gxcbmAvwMqz/6AALhSr1jiX
+          G0UC8FQMvoDt+ciB+uSJhg7KlxinKjYJnPGfhX+q2K+mmCGAmI/D6q7rFxE+bn09O+75
+          qgkTHi+suDVE6KG7L3n0alGd/qSevfomR77Snh6fQPdG6sEAZz3kehcpfVnq5/IuLFq9
+          FBrgmu52Jd4XZLQZKkDq6zYOJ69FUkGf93LZIV/OOaS+f+qkOGPCUkdKl7oEcgpVNY9S
+          RjBCduXnvi2CyQnnJVkBguGL5VlXwFXH+17Whs7oFWmdiG+4jzBRLIMz4EuIW09b8Su5
+          PW6+bBuXOifHA8KG5TMmjs5LYdCMPFnhTyDyO3a1 dummy@gitlab.com
         KEY
       end
 
       factory :rsa_deploy_key_2048, class: 'DeployKey'
     end
 
+    factory :rsa_key_4096 do
+      key do
+        <<~KEY.delete("\n")
+          ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDGSD77lLtjmzewiBs6nu2R5nu6oNkrA
+          kH/0co1fHHosKfRr+sWkSTKXOVcL7bhRu+tniGBmB5pn+i1qX7BXtrcnv//bCXWIp+me0
+          27L4RJa5/Ep077iiTJlzTpcV664xNUXC8mzBr601HR/Z2TzX5DWJvnyqqFkN7qHTYo/+I
+          oKECnKqNzI5SQrAxgi6sbWA5DFQ/nwcqsUSBo5gCCJ/0QPrR19yVV5lJA19EY2LawOb1S
+          JNOFo4mQupSlBZwvERZJ7IqhBTPtQIfrqqz5VJbI13jK3ViZTugIZqydWAhosUyejP3Sd
+          Cj1KMexrvV95tjUtmhVFlph4tKThQO0p9pXKZNCzYsbQTye6O6Hk2rojOJLyFWqNBVKtI
+          8Ymfu7OQWppRnuUFuhuuS515H1s888bZFMPsC74mPyo0Y7Q9wAoTnQ9Hw6b0J6OfY3PIR
+          VphaCmxh6b7dgSPFdD7TA6j0xk6PCTOIEzBKuc85B3GQc8Nt4sTv6fW8lGeuYWqepW74i
+          geC4qB6U3/3+p3nPdq/bTM1txrhnQsl1r4dv6TLZ51EtHp6sXayp0qd0pRaiavebXFC0i
+          aETLraQpye4FWbBL/8xTjQ/0VPrYVuUCDvDSMIIS3/9g7Kp7ERUDC9jUqOVonm4pTXL9i
+          ItiUBlK7Mob9C4fQIRFnVR00DCmkmVgw== dummy@gitlab.com
+        KEY
+      end
+    end
+
+    factory :rsa_key_5120 do
+      key do
+        <<~KEY.delete("\n")
+          ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACgQDxnZP0TucLH3zcrvt75DPNq+xKqOmJk
+          CEzTytKq4S5MDH0nlx+xOZ9WykhwDHXU0iZBJF7yRdLkZweYDJVKnBzr4t7QP5Sw2/ZdL
+          elvUMWGJjuz28x8Z+8NZ+IxL/exDz7itrhCsLupQhGO1obiIwf8xVzzPoxrQ9dxaN4x96
+          5N+QdQcld8O6xfpSE0p5Y3sRn3kp57aHWoNa/bUGZy0OHLr/ig0uc6EKyWsTmEESOgDyV
+          94wOyHR0KNGEENyxQt4BwAbEBn3Y41HKqD358KKh+XjbECebrrBFigdDL/eYFIUlstJ07
+          SK/HtYjZbiUZCPs8bJA+SBaLK0pGGqguM2LXRoMeMUZFwKKKS2LpRqjKGj3Qt7qMnp1Sk
+          VhiMnxNqL4nJnDOOVo07xDIPKqIBYO67/cp4Icv3IjKxy6K3EIpLr+iRCxcllpDogxolz
+          FC+pEDVpmEvcrGEv1ON6HcCdk/6Q8Iekr8rYDHpKCU5FF2uBHkqq7yNJ1/+NFC4dgyOo0
+          xCVL4D3DvDKNxFYkrzW4ICt0f5XcMnU10yS/OFXz8JwA3jvuLvMRe5JdFiIjb/l86+TgY
+          yvK8Y8N/UWgSgyjXUCv8nxdvpsxdz5h7HBF8E2DIxCVMC23655e5rp5eJW9EU9X5YFZc3
+          u6uWJ1f1aO+1ViTtqkPrqxovNDD+gVel8Ny6MJ4MvmDKY+eM8beNMSSf1n1Oyh/SvCffh
+          ZpUqrXdTr9qwZEOaC75T74AJ7KBl9VvO3vPLZuJrt38R2OZG/4SlNEUA6bb5TWQLtdor/
+          qpPN5jAskkAUzOh5L/M+dmq2jNn03U9xwORCYPZj+fFM9bL99/0knsV0ypZDZyWH dummy@gitlab.com
+        KEY
+      end
+    end
+
+    factory :rsa_key_8192 do
+      key do
+        <<~KEY.delete("\n")
+          ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAEAQC5jMyGtgMOVX4t2GuXkbirJA0Edr+ql
+          OH9grnRBPHPo0Npt6XE6ZN3J3hDULTQo03wmekGw42dxdNSgk+F0GjsUBrMLbqrk485MM
+          e0cUbP4lRXNu4ao87wPVM5fAsD4E3FQiZcI6Df011ZGIL7hGTHt6eafTfr9cJheRyYSu6
+          g06rlnFWbbtSh9oQ7Y6sfDLBcsC9ECcXwe3mwViuQXPIVomZ02EdnBbAhbGHDtA+ZbSvT
+          fraxOMjkxkVvvdjLxXEykpwVuZf8eZ+R/Js8jQ5RKvTZMbfxJNsGEqHD32s43ml4VF549
+          Qz2GJDXF7Cld/n3CT6wvw0mMPM0LnykL2v0CMr44bjIA3KsNEs5MhkcBO8sv5hGfcPhrp
+          m9WwI6gd9vdZVcxarVI+iQS947owvdn4VbEZXynCDqEEv3Zh+FA5p23mf2p7DkG/swiK/
+          IPrjr1wmsiWmwIUsENzJNyJtibKuRsBawC4ZdL797tFilSoTzSpriegSL13joPXz3eOHC
+          Vu4ATHMo3QyLfIFbxrf9PQ79nyOpHoX2YeFXvei3xFkGMundkOqeI+pnJKDyqbiLV7UVl
+          clua11QWNQZf1ZUd0n1wZ1g89de+wl3oJSRbSA5ZpveZEPstcMC/JhogY4JBYsvCT1yHO
+          oNWHo90NZQsUCjNnR+/FVaACtpt2zcPTjjbXvxwCDlT3gXTmTBp/kEZq6u8p+BOlqFgxc
+          P/sdAR8jWTin3Iw/YAcbqNgRHdjMUzJBrPQ5NcK6xFcmkOEQahdJDZs98xozCHkD4Urx6
+          +auTr/uqRYobKoNUNiYqN1n7/dfZjQJJVkHtKd06JTFx+7/SqyfrTKS+/EIf2Hypdy9r9
+          IFR+SWAOi11N/wflS/ZbH95Qt3STifXRecmHzyYGkMOZ+mg3Hi2YU0yn7k+P1jy627xud
+          pT9Ak3HWT5ji8tMyn9udL7m80dYpUiEAxoYZdbSSNCDaKP4ViABnGIeZreIujabI8IdtE
+          IjFQTaF2d5HTYjp28/qf576CFP5L7AGydypipYqZUmsYnay5YVjdm89He3TMD71SwspJl
+          POC4RnM0HS87OE+U0+mVaIe8YYbcjTekpVU9mkqsE/GQ34Egw79VMNNgWq5avOzpT8msC
+          lTJxgfJ1agGgigTvGxUM0FB07+sIdJxxNymAGpLKZ1op8xaJI3o8D86jWgI22za1zxUB5
+          il9U7+KOzaWo9mp3bmhvZWGDwzTXEZhUJYMRby7o6UxSHlA6fKE63JSDD2yhXk4CjsQRN
+          C7Ph9cYSB+Wa3i9Am4rRlJgrF79okmEOMpj1idliHkpIsy/k2CN9Lf2EIHOD4NMuLrSUH
+          4qJsPUq19ZbGIMdImD3vMS5b dummy@gitlab.com
+        KEY
+      end
+    end
+
     factory :dsa_key_2048 do
       key do
         <<~KEY.delete("\n")
-          ssh-dss AAAAB3NzaC1kc3MAAAEBAO/3/NPLA/zSFkMOCaTtGo+uos1flfQ5f038Uk+G
-          Y9AeLGzX+Srhw59GdVXmOQLYBrOt5HdGwqYcmLnE2VurUGmhtfeO5H+3p5pGJbkS0Gxp
-          YH1HRO9lWsncF3Hh1w4lYsDjkclDiSTdfTuN8F4Kb3DXNnVSCieeonp+B25F/CXagyTQ
-          /pvNmHFeYgGCVdnBtFdi+xfxaZ8NKdPrGggzokbKHElDZQ4Xo5EpdcyLajgM7nB2r2Rz
-          OrmeaevKi5lV68ehRa9Yyrb7vxvwiwBwOgqR/mnN7Gnaq1jUdmJY+ct04Qwx37f5jvhv
-          5gA4U40SGMoiHM8RFIN7Ksz0jsyX73MAAAAVALRWOfjfzHpK7KLz4iqDvvTUAevJAAAB
-          AEa9NZ+6y9iQ5erGsdfLTXFrhSefTG0NhghoO/5IFkSGfd8V7kzTvCHaFrcfpEA5kP8t
-          poeOG0TASB6tgGOxm1Bq4Wncry5RORBPJlAVpDGRcvZ931ddH7IgltEInS6za2uH6F/1
-          M1QfKePSLr6xJ1ZLYfP0Og5KTp1x6yMQvfwV0a+XdA+EPgaJWLWp/pWwKWa0oLUgjsIH
-          MYzuOGh5c708uZrmkzqvgtW2NgXhcIroRgynT3IfI2lP2rqqb3uuuE/qH5UCUFO+Dc3H
-          nAFNeQDT/M25AERdPYBAY5a+iPjIgO+jT7BfmfByT+AZTqZySrCyc7nNZL3YgGLK0l6A
-          1GgAAAEBAN9FpFOdIXE+YEZhKl1vPmbcn+b1y5zOl6N4x1B7Q8pD/pLMziWROIS8uLzb
-          aZ0sMIWezHIkxuo1iROMeT+jtCubn7ragaN6AX7nMpxYUH9+mYZZs/fyElt6wCviVhTI
-          zM+u7VdQsnZttOOlQfogHdL+SpeAft0DsfJjlcgQnsLlHQKv6aPqCPYUST2nE7RyW/Ex
-          PrMxLtOWt0/j8RYHbwwqvyeZqBz3ESBgrS9c5tBdBfauwYUV/E7gPLOU3OZFw9ue7o+z
-          wzoTZqW6Xouy5wtWvSLQSLT5XwOslmQz8QMBxD0AQyDfEFGsBCWzmbTgKv9uqrBjubsS
-          Taja+Cf9kMo== dummy@gitlab.com
+          ssh-dss AAAAB3NzaC1kc3MAAAEBALEB3sM2kPy6LKLiyL+UlDx2vzuKrzSD2nsW2Kb7
+          0ivIqDNJu5CbqIQSkjdMzJiocs33ESFqXid6ezOtVdDwXHJQRxKGalW1kBbFAPjtMxlD
+          bf559+7qN2zfCfcQsgTmNAZ7O+wltqJmyLv5i4QqNwPDvyeBvJ4C+770DzlcQtpkflKJ
+          X+O7i8Ylq34h6UTCTnjry+dFVm1xz97LPf7XuzXGZcAG/eGUNQgxQ2bferKnrpYOXx6c
+          ocSRj9W54nrRFMWuDeOspWp4MoYK0FRMfDQYPksUayGUnm1KQTGuDbB0ahRNCOm8b3tf
+          P9Z+vjANAkqenzDuXCpz2PU/Oj6/N/UAAAAhAPOLyut12Mjcp3eUXLe1xSoI5IRXSLso
+          W9no93dcFNprAAABAQCLhpqKY+PNcwbhhPruL+f+uROghHzDwRNX+e231F4wHHeDDomf
+          WyLVFj31XrHdDXZnS9tTTj5D2XWLovSSxYb3H7earTctmktL0lQ3HapujzvOkn+VM0pG
+          s6B3j54+AM3mg50KZdYWxxv+v/lb6oEcsCjfKNyRIx/5pqX6XI3dxl9MMIxrfVWpkNX+
+          FI68v1LVV61DC9PkNyEHU0v9YBOfrTiS21TIlVIZcSFhuDjg52MekfZAnoKaP7YFJNF3
+          fdCrXaU3hYQrwB9XdskBUppwxKGhf7O6SWEZhAEfPA9kgxaWHoJvsDz8aca576UNe7BP
+          mjzo/SLUX+P4uvcaffd+AAABAEqzpmwjzTxB+DV8C+0LnmKf3L/UlQWyGdmhd65rnbkH
+          GgRMAAkoh4GBOEHL5bznNRmO7X/H6g2fR7SEabxfbvb903KI4nbfFF+3QtnwyIbTBAcH
+          0893D3bi5rsaJcz+c6lBob2En2nThRciefXUk2oPzCQuDyFIyHLJikqRQVcalHCdQ00c
+          /H/JkiJedHNqaeU4TeMk8SM53Brjplj/iiJq+ujc5MlEgACdCwWp0BviFACEoYyFaa3R
+          kc7Xdm9vFpclm9fzgUfPloASA0SkO945in3mIqMfODTb4yRvbjk8If9483fEPgQkczpd
+          ptBz1VAKg8AmRcz1GmBIxs+Stn0= dummy@gitlab.com
         KEY
       end
     end
diff --git a/spec/lib/gitlab/ssh_public_key_spec.rb b/spec/lib/gitlab/ssh_public_key_spec.rb
index c15e29774b6..a6ea07e8b6d 100644
--- a/spec/lib/gitlab/ssh_public_key_spec.rb
+++ b/spec/lib/gitlab/ssh_public_key_spec.rb
@@ -76,7 +76,21 @@ describe Gitlab::SSHPublicKey, lib: true do
     subject { public_key }
 
     context 'with a valid SSH key' do
-      it { is_expected.to be_valid }
+      where(:factory) do
+        %i(rsa_key_2048
+           rsa_key_4096
+           rsa_key_5120
+           rsa_key_8192
+           dsa_key_2048
+           ecdsa_key_256
+           ed25519_key_256)
+      end
+
+      with_them do
+        let(:key) { attributes_for(factory)[:key] }
+
+        it { is_expected.to be_valid }
+      end
     end
 
     context 'with an invalid SSH key' do
@@ -117,6 +131,9 @@ describe Gitlab::SSHPublicKey, lib: true do
     where(:factory, :bits) do
       [
         [:rsa_key_2048, 2048],
+        [:rsa_key_4096, 4096],
+        [:rsa_key_5120, 5120],
+        [:rsa_key_8192, 8192],
         [:dsa_key_2048, 2048],
         [:ecdsa_key_256, 256],
         [:ed25519_key_256, 256]
@@ -141,8 +158,11 @@ describe Gitlab::SSHPublicKey, lib: true do
 
     where(:factory, :fingerprint) do
       [
-        [:rsa_key_2048, '2e:ca:dc:e0:37:29:ed:fc:f0:1d:bf:66:d4:cd:51:b1'],
-        [:dsa_key_2048, 'bc:c1:a4:be:7e:8c:84:56:b3:58:93:53:c6:80:78:8c'],
+        [:rsa_key_2048, '58:a8:9d:cd:1f:70:f8:5a:d9:e4:24:8e:da:89:e4:fc'],
+        [:rsa_key_4096, 'df:73:db:29:3c:a5:32:cf:09:17:7e:8e:9d:de:d7:f7'],
+        [:rsa_key_5120, 'fe:fa:3a:4d:7d:51:ec:bf:c7:64:0c:96:d0:17:8a:d0'],
+        [:rsa_key_8192, 'fb:53:7f:e9:2f:f7:17:aa:c8:32:52:06:8e:05:e2:82'],
+        [:dsa_key_2048, 'c8:85:1e:df:44:0f:20:00:3c:66:57:2b:21:10:5a:27'],
         [:ecdsa_key_256, '67:a3:a9:7d:b8:e1:15:d4:80:40:21:34:bb:ed:97:38'],
         [:ed25519_key_256, 'e6:eb:45:8a:3c:59:35:5f:e9:5b:80:12:be:7e:22:73']
       ]
diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb
index bf5703ac986..06d26ef89f1 100644
--- a/spec/models/key_spec.rb
+++ b/spec/models/key_spec.rb
@@ -12,6 +12,9 @@ describe Key, :mailer do
     it { is_expected.to validate_presence_of(:key) }
     it { is_expected.to validate_length_of(:key).is_at_most(5000) }
     it { is_expected.to allow_value(attributes_for(:rsa_key_2048)[:key]).for(:key) }
+    it { is_expected.to allow_value(attributes_for(:rsa_key_4096)[:key]).for(:key) }
+    it { is_expected.to allow_value(attributes_for(:rsa_key_5120)[:key]).for(:key) }
+    it { is_expected.to allow_value(attributes_for(:rsa_key_8192)[:key]).for(:key) }
     it { is_expected.to allow_value(attributes_for(:dsa_key_2048)[:key]).for(:key) }
     it { is_expected.to allow_value(attributes_for(:ecdsa_key_256)[:key]).for(:key) }
     it { is_expected.to allow_value(attributes_for(:ed25519_key_256)[:key]).for(:key) }
@@ -103,24 +106,6 @@ describe Key, :mailer do
     end
   end
 
-  context 'validate size' do
-    where(:key_content, :result) do
-      [
-        [Spec::Support::Helpers::KeyGeneratorHelper.new(512).generate,  false],
-        [Spec::Support::Helpers::KeyGeneratorHelper.new(8192).generate, false],
-        [Spec::Support::Helpers::KeyGeneratorHelper.new(1024).generate, true]
-      ]
-    end
-
-    with_them do
-      it 'validates the size of the key' do
-        key = build(:key, key: key_content)
-
-        expect(key.valid?).to eq(result)
-      end
-    end
-  end
-
   context 'validate it meets key restrictions' do
     where(:factory, :minimum, :result) do
       forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE