search

linagora / james-project

Mirror of Apache James Project
Apache License 2.0
72 stars 62 forks source link

[S3 SSEC] S3 multitenancy: SSEC key derivation #5279

Open Arsnael opened 1 month ago

Arsnael commented 1 month ago

Allows using an encryption key per blob (derived from the blobId) hence making each encryption key unique: if I break one key, I decrypt only one object, not all.

Configuration:

encryption.s3.sse.c.key.derivation.enabled=true

We could start by: domain + blobId enough to derivate the key

DoD: Integration tests + documentation

quantranhong1999 commented 1 month ago

So should we do https://github.com/linagora/james-project/issues/5280 first to determine the best way to derive the encryption key?

Arsnael commented 1 month ago

Not sure. we need a base implem I would say, and then you just change the salt in the code for key derivation and test easily. But that's my point of view

chibenwa commented 3 weeks ago

Baseline implementation could get merged and forgotten and causemigration headache if suboptimal.

IMO #5280 shall be conducted first, if not as part of the grooming...

chibenwa commented 3 weeks ago

Discussed with Xavier: PBKDF2WithHmacSHA256 with domain used as a salt is great, we can easily cache the resulting keys so that they are computed once.

This way we do not compromise on safety and performance.

Arsnael commented 3 weeks ago

So in case of algorthm broken on one object, logically you could decrypt all objects of a same domain? It's slightly better than one salt for all domains but is it enough for safety? Implem with one domain for example then key derivation does not matter.

Or should we allow different configurations with proper doc?

For example:

encryption.s3.sse.c.key.derivation.salt=domain|blobid

use domain as salt if you want to keep good perf with a minimum of safety, use blobid if you want to be an absolute maniac on safety att he cost of perf? Up to the admin. Of course defaults to domain

chibenwa commented 3 weeks ago

It's slightly better than one salt for all domains but is it enough for safety?

Yes, SSE-C already should derive keys on a per object basis and if not enough this is a choice we could re-challenge later: I'm happy with it and so do Xavier.

Using blobId as a salt BTW do not ensure cross-domain isolation constraints. (to do so it should be domain + blobId as a salt)