* Fix for PKI.TestStandby_Operations test to work in ENT
- Remove wait call to testhelpers.WaitForActiveNodeAndStandbys and
leverage testhelpers.WaitForStandbyNode instead.
* Use InmemBackendSetup for a proper HA backend in ENT
* Forward PKI revocation requests received by standby nodes to active node
- A refactoring that occurred in 1.13 timeframe removed what was
considered a specific check for standby nodes that wasn't required
as a writes should be returning ErrReadOnly.
- That sadly exposed a long standing bug where the errors from the
storage layer were not being properly wrapped, hiding the ErrReadOnly
coming from a write and failing the request.
* Add cl
* Add test for basic PKI operations against standby nodes
* Telemetry Metrics Configuration.
* Err Shadowing Fix (woah, semgrep is cool).
* Fix TestBackend_RevokePlusTidy_Intermediate
* Add Changelog.
* Fix memory leak. Code cleanup as suggested by Steve.
* Turn off metrics by default, breaking-change.
* Show on tidy-status before start-up.
* Fix tests
* make fmt
* Add emit metrics to periodicFunc
* Test not delivering unavailable metrics + fix.
* Better error message.
* Fixing the false-error bug.
* make fmt.
* Try to fix race issue, remove confusing comments.
* Switch metric counter variables to an atomic.Uint32
- Switch the metric counter variables to an atomic variable type
so that we are forced to properly load/store values to it
* Fix race-issue better by trying until the metric is sunk.
* make fmt.
* empty commit to retrigger non-race tests that all pass locally
---------
Co-authored-by: Steve Clark <steven.clark@hashicorp.com>
* 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.
* 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>
* 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>
* 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>
* 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>
- 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>
* 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>
* 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>
* 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
* 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>
* Fix for duplicate SANs in signed certificates when othernames are present in the CSR SAN extension and UseCSRValues is true.
When UseCSRValues is true (as is the case on the sign-verbatim endpoint), all extensions including Subject Alternative Names are copied from the CSR to the final certificate.
If the Subject Alternative Name in question contains any othernames (such as a Microsoft UPN) the SAN extension is added again as a workaround for an encoding issue (in function HandleOtherSANs).
Having duplicate x509v3 extensions is invalid and is rejected by openssl on Ubuntu 20.04, and also by Go since https://github.com/golang/go/issues/50988 (including in Go 1.19).
In this fix I do not add the extension from the CSR if it will be added during HandleOtherSANs.
* Added unittest and changelog entry.
* Fix RevocationSigAlg provisioning in GCP
GCP restricts keys to a certain type of signature, including hash
algorithm, so we must provision our RevocationSigAlg from the root
itself unconditionally in order for GCP to work.
This does change the default, but only for newly created certificates.
Additionally, we clarify that CRL building is not fatal to the import
process.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add inverse mapping for SignatureAlgorithm
By default we'd use .String() on x509.SignatureAlgorithm, but this
doesn't round-trip. Switch to a custom map that is round-trippable
and matches the constant name as there is no other way to get this info
presently.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test to ensure root creation sets rev_sig_alg
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Test round-tripping of SigAlgoNames, InvSigAlgoNames
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix failing Default Update test
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* PKI: Add support for signature_bits param to the intermediate/generate api
- Mainly to work properly with GCP backed managed keys, we need to
issue signatures that would match the GCP key algorithm.
- At this time due to https://github.com/golang/go/issues/45990 we
can't issue PSS signed CSRs, as the libraries in Go always request
a PKCS1v15.
- Add an extra check in intermediate/generate that validates the CSR's
signature before providing it back to the client in case we generated
a bad signature such as if an end-user used a GCP backed managed key
with a RSA PSS algorithm.
- GCP ignores the requested signature type and always signs with the
key's algorithm which can lead to a CSR that says it is signed with
a PKCS1v15 algorithm but is actually a RSA PSS signature
* Add cl
* PR feedback
* Fix interoperability concerns with PSS
When Go parses a certificate with rsaPSS OID, it will accept this
certificate but not parse the SubjectPublicKeyInfo, leaving the
PublicKeyAlgorithm and PublicKey fields blank, but otherwise not erring.
The same behavior occurs with rsaPSS OID CSRs.
On the other hand, when Go parses rsaPSS OID PKCS8 private keys, these
keys will fail to parse completely.
Thus, detect and fail on any empty PublicKey certs and CSRs, warning the
user that we cannot parse these correctly and thus refuse to operate.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Run more PKI tests in parallel
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add notes about PSS shortcomings to considerations
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Basics of Cert-Count Telemetry, changelog, "best attempt" slice to capture (and test for) duplicates, Move sorting of possibleDoubleCountedRevokedSerials to after compare of entries. Add values to counter when still initializing.
Set lists to nil after use, Fix atomic2 import, Delay reporting metrics until after deduplication has completed,
The test works now, Move string slice to helper function; Add backendUUID to gauge name.
* Add path to manually rebuild delta CRLs
The crl/rotate-delta path behaves like crl/rotate, triggering a
cluster-local rebuild of just the delta CRL. This is useful for when
delta CRLs are enabled with a longer-than-desired auto-rebuild period
after some high-profile revocations occur.
In the event delta CRLs are not enabled, this becomes a no-op.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for Delta CRL rebuilding
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update documentation about Delta CRLs
Also fixes a omission in the If-Modified-Since docs to mention that the
response header should probably also be passed through.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow tidy operations to be cancelled
When tidy operations take a long time to execute (and especially when
executing them automatically), having the ability to cancel them becomes
useful to reduce strain on Vault clusters (and let them be rescheduled
at a later time).
To this end, we add the /tidy-cancel write endpoint.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add missing auto-tidy synopsis / description
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add a pause duration between tidying certificates
By setting pause_duration, operators can have a little control over the
resource utilization of a tidy operation. While the list of certificates
remain in memory throughout the entire operation, a pause is added
between processing certificates and the revocation lock is released.
This allows other operations to occur during this gap and potentially
allows the tidy operation to consume less resources per unit of time
(due to the sleep -- though obviously consumes the same resources over
the time of the operation).
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for cancellation, pause
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add API docs on pause_duration, /tidy-cancel
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add lock releasing around tidy pause
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Reset cancel guard, return errors
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add remove_roots_from_chain flag to sign and issue pki apis
- Add a new flag to allow end-users to control if we return the
root/self-signed CA certificate within the list of certificates in
ca_chain field on issue and sign api calls.
* Add cl
* PR feedback
* Issuer renames should invalidate CRL cache times
When an issuer is renamed (or rather, two issuers' names are swapped in
quick succession), this is akin to the earlier identified default issuer
update condition. So, when any issuer is updated, go ahead and trigger
the invalidation logic.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix handling of delta CRL If-Modified-Since
The If-Modified-Since PR was proposed prior to the Delta CRL changes and
thus didn't take it into account. This follow-up commit fixes that,
addressing If-Modified-Since semantics for delta CRL fetching and
ensuring an accurate number is stored.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* honor header if-modified-since if present
* pathGetIssuerCRL first version
* check if modified since for CA endpoints
* fix date comparison for CA endpoints
* suggested changes and refactoring
* add writeIssuer to updateDefaultIssuerId and fix error
* Move methods out of storage.go into util.go
For the most part, these take a SC as param, but aren't directly storage
relevant operations. Move them out of storage.go as a result.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Use UTC timezone for storage
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Rework path_fetch for better if-modified-since handling
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Invalidate all issuers, CRLs on default write
When the default is updated, access under earlier timestamps will not
work as we're unclear if the timestamp is for this issuer or a previous
issuer. Thus, we need to invalidate the CRL and both issuers involved
(previous, next) by updating their LastModifiedTimes.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for If-Modified-Since
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Correctly invalidate default issuer changes
When the default issuer changes, we'll have to mark the invalidation on
PR secondary clusters, so they know to update their CRL mapping as well.
The swapped issuers will have an updated modification time (which will
eventually replicate down and thus be correct), but the CRL modification
time is cluster-local information and thus won't be replicated.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* make fmt
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor sendNotModifiedResponseIfNecessary
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation on if-modified-since
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>
* Refactor tidy steps into two separate helpers
This refactors the tidy go routine into two separate helpers, making it
clear where the boundaries of each are: variables are passed into these
method and concerns are separated. As more operations are rolled into
tidy, we can continue adding more helpers as appropriate. Additionally,
as we move to make auto-tidy occur, we can use these as points to hook
into periodic tidying.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor revInfo checking to helper
This allows us to validate whether or not a revInfo entry contains a
presently valid issuer, from the existing mapping. Coupled with the
changeset to identify the issuer on revocation, we can begin adding
capabilities to tidy to update this association, decreasing CRL build
time and increasing the performance of OCSP.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor issuer fetching for revocation purposes
Revocation needs to gracefully handle using the old legacy cert bundle,
so fetching issuers (and parsing them) needs to be done slightly
differently than other places. Refactor this from revokeCert into a
common helper that can be used by tidy.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow tidy to associate revoked certs, issuers
When revoking a certificate, we need to associate the issuer that signed
its certificate back to the revInfo entry. Historically this was
performed during CRL building (and still remains so), but when running
without CRL building and with only OCSP, performance will degrade as the
issuer needs to be found each time.
Instead, allow the tidy operation to take over this role, allowing us to
increase the performance of OCSP and CRL in this scenario, by decoupling
issuer identification from CRL building in the ideal case.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for tidy updates
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation on new tidy parameter, metrics
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor tidy config into shared struct
Finish adding metrics, status messages about new tidy operation.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow correct importing of certs without CRL KU
When Vault imports certificates without KU for CRLSign, we shouldn't
provision CRLUsage on the backing issuer; otherwise, we'll attempt to
build CRLs and Go will cause us to err out. This change makes it clear
(at issuer configuration time) that we can't possibly support this
operation and hopefully prevent users from running into the more cryptic
Go error.
Note that this does not apply for OCSP EKU: the EKU exists, per RFC 6960
Section 2.6 OCSP Signature Authority Delegation, to allow delegation of
OCSP signing to a child certificate. This EKU is not necessary on the
issuer itself, and generally assumes issuers are allowed to issue OCSP
responses regardless of KU/EKU.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add docs to clarify issue with import, CRL usage
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update website/content/api-docs/secret/pki.mdx
* Add additional test assertion
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add per-issuer AIA URI information
Per discussion on GitHub with @maxb, this allows issuers to have their
own copy of AIA URIs. Because each issuer has its own URLs (for CA and
CRL access), its necessary to mint their issued certs pointing to the
correct issuer and not to the global default issuer. For anyone using
multiple issuers within a mount, this change allows the issuer to point
back to itself via leaf's AIA info.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation on per-issuer AIA info
Also add it to the considerations page as something to watch out for.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for per-issuer AIA information
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor AIA setting on the issuer
This introduces a common helper per Steve's suggestion.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Clarify error messages w.r.t. AIA naming
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Clarify error messages regarding AIA URLs
This clarifies which request parameter the invalid URL is contained
in, disambiguating the sometimes ambiguous usage of AIA, per suggestion
by Max.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Rename getURLs -> getGlobalAIAURLs
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Correct AIA acronym expansion word orders
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix bad comment suggesting re-generating roots
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add two entries to URL tests
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Ignore EC PARAMETER blocks during issuer import
While older versions of Vault supported sending this, we broke such
support in 1.11. Ignore them from the manage issuers endpoint (which is
aliased to the old /config/ca path) -- but keep erring in the import
keys paths. The latter is a new endpoint not aliased to anything and
only expects a single PEM block.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add regression test for EC PARAMs during import
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor serial creation to common helper
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add BYOC revocation to PKI mount
This allows operators to revoke certificates via a PEM blob passed to
Vault. In particular, Vault verifies the signature on the certificate
from an existing issuer within the mount, ensuring that one indeed
issued this certificate. The certificate is then added to storage and
its serial submitted for revocation.
This allows certificates generated with no_store=true to be submitted
for revocation afterwards, given a full copy of the certificate. As a
consequence, all roles can now safely move to no_store=true (if desired
for performance) and revocation can be done on a case-by-case basis.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add docs on BYOC revocation
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add PEM length check to BYOC import
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for BYOC
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Guard against legacy CA bundle usage
This prevents usage of the BYOC cert on a hybrid 1.10/1.12 cluster with
an non-upgraded CA issuer bundle.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add PSS signature support to Vault PKI engine
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Use issuer's RevocationSigAlg for CRL signing
We introduce a new parameter on issuers, revocation_signature_algorithm
to control the signature algorithm used during CRL signing. This is
because the SignatureAlgorithm value from the certificate itself is
incorrect for this purpose: a RSA root could sign an ECDSA intermediate
with say, SHA256WithRSA, but when the intermediate goes to sign a CRL,
it must use ECDSAWithSHA256 or equivalent instead of SHA256WithRSA. When
coupled with support for PSS-only keys, allowing the user to set the
signature algorithm value as desired seems like the best approach.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add use_pss, revocation_signature_algorithm docs
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add PSS to signature role issuance test matrix
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow roots to self-identify revocation alg
When using PSS support with a managed key, sometimes the underlying
device will not support PKCS#1v1.5 signatures. This results in CRL
building failing, unless we update the entry's signature algorithm
prior to building the CRL for the new root.
With a RSA-type key and use_pss=true, we use the signature bits value to
decide which hash function to use for PSS support.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add clearer error message on failed import
When CRL building fails during cert/key import, due to PSS failures,
give a better indication to the user that import succeeded its just CRL
building that failed. This tells them the parameter to adjust on the
issuer and warns that CRL building will fail until this is fixed.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add case insensitive SigAlgo matching
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Convert UsePSS back to regular bool
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor PSS->certTemplate into helper function
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Proper string output on rev_sig_alg display
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Copy root's SignatureAlgorithm for CRL building
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
