hashicorp / vault

A tool for secrets management, encryption as a service, and privileged access management
https://www.vaultproject.io/
Other
30.84k stars 4.17k forks source link

Vault reorders PKI SAN list #20182

Open jdloft opened 1 year ago

jdloft commented 1 year ago

Describe the bug Vault reorders DNS SAN list when requesting a certificate. Firefox bug #1757758 details how Firefox evaluates SANs in a top-down order and will reject a cert at the first invalid line and Firefox bug #1196364 describes how Firefox rejects wildcard SANs for prefixes not found in the Public Suffix List.

The workaround then if one wants a certificate signed for non-wildcarded domains and a wildcarded custom TLD is to place the wildcard at or near the bottom of the SAN list after any valid SAN entries that will be used for Firefox. Vault reorders or sorts the list and prevents this from happening.

To Reproduce Steps to reproduce the behavior:

  1. Generate a certificate via the UI for DNS SANs "custom.local, *.custom.local"
  2. View certificate (openssl x509 -in cert.crt -text -noout)
  3. Observe the SAN list is reordered (DNS:*.custom.local, DNS:custom.local)

Expected behavior SAN list should be deduplicated without reordering.

Environment:

cipherboy commented 1 year ago

@jdloft Interesting, I would've expected this behavior to only apply to publicly chained CAs and not private CAs.

I think this line:

https://github.com/hashicorp/vault/blob/3acbeddf7a58915c41310fae41998e4985ca19e5/builtin/logical/pki/cert_util.go#L1146

Could be replaced with a line to strutil.RemoveDuplicatesStable but you'll need to duplicate this logic:

https://github.com/hashicorp/go-secure-stdlib/blob/c651aeae0c80f35905cf5a275c55e494c0eae09f/strutil/strutil.go#L56-L67

Let me know if you want to file a patch or if you want me to.

cipherboy commented 1 year ago

@jdloft I think this does have an unintended side-effect though: if a user requested a cert with identifiers in the order CUSTOM-INVALID, *.example.com, our sorting today would result in *.example.com, CUSTOM-INVALID being issued, which would pass in Firefox for e.g., mine.example.com, whereas with the "fixed" new request sorting of CUSTOM-INVALID, *.example.com would result in the request failing.

(CUSTOM-INVALID meaning an invalid DNS entry or entry that would otherwise fail Firefox's cert parsing).

If we do make this change, I think it would have to be 1.14 only, to avoid breaking people for the above reason (though, admittedly you are broken by this behavior) -- this behavior is original to the engine AFAICT.

jdloft commented 1 year ago

I agree, this is breaking. I'm not sure how important this is anyways; we ended up finding a workaround so this no longer breaks us. Maybe this should be left as documentation?

gunnarhaslinger commented 7 months ago

Hi @jdloft and @cipherboy

Thanks a lot for authoring and discussing this issue.

I'm affected by this VAULT-DNS-SAN-List sorting problem and would really appreciate if Vault could keep the requested SAN-ordering.

I'm happy you mentioned Firefox Issue #1757758 ... coincidentally I'm the author of exactly this Firefox-Bug, so I very well know the issue and consequences.

You are worried about breaking things when you change the functionality to just "Remove Duplicates" instead of "Deduplicate and Sort"? OK, it could change the behaviour for people using "invalid" Wildcard-TLD SANs with Firefox, but I cannot see a huge risk of making it worse than the current situation.

Currently you would sort all entries starting with an asterisk "*" to the top. So what your sort currently does is to increase the likeliness of being affected by the Firefox Issue to almost 90-100%. If you just keep the sorting as requested (do not resort) it is in the requesters responsibility to request a suitable order which works in his scenario. Currently the requester has no control about the order and a "*.myPrivateTLD" request would automatically be sorted to the top.

So I would please ask to change Vault behaviour to respect the original SAN-List-Sorting of the request.

luckyit-at commented 7 months ago

+1

You could also provide a setting like "Allow subdomains, allow global domains,..."

Then the sorting would be adopted based on the settings - either as before or as conveyed in the Request.