`GpgKeyNotFoundError` can block deletion of a `Source`

[cfm]

Description

One of #6667's Sources with last_updated = None cannot be deleted, because DELETE /sources/<source_uuid> returns HTTP 500 on a GpgKeyNotFoundError.

Steps to Reproduce

See #6667 for context. In the SecureDrop Client, select source politic paperweight and select Delete Source Account.

Expected Behavior

The source is deleted.

Actual Behavior

The source is not deleted, and the SecureDrop Client returns error:

Failed to delete source at server

sd-log

==> QubesIncomingLogs/sd-proxy/syslog.log <==
Nov  2 11:12:14 localhost 2022-11-02 11:12:14,503 - securedrop_proxy.proxy:260(proxy) ERROR: 500 Server Error: INTERNAL SERVER ERROR for url: http://77aml4ggknlndjnkshytyfstwuywlpuncmvr6qnxyuszmzkocobsi4ad.onion/api/v1/sources/eb2df246-c2d6-46dd-af81-83608411870f

==> QubesIncomingLogs/sd-app/syslog.log <==
Nov  2 11:12:15 localhost 2022-11-02 11:12:15,037 - securedrop_client.queue:183(process) ERROR: DeleteSourceJobException: Failed to delete source eb2df246-c2d6-46dd-af81-83608411870f due to BaseError('internal server error')

Server

[Wed Nov 02 18:05:04.394561 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784] ERROR:journalist_app:Exception on /api/v1/source
s/eb2df246-c2d6-46dd-af81-83608411870f [DELETE]
[Wed Nov 02 18:05:04.394587 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784] Traceback (most recent call last):
[Wed Nov 02 18:05:04.394591 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]   File "/opt/venvs/securedrop-app-code/lib/python3.8/site-packages/flask/app.py", line 2073, in wsgi_app
[Wed Nov 02 18:05:04.394594 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]     response = self.full_dispatch_request()
[Wed Nov 02 18:05:04.394596 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]   File "/opt/venvs/securedrop-app-code/lib/python3.8/site-packages/flask/app.py", line 1518, in full_dispatch_request
[Wed Nov 02 18:05:04.394599 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]     rv = self.handle_user_exception(e)
[Wed Nov 02 18:05:04.394601 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]   File "/opt/venvs/securedrop-app-code/lib/python3.8/site-packages/flask/app.py", line 1516, in full_dispatch_request
[Wed Nov 02 18:05:04.394603 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]     rv = self.dispatch_request()
[Wed Nov 02 18:05:04.394605 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]   File "/opt/venvs/securedrop-app-code/lib/python3.8/site-packages/flask/app.py", line 1502, in dispatch_request
[Wed Nov 02 18:05:04.394608 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]     return self.ensure_sync(self.view_functions[rule.endpoint])(**req.view_args)
[Wed Nov 02 18:05:04.394610 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]   File "/var/www/securedrop/journalist_app/api.py", line 136, in single_source
[Wed Nov 02 18:05:04.394612 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]     utils.delete_collection(source.filesystem_id)
[Wed Nov 02 18:05:04.394615 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]   File "/var/www/securedrop/journalist_app/utils.py", line 406, in delete_collection
[Wed Nov 02 18:05:04.394617 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]     EncryptionManager.get_default().delete_source_key_pair(filesystem_id)
[Wed Nov 02 18:05:04.394619 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]   File "/var/www/securedrop/encryption.py", line 152, in delete_source_key_pair
[Wed Nov 02 18:05:04.394621 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]     source_key_fingerprint = self.get_source_key_fingerprint(source_filesystem_id)
[Wed Nov 02 18:05:04.394623 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]   File "/var/www/securedrop/encryption.py", line 173, in get_source_key_fingerprint
[Wed Nov 02 18:05:04.394625 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]     source_key_details = self._get_source_key_details(source_filesystem_id)
[Wed Nov 02 18:05:04.394628 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]   File "/var/www/securedrop/encryption.py", line 241, in _get_source_key_details
[Wed Nov 02 18:05:04.394630 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784]     raise GpgKeyNotFoundError()
[Wed Nov 02 18:05:04.394638 2022] [wsgi:error] [pid 691:tid 126340659672832] [remote 127.0.0.1:56784] encryption.GpgKeyNotFoundError

Comments

Per #6667, because this source is not listed in the Journalist Interface, it cannot be deleted except directly at the SQLite shell.

[legoktm]

It seems like we should just catch and suppress the GpgKeyNotFoundError error since during deletion, that's what we want...?

[anorthall]

I will do this.

[anorthall]

@cfm I hope you don't mind a question - I'm having some difficulty making the Docker test environment reload my code when making changes. I've tried on both my own PC, and in a GitHub Codespace, and rebuilt it from scratch several times, and at no point can I get Docker to detect or reload my changes when making changes in [GIT_DIR]/securedrop.

I've tried looking in some likely places where this may be configured or enabled but I've found nothing. Any ideas? It's such a complex development environment that I'm not sure where to look. The last time I contributed, I recall it worked fine out of the box.

[cfm]

Good catch, @anorthall! I can reproduce this too. It looks like #6563 reintroduced #5594.

I'll file a pull request to fix this on Monday. In the meantime, try the following patch:

--- a/securedrop/sdconfig.py
+++ b/securedrop/sdconfig.py
@@ -43,6 +43,8 @@ class SourceInterfaceConfig(_FlaskAppConfig):
 
 @dataclass(frozen=True)
 class SecureDropConfig:
+    env: str
+
     JOURNALIST_APP_FLASK_CONFIG_CLS: JournalistInterfaceConfig
     SOURCE_APP_FLASK_CONFIG_CLS: SourceInterfaceConfig
 
@@ -115,6 +117,8 @@ def _parse_config_from_file(config_module_name: str) -> SecureDropConfig:
         config_from_local_file, "SCRYPT_PARAMS", dict(N=2**14, r=8, p=1)
     )
 
+    env = getattr(config_from_local_file, "env", "prod")
+
     try:
         final_securedrop_root = Path(config_from_local_file.SECUREDROP_ROOT)
     except AttributeError:
@@ -188,6 +192,7 @@ def _parse_config_from_file(config_module_name: str) -> SecureDropConfig:
     )
 
     return SecureDropConfig(
+        env=env,
         JOURNALIST_APP_FLASK_CONFIG_CLS=parsed_journ_flask_config,
         SOURCE_APP_FLASK_CONFIG_CLS=parsed_source_flask_config,
         GPG_KEY_DIR=final_gpg_key_dir,

[anorthall]

Thanks for the quick response and patch @cfm - I'll give it a go. Glad it wasn't just me.

[anorthall]

Sorry, life has taken over a bit and I haven't had a chance to work on this. Just posting here to let any other contributors know that they are free to have a go - although it's still on my to-do list if nobody else does in a few months.

[bnewc]

Unless this issue has been resolved elsewhere, I'd like to tackle it. As I understand from this thread, I may be able to fix the bug simply by properly handling the GpgKeyNotFoundError?

[bnewc]

It looks like this bug was fixed in commit https://github.com/freedomofpress/securedrop/commit/8ff264dbfc23bc34173f4d43527641afac6fc5d8 after this thread went silent. Specifically, these lines catch any GpgKeyNotFoundError when a deletion attempt is made. I have not replicated the bug context in my development environment, but judging from the traceback provided by @cfm , DELETE requests should no longer throw an HTTP 500 under these circumstances.