Chef-13: turn on zypper gpg checks by defaultlcg/zypper-gpg

Adds a gpg_check property to the resource to turn them off on a
per-resource basis.  The global config is also preserved.  The
global config will warn once if you have it set to turn off
gpg checks.  The per-resource property will not warn (presumably
you know you're doing something bad when you turn it off).

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

5 files changed, 46 insertions, 93 deletions

diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb
index fb41cfae99..51ddbb5649 100644
--- a/chef-config/lib/chef-config/config.rb
+++ b/chef-config/lib/chef-config/config.rb
@@ -644,11 +644,10 @@ module ChefConfig
     # generation (server generates client keys).
     default(:local_key_generation) { true }
 
-    # Zypper package provider gpg checks. Set to true to enable package
-    # gpg signature checking. This will be default in the
-    # future. Setting to false disables the warnings.
-    # Leaving this set to nil or false is a security hazard!
-    default :zypper_check_gpg, nil
+    # Zypper package provider gpg checks. Set to false to disable package
+    # gpg signature checking globally.  This will warn you that it is a
+    # bad thing to do.
+    default :zypper_check_gpg, true
 
     # Report Handlers
     default :report_handlers, []
diff --git a/lib/chef/application.rb b/lib/chef/application.rb
index 86078300c2..eba0d48e57 100644
--- a/lib/chef/application.rb
+++ b/lib/chef/application.rb
@@ -48,6 +48,7 @@ class Chef
       configure_chef
       configure_logging
       configure_encoding
+      emit_warnings
     end
 
     # Get this party started
@@ -79,6 +80,10 @@ class Chef
       end
     end
 
+    def emit_warnings
+      Chef::Log.warn "Chef::Config[:zypper_check_gpg] is set to false which disables security checking on zypper packages" unless Chef::Config[:zypper_check_gpg]
+    end
+
     # Parse configuration (options and config file)
     def configure_chef
       parse_options
diff --git a/lib/chef/provider/package/zypper.rb b/lib/chef/provider/package/zypper.rb
index 45c6c91f60..7b8fd6c426 100644
--- a/lib/chef/provider/package/zypper.rb
+++ b/lib/chef/provider/package/zypper.rb
@@ -2,7 +2,7 @@
 #
 # Authors:: Adam Jacob (<adam@chef.io>)
 #           Ionuț Arțăriși (<iartarisi@suse.cz>)
-# Copyright:: Copyright 2008-2016, Chef Software, Inc.
+# Copyright:: Copyright 2008-2017, Chef Software Inc.
 #             Copyright 2013-2016, SUSE Linux GmbH
 # License:: Apache License, Version 2.0
 #
@@ -145,17 +145,7 @@ class Chef
         end
 
         def gpg_checks
-          case Chef::Config[:zypper_check_gpg]
-          when true
-            nil
-          when false
-            "--no-gpg-checks"
-          when nil
-            Chef::Log.warn("Chef::Config[:zypper_check_gpg] was not set. " \
-              "All packages will be installed without gpg signature checks. " \
-              "This is a security hazard.")
-            "--no-gpg-checks"
-          end
+          "--no-gpg-checks" unless new_resource.gpg_check
         end
       end
     end
diff --git a/lib/chef/resource/zypper_package.rb b/lib/chef/resource/zypper_package.rb
index f9e3eef49e..6c6e308159 100644
--- a/lib/chef/resource/zypper_package.rb
+++ b/lib/chef/resource/zypper_package.rb
@@ -23,6 +23,8 @@ class Chef
     class ZypperPackage < Chef::Resource::Package
       resource_name :zypper_package
       provides :package, platform_family: "suse"
+
+      property :gpg_check, [ TrueClass, FalseClass ], default: lazy { Chef::Config[:zypper_check_gpg] }
     end
   end
 end
diff --git a/spec/unit/provider/package/zypper_spec.rb b/spec/unit/provider/package/zypper_spec.rb
index 7e6f204b64..f3c31dc730 100644
--- a/spec/unit/provider/package/zypper_spec.rb
+++ b/spec/unit/provider/package/zypper_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
-# Copyright:: Copyright 2008-2016, Chef Software, Inc.
+# Copyright:: Copyright 2008-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -111,23 +111,29 @@ describe Chef::Provider::Package::Zypper do
 
   describe "install_package" do
     it "should run zypper install with the package name and version" do
-      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(true)
       shell_out_expectation!(
         "zypper", "--non-interactive", "install", "--auto-agree-with-licenses", "emacs=1.0"
       )
       provider.install_package(["emacs"], ["1.0"])
     end
-    it "should run zypper install without gpg checks" do
-      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
+
+    it "should run zypper install with gpg checks" do
       shell_out_expectation!(
-        "zypper", "--non-interactive", "--no-gpg-checks", "install", "--auto-agree-with-licenses", "emacs=1.0"
+        "zypper", "--non-interactive", "install", "--auto-agree-with-licenses", "emacs=1.0"
       )
       provider.install_package(["emacs"], ["1.0"])
     end
-    it "should warn about gpg checks on zypper install" do
-      expect(Chef::Log).to receive(:warn).with(
-        /All packages will be installed without gpg signature checks/
+
+    it "setting the property should disable gpg checks" do
+      new_resource.gpg_check false
+      shell_out_expectation!(
+        "zypper", "--non-interactive", "--no-gpg-checks", "install", "--auto-agree-with-licenses", "emacs=1.0"
       )
+      provider.install_package(["emacs"], ["1.0"])
+    end
+
+    it "setting the config variable should disable gpg checks" do
+      Chef::Config[:zypper_check_gpg] = false
       shell_out_expectation!(
         "zypper", "--non-interactive", "--no-gpg-checks", "install", "--auto-agree-with-licenses", "emacs=1.0"
       )
@@ -137,29 +143,20 @@ describe Chef::Provider::Package::Zypper do
 
   describe "upgrade_package" do
     it "should run zypper update with the package name and version" do
-      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(true)
       shell_out_expectation!(
         "zypper", "--non-interactive", "install", "--auto-agree-with-licenses", "emacs=1.0"
       )
       provider.upgrade_package(["emacs"], ["1.0"])
     end
-    it "should run zypper update without gpg checks" do
-      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
-      shell_out_expectation!(
-        "zypper", "--non-interactive", "--no-gpg-checks", "install", "--auto-agree-with-licenses", "emacs=1.0"
-      )
-      provider.upgrade_package(["emacs"], ["1.0"])
-    end
-    it "should warn about gpg checks on zypper upgrade" do
-      expect(Chef::Log).to receive(:warn).with(
-        /All packages will be installed without gpg signature checks/
-      )
+    it "should run zypper update without gpg checks when setting the property" do
+      new_resource.gpg_check false
       shell_out_expectation!(
         "zypper", "--non-interactive", "--no-gpg-checks", "install", "--auto-agree-with-licenses", "emacs=1.0"
       )
       provider.upgrade_package(["emacs"], ["1.0"])
     end
-    it "should run zypper upgrade without gpg checks" do
+    it "should run zypper update without gpg checks when setting the config variable" do
+      Chef::Config[:zypper_check_gpg] = false
       shell_out_expectation!(
         "zypper", "--non-interactive", "--no-gpg-checks", "install", "--auto-agree-with-licenses", "emacs=1.0"
       )
@@ -171,7 +168,6 @@ describe Chef::Provider::Package::Zypper do
 
     context "when package version is not explicitly specified" do
       it "should run zypper remove with the package name" do
-        allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(true)
         shell_out_expectation!(
             "zypper", "--non-interactive", "remove", "emacs"
         )
@@ -181,25 +177,22 @@ describe Chef::Provider::Package::Zypper do
 
     context "when package version is explicitly specified" do
       it "should run zypper remove with the package name" do
-        allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(true)
         shell_out_expectation!(
           "zypper", "--non-interactive", "remove", "emacs=1.0"
         )
         provider.remove_package(["emacs"], ["1.0"])
       end
       it "should run zypper remove without gpg checks" do
-        allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
+        new_resource.gpg_check false
         shell_out_expectation!(
             "zypper", "--non-interactive", "--no-gpg-checks", "remove", "emacs=1.0"
         )
         provider.remove_package(["emacs"], ["1.0"])
       end
-      it "should warn about gpg checks on zypper remove" do
-        expect(Chef::Log).to receive(:warn).with(
-          /All packages will be installed without gpg signature checks/
-        )
+      it "should run zypper remove without gpg checks when the config is false" do
+        Chef::Config[:zypper_check_gpg] = false
         shell_out_expectation!(
-          "zypper", "--non-interactive", "--no-gpg-checks", "remove", "emacs=1.0"
+            "zypper", "--non-interactive", "--no-gpg-checks", "remove", "emacs=1.0"
         )
         provider.remove_package(["emacs"], ["1.0"])
       end
@@ -209,21 +202,19 @@ describe Chef::Provider::Package::Zypper do
   describe "purge_package" do
     it "should run remove with the name and version and --clean-deps" do
       shell_out_expectation!(
-        "zypper", "--non-interactive", "--no-gpg-checks", "remove", "--clean-deps", "emacs=1.0"
+        "zypper", "--non-interactive", "remove", "--clean-deps", "emacs=1.0"
       )
       provider.purge_package(["emacs"], ["1.0"])
     end
     it "should run zypper purge without gpg checks" do
-      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
+      new_resource.gpg_check false
       shell_out_expectation!(
         "zypper", "--non-interactive", "--no-gpg-checks", "remove", "--clean-deps", "emacs=1.0"
       )
       provider.purge_package(["emacs"], ["1.0"])
     end
-    it "should warn about gpg checks on zypper purge" do
-      expect(Chef::Log).to receive(:warn).with(
-        /All packages will be installed without gpg signature checks/
-      )
+    it "should run zypper purge without gpg checks when the config is false" do
+      Chef::Config[:zypper_check_gpg] = false
       shell_out_expectation!(
         "zypper", "--non-interactive", "--no-gpg-checks", "remove", "--clean-deps", "emacs=1.0"
       )
@@ -233,29 +224,13 @@ describe Chef::Provider::Package::Zypper do
 
   describe "lock_package" do
     it "should run zypper addlock with the package name" do
-      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(true)
       shell_out_expectation!(
         "zypper", "--non-interactive", "addlock", "emacs"
       )
       provider.lock_package(["emacs"], [nil])
     end
     it "should run zypper addlock without gpg checks" do
-      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
-      shell_out_expectation!(
-        "zypper", "--non-interactive", "--no-gpg-checks", "addlock", "emacs"
-      )
-      provider.lock_package(["emacs"], [nil])
-    end
-    it "should warn about gpg checks on zypper addlock" do
-      expect(Chef::Log).to receive(:warn).with(
-        /All packages will be installed without gpg signature checks/
-      )
-      shell_out_expectation!(
-        "zypper", "--non-interactive", "--no-gpg-checks", "addlock", "emacs"
-      )
-      provider.lock_package(["emacs"], [nil])
-    end
-    it "should run zypper addlock without gpg checks" do
+      new_resource.gpg_check false
       shell_out_expectation!(
         "zypper", "--non-interactive", "--no-gpg-checks", "addlock", "emacs"
       )
@@ -265,29 +240,13 @@ describe Chef::Provider::Package::Zypper do
 
   describe "unlock_package" do
     it "should run zypper removelock with the package name" do
-      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(true)
       shell_out_expectation!(
         "zypper", "--non-interactive", "removelock", "emacs"
       )
       provider.unlock_package(["emacs"], [nil])
     end
     it "should run zypper removelock without gpg checks" do
-      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
-      shell_out_expectation!(
-        "zypper", "--non-interactive", "--no-gpg-checks", "removelock", "emacs"
-      )
-      provider.unlock_package(["emacs"], [nil])
-    end
-    it "should warn about gpg checks on zypper removelock" do
-      expect(Chef::Log).to receive(:warn).with(
-        /All packages will be installed without gpg signature checks/
-      )
-      shell_out_expectation!(
-        "zypper", "--non-interactive", "--no-gpg-checks", "removelock", "emacs"
-      )
-      provider.unlock_package(["emacs"], [nil])
-    end
-    it "should run zypper removelock without gpg checks" do
+      new_resource.gpg_check false
       shell_out_expectation!(
         "zypper", "--non-interactive", "--no-gpg-checks", "removelock", "emacs"
       )
@@ -303,7 +262,7 @@ describe Chef::Provider::Package::Zypper do
     describe "install_package" do
       it "should run zypper install with the package name and version" do
         shell_out_expectation!(
-          "zypper", "--no-gpg-checks", "install", "--auto-agree-with-licenses", "-y", "emacs"
+          "zypper", "install", "--auto-agree-with-licenses", "-y", "emacs"
         )
         provider.install_package(["emacs"], ["1.0"])
       end
@@ -312,7 +271,7 @@ describe Chef::Provider::Package::Zypper do
     describe "upgrade_package" do
       it "should run zypper update with the package name and version" do
         shell_out_expectation!(
-          "zypper", "--no-gpg-checks", "install", "--auto-agree-with-licenses", "-y", "emacs"
+          "zypper", "install", "--auto-agree-with-licenses", "-y", "emacs"
         )
         provider.upgrade_package(["emacs"], ["1.0"])
       end
@@ -321,7 +280,7 @@ describe Chef::Provider::Package::Zypper do
     describe "remove_package" do
       it "should run zypper remove with the package name" do
         shell_out_expectation!(
-           "zypper", "--no-gpg-checks", "remove", "-y", "emacs"
+           "zypper", "remove", "-y", "emacs"
         )
         provider.remove_package(["emacs"], ["1.0"])
       end
@@ -330,17 +289,15 @@ describe Chef::Provider::Package::Zypper do
 
   describe "when installing multiple packages" do # https://github.com/chef/chef/issues/3570
     it "should install an array of package names and versions" do
-      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
       shell_out_expectation!(
-        "zypper", "--non-interactive", "--no-gpg-checks", "install", "--auto-agree-with-licenses", "emacs=1.0", "vim=2.0"
+        "zypper", "--non-interactive", "install", "--auto-agree-with-licenses", "emacs=1.0", "vim=2.0"
       )
       provider.install_package(%w{emacs vim}, ["1.0", "2.0"])
     end
 
     it "should remove an array of package names and versions" do
-      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
       shell_out_expectation!(
-        "zypper", "--non-interactive", "--no-gpg-checks", "remove", "emacs=1.0", "vim=2.0"
+        "zypper", "--non-interactive", "remove", "emacs=1.0", "vim=2.0"
       )
       provider.remove_package(%w{emacs vim}, ["1.0", "2.0"])
     end