Skip to content

Commit

Permalink
signingscript: remove support for deprecated/unused gpg format (#998)
Browse files Browse the repository at this point in the history
This was replaced by `autograph_gpg` a long time ago (sending a hash for
autograph to sign, instead of the entire payload).
  • Loading branch information
jcristau committed May 17, 2024
1 parent 61125a1 commit 46015b0
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 42 deletions.
1 change: 0 additions & 1 deletion signingscript/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ This is a best effort list of supported signing formats and what they correspond
- `autograph_authenticode_ev`: sign windows binary using autograph, using the EV (extended validation) code signing certificate, necessary for windows kernel modules
- `autograph_debsign`: gpg-sign a debian changes file and associated dsc and/or buildinfo, using autograph
- `autograph_gpg`: get a detached PGP signature for a file, using autograph's data signing endpoint
- `gpg`: [DEPRECATED] [UNUSED] get a detached PGP signature for a file, using autograph's file signing endpoint
- `autograph_hash_only_mar384`: sign a mar file, using autograph's hash signing endpoint
- `autograph_mar384`: [DEPRECATED] sign a mar file using autograph's file signing endpoint
- `autograph_stage_mar384`: sign a mar file, using autograph's hash signing endpoint. This uses autograph stage, so is intended for testing only (no production certificates)
Expand Down
4 changes: 2 additions & 2 deletions signingscript/src/signingscript/script.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ async def async_main(context):
work_dir = context.config["work_dir"]
async with aiohttp.ClientSession() as session:
all_signing_formats = task_signing_formats(context)
if "gpg" in all_signing_formats or "autograph_gpg" in all_signing_formats:
if "autograph_gpg" in all_signing_formats:
if not context.config.get("gpg_pubkey"):
raise Exception("GPG format is enabled but gpg_pubkey is not defined")
if not os.path.exists(context.config["gpg_pubkey"]):
Expand Down Expand Up @@ -61,7 +61,7 @@ async def async_main(context):
for source in output_files:
source = os.path.relpath(source, work_dir)
copy_to_dir(os.path.join(work_dir, source), context.config["artifact_dir"], target=source)
if "gpg" in path_dict["formats"] or "autograph_gpg" in path_dict["formats"]:
if "autograph_gpg" in path_dict["formats"]:
copy_to_dir(context.config["gpg_pubkey"], context.config["artifact_dir"], target="public/build/KEY")

# notarization_stacked is a special format that takes in all files at once instead of sequentially like other formats
Expand Down
20 changes: 0 additions & 20 deletions signingscript/src/signingscript/sign.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,26 +177,6 @@ async def sign_file(context, from_, fmt, to=None, **kwargs):
return to or from_


# sign_gpg {{{1
async def sign_gpg(context, from_, fmt, **kwargs):
"""Create a detached armored signature with the gpg key.
Because this function returns a list, gpg must be the final signing format.
Args:
context (Context): the signing context
from_ (str): the source file to sign
fmt (str): the format to sign with
Returns:
list: the path to the signed file, and sig.
"""
to = f"{from_}.asc"
await sign_file(context, from_, fmt, to=to)
return [from_, to]


# sign_macapp {{{1
async def sign_macapp(context, from_, fmt, **kwargs):
"""Sign a macapp.
Expand Down
4 changes: 1 addition & 3 deletions signingscript/src/signingscript/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
sign_debian_pkg,
sign_file,
sign_file_detached,
sign_gpg,
sign_gpg_with_autograph,
sign_macapp,
sign_mar384_with_autograph_hash,
Expand All @@ -39,7 +38,6 @@
{
"autograph_hash_only_mar384": sign_mar384_with_autograph_hash,
"autograph_stage_mar384": sign_mar384_with_autograph_hash,
"gpg": sign_gpg,
"autograph_gpg": sign_gpg_with_autograph,
"macapp": sign_macapp,
"widevine": sign_widevine,
Expand Down Expand Up @@ -187,7 +185,7 @@ def _sort_formats(formats):
"""
# Widevine formats must be after other formats other than macapp; GPG must
# be last.
for fmt in ("widevine", "autograph_widevine", "autograph_omnija", "macapp", "autograph_rsa", "gpg", "autograph_gpg"):
for fmt in ("widevine", "autograph_widevine", "autograph_omnija", "macapp", "autograph_rsa", "autograph_gpg"):
if fmt in formats:
formats.remove(fmt)
formats.append(fmt)
Expand Down
6 changes: 3 additions & 3 deletions signingscript/tests/test_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ async def fake_notarize_stacked(_, filelist_dict, *args, **kwargs):

@pytest.mark.asyncio
async def test_async_main_gpg(tmpdir, tmpfile, mocker):
formats = ["gpg"]
formats = ["autograph_gpg"]
fake_gpg_pubkey = tmpfile
mocked_copy_to_dir = mocker.Mock()
mocker.patch.object(script, "copy_to_dir", new=mocked_copy_to_dir)
Expand All @@ -64,7 +64,7 @@ async def test_async_main_gpg(tmpdir, tmpfile, mocker):

@pytest.mark.asyncio
async def test_async_main_gpg_no_pubkey_defined(tmpdir, mocker):
formats = ["gpg"]
formats = ["autograph_gpg"]

try:
await async_main_helper(tmpdir, mocker, formats)
Expand All @@ -74,7 +74,7 @@ async def test_async_main_gpg_no_pubkey_defined(tmpdir, mocker):

@pytest.mark.asyncio
async def test_async_main_gpg_pubkey_doesnt_exist(tmpdir, mocker):
formats = ["gpg"]
formats = ["autograph_gpg"]

try:
await async_main_helper(tmpdir, mocker, formats, {"gpg_pubkey": "faaaaaaake"})
Expand Down
7 changes: 0 additions & 7 deletions signingscript/tests/test_sign.py
Original file line number Diff line number Diff line change
Expand Up @@ -402,13 +402,6 @@ async def test_sign_mar384_with_autograph_hash_returns_invalid_signature_length(
assert json.load(mocked_session.post.call_args[1]["data"]) == [{"input": "YjY0bWFyaGFzaA=="}]


# sign_gpg {{{1
@pytest.mark.asyncio
async def test_sign_gpg(context, mocker):
mocker.patch.object(sign, "sign_file", new=noop_async)
assert await sign.sign_gpg(context, "from", "blah") == ["from", "from.asc"]


# sign_macapp {{{1
@pytest.mark.asyncio
@pytest.mark.parametrize("filename,expected", (("foo.dmg", "foo.tar.gz"), ("foo.tar.bz2", "foo.tar.bz2")))
Expand Down
17 changes: 11 additions & 6 deletions signingscript/tests/test_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,12 @@ def task_defn():
"scopes": ["signing"],
"payload": {
"upstreamArtifacts": [
{"taskType": "build", "taskId": "VALID_TASK_ID", "formats": ["gpg"], "paths": ["public/build/firefox-52.0a1.en-US.win64.installer.exe"]}
{
"taskType": "build",
"taskId": "VALID_TASK_ID",
"formats": ["autograph_gpg"],
"paths": ["public/build/firefox-52.0a1.en-US.win64.installer.exe"],
}
]
},
}
Expand All @@ -53,7 +58,7 @@ def task_defn_authenticode_comment():
{
"taskType": "build",
"taskId": "VALID_TASK_ID",
"formats": ["gpg"],
"formats": ["autograph_gpg"],
"paths": ["public/build/firefox-52.0a1.en-US.win64.installer.exe"],
"authenticode_comment": "Foo Installer",
}
Expand All @@ -79,8 +84,8 @@ def test_task_cert_type_error(context):

# task_signing_formats {{{1
def test_task_signing_formats(context):
context.task = {"payload": {"upstreamArtifacts": [{"formats": ["mar", "gpg"]}]}, "scopes": [TEST_CERT_TYPE]}
assert {"mar", "gpg"} == stask.task_signing_formats(context)
context.task = {"payload": {"upstreamArtifacts": [{"formats": ["mar", "autograph_gpg"]}]}, "scopes": [TEST_CERT_TYPE]}
assert {"mar", "autograph_gpg"} == stask.task_signing_formats(context)


def test_task_signing_formats_support_several_projects(context):
Expand Down Expand Up @@ -146,7 +151,7 @@ def fake_log(context, new_files, *args):
(
# Hardcoded cases
("autograph_hash_only_mar384", stask.sign_mar384_with_autograph_hash),
("gpg", stask.sign_gpg),
("autograph_gpg", stask.sign_gpg_with_autograph),
("macapp", stask.sign_macapp),
("widevine", stask.sign_widevine),
("autograph_authenticode_sha2", stask.sign_authenticode),
Expand Down Expand Up @@ -174,7 +179,7 @@ def test_get_signing_function_from_format(format, expected):
# build_filelist_dict {{{1
def test_build_filelist_dict(context, task_defn):
full_path = os.path.join(context.config["work_dir"], "cot", "VALID_TASK_ID", "public/build/firefox-52.0a1.en-US.win64.installer.exe")
expected = {"public/build/firefox-52.0a1.en-US.win64.installer.exe": {"full_path": full_path, "formats": ["gpg"]}}
expected = {"public/build/firefox-52.0a1.en-US.win64.installer.exe": {"full_path": full_path, "formats": ["autograph_gpg"]}}
context.task = task_defn

# first, the file is missing...
Expand Down

0 comments on commit 46015b0

Please sign in to comment.