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

Add RFC 1005: Realm key rotation #5303

Merged
merged 3 commits into from
Nov 10, 2023
Merged

Add RFC 1005: Realm key rotation #5303

merged 3 commits into from
Nov 10, 2023

Conversation

touilleMan
Copy link
Member

No description provided.

FirelightFlagboy
FirelightFlagboy reviewed Sep 22, 2023
View reviewed changes
FirelightFlagboy
FirelightFlagboy reviewed Sep 22, 2023
View reviewed changes
@FirelightFlagboy FirelightFlagboy linked an issue Sep 25, 2023 that may be closed by this pull request
Design strategy for data migration (RFC) #5350
Closed
@touilleMan touilleMan force-pushed the rfc-realm-key-rotation branch 4 times, most recently from 5ac101a to ed6bf70 Compare October 4, 2023 12:09
@touilleMan touilleMan force-pushed the rfc-realm-key-rotation branch from ed6bf70 to 1a82f37 Compare October 5, 2023 07:54
FirelightFlagboy
FirelightFlagboy reviewed Oct 9, 2023
View reviewed changes
FirelightFlagboy
FirelightFlagboy reviewed Oct 9, 2023
View reviewed changes
@touilleMan touilleMan force-pushed the rfc-realm-key-rotation branch 3 times, most recently from b73999d to 1c22618 Compare October 9, 2023 10:42
@touilleMan touilleMan force-pushed the rfc-realm-key-rotation branch 3 times, most recently from 94e67a1 to c87d745 Compare October 9, 2023 21:36
@FirelightFlagboy FirelightFlagboy marked this pull request as ready for review October 10, 2023 07:06
FirelightFlagboy
FirelightFlagboy reviewed Oct 10, 2023
View reviewed changes
FirelightFlagboy
FirelightFlagboy reviewed Oct 10, 2023
View reviewed changes
FirelightFlagboy
FirelightFlagboy reviewed Oct 10, 2023
View reviewed changes
@FirelightFlagboy
Copy link
Contributor

General comment about the markdown format:

  • The headers that start with a number value e.g: ## 1 Foobar should have a separator (recommend using a carret - to be consistent).

  • I recommend that list item start with a capital and end with a point .:

    - Foo bar.
- Foo do bar.
- Bar is Foo.

FirelightFlagboy
FirelightFlagboy reviewed Oct 10, 2023
View reviewed changes
mmmarcos
mmmarcos reviewed Oct 11, 2023
View reviewed changes
Copy link
Contributor

@mmmarcos mmmarcos left a comment

Choose a reason for hiding this comment

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

Partial review (sections 1 and 2)

mmmarcos
mmmarcos reviewed Oct 12, 2023
View reviewed changes
Copy link
Contributor

@mmmarcos mmmarcos left a comment

Choose a reason for hiding this comment

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

More comments on section 2

touilleMan
touilleMan commented Oct 26, 2023
View reviewed changes
touilleMan
touilleMan commented Oct 26, 2023
View reviewed changes
docs/rfcs/1005-realm-key-rotation.md Outdated

> **TODO:**
>
> The fields `encryption_algorithm` & `hash_algorithm` allows for future evolution.
Copy link
Member Author

Choose a reason for hiding this comment

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

A new RFC will be opened to discuss this

The idea is basically to modify file manifest to references the file blocks as (block_id, key_index, block_hash), this way all data using symmetric encryption will use the same key

Copy link
Contributor

@FirelightFlagboy FirelightFlagboy Nov 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Did you create an issue to trace the requirement of the new RFC ? (If you give me the requirement I could create it for you)

@touilleMan touilleMan force-pushed the rfc-realm-key-rotation branch from c87d745 to df39f3e Compare October 31, 2023 17:49
@touilleMan touilleMan force-pushed the rfc-realm-key-rotation branch from df39f3e to 50b995b Compare October 31, 2023 20:43
FirelightFlagboy
FirelightFlagboy reviewed Nov 2, 2023
View reviewed changes
docs/rfcs/1005-realm-key-rotation.md
Comment on lines +677 to +680
To avoid being blocked by a workspace forever in maintenance (e.g. workspace owned by
a single user that got revoked while it was re-encrypting it), we also provide a cli for
removing realm vlobs (cannot remove a realm given it is related to certificates) or
organization form the database.
Copy link
Contributor

Choose a reason for hiding this comment

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

This CLI already exist or do we need to add an issue tracking the need ?

Copy link
Member Author

Choose a reason for hiding this comment

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

this cli doesn't exist, but I think this is part of the key rotation implementation so no need for a specific issue for this for the moment

Copy link
Contributor

Choose a reason for hiding this comment

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

I've create the issue anyway (#5782), in the best case it would be merge with the implementation of the RFC

FirelightFlagboy
FirelightFlagboy reviewed Nov 2, 2023
View reviewed changes
@touilleMan touilleMan force-pushed the rfc-realm-key-rotation branch from 50b995b to d263212 Compare November 10, 2023 11:46
@touilleMan touilleMan added this pull request to the merge queue Nov 10, 2023
Merged via the queue into master with commit 58738b9 Nov 10, 2023
@touilleMan touilleMan deleted the rfc-realm-key-rotation branch November 10, 2023 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmmarcos mmmarcos mmmarcos left review comments

@FirelightFlagboy FirelightFlagboy FirelightFlagboy left review comments

@TimeEngineer TimeEngineer TimeEngineer approved these changes

Assignees

@touilleMan touilleMan

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Design strategy for data migration (RFC)
4 participants
@touilleMan @FirelightFlagboy @mmmarcos @TimeEngineer