Merge pull request #11802 from chef/mp/azure-key-vault

Secrets: Azure Key Vault fetcher; versioned secret support

10 files changed, 162 insertions, 47 deletions

diff --git a/lib/chef/dsl/secret.rb b/lib/chef/dsl/secret.rb
index 9cc7d6b3da..28f49a52a6 100644
--- a/lib/chef/dsl/secret.rb
+++ b/lib/chef/dsl/secret.rb
@@ -29,12 +29,15 @@ class Chef
       # that resource as 'sensitive', preventing resource data from being logged.  See [Chef::Resource#sensitive].
       #
       # @option name [Object] The identifier or name for this secret
+      # @option version [Object] The secret version. If a service supports versions
+      #                          and no version is provided, the latest version will be fetched.
       # @option service [Symbol] The service identifier for the service that will
-      #                         perform the secret lookup
+      #                         perform the secret lookup. See
+      #                         [Chef::SecretFetcher::SECRET_FETCHERS]
       # @option config [Hash] The configuration that the named service expects
       #
-      # @return result [Object] The response object type is determined by the fetcher. See fetcher documentation
-      # to know what to expect for a given service.
+      # @return result [Object] The response object type is determined by the fetcher but will usually be a string or a hash.
+      # See individual fetcher documentation to know what to expect for a given service.
       #
       # @example
       #
@@ -44,20 +47,11 @@ class Chef
       #   value = secret(name: "test1", service: :example, config: { "test1" => "value1" })
       #   log "My secret is #{value}"
       #
-      #   value = secret(name: "test1", service: :aws_secrets_manager, config: { region: "us-west-1" })
-      #   log "My secret is #{value.secret_string}"
-      #
-      # @note
-      #
-      # This is pretty straightforward, but should also extend nicely to support
-      # named config (as 'service') with override config. Some future potential
-      # usage examples:
-      #   value = secret(name: "test1") # If a default is configured
-      #   value = secret(name: "test1", service: "my_aws_east")
-      #   value = secret(name: "test1", service: "my_aws_west", config: { region: "override-region" })
-      def secret(name: nil, service: nil, config: nil)
+      #   value = secret(name: "test1", service: :aws_secrets_manager, version: "v1", config: { region: "us-west-1" })
+      #   log "My secret is #{value}"
+      def secret(name: nil, version: nil, service: nil, config: nil)
         sensitive(true) if is_a?(Chef::Resource)
-        Chef::SecretFetcher.for_service(service, config).fetch(name)
+        Chef::SecretFetcher.for_service(service, config).fetch(name, version)
       end
     end
   end
diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index 6a90b0cc52..8e9bced8bb 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -295,6 +295,7 @@ class Chef
       class ConfigurationInvalid < RuntimeError; end
       class FetchFailed < RuntimeError; end
       class MissingSecretName < RuntimeError; end
+      class InvalidSecretName < RuntimeError; end
 
       class InvalidFetcherService < RuntimeError
         def initialize(given, fetcher_service_names)
@@ -308,6 +309,7 @@ class Chef
         end
       end
 
+      class MissingVaultName < RuntimeError; end
     end
 
     # Exception class for collecting multiple failures. Used when running
diff --git a/lib/chef/secret_fetcher.rb b/lib/chef/secret_fetcher.rb
index 6745da00f6..9b8acd3790 100644
--- a/lib/chef/secret_fetcher.rb
+++ b/lib/chef/secret_fetcher.rb
@@ -21,7 +21,7 @@ require_relative "exceptions"
 class Chef
   class SecretFetcher
 
-    SECRET_FETCHERS = %i{example aws_secrets_manager}.freeze
+    SECRET_FETCHERS = %i{example aws_secrets_manager azure_key_vault}.freeze
 
     # Returns a configured and validated instance
     # of a [Chef::SecretFetcher::Base]  for the given
@@ -38,6 +38,9 @@ class Chef
                 when :aws_secrets_manager
                   require_relative "secret_fetcher/aws_secrets_manager"
                   Chef::SecretFetcher::AWSSecretsManager.new(config)
+                when :azure_key_vault
+                  require_relative "secret_fetcher/azure_key_vault"
+                  Chef::SecretFetcher::AzureKeyVault.new(config)
                 when nil, ""
                   raise Chef::Exceptions::Secret::MissingFetcher.new(SECRET_FETCHERS)
                 else
@@ -46,7 +49,6 @@ class Chef
       fetcher.validate!
       fetcher
     end
-
   end
 end
 
diff --git a/lib/chef/secret_fetcher/aws_secrets_manager.rb b/lib/chef/secret_fetcher/aws_secrets_manager.rb
index f5508cf59b..954de943ef 100644
--- a/lib/chef/secret_fetcher/aws_secrets_manager.rb
+++ b/lib/chef/secret_fetcher/aws_secrets_manager.rb
@@ -24,6 +24,9 @@ class Chef
   # A fetcher that fetches a secret from AWS Secrets Manager
   # In this initial iteration it defaults to authentication via instance profile.
   # It is possible to pass options that configure it to use alternative credentials.
+  # This implementation supports fetching with version.
+  #
+  # NOTE: This does not yet support automatic retries, which the AWS client does by default.
   #
   # For configuration options see https://docs.aws.amazon.com/sdk-for-ruby/v3/api/Aws/SecretsManager/Client.html#initialize-instance_method
   #
@@ -33,31 +36,18 @@ class Chef
   # Usage Example:
   #
   # fetcher = SecretFetcher.for_service(:aws_secrets_manager, { region: "us-east-1" })
-  # fetcher.fetch("secretkey1")
+  # fetcher.fetch("secretkey1", "v1")
   class SecretFetcher
     class AWSSecretsManager < Base
-      DEFAULT_AWS_OPTS = {} # rubocop: disable Style/MutableConstant
-      def validate!
-        # Note that we are not doing any validation of required configuration here, we will
-        # rely on the API client to do that for us, since it will work with the merge of
-        # the config we provide, env-based config, and/or an appropriate profile in ~/.aws
-
-        # Instantiating the client is an opportunity for an API provider to do validation,
-        # so we'll do that first here.
-        client
-      end
-
       # @param identifier [String] the secret_id
+      # @param version [String] the secret version. Not usd at this time
       # @return Aws::SecretsManager::Types::GetSecretValueResponse
-      def do_fetch(identifier)
-        result = client.get_secret_value(secret_id: identifier)
+      def do_fetch(identifier, version)
+        client = Aws::SecretsManager::Client.new
+        result = client.get_secret_value(secret_id: identifier, version_stage: version)
         # These fields are mutually exclusive
         result.secret_string || result.secret_binary
       end
-
-      def client
-        @client ||= Aws::SecretsManager::Client.new(DEFAULT_AWS_OPTS.merge(config))
-      end
     end
   end
 end
diff --git a/lib/chef/secret_fetcher/azure_key_vault.rb b/lib/chef/secret_fetcher/azure_key_vault.rb
new file mode 100644
index 0000000000..734bd94df1
--- /dev/null
+++ b/lib/chef/secret_fetcher/azure_key_vault.rb
@@ -0,0 +1,56 @@
+require_relative "base"
+
+class Chef
+  class SecretFetcher
+    # == Chef::SecretFetcher::AWSSecretsManager
+    # A fetcher that fetches a secret from Azure Key Vault. Supports fetching with version.
+    #
+    # In this initial iteration this authenticates via token obtained from the OAuth2  /token
+    # endpoint.
+    #
+    # Usage Example:
+    #
+    # fetcher = SecretFetcher.for_service(:azure_key_vault)
+    # fetcher.fetch("secretkey1", "v1")
+    class AzureKeyVault < Base
+      def validate!
+        @vault = config[:vault]
+        if @vault.nil?
+          raise Chef::Exceptions::Secret::MissingVaultName.new("You must provide a vault name to service options as vault: 'vault_name'")
+        end
+      end
+
+      def do_fetch(name, version)
+        token = fetch_token
+
+        # Note that `version` is optional after the final `/`. If nil/"", the latest secret version will be fetched.
+        secret_uri = URI.parse("https://#{@vault}.vault.azure.net/secrets/#{name}/#{version}?api-version=7.2")
+        http = Net::HTTP.new(secret_uri.host, secret_uri.port)
+        http.use_ssl = true
+
+        response = http.get(secret_uri, { "Authorization" => "Bearer #{token}",
+                                          "Content-Type" => "application/json" })
+
+        # If an exception is not raised, we can be reasonably confident of the
+        # shape of the result.
+        result = JSON.parse(response.body)
+        if result.key? "value"
+          result["value"]
+        else
+          raise Chef::Exceptions::Secret::FetchFailed.new("#{result["error"]["code"]}: #{result["error"]["message"]}")
+        end
+      end
+
+      def fetch_token
+        token_uri = URI.parse("http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource=https%3A%2F%2Fvault.azure.net")
+        http = Net::HTTP.new(token_uri.host, token_uri.port)
+        response = http.get(token_uri, { "Metadata" => "true" })
+        body = JSON.parse(response.body)
+        body["access_token"]
+      end
+    end
+  end
+end
+
+
+
diff --git a/lib/chef/secret_fetcher/base.rb b/lib/chef/secret_fetcher/base.rb
index a80ecea0fb..36b63990c0 100644
--- a/lib/chef/secret_fetcher/base.rb
+++ b/lib/chef/secret_fetcher/base.rb
@@ -37,14 +37,15 @@ class Chef
       # Fetch the named secret by invoking implementation-specific [Chef::SecretFetcher::Base#do_fetch]
       #
       # @param name [Object] the name or identifier of the secret.
+      # @param version [Object] Optional version of the secret to fetch.
       # @note - the name parameter will probably see a narrowing of type as we learn more about different integrations.
-      # @return [Object] the result of the secret fetch
+      # @return [Object] the fetched secret
       # @raise [Chef::Exceptions::Secret::MissingSecretName] when secret name is not provided
       # @raise [Chef::Exceptions::Secret::FetchFailed] when the underlying attempt to fetch the secret fails.
-      def fetch(name)
-        raise Chef::Exceptions::Secret::MissingSecretName.new if name.nil? || name.to_s == ""
+      def fetch(name, version = nil)
+        raise Chef::Exceptions::Secret::MissingSecretName.new if name.to_s == ""
 
-        do_fetch(name)
+        do_fetch(name, version)
       end
 
       # Validate that the instance is correctly configured.
@@ -57,12 +58,15 @@ class Chef
       # @param identifier [Object] Unique identifier of the secret to be retrieved.
       # When invoked via DSL, this is pre-verified to be not nil/not empty string.
       # The expected data type and form can vary by implementation.
+      # @param version [Object] Optional version of the secret to be retrieved.  If not
+      # provided, implementations are expected to fetch the most recent version of the
+      # secret by default.
       #
       # @return [Object] The secret as returned from the implementation.  The data type
       # will vary implementation.
       #
       # @raise [Chef::Exceptions::Secret::FetchFailed] if the secret could not be fetched
-      def do_fetch(identifier); raise NotImplementedError.new; end
+      def do_fetch(identifier, version); raise NotImplementedError.new; end
     end
   end
 end
diff --git a/lib/chef/secret_fetcher/example.rb b/lib/chef/secret_fetcher/example.rb
index 7a0e671929..28b806bf31 100644
--- a/lib/chef/secret_fetcher/example.rb
+++ b/lib/chef/secret_fetcher/example.rb
@@ -36,7 +36,7 @@ class Chef
         end
       end
 
-      def do_fetch(identifier)
+      def do_fetch(identifier, version)
         raise Chef::Exceptions::Secret::FetchFailed.new("Secret #{identifier}) not found.") unless config.key?(identifier)
 
         config[identifier]
diff --git a/spec/unit/dsl/secret_spec.rb b/spec/unit/dsl/secret_spec.rb
index 99699b253e..ee25c511ee 100644
--- a/spec/unit/dsl/secret_spec.rb
+++ b/spec/unit/dsl/secret_spec.rb
@@ -24,7 +24,7 @@ class SecretDSLTester
 end
 
 class SecretFetcherImpl < Chef::SecretFetcher::Base
-  def do_fetch(name)
+  def do_fetch(name, version)
     name
   end
 end
@@ -38,7 +38,7 @@ describe Chef::DSL::Secret do
   it "uses SecretFetcher.for_service to find the fetcher" do
     substitute_fetcher = SecretFetcherImpl.new({})
     expect(Chef::SecretFetcher).to receive(:for_service).with(:example, {}).and_return(substitute_fetcher)
-    expect(substitute_fetcher).to receive(:fetch).with "key1"
+    expect(substitute_fetcher).to receive(:fetch).with("key1", nil)
     dsl.secret(name: "key1", service: :example, config: {})
   end
 
diff --git a/spec/unit/secret_fetcher/azure_key_vault_spec.rb b/spec/unit/secret_fetcher/azure_key_vault_spec.rb
new file mode 100644
index 0000000000..cf8c5733c9
--- /dev/null
+++ b/spec/unit/secret_fetcher/azure_key_vault_spec.rb
@@ -0,0 +1,63 @@
+
+#
+# Author:: Marc Paradise <marc@chef.io>
+# Copyright:: Copyright (c) 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_relative "../../spec_helper"
+require "chef/secret_fetcher"
+require "chef/secret_fetcher/azure_key_vault"
+
+describe Chef::SecretFetcher::AzureKeyVault do
+  let(:config) { { vault: "myvault" } }
+  let(:fetcher) { Chef::SecretFetcher::AzureKeyVault.new(config) }
+
+  context "when validating configuration and configuration is missing :vault" do
+    context "and configuration does not have a 'vault'" do
+      let(:config) { {} }
+      it "raises a MissingVaultError error on validate!" do
+        expect { fetcher.validate! }.to raise_error(Chef::Exceptions::Secret::MissingVaultName)
+      end
+    end
+  end
+
+  context "when performing a fetch" do
+    let(:body) { "" }
+    let(:response_mock) { double("response", body: body) }
+    let(:http_mock) { double("http", :get => response_mock, :use_ssl= => nil) }
+
+    before do
+      allow(fetcher).to receive(:fetch_token).and_return "a token"
+      allow(Net::HTTP).to receive(:new).and_return(http_mock)
+    end
+
+    context "and a valid response is received" do
+      let(:body) { '{ "value" : "my secret value" }' }
+      it "returns the expected response" do
+        expect(fetcher.fetch("value")).to eq "my secret value"
+      end
+    end
+
+    context "and an error response is received in the body" do
+      let(:body) { '{ "error" : { "code" : 404, "message" : "secret not found" } }' }
+      it "raises FetchFailed" do
+        expect { fetcher.fetch("value") }.to raise_error(Chef::Exceptions::Secret::FetchFailed)
+      end
+    end
+
+  end
+end
+
diff --git a/spec/unit/secret_fetcher_spec.rb b/spec/unit/secret_fetcher_spec.rb
index c352585266..545176f65a 100644
--- a/spec/unit/secret_fetcher_spec.rb
+++ b/spec/unit/secret_fetcher_spec.rb
@@ -20,7 +20,7 @@ require "chef/secret_fetcher"
 require "chef/secret_fetcher/example"
 
 class SecretFetcherImpl < Chef::SecretFetcher::Base
-  def do_fetch(name)
+  def do_fetch(name, version)
     name
   end
 
@@ -39,6 +39,10 @@ describe Chef::SecretFetcher do
       Chef::SecretFetcher.for_service(:example, {})
     end
 
+    it "resolves the Azure Key Vault fetcher without error" do
+      Chef::SecretFetcher.for_service(:azure_key_vault, vault: "invalid")
+    end
+
     it "resolves the AWS fetcher without error" do
       Chef::SecretFetcher.for_service(:aws_secrets_manager, region: "invalid")
     end
@@ -67,8 +71,8 @@ describe Chef::SecretFetcher do
     }
 
     it "fetches from the underlying service when secret name is provided " do
-      expect(fetcher_impl).to receive(:fetch).with("key1")
-      fetcher.fetch("key1")
+      expect(fetcher_impl).to receive(:fetch).with("key1", "v1")
+      fetcher.fetch("key1", "v1")
     end
 
     it "raises an error when the secret name is not provided" do