From 4575d5f2a98cc1d6d7be1f0cc957f514ffc4659b Mon Sep 17 00:00:00 2001 From: Dylan Cochran Date: Tue, 18 Jul 2017 12:38:35 -0400 Subject: [PATCH 01/11] (SIMP-3429) libkv::list isn't always removing keys libkv::atomic_list was not properly stripping / from the beginning of the path, and instead was stripping every /, breaking the regex. SIMP-3429 #close --- lib/puppet_x/libkv/consul_provider.rb | 2 +- spec/spec_helper.rb | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/puppet_x/libkv/consul_provider.rb b/lib/puppet_x/libkv/consul_provider.rb index 8de0e01..80e2f19 100644 --- a/lib/puppet_x/libkv/consul_provider.rb +++ b/lib/puppet_x/libkv/consul_provider.rb @@ -283,7 +283,7 @@ def atomic_list(params) if (last_char != "/") key = key + "/" end - reg = Regexp.new("^" + @basepath.gsub(/\//, "") + key) + reg = Regexp.new("^" + @basepath.gsub(/^\//, "") + key) unless (value == nil) value.each do |entry| nkey = entry["Key"].gsub(reg,"") diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 04474b9..87fddbb 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -255,6 +255,13 @@ def providers() # "softfail" => false, # "should_error" => false, # }, + { + "name" => "consul with ssl and without auth and with daemon and multiple path segments", + "url" => "consul+ssl+noverify://172.17.0.1:10501/puppet/default/", + "serialize" => true, + "softfail" => false, + "should_error" => false, + }, { "name" => "consul with ssl and without auth and with daemon", "url" => "consul+ssl+noverify://172.17.0.1:10501/puppet", From 58deeed1caca33f7e5e63d74905b6c0f59da4003 Mon Sep 17 00:00:00 2001 From: Dylan Cochran Date: Tue, 18 Jul 2017 13:45:21 -0400 Subject: [PATCH 02/11] (SIMP-3446) Add parameters to reocnfigure http and https listen Add libkv::consul::http_listen and libkv::consul::https_listen to reconfigure http and https listen addresses, for scenarios where the user will want to (ie fips and vagrant) SIMP-3446 #close --- data/common.yaml | 3 --- manifests/consul.pp | 14 ++++++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/data/common.yaml b/data/common.yaml index 7e1a0f3..4dd9daa 100644 --- a/data/common.yaml +++ b/data/common.yaml @@ -5,9 +5,6 @@ libkv::consul::puppet_cert_path: '/etc/puppetlabs/puppet/ssl' libkv::consul::config_hash: acl_datacenter: "dc1" acl_default_policy: "deny" - addresses: - http: '127.0.0.1' - https: '0.0.0.0' ports: https: 8501 http: 8500 diff --git a/manifests/consul.pp b/manifests/consul.pp index 05ce21e..9e535f8 100644 --- a/manifests/consul.pp +++ b/manifests/consul.pp @@ -10,6 +10,8 @@ $bootstrap = false, $dont_copy_files = false, $serverhost = undef, + $http_listen = '127.0.0.1', + $https_listen = '0.0.0.0', $advertise = undef, $datacenter = undef, $puppet_cert_path, @@ -203,10 +205,14 @@ # Use softfail to get around issues if the service isn't up $hash = lookup('consul::config_hash', { "default_value" => {} }) $class_hash = { - 'server' => $server, - 'node_name' => $::hostname, - 'retry_join' => [ $_serverhost ], - 'advertise_addr' => $_advertise, + 'server' => $server, + 'node_name' => $::hostname, + 'retry_join' => [ $_serverhost ], + 'advertise_addr' => $_advertise, + 'addresses' => { + 'http' => $http_listen, + 'https' => $https_listen, + }, } $merged_hash = $hash + $class_hash + $_datacenter + $config_hash + $_key_hash + $_token_hash + $_bootstrap_hash + $_cert_hash class { '::consul': From 315df532af82c33eca37b4c535b6daf995beb5b1 Mon Sep 17 00:00:00 2001 From: Dylan Cochran Date: Tue, 18 Jul 2017 15:20:59 -0400 Subject: [PATCH 03/11] Add some docs --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9d0a450..454bcd4 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,8 @@ libkv is an abstract library that allows puppet to access a distributed key value store, like consul or etcd. This library implements all the basic key/value primitives, get, put, list, delete. It also exposes any 'check and set' functionality the underlying store supports. This allows building of safe atomic operations, to build complex distributed systems. This library supports loading 'provider' modules that exist in other modules, and provides a first class api. +Each key specified must match the regex /^\/[a-zA-Z0-9._:\-\/]*$/; additionally, '/./' and '/../' are disallowed in all providers as key components. + When any libkv function is called, it will first call `lookup()` and attempt to find a value for libkv::url from hiera. This url specifies the provider name, the host, the port, and the path in the underlying store. For example: ```yaml libkv::url: 'consul://127.0.0.1:8500/puppet' @@ -47,7 +49,7 @@ libkv::url: 'etcd://127.0.0.1:2380/puppet/%{environment}/' libkv::url: 'consul://127.0.0.1:8500/puppet/%{trusted.extensions.pp_department}/%{environment}' ``` -Additionally libkv uses lookup to store authentication information. This information can range from ssl client certificates, access tokens, or usernames and passwords. The options are provider specific, so it's best +Additionally libkv uses lookup to store authentication information. This information can range from ssl client certificates, access tokens, or usernames and passwords. The options are provider specific, as are the authentication options available. libkv currently supports the following providers: From f72550edd86be0db92b6a21c80662b4aab9380b5 Mon Sep 17 00:00:00 2001 From: Dylan Cochran Date: Tue, 18 Jul 2017 15:23:57 -0400 Subject: [PATCH 04/11] Add some more docs --- README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/README.md b/README.md index 454bcd4..3e8fa5a 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,19 @@ libkv is an abstract library that allows puppet to access a distributed key value store, like consul or etcd. This library implements all the basic key/value primitives, get, put, list, delete. It also exposes any 'check and set' functionality the underlying store supports. This allows building of safe atomic operations, to build complex distributed systems. This library supports loading 'provider' modules that exist in other modules, and provides a first class api. +For example, you can use the following to store hostnames, and then read all the known hostnames from consul and generate a hosts file: + +```puppet +libkv::put("/hosts/${fqdn}", $::ipaddress) + +$hosts = libkv::list("/hosts") +$hosts.each |$host, $ip | { + host { $host: + ip => $ip, + } +} +``` + Each key specified must match the regex /^\/[a-zA-Z0-9._:\-\/]*$/; additionally, '/./' and '/../' are disallowed in all providers as key components. When any libkv function is called, it will first call `lookup()` and attempt to find a value for libkv::url from hiera. This url specifies the provider name, the host, the port, and the path in the underlying store. For example: From 3c4805524f2dea656749a1491038666afd343a60 Mon Sep 17 00:00:00 2001 From: Dylan Cochran Date: Tue, 18 Jul 2017 15:24:47 -0400 Subject: [PATCH 05/11] Fix fact --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3e8fa5a..9f1bfe7 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ libkv is an abstract library that allows puppet to access a distributed key valu For example, you can use the following to store hostnames, and then read all the known hostnames from consul and generate a hosts file: ```puppet -libkv::put("/hosts/${fqdn}", $::ipaddress) +libkv::put("/hosts/${::clientcert}", $::ipaddress) $hosts = libkv::list("/hosts") $hosts.each |$host, $ip | { From 58ea6888aa1a8296a55ad270bff1b24cc56b92ec Mon Sep 17 00:00:00 2001 From: Dylan Cochran Date: Tue, 18 Jul 2017 15:26:58 -0400 Subject: [PATCH 06/11] Clarify key names --- README.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9f1bfe7..9e2f627 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,17 @@ $hosts.each |$host, $ip | { } ``` -Each key specified must match the regex /^\/[a-zA-Z0-9._:\-\/]*$/; additionally, '/./' and '/../' are disallowed in all providers as key components. +Each key specified *must* contain only the following characters: +* a-z +* A-Z +* 0-9 +* . +* _ +* : +* - +* / + +Additionally, '/./' and '/../' are disallowed in all providers as key components. The key name also *must* begin with '/' When any libkv function is called, it will first call `lookup()` and attempt to find a value for libkv::url from hiera. This url specifies the provider name, the host, the port, and the path in the underlying store. For example: ```yaml From 4d55c1352fa31a45788cd9cc2e48726274dca629 Mon Sep 17 00:00:00 2001 From: Dylan Cochran Date: Tue, 18 Jul 2017 15:27:51 -0400 Subject: [PATCH 07/11] Add special chars --- README.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/README.md b/README.md index 9e2f627..1ec0357 100644 --- a/README.md +++ b/README.md @@ -55,11 +55,7 @@ Each key specified *must* contain only the following characters: * a-z * A-Z * 0-9 -* . -* _ -* : -* - -* / +* The following special characters: ._:-/ Additionally, '/./' and '/../' are disallowed in all providers as key components. The key name also *must* begin with '/' From 5137f34ed8b6f9f31689ad3fc481b30b295eade1 Mon Sep 17 00:00:00 2001 From: Dylan Cochran Date: Tue, 18 Jul 2017 15:30:03 -0400 Subject: [PATCH 08/11] Clarify libkv::auth --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1ec0357..8ea6e1d 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ libkv::url: 'etcd://127.0.0.1:2380/puppet/%{environment}/' libkv::url: 'consul://127.0.0.1:8500/puppet/%{trusted.extensions.pp_department}/%{environment}' ``` -Additionally libkv uses lookup to store authentication information. This information can range from ssl client certificates, access tokens, or usernames and passwords. The options are provider specific, as are the authentication options available. +Additionally libkv uses lookup to store authentication information. This information can range from ssl client certificates, access tokens, or usernames and passwords. It is exposed as a hash named libkv::auth, and will be merged by default. The keys in the auth token are passed as is to the provider, and can vary between providers. Please read the documentation on configuring 'libkv::auth' for each provider libkv currently supports the following providers: From b5d0b71e1304131e3cdd731aab0b16c16d80ae66 Mon Sep 17 00:00:00 2001 From: Dylan Cochran Date: Tue, 18 Jul 2017 15:32:00 -0400 Subject: [PATCH 09/11] Fix gen script --- scripts/readme.erb | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/scripts/readme.erb b/scripts/readme.erb index 1739fe1..65eb1d1 100644 --- a/scripts/readme.erb +++ b/scripts/readme.erb @@ -20,6 +20,27 @@ libkv is an abstract library that allows puppet to access a distributed key value store, like consul or etcd. This library implements all the basic key/value primitives, get, put, list, delete. It also exposes any 'check and set' functionality the underlying store supports. This allows building of safe atomic operations, to build complex distributed systems. This library supports loading 'provider' modules that exist in other modules, and provides a first class api. +For example, you can use the following to store hostnames, and then read all the known hostnames from consul and generate a hosts file: + +```puppet +libkv::put("/hosts/${::clientcert}", $::ipaddress) + +$hosts = libkv::list("/hosts") +$hosts.each |$host, $ip | { + host { $host: + ip => $ip, + } +} +``` + +Each key specified *must* contain only the following characters: +* a-z +* A-Z +* 0-9 +* The following special characters: ._:-/ + +Additionally, '/./' and '/../' are disallowed in all providers as key components. The key name also *must* begin with '/' + When any libkv function is called, it will first call `lookup()` and attempt to find a value for libkv::url from hiera. This url specifies the provider name, the host, the port, and the path in the underlying store. For example: ```yaml libkv::url: 'consul://127.0.0.1:8500/puppet' @@ -29,7 +50,7 @@ libkv::url: 'etcd://127.0.0.1:2380/puppet/%{environment}/' libkv::url: 'consul://127.0.0.1:8500/puppet/%{trusted.extensions.pp_department}/%{environment}' ``` -Additionally libkv uses lookup to store authentication information. This information can range from ssl client certificates, access tokens, or usernames and passwords. The options are provider specific, so it's best +Additionally libkv uses lookup to store authentication information. This information can range from ssl client certificates, access tokens, or usernames and passwords. It is exposed as a hash named libkv::auth, and will be merged by default. The keys in the auth token are passed as is to the provider, and can vary between providers. Please read the documentation on configuring 'libkv::auth' for each provider libkv currently supports the following providers: From f7f7485dc4ae7728c15b502e3bf739b82e9460fd Mon Sep 17 00:00:00 2001 From: Dylan Cochran Date: Tue, 18 Jul 2017 14:51:26 -0400 Subject: [PATCH 10/11] (SIMP-3001) Prevent '.' and '..' from being used in keys Check for uses of /./ or /../ in key names, to prevent potential relative pathing attacks for providers that support relative links, particularly file based providers SIMP-3001 #close --- lib/puppet_x/libkv/libkv.rb | 10 +++++++--- spec/functions/libkv_get_spec.rb | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/lib/puppet_x/libkv/libkv.rb b/lib/puppet_x/libkv/libkv.rb index d19148c..ff3bd81 100644 --- a/lib/puppet_x/libkv/libkv.rb +++ b/lib/puppet_x/libkv/libkv.rb @@ -110,6 +110,10 @@ def sanitize_input(symbol, params) unless (regex =~ params[name]) raise error_msg end + dot_regex = /\/\.\.*\// + if (dot_regex =~ params[name]) + raise "the value of '#{name}': '#{params[name]}' contains ./ or ../, which is invalid in a key name" + end else unless (params[name].class.to_s == definition) @@ -130,7 +134,7 @@ def method_missing(symbol, url, auth, *args, &block) # if (params['dd'] == true) # binding.pry # end - + # Pre-provider mangling unless (params.key?("serialize")) params["serialize"] = true @@ -246,11 +250,11 @@ def delete_metadata(params, object) meta[0] = params.dup meta[0]["key"] = "#{params['key']}.meta" begin - metadata = object.send(:delete, *meta) + metadata = object.send(:delete, *meta) rescue end end - + def get_metadata(params, object) meta = [] meta[0] = params.dup diff --git a/spec/functions/libkv_get_spec.rb b/spec/functions/libkv_get_spec.rb index cc315a7..f78236b 100644 --- a/spec/functions/libkv_get_spec.rb +++ b/spec/functions/libkv_get_spec.rb @@ -30,7 +30,23 @@ result = subject.execute(params) expect(result).to eql(nil) end + it "should throw an exception if the key contains '..' as a component" do + params = { + 'key' => "/get/directory1/test", + 'value' => "directory1", + }.merge(shared_params) + call_function("libkv::put", params) + params = { + 'key' => "/get/directory2/test", + 'value' => "directory2", + }.merge(shared_params) + call_function("libkv::put", params) + params = { + 'key' => "/get/directory1/../directory2/test", + }.merge(shared_params) + is_expected.to run.with_params(params).and_raise_error(Exception); + end datatype_testspec.each do |hash| if (providerinfo["serialize"] == true) klass = hash[:class] From d10f009690b2c2e1272f01bebbf19c3910d4f357 Mon Sep 17 00:00:00 2001 From: Dylan Cochran Date: Tue, 18 Jul 2017 15:35:39 -0400 Subject: [PATCH 11/11] Release 0.4.2 --- CHANGELOG | 5 +++++ metadata.json | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 44d73cc..74fd5ec 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,8 @@ +* Tue Jul 18 2017 Dylan Cochran - 0.4.2 +- (SIMP-3001) Prevent '.' and '..' from being used in keys +- (SIMP-3446) Add parameters to reconfigure http and https listen +- (SIMP-3429) libkv::list isn't always removing keys + * Tue Jul 18 2017 Dylan Cochran - 0.4.1 - Always copy over consul-acl, and update metadata diff --git a/metadata.json b/metadata.json index 10c9de9..e5e37a9 100644 --- a/metadata.json +++ b/metadata.json @@ -1,6 +1,6 @@ { "name": "simp-libkv", - "version": "0.4.1", + "version": "0.4.2", "author": "simp", "summary": "", "license": "Apache-2.0",