From e7ef1532b35a9bd003c6dcaeb1f3e399a72053a5 Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Fri, 22 Jun 2012 15:56:11 -0700 Subject: [PATCH] Always use the local file_bucket on master This fixes an issue where the master could be made to issue arbitrary HTTPS requests through carefully constructed URLs. Now the master will always use the file_bucket, whereas other applications retain their behavior of dynamically selecting the source (rest or file) based on the particular request. --- lib/puppet/application/master.rb | 2 + lib/puppet/file_bucket/file.rb | 3 +- .../file_bucket/file/indirection_hooks.rb | 9 ---- .../indirector/file_bucket_file/selector.rb | 49 +++++++++++++++++++ spec/integration/file_bucket/file_spec.rb | 44 +++++++++++++++++ .../file_bucket_file/selector_spec.rb | 29 +++++++++++ 6 files changed, 125 insertions(+), 11 deletions(-) delete mode 100644 lib/puppet/file_bucket/file/indirection_hooks.rb create mode 100644 lib/puppet/indirector/file_bucket_file/selector.rb create mode 100644 spec/integration/file_bucket/file_spec.rb create mode 100644 spec/unit/indirector/file_bucket_file/selector_spec.rb diff --git a/lib/puppet/application/master.rb b/lib/puppet/application/master.rb index 9c344857345..020a942f50e 100644 --- a/lib/puppet/application/master.rb +++ b/lib/puppet/application/master.rb @@ -144,6 +144,8 @@ def setup Puppet::FileServing::Content.indirection.terminus_class = :file_server Puppet::FileServing::Metadata.indirection.terminus_class = :file_server + Puppet::FileBucket::File.indirection.terminus_class = :file + # Configure all of the SSL stuff. if Puppet::SSL::CertificateAuthority.ca? Puppet::SSL::Host.ca_location = :local diff --git a/lib/puppet/file_bucket/file.rb b/lib/puppet/file_bucket/file.rb index 08c0329f173..f9d25218ae5 100644 --- a/lib/puppet/file_bucket/file.rb +++ b/lib/puppet/file_bucket/file.rb @@ -8,8 +8,7 @@ class Puppet::FileBucket::File # There are mechanisms to save and load this file locally and remotely in puppet/indirector/filebucketfile/* # There is a compatibility class that emulates pre-indirector filebuckets in Puppet::FileBucket::Dipper extend Puppet::Indirector - require 'puppet/file_bucket/file/indirection_hooks' - indirects :file_bucket_file, :terminus_class => :file, :extend => Puppet::FileBucket::File::IndirectionHooks + indirects :file_bucket_file, :terminus_class => :selector attr :contents attr :bucket_path diff --git a/lib/puppet/file_bucket/file/indirection_hooks.rb b/lib/puppet/file_bucket/file/indirection_hooks.rb deleted file mode 100644 index 58c292745da..00000000000 --- a/lib/puppet/file_bucket/file/indirection_hooks.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'puppet/file_bucket/file' - -# This module is used to pick the appropriate terminus -# in filebucket indirections. -module Puppet::FileBucket::File::IndirectionHooks - def select_terminus(request) - return(request.protocol == 'https' ? :rest : Puppet::FileBucket::File.indirection.terminus_class) - end -end diff --git a/lib/puppet/indirector/file_bucket_file/selector.rb b/lib/puppet/indirector/file_bucket_file/selector.rb new file mode 100644 index 00000000000..51fc7711f45 --- /dev/null +++ b/lib/puppet/indirector/file_bucket_file/selector.rb @@ -0,0 +1,49 @@ +require 'puppet/indirector/code' + +module Puppet::FileBucketFile + class Selector < Puppet::Indirector::Code + desc "Select the terminus based on the request" + + def select(request) + if request.protocol == 'https' + :rest + else + :file + end + end + + def get_terminus(request) + indirection.terminus(select(request)) + end + + def head(request) + get_terminus(request).head(request) + end + + def find(request) + get_terminus(request).find(request) + end + + def save(request) + get_terminus(request).save(request) + end + + def search(request) + get_terminus(request).search(request) + end + + def destroy(request) + get_terminus(request).destroy(request) + end + + def authorized?(request) + terminus = get_terminus(request) + if terminus.respond_to?(:authorized?) + terminus.authorized?(request) + else + true + end + end + end +end + diff --git a/spec/integration/file_bucket/file_spec.rb b/spec/integration/file_bucket/file_spec.rb new file mode 100644 index 00000000000..2f4010e417a --- /dev/null +++ b/spec/integration/file_bucket/file_spec.rb @@ -0,0 +1,44 @@ +#!/usr/bin/env rspec + +require 'spec_helper' + +require 'puppet/file_bucket/file' + +describe Puppet::FileBucket::File do + describe "#indirection" do + before :each do + # Never connect to the network, no matter what + described_class.indirection.terminus(:rest).class.any_instance.stubs(:find) + end + + describe "when running the master application" do + before :each do + Puppet::Application[:master].setup_terminuses + end + + { + "md5/d41d8cd98f00b204e9800998ecf8427e" => :file, + "https://puppetmaster:8140/production/file_bucket_file/md5/d41d8cd98f00b204e9800998ecf8427e" => :file, + }.each do |key, terminus| + it "should use the #{terminus} terminus when requesting #{key.inspect}" do + described_class.indirection.terminus(terminus).class.any_instance.expects(:find) + + described_class.indirection.find(key) + end + end + end + + describe "when running another application" do + { + "md5/d41d8cd98f00b204e9800998ecf8427e" => :file, + "https://puppetmaster:8140/production/file_bucket_file/md5/d41d8cd98f00b204e9800998ecf8427e" => :rest, + }.each do |key, terminus| + it "should use the #{terminus} terminus when requesting #{key.inspect}" do + described_class.indirection.terminus(terminus).class.any_instance.expects(:find) + + described_class.indirection.find(key) + end + end + end + end +end diff --git a/spec/unit/indirector/file_bucket_file/selector_spec.rb b/spec/unit/indirector/file_bucket_file/selector_spec.rb new file mode 100644 index 00000000000..31441be6d92 --- /dev/null +++ b/spec/unit/indirector/file_bucket_file/selector_spec.rb @@ -0,0 +1,29 @@ +#!/usr/bin/env rspec +require 'spec_helper' + +require 'puppet/indirector/file_bucket_file/selector' +require 'puppet/indirector/file_bucket_file/file' +require 'puppet/indirector/file_bucket_file/rest' + +describe Puppet::FileBucketFile::Selector do + %w[head find save search destroy].each do |method| + describe "##{method}" do + it "should proxy to rest terminus for https requests" do + request = stub 'request', :protocol => 'https' + + Puppet::FileBucketFile::Rest.any_instance.expects(method).with(request) + + subject.send(method, request) + end + + it "should proxy to file terminus for other requests" do + request = stub 'request', :protocol => 'file' + + Puppet::FileBucketFile::File.any_instance.expects(method).with(request) + + subject.send(method, request) + end + end + end +end +