Move certificate extensions to entity alias metadata

[MochaCaffe]

Is your feature request related to a problem? Please describe.
@peterverraedt submitted a PR #13348 to add TLS certificates ASN1 extensions as identity metadata in Vault when using the cert auth method.
This would enable using certificates metadata in ACL templating to set a proper policy scope for each machine when accessing secrets.
However, it looks like extensions are not registered at all in Vault. It is expected that these should be set in the metadata property of an entity.

When looking at an entity used by a client using cert method:

vault read identity/entity/id/<id> | grep metadata

Key                    Value
---                    -----
metadata               <nil>

Describe the solution you'd like
I propose to save certificate metadata in the alias instead of using the parent entity.
In addition to solving this issue, I think using the alias is a more coherent approach, as it is closely linked with the certificate unlike the parent entity which is more global to the user
Related PR: #14419

[heatherezell]

Hi @MochaCaffe! We discussed this request in our engineering team sync today. While the request is reasonable, we'd like to take some time to dig into the potential implementation details and validate that there won't be the potential for unintended consequences. As a result, it may take longer for a final decision to be made, whether it's that we accept a PR from the community, or handle the work internally as part of an upcoming release, or if the decision ends up being that it would be safer (from a functionality, compatibility, or security point of view) not to take this request on. We will link this request internally and work with product and engineering leaders to come to a decision. Thanks in advance for your patience! :)

[HridoyRoy]

Hi @MochaCaffe ,
Thanks for submitting this issue! The change makes sense to us as the right path forward for enabling using certificates metadata in ACL templating. However, we're worried about storage concerns regarding this change, so we'd like for this to be an opt-in feature.

[MochaCaffe]

Hi @HridoyRoy , currently, only cert metadata having their keys present in the field "Allowed metadata extensions" in the auth method are stored. By default, this field is empty, letting the feature act as an "opt-in"

[HridoyRoy]

Hi @HridoyRoy , currently, only cert metadata having their keys present in the field "Allowed metadata extensions" in the auth method are stored. By default, this field is empty, letting the feature act as an "opt-in"

That makes sense to me. Still, as far as I understand it, currently allowed_metadata_extensions is used to attach the cert metadata to the entity. If a vault user is using this feature, and then upgrades to a new vault version where allowed_metadata_extensions attaches cert metadata to all entity aliases, the unintended storage cost will still be very high. So I think we still need a feature toggle for this, but I'll double check with some other folks on the engineering team just to be sure and report back if we think the toggle may not be necessary after all.

[HridoyRoy]

Update: we're currently investigating whether or not allowed_metadata_extensions gets added to the entity metadata. If not, then we don't necessarily have to worry about the upgrade scenario. Additionally, adding allowed_metadata_extensions to the entity alias metadata as opposed to the entity metadata might cause new aliases to be created every time a cert gets rotated. I'm not too sure about this, so it's something we'll be digging into.

I'm going to remove the waiting-for-response label on this issue while we follow up on these topics.

Thanks,
Roy

[pmmukh]

Hi! Just dropping an update here, so after digging into the code surrounding alias metadata updates, we've found that updating it does result in duplicate entity creation, when the alias is for a local auth mount on a non-primary cluster of a multi-cluster setup. We would ideally want to solve that issue first, before reviewing and merging this fix, so in the problematic scenario we don't end up with duplicate entities for the same mount every time a certificate gets rotated. We're discussing this internally, and I will be sure to drop updates as and when we make progress on this!
I do think it makes sense to be putting this info in the alias metadata versus the entity metadata proposed in the original issue, since the alias metadata is more logically tied to the cert auth mount, and it avoids overwrite issues if an entity is tied to multiple cert auth mounts.