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

Have securedrop-admin emit distinct error messages #7272

Merged
merged 1 commit into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 7 additions & 14 deletions admin/securedrop_admin/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1066,31 +1066,24 @@ def update(args: argparse.Namespace) -> int:
and len(good_sig_matches) == 1
and bad_sig_text not in sig_result
):
# Finally, we check that there is no branch of the same name
# prior to reporting success.
# Check for duplicate branch name
cmd = ["git", "show-ref", "--heads", "--verify", f"refs/heads/{latest_tag}"]
try:
# We expect this to produce a non-zero exit code, which
# will produce a subprocess.CalledProcessError
subprocess.check_output(cmd, stderr=subprocess.STDOUT, cwd=args.root)
sdlog.info("Signature verification failed.")
sdlog.error("Update failed: Branch name collision detected")
return 1
except subprocess.CalledProcessError as e:
if "not a valid ref" in e.output.decode("utf-8"):
# Then there is no duplicate branch.
sdlog.info("Signature verification successful.")
else: # If any other exception occurs, we bail.
sdlog.info("Signature verification failed.")
else:
sdlog.error("Update failed: Git command error")
return 1
else: # If anything else happens, fail and exit 1
sdlog.info("Signature verification failed.")
else:
sdlog.error("Update failed: Invalid signature format")
return 1

except subprocess.CalledProcessError:
# If there is no signature, or if the signature does not verify,
# then git tag -v exits subprocess.check_output will exit 1
# and subprocess.check_output will throw a CalledProcessError
sdlog.info("Signature verification failed.")
sdlog.error("Update failed: Missing or invalid signature")
return 1

# Only if the proper signature verifies do we check out the latest
Expand Down
4 changes: 2 additions & 2 deletions admin/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ def test_update_fails_when_no_signature_present(securedrop_git_repo):
child = pexpect.spawn(f"coverage run {cmd} --root {ansible_base} update")
output = child.read()
assert b"Updated to SecureDrop" not in output
assert b"Signature verification failed" in output
assert b"Update failed: Missing or invalid signature" in output

child.expect(pexpect.EOF, timeout=10) # Wait for CLI to exit
child.close()
Expand Down Expand Up @@ -682,7 +682,7 @@ def test_update_with_duplicate_branch_and_tag(securedrop_git_repo):
# Verify that we do not falsely check out a branch instead of a tag.
assert b"Switched to branch" not in output
assert b"Updated to SecureDrop" not in output
assert b"Signature verification failed" in output
assert b"Update failed: Branch name collision detected" in output

child.expect(pexpect.EOF, timeout=10) # Wait for CLI to exit
child.close()
Expand Down
10 changes: 5 additions & 5 deletions admin/tests/test_securedrop-admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ def test_update_signature_does_not_verify(self, tmpdir, caplog):
with mock.patch("subprocess.check_output", return_value=git_output):
ret_code = securedrop_admin.update(args)
assert "Applying SecureDrop updates..." in caplog.text
assert "Signature verification failed." in caplog.text
assert "Update failed: Invalid signature format" in caplog.text
assert "Updated to SecureDrop" not in caplog.text
assert ret_code != 0

Expand All @@ -406,7 +406,7 @@ def test_update_malicious_key_named_fingerprint(self, tmpdir, caplog):
with mock.patch("subprocess.check_output", return_value=git_output):
ret_code = securedrop_admin.update(args)
assert "Applying SecureDrop updates..." in caplog.text
assert "Signature verification failed." in caplog.text
assert "Update failed: Invalid signature format" in caplog.text
assert "Updated to SecureDrop" not in caplog.text
assert ret_code != 0

Expand All @@ -428,7 +428,7 @@ def test_update_malicious_key_named_good_sig(self, tmpdir, caplog):
with mock.patch("subprocess.check_output", return_value=git_output):
ret_code = securedrop_admin.update(args)
assert "Applying SecureDrop updates..." in caplog.text
assert "Signature verification failed." in caplog.text
assert "Update failed: Invalid signature format" in caplog.text
assert "Updated to SecureDrop" not in caplog.text
assert ret_code != 0

Expand All @@ -451,7 +451,7 @@ def test_update_malicious_key_named_good_sig_fingerprint(self, tmpdir, caplog):
with mock.patch("subprocess.check_output", return_value=git_output):
ret_code = securedrop_admin.update(args)
assert "Applying SecureDrop updates..." in caplog.text
assert "Signature verification failed." in caplog.text
assert "Update failed: Invalid signature format" in caplog.text
assert "Updated to SecureDrop" not in caplog.text
assert ret_code != 0

Expand All @@ -469,7 +469,7 @@ def test_no_signature_on_update(self, tmpdir, caplog):
):
ret_code = securedrop_admin.update(args)
assert "Applying SecureDrop updates..." in caplog.text
assert "Signature verification failed." in caplog.text
assert "Update failed: Missing or invalid signature" in caplog.text
assert "Updated to SecureDrop" not in caplog.text
assert ret_code != 0

Expand Down
Loading