From 7044a3a54a4ee4dd45af111727df2ff512db1a22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Fri, 16 Feb 2018 11:32:08 -0500 Subject: [PATCH] Validate SSH keys through the sshkey gem --- Gemfile | 1 + Gemfile.lock | 2 + lib/gitlab/ssh_public_key.rb | 2 +- spec/factories/keys.rb | 112 ++++++++++++++++++++----- spec/lib/gitlab/ssh_public_key_spec.rb | 26 +++++- spec/models/key_spec.rb | 21 +---- 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