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

Vault reorders PKI SAN list #20182

Open
jdloft opened this issue Apr 14, 2023 · 5 comments
Open

Vault reorders PKI SAN list #20182

jdloft opened this issue Apr 14, 2023 · 5 comments

Comments

@jdloft
Copy link

jdloft commented Apr 14, 2023

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:

  • Vault Server Version (retrieve with vault status): 1.12.1
  • Vault CLI Version (retrieve with vault version): N/A
  • Server Operating System/Architecture: Docker image hashicorp/vault:1.12.1 on CentOS 7
@cipherboy
Copy link
Contributor

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

I think this line:

cnAlt := strutil.ParseDedupAndSortStrings(cnAltRaw.(string), ",")

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
Copy link
Contributor

cipherboy commented Apr 17, 2023

@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
Copy link
Author

jdloft commented Apr 20, 2023

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
Copy link

gunnarhaslinger commented Jan 16, 2024

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
Copy link

luckyit-at commented Jan 23, 2024

+1

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

  • "Enforce SAN-Sorting from the request"

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants