From 87434ecc3d3f5fcb59c148de6f11fe1add043a3d Mon Sep 17 00:00:00 2001 From: Tyler Cloke Date: Mon, 11 Apr 2016 10:00:49 -0700 Subject: [PATCH 1/2] Add Chef Server 12.5 support and decreased surface area with Chef dependenices. The following changes / improvements were made: + Added support for the org scoped users key endpoint introduced in Chef Server 12.5.0. This will fix issues related to clients not being able to load users due to permissions errors. + Removed chef_patch classes in favor of a ChefVault::ChefKey interface with less surface area. Since all we want is the key, this simplifies the interface to Chef. It attempts to hit the org scoped user and client endpoints, and if it can't find them, uses Chef::ApiClient and Chef::User to hit API V0 and load the keys directly from the objects as it traditionally works (but without exposing the entire object). This means, Chef Vault should support all Chef Server 12 versions, as well as some Chef Server 11 versions. However, you will still hit the client loading user permissions issue if you are hitting a Chef Server before 12.5.0. + Better error messages related to user versioning errors and how to fix them. + Better test coverage and overall cleaner interfaces. --- .travis.yml | 1 - README.md | 53 ++++++ chef-vault.gemspec | 9 +- features/vault_create.feature | 2 +- hooks/pre-commit | 43 +++++ lib/chef-vault.rb | 4 +- lib/chef-vault/chef_api.rb | 39 ++++ lib/chef-vault/chef_key.rb | 149 ++++++++++++++++ lib/chef-vault/chef_patch/api_client.rb | 45 ----- lib/chef-vault/chef_patch/user.rb | 33 ---- lib/chef-vault/item.rb | 121 ++++++------- lib/chef-vault/item_keys.rb | 23 ++- lib/chef/knife/vault_show.rb | 8 +- lib/chef/knife/vault_update.rb | 4 +- spec/chef-vault/chef_api_spec.rb | 39 ++++ spec/chef-vault/chef_key_spec.rb | 227 ++++++++++++++++++++++++ spec/chef-vault/item_keys_spec.rb | 52 ++++++ spec/chef-vault/item_spec.rb | 169 ++++++++++-------- 18 files changed, 775 insertions(+), 246 deletions(-) create mode 100755 hooks/pre-commit create mode 100644 lib/chef-vault/chef_api.rb create mode 100644 lib/chef-vault/chef_key.rb delete mode 100644 lib/chef-vault/chef_patch/api_client.rb delete mode 100644 lib/chef-vault/chef_patch/user.rb create mode 100644 spec/chef-vault/chef_api_spec.rb create mode 100644 spec/chef-vault/chef_key_spec.rb diff --git a/.travis.yml b/.travis.yml index 9b11085..e7028fa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,6 @@ branches: - master rvm: - "1.9.3-p551" - - "2.0.0-p647" - "2.1.6" - "2.2.2" install: bundle install --binstubs diff --git a/README.md b/README.md index 0673329..96ac025 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,59 @@ This plugin is distributed as a Ruby Gem. To install it, run: Depending on your system's configuration, you may need to run this command with root privileges. +## DEVELOPMENT: + +### Running Your Changes + +To run your changes locally: + +``` +bundle install +bundle exec knife vault +``` + +### Testing + +#### Rspec Tests + +There are some unit tests that can be run with: + +``` +bundle exec rspec spec/ +``` + +#### Cucumber Testing + +There are cucumber tests. Run the whole suite with: + +``` +bundle exec rake features +``` + +If you get any failures, you can run the specific feature that failed with: + +``` +bundle exec cucumber features/.feature +``` + +If you want to test things out directly, after a failure you can go into the test +directory and try out the commands that failed: + +``` +cd tmp/aruba +bundle exec knife +``` + +Optionally add `-VV` to the above to get a full stacktrace. + +### Rubocop Errors + +If you are seeing rubocop errors in travis for your pull request, run: + +`bundle exec chefstyle -a` + +This will fix up your rubocop errors automatically, and warn you about any it can't. + ## KNIFE COMMANDS: See KNIFE_EXAMPLES.md for examples of commands diff --git a/chef-vault.gemspec b/chef-vault.gemspec index 3fd39d5..b53d6a9 100644 --- a/chef-vault.gemspec +++ b/chef-vault.gemspec @@ -39,14 +39,17 @@ Gem::Specification.new do |s| s.add_development_dependency "aruba", "~> 0.6" s.add_development_dependency "simplecov", "~> 0.9" s.add_development_dependency "simplecov-console", "~> 0.2" - s.add_development_dependency "rubocop", "~> 0.30" # Chef 12 and higher pull in Ohai 8, which needs Ruby v2 # so only in the case of a CI build on ruby v1, we constrain # chef to 11 or lower so that we can maintain CI test coverage # of older versions if ENV.key?("TRAVIS_BUILD") && RUBY_VERSION =~ /^1/ s.add_development_dependency "chef", "~> 11.18" - else - s.add_development_dependency "chef", ">= 0.10.10" + elsif ENV.key?("TRAVIS_BUILD") && RUBY_VERSION == "2.1.6" + # Test version of Chef with Chef Zero before + # /orgs/org/users/user/keys endpoint was added. + s.add_development_dependency "chef", "12.8.1" + else # Test most current version of Chef on 2.2.2 + s.add_development_dependency "chef" end end diff --git a/features/vault_create.feature b/features/vault_create.feature index fe3387c..6fb1868 100644 --- a/features/vault_create.feature +++ b/features/vault_create.feature @@ -51,4 +51,4 @@ Feature: knife vault create Given a local mode chef repo with nodes 'one,two' And I create a vault item 'test/item' containing the JSON '{"foo": "bar"}' encrypted for 'one,two,three' with 'alice' as admin Then the exit status should not be 0 - And the output should contain "FATAL: Could not find alice in users or clients!" + And the output should contain "FATAL: Could not find default key for alice in users or clients!" diff --git a/hooks/pre-commit b/hooks/pre-commit new file mode 100755 index 0000000..31e6c3d --- /dev/null +++ b/hooks/pre-commit @@ -0,0 +1,43 @@ +#!/usr/bin/env ruby +output = `bundle exec chefstyle -a` +if !$?.success? + puts "pre-commit hook: Tried to run `bundle exec chefstyle -a` to autocleanup errors, but it failed with output:" + puts output +end + +detected = /(\d+) offenses detected/.match(output) +corrected = /(\d+) offenses corrected/.match(output) + +# no errors detected by chefstyle +exit 0 if detected.nil? + +# chefstyle found errors +if !detected.nil? + # get the first result from the capture group that isn't the whole capture + num_detected = detected.to_a[1].to_i + num_corrected = if corrected.nil? + 0 + else + corrected.to_a[1].to_i + end + if num_detected == num_corrected + puts < +# Copyright:: Copyright 2016, Chef Software, Inc. +# License:: Apache License, Version 2.0 + +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at + +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require "chef/server_api" + +class ChefVault + class ChefApi + + def rest_v0 + @rest_v0 ||= Chef::ServerAPI.new(Chef::Config[:chef_server_root], { :api_version => "0" }) + end + + def rest_v1 + @rest_v1 ||= Chef::ServerAPI.new(Chef::Config[:chef_server_root], { :api_version => "1" }) + end + + def org_scoped_rest_v0 + @org_scoped_rest_v0 ||= Chef::ServerAPI.new(Chef::Config[:chef_server_url], { :api_version => "0" }) + end + + def org_scoped_rest_v1 + @org_scoped_rest_v1 ||= Chef::ServerAPI.new(Chef::Config[:chef_server_url], { :api_version => "1" }) + end + + end +end diff --git a/lib/chef-vault/chef_key.rb b/lib/chef-vault/chef_key.rb new file mode 100644 index 0000000..047b8a9 --- /dev/null +++ b/lib/chef-vault/chef_key.rb @@ -0,0 +1,149 @@ +# Author:: Tyler Cloke +# Copyright:: Copyright 2016, Chef Software, Inc. +# License:: Apache License, Version 2.0 + +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at + +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require "json" + +class ChefVault + class ChefKey + + attr_accessor :key_string + attr_reader :actor_type + attr_reader :actor_name + + def initialize(actor_type, actor_name) + if actor_type != "clients" && actor_type != "admins" + raise "You must pass either 'clients' or 'admins' as the first argument to ChefVault::ChefKey.new." + end + @actor_type = actor_type + @actor_name = actor_name + end + + def key + @key ||= is_admin? ? get_admin_key : get_client_key + end + + def get_admin_key + # chef vault currently only supports using the default key + get_key("users") + rescue Net::HTTPServerException => http_error + # if we failed to find an admin key, attempt to load a client key by the same name + case http_error.response.code + when "403" + print_forbidden_error + raise http_error + when "404" + begin + $stdout.puts "WARNING: The default key for #{actor_name} not found in users, trying client keys." + get_key("clients") + rescue Net::HTTPServerException => http_error + case http_error.response.code + when "404" + raise ChefVault::Exceptions::AdminNotFound, + "FATAL: Could not find default key for #{actor_name} in users or clients!" + when "403" + print_forbidden_error + raise http_error + else + raise http_error + end + end + else + raise http_error + end + end + + def get_client_key + begin + get_key("clients") + rescue Net::HTTPServerException => http_error + if http_error.response.code == "403" + print_forbidden_error + raise http_error + elsif http_error.response.code == "404" + raise ChefVault::Exceptions::ClientNotFound, + "#{actor_name} is not a valid chef client and/or node" + else + raise http_error + end + end + end + + def is_client? + actor_type == "clients" + end + + def is_admin? + actor_type == "admins" + end + + # @private + + def api + @api ||= ChefVault::ChefApi.new + end + + # Use API V0 to load the public_key directly from the user object + # using the chef-client code. + def chef_api_client + @chef_api_client ||= begin + require "chef/api_client" + Chef::ApiClient + end + end + + # Similar thing as above but for client. + def chef_user + @chef_user ||= begin + require "chef/user" + Chef::User + end + end + + def get_key(request_actor_type) + api.org_scoped_rest_v1.get("#{request_actor_type}/#{actor_name}/keys/default")["public_key"] + # If the keys endpoint doesn't exist, try getting it directly from the V0 chef object. + rescue Net::HTTPServerException => http_error + raise http_error unless http_error.response.code == "404" + if request_actor_type == "clients" + chef_api_client.load(actor_name).public_key + else + user = chef_user.load(actor_name).public_key + end + end + + def print_forbidden_error + $stdout.puts <= 12.5 or make this request using a user. + +If you are on Chef Server == 12.5.0 + All clients and users have access to the public keys endpoint. Getting + this error on 12.5.0 is unexpected regardless of what your + public_key_read_access_group contains. + +If you are on Chef Server > 12.5.1 + Has your public_key_read_access_group been modified? This group controls + read access on public keys within your org. It defaults to the users + and client groups, so all org actors should have permission unless + the defaults have been changed. + +EOF + end + end +end diff --git a/lib/chef-vault/chef_patch/api_client.rb b/lib/chef-vault/chef_patch/api_client.rb deleted file mode 100644 index 138dac2..0000000 --- a/lib/chef-vault/chef_patch/api_client.rb +++ /dev/null @@ -1,45 +0,0 @@ -# Author:: Kevin Moser -# Copyright:: Copyright 2013, Nordstrom, Inc. -# License:: Apache License, Version 2.0 - -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at - -# http://www.apache.org/licenses/LICENSE-2.0 - -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -class ChefVault - module ChefPatch - class ApiClient < Chef::ApiClient - # Fix an issue where core Chef::ApiClient does not load - # the private key for Chef 10 - def self.load(name) - response = http_api.get("clients/#{name}") - if response.is_a?(Chef::ApiClient) - response - else - client = Chef::ApiClient.new - client.name(response["clientname"] || response["name"]) - - if response["certificate"] - der = OpenSSL::X509::Certificate.new response["certificate"] - client.public_key der.public_key.to_s - end - - if response["public_key"] - der = OpenSSL::PKey::RSA.new response["public_key"] - client.public_key der.public_key.to_s - end - - client - end - end - end - end -end diff --git a/lib/chef-vault/chef_patch/user.rb b/lib/chef-vault/chef_patch/user.rb deleted file mode 100644 index 66f1ff2..0000000 --- a/lib/chef-vault/chef_patch/user.rb +++ /dev/null @@ -1,33 +0,0 @@ -# Author:: Kevin Moser -# Copyright:: Copyright 2013, Nordstrom, Inc. -# License:: Apache License, Version 2.0 - -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at - -# http://www.apache.org/licenses/LICENSE-2.0 - -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -class ChefVault - module ChefPatch - class User < Chef::User - # def from_hash for our implementation because name is not being - # set correctly for Chef 10 server - def superclass.from_hash(user_hash) - user = Chef::User.new - user.name user_hash["username"] ? user_hash["username"] : user_hash["name"] - user.private_key user_hash["private_key"] if user_hash.key?("private_key") - user.password user_hash["password"] if user_hash.key?("password") - user.public_key user_hash["public_key"] - user.admin user_hash["admin"] - user - end - end - end -end diff --git a/lib/chef-vault/item.rb b/lib/chef-vault/item.rb index 864a06c..0c1971c 100644 --- a/lib/chef-vault/item.rb +++ b/lib/chef-vault/item.rb @@ -69,15 +69,16 @@ def initialize(vault, name, opts = {}) @client_key_path = opts[:client_key_path] end + # private def load_keys(vault, keys) @keys = ChefVault::ItemKeys.load(vault, keys) @secret = secret end - def clients(search_or_client = nil, action = :add) + def clients(search_or_client, action = :add) if search_or_client.is_a?(Chef::ApiClient) handle_client_action(search_or_client, action) - elsif search_or_client + else results_returned = false query = Chef::Search::Query.new query.search(:node, search_or_client) do |node| @@ -85,8 +86,8 @@ def clients(search_or_client = nil, action = :add) case action when :add begin - client = load_client(node.name) - add_client(client) + client_key = load_public_key(node.name, "clients") + add_client(client_key) rescue ChefVault::Exceptions::ClientNotFound $stdout.puts "node '#{node.name}' has no private key; skipping" end @@ -94,19 +95,21 @@ def clients(search_or_client = nil, action = :add) delete_client_or_node(node) else raise ChefVault::Exceptions::KeysActionNotValid, - "#{action} is not a valid action" + "#{action} is not a valid action" end end unless results_returned $stdout.puts "WARNING: No clients were returned from search, you may not have "\ - "got what you expected!!" + "got what you expected!!" end - else - keys.clients end end + def get_clients + keys.clients + end + def search(search_query = nil) if search_query keys.search_query(search_query) @@ -115,25 +118,26 @@ def search(search_query = nil) end end - def admins(admins = nil, action = :add) - if admins - admins.split(",").each do |admin| - admin.strip! - case action - when :add - keys.add(load_admin(admin), @secret, "admins") - when :delete - keys.delete(admin, "admins") - else - raise ChefVault::Exceptions::KeysActionNotValid, - "#{action} is not a valid action" - end + def admins(admin_string, action = :add) + admin_string.split(",").each do |admin| + admin.strip! + admin_key = load_public_key(admin, "admins") + case action + when :add + keys.add(admin_key, @secret) + when :delete + keys.delete(admin_key) + else + raise ChefVault::Exceptions::KeysActionNotValid, + "#{action} is not a valid action" end - else - keys.admins end end + def get_admins + keys.admins + end + def remove(key) @raw_data.delete(key) end @@ -158,19 +162,19 @@ def secret def rotate_keys!(clean_unknown_clients = false) @secret = generate_secret - unless clients.empty? + unless get_clients.empty? # a bit of a misnomer; this doesn't remove unknown # admins, just clients which are nodes remove_unknown_nodes if clean_unknown_clients # re-encrypt the new shared secret for all remaining clients - clients.each do |client| + get_clients.each do |client| clients("name:#{client}") end end - unless admins.empty? + unless get_admins.empty? # re-encrypt the new shared secret for all admins - admins.each do |admin| + get_admins.each do |admin| admins(admin) end end @@ -179,6 +183,7 @@ def rotate_keys!(clean_unknown_clients = false) reload_raw_data end + # private def generate_secret(key_size = 32) # Defaults to 32 bytes, as this is the size that a Chef # Encrypted Data Bag Item will digest all secrets down to anyway @@ -285,6 +290,11 @@ def self.load(vault, name, opts = {}) item end + def delete_client(client_name) + client_key = load_public_key(client_name, "clients") + keys.delete(client_key) + end + # determines if a data bag item looks like a vault # @param vault [String] the name of the data bag # @param name [String] the name of the item in the data bag @@ -368,39 +378,8 @@ def reload_raw_data @raw_data end - def load_admin(admin) - begin - admin = ChefVault::ChefPatch::User.load(admin) - rescue Net::HTTPServerException => http_error - if http_error.response.code == "404" - begin - $stdout.puts "WARNING: #{admin} not found in users, trying clients." - admin = load_client(admin) - rescue ChefVault::Exceptions::ClientNotFound - raise ChefVault::Exceptions::AdminNotFound, - "FATAL: Could not find #{admin} in users or clients!" - end - else - raise http_error - end - end - - admin - end - - def load_client(client) - begin - client = ChefVault::ChefPatch::ApiClient.load(client) - rescue Net::HTTPServerException => http_error - if http_error.response.code == "404" - raise ChefVault::Exceptions::ClientNotFound, - "#{client} is not a valid chef client and/or node" - else - raise http_error - end - end - - client + def load_public_key(actor_name, type) + ChefVault::ChefKey.new(type, actor_name) end # removes unknown nodes by performing a node search @@ -412,13 +391,13 @@ def remove_unknown_nodes # build a list of clients to remove so we don't # mutate the clients while iterating over search results clients_to_remove = [] - clients.each do |nodename| + get_clients.each do |nodename| clients_to_remove.push(nodename) unless node_exists?(nodename) end # now delete any flagged clients from the keys data bag clients_to_remove.each do |client| $stdout.puts "Removing unknown client '#{client}'" - keys.delete(client, "clients") + keys.delete(load_public_key(client, "clients")) end end @@ -446,7 +425,7 @@ def node_exists?(nodename) # @return [Boolean] whether the client exists or not def client_exists?(clientname) begin - ChefVault::ChefPatch::ApiClient.load(clientname) + Chef::ApiClient.load(clientname) rescue Net::HTTPServerException => http_error return false if http_error.response.code == "404" raise http_error @@ -458,27 +437,29 @@ def client_exists?(clientname) # @param client [Chef::ApiClient] the API client to operate on # @param action [Symbol] :add or :delete # @return [void] - def handle_client_action(client, action) + def handle_client_action(api_client, action) case action when :add - add_client(client) + client_key = load_public_key(api_client.name, "clients") + add_client(client_key) when :delete - delete_client_or_node(client) + delete_client_or_node(api_client) end end # adds a client to the vault item keys # @param client [Chef::ApiClient] the API client to add # @return [void] - def add_client(client) - keys.add(client, @secret, "clients") + def add_client(client_key) + keys.add(client_key, @secret) end # removes a client to the vault item keys - # @param client_or_node [Chef::ApiClient,Chef::Node] the API client or node to remove + # @param client_or_node [Chef::ApiClient, Chef::Node] the API client or node to remove # @return [void] def delete_client_or_node(client_or_node) - keys.delete(client_or_node.name, "clients") + client_key = load_public_key(client_or_node.name, "clients") + keys.delete(client_key) end end end diff --git a/lib/chef-vault/item_keys.rb b/lib/chef-vault/item_keys.rb index d9b0a09..d9f711d 100644 --- a/lib/chef-vault/item_keys.rb +++ b/lib/chef-vault/item_keys.rb @@ -34,22 +34,20 @@ def include?(key) @raw_data.keys.include?(key) end - def add(chef_client, data_bag_shared_secret, type) + def add(chef_key, data_bag_shared_secret) + type = chef_key.actor_type unless @raw_data.key?(type) raise ChefVault::Exceptions::V1Format, "cannot manage a v1 vault. See UPGRADE.md for help" end - public_key = OpenSSL::PKey::RSA.new chef_client.public_key - self[chef_client.name] = - Base64.encode64(public_key.public_encrypt(data_bag_shared_secret)) - - @raw_data[type] << chef_client.name unless @raw_data[type].include?(chef_client.name) + self[chef_key.actor_name] = ChefVault::ItemKeys.encode_key(chef_key.key, data_bag_shared_secret) + @raw_data[type] << chef_key.actor_name unless @raw_data[type].include?(chef_key.actor_name) @raw_data[type] end - def delete(chef_client, type) - raw_data.delete(chef_client) - raw_data[type].delete(chef_client) + def delete(chef_key) + raw_data.delete(chef_key.actor_name) + raw_data[chef_key.actor_type].delete(chef_key.actor_name) end def search_query(search_query = nil) @@ -128,5 +126,12 @@ def self.load(vault, name) from_data_bag_item(data_bag_item) end + + # @private + + def self.encode_key(key_string, data_bag_shared_secret) + public_key = OpenSSL::PKey::RSA.new(key_string) + Base64.encode64(public_key.public_encrypt(data_bag_shared_secret)) + end end end diff --git a/lib/chef/knife/vault_show.rb b/lib/chef/knife/vault_show.rb index 3cc09b7..f3d0dc8 100644 --- a/lib/chef/knife/vault_show.rb +++ b/lib/chef/knife/vault_show.rb @@ -58,13 +58,13 @@ def print_values(vault, item, values) when "search" extra_data["search_query"] = vault_item.search when "admins" - extra_data["admins"] = vault_item.admins + extra_data["admins"] = vault_item.get_admins when "clients" - extra_data["clients"] = vault_item.clients + extra_data["clients"] = vault_item.get_clients when "all" extra_data["search_query"] = vault_item.search - extra_data["admins"] = vault_item.admins - extra_data["clients"] = vault_item.clients + extra_data["admins"] = vault_item.get_admins + extra_data["clients"] = vault_item.get_clients end end diff --git a/lib/chef/knife/vault_update.rb b/lib/chef/knife/vault_update.rb index a1db0ea..d3aa7cb 100644 --- a/lib/chef/knife/vault_update.rb +++ b/lib/chef/knife/vault_update.rb @@ -64,10 +64,10 @@ def run # Keys management first if clean - clients = vault_item.clients().clone().sort() + clients = vault_item.get_clients().clone().sort() clients.each do |client| $stdout.puts "Deleting #{client}" - vault_item.keys.delete(client, "clients") + vault_item.delete_client(client) end end diff --git a/spec/chef-vault/chef_api_spec.rb b/spec/chef-vault/chef_api_spec.rb new file mode 100644 index 0000000..6dab04b --- /dev/null +++ b/spec/chef-vault/chef_api_spec.rb @@ -0,0 +1,39 @@ +RSpec.describe ChefVault::ChefApi do + let(:root_url) { "https://localhost" } + let(:scoped_url) { "https://localhost/organizations/fakeorg" } + let(:api_v0_hash) { { :api_version => "0" } } + let(:api_v1_hash) { { :api_version => "1" } } + + before do + Chef::Config[:chef_server_root] = root_url + Chef::Config[:chef_server_url] = scoped_url + end + + describe '#rest_v0' do + it "returns an instance of Chef::ServerAPI set to use API version 0 scoped to root" do + expect(Chef::ServerAPI).to receive(:new).with(root_url, api_v0_hash) + subject.rest_v0 + end + end + + describe '#rest_v1' do + it "returns an instance of Chef::ServerAPI set to use API version 0 scoped to root" do + expect(Chef::ServerAPI).to receive(:new).with(root_url, api_v1_hash) + subject.rest_v1 + end + end + + describe '#org_scoped_rest_v0' do + it "returns an instance of Chef::ServerAPI set to use API version 0 scoped to root" do + expect(Chef::ServerAPI).to receive(:new).with(scoped_url, api_v0_hash) + subject.org_scoped_rest_v0 + end + end + + describe '#org_scoped_rest_v1' do + it "returns an instance of Chef::ServerAPI set to use API version 0 scoped to root" do + expect(Chef::ServerAPI).to receive(:new).with(scoped_url, api_v1_hash) + subject.org_scoped_rest_v1 + end + end +end diff --git a/spec/chef-vault/chef_key_spec.rb b/spec/chef-vault/chef_key_spec.rb new file mode 100644 index 0000000..91e9865 --- /dev/null +++ b/spec/chef-vault/chef_key_spec.rb @@ -0,0 +1,227 @@ +require "spec_helper" + +RSpec.describe ChefVault::ChefKey do + let(:actor_name) { "actor" } + let(:public_key_string) { + "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAyMXT9IOV9pkQsxsnhSx8\n8RX6GW3caxkjcXFfHg6E7zUVBFAsfw4B1D+eHAks3qrDB7UrUxsmCBXwU4dQHaQy\ngAn5Sv0Jc4CejDNL2EeCBLZ4TF05odHmuzyDdPkSZP6utpR7+uF7SgVQedFGySIB\nih86aM+HynhkJqgJYhoxkrdo/JcWjpk7YEmWb6p4esnvPWOpbcjIoFs4OjavWBOF\niTfpkS0SkygpLi/iQu9RQfd4hDMWCc6yh3Th/1nVMUd+xQCdUK5wxluAWSv8U0zu\nhiIlZNazpCGHp+3QdP3f6rebmQA8pRM8qT5SlOvCYPk79j+IMUVSYrR4/DTZ+VM+\naQIDAQAB\n-----END PUBLIC KEY-----\n" + } + + let(:key_response) do + { + "name" => "default", + "public_key" => "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAyMXT9IOV9pkQsxsnhSx8\n8RX6GW3caxkjcXFfHg6E7zUVBFAsfw4B1D+eHAks3qrDB7UrUxsmCBXwU4dQHaQy\ngAn5Sv0Jc4CejDNL2EeCBLZ4TF05odHmuzyDdPkSZP6utpR7+uF7SgVQedFGySIB\nih86aM+HynhkJqgJYhoxkrdo/JcWjpk7YEmWb6p4esnvPWOpbcjIoFs4OjavWBOF\niTfpkS0SkygpLi/iQu9RQfd4hDMWCc6yh3Th/1nVMUd+xQCdUK5wxluAWSv8U0zu\nhiIlZNazpCGHp+3QdP3f6rebmQA8pRM8qT5SlOvCYPk79j+IMUVSYrR4/DTZ+VM+\naQIDAQAB\n-----END PUBLIC KEY-----\n", + "expiration_date" => "infinity", + } + end + + let(:http_response_code) do + "404" + end + + let(:http_error) do + http_response = double("http error") + allow(http_response).to receive(:code).and_return(http_response_code) + Net::HTTPServerException.new("http error message", http_response) + end + + let(:api) { double("api") } + + subject(:chef_key) { described_class.new(actor_type, actor_name) } + + describe '#new' do + context "when something besides 'clients' or 'users' is passed" do + let(:actor_type) { "charmander" } + it "throws an error" do + expect { described_class.new("charmander", actor_name) }.to raise_error + end + end + end + + shared_examples_for "get_key_handling" do + context "when the default key exists for the requested client" do + it "sets up a valid key" do + expect(chef_key).to receive(:get_key).with(request_actor_type).and_return(public_key_string) + expect(chef_key.send(method)).to eq(public_key_string) + end + end + + context "when get_key returns an http error" do + before do + allow(chef_key).to receive(:get_key).with(request_actor_type).and_raise(http_error) + end + + context "when the error code is not 404 or 403" do + let(:http_response_code) { "500" } + + it "raises the original error" do + expect { chef_key.send(method) }.to raise_error(http_error) + end + end + + context "when the error code is 403" do + let(:http_response_code) { "403" } + + it "prints information for the user to resolve the issue and raises the original error" do + expect(chef_key).to receive(:print_forbidden_error) + expect { chef_key.send(method) }.to raise_error(http_error) + end + end + end + end + + describe '#get_client_key' do + let(:request_actor_type) { "clients" } + let(:actor_type) { "clients" } + let(:method) { :get_client_key } + + it_should_behave_like "get_key_handling" + + context "when get_key returns an http error" do + before do + allow(chef_key).to receive(:get_key).with(actor_type).and_raise(http_error) + end + + context "when the error code is 404" do + let(:http_response_code) { "404" } + + it "rasies ChefVault::Exceptions::ClientNotFound" do + expect { chef_key.get_client_key }.to raise_error(ChefVault::Exceptions::ClientNotFound) + end + end + end + end # get_client_key + + describe '#get_admin_key' do + let(:request_actor_type) { "users" } + let(:actor_type) { "admins" } + let(:method) { :get_admin_key } + + it_should_behave_like "get_key_handling" + + context "when the first get_key for users returns an http error" do + before do + allow(chef_key).to receive(:get_key).with(request_actor_type).and_raise(http_error) + end + + context "when the error code from the users get is a 404" do + let(:http_response_code) { "404" } + + context "when the second get_key for clients returns an http error" do + + let(:http_error_2) do + http_response = double("http error") + allow(http_response).to receive(:code).and_return(http_response_code_2) + Net::HTTPServerException.new("http error message", http_response) + end + + before do + allow(chef_key).to receive(:get_key).with("clients").and_raise(http_error_2) + end + + context "when it is a 404" do + let(:http_response_code_2) { "404" } + + it "rasies ChefVault::Exceptions::AdminNotFound" do + expect { chef_key.get_admin_key }.to raise_error(ChefVault::Exceptions::AdminNotFound) + end + end + + context "when it is not a 404" do + let(:http_response_code_2) { "500" } + + it "raises the original error" do + expect { chef_key.get_admin_key }.to raise_error(http_error_2) + + end + + end + end # when the second get_key for clients returns an http error + + context "when the second get_key for clients exists with the same name as the admin requested" do + it "strangely returns the client key as an admin key" do + expect(chef_key).to receive(:get_key).with(request_actor_type).and_return(public_key_string) + expect(chef_key.send(method)).to eq(public_key_string) + end + end + end # when the first get_key for users returns an http erro + end + end # get_admin_key + + describe '#get_key' do + + shared_examples_for "a properly retrieved and error handled key fetch" do + # mock out the API + before do + allow(chef_key).to receive(:api).and_return(api) + [:rest_v0, :rest_v1, :org_scoped_rest_v0, :org_scoped_rest_v1].each do |method| + allow(api).to receive(method) + end + end + + context "when keys/default returns 200 for org scoped endpoint" do + before do + allow(api.org_scoped_rest_v1).to receive(:get).with("#{request_actor_type}/#{actor_name}/keys/default").and_return(key_response) + end + + it "returns the public_key" do + expect(chef_key.get_key(request_actor_type)).to eql(public_key_string) + end + + it "hits the proper endpoint" do + expect(api.org_scoped_rest_v1).to receive(:get).with("#{request_actor_type}/#{actor_name}/keys/default") + chef_key.get_key(request_actor_type) + end + end + + context "when a 500 is returned" do + let(:http_response_code) { "500" } + before do + allow(api.org_scoped_rest_v1).to receive(:get).with("#{request_actor_type}/#{actor_name}/keys/default").and_raise(http_error) + end + + it "raises the http error" do + expect { chef_key.get_key(request_actor_type) }.to raise_error(http_error) + end + end + + context "when keys/default returns 404" do + let(:http_response_code) { "404" } + let(:chef_object) { double("chef object") } + + before do + allow(api.org_scoped_rest_v1).to receive(:get).with("#{request_actor_type}/#{actor_name}/keys/default").and_raise(http_error) + allow(chef_object_type).to receive(:load).with(actor_name).and_return(chef_object) + allow(chef_object).to receive(:public_key).and_return(public_key_string) + end + + it "tries to load the object via Chef::_v1" do + expect(chef_object_type).to receive(:load).with(actor_name) + chef_key.get_key(request_actor_type) + end + + context "when the Chef::_v1 object loads properly" do + it "returns the public key" do + expect(chef_key.get_key(request_actor_type)).to eql(public_key_string) + end + end + end + end # shared_examples_for + + context "when a client is passed" do + let(:request_actor_type) { "clients" } + let(:actor_type) { "clients" } + let(:chef_object_type) { Chef::ApiClient } + + it_behaves_like "a properly retrieved and error handled key fetch" + end + + context "when an admin is passed" do + let(:request_actor_type) { "users" } + let(:actor_type) { "admins" } + let(:chef_object_type) { Chef::User } + + it_behaves_like "a properly retrieved and error handled key fetch" + end + + end +end diff --git a/spec/chef-vault/item_keys_spec.rb b/spec/chef-vault/item_keys_spec.rb index 8cfd1dc..09791b4 100644 --- a/spec/chef-vault/item_keys_spec.rb +++ b/spec/chef-vault/item_keys_spec.rb @@ -1,6 +1,7 @@ RSpec.describe ChefVault::ItemKeys do describe '#new' do let(:keys) { ChefVault::ItemKeys.new("foo", "bar") } + let(:shared_secret) { "super_secret" } it "'foo' is assigned to @data_bag" do expect(keys.data_bag).to eq "foo" @@ -18,6 +19,57 @@ expect(keys["admins"]).to eq [] end + describe "key mgmt operations" do + let(:public_key_string) { + "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAyMXT9IOV9pkQsxsnhSx8\n8RX6GW3caxkjcXFfHg6E7zUVBFAsfw4B1D+eHAks3qrDB7UrUxsmCBXwU4dQHaQy\ngAn5Sv0Jc4CejDNL2EeCBLZ4TF05odHmuzyDdPkSZP6utpR7+uF7SgVQedFGySIB\nih86aM+HynhkJqgJYhoxkrdo/JcWjpk7YEmWb6p4esnvPWOpbcjIoFs4OjavWBOF\niTfpkS0SkygpLi/iQu9RQfd4hDMWCc6yh3Th/1nVMUd+xQCdUK5wxluAWSv8U0zu\nhiIlZNazpCGHp+3QdP3f6rebmQA8pRM8qT5SlOvCYPk79j+IMUVSYrR4/DTZ+VM+\naQIDAQAB\n-----END PUBLIC KEY-----\n" + } + + shared_examples_for "proper key management" do + let(:chef_key) { ChefVault::ChefKey.new(type, name) } + before do + allow(chef_key).to receive(:key) { public_key_string } + keys.add(chef_key, shared_secret) + end + + describe "#add" do + after do + keys.delete(chef_key) + end + + it "stores the encoded key in the data bag item under the actor's name and the name in the raw data" do + expect(described_class).to receive(:encode_key).with(public_key_string, shared_secret).and_return("encrypted_result") + keys.add(chef_key, shared_secret) + expect(keys[name]).to eq("encrypted_result") + expect(keys[type].include?(name)).to eq(true) + end + end + + describe '#delete' do + before do + keys.add(chef_key, shared_secret) + end + + it "removes the actor's name from the data bag and from the array for the actor's type" do + keys.delete(chef_key) + expect(keys.has_key?(chef_key.actor_name)).to eq(false) + expect(keys[type].include?(name)).to eq(false) + end + end + end + + context "when a client is added" do + let(:name) { "client_name" } + let(:type) { "clients" } + it_should_behave_like "proper key management" + end + + context "when a admin is added" do + let(:name) { "admin_name" } + let(:type) { "admins" } + it_should_behave_like "proper key management" + end + end + context "when running with chef-solo" do describe "#find_solo_path" do context "when data_bag_path is an array" do diff --git a/spec/chef-vault/item_spec.rb b/spec/chef-vault/item_spec.rb index 39dbffe..47d26ce 100644 --- a/spec/chef-vault/item_spec.rb +++ b/spec/chef-vault/item_spec.rb @@ -1,54 +1,4 @@ require "openssl" -require "rspec/core/shared_context" - -# it turns out that simulating a node that doesn't have a public -# key is not a simple thing -module BorkedNodeWithoutPublicKey - extend RSpec::Core::SharedContext - - before do - # a node 'foo' with a public key - node_foo = double("chef node foo") - allow(node_foo).to receive(:name).and_return("foo") - client_foo = double("chef apiclient foo") - allow(client_foo).to receive(:name).and_return("foo") - privkey = OpenSSL::PKey::RSA.new(1024) - pubkey = privkey.public_key - allow(client_foo).to receive(:public_key).and_return(pubkey.to_pem) - # a node 'bar' without a public key - node_bar = double("chef node bar") - allow(node_bar).to receive(:name).and_return("bar") - # fake out searches to return both of our nodes - query_result = double("chef search results") - allow(query_result) - .to receive(:search) - .with(Symbol, String) - .and_yield(node_foo).and_yield(node_bar) - allow(Chef::Search::Query) - .to receive(:new) - .and_return(query_result) - # create a new vault item - @vaultitem = ChefVault::Item.new("foo", "bar") - @vaultitem["foo"] = "bar" - # make the vault item return the apiclient for foo but raise for bar - allow(@vaultitem).to receive(:load_client).with("foo") - .and_return(client_foo) - allow(@vaultitem).to receive(:load_client).with("bar") - .and_raise(ChefVault::Exceptions::ClientNotFound) - # make the vault item fall back to 'load-admin-as-client' behaviour - http_response = double("http not found") - allow(http_response).to receive(:code).and_return("404") - http_not_found = Net::HTTPServerException.new("not found", http_response) - allow(ChefVault::ChefPatch::User) - .to receive(:load) - .with("foo") - .and_return(client_foo) - allow(ChefVault::ChefPatch::User) - .to receive(:load) - .with("bar") - .and_raise(http_not_found) - end -end RSpec.describe ChefVault::Item do before do @@ -62,6 +12,10 @@ module BorkedNodeWithoutPublicKey subject(:item) { ChefVault::Item.new("foo", "bar") } + before do + item["foo"] = "bar" + end + describe "vault probe predicates" do before do # a normal data bag item @@ -245,10 +199,10 @@ module BorkedNodeWithoutPublicKey allow(Chef::Search::Query).to receive(:new).and_return(query) allow(query).to receive(:search).and_yield(node) - client = double("client", - name: "testclient", - public_key: OpenSSL::PKey::RSA.new(1024).public_key) - allow(ChefVault::ChefPatch::ApiClient).to receive(:load).and_return(client) + client_key = double("client_key", + name: "testnode", + public_key: OpenSSL::PKey::RSA.new(1024).public_key) + allow(item).to receive(:load_public_key).with("testnode", "clients").and_return(client_key) expect(item).not_to receive(:save) expect(keys).to receive(:save) @@ -257,38 +211,101 @@ module BorkedNodeWithoutPublicKey end describe '#clients' do - include BorkedNodeWithoutPublicKey + context "when search returns a node with a valid client backing it and one without a valid client" do + let(:node_with_valid_client) { double("chef node valid") } + let(:node_without_valid_client) { double("chef node no valid client") } + let(:query_result) { double("chef search results") } + let(:client_key) { double("chef key") } + + before do + # node with valid client proper loads client key + allow(node_with_valid_client).to receive(:name).and_return("foo") + allow(item).to receive(:load_public_key).with("foo", "clients").and_return(client_key) + privkey = OpenSSL::PKey::RSA.new(1024) + pubkey = privkey.public_key + allow(client_key).to receive(:key).and_return(pubkey.to_pem) + allow(client_key).to receive(:actor_name).and_return("foo") + allow(client_key).to receive(:actor_type).and_return("clients") + + # node without client throws relevant error on key load + allow(node_without_valid_client).to receive(:name).and_return("bar") + allow(item).to receive(:load_public_key).with("bar", "clients").and_raise(ChefVault::Exceptions::ClientNotFound) + + allow(query_result) + .to receive(:search) + .with(Symbol, String) + .and_yield(node_with_valid_client).and_yield(node_without_valid_client) + allow(Chef::Search::Query) + .to receive(:new) + .and_return(query_result) + end - it "should not blow up when search returns a node without a public key" do - # try to set clients when we know a node is missing a public key - # this should not die as of v2.4.1 - expect { @vaultitem.clients("*:*") }.not_to raise_error - end + it "should not blow up when search returns a node without a public key" do + # try to set clients when we know a node is missing a public key + # this should not die as of v2.4.1 + expect { item.clients("*:*") }.not_to raise_error + end - it "should emit a warning if search returns a node without a public key" do - # it should however emit a warning that you have a borked node - expect { @vaultitem.clients("*:*") } - .to output(/node 'bar' has no private key; skipping/).to_stdout + it "should emit a warning if search returns a node without a public key" do + # it should however emit a warning that you have a borked node + expect { item.clients("*:*") } + .to output(/node 'bar' has no private key; skipping/).to_stdout + end end - it "should accept a client object and not perform a search" do - client = Chef::ApiClient.new - client.name "foo" - privkey = OpenSSL::PKey::RSA.new(1024) - pubkey = privkey.public_key - client.public_key(pubkey.to_pem) - expect(Chef::Search::Query).not_to receive(:new) - expect(ChefVault::ChefPatch::User).not_to receive(:load) - @vaultitem.clients(client) - expect(@vaultitem.clients).to include("foo") + context "when a Chef::ApiClient is passed" do + let(:client) { Chef::ApiClient.new } + let(:client_name) { "foo" } + let(:client_key) { double("chef key") } + + before do + client.name client_name + privkey = OpenSSL::PKey::RSA.new(1024) + pubkey = privkey.public_key + allow(item).to receive(:load_public_key).with(client_name, "clients").and_return(client_key) + allow(client_key).to receive(:key).and_return(pubkey.to_pem) + allow(client_key).to receive(:actor_name).and_return(client_name) + allow(client_key).to receive(:actor_type).and_return("clients") + end + + context "when no action is passed" do + it "default to add and properly add the client" do + item.clients(client) + expect(item.get_clients).to include(client_name) + end + + it "does not perform a query" do + expect(Chef::Search::Query).not_to receive(:new) + item.clients(client) + end + end + + context "when the delete action is passed on an existing client" do + before do + # add the client + item.clients(client) + end + + it "properly deletes the client" do + item.clients(client, :delete) + expect(item.get_clients).to_not include(client_name) + end + + it "does not perform a query" do + expect(Chef::Search::Query).not_to receive(:new) + item.clients(client, :delete) + end + end end end describe '#admins' do - include BorkedNodeWithoutPublicKey + before do + allow(item).to receive(:load_public_key).with("foo", "admins").and_raise(ChefVault::Exceptions::AdminNotFound) + end it "should blow up if you try to use a node without a public key as an admin" do - expect { @vaultitem.admins("foo,bar") } + expect { item.admins("foo,bar") } .to raise_error(ChefVault::Exceptions::AdminNotFound) end end From c4d46fc5eb25b340efaa580b6db0653fc2ef5471 Mon Sep 17 00:00:00 2001 From: Tyler Cloke Date: Wed, 13 Apr 2016 16:21:00 -0700 Subject: [PATCH 2/2] Git hook for helping to keep rubocop / chefstyle up to date. --- README.md | 10 ++++++++++ lib/chef-vault/chef_key.rb | 2 +- lib/chef/knife/vault_update.rb | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 96ac025..eb44b33 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,16 @@ with root privileges. ## DEVELOPMENT: +### Git Hooks + +There is a git pre-commit hook to help you keep your chefstyle up to date. +If you wish to use it, simply: + +``` +mv hooks/pre-commit .git/hooks/ +chmod +x .git/hooks/pre-commit +``` + ### Running Your Changes To run your changes locally: diff --git a/lib/chef-vault/chef_key.rb b/lib/chef-vault/chef_key.rb index 047b8a9..e88cf8d 100644 --- a/lib/chef-vault/chef_key.rb +++ b/lib/chef-vault/chef_key.rb @@ -120,7 +120,7 @@ def get_key(request_actor_type) if request_actor_type == "clients" chef_api_client.load(actor_name).public_key else - user = chef_user.load(actor_name).public_key + chef_user.load(actor_name).public_key end end diff --git a/lib/chef/knife/vault_update.rb b/lib/chef/knife/vault_update.rb index d3aa7cb..fda2b39 100644 --- a/lib/chef/knife/vault_update.rb +++ b/lib/chef/knife/vault_update.rb @@ -64,7 +64,7 @@ def run # Keys management first if clean - clients = vault_item.get_clients().clone().sort() + clients = vault_item.get_clients.clone().sort() clients.each do |client| $stdout.puts "Deleting #{client}" vault_item.delete_client(client)