Reject ssh keys that break gitolite.

Failing test. Working check.

Reject ssh keys that break gitolite.
Failing test. Working check.
Sytse Sijbrandij
1 parent 8f9a450e
Showing 3 changed files with 35 additions and 6 deletions Show diff stats
app/models/key.rb
spec/factories.rb
spec/models/key_spec.rb
@@ -18,7 +18,7 @@ class Key &lt; 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 &lt; 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)
@@ -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
@@ -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
...	...	@@ -18,7 +18,7 @@ class Key < ActiveRecord::Base
18	18	before_save :set_identifier
19	19	before_validation :strip_white_space
20	20	delegate :name, :email, to: :user, prefix: true
21		- validate :unique_key
	21	+ validate :unique_key, :fingerprintable_key
22	22
23	23	def strip_white_space
24	24	self.key = self.key.strip unless self.key.blank?
...	...	@@ -32,6 +32,21 @@ class Key < ActiveRecord::Base
32	32	end
33	33	end
34	34
	35	+ def fingerprintable_key
	36	+ return true unless key # Don't test if there is no key.
	37	+ # `ssh-keygen -lf /dev/stdin <<< "#{key}"` errors with: redirection unexpected
	38	+ file = Tempfile.new('key_file')
	39	+ begin
	40	+ file.puts key
	41	+ file.rewind
	42	+ fingerprint_output = `ssh-keygen -lf #{file.path} 2>&1` # Catch stderr.
	43	+ ensure
	44	+ file.close
	45	+ file.unlink # deletes the temp file
	46	+ end
	47	+ errors.add(:key, "can't be fingerprinted") if fingerprint_output.match("failed")
	48	+ end
	49	+
35	50	def set_identifier
36	51	if is_deploy_key
37	52	self.identifier = "deploy_" + Digest::MD5.hexdigest(key)
...	...
...	...	@@ -83,11 +83,7 @@ FactoryGirl.define do
83	83	factory :key do
84	84	title
85	85	key do
86		- """
87		- ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4
88		- 596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4
89		- soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=
90		- """
	86	+ "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0="
91	87	end
92	88
93	89	factory :deploy_key do
...	...	@@ -97,6 +93,12 @@ FactoryGirl.define do
97	93	factory :personal_key do
98	94	user
99	95	end
	96	+
	97	+ factory :key_with_a_space_in_the_middle do
	98	+ key do
	99	+ "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa ++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0="
	100	+ end
	101	+ end
100	102	end
101	103
102	104	factory :milestone do
...	...
...	...	@@ -46,4 +46,16 @@ describe Key do
46	46	end
47	47	end
48	48	end
	49	+
	50	+ context "validate it is a fingerprintable key" do
	51	+ let(:user) { Factory.create(:user) }
	52	+
	53	+ it "accepts the fingerprintable key" do
	54	+ build(:key, user: user).should be_valid
	55	+ end
	56	+
	57	+ it "rejects the unfingerprintable key" do
	58	+ build(:key_with_a_space_in_the_middle).should_not be_valid
	59	+ end
	60	+ end
49	61	end
...	...