* 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>
* Add tests for zlint-clean CA building
This test ensures that we can consistently pass ZLint's CA linting
tests on a root certificate generated by Vault. In particular, nominal
requirements are placed on the structure on the issuer's Subject, which
we supply, and the remaining requirements pass.
The one exception is we include both RFC and CA/BF BR lints in the
default zlint checks; this means ECDSA P-521 (which isn't accepted by
Mozilla's root store policies) is rejected, so we ignore to lints
related to that.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add options to copy to/from container, fix stopping
Stopping the container takes a bit of time for some unknown reason so
I've instead opted to shorten the sleep in the zlint tests to avoid
consuming resources too long after the test finish.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Make zlint tests execute in parallel
This improves the overall test time of the zlint tests, making the
container build up front once (provisioning zlint), and then copying the
cert into the new container image later.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* make fmt
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix tidy-status, tidy-cancel on PR Secondaries
PKI's tidy-status included a bug that prevented PR secondary nodes from
responding with the status of the running tidy operation: while the
operation constructor correctly forwarded the node on PR standby
instances, the handler itself forwarded also on PR secondary nodes.
This is incorrect as the PR secondary nodes are the active node in the
local PR cluster, and run tidy operations otherwise.
This meant that while auto-tidy and tidy operations would run, there was
no insight into the process.
When implementing tidy-cancel, tidy-status's handler logic was reused,
duplicating the bug there 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>
When revoking an issuer, we immediately force a full rebuild of all CRLs
(complete and delta). However, we had forgotten to guard the delta CRL's
inclusion of augmented issuers, resulting in double-listing the issuer's
serial number on both the complete and the delta CRL. This isn't
necessary as the delta's referenced complete CRL number has incremented
to the point where the issuer itself was included on the complete CRL.
Avoid this double reference and don't include issuers on delta CRLs;
they should always appear only on the complete CRL.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
- Add some unit tests around the OCSP response validation that we
are using the proper signature algorithms.
- Add in test cases as well to validate SHA384 and SHA512 requested hash support
* 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>
* Allow OCSP to use issuer's RevocationSigAlgo
When an issuer specifies a RevocationSigAlgo, we should largely follow
this for both CRLs and OCSP. However, x/crypto/ocsp lacks support for
PSS signatures, so we drop these down to PKCS#1v1.5 instead.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add warning when issuer has PSS-based RevSigAlgo
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add note about OCSP and PSS support
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
* PKI: Do not load revoked certificates if CRL has been disabled
- Restore the prior behavior of not reading in all revoked certificates
if the CRL has been disabled as there might be performance issues
if a customer had or is still revoking a lot of certificates.
* Add cl
When adding delta CRL support, we unconditionally added the delta
indicator extension to the main CRL. We shouldn't have done this, and
instead only added it conditionally when we were building delta CRLs.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* 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.
* Don't race for CRL rebuilding capability check
Core has recently seen some data races during SystemView/replication
updates between them and the PKI subsystem. This is because this
SystemView access occurs outside of a request (during invalidation
handling) and thus the proper lock isn't held.
Because replication status cannot change within the lifetime of a plugin
(and instead, if a node switches replication status, the entire plugin
instance will be torn down and recreated), it is safe to cache this
once, at plugin startup, and use it throughout its lifetime.
Thus, we replace this SystemView access with a stored boolean variable
computed ahead of time.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update builtin/logical/pki/backend.go
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This option was elided from the default value for the usage field. This
results in issuers "losing" ocsp-signing when they're POST updated. Most
issuers will want OCSP signing by default, so it makes sense to add this
as the default.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* 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
We switch these fields to use the explicit default value (computing the
time in seconds appropriately).
As reported by @beornf, thanks!
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add ability to perform automatic tidy operations
This enables the PKI secrets engine to allow tidy to be started
periodically by the engine itself, avoiding the need for interaction.
This operation is disabled by default (to avoid load on clusters which
don't need tidy to be run) but can be enabled.
In particular, a default tidy configuration is written (via
/config/auto-tidy) which mirrors the options passed to /tidy. Two
additional parameters, enabled and interval, are accepted, allowing
auto-tidy to be enabled or disabled and controlling the interval
(between successful tidy runs) to attempt auto-tidy.
Notably, a manual execution of tidy will delay additional auto-tidy
operations. Status is reported via the existing /tidy-status endpoint.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation on auto-tidy
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for auto-tidy
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Prevent race during parallel testing
We modified the RollbackManager's execution window to allow more
faithful testing of the periodicFunc. However, the TestAutoRebuild and
the new TestAutoTidy would then race against each other for modifying
the period and creating their clusters (before resetting to the old
value).
This changeset adds a lock around this, preventing the races.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Use tidyStatusLock to gate lastTidy time
This prevents a data race between the periodic func and the execution of
the running tidy.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add read lock around tidyStatus gauges
When reading from tidyStatus for computing gauges, since the underlying
values aren't atomics, we really should be gating these with a read lock
around the status access.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* 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>
* Allow generation of up-to-date delta CRLs
While switching to periodic rebuilds of CRLs alleviates the constant
rebuild pressure on Vault during times of high revocation, the CRL
proper becomes stale. One response to this is to switch to OCSP, but not
every system has support for this. Additionally, OCSP usually requires
connectivity and isn't used to augment a pre-distributed CRL (and is
instead used independently).
By generating delta CRLs containing only new revocations, an existing
CRL can be supplemented with newer revocations without requiring Vault
to rebuild all complete CRLs. Admins can periodically fetch the delta
CRL and add it to the existing CRL and applications should be able to
support using serials from both.
Because delta CRLs are emptied when the next complete CRL is rebuilt, it
is important that applications fetch the delta CRL and correlate it to
their complete CRL; if their complete CRL is older than the delta CRL's
extension number, applications MUST fetch the newer complete CRL to
ensure they have a correct combination.
This modifies the revocation process and adds several new configuration
options, controlling whether Delta CRLs are enabled and when we'll
rebuild it.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for delta CRLs
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation on delta CRLs
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Address review feedback: fix several bugs
Thanks Steve!
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Correctly invoke periodic func on active nodes
We need to ensure we read the updated config (in case of OCSP request
handling on standby nodes), but otherwise want to avoid CRL/DeltaCRL
re-building.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-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>
* Add ocsp_expiry configuration field to PKI crl config
- Add a new configurable duration field to the crl configuration to
allow operator control of how long an OCSP response can be cached
for.
- This is useful for how long a server like NGINX/Apache is
allowed to cache the response for OCSP stapling.
- A value of 0 means no one should cache the response.
- Address an issue discovered that we did not upgrade existing crl
configurations properly
* PR feedback
* Refactor CRL tests to use /sys/mounts
Thanks Steve for the approach! This also address nits from Kit.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Skip CRL building steps when disabled
This skips a number of steps during CRL build when it is disabled (and
forceNew is not set). In particular, we avoid fetching issuers, we avoid
associating issuers with revocation entries (and building that in-memory
mapping), making CRL building more efficient.
This means that there'll again be very little overhead on clusters with
the CRL disabled.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Prevent revoking roots from appearing on own CRLs
This change ensures that when marking a root as revoked, it no longer
appears on its own CRL. Very few clients support this event (as
generally only leaves/intermediates are checked for presence on a
parent's CRL) and it is technically undefined behavior (if the root is
revoked, its own CRL should be untrusted and thus including it on its
own CRL isn't a safe/correct distribution channel).
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Ensure stability of revInfo issuer identification
As mentioned by Kit, iterating through each revInfoEntry and associating
the first issuer which matches it can cause churn when many (equivalent)
issuers are in the system and issuers come and go (via CRLSigning usage,
which has been modified in this release as well). Because we'd not
include issuers without CRLSigning usage, we'd cause our verification
helper, isRevInfoIssuerValid, to think the issuer ID is no longer value
(when instead, it just lacks crlSigning bits).
We address this by pulling in all issuers we know of for the
identification. This allows us to keep valid-but-not-for-signing
issuers, and use other representatives of their identity set for
signing/building the CRL (if they are enabled for such usage).
As a side effect, we now no longer place these entries on the default
CRL in the event all issuers in the CRL set are without the usage.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
This is only for the last commit.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Identify issuer on revocation
When we attempt to revoke a leaf certificate, we already parse all of
the issuers within the mount (to x509.Certificate) to ensure we don't
accidentally revoke an issuer via the leaf revocation endpoint. We can
reuse this information to associate the issuer (via issuer/subject
comparison and signature checking) to the revoked cert in its revocation
info. This will help OCSP, avoiding the case where the OCSP handler
needs to associate a certificate to its issuer.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test to ensure issuers are identified
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>
* Handle multiple matching issuers in OCSP requests
- Select the first issuer that matches our request hashes and has
the OCSP signing usage enabled. This might not match the exact
issuer id that issued the certificate but the signatures will be
okay.
* PR feedback
Previously we used the global backend-set crlLifetime as a default
value. However, this was refactored into a new defaultCrlConfig instead,
which we should reply with when the CRL configuration has not been set
yet. In particular, the 72h default expiry (and new 12h auto-rebuild
grace period) was added and made explicit.
This fixes the broken UI test.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow automatic rebuilding of CRLs
When enabled, periodic rebuilding of CRLs will improve PKI mounts in two
way:
1. Reduced load during periods of high (new) revocations, as the CRL
isn't rebuilt after each revocation but instead on a fixed schedule.
2. Ensuring the CRL is never stale as long as the cluster remains up,
by checking for next CRL expiry and regenerating CRLs before that
happens. This may increase cluster load when operators have large
CRLs that they'd prefer to let go stale, rather than regenerating
fresh copies.
In particular, we set a grace period before expiration of CRLs where,
when the periodic function triggers (about once a minute), we check
upcoming CRL expirations and check if we need to rebuild the CRLs.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation on periodic rebuilding
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow modification of rollback period for testing
When testing backends that use the periodic func, and specifically,
testing the behavior of that periodic func, waiting for the usual 1m
interval can lead to excessively long test execution. By switching to a
shorter period--strictly for testing--we can make these tests execute
faster.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for auto-rebuilding of CRLs
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Remove non-updating getConfig variant
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Avoid double reload of config
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor existing CRL function to storage getRevocationConfig
* Introduce ocsp_disable config option in config/crl
* Introduce OCSPSigning usage flag on issuer
* Add ocsp-request passthrough within lower layers of Vault
* Add OCSP responder to Vault PKI
* Add API documentation for OCSP
* Add cl
* Revert PKI storage migration modifications for OCSP
* Smaller PR feedback items
- pki.mdx doc update
- parens around logical.go comment to indicate DER encoded request is
related to OCSP and not the snapshots
- Use AllIssuers instead of writing them all out
- Drop zero initialization of crl config's Disable flag if not present
- Upgrade issuer on the fly instead of an initial migration
* Additional clean up backing out the writeRevocationConfig refactoring
* Remove Dirty issuer flag and update comment about not writing upgrade to
storage
* Address PR feedback and return Unknown response when mismatching issuer
* make fmt
* PR Feedback.
* More PR feedback
- Leverage ocsp response constant
- Remove duplicate errors regarding unknown issuers
* Migrate existing PKI mounts that only contains a key
- We missed testing a use-case of the migration that someone has a PKI
mount point that generated a CSR but never called set-signed back on
that mount point so it only contains a key.
* Add cl
* 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>
* Allow marking issuers as revoked
This allows PKI's issuers to be considered revoked and appear on each
others' CRLs. We disable issuance (via removing the usage) and prohibit
modifying the usage via the regular issuer management interface.
A separate endpoint is necessary because issuers (especially if signed
by a third-party CA using incremental serial numbers) might share a
serial number (e.g., an intermediate under cross-signing might share the
same number as an external root or an unrelated intermediate).
When the next CRL rebuild happens, this issuer will then appear on
others issuers CRLs, if they validate this issuer's certificate.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation on revoking issuers
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for issuer revocation semantics
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Notate that CRLs will be rebuilt
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix timestamp field from _utc -> to _rfc3339
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Ensure serial-based accesses shows as revoked
Thanks Kit!
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add warning when revoking default issuer
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow Proof of Possession based revocation
Revocation by proof of possession ensures that we have a private key
matching the (provided or stored) certificate. This allows callers to
revoke certificate they own (as proven by holding the corresponding
private key), without having an admin create innumerable ACLs around
the serial_number parameter for every issuance/user.
We base this on Go TLS stack's verification of certificate<->key
matching, but extend it where applicable to ensure curves match, the
private key is indeed valid, and has the same structure as the
corresponding public key from the certificate.
This endpoint currently is authenticated, allowing operators to disable
the endpoint if it isn't desirable to use, via ACL policies.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Clarify error message on ParseDERKey
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Leave revoke-with-key authenticated
After some discussion, given the potential for DoS (via submitting a lot
of keys/certs to validate, including invalid pairs), it seems best to
leave this as an authenticated endpoint. Presently in Vault, there's no
way to have an authenticated-but-unauthorized path (i.e., one which
bypasses ACL controls), so it is recommended (but not enforced) to make
this endpoint generally available by permissive ACL policies.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add API documentation on PoP
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add acceptance tests for Proof of Possession
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Exercise negative cases in PoP 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>
This option is known to cause problems with large numbers of issued
certificates. Ensure admins are warned about the impact of this field
and encourage them to disable it.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
strings.ReplaceAll(s, old, new) is a wrapper function for
strings.Replace(s, old, new, -1). But strings.ReplaceAll is more
readable and removes the hardcoded -1.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.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>
* Allow old certs to be cross-signed
In Vault 1.11, we introduced cross-signing support, but the earlier SKID
field change in Vault 1.10 causes problems: notably, certs created on
older versions of Vault (<=1.9) or outside of Vault (with a different
SKID method) cannot be cross-signed and validated in OpenSSL.
In particular, OpenSSL appears to be unique in requiring a SKID/AKID
match for chain building. If AKID and SKID are present on an otherwise
valid client/parent cert pair and the values are different, OpenSSL will
not build a valid path over those two, whereas most other chain
validation implementations will.
Regardless, to have proper cross-signing support, we really aught to
support copying an SKID. This adds such support to the sign-intermediate
endpoint. Support for the /issue endpoint is not added, as cross-signing
leaf certs isn't generally useful and can accept random SKIDs.
Resolves: #16461
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Address review feedback, fix tests
Also adds a known-answer test using LE R3 CA's SKID.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Address review feedback regarding separators
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This decreases the total time to run the test suite significantly. From
the last PR, we were at 151s:
> [cipherboy@xps15 pki]$ go test -count=1 github.com/hashicorp/vault/builtin/logical/pki
> ok github.com/hashicorp/vault/builtin/logical/pki 151.182s
Now we're around 60s:
> [cipherboy@xps15 pki]$ go test -count=1 github.com/hashicorp/vault/builtin/logical/pki
> ok github.com/hashicorp/vault/builtin/logical/pki 61.838s
Notably, Go will correctly handle parallelizing tests across both
packages and within a package, so this shouldn't really impact test
runners (if they're already saturated).
The only gotcha in this approach is that the call to t.Run(...) becomes
effectively async; this means we either need to not mark the test as
parallel or shadow any loop variables inside the scope of the loop to
allow the t.Run to have the correct copy.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This tells the user that the next step should be to configure AIA URLs
on this newly imported issuer/mount point. Ideally this should occur
before any leaves are issued such that they have the correct
information.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* pki: When a role sets key_type to any ignore key_bits value when signing
- Bypass the validation for the role's key_bits value when signing CSRs
if the key_type is set to any. We still validate the key is at least
2048 for RSA backed CSRs as we did in 1.9.x and lower.
structs and mapstructure aren't really used within Vault much any more,
so we should start removing them. Luckily there was only one externally
accessible place where structs was used (AIA URLs config) so that was
easy to remove. The rest is mostly structure tag changes.
path_roles_tests.go relied on mapstructure in some places that broke,
but otherwise backend_test.go hasn't yet been modified to remove the
dependency on mapstructure. These didn't break as the underlying
CertBundle didn't get mapstructure support removed (as its in the SDK).
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This will allow us to refactor the storage functions to take additional
parameters (or backend-inferred values) in the future. In particular, as
we look towards adding a storage cache layer, we'll need to add this to
the backend, which is now accessible from all storage functions.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
When tidy is called without arguments, we kick off a tidy operation with
no targets. This results in nothing being done, though the user might
reasonably expect some results.
Throw a warning in this case, so the user knows not to expect anything.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Return errors on short PEM bundles (keys, issuers)
When users pass the path of the bundle to the API, rather than the
contents of the bundle (say, by omitting the `@` symbol on a Vault CLI
request), give a better error message indicating to the user what the
potential problem might be. While a larger bound for certificates was
given (75 bytes, likely 100 would be fine as well), a smaller bound had
to be chosen for keys as there's less standard DER encoding data around
them.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add signature_bits to sign-intermediate
This endpoint was lacking the signature_bits field like all the other
endpoints. Notably, in #15478, the ability to customize the intermediate
CSR's signature bits was removed without checking for the ability to
customize the final (root-signed) intermediate certificate's value.
This adds in that missing ability, bringing us parity with root
generation and role-based signing.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add signature_bits to sign-verbatim
This endpoint was also lacking the signature_bits field, preventing
other signature hash functions from being utilized here.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test for revocation under intermediate CA
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow revocation of certs with key-less issuers
In Vault 1.11's multiple issuer functionality, we incorrectly fetched
the full CA signing bundle for validating revocation of leaf certs (when
attempting to prohibit revocation of issuers in the mount). When the
issuer lacked a key (such as the root issuer on an intermediate mount),
this signing bundle creation failed.
Instead of fetching the full CA signing bundle, fetch instead the raw
certutil.CertBundle and parse it (to x509.Certificate form) ourselves.
This manifests as the error on revocation:
> URL: PUT http://127.0.0.1:8200/v1/pki_int/revoke
> * could not fetch the CA certificate for issuer id 156e1b99-4f04-5b5e-0036-cc0422c0c0d3: unable to fetch corresponding key for issuer 156e1b99-4f04-5b5e-0036-cc0422c0c0d3; unable to use this issuer for signing
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add cn_validations PKI Role parameter
This new parameter allows disabling all validations on a common name,
enabled by default on sign-verbatim and issuer generation options.
Presently, the default behavior is to allow either an email address
(denoted with an @ in the name) or a hostname to pass validation.
Operators can restrict roles to just a single option (e.g., for email
certs, limit CNs to have strictly email addresses and not hostnames).
By setting the value to `disabled`, CNs of other formats can be accepted
without validating their contents against our minimal correctness checks
for email/hostname/wildcard that we typically apply even when broad
permissions (allow_any_name=true, enforce_hostnames=false, and
allow_wildcard_certificates=true) are granted on the role.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update PKI tests for cn_validation support
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add PKI API documentation on cn_validations
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor role issuance tests to use direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 5.879s
After:
github.com/hashicorp/vault/builtin/logical/pki 1.063s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor role key bit tests to use direct backend
Also removes redundant cases.
Before:
github.com/hashicorp/vault/builtin/logical/pki 136.605s
After:
github.com/hashicorp/vault/builtin/logical/pki 24.713s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor common name test to use direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 4.767s
After:
github.com/hashicorp/vault/builtin/logical/pki 0.611s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor device cert tests to use direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 4.725s
After:
github.com/hashicorp/vault/builtin/logical/pki 0.402s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor invalid parameter test to use direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 3.777s
After:
github.com/hashicorp/vault/builtin/logical/pki 0.021s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor Alt Issuer tests to use direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 4.560s
After:
github.com/hashicorp/vault/builtin/logical/pki 0.111s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor root idempotency tests to use direct backend
As a result, we've had to import a root cert from elsewhere in the test
suite, rather than using the one off the cluster.
Before:
github.com/hashicorp/vault/builtin/logical/pki 4.399s
After:
github.com/hashicorp/vault/builtin/logical/pki 0.523s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Move PKI direct backend helpers to common location
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor OID SANs test to direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 5.284s
After:
github.com/hashicorp/vault/builtin/logical/pki 0.808s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor allowed serial numbers test to direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 4.789s
After:
github.com/hashicorp/vault/builtin/logical/pki 0.600s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor URI SANs to use direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 4.245s
After:
github.com/hashicorp/vault/builtin/logical/pki 0.600s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor Full Chain CA tests to direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 14.503s
After:
github.com/hashicorp/vault/builtin/logical/pki 2.082s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update Allow Past CA tests to use direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 4.323s
After:
github.com/hashicorp/vault/builtin/logical/pki 0.322s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Convert existing-key root test to direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 4.430s
After:
github.com/hashicorp/vault/builtin/logical/pki 0.370s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor CRL enable/disable tests to use direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 5.738s
After:
github.com/hashicorp/vault/builtin/logical/pki 2.482s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update intermediate existing key tests to use direct backend
Before:
github.com/hashicorp/vault/builtin/logical/pki 4.182s
After:
github.com/hashicorp/vault/builtin/logical/pki 0.416s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor Issuance TTL verification tests to use direct backend
Also shorten sleep duration slightly by precisely calculating it
relative to the actual cert life time.
Before:
github.com/hashicorp/vault/builtin/logical/pki 19.755s
After:
github.com/hashicorp/vault/builtin/logical/pki 11.521s
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
- Selecting a constant default value exposed a possible edge case
that the migration would fail if a previous migration contained the
same issuer or key name.
* Support for CPS URLs in Custom Policy Identifiers.
* go fmt
* Add Changelog
* Fix panic in test-cases.
* Update builtin/logical/pki/path_roles.go
Fix intial nil identifiers.
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
* Make valid policy OID so don't break ASN parse in test.
* Add test cases.
* go fmt.
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
- Do not set the first issuer we attempt to import as the default issuer unless
it has a corresponding key.
- Add the ability to set a default issuer if none exist and we import it's corresponding key after the fact.
- Add a warning to an end-user if we imported multiple issuers with keys and we
choose one of them as the default value.
* Add integration tests for aliased PKI paths (root/rotate, root/replace)
- Add tests for the two api endpoints
- Also return the issuer_name field within the generate root api response
* Add key_name to generate root api endpoint response and doc updates
- Since we are now returning issuer_name, we should also return key_name
- Update the api-docs for the generate root endpoint responses and add
missing arguments that we accept.
* Allow role-based sign-verbatim with chosen issuer
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add warning with missing requested verbatim role
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update builtin/logical/pki/backend.go
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
* Add tests for role patching
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Prevent bad issuer names on update
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation on PATCH operations
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add a warning when Issuing Certificate set on a role does not resolve.
* Ivanka's requests - add a warning on deleting issuer or changing it's name.
* Fix nil checks; reduce number of roles to iterate through; only verify roles after migration.
* Fix semgrep failure, ignore roles deleted behind our back.
* Patch functionality for roles
* Make Patch Roles work again, add back patch issuers.
* Add changelog.
* Fix nil-reversion on empty response.
* Panics are bad. don't do that.
* Return signed ca as part of ca_chain field within sign-intermediate
- When signing a CA certificate we should include it along with the signing CA's CA chain in the response.
* PKI - Add not_before_duration API parameter to:
- Root CA generation
- Intermediate CA generation
- Intermediate CA signing
* Move not_before_duration to addCACommonFields
This gets applied on both root generation and intermediate signing,
which is the correct place to apply this.
Co-authored-by: guysv <sviryguy@gmail.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Resolves: #10631
Co-authored-by: guysv <sviryguy@gmail.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test case for root/generate, sign-intermediate
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update path role description
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add new not_before_duration to relevant docs
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: guysv <sviryguy@gmail.com>
* Add warning on missing AIA info fields
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog:
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add a warning when Issuing Certificate set on a role does not resolve.
* Ivanka's requests - add a warning on deleting issuer or changing it's name.
* reduce number of roles to iterate through; only verify roles after migration. ignore roles deleted behind our back.
* Protect against key and issuer name re-use
- While importing keys and issuers verify that the provided name if any has not been used by another key that we did not match against.
- Validate an assumption within the key import api, that we were provided a single key
- Add additional tests on the new key generation and key import handlers.
* Protect key import api end-users from using "default" as a name
- Do not allow end-users to provide the value of default as a name for key imports
as that would lead to weird and wonderful behaviors to the end-user.
* Add missing api-docs for PKI key import
* Warn on empty Subject field for issuers
When generating a root or signing an intermediate certificate, it is
possible to have Vault generate a certificate with an empty Subject.
These don't validate in most TLS implementations well, so add a warning.
Note that non-Common Name fields could be present to make a non-empty
subject, so simply requiring a CommonName isn't strictly the best.
For example:
$ vault write pki/root/generate/exported common_name=""
WARNING! The following warnings were returned from Vault:
* This issuer certificate was generated without a Subject; this makes
it likely that issuing leaf certs with this certificate will cause TLS
validation libraries to reject this certificate.
....
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Remove signature_bits on intermediate generate
This extraneous field wasn't respected during intermediate generation
and it isn't clear that it should be. Strictly, this field, if it were
to exist, would control the CSR's internal signature algorithm (certutil
defaults to the sane SHA-256 here). However, there's little value in
changing this as the signing authority can and probably will override
the final certificate's signature bits value, completely ignoring
whatever was in the provided CSR.
Removing this field will now cause warnings for those providing the
parameter (which already wasn't respected), which is the desired
behavior. No breakage should occur as a result of this change.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Always return non-nil CRL configuration
When using the default CRL configuration (as none has been set), return
the default configuration rather than inferring it in buildCRL. This
additionally allows us to return the default configuration on GET
operations to /config/crl.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Always return non-nil URL configuration
When using the default (empty) URL configuration as none has been set,
return the default configuration rather than inferring it inside of
fetchCAInfoByIssuerId or generateCert. This additionally allows us to
return the default configuration on GET operations to /config/urls.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* secret/pki: Return correct algorithm type from key fetch api for managed keys
- fix an issue that key_type field returned from the key fetch api had
the ManagedPrivateKey type instead of the real algorithm of the managed key.
* Remove key_type from key list PKI operation. Partial revert of #15435
- The key_type field should be used solely for the key algorithm but as implemented
we would be returning the value ManagedPrivateKey for managed keys which is not
in sync with the rest of the apis. We also did not want to take the performance
hit if many managed keys existed so we will simply remove the field from the list
operation
- No point in writing any logs if no previous bundle exists
- Only log output and schedule a CRL rebuild is we actually migration something
- Do not log on PKI storage version set/checks.
* Use "not_before_duration" fiueld from role if above 0
* 'test' and update docs
* changelog file
* Requested changes - improved test and better description to changelog
* changelog description:
* update to ttl and not_before_duration API docs
