Consider including the Saved Object's top-level created_by and updated_by properties in the AAD automatically.

[azasypkin]

Summary

Now that we have top-level created_by and updated_by properties in the Saved Objects, it makes sense to automatically include them in the AAD for encrypted Saved Objects if present. The tricky part will be to not break existing ESOs with populated created_by and updated_by (to be released in 8.14.0 - #179344).

[elasticmachine]

Pinging @elastic/kibana-security (Team:Security)

[legrego]

The tricky part will be to not break existing ESOs with populated created_by and updated_by (to be released in 8.14.0 - #179344).

Yeah that's a good point. What options can we explore?

1. Prevent specific SOs from including these attributes if we detect they are "pre-existing" somehow.
2. Try to decrypt with and without these fields inside the AAD, and see which one works (sounds like a bad idea)
3. Perform a massive ESO migration to re-encrypt these SOs with the new AAD fields (sounds like a bad idea)
4. ??

[jeramysoucy]

1. Prevent specific SOs from including these attributes if we detect they are "pre-existing" somehow.

This option wouldn't be too bad, but might seem hacky. We could just look at the creation time choose to add the extra AAD fields if created after xxxx/xx/xx.

2. Try to decrypt with and without these fields inside the AAD, and see which one works (sounds like a bad idea)

Yeah, this could add a lot of overhead in some situations. We use this sort of method when there are multiple decrypt keys, do we have an idea of the possible impact from that scenario as a baseline?

3. Perform a massive ESO migration to re-encrypt these SOs with the new AAD fields (sounds like a bad idea)

Yeah this could be really painful. Do we have telemetry on average number of ESOs/deployment, or on key rotation mass re-encrypt duration?

??

No other ideas yet, but I'm still thinking through it. I am trying to think if there might be a way to use model versions to support option 1, but not sure that makes sense or is feasible yet.

[azasypkin]

1. Prevent specific SOs from including these attributes if we detect they are "pre-existing" somehow.

This option wouldn't be too bad, but might seem hacky. We could just look at the creation time choose to add the extra AAD fields if created after xxxx/xx/xx.

I don't know yet how we can do that, but conceptually, it feels like the ideal solution to me. Dealing with the creation date might be tricky since we don't know when the customer upgrades to a newer version, but if we could know the exact version in which the SO was created or updated (whichever is the latest), it would solve our problems. Sounds familiar, right (Versioned Encryption Definition)? 🙂

2. Try to decrypt with and without these fields inside the AAD, and see which one works (sounds like a bad idea)

Yeah, this could add a lot of overhead in some situations. We use this sort of method when there are multiple decrypt keys, do we have an idea of the possible impact from that scenario as a baseline?

If we say that we use updated_by instead of created_by as AAD when updated_by is present (can we?), then the impact might not be that large since it will only affect objects created before our changes and haven't been updated since. However, what concerns me with this option isn't the performance impact, but that it defeats the purpose of AAD. Now, it would be possible to decrypt any secret encrypted without created_by in AAD in a context where user information is present, if a malicious actor manages to change the context somehow.

3. Perform a massive ESO migration to re-encrypt these SOs with the new AAD fields (sounds like a bad idea)

Yeah this could be really painful. Do we have telemetry on average number of ESOs/deployment, or on key rotation mass re-encrypt duration?

Agree, it can be quite troublesome. I did some measurements around rotation here: #72420 (see tables at the bottom of the PR description), but it makes sense to re-measure if we decide to go this route.

4. ??

No other ideas yet, but I'm still thinking through it. I am trying to think if there might be a way to use model versions to support option 1, but not sure that makes sense or is feasible yet.

Same, no ideas yet, but I will think about it. We could embed the version of the "built-in" AAD into the encrypted value directly in one way or another (e.g., -->aad_v1<--_a23baXzxxxxx), but then we'll be re-inventing a poor-man's version of what we already designed in the VED RFC.

[legrego]

I don't like this option, but I mention it in case it sparks a better idea for some else.

We could exclude all current ESO types from using these fields, but include them by default for any new ESO types that are introduced. We don't introduce new ESO types all that frequently, so this wouldn't provide a meaningful benefit. It also provides zero benefit for our current ESOs.

[jeramysoucy]

Sounds familiar, right (Versioned Encryption Definition)? 🙂
...
but then we'll be re-inventing a poor-man's version of what we already designed in the VED RFC.

So it's settled. We'll implement the VED system. 👍 😉

But you're right, I didn't think this all the way through. For serverless ZDT, there are further implications. Even if we could access the updated time and decide when to include the extra AAD during decryption, the previous version of Kibana will fail trying to decrypt any of the re-encrypted objects. This would only be a concern for one version upgrade, but still something to consider.

[jeramysoucy]

I don't think we can automatically include updated_by because technically, partial updates are possible on attributes that are not encrypted or part of AAD. If we want to include updated_by, we'd have to force re-encryption even on valid partial updates.

[azasypkin]

I don't think we can automatically include updated_by because technically, partial updates are possible on attributes that are not encrypted or part of AAD. If we want to include updated_by, we'd have to force re-encryption even on valid partial updates.

Oh, yeah, it's a good point. Unless we can somehow rely on a new behavior in SO land shared in #50256 (comment)

[jeramysoucy]

Another thought - we used to include namespace in AAD for ESOs with the legacy single namespace type. But we currently do not include namespace(s) as part of AAD for any multi-namespace SO types, including multiple-isolated. We should consider adding namespace(s) to AAD because they offer a unique protection that other fields cannot - validating an ESO hasn't been maliciously or accidentally moved to a different space. With this, we run into the same issue - how do we handle whether we should include it when decrypting an existing ESO, and ZDT.