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.

Sign up for GitHub

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

Jump to bottom

add support for development signing #1247

Merged
merged 2 commits into from
Jan 12, 2019
Merged

add support for development signing #1247

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95811c4
Merge pull request #1198 from brave/issues/2794
tmancey Jan 8, 2019
6c7a72f
Merge pull request #1253 from brave/issues/2794
bridiver Jan 8, 2019
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
7 changes: 1 addition & 6 deletions BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,12 +407,7 @@ if (is_mac) {
"CHROMIUM_SHORT_NAME=$chrome_product_short_name",
"CHROMIUM_CREATOR=$chrome_mac_creator_code",
]
# brave_channel is an empty string for release
if (brave_channel == "") {
extra_substitutions += ["CHROMIUM_BUNDLE_ID=$chrome_mac_bundle_id"]
} else {
extra_substitutions += ["CHROMIUM_BUNDLE_ID=$chrome_mac_bundle_id.$brave_channel"]
}
extra_substitutions += ["CHROMIUM_BUNDLE_ID=$chrome_mac_bundle_id"]

sources = [
"//chrome/app/chrome_exe_main_mac.cc",
Expand Down
2 changes: 1 addition & 1 deletion app/theme/brave/BRANDING.beta
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ PRODUCT_SHORTNAME=Brave
PRODUCT_INSTALLER_FULLNAME=Brave Installer
PRODUCT_INSTALLER_SHORTNAME=Brave Installer
COPYRIGHT=Copyright 2016 The Brave Authors. All rights reserved.
MAC_BUNDLE_ID=com.brave.Browser
MAC_BUNDLE_ID=com.brave.Browser.beta
MAC_CREATOR_CODE=Cr24
MAC_TEAM_ID=KL8N8XSYF4
2 changes: 1 addition & 1 deletion app/theme/brave/BRANDING.dev
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ PRODUCT_SHORTNAME=Brave
PRODUCT_INSTALLER_FULLNAME=Brave Installer
PRODUCT_INSTALLER_SHORTNAME=Brave Installer
COPYRIGHT=Copyright 2016 The Brave Authors. All rights reserved.
MAC_BUNDLE_ID=com.brave.Browser
MAC_BUNDLE_ID=com.brave.Browser.dev
MAC_CREATOR_CODE=Cr24
MAC_TEAM_ID=KL8N8XSYF4
4 changes: 2 additions & 2 deletions app/theme/brave/BRANDING.development
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ PRODUCT_SHORTNAME=Brave
PRODUCT_INSTALLER_FULLNAME=Brave Installer
PRODUCT_INSTALLER_SHORTNAME=Brave Installer
COPYRIGHT=Copyright 2016 The Brave Authors. All rights reserved.
MAC_BUNDLE_ID=com.brave.Browser
MAC_BUNDLE_ID=com.brave.Browser.development
MAC_CREATOR_CODE=Cr24
MAC_TEAM_ID=
MAC_TEAM_ID=KL8N8XSYF4
2 changes: 1 addition & 1 deletion app/theme/brave/BRANDING.nightly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ PRODUCT_SHORTNAME=Brave
PRODUCT_INSTALLER_FULLNAME=Brave Installer
PRODUCT_INSTALLER_SHORTNAME=Brave Installer
COPYRIGHT=Copyright 2016 The Brave Authors. All rights reserved.
MAC_BUNDLE_ID=com.brave.Browser
MAC_BUNDLE_ID=com.brave.Browser.nightly
MAC_CREATOR_CODE=Cr24
MAC_TEAM_ID=KL8N8XSYF4
40 changes: 27 additions & 13 deletions build/mac/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import("//brave/build/config.gni")
import("//build/config/mac/base_rules.gni")

declare_args() {
# find with `security find-identity -v -p codesigning`
mac_signing_identifier = ""
mac_installer_signing_identifier = ""
mac_signing_keychain = "login"
mac_signing_output_prefix = "signing"
}

_packaging_dir = "$root_out_dir/$chrome_product_full_name Packaging"
keychain_db = getenv("HOME") + "/Library/Keychains/${mac_signing_keychain}.keychain-db"

_target_app_path = "$root_out_dir/signing/$brave_exe"
_target_app_path = "$root_out_dir/$mac_signing_output_prefix/$brave_exe"

group("brave") {}

Expand All @@ -18,19 +21,29 @@ action("sign_app") {

script = "//build/gn_run_binary.py"
shell_script = "//brave/build/mac/sign_app.sh"
if (brave_channel == "") {
provisioning_profile = "//brave/build/mac/release.provisionprofile"
}
else {
provisioning_profile = "//brave/build/mac/${brave_channel}.provisionprofile"
}

deps = [
"//brave:chrome_app",
"//chrome/installer/mac"
]

if (is_official_build) {
if (brave_channel == "") {
provisioning_profile = "//brave/build/mac/release.provisionprofile"
} else {
provisioning_profile = "//brave/build/mac/${brave_channel}.provisionprofile"
}
} else {
# an empty dummy file just to simplify things
provisioning_profile = "//brave/build/mac/dummy.provisionprofile"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is changing; is an empty dummy.provisionprofile OK for non-official builds? (have you tested this code-path?)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic isn't changing, signing never worked for non-official builds. This is in fact the only code path I have tested because I can't test the dev/beta signing

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for dev signing we only pass a provision profile to simplify things with the number of arguments. It isn't actually used in the signing script when --development (below) is passed

}
inputs = [
shell_script,
brave_app,
provisioning_profile,
"$_packaging_dir/sign_app.sh",
"$_packaging_dir/sign_versioned_dir.sh",
"$_packaging_dir/entitlements.plist",
]
outputs = [ _target_app_path ]
args = [
Expand All @@ -43,11 +56,12 @@ action("sign_app") {
keychain_db,
mac_signing_identifier,
rebase_path(provisioning_profile, root_out_dir),
rebase_path("$_packaging_dir/entitlements.plist"),
]
deps = [
"//brave:chrome_app",
"//chrome/installer/mac"
]

if (!is_official_build) {
args += [ "--development" ]
}
}

action("create_pkg") {
Expand Down Expand Up @@ -78,7 +92,7 @@ action("sign_pkg") {
shell_script,
"$root_out_dir/unsigned/$brave_pkg",
]
outputs = [ "${root_out_dir}/signing/$brave_pkg" ]
outputs = [ "${root_out_dir}/$mac_signing_output_prefix/$brave_pkg" ]
args = [
rebase_path(shell_script, root_build_dir),
rebase_path("$root_out_dir/unsigned/$brave_pkg"),
Expand Down Expand Up @@ -122,7 +136,7 @@ action("sign_dmg") {
shell_script,
"$root_out_dir/unsigned/$brave_dmg",
]
outputs = [ "${root_out_dir}/signing/$brave_dmg" ]
outputs = [ "${root_out_dir}/$mac_signing_output_prefix/$brave_dmg" ]
args = [
rebase_path(shell_script, root_build_dir),
rebase_path("$root_out_dir/unsigned/$brave_dmg"),
Expand Down
Empty file added build/mac/dummy.provisionprofile
View file
Open in desktop
Empty file.
14 changes: 9 additions & 5 deletions build/mac/sign_app.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
set -euo pipefail

if [[ $# -lt "4" ]]; then
echo "usage: $0 <source_app> <dest_app> <packaging_dir> <mac_signing_keychain> <mac_signing_identifier> <mac_provisioning_profile>"
echo "usage: $0 <source_app> <dest_app> <packaging_dir> <mac_signing_keychain> <mac_signing_identifier> <mac_provisioning_profile> <extra_flags>"
exit 1
fi

Expand All @@ -13,8 +13,12 @@ PKG_DIR="${3}"
MAC_SIGNING_KEYCHAIN="${4}"
MAC_SIGNING_IDENTIFIER="${5}"
MAC_PROVISIONING_PROFILE="${6}"

app_name="$(basename $SOURCE)"
MAC_ENTITLEMENTS_PLIST="${7}"
if [[ ${#} -eq 8 ]]; then
EXTRA_FLAGS="${8}"
else
EXTRA_FLAGS=""
fi

function check_exit() {
return=$?;
Expand All @@ -38,6 +42,6 @@ mkdir -p "$tmpdir"

cp -a "$SOURCE" "$DEST"

"${PKG_DIR}/sign_versioned_dir.sh" "$DEST" "$MAC_SIGNING_KEYCHAIN" "$MAC_SIGNING_IDENTIFIER"
"${PKG_DIR}/sign_versioned_dir.sh" "$DEST" "$MAC_SIGNING_KEYCHAIN" "$MAC_SIGNING_IDENTIFIER" "$EXTRA_FLAGS"

"${PKG_DIR}/sign_app.sh" "$DEST" "$MAC_SIGNING_KEYCHAIN" "$MAC_SIGNING_IDENTIFIER" "$MAC_PROVISIONING_PROFILE" "${PKG_DIR}/entitlements.plist"
"${PKG_DIR}/sign_app.sh" "$DEST" "$MAC_SIGNING_KEYCHAIN" "$MAC_SIGNING_IDENTIFIER" "$MAC_PROVISIONING_PROFILE" "${MAC_ENTITLEMENTS_PLIST}" "$EXTRA_FLAGS"
2 changes: 1 addition & 1 deletion chromium_src/base/mac/foundation_util.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
}

#if !defined(OFFICIAL_BUILD)
return "com.brave.Browser.development";
return "com.brave.Browser";
Copy link
Member

@bsclifton bsclifton Jan 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this need to change? Was it defaulting non-official builds to the dev ID?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually this shouldn't be needed. I think it's just leftover from when I couldn't get it to work when everything was set to com.brave.Browser.development. I'll check now

#else
switch (chrome::GetChannel()) {
case version_info::Channel::CANARY:
Expand Down
18 changes: 3 additions & 15 deletions patches/chrome-installer-mac-sign_app.sh.in.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,8 @@
diff --git a/chrome/installer/mac/sign_app.sh.in b/chrome/installer/mac/sign_app.sh.in
index 89b0028a1fb782c72021c4ce7f5db54d6dcc9b22..8f7a7b36cd66f827cd48180f96422de4001a1c71 100644
index 89b0028a1fb782c72021c4ce7f5db54d6dcc9b22..934363520f8212cddaab525e31c7bbf4a023dd73 100644
--- a/chrome/installer/mac/sign_app.sh.in
+++ b/chrome/installer/mac/sign_app.sh.in
@@ -47,10 +47,7 @@ else
fi

script_dir="$(dirname "${0}")"
-source "${script_dir}/variables.sh"
-
-# Use custom resource rules for the browser application.
-browser_app_rules="${script_dir}/app_resource_rules.plist"
+codesign_id=$codesign_id source "${script_dir}/variables.sh"

contents_dir="${app_path}/Contents"
versioned_dir="${contents_dir}/Versions/@VERSION@"
@@ -70,10 +67,10 @@ fi
@@ -70,10 +70,10 @@ fi

requirement="\
designated => \
Expand All @@ -29,7 +17,7 @@ index 89b0028a1fb782c72021c4ce7f5db54d6dcc9b22..8f7a7b36cd66f827cd48180f96422de4
${requirement_suffix} \
"

@@ -81,7 +78,6 @@ codesign_cmd=(
@@ -81,7 +81,6 @@ codesign_cmd=(
codesign --sign "${codesign_id}" --keychain "${codesign_keychain}"
"${browser_app}"
--options "${enforcement_flags_app}"
Expand Down
7 changes: 3 additions & 4 deletions patches/chrome-installer-mac-variables.sh.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
diff --git a/chrome/installer/mac/variables.sh b/chrome/installer/mac/variables.sh
index cb07976f21bacda46b4a2b7db229d869dca80f1a..00c389cea0cad4766559b1e99bb8c98584736f01 100644
index cb07976f21bacda46b4a2b7db229d869dca80f1a..f75c2b4ccae6418fdf2b2f853107778deff39f2d 100644
--- a/chrome/installer/mac/variables.sh
+++ b/chrome/installer/mac/variables.sh
@@ -19,5 +19,6 @@ enforcement_flags_installer_tools="${enforcement_flags_helpers},kill"
@@ -19,5 +19,5 @@ enforcement_flags_installer_tools="${enforcement_flags_helpers},kill"
# contains the hash of the certificate used to sign Chrome. When transitioning
# signing certs, this may include the hash of both the old and new certificate.
requirement_suffix="\
-and certificate leaf = H\"c9a99324ca3fcb23dbcc36bd5fd4f9753305130a\" \
+and (certificate leaf = H\"${codesign_id}\" or \
+certificate leaf = H\"${codesign_id}\") \
+and certificate leaf = H\"${codesign_id}\" \
"