-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
cask/audit: update signing checks for app, binary, and pkg #17031
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -482,13 +482,25 @@ | |
odebug "Auditing signing" | ||
|
||
extract_artifacts do |artifacts, tmpdir| | ||
is_container = artifacts.any? { |a| a.is_a?(Artifact::App) || a.is_a?(Artifact::Pkg) } | ||
|
||
artifacts.each do |artifact| | ||
next if artifact.is_a?(Artifact::Binary) && is_container == true | ||
|
||
artifact_path = artifact.is_a?(Artifact::Pkg) ? artifact.path : artifact.source | ||
path = tmpdir/artifact_path.relative_path_from(cask.staged_path) | ||
|
||
next unless path.exist? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was silently skipping over paths not found, which was causing us not to audit direct binary downloads properly. |
||
path = tmpdir/artifact_path.relative_path_from(cask.staged_path) | ||
|
||
result = system_command("spctl", args: ["--assess", "--type", "install", path], print_stderr: false) | ||
result = case artifact | ||
when Artifact::Pkg | ||
system_command("spctl", args: ["--assess", "--type", "install", path], print_stderr: false) | ||
when Artifact::App | ||
system_command("spctl", args: ["--assess", "--type", "execute", path], print_stderr: false) | ||
when Artifact::Binary | ||
system_command("codesign", args: ["--verify", path], print_stderr: false) | ||
else | ||
add_error "Unknown artifact type: #{artifact.class}", location: cask.url.location | ||
end | ||
Comment on lines
+495
to
+503
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tested against a range of applications and binaries, this reports the same as tools like Apparency and Whatsyoursign. Happy to publish a results report run against applications locally for review. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, I believe you, great work! |
||
|
||
next if result.success? | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ def extract_nestedly(to: nil, basename: nil, verbose: false, prioritize_extensio | |
|
||
sig { override.params(unpack_dir: Pathname, basename: Pathname, verbose: T::Boolean).returns(T.untyped) } | ||
def extract_to_dir(unpack_dir, basename:, verbose: false) | ||
FileUtils.cp path, unpack_dir/basename, preserve: true, verbose: | ||
FileUtils.cp path, unpack_dir/basename.sub(/^[\da-f]{64}--/, ""), preserve: true, verbose: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed during the audit signing that this caused audit to fail as the paths didn't exist. This applied to direct binary downloads only. Making this adjustment now causes the paths to be found. |
||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's a standalone binary (typically a shell script) included in or with an application bundle, we can skip auditing it.
Many times those scripts aren't signed, but if the application is we'll consider it trusted. If the app isn't trusted, it will fail the normal check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!