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

feat(backup): Support import decryption #58128

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

azaslavsky
Copy link
Contributor

This is the follow up to #58015, adding the corresponding --decrypt_with flag to decrypt tarballs at import time.

Closes getsentry/team-ospo#207

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 13, 2023
@codecov
Copy link

codecov bot commented Oct 14, 2023

Codecov Report

Merging #58128 (73aa12b) into master (52947aa) will increase coverage by 0.00%.
Report is 44 commits behind head on master.
The diff coverage is 86.66%.

@@           Coverage Diff           @@
##           master   #58128   +/-   ##
=======================================
  Coverage   79.04%   79.05%           
=======================================
  Files        5135     5135           
  Lines      223466   223480   +14     
  Branches    37622    37626    +4     
=======================================
+ Hits       176643   176673   +30     
+ Misses      41173    41152   -21     
- Partials     5650     5655    +5     
Files Coverage Δ
src/sentry/backup/exports.py 82.92% <100.00%> (-3.99%) ⬇️
src/sentry/testutils/helpers/backups.py 90.29% <100.00%> (-0.26%) ⬇️
src/sentry/backup/imports.py 88.96% <87.50%> (-0.08%) ⬇️
src/sentry/backup/helpers.py 89.00% <84.48%> (-6.35%) ⬇️

... and 42 files with indirect coverage changes

Base automatically changed from azaslavsky/backup/export_encryption to master October 16, 2023 04:48
@azaslavsky azaslavsky force-pushed the azaslavsky/backup/import_decryption branch from e80a8dd to 016d82c Compare October 16, 2023 05:02
@azaslavsky azaslavsky marked this pull request as ready for review October 16, 2023 05:02
@azaslavsky azaslavsky requested a review from a team as a code owner October 16, 2023 05:02
@azaslavsky azaslavsky requested a review from mdtro October 16, 2023 16:19

tmp_tarball_path = Path(tmp_dir).joinpath("input.tar")
with open(tmp_tarball_path, "wb") as i, open(tmp_pub_key_path, "rb") as p:
i.write(create_encrypted_export_tarball(data, p).getvalue())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth it to have a testcase encrypting an export tarbell without this function create_encrypted_export_tarbell? Since technically changing the behavior in this function could lead to all these test cases failing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I copied out the logic into this testing method.

backup_encryptor = Fernet(data_encryption_key)
encrypted_json_export = backup_encryptor.encrypt(json.dumps(json_export).encode("utf-8"))

# Encrypt the newly minted DEK using symmetric public key encryption.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Encrypt the newly minted DEK using symmetric public key encryption.
# Encrypt the newly minted DEK using asymmetric public key encryption.

The DEK is encrypted with asymmetric encryption. Once encrypted with the public key, it can only be decrypted with the private key.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, done!

This is the follow up to #58015, adding the corresponding
`--decrypt_with` flag to decrypt tarballs at import time.

Closes getsentry/team-ospo#207
@azaslavsky azaslavsky force-pushed the azaslavsky/backup/import_decryption branch from 016d82c to 73aa12b Compare October 16, 2023 19:04
@azaslavsky azaslavsky merged commit 6801cb7 into master Oct 16, 2023
50 of 51 checks passed
@azaslavsky azaslavsky deleted the azaslavsky/backup/import_decryption branch October 16, 2023 20:01
@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support export encryption and corresponding import decryption
3 participants