From 84f866ffafbb27a6aa6a1eea92c393a63829e242 Mon Sep 17 00:00:00 2001 From: Eli Young Date: Wed, 28 Jan 2015 15:28:54 -0800 Subject: (MODULES-1738) Don't modify global seed in fqdn_rotate() As per puppetlabs/puppet@292233c, this leaves the global seed in a deterministic state, which is bad. Puppet::Util.deterministic_rand() exists to avoid running into this issue, but is only present starting in Puppet 3.2.0. --- spec/functions/fqdn_rotate_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'spec/functions/fqdn_rotate_spec.rb') diff --git a/spec/functions/fqdn_rotate_spec.rb b/spec/functions/fqdn_rotate_spec.rb index 40057d4..673a8a3 100755 --- a/spec/functions/fqdn_rotate_spec.rb +++ b/spec/functions/fqdn_rotate_spec.rb @@ -40,4 +40,21 @@ describe "the fqdn_rotate function" do result = scope.function_fqdn_rotate([value]) result.size.should(eq(4)) end + + it "should use the Puppet::Util.deterministic_rand function if available" do + scope.expects(:lookupvar).with("::fqdn").returns("127.0.0.1") + if Puppet::Util.respond_to?(:deterministic_rand) + Puppet::Util.expects(:deterministic_rand).with(113646079810780526294648115052177588845,4) + end + scope.function_fqdn_rotate(["asdf"]) + end + + it "should not leave the global seed in a deterministic state" do + scope.expects(:lookupvar).with("::fqdn").returns("127.0.0.1").twice + scope.function_fqdn_rotate(["asdf"]) + rand1 = rand() + scope.function_fqdn_rotate(["asdf"]) + rand2 = rand() + expect(rand1).not_to eql(rand2) + end end -- cgit v1.2.3 From f3e79ddcd56a221c7799b35efde7e9803a5c7923 Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Mon, 1 Jun 2015 12:21:59 +0100 Subject: Convert tests to use plain rspec-puppet Tests in the new style produces the following documentation output: abs should not eq nil should run abs() and raise an Puppet::ParseError should run abs(-34) and return 34 should run abs("-34") and return 34 should run abs(34) and return 34 should run abs("34") and return 34 --- spec/functions/fqdn_rotate_spec.rb | 71 ++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 33 deletions(-) (limited to 'spec/functions/fqdn_rotate_spec.rb') diff --git a/spec/functions/fqdn_rotate_spec.rb b/spec/functions/fqdn_rotate_spec.rb index 673a8a3..fe54490 100755 --- a/spec/functions/fqdn_rotate_spec.rb +++ b/spec/functions/fqdn_rotate_spec.rb @@ -1,60 +1,65 @@ -#! /usr/bin/env ruby -S rspec require 'spec_helper' -describe "the fqdn_rotate function" do - let(:scope) { PuppetlabsSpec::PuppetInternals.scope } +describe 'fqdn_rotate' do + it { is_expected.not_to eq(nil) } + it { is_expected.to run.with_params().and_raise_error(Puppet::ParseError, /wrong number of arguments/i) } + it { is_expected.to run.with_params(0).and_raise_error(Puppet::ParseError, /Requires either array or string to work with/) } + it { is_expected.to run.with_params({}).and_raise_error(Puppet::ParseError, /Requires either array or string to work with/) } + it { + pending("Current implementation ignores parameters after the first.") + is_expected.to run.with_params("one", "two").and_raise_error(Puppet::ParseError) + } + it { is_expected.to run.with_params('').and_return('') } + it { is_expected.to run.with_params('a').and_return('a') } - it "should exist" do - expect(Puppet::Parser::Functions.function("fqdn_rotate")).to eq("function_fqdn_rotate") - end - - it "should raise a ParseError if there is less than 1 arguments" do - expect { scope.function_fqdn_rotate([]) }.to( raise_error(Puppet::ParseError)) - end + it { is_expected.to run.with_params([]).and_return([]) } + it { is_expected.to run.with_params(['a']).and_return(['a']) } it "should rotate a string and the result should be the same size" do - scope.expects(:lookupvar).with("::fqdn").returns("127.0.0.1") - result = scope.function_fqdn_rotate(["asdf"]) - expect(result.size).to(eq(4)) + expect(fqdn_rotate("asdf").size).to eq(4) end it "should rotate a string to give the same results for one host" do - scope.expects(:lookupvar).with("::fqdn").returns("127.0.0.1").twice - expect(scope.function_fqdn_rotate(["abcdefg"])).to eql(scope.function_fqdn_rotate(["abcdefg"])) + val1 = fqdn_rotate("abcdefg", :host => 'one') + val2 = fqdn_rotate("abcdefg", :host => 'one') + expect(val1).to eq(val2) end it "should rotate a string to give different values on different hosts" do - scope.expects(:lookupvar).with("::fqdn").returns("127.0.0.1") - val1 = scope.function_fqdn_rotate(["abcdefghijklmnopqrstuvwxyz01234567890987654321"]) - scope.expects(:lookupvar).with("::fqdn").returns("127.0.0.2") - val2 = scope.function_fqdn_rotate(["abcdefghijklmnopqrstuvwxyz01234567890987654321"]) - expect(val1).not_to eql(val2) + val1 = fqdn_rotate("abcdefg", :host => 'one') + val2 = fqdn_rotate("abcdefg", :host => 'two') + expect(val1).not_to eq(val2) end it "should accept objects which extend String" do - class AlsoString < String - end - - scope.expects(:lookupvar).with("::fqdn").returns("127.0.0.1") - value = AlsoString.new("asdf") - result = scope.function_fqdn_rotate([value]) - result.size.should(eq(4)) + result = fqdn_rotate(AlsoString.new('asdf')) + expect(result).to eq('dfas') end - it "should use the Puppet::Util.deterministic_rand function if available" do - scope.expects(:lookupvar).with("::fqdn").returns("127.0.0.1") + it "should use the Puppet::Util.deterministic_rand function" do if Puppet::Util.respond_to?(:deterministic_rand) Puppet::Util.expects(:deterministic_rand).with(113646079810780526294648115052177588845,4) + fqdn_rotate("asdf") + else + skip 'Puppet::Util#deterministic_rand not available' end - scope.function_fqdn_rotate(["asdf"]) end it "should not leave the global seed in a deterministic state" do - scope.expects(:lookupvar).with("::fqdn").returns("127.0.0.1").twice - scope.function_fqdn_rotate(["asdf"]) + fqdn_rotate("asdf") rand1 = rand() - scope.function_fqdn_rotate(["asdf"]) + fqdn_rotate("asdf") rand2 = rand() expect(rand1).not_to eql(rand2) end + + def fqdn_rotate(value, args = {}) + host = args[:host] || '127.0.0.1' + + # workaround not being able to use let(:facts) because some tests need + # multiple different hostnames in one context + scope.stubs(:lookupvar).with("::fqdn").returns(host) + + scope.function_fqdn_rotate([value]) + end end -- cgit v1.2.3 From 601f681787c8d6c02bb3566b8cefde289377be0e Mon Sep 17 00:00:00 2001 From: Eli Young Date: Thu, 28 May 2015 18:15:05 -0700 Subject: fqdn_rotate: Don't use the value itself as part of the random seed Previously, the random number generator was seeded with the array or string to be rotated in addition to any values specifically provided for seeding. This behavior is potentially insecure in that it allows an attacker who can modify the source data to choose the post-shuffle order. --- spec/functions/fqdn_rotate_spec.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'spec/functions/fqdn_rotate_spec.rb') diff --git a/spec/functions/fqdn_rotate_spec.rb b/spec/functions/fqdn_rotate_spec.rb index fe54490..6c76781 100755 --- a/spec/functions/fqdn_rotate_spec.rb +++ b/spec/functions/fqdn_rotate_spec.rb @@ -5,10 +5,6 @@ describe 'fqdn_rotate' do it { is_expected.to run.with_params().and_raise_error(Puppet::ParseError, /wrong number of arguments/i) } it { is_expected.to run.with_params(0).and_raise_error(Puppet::ParseError, /Requires either array or string to work with/) } it { is_expected.to run.with_params({}).and_raise_error(Puppet::ParseError, /Requires either array or string to work with/) } - it { - pending("Current implementation ignores parameters after the first.") - is_expected.to run.with_params("one", "two").and_raise_error(Puppet::ParseError) - } it { is_expected.to run.with_params('').and_return('') } it { is_expected.to run.with_params('a').and_return('a') } @@ -38,7 +34,7 @@ describe 'fqdn_rotate' do it "should use the Puppet::Util.deterministic_rand function" do if Puppet::Util.respond_to?(:deterministic_rand) - Puppet::Util.expects(:deterministic_rand).with(113646079810780526294648115052177588845,4) + Puppet::Util.expects(:deterministic_rand).with(44489829212339698569024999901561968770,4) fqdn_rotate("asdf") else skip 'Puppet::Util#deterministic_rand not available' -- cgit v1.2.3 From b436216fe68c11d67cc15aec83ce0f1eeb7ededf Mon Sep 17 00:00:00 2001 From: Eli Young Date: Mon, 1 Jun 2015 16:29:39 -0700 Subject: fqdn_rotate: Add tests for custom seeds --- spec/functions/fqdn_rotate_spec.rb | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'spec/functions/fqdn_rotate_spec.rb') diff --git a/spec/functions/fqdn_rotate_spec.rb b/spec/functions/fqdn_rotate_spec.rb index 6c76781..db7a717 100755 --- a/spec/functions/fqdn_rotate_spec.rb +++ b/spec/functions/fqdn_rotate_spec.rb @@ -21,6 +21,18 @@ describe 'fqdn_rotate' do expect(val1).to eq(val2) end + it "allows extra arguments to control the random rotation on a single host" do + val1 = fqdn_rotate("abcdefg", :extra_identifier => [1, "different", "host"]) + val2 = fqdn_rotate("abcdefg", :extra_identifier => [2, "different", "host"]) + expect(val1).not_to eq(val2) + end + + it "considers the same host and same extra arguments to have the same random rotation" do + val1 = fqdn_rotate("abcdefg", :extra_identifier => [1, "same", "host"]) + val2 = fqdn_rotate("abcdefg", :extra_identifier => [1, "same", "host"]) + expect(val1).to eq(val2) + end + it "should rotate a string to give different values on different hosts" do val1 = fqdn_rotate("abcdefg", :host => 'one') val2 = fqdn_rotate("abcdefg", :host => 'two') @@ -51,11 +63,13 @@ describe 'fqdn_rotate' do def fqdn_rotate(value, args = {}) host = args[:host] || '127.0.0.1' + extra = args[:extra_identifier] || [] # workaround not being able to use let(:facts) because some tests need # multiple different hostnames in one context scope.stubs(:lookupvar).with("::fqdn").returns(host) - scope.function_fqdn_rotate([value]) + function_args = [value] + extra + scope.function_fqdn_rotate(function_args) end end -- cgit v1.2.3