CHEF-5815: Update signing of all msi packages with new cli tool - `sm…

…ctl` (#1128) * PARAMS: Add keypair_alias to signing_identity hash to be received in params Signed-off-by: Sonu Saha <[email protected]> * TOOL: Add signing capability with new tool Signed-off-by: Sonu Saha <[email protected]> * CHORE: Add some debug statements with puts - remove this later Signed-off-by: Sonu Saha <[email protected]> * Update signing command for smctl with --fingerprint Signed-off-by: Sonu Saha <[email protected]> * testing patch here, trying to remove + Signed-off-by: Sean Simmons <[email protected]> * reverting this change, it did not remove the + in the file name Signed-off-by: Sean Simmons <[email protected]> * HACK: Temporary workaround to remove '+' from package name; causing signing to fail Signed-off-by: Sonu Saha <[email protected]> * REVERT changes made to subsituting + with _ Signed-off-by: Sonu Saha <[email protected]> * Update semver to include '_' instead of '+' Signed-off-by: Sonu Saha <[email protected]> * CHORE: Add temporary logs & change + to _ Signed-off-by: Sonu Saha <[email protected]> * changing from _ to - Signed-off-by: Sean Simmons <[email protected]> * changing from _ to - Signed-off-by: Sean Simmons <[email protected]> * changing from _ to - Signed-off-by: Sean Simmons <[email protected]> * CHEF-5815: Update temporary log messages - we are retaining it Signed-off-by: Sonu Saha <[email protected]> * CHEF-5815: Remove old method for signing Signed-off-by: Sonu Saha <[email protected]> * LINT: Fix lint offense and missing end Signed-off-by: Sonu Saha <[email protected]> * SPECS: Fix test as per new signing method changes Signed-off-by: Sonu Saha <[email protected]> * removing log.info and adding log.debug Signed-off-by: Sean Simmons <[email protected]> * updating based off Marc's Comment's Signed-off-by: Sean Simmons <[email protected]> * CHORE: Update log messages Signed-off-by: Sonu Saha <[email protected]> --------- Signed-off-by: Sonu Saha <[email protected]> Signed-off-by: Sean Simmons <[email protected]> Co-authored-by: Sean Simmons <[email protected]>
chef · Sep 22, 2023 · ad5e90b · ad5e90b
1 parent 26c34d2
commit ad5e90b
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 98 deletions.
diff --git a/lib/omnibus/build_version.rb b/lib/omnibus/build_version.rb
@@ -87,13 +87,18 @@ def initialize(path = Config.project_root)
     #   and can be influenced by users.
     def semver
       build_tag = version_tag
+      log.debug(log_key) { "#{self.class}##{__method__} - build tag: #{build_tag}" }
 
       # PRERELEASE VERSION
+      log.debug(log_key) { "#{self.class}##{__method__} - prerelease_version?: #{prerelease_version?}" }
+
       if prerelease_version?
         # ensure all dashes are dots per precedence rules (#12) in Semver
         # 2.0.0-rc.1
+        log.debug(log_key) { "#{self.class}##{__method__} - prerelease_tag: #{prerelease_tag}" }
         prerelease = prerelease_tag.tr("-", ".")
         build_tag << "-" << prerelease
+        log.debug(log_key) { "#{self.class}##{__method__} - build_tag after prerelease: #{build_tag}" }
       end
 
       # BUILD VERSION
@@ -106,6 +111,7 @@ def semver
       #
       # format: YYYYMMDDHHMMSS example: 20130131123345
       if Config.append_timestamp
+        log.debug(log_key) { "#{self.class}##{__method__} - build_start_time: #{build_start_time}" }
         build_version_items << build_start_time
       end
 
@@ -114,13 +120,18 @@ def semver
       #
       # format: git.COMMITS_SINCE_TAG.GIT_SHA example: git.207.694b062
       unless commits_since_tag == 0
+        log.debug(log_key) { "#{self.class}##{__method__} - commits_since_tag: #{commits_since_tag}" }
+        log.debug(log_key) { "#{self.class}##{__method__} - git_sha_tag: #{git_sha_tag}" }
         build_version_items << ["git", commits_since_tag, git_sha_tag].join(".")
       end
 
       unless build_version_items.empty?
-        build_tag << "+" << build_version_items.join(".")
+        log.debug(log_key) { "#{self.class}##{__method__} - build_version_items: #{build_version_items}" }
+        build_tag << "-" << build_version_items.join(".")
       end
 
+      log.debug(log_key) { "#{self.class}##{__method__} - final build_tag returned: #{build_tag}" }
+
       build_tag
     end
 

diff --git a/lib/omnibus/build_version_dsl.rb b/lib/omnibus/build_version_dsl.rb
@@ -117,8 +117,11 @@ def build_version=(new_version)
     # @param [String] version
     # @return [String]
     def maybe_append_timestamp(version)
+      log.debug(log_key) { "#{self.class}##{__method__} - Config.append_timestamp: #{Config.append_timestamp}" }
+      log.debug(log_key) { "#{self.class}##{__method__} -  version: #{version}" }
+      log.debug(log_key) { "#{self.class}##{__method__} - has_timestamp?(version): #{has_timestamp?(version)}" }
       if Config.append_timestamp && !has_timestamp?(version)
-        [version, Omnibus::BuildVersion.build_start_time].join("+")
+        [version, Omnibus::BuildVersion.build_start_time].join("-")
       else
         version
       end
@@ -132,7 +135,7 @@ def maybe_append_timestamp(version)
     # @param [String] version
     # @return [Boolean]
     def has_timestamp?(version)
-      _ver, build_info = version.split("+")
+      _ver, build_info = version.split("-")
       return false if build_info.nil?
 
       build_info.split(".").any? do |part|

diff --git a/lib/omnibus/packagers/appx.rb b/lib/omnibus/packagers/appx.rb
@@ -52,6 +52,11 @@ class Packager::APPX < Packager::WindowsBase
 
     # @see Base#package_name
     def package_name
+      log.debug(log_key) { "#{self.class}##{__method__} - package_name: #{project.package_name}" }
+      log.debug(log_key) { "#{self.class}##{__method__} - build_version: #{project.build_version}" }
+      log.debug(log_key) { "#{self.class}##{__method__} - build_iteration: #{project.build_iteration}" }
+      log.debug(log_key) { "#{self.class}##{__method__} - Config.windows_arch: #{Config.windows_arch}" }
+
       "#{project.package_name}-#{project.build_version}-#{project.build_iteration}-#{Config.windows_arch}.appx"
     end
 

diff --git a/lib/omnibus/packagers/windows_base.rb b/lib/omnibus/packagers/windows_base.rb
@@ -16,9 +16,6 @@
 
 module Omnibus
   class Packager::WindowsBase < Packager::Base
-    DEFAULT_TIMESTAMP_SERVERS = ["http://timestamp.digicert.com",
-                                 "http://timestamp.verisign.com/scripts/timestamp.dll"].freeze
-
     #
     # Set the signing certificate name
     #
@@ -59,9 +56,18 @@ def signing_identity(thumbprint = NULL, params = NULL)
             raise InvalidValue.new(:params, "be a Hash")
           end
 
-          valid_keys = %i{store timestamp_servers machine_store algorithm}
+          valid_keys = %i{store machine_store algorithm keypair_alias}
           invalid_keys = params.keys - valid_keys
           unless invalid_keys.empty?
+
+            # log a deprecated warning if timestamp_server is used
+            if invalid_keys.include?(:timestamp_servers)
+              log.deprecated(log_key) do
+                "The signing_identity is updated to use smctl.exe. which does not require timestamp_servers" \
+                "Please remove timestamp_servers from your signing_identity"
+              end
+            end
+
             raise InvalidValue.new(:params, "contain keys from [#{valid_keys.join(", ")}]. "\
                                    "Found invalid keys [#{invalid_keys.join(", ")}]")
           end
@@ -77,9 +83,8 @@ def signing_identity(thumbprint = NULL, params = NULL)
 
         @signing_identity[:store] = params[:store] || "My"
         @signing_identity[:algorithm] = params[:algorithm] || "SHA256"
-        servers = params[:timestamp_servers] || DEFAULT_TIMESTAMP_SERVERS
-        @signing_identity[:timestamp_servers] = [servers].flatten
         @signing_identity[:machine_store] = params[:machine_store] || false
+        @signing_identity[:keypair_alias] = params[:keypair_alias]
       end
 
       @signing_identity
@@ -102,41 +107,41 @@ def timestamp_servers
       signing_identity[:timestamp_servers]
     end
 
+    def keypair_alias
+      signing_identity[:keypair_alias]
+    end
+
     def machine_store?
       signing_identity[:machine_store]
     end
 
-    #
-    # Iterates through available timestamp servers and tries to sign
-    # the file with with each server, stopping after the first to succeed.
-    # If none succeed, an exception is raised.
-    #
+    # signs the package with the given certificate
     def sign_package(package_file)
-      success = false
-      timestamp_servers.each do |ts|
-        success = try_sign(package_file, ts)
-        break if success
-      end
-      raise FailedToSignWindowsPackage.new unless success
+      raise FailedToSignWindowsPackage.new unless is_signed?(package_file)
     end
 
-    def try_sign(package_file, url)
+    def is_signed?(package_file)
       cmd = [].tap do |arr|
-        arr << "signtool.exe"
-        arr << "sign /v"
-        arr << "/t #{url}"
-        arr << "/fd #{algorithm}"
-        arr << "/sm" if machine_store?
-        arr << "/s #{cert_store_name}"
-        arr << "/sha1 #{thumbprint}"
-        arr << "/d #{project.package_name}"
-        arr << "\"#{package_file}\""
+        arr << "smctl.exe"
+        arr << "sign"
+        arr << "--fingerprint #{thumbprint}"
+        arr << "--input #{package_file}"
       end.join(" ")
+
       status = shellout(cmd)
+
+      log.debug(log_key) { "#{self.class}##{__method__} - package_file: #{package_file}" }
+      log.debug(log_key) { "#{self.class}##{__method__} - cmd: #{cmd}" }
+      log.debug(log_key) { "#{self.class}##{__method__} - status: #{status}" }
+      log.debug(log_key) { "#{self.class}##{__method__} - status.exitstatus: #{status.exitstatus}" }
+      log.debug(log_key) { "#{self.class}##{__method__} - status.stdout: #{status.stdout}" }
+      log.debug(log_key) { "#{self.class}##{__method__} - status.stderr: #{status.stderr}" }
+
+      # log the error if the signing failed
       if status.exitstatus != 0
         log.warn(log_key) do
           <<-EOH.strip
-                Failed to add timestamp with timeserver #{url}
+                Failed to verify signature of #{package_file}
 
                 STDOUT
                 ------
@@ -148,6 +153,7 @@ def try_sign(package_file, url)
           EOH
         end
       end
+
       status.exitstatus == 0
     end
 

diff --git a/spec/unit/build_version_dsl_spec.rb b/spec/unit/build_version_dsl_spec.rb
@@ -34,19 +34,19 @@ module Omnibus
       before { Config.append_timestamp(true) }
 
       it "appends a timestamp to a static (String) version" do
-        expect(subject_with_version.build_version).to eq("1.0.0+#{today_string}")
+        expect(subject_with_version.build_version).to eq("1.0.0-#{today_string}")
       end
 
       it "doesn't append timestamp to something that already looks like it has a timestamp" do
-        semver = "1.0.0+#{today_string}.git.222.694b062"
-        expect(described_class.new(semver).build_version).to eq("1.0.0+#{today_string}.git.222.694b062")
+        semver = "1.0.0-#{today_string}.git.222.694b062"
+        expect(described_class.new(semver).build_version).to eq("1.0.0-#{today_string}.git.222.694b062")
       end
 
       it "appends a timestamp to a DSL-built version" do
         allow(BuildVersion).to receive(:new).and_return(BuildVersion.new)
         allow(BuildVersion).to receive(:new).with("/etc/zoo").and_return(zoo_version)
         subject_with_description.resolve(zoo_software)
-        expect(subject_with_description.build_version).to eq("5.5.5+#{today_string}")
+        expect(subject_with_description.build_version).to eq("5.5.5-#{today_string}")
       end
     end
 

diff --git a/spec/unit/build_version_spec.rb b/spec/unit/build_version_spec.rb
@@ -109,12 +109,12 @@ module Omnibus
       end
 
       it "generates a version matching format 'MAJOR.MINOR.PATCH-PRERELEASE+TIMESTAMP.git.COMMITS_SINCE.GIT_SHA'" do
-        expect(build_version.semver).to match(/11.0.0-alpha1\+#{today_string}[0-9]+.git.207.694b062/)
+        expect(build_version.semver).to match(/11.0.0-alpha1\-#{today_string}[0-9]+.git.207.694b062/)
       end
 
       it "uses ENV['BUILD_TIMESTAMP'] to generate timestamp if set" do
         stub_env("BUILD_TIMESTAMP", "2012-12-25_16-41-40")
-        expect(build_version.semver).to eq("11.0.0-alpha1+20121225164140.git.207.694b062")
+        expect(build_version.semver).to eq("11.0.0-alpha1-20121225164140.git.207.694b062")
       end
 
       it "fails on invalid ENV['BUILD_TIMESTAMP'] values" do
@@ -124,7 +124,7 @@ module Omnibus
 
       it "uses ENV['BUILD_ID'] to generate timestamp if set and BUILD_TIMESTAMP is not set" do
         stub_env("BUILD_ID", "2012-12-25_16-41-40")
-        expect(build_version.semver).to eq("11.0.0-alpha1+20121225164140.git.207.694b062")
+        expect(build_version.semver).to eq("11.0.0-alpha1-20121225164140.git.207.694b062")
       end
 
       it "fails on invalid ENV['BUILD_ID'] values" do
@@ -136,36 +136,36 @@ module Omnibus
         let(:git_describe) { "11.0.0-alpha-3-207-g694b062" }
 
         it "converts all dashes to dots" do
-          expect(build_version.semver).to match(/11.0.0-alpha.3\+#{today_string}[0-9]+.git.207.694b062/)
+          expect(build_version.semver).to match(/11.0.0-alpha.3\-#{today_string}[0-9]+.git.207.694b062/)
         end
       end
 
       context "exact version" do
         let(:git_describe) { "11.0.0-alpha2" }
 
         it "appends a timestamp with no git info" do
-          expect(build_version.semver).to match(/11.0.0-alpha2\+#{today_string}[0-9]+/)
+          expect(build_version.semver).to match(/11.0.0-alpha2\-#{today_string}[0-9]+/)
         end
       end
 
       describe "appending a timestamp" do
         let(:git_describe) { "11.0.0-alpha-3-207-g694b062" }
         context "by default" do
           it "appends a timestamp" do
-            expect(build_version.semver).to match(/11.0.0-alpha.3\+#{today_string}[0-9]+.git.207.694b062/)
+            expect(build_version.semver).to match(/11.0.0-alpha.3\-#{today_string}[0-9]+.git.207.694b062/)
           end
         end
 
         context "when Config.append_timestamp is true" do
           it "appends a timestamp" do
-            expect(build_version.semver).to match(/11.0.0-alpha.3\+#{today_string}[0-9]+.git.207.694b062/)
+            expect(build_version.semver).to match(/11.0.0-alpha.3\-#{today_string}[0-9]+.git.207.694b062/)
           end
         end
 
         context "when Config.append_timestamp is false" do
           before { Config.append_timestamp(false) }
           it "does not append a timestamp" do
-            expect(build_version.semver).to match(/11.0.0-alpha.3\+git.207.694b062/)
+            expect(build_version.semver).to match(/11.0.0-alpha.3\-git.207.694b062/)
           end
         end
       end

diff --git a/spec/unit/packagers/appx_spec.rb b/spec/unit/packagers/appx_spec.rb
@@ -129,39 +129,18 @@ module Omnibus
           allow(subject).to receive(:shellout!)
         end
 
-        describe "#timestamp_servers" do
-          it "defaults to using ['http://timestamp.digicert.com','http://timestamp.verisign.com/scripts/timestamp.dll']" do
+        describe "#keypair_alias" do
+          it "defaults to 'Chef Software, Inc.'" do
             subject.signing_identity("foo")
-            expect(subject).to receive(:try_sign).with(appx, "http://timestamp.digicert.com").and_return(false)
-            expect(subject).to receive(:try_sign).with(appx, "http://timestamp.verisign.com/scripts/timestamp.dll").and_return(true)
+            expect(subject).to receive(:is_signed?).with(appx).and_return(true)
             subject.sign_package(appx)
           end
 
-          it "uses the timestamp server if provided through the #timestamp_server dsl" do
-            subject.signing_identity("foo", timestamp_servers: "http://fooserver")
-            expect(subject).to receive(:try_sign).with(appx, "http://fooserver").and_return(true)
+          it "uses the keypair alias if provided through the #keypair_alias dsl" do
+            subject.signing_identity("foo", keypair_alias: "bar")
+            expect(subject).to receive(:is_signed?).with(appx).and_return(true)
             subject.sign_package(appx)
           end
-
-          it "tries all timestamp server if provided through the #timestamp_server dsl" do
-            subject.signing_identity("foo", timestamp_servers: ["http://fooserver", "http://barserver"])
-            expect(subject).to receive(:try_sign).with(appx, "http://fooserver").and_return(false)
-            expect(subject).to receive(:try_sign).with(appx, "http://barserver").and_return(true)
-            subject.sign_package(appx)
-          end
-
-          it "tries all timestamp server if provided through the #timestamp_servers dsl and stops at the first available" do
-            subject.signing_identity("foo", timestamp_servers: ["http://fooserver", "http://barserver"])
-            expect(subject).to receive(:try_sign).with(appx, "http://fooserver").and_return(true)
-            expect(subject).not_to receive(:try_sign).with(appx, "http://barserver")
-            subject.sign_package(appx)
-          end
-
-          it "raises an exception if there are no available timestamp servers" do
-            subject.signing_identity("foo", timestamp_servers: "http://fooserver")
-            expect(subject).to receive(:try_sign).with(appx, "http://fooserver").and_return(false)
-            expect { subject.sign_package(appx) }.to raise_error(FailedToSignWindowsPackage)
-          end
         end
       end
     end

diff --git a/spec/unit/packagers/msi_spec.rb b/spec/unit/packagers/msi_spec.rb
@@ -554,37 +554,16 @@ module Omnibus
           allow(subject).to receive(:shellout!)
         end
 
-        describe "#timestamp_servers" do
-          it "defaults to using ['http://timestamp.digicert.com','http://timestamp.verisign.com/scripts/timestamp.dll']" do
-            subject.signing_identity("foo")
-            expect(subject).to receive(:try_sign).with(msi, "http://timestamp.digicert.com").and_return(false)
-            expect(subject).to receive(:try_sign).with(msi, "http://timestamp.verisign.com/scripts/timestamp.dll").and_return(true)
+        describe "#keypair_alias" do
+          it "uses the keypair alias if provided through the #keypair_alias dsl" do
+            subject.signing_identity("foo", keypair_alias: "bar")
+            expect(subject).to receive(:is_signed?).with(msi).and_return(true)
             subject.sign_package(msi)
           end
 
-          it "uses the timestamp server if provided through the #timestamp_server dsl" do
-            subject.signing_identity("foo", timestamp_servers: "http://fooserver")
-            expect(subject).to receive(:try_sign).with(msi, "http://fooserver").and_return(true)
-            subject.sign_package(msi)
-          end
-
-          it "tries all timestamp server if provided through the #timestamp_server dsl" do
-            subject.signing_identity("foo", timestamp_servers: ["http://fooserver", "http://barserver"])
-            expect(subject).to receive(:try_sign).with(msi, "http://fooserver").and_return(false)
-            expect(subject).to receive(:try_sign).with(msi, "http://barserver").and_return(true)
-            subject.sign_package(msi)
-          end
-
-          it "tries all timestamp server if provided through the #timestamp_servers dsl and stops at the first available" do
-            subject.signing_identity("foo", timestamp_servers: ["http://fooserver", "http://barserver"])
-            expect(subject).to receive(:try_sign).with(msi, "http://fooserver").and_return(true)
-            expect(subject).not_to receive(:try_sign).with(msi, "http://barserver")
-            subject.sign_package(msi)
-          end
-
-          it "raises an exception if there are no available timestamp servers" do
-            subject.signing_identity("foo", timestamp_servers: "http://fooserver")
-            expect(subject).to receive(:try_sign).with(msi, "http://fooserver").and_return(false)
+          it "raises an exception if the signing fails" do
+            subject.signing_identity("foo", keypair_alias: "bar")
+            expect(subject).to receive(:is_signed?).with(msi).and_return(false)
             expect { subject.sign_package(msi) }.to raise_error(FailedToSignWindowsPackage)
           end
         end