Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CHEF-5815: Update signing of all msi packages with new cli tool - smctl #1128

Merged
merged 20 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
ba900fb
PARAMS: Add keypair_alias to signing_identity hash to be received in …
ahasunos Sep 13, 2023
136f6eb
TOOL: Add signing capability with new tool
ahasunos Sep 13, 2023
540d487
CHORE: Add some debug statements with puts - remove this later
ahasunos Sep 13, 2023
9b3c8f2
Update signing command for smctl with --fingerprint
ahasunos Sep 13, 2023
fad5ed4
testing patch here, trying to remove +
sean-simmons-progress Sep 14, 2023
993aed2
reverting this change, it did not remove the + in the file name
sean-simmons-progress Sep 14, 2023
db439e4
HACK: Temporary workaround to remove '+' from package name; causing s…
ahasunos Sep 14, 2023
806826c
REVERT changes made to subsituting + with _
ahasunos Sep 14, 2023
a03e03b
Update semver to include '_' instead of '+'
ahasunos Sep 14, 2023
10269ea
CHORE: Add temporary logs & change + to _
ahasunos Sep 14, 2023
e8d08c5
changing from _ to -
sean-simmons-progress Sep 14, 2023
2546a1d
changing from _ to -
sean-simmons-progress Sep 14, 2023
2fef25c
changing from _ to -
sean-simmons-progress Sep 14, 2023
08f3a22
CHEF-5815: Update temporary log messages - we are retaining it
ahasunos Sep 14, 2023
0e583f2
CHEF-5815: Remove old method for signing
ahasunos Sep 14, 2023
a6b6058
LINT: Fix lint offense and missing end
ahasunos Sep 14, 2023
cd05cc5
SPECS: Fix test as per new signing method changes
ahasunos Sep 14, 2023
e06e993
removing log.info and adding log.debug
sean-simmons-progress Sep 19, 2023
79775ae
updating based off Marc's Comment's
sean-simmons-progress Sep 19, 2023
004d635
CHORE: Update log messages
ahasunos Sep 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion lib/omnibus/build_version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,18 @@ def initialize(path = Config.project_root)
# and can be influenced by users.
def semver
build_tag = version_tag
log.info(log_key) { "class Omnibus::BuildVersion - semver method: build tag: #{build_tag}" }
ahasunos marked this conversation as resolved.
Show resolved Hide resolved

# PRERELEASE VERSION
log.info(log_key) { "class Omnibus::BuildVersion - semver 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.info(log_key) { "class Omnibus::BuildVersion - semver method: prerelease_tag: #{prerelease_tag}" }
prerelease = prerelease_tag.tr("-", ".")
build_tag << "-" << prerelease
log.info(log_key) { "class Omnibus::BuildVersion - semver method: build_tag after prerelease: #{build_tag}" }
end

# BUILD VERSION
Expand All @@ -106,6 +111,7 @@ def semver
#
# format: YYYYMMDDHHMMSS example: 20130131123345
if Config.append_timestamp
log.info(log_key) { "class Omnibus::BuildVersion - semver method: build_start_time: #{build_start_time}" }
build_version_items << build_start_time
end

Expand All @@ -114,13 +120,18 @@ def semver
#
# format: git.COMMITS_SINCE_TAG.GIT_SHA example: git.207.694b062
unless commits_since_tag == 0
log.info(log_key) { "class Omnibus::BuildVersion - semver method: commits_since_tag: #{commits_since_tag}" }
log.info(log_key) { "class Omnibus::BuildVersion - semver 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.info(log_key) { "class Omnibus::BuildVersion - semver method: build_version_items: #{build_version_items}" }
build_tag << "-" << build_version_items.join(".")
end

log.info(log_key) { "class Omnibus::BuildVersion - semver method: final build_tag returned: #{build_tag}" }

build_tag
end

Expand Down
7 changes: 5 additions & 2 deletions lib/omnibus/build_version_dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,11 @@ def build_version=(new_version)
# @param [String] version
# @return [String]
def maybe_append_timestamp(version)
log.info(log_key) { "class Omnibus::BuildVersionDSL - maybe_append_timestamp method: Config.append_timestamp: #{Config.append_timestamp}" }
ahasunos marked this conversation as resolved.
Show resolved Hide resolved
log.info(log_key) { "class Omnibus::BuildVersionDSL - maybe_append_timestamp method: version: #{version}" }
log.info(log_key) { "class Omnibus::BuildVersionDSL - maybe_append_timestamp 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("-")
sean-simmons-progress marked this conversation as resolved.
Show resolved Hide resolved
else
version
end
Expand All @@ -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|
Expand Down
5 changes: 5 additions & 0 deletions lib/omnibus/packagers/appx.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ class Packager::APPX < Packager::WindowsBase

# @see Base#package_name
def package_name
log.info(log_key) { "class Omnibus::Packager::APPX - package_name method: package_name: #{project.package_name}" }
log.info(log_key) { "class Omnibus::Packager::APPX - package_name method: build_version: #{project.build_version}" }
log.info(log_key) { "class Omnibus::Packager::APPX - package_name method: build_iteration: #{project.build_iteration}" }
log.info(log_key) { "class Omnibus::Packager::APPX - package_name method: Config.windows_arch: #{Config.windows_arch}" }

"#{project.package_name}-#{project.build_version}-#{project.build_iteration}-#{Config.windows_arch}.appx"
end

Expand Down
62 changes: 34 additions & 28 deletions lib/omnibus/packagers/windows_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"
marcparadise marked this conversation as resolved.
Show resolved Hide resolved
marcparadise marked this conversation as resolved.
Show resolved Hide resolved
arr << "sign"
arr << "--fingerprint #{thumbprint}"
arr << "--input #{package_file}"
end.join(" ")

status = shellout(cmd)

log.info(log_key) { "class Omnibus::Packager::WindowsBase - is_signed? method: package_file: #{package_file}" }
ahasunos marked this conversation as resolved.
Show resolved Hide resolved
log.info(log_key) { "class Omnibus::Packager::WindowsBase - is_signed? method: cmd: #{cmd}" }
log.info(log_key) { "class Omnibus::Packager::WindowsBase - is_signed? method: status: #{status}" }
log.info(log_key) { "class Omnibus::Packager::WindowsBase - is_signed? method: status.exitstatus: #{status.exitstatus}" }
log.info(log_key) { "class Omnibus::Packager::WindowsBase - is_signed? method: status.stdout: #{status.stdout}" }
log.info(log_key) { "class Omnibus::Packager::WindowsBase - is_signed? 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
------
Expand All @@ -148,6 +153,7 @@ def try_sign(package_file, url)
EOH
end
end

status.exitstatus == 0
end

Expand Down
8 changes: 4 additions & 4 deletions spec/unit/build_version_dsl_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 8 additions & 8 deletions spec/unit/build_version_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
33 changes: 6 additions & 27 deletions spec/unit/packagers/appx_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 7 additions & 28 deletions spec/unit/packagers/msi_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down