1
From a05502f2d8c627c08ae33c2fb49d5dbe36affb16 Mon Sep 17 00:00:00 2001
2
From: Daniel Pittman <daniel@puppetlabs.com>
3
Date: Wed, 28 Sep 2011 23:59:49 -0700
4
Subject: [PATCH] (#9793) "secure" indirector file backed terminus base class.
6
The file base class in the indirector trusted the request key directly, which
7
made it vulnerable to the same potential for injection attacks as other
10
However, this is somewhat mitigated by the fact that base class is entirely
11
unused. We can simple eliminate it from the system, because nothing is more
12
secure than code that doesn't exist.
14
The only consumer of the code was in the tests, and didn't care what base
15
class was used, so that was substituted with a continuing class.
17
Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
19
lib/puppet/indirector/file.rb | 79 ---------------
20
spec/unit/indirector/file_spec.rb | 179 ---------------------------------
21
spec/unit/indirector/terminus_spec.rb | 6 +-
22
3 files changed, 3 insertions(+), 261 deletions(-)
23
delete mode 100644 lib/puppet/indirector/file.rb
24
delete mode 100755 spec/unit/indirector/file_spec.rb
26
diff --git a/lib/puppet/indirector/file.rb b/lib/puppet/indirector/file.rb
27
deleted file mode 100644
28
index b3746b7..0000000
29
--- a/lib/puppet/indirector/file.rb
32
-require 'puppet/indirector/terminus'
34
-# Store instances as files, usually serialized using some format.
35
-class Puppet::Indirector::File < Puppet::Indirector::Terminus
36
- # Where do we store our data?
38
- name = Puppet.run_mode.master? ? :server_datadir : :client_datadir
40
- File.join(Puppet.settings[name], self.class.indirection_name.to_s)
43
- def file_format(path)
44
- path =~ /\.(\w+)$/ and return $1
47
- def file_path(request)
48
- File.join(data_directory, request.key + ".#{serialization_format}")
51
- def latest_path(request)
52
- files = Dir.glob(File.join(data_directory, request.key + ".*"))
53
- return nil if files.empty?
55
- # Return the newest file.
56
- files.sort { |a, b| File.stat(b).mtime <=> File.stat(a).mtime }[0]
59
- def serialization_format
60
- model.default_format
63
- # Remove files on disk.
64
- def destroy(request)
67
- Dir.glob(File.join(data_directory, request.key.to_s + ".*")).each do |file|
72
- raise Puppet::Error, "Could not remove #{request.key}: #{detail}"
75
- raise Puppet::Error, "Could not find files for #{request.key} to remove" unless removed
78
- # Return a model instance for a given file on disk.
80
- return nil unless path = latest_path(request)
81
- format = file_format(path)
83
- raise ArgumentError, "File format #{format} is not supported by #{self.class.indirection_name}" unless model.support_format?(format)
86
- return model.convert_from(format, File.read(path))
88
- raise Puppet::Error, "Could not convert path #{path} into a #{self.class.indirection_name}: #{detail}"
92
- # Save a new file to disk.
94
- path = file_path(request)
96
- dir = File.dirname(path)
98
- raise Puppet::Error.new("Cannot save #{request.key}; parent directory #{dir} does not exist") unless File.directory?(dir)
101
- File.open(path, "w") { |f| f.print request.instance.render(serialization_format) }
103
- raise Puppet::Error, "Could not write #{request.key}: #{detail}" % [request.key, detail]
111
diff --git a/spec/unit/indirector/file_spec.rb b/spec/unit/indirector/file_spec.rb
112
deleted file mode 100755
113
index b72bf4d..0000000
114
--- a/spec/unit/indirector/file_spec.rb
117
-#!/usr/bin/env rspec
118
-require 'spec_helper'
119
-require 'puppet/indirector/file'
122
-describe Puppet::Indirector::File do
124
- Puppet::Indirector::Terminus.stubs(:register_terminus_class)
125
- @model = mock 'model'
126
- @indirection = stub 'indirection', :name => :mystuff, :register_terminus_type => nil, :model => @model
127
- Puppet::Indirector::Indirection.stubs(:instance).returns(@indirection)
129
- module Testing; end
130
- @file_class = class Testing::MyFile < Puppet::Indirector::File
134
- @searcher = @file_class.new
139
- @request = stub 'request', :key => @path
142
- describe "when finding files" do
143
- it "should provide a method to return file contents at a specified path" do
144
- @searcher.should respond_to(:find)
147
- it "should use the server data directory plus the indirection name if the run_mode is master" do
148
- Puppet.run_mode.expects(:master?).returns true
149
- Puppet.settings.expects(:value).with(:server_datadir).returns "/my/dir"
151
- @searcher.data_directory.should == File.join("/my/dir", "mystuff")
154
- it "should use the client data directory plus the indirection name if the run_mode is not master" do
155
- Puppet.run_mode.expects(:master?).returns false
156
- Puppet.settings.expects(:value).with(:client_datadir).returns "/my/dir"
158
- @searcher.data_directory.should == File.join("/my/dir", "mystuff")
161
- it "should use the newest file in the data directory matching the indirection key without extension" do
162
- @searcher.expects(:data_directory).returns "/data/dir"
163
- @request.stubs(:key).returns "foo"
164
- Dir.expects(:glob).with("/data/dir/foo.*").returns %w{/data1.stuff /data2.stuff}
166
- stat1 = stub 'data1', :mtime => (Time.now - 5)
167
- stat2 = stub 'data2', :mtime => Time.now
168
- File.expects(:stat).with("/data1.stuff").returns stat1
169
- File.expects(:stat).with("/data2.stuff").returns stat2
171
- @searcher.latest_path(@request).should == "/data2.stuff"
174
- it "should return nil when no files are found" do
175
- @searcher.stubs(:latest_path).returns nil
177
- @searcher.find(@request).should be_nil
180
- it "should determine the file format from the file extension" do
181
- @searcher.file_format("/data2.pson").should == "pson"
184
- it "should fail if the model does not support the file format" do
185
- @searcher.stubs(:latest_path).returns "/my/file.pson"
187
- @model.expects(:support_format?).with("pson").returns false
189
- lambda { @searcher.find(@request) }.should raise_error(ArgumentError)
193
- describe "when saving files" do
195
- @content = "my content"
196
- @file = stub 'file', :content => @content, :path => @path, :name => @path, :render => "mydata"
197
- @request.stubs(:instance).returns @file
200
- it "should provide a method to save file contents at a specified path" do
201
- @searcher.should respond_to(:save)
204
- it "should choose the file extension based on the default format of the model" do
205
- @model.expects(:default_format).returns "pson"
207
- @searcher.serialization_format.should == "pson"
210
- it "should place the file in the data directory, named after the indirection, key, and format" do
211
- @searcher.stubs(:data_directory).returns "/my/dir"
212
- @searcher.stubs(:serialization_format).returns "pson"
214
- @request.stubs(:key).returns "foo"
215
- @searcher.file_path(@request).should == File.join("/my/dir", "foo.pson")
218
- it "should fail intelligently if the file's parent directory does not exist" do
219
- @searcher.stubs(:file_path).returns "/my/dir/file.pson"
220
- @searcher.stubs(:serialization_format).returns "pson"
222
- @request.stubs(:key).returns "foo"
223
- File.expects(:directory?).with(File.join("/my/dir")).returns(false)
225
- proc { @searcher.save(@request) }.should raise_error(Puppet::Error)
228
- it "should render the instance using the file format and print it to the file path" do
229
- @searcher.stubs(:file_path).returns "/my/file.pson"
230
- @searcher.stubs(:serialization_format).returns "pson"
232
- File.stubs(:directory?).returns true
234
- @request.instance.expects(:render).with("pson").returns "data"
236
- fh = mock 'filehandle'
237
- File.expects(:open).with("/my/file.pson", "w").yields fh
238
- fh.expects(:print).with("data")
240
- @searcher.save(@request)
243
- it "should fail intelligently if a file cannot be written" do
244
- filehandle = mock 'file'
245
- File.stubs(:directory?).returns(true)
246
- File.stubs(:open).yields(filehandle)
247
- filehandle.expects(:print).raises(ArgumentError)
249
- @searcher.stubs(:file_path).returns "/my/file.pson"
250
- @model.stubs(:default_format).returns "pson"
252
- @instance.stubs(:render).returns "stuff"
254
- proc { @searcher.save(@request) }.should raise_error(Puppet::Error)
258
- describe "when removing files" do
259
- it "should provide a method to remove files" do
260
- @searcher.should respond_to(:destroy)
263
- it "should remove files in all formats found in the data directory that match the request key" do
264
- @searcher.stubs(:data_directory).returns "/my/dir"
265
- @request.stubs(:key).returns "me"
267
- Dir.expects(:glob).with(File.join("/my/dir", "me.*")).returns %w{/one /two}
269
- File.expects(:unlink).with("/one")
270
- File.expects(:unlink).with("/two")
272
- @searcher.destroy(@request)
275
- it "should throw an exception if no file is found" do
276
- @searcher.stubs(:data_directory).returns "/my/dir"
277
- @request.stubs(:key).returns "me"
279
- Dir.expects(:glob).with(File.join("/my/dir", "me.*")).returns []
281
- proc { @searcher.destroy(@request) }.should raise_error(Puppet::Error)
284
- it "should fail intelligently if a file cannot be removed" do
285
- @searcher.stubs(:data_directory).returns "/my/dir"
286
- @request.stubs(:key).returns "me"
288
- Dir.expects(:glob).with(File.join("/my/dir", "me.*")).returns %w{/one}
290
- File.expects(:unlink).with("/one").raises ArgumentError
292
- proc { @searcher.destroy(@request) }.should raise_error(Puppet::Error)
296
diff --git a/spec/unit/indirector/terminus_spec.rb b/spec/unit/indirector/terminus_spec.rb
297
index 2f37c1f..ccd6fd2 100755
298
--- a/spec/unit/indirector/terminus_spec.rb
299
+++ b/spec/unit/indirector/terminus_spec.rb
301
require 'spec_helper'
302
require 'puppet/defaults'
303
require 'puppet/indirector'
304
-require 'puppet/indirector/file'
305
+require 'puppet/indirector/memory'
307
describe Puppet::Indirector::Terminus, :'fails_on_ruby_1.9.2' => true do
309
@@ -201,14 +201,14 @@ describe Puppet::Indirector::Terminus, " when parsing class constants for indire
310
@subclass.expects(:indirection=).with(:test_ind)
311
@subclass.stubs(:name=)
312
@subclass.stubs(:terminus_type=)
313
- Puppet::Indirector::File.inherited(@subclass)
314
+ Puppet::Indirector::Memory.inherited(@subclass)
317
it "should convert the indirection name to a downcased symbol" do
318
@subclass.expects(:indirection=).with(:test_ind)
319
@subclass.stubs(:name=)
320
@subclass.stubs(:terminus_type=)
321
- Puppet::Indirector::File.inherited(@subclass)
322
+ Puppet::Indirector::Memory.inherited(@subclass)
325
it "should convert camel case to lower case with underscores as word separators" do
added
removed
debian/patches/secure-indirector-file-backed-terminus-base-cla.patch
