Allow Multiple Issuers in PKI Secret Engine Mounts - PKI Pod

[cipherboy]

Overview

This pull request adds support for multiple issuers (mixed roots and intermediates) under the same PKI secrets engine mount point. We expose two new major API paths, /issuer{s} and /key{s}, for managing issuing certificates and their associated keys respectively.

The intent of this design was to enable a forward-rotation strategy for root CAs, with additional goals of aiding the live rotation of intermediates as well. Previously, to rotate issuing certificates in Vault 1.10.x and earlier, either the mount would have to be completely removed (and replaced with a new one -- with all audit and role information recreated) or the root/intermediate would have to have been deleted.

Existing APIs are preserved and will continue to function. We introduce the notion of a "default" issuer (/config/issuers) to handle underlying selection of issuer when existing APIs elide that information. Role now allow selection of an explicit issuer, allowing operators to choose issuers for users without them necessarily noticing the impact.

Unless otherwise unauthenticated, many of these new APIs should be considered privileged and adequately ACL'd.

Additionally, as a design choice, all certificate authorities (whether the mount's issuing certificate with backing key or a parent root certificate without key) in the mount have an issuer entry. This allows a mount's functional issuers to change over time (including the introduction of a soft delete, wherein both keys and certificates for an issuer are perserved, but the ability to revoke its certificates remains). There's now explicit chain building support as well; previously on many intermediate mounts, the parent root certificate(s) were not imported -- they can now be imported and exposed to callers.

However, as a result of this design, we open the possibility for many new, streamlined usages of the PKI secrets engine. To give just one example, a twin-intermediate approach is now viable, wherein high-impact certs could be signed by an HSM-backed intermediate and low-impact certs could be signed by a Vault-backed intermediate, all chosen by the operator based on user role selection. Users could be transparently switched between intermediates, without needing to update their client configuration. This is enabled by allowing multiple active issuers in the same mount.

---

Review Strategy

This PR is segmented into a number of incremental commits; see incremental commit messages and changes for details.

At a high level, we'd love to see the following questions answered:

1. Are there any UX gaps in this API as proposed now, that need to be addressed (either as part of this PR or separately)?
2. Is forwarding of requests handled correctly? (Active node of the primary cluster needs to handle much of issuer&key responses along with role and configuration information). Discussed in call Initial Website Import #1.
3. Do we need a read lock on the active primary/secondary &c for issuer consistency? These should be relatively low-traffic APIs, but there's a risk. How would such a distributed lock be created and held? Notably, issuers are only modified on the active node of the primary cluster, but these get replicated on storage to other nodes in secondary clusters, and thus they may have a temporarily inconsistent view of storage that may allow requests to suceed/fail until they've sufficiently caught up. Discussed in call Demo updates #3.
4. There's many places RawSubject and RawIssuer are used directly for comparisons, but I believe this is incorrect; in particular, RDNs are SEQUENCEs of SETs, which means the internal comparison should be an unordered comparison, meaning we may incorrectly deny some valid links due to differing order of RDN pieces. Is this critical? I don't believe so right now as we haven't previously had support for this.
5. As part of this work, CRLs will now be rebuilt on secondary clusters when necessary; in particular, this means that when adding a new secondary cluster, it will no longer serve a nil CRL until a certificate is revoked (or the CRL is manually rotated). This shouldn't be a problem.
6. Are there any new security concerns or are the privileged APIs largely understood? I think we largely understand the security concerns and access to issuers/ and issuer/ authed endpoints could/should be restricted by policy, not by sudo permission.

Outstanding Work

There's a bit of work left to enable managed keys in Enterprise. @stevendpclark has most of this work already in branches and it will be opened in parallel. When this merges, we'll do an OSS->ENT merge and then merge that PR.

We aim to store the issuer ID used to issue requests (when no_store=false); this work isn't done but doesn't hold up this merge.

There's another piece of correctness around issuer import (ensuring correct comparisons even when PEM contents differ); this also shouldn't hold up the merge.

There's additional work to allow revoking an issuer if another issuer in the same mount signed it; this can happen after the main merge.

Additional, more comprehensive integration tests (for this feature set in particular) are still WIP and will be merged later.

The many UI changes to incoprorate this feature set will come at a separate point in time.

Extensive documentation for this change set is still outstanding; API docs are started in a separate PR. Other docs, learn guides, and tutorials are still outstanding.

Miscellanea

Incremental PRs

In working towards this feature, many incremental PRs were made towards this feature. We've used the rotation label for this; please feel free to walk through earlier discussions on earlier PRs for some discussion on these topics.

Hashicorp Internal Context

See internal RFC VLT-191 for extensive information about this change set; feel free to ask questions on #team-vault-crypto.

Meeting invites will be scheduled to give crypto team and core team a place to discuss this PR and start reviewing it. In discussion with @sgmiller, we felt it was best to open a single large PR, rather than attempting to recreate the incremental PRs above. This PR is essentially a large refactoring of the PKI code base, breaking several

Thanks

Many thanks to @kitography and @stevendpclark who helped with implementation of this feature!

Additionally, many thanks to everyone else who contributed at earlier stages, whether with great UX designs (@ivana-mcconnell :D), BK, Jeff Mitchell and Scott, the entire Vault Crypto team, or anyone else who reviewed drafts of this RFC, plus anyone else I forgot to thank.

If possible, I'd like to see this rebased (rather than squash merged), to give adequate credit to everyone who's contributed along the way. Many internal PRs were squashed merged and I believe the commit series makes most sense separately rather than as a large single change.