Eliminate flag-for-reply process #5954
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The changes in sources of randomness in Linux, and libgcrypt's use of them, have eliminated any need there might have been to check the amount of entropy available. Keypair generation takes less than a second on current CPUs. There's no need to defer key generation or to perform it asynchronously.
3 tasks
eloquence
reviewed
May 20, 2021
| source.flagged = True | ||
| db.session.commit() | ||
| return jsonify({'message': 'Source flagged for reply'}), 200 | ||
| return jsonify({'message': 'Sources no longer need to be flagged for reply'}), 200 |
There was a problem hiding this comment.
It may be worth marking the endpoint explicitly as deprecated, to be removed in a future release.
There was a problem hiding this comment.
I guess we can mark it as deprecated in the documentation, then we don't have to update this PR.
kushaldas
approved these changes
Jun 4, 2021
| source.flagged = True | ||
| db.session.commit() | ||
| return jsonify({'message': 'Source flagged for reply'}), 200 | ||
| return jsonify({'message': 'Sources no longer need to be flagged for reply'}), 200 |
There was a problem hiding this comment.
I guess we can mark it as deprecated in the documentation, then we don't have to update this PR.
redshiftzero
approved these changes
Jun 4, 2021
39 tasks
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Status
Ready for review
Description of Changes
Fixes #1584.
This PR removes the
havegedpackage, the checks of the entropy pool before generating source keypairs, and asynchronous source key generation. Source keys are now generated if they don't exist whenever a source visits/lookupor submits something, and theSource.flaggedcolumn and all associated code have been removed. Theflagroute in the journalist API is now a no-op, always returning a 200 status with a message explaining that the action is no longer needed. Theis_flaggedattribute of the JSON representation of sources returned from the API is now always false.Testing
git checkout 1584-trust-in-disordermake devdocker exec -it securedrop-dev-0 bash) runsqlite3 /var/lib/securedrop/db.sqlite "select id, filesystem_id from sources order by id"andgpg --homedir /var/lib/securedrop/keys/ --list-keysto verify that the new source has a key.docker exec -it securedrop-dev-0 bash.sqlite3 /var/lib/securedrop/db.sqlite "select filesystem_id from sources where journalist_designation = 'its-journalist-designation'"gpg --homedir /var/lib/securedrop/keys --yes --pinentry-mode=loopback --delete-secret-keys FILESYSTEM_IDgpg --homedir /var/lib/securedrop/keys --yes --pinentry-mode=loopback --delete-keys FILESYSTEM_IDredis-cli del sd/crypto-util/fingerprintsDeployment
What could go wrong? 🙂
I believe the changed logic will cover existing cases of sources without keys. Any source that was flagged for reply should get a key the next time they log in, simply because they don't have a key. The changed messaging in the journalist interface could be confusing for journalists who've had to flag sources for reply, but the effective flow is the same: you just have to wait for the source to return.
Checklist
If you made changes to the server application code:
make lint) and tests (make test) pass in the development containerIf you made changes to the system configuration:
If you added or removed a file deployed with the application:
If you made non-trivial code changes:
Choose one of the following: