* Use UTC for leaf exceeding CA's notAfter
When generating a leaf which exceeds the CA's validity period, Vault's
error message was confusing as the leaf would use the server's time
zone, but the CA's notAfter date would use UTC. This could cause
user confusion as the leaf's expiry might look before the latter, due
to using different time zones. E.g.:
> cannot satisfy request, as TTL would result in notAfter
> 2023-03-06T16:41:09.757694-08:00 that is beyond the expiration of
> the CA certificate at 2023-03-07T00:29:52Z
Consistently use UTC for this instead.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Apply URL encoding/unencoding to OCSP Get requests
- Missed this during development and sadly the unit tests were written
at a level that did not expose this issue originally, there are
certain combinations of issuer cert + serial that lead to base64
data containing a '/' which will lead to the OCSP handler not getting
the full parameter.
- Do as the spec says, this should be treated as url-encoded data.
* Add cl
* Add higher level PKI OCSP GET/POST tests
* Rename PKI ocsp files to path_ocsp to follow naming conventions
* make fmt
* PKI Unified CRL/OCSP apis should be ent only
- Do not enable any of the unified crl/ocsp related apis on OSS.
* Rollback refactoring of pathFetchCRLViaCertPath
- As pointed out in the PR, this method isn't actually being used at
the moment with the <serial> handler, pathFetchValid, matching
everything under the cert/XXXX path.
* Fix schema for ent/oss diff
- Define the OSS vs ENT urls we want to see within the schema
definition even if they aren't really going to be used in the end.
* Address pki::TestAutoRebuild flakiness
- Wait for a CRL change before progressing to the next step after
we change configuration. Prior to this we would be racing against
the CRL reloading from the configuration change.
* Read total cert counts with atomic.LoadUint32(...)
When generating the tidy status, we read the values of two backend
atomics, b.certCount and b.revokedCertCount, without using the atomic
load operation. This resulted in a data race when the status was read
at the same time as an on-going tidy operation:
WARNING: DATA RACE
Write at 0x00c00c77680c by goroutine 90522:
sync/atomic.AddInt32()
/usr/local/go/src/runtime/race_amd64.s:281 +0xb
sync/atomic.AddUint32()
<autogenerated>:1 +0x1a
github.com/hashicorp/vault/builtin/logical/pki.(*backend).tidyStatusIncRevokedCertCount()
/home/circleci/go/src/github.com/hashicorp/vault/builtin/logical/pki/path_tidy.go:1236 +0x107
github.com/hashicorp/vault/builtin/logical/pki.(*backend).doTidyRevocationStore()
/home/circleci/go/src/github.com/hashicorp/vault/builtin/logical/pki/path_tidy.go:525 +0x1404
github.com/hashicorp/vault/builtin/logical/pki.(*backend).startTidyOperation.func1.1()
/home/circleci/go/src/github.com/hashicorp/vault/builtin/logical/pki/path_tidy.go:290 +0x1a4
github.com/hashicorp/vault/builtin/logical/pki.(*backend).startTidyOperation.func1()
/home/circleci/go/src/github.com/hashicorp/vault/builtin/logical/pki/path_tidy.go:342 +0x278
Previous read at 0x00c00c77680c by goroutine 90528:
reflect.Value.Uint()
/usr/local/go/src/reflect/value.go:2584 +0x195
encoding/json.uintEncoder()
/usr/local/go/src/encoding/json/encode.go:562 +0x45
encoding/json.ptrEncoder.encode()
/usr/local/go/src/encoding/json/encode.go:944 +0x3c2
encoding/json.ptrEncoder.encode-fm()
<autogenerated>:1 +0x90
encoding/json.(*encodeState).reflectValue()
/usr/local/go/src/encoding/json/encode.go:359 +0x88
encoding/json.interfaceEncoder()
/usr/local/go/src/encoding/json/encode.go:715 +0x17b
encoding/json.mapEncoder.encode()
/usr/local/go/src/encoding/json/encode.go:813 +0x854
... more stack trace pointing into JSON encoding and http
handler...
In particular, because the tidy status was directly reading the uint
value without resorting to the atomic side, the JSON serialization could
race with a later atomic update.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Also use atomic load in tests
Because no tidy operation is running here, it should be safe to read the
pointed value directly, but use the safer atomic.Load for consistency.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
- This has been done to help diagnose errors in the future so that
we get the callers in the trace's when we fail and not just the
helper's trace output.
* Allow unification of revocations on other clusters
If a BYOC revocation occurred on cluster A, while the cert was initially
issued and stored on cluster B, we need to use the invalidation on the
unified entry to detect this: the revocation queues only work for
non-PoP, non-BYOC serial only revocations and thus this BYOC would be
immediately accepted on cluster A. By checking all other incoming
revocations for duplicates on a given cluster, we can ensure that
unified revocation is consistent across clusters.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Use time-of-use locking for global revocation processing
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Unified revocation migration code
- Add a periodic function that will list the local revocations
and if any are missing from the unified revocation area will
force a write to the unified revocation folder/remote instance.
* PR Feedback
- Do not transfer expired certificates to unified space from local
- Move new periodic code into a periodic.go file
- Add a flag so we only run this stuff once if all is good, with
a force flag if we encounter errors or if unified_crl is toggled
on
* PR feedback take 2
- Return a detailed reponse within the list api that an end-user can
use to determine what clusters revoked the certificate on from the
pki/certs/unified-revoked LIST api.
- Return colon delimited serial numbers from the certs/revocation-queue
LIST api
* Add new tidy operation for cross revoked certs
This operation allows tidying of the cross-cluster revocation storage.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix missing cancels, status values
Previous additions to tidy didn't have enough cancel operations and left
out some new values from the status operation.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
- I missed this in the original review, that we were storing the
unified-crl in a cluster-local storage area so none of the other
hosts would receive it.
- Discovered while writing unit tests, the main cluster had the unified
crl but the other clusters would return an empty response
* The fields.
* UserID set, add to certificate
* Changelog.
* Fix test (set default).
* Add UserID constant to certutil, revert extension changes
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add user_ids as field for leaf signing
Presumably, this isn't necessary for CAs, given that CAs probably don't
have a user ID corresponding to them.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Support setting multiple user_ids in Subject
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow any User ID with sign-verbatim
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for User IDs in PKI
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add docs about user_ids, allowed_user_ids
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: Alexander Scheel <alex.scheel@hashicorp.com>
- It does not make sense to allow operators to enable the cross-cluster
revocation features on local mounts as they will never have a
corresponding mount on the other cluster.
* Add unified CRL config storage helpers
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add support to build unified CRLs
This allows us to build unified versions of both the complete and delta
CRLs. This mostly involved creating a new variant of the
unified-specific CRL builder, fetching certs from each cluster's storage
space.
Unlike OCSP, here we do not unify the node's local storage with the
cross-cluster storage: this node is the active of the performance
primary, so writes to unified storage happen exactly the same as
writes to cluster-local storage, meaning the two are always in
sync. Other performance secondaries do not rebuild the CRL, and hence
the out-of-sync avoidance that we'd like to solve with the OCSP
responder is not necessary to solve here.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add ability to fetch unified CRLs
This adds to the path-fetch APIs the ability to return the unified CRLs.
We update the If-Modified-Since infrastructure to support querying the
unified CRL specific data and fetchCertBySerial to support all unified
variants. This works for both the default/global fetch APIs and the
issuer-specific fetch APIs.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Rebuild CRLs on unified status changes
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Handle rebuilding CRLs due to either changing
This allows detecting if the Delta CRL needs to be rebuilt because
either the local or the unified CRL needs to be rebuilt. We never
trigger rebuilding the unified delta on a non-primary cluster.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Ensure serials aren't added to unified CRL twice
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Write delta WAL entries for unified CRLs
When we'd ordinarily write delta WALs for local CRLs, we also need to
populate the cross-cluster delta WAL. This could cause revocation to
appear to fail if the two clusters are disconnected, but notably regular
cross-cluster revocation would also fail.
Notably, this commit also changes us to not write Delta WALs when Delta
CRLs is disabled (versus previously doing it when auto rebuild is
enabled in case Delta CRLs were later asked for), and instead,
triggering rebuilding a complete CRL so we don't need up-to-date Delta
WAL info.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update IMS test for forced CRL rebuilds
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Move comment about perf-primary only invalidation
Also remove noisy debug log.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Remove more noisy log statements during queue
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Skip revocation entries from our current cluster
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add locking and comment about tidying revoke queue
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Switch to time.Since for tidy
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor tidyStatuses into path_tidy.go
Leaving these in backend.go often causes us to miss adding useful values
to tidyStatus when we add a new config parameter.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Track the number of deleted revocation request
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow tidy to remove confirmed revocation requests
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add missing field to tidy test
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add global, cross-cluster revocation queue to PKI
This adds a global, cross-cluster replicated revocation queue, allowing
operators to revoke certificates by serial number across any cluster. We
don't support revoking with private key (PoP) in the initial
implementation.
In particular, building on the PBPWF work, we add a special storage
location for handling non-local revocations which gets replicated up to
the active, primary cluster node and back down to all secondary PR
clusters. These then check the pending revocation entry and revoke the
serial locally if it exists, writing a cross-cluster confirmation entry.
Listing capabilities are present under pki/certs/revocation-queue,
allowing operators to see which certs are present. However, a future
improvement to the tidy subsystem will allow automatic cleanup of stale
entries.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow tidying revocation queue entries
No manual operator control of revocation queue entries are allowed.
However, entries are stored with their request time, allowing tidy to,
after a suitable safety buffer, remove these unconfirmed and presumably
invalid requests.
Notably, when a cluster goes offline, it will be unable to process
cross-cluster revocations for certificates it holds. If tidy runs,
potentially valid revocations may be removed. However, it is up to the
administrator to ensure the tidy window is sufficiently long that any
required maintenance is done (or, prior to maintenance when an issue is
first noticed, tidy is temporarily disabled).
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Only allow enabling global revocation queue on Vault Enterprise
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Use a locking queue to handle revocation requests
This queue attempts to guarantee that PKI's invalidateFunc won't have
to wait long to execute: by locking only around access to the queue
proper, and internally using a list, we minimize the time spent locked,
waiting for queue accesses.
Previously, we held a lock during tidy and processing that would've
prevented us from processing invalidateFunc calls.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* use_global_queue->cross_cluster_revocation
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Grab revocation storage lock when processing queue
We need to grab the storage lock as we'll actively be revoking new
certificates in the revocation queue. This ensures nobody else is
competing for storage access, across periodic funcs, new revocations,
and tidy operations.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix expected tidy status test
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow probing RollbackManager directly in tests
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Address review feedback on revocationQueue
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add more cancel checks, fix starting manual tidy
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor CRL building into separate functions
This will allow us to add the ability to add and build a unified CRL
across all clusters, reusing logic that is common to both, but letting
each have their own certificate lists.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Rename localCRLConfigEntry->internalCRLConfigEntry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Rename Delta WALs to Local Delta WALs
This adds clarity that we'll have a separate local and remote Delta CRL
and WALs for each.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Rename revokeCert variable to identify serial number formatting
* Refactor out lease specific behavior out of revokeCert
- Isolate the specific behavior regarding revoking lease specific
certificates outside of the revokeCert function and into the only
caller that leveraged used it.
- This allows us to simplify revokeCert a little bit and keeps the
function purely about revoking a certificate
* Within revokeCert short circuit the already revoked use-case
- Make the function a little easier to process by exiting early
if the certificate has already been revoked.
* Do not load certificates from storage multiple times during revocation
- Isolate the loading of a certificate and parsing of a certificate
into a single attempt, either when provided the certificate for BYOC
revocation or strictly from storage for the other revocation types.
* With BYOC write certificate entry using dashes not the legacy colon char
* Allow tidy to backup legacy CA bundles
With the new tidy_move_legacy_ca_bundle option, we'll use tidy to move
the legacy CA bundle from /config/ca_bundle to /config/ca_bundle.bak.
This does two things:
1. Removes ca_bundle from the hot-path of initialization after initial
migration has completed. Because this entry is seal wrapped, this
may result in performance improvements.
2. Allows recovery of this value in the event of some other failure
with migration.
Notably, this cannot occur during migration in the unlikely (and largely
unsupported) case that the operator immediately downgrades to Vault
<1.11.x. Thus, we reuse issuer_safety_buffer; while potentially long,
tidy can always be run manually with a shorter buffer (and only this
flag) to manually move the bundle if necessary.
In the event of needing to recover or undo this operation, it is
sufficient to use sys/raw to read the backed up value and subsequently
write it to its old path (/config/ca_bundle).
The new entry remains seal wrapped, but otherwise isn't used within the
code and so has better performance characteristics.
Performing a fat deletion (DELETE /root) will again remove the backup
like the old legacy bundle, preserving its wipe characteristics.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation about new tidy parameter
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for migration scenarios
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Clean up time comparisons
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Correctly distinguish empty issuer names
When using client.Logical().JSONMergePatch(...) with an empty issuer
name, patch incorrectly reports:
> issuer name contained invalid characters
In this case, both the error in getIssuerName(...) is incorrect and
patch should allow setting an empty issuer name explicitly.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add cluster_aia_path templating variable
Per discussion with maxb, allow using a non-Vault distribution point
which may use an insecure transport for RFC 5280 compliance.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Address feedback from Max
Co-authored-by: Max Bowsher <maxbowsher@gmail.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: Max Bowsher <maxbowsher@gmail.com>
* Add issuer reference info on JSON endpoint
This endpoint is unauthenticated and shouldn't contain sensitive
information. However, listing the issuers (LIST /issuers) already
returns both the issuer ID and the issuer name (if any) so this
information is safe to return here.
When fetching /pki/issuer/default/json, it would be nice to know exactly
which issuer ID and name it corresponds to, without having to fetch the
authenticated endpoint as well.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
A lot of places took a (context, backend, request) tuple, ignoring the
request proper and only using it for its storage. This (modified) tuple
is exactly the set of elements in the shared storage context, so we
should be using that instead of manually passing all three elements
around.
This simplifies a few places where we'd generate a storage context at
the request level and then split it apart only to recreate it again
later (e.g., CRL building).
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
- Nick brought this to our attention, one of the PKI test suites
is overwriting the production code's value leading to a data race
issue.
- Remove the setting of the variable with the same value from the test
suite.
* Respond with data to all writes in PKI engine
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow templating of cluster-local AIA URIs
This adds a new configuration path, /config/cluster, which retains
cluster-local configuration. By extending /config/urls and its issuer
counterpart to include an enable_templating parameter, we can allow
operators to correctly identify the particular cluster a cert was
issued on, and tie its AIA information to this (cluster, issuer) pair
dynamically.
Notably, this does not solve all usage issues around AIA URIs: the CRL
and OCSP responder remain local, meaning that some merge capability is
required prior to passing it to other systems if they use CRL files and
must validate requests with certs from any arbitrary PR cluster.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation about templated AIAs
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* AIA URIs -> AIA URLs
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* issuer.AIAURIs might be nil
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow non-nil response to config/urls
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Always validate URLs on config update
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Ensure URLs lack templating parameters
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Review feedback
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Address a nil panic when writing an empty POST request to the ocsp handler
- Seems when no JSON body is sent with a POST request Vault will not
populate the HTTPRequest member variable which caused the nil panic
- vault write -force pki/ocsp
- Add a check for it and the Body member variable to be nil before use.
* Add cl
* Rename integation_test.go->integration_test.go
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add ability to fetch container's network addresses
This lets us return the on-network container address, allowing us to
spawn client containers which contact server containers.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add integration tests with nginx, curl, wget, Go
We build new integration tests, spawning a test instance on nginx and
ensuring we can connect with a variety of clients against a variety of
CA and leaf certificate types. This will ultimately let us detect issues
with compatibility as we expand the matrix of supported servers and
clients.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Make runner reference unique
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Attempt to fix CI with longer wait
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Finish moving nginx tests to pkiext package
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* make fmt
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add more debugging, work on CircleCI
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* New PKI API to generate and sign a CRL based on input data
- Add a new PKI API that allows an end-user to feed in all the
information required to generate and sign a CRL by a given issuer.
- This is pretty powerful API allowing an escape hatch for 3rd parties
to craft customized CRLs with extensions based on their individual
needs
* Add api-docs and error if reserved extension is provided as input
* Fix copy/paste error in Object Identifier constants
* Return nil on errors instead of partially filled slices
* Add cl
* wip
* Add cached OCSP client support to Cert Auth
* ->pointer
* Code cleanup
* Fix unit tests
* Use an LRU cache, and only persist up to 1000 of the most recently used values to stay under the storage entry limit
* Fix caching, add fail open mode parameter to cert auth roles
* reduce logging
* Add the retry client and GET then POST logic
* Drop persisted cache, make cache size configurable, allow for parallel testing of multiple servers
* dead code
* Update builtin/credential/cert/path_certs.go
Co-authored-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Hook invalidate to reinit the ocsp cache size
* locking
* Conditionally init the ocsp client
* Remove cache size config from cert configs, it's a backend global
* Add field
* Remove strangely complex validity logic
* Address more feedback
* Rework error returning logic
* More edge cases
* MORE edge cases
* Add a test matrix with a builtin responder
* changelog
* Use an atomic for configUpdated
* Actually use ocsp_enabled, and bind to a random port for testing
* Update builtin/credential/cert/path_login.go
Co-authored-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor unit tests
* Add status to cache
* Make some functions private
* Rename for testing, and attribute
* Up to date gofumpt
* remove hash from key, and disable the vault dependent unit test
* Comment out TestMultiOCSP
* imports
* more imports
* Address semgrep results
* Attempt to pass some sort of logging to test_responder
* fix overzealous search&replace
Co-authored-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add new PKI api to combine and sign different CRLs from the same issuer
- Add a new PKI api /issuer/<issuer ref>/resign-crls that will allow
combining and signing different CRLs that were signed by the same
issuer.
- This allows external actors to combine CRLs into a single CRL across
different Vault clusters that share the CA certificate and key material
such as performance replica clusters and the primary cluster
* Update API docs
* PR Feedback - Delta CRL rename
* Update to latest version of main
* PR Feedback - Get rid of the new caEntry struct
* Address PR feedback in api-docs and PEM encoded response
* Export CreateBackendWithStorage for pkiext
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Move zlint_test.go to pkiext
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix mount all test to ignore pkiext
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Credit to Steve for finding this one.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add automatic tidy of expired issuers
To aid PKI users like Consul, which periodically rotate intermediates,
and provided a little more consistency with older versions of Vault
which would silently (and dangerously!) replace the configured CA on
root/intermediate generation, we introduce an automatic tidy of expired
issuers.
This includes a longer safety buffer (1 year) and logging of the
relevant issuer information prior to deletion (certificate contents, key
ID, and issuer ID/name) to allow admins to recover this value if
desired, or perform further cleanup of keys.
From my PoV, removal of the issuer is thus a relatively safe operation
compared to keys (which I do not feel comfortable removing) as they can
always be re-imported if desired. Additionally, this is an opt-in tidy
operation, not enabled by default. Lastly, most major performance
penalties comes with lots of issuers within the mount, not as much
large numbers of keys (as only new issuer creation/import operations are
affected, unlike LIST /issuers which is a public, unauthenticated
endpoint).
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test for tidy
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add docs on tidy of issuers
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Restructure logging
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add missing fields to expected tidy output
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Also remove one duplicate error masked by return.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Correctly preserve other issuer config params
When setting a new default issuer, our helper function would overwrite
other parameters in the issuer configuration entry. However, up until
now, there were none.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add new parameter to allow default to follow new
This parameter will allow operators to have the default issuer
automatically update when a new root is generated or a single issuer
with a key (potentially with others lacking key) is imported.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Storage migration tests fail on new members
These internal members shouldn't be tested by the storage migration
code, and so should be elided from the test results.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Follow new issuer on root generation, import
This updates the two places where issuers can be created (outside of
legacy CA bundle migration which already sets the default) to follow
newly created issuers when the config is set.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test for new default-following behavior
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add new API to PKI to list revoked certificates
- A new API that will return the list of serial numbers of
revoked certificates on the local cluster.
* Add cl
* PR feedback
* Ensure correct write ordering in rebuildIssuersChains
When troubleshooting a recent migration failure from 1.10->1.11, it was
noted that some PKI mounts had bad chain construction despite having
valid, chaining issuers. Due to the cluster's leadership trashing
between nodes, the migration logic was re-executed several times,
partially succeeding each time. While the legacy CA bundle migration
logic was written with this in mind, one shortcoming in the chain
building code lead us to truncate the ca_chain: by sorting the list of
issuers after including non-written issuers (with random IDs), these
issuers would occasionally be persisted prior to storage _prior_ to
existing CAs with modified chains.
The migration code carefully imported the active issuer prior to its
parents. However, due to this bug, there was a chance that, if write to
the pending parent succeeded but updating the active issuer didn't, the
active issuer's ca_chain field would only contain the self-reference and
not the parent's reference as well. Ultimately, a workaround of setting
and subsequently unsetting a manual chain would force a chain
regeneration.
In this patch, we simply fix the write ordering: because we need to
ensure a stable chain sorting, we leave the sort location in the same
place, but delay writing the provided referenceCert to the last
position. This is because the reference is meant to be the user-facing
action: without transactional write capabilities, other chains may
succeed, but if the last user-facing action fails, the user will
hopefully retry the action. This will also correct migration, by
ensuring the subsequent issuer import will be attempted again,
triggering another chain build and only persisting this issuer when
all other issuers have also been updated.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Remigrate ca_chains to fix any missing issuers
In the previous commit, we identified an issue that would occur on
legacy issuer migration to the new storage format. This is easy enough
to detect for any given mount (by an operator), but automating scanning
and remediating all PKI mounts in large deployments might be difficult.
Write a new storage migration version to regenerate all chains on
upgrade, once.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add issue to PKI considerations documentation
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Correct %v -> %w in chain building errs
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
When running the test suite in CI (where requests are centralized from
relatively few IPs), we'd occasionally hit Dockerhub's rate limits.
Luckily Hashicorp runs a (limited) public mirror of the containers we
need, so we can switch to them here in the tests.
For consistency between developer and CI, we've opted to have the tests
always pull from the Hashicorp mirror, rather than updating the CI
runner to prefer the mirror.
We exclude nomad and influxdb as we don't presently mirror these repos.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Return revocation info within existing certs/<serial> api
- The api already returned both the certificate and a revocation_time
field populated. Update the api to return revocation_time_rfc3339
as we do elsewhere and also the issuer id if it was revoked.
- This will allow callers to associate a revoked cert with an issuer
* Add cl
* PR feedback (docs update)
* Bump validity period check to satisfy CircleCI
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update builtin/logical/pki/backend_test.go
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add regression test for default CRL expiry
Also fixes a bug w.r.t. upgrading older entries and missing the Delta
Rebuild Interval field, setting it to the default.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog for earlier PR
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
