This is an automated email from the ASF dual-hosted git repository. jimjag pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/openoffice-devtools.git
commit bd2f1776f58ee4ec16317effe9973f8df9705cb1 Author: Jim Jagielski <[email protected]> AuthorDate: Wed Sep 16 10:09:37 2026 -0400 Harden remote macOS release signing --- release-scripts/macosx-remote-sign.sh | 93 +++++++++---- release-scripts/tests/conftest.py | 43 +++++- release-scripts/tests/test_macosx_remote_sign.py | 169 ++++++++++++++++++++--- 3 files changed, 262 insertions(+), 43 deletions(-) diff --git a/release-scripts/macosx-remote-sign.sh b/release-scripts/macosx-remote-sign.sh index 0af7dc4..6d9ac20 100755 --- a/release-scripts/macosx-remote-sign.sh +++ b/release-scripts/macosx-remote-sign.sh @@ -1,11 +1,11 @@ #!/bin/bash # -# macosx-remote-sign.sh : sign an unsigned Apache OpenOffice macOS .dmg on a +# macosx-remote-sign.sh : sign an Apache OpenOffice macOS .dmg on a # machine separate from the one that built it, so only this one machine ever # needs the Developer ID Application private key (and, if used, the notary # credentials) - build servers never touch that key material. # -# ./macosx-remote-sign.sh [options] <unsigned.dmg> <signed-output.dmg> +# ./macosx-remote-sign.sh [options] <input.dmg> <signed-output.dmg> # # This is release engineering tooling, not product source, so it lives here # in openoffice-devtools rather than in the openoffice/main tree. See @@ -35,8 +35,10 @@ # --notarize PROFILE # passed through to macosx-codesign.sh for both the # extracted app and the rebuilt dmg (see its --help) -# --release passed through: fail if spctl rejects the result -# --sha256 HASH verify <unsigned.dmg> against this checksum before +# --release fail if spctl rejects the result (required for release) +# --non-release permit signing without notarization or Gatekeeper +# acceptance; intended only for diagnostics +# --sha256 HASH verify <input.dmg> against this checksum before # doing anything else (a bare digest or a full # "shasum -a 256" checksum line both work) # -h, --help @@ -59,6 +61,7 @@ KEYCHAIN="" ENTITLEMENTS="" NOTARY_PROFILE="" RELEASE=no +NON_RELEASE=no EXPECT_SHA256="" SRC_DMG="" OUT_DMG="" @@ -78,10 +81,11 @@ while [ $# -gt 0 ]; do [ $# -ge 2 ] || { echo "$1 requires an argument" >&2; exit 2; } NOTARY_PROFILE="$2"; shift 2 ;; --release) RELEASE=yes; shift ;; + --non-release) NON_RELEASE=yes; shift ;; --sha256) [ $# -ge 2 ] || { echo "$1 requires an argument" >&2; exit 2; } EXPECT_SHA256="$2"; shift 2 ;; - -h|--help) sed -n '2,50p' "$0"; exit 0 ;; + -h|--help) sed -n '2,52p' "$0"; exit 0 ;; -*) echo "unknown option: $1" >&2; exit 2 ;; *) if [ -z "$SRC_DMG" ]; then SRC_DMG="$1" @@ -93,13 +97,22 @@ while [ $# -gt 0 ]; do done [ -n "$SRC_DMG" ] && [ -n "$OUT_DMG" ] || { - echo "usage: $(basename "$0") [options] <unsigned.dmg> <signed-output.dmg>" >&2 + echo "usage: $(basename "$0") [options] <input.dmg> <signed-output.dmg>" >&2 exit 2 } [ -n "$IDENTITY" ] && [ "$IDENTITY" != "-" ] || { echo "-i/--identity is required and must be a real Developer ID, not \"-\"" >&2 exit 2 } +if [ "$NON_RELEASE" = yes ]; then + [ "$RELEASE" = no ] || { + echo "--release and --non-release cannot be used together" >&2 + exit 2 + } +elif [ -z "$NOTARY_PROFILE" ] || [ "$RELEASE" = no ]; then + echo "release signing requires --notarize PROFILE and --release; use --non-release only for diagnostics" >&2 + exit 2 +fi [ -x "$CODESIGN" ] || { echo "macosx-codesign.sh not found beside this script ($CODESIGN) - see this script's header for the required deployment file list" >&2 exit 2 @@ -110,6 +123,7 @@ case "$OUT_DMG" in *.dmg) ;; *) OUT_DMG="$OUT_DMG.dmg" ;; esac +[ -d "$OUT_DMG" ] && { echo "output path is a directory: $OUT_DMG" >&2; exit 2; } if [ "$SRC_DMG" -ef "$OUT_DMG" ]; then echo "output path is the input dmg: $OUT_DMG (write to a different file)" >&2 exit 2 @@ -119,12 +133,43 @@ MOUNT_POINT="" STAGING_DIR="" TEMP_DMG="" cleanup() { - [ -z "$MOUNT_POINT" ] || hdiutil detach "$MOUNT_POINT" -quiet 2>/dev/null || true + if [ -n "$MOUNT_POINT" ]; then + hdiutil detach "$MOUNT_POINT" -quiet 2>/dev/null || true + rmdir "$MOUNT_POINT" 2>/dev/null || true + fi [ -z "$STAGING_DIR" ] || rm -rf "$STAGING_DIR" [ -z "$TEMP_DMG" ] || rm -f "$TEMP_DMG" } trap cleanup EXIT +mount_readonly() { + local dmg="$1" + MOUNT_POINT=$(mktemp -d "${TMPDIR:-/tmp}/macosx-remote-sign.mount.XXXXXX") + if ! hdiutil attach -readonly -nobrowse -mountpoint "$MOUNT_POINT" "$dmg" >/dev/null; then + echo "could not mount $dmg" >&2 + return 1 + fi +} + +detach_mounted() { + local mount_point="$MOUNT_POINT" + hdiutil detach "$mount_point" -quiet + rmdir "$mount_point" 2>/dev/null || true + MOUNT_POINT="" +} + +require_developer_id_signature() { + local target="$1" details + details=$(codesign -dv --verbose=4 "$target" 2>&1) || { + echo "could not inspect signature: $target" >&2 + return 1 + } + printf '%s\n' "$details" | grep -q '^[[:space:]]*Authority=Developer ID Application:' || { + echo "not signed with a Developer ID Application certificate: $target" >&2 + return 1 + } +} + if [ -n "$EXPECT_SHA256" ]; then echo "==> verifying checksum of $SRC_DMG" # Accept either a bare digest or a full checksum line as written by @@ -140,22 +185,7 @@ if [ -n "$EXPECT_SHA256" ]; then fi echo "==> mounting $SRC_DMG" -# Parse the -plist form rather than the human-readable table: the mount point -# is whichever system-entity has one (the first often does not), and its -# tab-column position is not a documented guarantee. -ATTACH_PLIST=$(hdiutil attach -readonly -nobrowse -plist "$SRC_DMG") -MOUNT_POINT="" -ATTACH_COUNT=$(printf '%s' "$ATTACH_PLIST" | plutil -extract system-entities raw -o - -) -i=0 -while [ "$i" -lt "$ATTACH_COUNT" ]; do - mp=$(printf '%s' "$ATTACH_PLIST" | plutil -extract "system-entities.$i.mount-point" raw -o - - 2>/dev/null || true) - [ -z "$mp" ] || MOUNT_POINT="$mp" - i=$((i + 1)) -done -[ -n "$MOUNT_POINT" ] && [ -d "$MOUNT_POINT" ] || { - echo "could not mount $SRC_DMG" >&2 - exit 1 -} +mount_readonly "$SRC_DMG" VOLUME_NAME=$(diskutil info "$MOUNT_POINT" | awk '/Volume Name/{sub(/^[^:]*: +/, ""); print; exit}') [ -n "$VOLUME_NAME" ] || VOLUME_NAME=$(basename "$MOUNT_POINT") @@ -166,8 +196,7 @@ echo "==> copying volume contents to $STAGING_DIR" # background image, all of which the rebuilt dmg below should keep too. ditto "$MOUNT_POINT" "$STAGING_DIR" -hdiutil detach "$MOUNT_POINT" -quiet -MOUNT_POINT="" +detach_mounted apps=("$STAGING_DIR"/*.app) [ -d "${apps[0]}" ] || { echo "no .app bundle found in $SRC_DMG" >&2; exit 1; } @@ -186,6 +215,7 @@ sign_args=(-i "$IDENTITY") echo "==> signing $APP" "$CODESIGN" "${sign_args[@]}" "$APP" +require_developer_id_signature "$APP" echo "==> building $OUT_DMG (volume: $VOLUME_NAME)" mkdir -p "$(dirname "$OUT_DMG")" @@ -197,6 +227,19 @@ hdiutil create -srcfolder "$STAGING_DIR" -volname "$VOLUME_NAME" -fs HFS+ -forma echo "==> signing $TEMP_DMG" "$CODESIGN" "${sign_args[@]}" "$TEMP_DMG" +require_developer_id_signature "$TEMP_DMG" + +if [ -n "$NOTARY_PROFILE" ]; then + echo "==> validating enclosed app staple" + mount_readonly "$TEMP_DMG" + final_apps=("$MOUNT_POINT"/*.app) + [ -d "${final_apps[0]}" ] && [ ${#final_apps[@]} -eq 1 ] || { + echo "rebuilt dmg does not contain exactly one .app" >&2 + exit 1 + } + xcrun stapler validate "${final_apps[0]}" + detach_mounted +fi mv -f "$TEMP_DMG" "$OUT_DMG" TEMP_DMG="" diff --git a/release-scripts/tests/conftest.py b/release-scripts/tests/conftest.py index 3c0f37c..fd71d16 100644 --- a/release-scripts/tests/conftest.py +++ b/release-scripts/tests/conftest.py @@ -40,7 +40,7 @@ WRAPPER = SCRIPT_DIR / "macosx-remote-sign.sh" IDENTITY = "Developer ID Application: Test" requires_macos = pytest.mark.skipif( - os.uname().sysname != "Darwin", reason="requires macOS (hdiutil, diskutil, plutil)" + os.uname().sysname != "Darwin", reason="requires macOS (hdiutil and diskutil)" ) @@ -96,15 +96,26 @@ class Signer: self.path = root / name self.path.mkdir(parents=True) self.log = _stub_log(self.path) + self.xcrun_log = self.path / "xcrun.log" + self.xcrun_log.write_text("") + self.events = self.path / "events.log" + self.events.write_text("") + self.bin = self.path / ".test-bin" + self.bin.mkdir() self.fail = fail shutil.copy(WRAPPER, self.path / "macosx-remote-sign.sh") self._write_stub() + self._write_tool_stubs() def _write_stub(self): stub = self.path / "macosx-codesign.sh" body = [ "#!/bin/bash", 'printf "STUB: %s\\n" "$*" >> "$STUB_LOG"', + 'case "${!#}" in', + ' *.dmg) printf "sign-dmg\\n" >> "$STUB_EVENTS" ;;', + ' *) printf "sign-app\\n" >> "$STUB_EVENTS"; touch "${!#}/Contents/.signed-by-stub" ;;', + "esac", ] if self.fail == "dmg": body += [ @@ -116,11 +127,38 @@ class Signer: stub.write_text("\n".join(body) + "\n") stub.chmod(stub.stat().st_mode | stat.S_IXUSR) + def _write_tool_stubs(self): + codesign = self.bin / "codesign" + codesign.write_text( + "#!/bin/bash\n" + 'case "${!#}" in\n' + ' *.dmg) authority="${STUB_DMG_AUTHORITY:-Developer ID Application: Test}" ;;\n' + ' *) authority="${STUB_APP_AUTHORITY:-Developer ID Application: Test}" ;;\n' + "esac\n" + 'printf "Authority=%s\\n" "$authority" >&2\n' + "exit 0\n" + ) + codesign.chmod(0o755) + + xcrun = self.bin / "xcrun" + xcrun.write_text( + "#!/bin/bash\n" + 'printf "XCRUN: %s\\n" "$*" >> "$STUB_XCRUN_LOG"\n' + 'printf "validate:%s\\n" "${!#}" >> "$STUB_EVENTS"\n' + '[ -f "${!#}/Contents/.signed-by-stub" ] || exit 2\n' + '[ "${STUB_STAPLER_FAIL:-no}" = no ] || exit 1\n' + "exit 0\n" + ) + xcrun.chmod(0o755) + def run(self, *args, env=None): run_env = os.environ.copy() run_env["STUB_LOG"] = str(self.log) + run_env["STUB_XCRUN_LOG"] = str(self.xcrun_log) + run_env["STUB_EVENTS"] = str(self.events) if env: run_env.update(env) + run_env["PATH"] = f"{self.bin}:{run_env['PATH']}" return subprocess.run( [str(self.path / "macosx-remote-sign.sh"), *args], capture_output=True, @@ -128,6 +166,9 @@ class Signer: env=run_env, ) + def run_non_release(self, *args, env=None): + return self.run("--non-release", *args, env=env) + @pytest.fixture def workdir(tmp_path): diff --git a/release-scripts/tests/test_macosx_remote_sign.py b/release-scripts/tests/test_macosx_remote_sign.py index 6534765..d8ad1f3 100644 --- a/release-scripts/tests/test_macosx_remote_sign.py +++ b/release-scripts/tests/test_macosx_remote_sign.py @@ -35,7 +35,8 @@ pytestmark = requires_macos def test_help_exits_zero(signer): r = signer.run("--help") assert r.returncode == 0 - assert "unsigned.dmg" in r.stdout + assert "input.dmg" in r.stdout + assert "--non-release" in r.stdout def test_no_arguments_exits_two(signer): @@ -67,11 +68,51 @@ def test_adhoc_identity_exits_two(signer, workdir): assert signer.run("-i", "-", src, signer.path / "out.dmg").returncode == 2 [email protected]( + "mode_args", + [ + pytest.param((), id="neither"), + pytest.param(("--release",), id="release-only"), + pytest.param(("--notarize", "test-profile"), id="notarize-only"), + ], +) +def test_release_requires_notarize_and_release(signer, workdir, mode_args): + src = fixture_dmg(workdir, "args") + out = signer.path / "out.dmg" + r = signer.run("-i", IDENTITY, *mode_args, src, out) + assert r.returncode == 2 + assert "requires --notarize PROFILE and --release" in r.stderr + assert not out.exists() + + +def test_release_and_non_release_conflict(signer, workdir): + src = fixture_dmg(workdir, "args") + r = signer.run( + "-i", IDENTITY, "--release", "--non-release", src, signer.path / "out.dmg" + ) + assert r.returncode == 2 + assert "cannot be used together" in r.stderr + + def test_missing_source_exits_one(signer, workdir): - r = signer.run("-i", IDENTITY, workdir / "nope.dmg", signer.path / "out.dmg") + r = signer.run_non_release( + "-i", IDENTITY, workdir / "nope.dmg", signer.path / "out.dmg" + ) assert r.returncode == 1 +def test_failed_attach_removes_mountpoint(signer): + src = signer.path / "invalid.dmg" + src.write_text("not a disk image\n") + mount_root = signer.path / "mount-tmp" + mount_root.mkdir() + r = signer.run_non_release( + "-i", IDENTITY, src, signer.path / "out.dmg", env={"TMPDIR": str(mount_root)} + ) + assert r.returncode != 0 + assert list(mount_root.iterdir()) == [] + + def test_missing_sibling_signer_exits_two(workdir, tmp_path): import shutil @@ -82,7 +123,14 @@ def test_missing_sibling_signer_exits_two(workdir, tmp_path): shutil.copy(WRAPPER, d / "macosx-remote-sign.sh") src = fixture_dmg(workdir, "args") r = subprocess.run( - [str(d / "macosx-remote-sign.sh"), "-i", IDENTITY, str(src), str(d / "out.dmg")], + [ + str(d / "macosx-remote-sign.sh"), + "--non-release", + "-i", + IDENTITY, + str(src), + str(d / "out.dmg"), + ], capture_output=True, text=True, ) @@ -96,14 +144,25 @@ def test_directory_output_exits_two(signer, workdir): src = fixture_dmg(workdir, "args") out = signer.path / "isadir" out.mkdir() - r = signer.run("-i", IDENTITY, src, out) + r = signer.run_non_release("-i", IDENTITY, src, out) assert r.returncode == 2 assert "directory" in r.stderr +def test_normalized_directory_output_exits_two(signer, workdir): + src = fixture_dmg(workdir, "args") + requested = signer.path / "isadir" + normalized = signer.path / "isadir.dmg" + normalized.mkdir() + r = signer.run_non_release("-i", IDENTITY, src, requested) + assert r.returncode == 2 + assert "directory" in r.stderr + assert list(normalized.iterdir()) == [] + + def test_missing_dmg_suffix_is_normalized(signer, workdir): src = fixture_dmg(workdir, "args") - r = signer.run("-i", IDENTITY, src, signer.path / "suffixed") + r = signer.run_non_release("-i", IDENTITY, src, signer.path / "suffixed") assert r.returncode == 0 assert (signer.path / "suffixed.dmg").exists() assert not (signer.path / "suffixed").exists() @@ -111,7 +170,7 @@ def test_missing_dmg_suffix_is_normalized(signer, workdir): def test_output_equal_to_input_exits_two(signer, workdir): src = fixture_dmg(workdir, "args") - r = signer.run("-i", IDENTITY, src, src) + r = signer.run_non_release("-i", IDENTITY, src, src) assert r.returncode == 2 assert "input" in r.stderr @@ -135,14 +194,14 @@ def test_sha256_accepts_digest_and_lines(signer, workdir, fmt): src = fixture_dmg(workdir, "args") value = fmt(src, _digest(src)) out = signer.path / f"sum-{abs(hash(value))}.dmg" - r = signer.run("-i", IDENTITY, "--sha256", value, src, out) + r = signer.run_non_release("-i", IDENTITY, "--sha256", value, src, out) assert r.returncode == 0, r.stderr def test_sha256_mismatch_exits_one_and_writes_nothing(signer, workdir): src = fixture_dmg(workdir, "args") out = signer.path / "sum-bad.dmg" - r = signer.run("-i", IDENTITY, "--sha256", "0" * 64, src, out) + r = signer.run_non_release("-i", IDENTITY, "--sha256", "0" * 64, src, out) assert r.returncode == 1 assert not out.exists() @@ -152,14 +211,14 @@ def test_sha256_mismatch_exits_one_and_writes_nothing(signer, workdir): def test_no_app_exits_one(signer, workdir): src = fixture_dmg(workdir, "zeroapp", app_count=0, extras=False) - r = signer.run("-i", IDENTITY, src, signer.path / "zero.dmg") + r = signer.run_non_release("-i", IDENTITY, src, signer.path / "zero.dmg") assert r.returncode == 1 assert "no .app" in r.stderr def test_multiple_app_exits_one(signer, workdir): src = fixture_dmg(workdir, "twoapp", app_count=2, extras=False) - r = signer.run("-i", IDENTITY, src, signer.path / "two.dmg") + r = signer.run_non_release("-i", IDENTITY, src, signer.path / "two.dmg") assert r.returncode == 1 assert "exactly one" in r.stderr @@ -170,16 +229,92 @@ def test_multiple_app_exits_one(signer, workdir): def test_normal_run_publishes_output_and_no_temp(signer, workdir): src = fixture_dmg(workdir, "args") out = signer.path / "out.dmg" - r = signer.run("-i", IDENTITY, src, out) + r = signer.run_non_release("-i", IDENTITY, src, out) assert r.returncode == 0, r.stderr assert out.exists() assert list(signer.path.glob("out.dmg.tmp.*")) == [] +def test_release_forwards_options_and_validates_enclosed_staple(signer, workdir): + src = fixture_dmg(workdir, "args") + out = signer.path / "release.dmg" + r = signer.run( + "-i", + IDENTITY, + "-k", + "/tmp/test.keychain-db", + "-e", + "/tmp/test-entitlements.plist", + "--notarize", + "test-profile", + "--release", + src, + out, + ) + assert r.returncode == 0, r.stderr + assert out.exists() + + calls = signer.log.read_text().splitlines() + assert len(calls) == 2 + expected = ( + f"-i {IDENTITY} -k /tmp/test.keychain-db -e /tmp/test-entitlements.plist " + "--notarize test-profile --release" + ) + assert expected in calls[0] + assert expected in calls[1] + + staple_calls = signer.xcrun_log.read_text().splitlines() + assert len(staple_calls) == 1 + assert staple_calls[0].startswith("XCRUN: stapler validate ") + assert staple_calls[0].endswith("/App1.app") + + events = signer.events.read_text().splitlines() + assert len(events) == 3 + assert events[:2] == ["sign-app", "sign-dmg"] + assert events[2].startswith("validate:") + assert "/macosx-remote-sign.mount." in events[2] + assert events[2].endswith("/App1.app") + + [email protected]( + "env", + [ + pytest.param({"STUB_APP_AUTHORITY": "Apple Development: Test"}, id="app"), + pytest.param({"STUB_DMG_AUTHORITY": "Apple Development: Test"}, id="dmg"), + ], +) +def test_non_developer_id_signature_is_not_published(signer, workdir, env): + src = fixture_dmg(workdir, "args") + out = signer.path / "wrong-identity.dmg" + r = signer.run_non_release("-i", IDENTITY, src, out, env=env) + assert r.returncode != 0 + assert "not signed with a Developer ID Application certificate" in r.stderr + assert not out.exists() + assert list(signer.path.glob("wrong-identity.dmg.tmp.*")) == [] + + +def test_enclosed_staple_failure_is_not_published(signer, workdir): + src = fixture_dmg(workdir, "args") + out = signer.path / "bad-staple.dmg" + r = signer.run( + "-i", + IDENTITY, + "--notarize", + "test-profile", + "--release", + src, + out, + env={"STUB_STAPLER_FAIL": "yes"}, + ) + assert r.returncode != 0 + assert not out.exists() + assert list(signer.path.glob("bad-staple.dmg.tmp.*")) == [] + + def test_whole_volume_survives(signer, workdir): src = fixture_dmg(workdir, "args") out = signer.path / "out.dmg" - assert signer.run("-i", IDENTITY, src, out).returncode == 0 + assert signer.run_non_release("-i", IDENTITY, src, out).returncode == 0 with mount_point(out) as mp: assert (mp / "READMEs").is_dir() assert (mp / "Applications").is_symlink() @@ -190,7 +325,7 @@ def test_whole_volume_survives(signer, workdir): def test_original_volume_name_reused(signer, workdir): src = fixture_dmg(workdir, "args") out = signer.path / "out.dmg" - assert signer.run("-i", IDENTITY, src, out).returncode == 0 + assert signer.run_non_release("-i", IDENTITY, src, out).returncode == 0 assert volume_name(out) == "remote-sign-args" @@ -209,7 +344,7 @@ def test_volume_name_parse_keeps_colons(signer, workdir): stub.chmod(0o755) src = fixture_dmg(workdir, "colon", extras=False) out = signer.path / "colon.dmg" - r = signer.run( + r = signer.run_non_release( "-i", IDENTITY, src, out, env={"PATH": f"{stub_bin}:{os.environ['PATH']}"} ) assert r.returncode == 0, r.stderr @@ -223,7 +358,7 @@ def test_dmg_sign_failure_writes_nothing(tmp_path, workdir): signer = Signer(tmp_path, "w-dmgfail", fail="dmg") src = fixture_dmg(workdir, "args") out = signer.path / "final.dmg" - r = signer.run("-i", IDENTITY, src, out) + r = signer.run_non_release("-i", IDENTITY, src, out) assert r.returncode != 0 assert not out.exists() assert list(signer.path.glob("final.dmg.tmp.*")) == [] @@ -236,7 +371,7 @@ def test_app_sign_failure_writes_nothing(tmp_path, workdir): stub.chmod(0o755) src = fixture_dmg(workdir, "args") out = signer.path / "final.dmg" - r = signer.run("-i", IDENTITY, src, out) + r = signer.run_non_release("-i", IDENTITY, src, out) assert r.returncode != 0 assert not out.exists() @@ -252,6 +387,6 @@ def test_spaces_in_paths(tmp_path, workdir): shutil.copy(src, spaced_in) out = signer.path / "out dir" / "signed out.dmg" - r = signer.run("-i", IDENTITY, spaced_in, out) + r = signer.run_non_release("-i", IDENTITY, spaced_in, out) assert r.returncode == 0, r.stderr assert out.exists()
