-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Concern about change to ca_chain
behaviour in 1.11.0-rc1 - confusing changelog entry for PR that was not merged, unclear if changes were introduced elsewhere
#16057
Comments
It appears that this changelog entry arrived via #15500, even though #15155 was never actually merged. It's difficult to understand the meaning of this changelog entry, since it was originally written for a PR which wasn't merged. As a result, I'm not sure if there's an issue here or not. (Other than the changelog being confusing - that's definitely an issue.) |
ca_chain
behaviour in 1.11.0-rc1 - may need revertingca_chain
behaviour in 1.11.0-rc1 - confusing changelog entry for PR that was not merged, unclear if changes were introduced elsewhree
ca_chain
behaviour in 1.11.0-rc1 - confusing changelog entry for PR that was not merged, unclear if changes were introduced elsewhreeca_chain
behaviour in 1.11.0-rc1 - confusing changelog entry for PR that was not merged, unclear if changes were introduced elsewhere
\o Hey there @maxb The linked PR was manually merged into the There are actually several reasons for this change. Firstly, RFC 8466 notes that the root MAY be omitted. MAY is relatively weak in RFC language. For maximum compatibility, the root should not be omitted. Notably, it is much easier to remove the root (on the server) once presented (by Vault), than it is to guess at the location of a root (in Vault) if it isn't present. I personally wouldn't give end-users (consumers of the intermediate/signing endpoints) access to the root mount anyways, though the raw Secondly, note that if the root is reissued (i.e., same key material and Subject but different expiration), the new root may not be present in browsers' trust stores, but will be validated through the legacy root (if they don't check root expiration -- see Let's Encrypt's legacy Android support threads for a great example of that). If servers are properly configured to serve the entire chain, this type of temporal rotation (without re-keying) is possible without impact to clients that have been offline since the new root was generated (and thus haven't updated their trust stores yet). This type of reissuance is now possible in Vault 1.11 and thus it makes sense to support building those chains. Finally, note that the |
Interesting, I learnt something new, I had thought this would be a hard failure:
Thanks. |
Hi @cipherboy I'd like to re-open this for further discussion please.
I don't follow... why do you say it's a best practice? Either they already have the root CA in their trust store, and don't need another copy of it - or they do not, in which case sending a self-signed certificate in the TLS handshake is not going to induce them to start trusting it.
Now that I have had time to review the linked document, I realise it does not match what you are asserting here. Let's Encrypt's description of the situation, is that rather than referring to a "temporally renewed" root in the certificate chain, Android simply does not care at all about expiration dates of roots. The specific part of your linked document which I'm referencing in support of this is:
|
With the new argument introduced in #16935 for Vault 1.12, end-users should be able to get back the old behavior for mounts that were initialized with the |
This caught my eye in the 1.11.0-rc1 changelog:
secrets/pki: The ca_chain response field within issuing (/pki/issue/:role) and signing APIs will now include the root CA certificate if the mount is aware of it. [/~https://github.com//pull/15155]
It is frustrating that the linked PR contains no explanation for the reason for the change. Also frustrating that actual linked PR was never merged - I guess the changelog has the wrong number?
There is a solid technical reason for not including the root CA certificate in
ca_chain
fields. The chain purpose is to build the chain of trust through intermediate CAs. Including root CAs in the chain (i.e. CAs where issuer == subject) has no role in chain building, and just wastes bytes on the network during TLS handshakes.The text was updated successfully, but these errors were encountered: