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.

Sign up for GitHub

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

Jump to bottom

If the user deletes or changes a key in the DB, all previous encrypted rows will error when accessing #2348

Closed
gak opened this issue Aug 14, 2024 · 1 comment · Fixed by #2444
Closed

If the user deletes or changes a key in the DB, all previous encrypted rows will error when accessing #2348

gak opened this issue Aug 14, 2024 · 1 comment · Fixed by #2444
Assignees
@gak
Labels
security relates to security (regardless of priority)

Comments

@gak
Copy link
Contributor

gak commented Aug 14, 2024

Deleting a row is easy for the user to do without realising the implications.

However updating the row required a fair amount of effort so maybe less important.

Solution A

FK from each encrypted row into the keys table, so that the rows must be deleted before deleting the key. This won't help with a user modifying the key.

Solution B

Have some data in the DB that is encrypted and checked on startup. If it can't decrypt we know the key was deleted or changed.
@gak gak added triage Issue needs triaging security relates to security (regardless of priority) labels Aug 14, 2024
@ftl-robot ftl-robot mentioned this issue Aug 13, 2024
Open
@gak gak mentioned this issue Aug 14, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Aug 14, 2024
@gak @github-actions @matt2e
feat(KMS): derive keys for logs and async (#2338)
2a3edbc 
Fixes #2290 
Follows #2312 
Needs work: #2346 #2348 

> [!CAUTION]
> Will nuke logs and async columns!

- Uses KMS via tink `FTL_KMS_URI`, so `fake-kms://` or `aws-kms://` will
work. Omitting will not encrypt.
- Remove old plaintext keys envs.

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Matt Toohey <[email protected]>
@gak gak added backlog Not pressing, but should be looked at someday and removed triage Issue needs triaging backlog Not pressing, but should be looked at someday labels Aug 14, 2024
@gak gak self-assigned this Aug 19, 2024
@gak
Copy link
Contributor Author

gak commented Aug 19, 2024

Goal to just create two fields in the keys table for each subkey and just encrypt a common test word.

@gak gak mentioned this issue Aug 21, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Aug 21, 2024
@gak
fix: bug with subkeys, verify test fields on startup (#2444)
3da112c 
Fixes #2348
Fixes #2466
@gak gak closed this as completed in #2444 Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gak gak

Labels
security relates to security (regardless of priority)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: bug with subkeys, verify test fields on startup TBD54566975/ftl
1 participant
@gak