From f3ce02b5c944e5956eb7208506ec513623d092d8 Mon Sep 17 00:00:00 2001 From: Sytse Sijbrandij Date: Fri, 21 Sep 2012 18:22:43 +0200 Subject: [PATCH 1/4] Reject ssh keys that break gitolite. Failing test. Working check. --- app/models/key.rb | 17 ++++++++++++++++- spec/factories.rb | 12 +++++++----- spec/models/key_spec.rb | 12 ++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/app/models/key.rb b/app/models/key.rb index a39a4a16..3982a946 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -18,7 +18,7 @@ class Key < ActiveRecord::Base before_save :set_identifier before_validation :strip_white_space delegate :name, :email, to: :user, prefix: true - validate :unique_key + validate :unique_key, :fingerprintable_key def strip_white_space self.key = self.key.strip unless self.key.blank? @@ -32,6 +32,21 @@ class Key < ActiveRecord::Base end end + def fingerprintable_key + return true unless key # Don't test if there is no key. + # `ssh-keygen -lf /dev/stdin <<< "#{key}"` errors with: redirection unexpected + file = Tempfile.new('key_file') + begin + file.puts key + file.rewind + fingerprint_output = `ssh-keygen -lf #{file.path} 2>&1` # Catch stderr. + ensure + file.close + file.unlink # deletes the temp file + end + errors.add(:key, "can't be fingerprinted") if fingerprint_output.match("failed") + end + def set_identifier if is_deploy_key self.identifier = "deploy_" + Digest::MD5.hexdigest(key) diff --git a/spec/factories.rb b/spec/factories.rb index 92790a3f..848fc01f 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -83,11 +83,7 @@ FactoryGirl.define do factory :key do title key do - """ - ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4 - 596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4 - soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0= - """ + "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=" end factory :deploy_key do @@ -97,6 +93,12 @@ FactoryGirl.define do factory :personal_key do user end + + factory :key_with_a_space_in_the_middle do + key do + "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa ++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=" + end + end end factory :milestone do diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 85cd291d..9bb31a16 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -46,4 +46,16 @@ describe Key do end end end + + context "validate it is a fingerprintable key" do + let(:user) { Factory.create(:user) } + + it "accepts the fingerprintable key" do + build(:key, user: user).should be_valid + end + + it "rejects the unfingerprintable key" do + build(:key_with_a_space_in_the_middle).should_not be_valid + end + end end From 81ffb0afddfe101910cff3dd51fe27dd85e11f28 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 25 Sep 2012 14:07:13 +0200 Subject: [PATCH 2/4] Add valid looking key to test. Conflicts: features/steps/profile/profile_ssh_keys.rb --- features/steps/profile/profile_ssh_keys.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/steps/profile/profile_ssh_keys.rb b/features/steps/profile/profile_ssh_keys.rb index 96df2d73..535c3862 100644 --- a/features/steps/profile/profile_ssh_keys.rb +++ b/features/steps/profile/profile_ssh_keys.rb @@ -13,7 +13,7 @@ class ProfileSshKeys < Spinach::FeatureSteps And 'I submit new ssh key "Laptop"' do fill_in "key_title", :with => "Laptop" - fill_in "key_key", :with => "ssh-rsa publickey234=" + fill_in "key_key", :with => "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAzrEJUIR6Y03TCE9rIJ+GqTBvgb8t1jI9h5UBzCLuK4VawOmkLornPqLDrGbm6tcwM/wBrrLvVOqi2HwmkKEIecVO0a64A4rIYScVsXIniHRS6w5twyn1MD3sIbN+socBDcaldECQa2u1dI3tnNVcs8wi77fiRe7RSxePsJceGoheRQgC8AZ510UdIlO+9rjIHUdVN7LLyz512auAfYsgx1OfablkQ/XJcdEwDNgi9imI6nAXhmoKUm1IPLT2yKajTIC64AjLOnE0YyCh6+7RFMpiMyu1qiOCpdjYwTgBRiciNRZCH8xIedyCoAmiUgkUT40XYHwLuwiPJICpkAzp7Q== user@laptop" click_button "Save" end From 012dc2278c1b7cb3cf4021912fca81c4ccf74b2f Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 25 Sep 2012 15:29:17 +0200 Subject: [PATCH 3/4] Valid key for deploy keys spec test. --- spec/requests/projects_deploy_keys_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/projects_deploy_keys_spec.rb b/spec/requests/projects_deploy_keys_spec.rb index 894aa6d3..df1be79d 100644 --- a/spec/requests/projects_deploy_keys_spec.rb +++ b/spec/requests/projects_deploy_keys_spec.rb @@ -42,7 +42,7 @@ describe "Projects", "DeployKeys" do describe "fill in" do before do fill_in "key_title", with: "laptop" - fill_in "key_key", with: "ssh-rsa publickey234=" + fill_in "key_key", with: "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAzrEJUIR6Y03TCE9rIJ+GqTBvgb8t1jI9h5UBzCLuK4VawOmkLornPqLDrGbm6tcwM/wBrrLvVOqi2HwmkKEIecVO0a64A4rIYScVsXIniHRS6w5twyn1MD3sIbN+socBDcaldECQa2u1dI3tnNVcs8wi77fiRe7RSxePsJceGoheRQgC8AZ510UdIlO+9rjIHUdVN7LLyz512auAfYsgx1OfablkQ/XJcdEwDNgi9imI6nAXhmoKUm1IPLT2yKajTIC64AjLOnE0YyCh6+7RFMpiMyu1qiOCpdjYwTgBRiciNRZCH8xIedyCoAmiUgkUT40XYHwLuwiPJICpkAzp7Q== user@laptop" end it { expect { click_button "Save" }.to change {Key.count}.by(1) } From 7284c58c5fb54965b078a2ba9e3634479700416d Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 25 Sep 2012 15:54:07 +0200 Subject: [PATCH 4/4] Don't test if invalid key is valid. --- spec/factories_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb index 5ccc17bd..caf9ac9a 100644 --- a/spec/factories_spec.rb +++ b/spec/factories_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' FactoryGirl.factories.map(&:name).each do |factory_name| + next if :key_with_a_space_in_the_middle == factory_name describe "#{factory_name} factory" do it 'should be valid' do build(factory_name).should be_valid