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

Separate encryption key for each tenant #752

Closed
dirkam opened this issue Nov 12, 2021 · 8 comments
Closed

Separate encryption key for each tenant #752

dirkam opened this issue Nov 12, 2021 · 8 comments
Assignees
Labels
feature New feature or request feedback wanted

Comments

@dirkam
Copy link

dirkam commented Nov 12, 2021

Description

From a security point of view it'd be a great option to have different encryption keys for each tenant. By default Laravel uses the APP_KEY env variable for the encryption, which if I'm not mistaken is shared by all tenants, meaning that all encrypted data can be decrypted with the given key regardless of the tenant.

Why this should be added

Separate encryption key for each tenant would greatly enhance the security of any multi-tenant application, could help with compliance requirements, etc.

@dirkam dirkam added the feature New feature or request label Nov 12, 2021
@darkons
Copy link

darkons commented Nov 15, 2021

On tenant creation you can generate an APP_KEY and store in your tenant data column:

$tenantKey = 'base64:' . base64_encode(
     \Illuminate\Encryption\Encrypter::generateKey($this->laravel['config']['app.cipher'])
);

Then you can use Tenant Config feature to configure it.

@stancl
Copy link
Member

stancl commented Nov 15, 2021

That works but there aren't really any security gains associated with using this.

Separate encryption key for each tenant would greatly enhance the security of any multi-tenant application

How? And if you used tenant-specific keys where would you store them?

@dirkam
Copy link
Author

dirkam commented Nov 15, 2021

Well, this is not an easy question, of course. The default Laravel implementation stores the key in the .env file. In that case at least it's separated from the database. If the databased is dumped due to some kind of vulnerability then the key is not present in the DB. Or, if the DB is physically separated from the application (i.e. the DB is on a different server) then compromising one of the servers might not be enough to decrypt the values.

@darkons idea works I guess, but in that case the key is stored in the DB, which is a not a good idea, I'm afraid.

What we could do is to store the tenant's key in a file, not in the DB. And the tenants' keys could be encrypted with the central's key (.env APP_KEY).

What do you think?

@stancl
Copy link
Member

stancl commented Nov 15, 2021

Ultimately if the app can access the key in any way (it needs to, so that it's able to serve tenants), then the type of security compromise that you're trying to protect against would still lead to all tenant data being accessible.

One valid use case of this can be to make sure sessions/cookies are different on each domain I think.

@dirkam
Copy link
Author

dirkam commented Nov 16, 2021

That's true, but that's the case with all encryption. Even if you store the keys in HSM the app still needs to somehow decrpyt the data so at least in the memory there will be something that could be used by attackers, but that's out of the scope of this question.
Sessions/cookies are indeed a great example. Signed URLs, too.
The main thing is to protect the data at rest. Also, most of the compliance requirements states to use different encryption keys in a multi-tenant environment (along with different databases and database users, but these are covered as far as I'm aware). And it just makes sense in a secure by design approach IMO.

@dirkam
Copy link
Author

dirkam commented Aug 12, 2022

@stancl will v4 provide something for this?

@stancl
Copy link
Member

stancl commented Aug 14, 2022

We can consider that, but I'd need to hear some arguments about actual security benefits that'd come from this.

As I stated above, an attacker would gain access to data by compromising the application in the vast majority of cases. And when the application is compromised, all tenant keys would be exposed because the application has access to them.

I can only see this being beneficial in cases where the attacker would gain access to a single tenant's data. Which is only realistic in uncommon setups. And even then — what would this actually protect against? If the data were encrypted using the app key and the attacker had the app key, then he presumably compromised the application already, thus gaining access to all tenant keys. And if the attacker would only have the data but not the key, there's no difference in what key is used.

@Obyka
Copy link

Obyka commented Jul 23, 2024

One can imagine that depending on the tenant, a different security posture may be adopted.
For example, some tenants might want APP_KEY rotation while others might not.

You can also imagine a use case where the key is weak / compromised because of a tenant implementation, you then would benefit from a separated one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request feedback wanted
Projects
None yet
Development

No branches or pull requests

4 participants