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

Add baseName to ExternalContent #71

Merged
merged 25 commits into from
Aug 17, 2023
Merged

Conversation

matheus23
Copy link
Member

We explicitly add baseName to the ExternalContent record.
Usually baseName = file.header.name.add(externalContent.key), but in some cases it's nice to for it to be something different.
This is nice, because it makes ExternalContent self-contained. You only need the information from ExternalContent to be able to read the whole content bytes.
This allows us to potentially use it in other places (e.g. thinking of #50), as well as copying it over from other PrivateFiles, in case a file gets moved or copied, without having to re-encrypt a bunch of data.

merge order

  1. Update rsa modulus endianess to match most protocols #66
  2. Remove namefilter.md and add NameAccumulator verification algorithm #68
  3. Specify Blake3's derive_key algorithm instead of SHA3 #69
  4. Switch from AES-GCM to XChaCha20-Poly1305 #70
  5. This PR

@@ -211,7 +212,9 @@ Private file content has two variants: inlined or externalized. Externalized con

#### 3.1.4.1 Externalized Content
Copy link
Member

Choose a reason for hiding this comment

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

Nit: it would be good to expand section 3.1.4. I've had to re-load a bunch of context about the difference between inline and external content, which means that it will very likely be confusing to someone new coming in.

Copy link
Member

Choose a reason for hiding this comment

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

Specifically some examples of when you'd want one or the other would be helpful

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to expand the section a little bit 👀
LMK what you think

Copy link
Member

Choose a reason for hiding this comment

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

Super great! Thanks :)

@@ -211,7 +212,9 @@ Private file content has two variants: inlined or externalized. Externalized con

#### 3.1.4.1 Externalized Content

Since external content blocks are separate from the header, they MUST have a unique `NameAccumulator` derived from a random key (to avoid forcing lookups to go through the header). If the key were derived from the header's key, then the file would be re-encrypted e.g. every time the metadata changed. See [sharded file content access algorithm](#44-sharded-file-content-access) for more detail.
Since external content blocks are separate from the header, they each MUST have a `NameAccumulator` that is different than the file's `NameAccumulator`. We allow these names to have an arbitrary base. For the normal case, the base name is RECOMMENDED to be the file's name with the externalized content's encryption `key`, hashed to a prime, added to it as a name segment.
Copy link
Member

Choose a reason for hiding this comment

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

Is "base" here a term defined elsewhere? It may be worth linking to it, or using a more descriptive term here

Copy link
Member

@expede expede Aug 10, 2023

Choose a reason for hiding this comment

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

Perhaps it's this field?

baseName: NameAccumulator

Copy link
Member

Choose a reason for hiding this comment

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

the file's name with

The nameaccumulator, or the human readable name? The with here is concatenation?

Copy link
Member Author

Choose a reason for hiding this comment

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

The NameAccumulator. I tried to make this more explicit by saying "file's name from its header".
LMK if that's better :)

@matheus23 matheus23 force-pushed the matheus23/external-content-name branch from d0b834b to 637f1da Compare August 11, 2023 14:17
Base automatically changed from matheus23/xchacha20-poly1305 to main August 11, 2023 14:59
Copy link
Member

@expede expede left a comment

Choose a reason for hiding this comment

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

Looks good! 👏

@@ -211,7 +212,9 @@ Private file content has two variants: inlined or externalized. Externalized con

#### 3.1.4.1 Externalized Content
Copy link
Member

Choose a reason for hiding this comment

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

Super great! Thanks :)

@matheus23 matheus23 merged commit 58b62bf into main Aug 17, 2023
1 check passed
@matheus23 matheus23 deleted the matheus23/external-content-name branch August 17, 2023 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants