Commit graph

468 commits

Author SHA1 Message Date
Alexander Scheel a8764e0cf1
Refactor PKI to use shared storage context (#18266)
A lot of places took a (context, backend, request) tuple, ignoring the
request proper and only using it for its storage. This (modified) tuple
is exactly the set of elements in the shared storage context, so we
should be using that instead of manually passing all three elements
around.

This simplifies a few places where we'd generate a storage context at
the request level and then split it apart only to recreate it again
later (e.g., CRL building).

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
2022-12-08 09:27:02 -05:00
Steven Clark 735a8d4b31
Address race in PKI test case (#18267)
- 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.
2022-12-08 09:11:05 -05:00
Alexander Scheel 2398634862
Respond with data to all writes in PKI engine (#18222)
* 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>
2022-12-05 10:40:39 -05:00
Alexander Scheel f86fdf530f
Allow templating cluster-local AIA URIs (#18199)
* 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>
2022-12-05 10:38:26 -05:00
Steven Clark 9bf3b4c582
Do not use possibly nil HttpRequest object in default OCSP handler (#18190) 2022-12-01 13:23:41 -05:00
Steven Clark 826e87884e
Address a nil panic when writing an empty POST request to the ocsp handler (#18184)
* Address a nil panic when writing an empty POST request to the ocsp handler

 - Seems when no JSON body is sent with a POST request Vault will not
   populate the HTTPRequest member variable which caused the nil panic
   - vault write -force pki/ocsp
 - Add a check for it and the Body member variable to be nil before use.

* Add cl
2022-12-01 15:10:12 +00:00
Alexander Scheel a8faa543e6
Add pki nginx/wget/curl/Go integration tests (#17320)
* Rename integation_test.go->integration_test.go

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add ability to fetch container's network addresses

This lets us return the on-network container address, allowing us to
spawn client containers which contact server containers.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add integration tests with nginx, curl, wget, Go

We build new integration tests, spawning a test instance on nginx and
ensuring we can connect with a variety of clients against a variety of
CA and leaf certificate types. This will ultimately let us detect issues
with compatibility as we expand the matrix of supported servers and
clients.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Make runner reference unique

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Attempt to fix CI with longer wait

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Finish moving nginx tests to pkiext package

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* make fmt

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add more debugging, work on CircleCI

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
2022-11-23 15:00:18 -05:00
Steven Clark 92c1a2bd0a
New PKI API to generate and sign a CRL based on input data (#18040)
* 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
2022-11-22 11:41:04 -05:00
Scott Miller b51b2a7027
Add cached OCSP client support to Cert Auth (#17093)
* 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>
2022-11-21 10:39:24 -06:00
Steven Clark 01e87c481c
Add new PKI api to combine and sign different CRLs from the same issuer (#17813)
* 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
2022-11-17 16:53:05 -05:00
Alexander Scheel c25b90831e
Move pki docker tests to pkiext (#17928)
* 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>
2022-11-14 18:26:26 -05:00
Alexander Scheel a0136f5f19
Handle removed default issuers (#17930)
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>
2022-11-14 15:13:39 -05:00
Alexander Scheel 5a2ee4ca7a
Add automatic tidy of expired issuers (#17823)
* 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>
2022-11-10 10:53:26 -05:00
Alexander Scheel e7d57bfe90
Move from %v->%w for errs (#17860)
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>
2022-11-09 15:40:26 -05:00
Alexander Scheel 06f30de35f
Optional automatic default issuer selection (#17824)
* 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>
2022-11-08 14:40:29 -05:00
Steven Clark 419ba9159c
Add new API to PKI to list revoked certificates (#17779)
* 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
2022-11-03 14:17:17 -04:00
Alexander Scheel ffa4825693
PKI - Fix order of chain building writes (#17772)
* 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>
2022-11-03 11:50:03 -04:00
Alexander Scheel 8e6e53cf63
Use hashicorp mirror for container pulls (#17778)
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>
2022-11-02 13:33:17 -04:00
Steven Clark 550fbdc41c
Return revocation info within existing certs/<serial> api (#17774)
* 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)
2022-11-02 13:06:04 -04:00
Alexander Scheel 5e0ce5ec3e
Bump validity period check to satisfy CircleCI (#17740)
* 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>
2022-10-31 10:01:09 -04:00
Alexander Scheel d67023c3b3
Add empty expiry crlConfig upgrade test (#17701)
* 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>
2022-10-27 11:20:12 -04:00
James Protzman a47848706e
Default crl expiry (#17693)
Ref: https://github.com/hashicorp/vault/issues/17642
2022-10-27 10:47:17 -04:00
Nick Cabatoff 8e67651dcd
Fix a data race with rollbackPeriod. (#17387) 2022-10-13 09:59:07 -04:00
Alexander Scheel daf29de742
Add pki zlint tests (#17305)
* 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>
2022-10-12 13:29:06 -04:00
Alexander Scheel d48e739b1d
Fix tidy-status, tidy-cancel on PR Secondaries (#17497)
* 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>
2022-10-12 09:15:06 -04:00
Alexander Scheel fdb13d7481
Don't include issuers on delta CRLs (#17463)
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>
2022-10-07 12:36:22 -04:00
Steven Clark f0bf670b0c
Unit tests that validate OCSP signatures leverage revocation signature algo (#17452)
- 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
2022-10-07 12:33:17 -04:00
Ruben De Visscher 1f0cf558a9
Fix for duplicate SANs in signed certificates (#16700)
* 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.
2022-10-07 12:19:08 -04:00
Alexander Scheel 412603befd
Fix RevocationSigAlg provisioning in GCP (#17449)
* 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>
2022-10-06 17:50:49 -04:00
Alexander Scheel b85d6ec434
Fix RevocationSigAlgo support in OCSP (#17436)
* 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>
2022-10-06 12:01:12 -04:00
Steven Clark 10ecf10248
PKI: Add support for signature_bits param to the intermediate/generate api (#17388)
* 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
2022-10-03 12:39:54 -04:00
Steven Clark bb1d36f401
PKI: Do not load revoked certificates if CRL has been disabled (#17385)
* 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
2022-10-03 10:04:32 -04:00
Alexander Scheel ccdd55529c
Remove delta indicator on main CRL (#17334)
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>
2022-09-27 17:44:38 -04:00
Kit Haines 2d58591feb
Fix non-atomic read of atomic value fix (#17255)
* Always load to access certCount

* Test-reads of the atomic value.
2022-09-21 11:24:34 -04:00
Alexander Scheel ad3a093b40
Prevent PSS with Go-incompatible CAs, CSRs, Private Keys (#17223)
* 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>
2022-09-20 17:30:58 -04:00
Kit Haines f2adbb3e47
Basics of Cert-Count Non-Locking Telemetry (#16676)
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.
2022-09-20 10:32:20 -07:00
Alexander Scheel c0264c923d
Don't race for CRL rebuilding capability check (#17185)
* 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>
2022-09-19 11:41:32 -04:00
Kit Haines 71d9c33802
Add "plumbing" for surfacing warnings, and warning overwriting ttl (#17073)
* Add "plumbing" for surfacing warnings, and add warning about TTL > maxTTL when issuing a cert.
2022-09-15 12:38:33 -07:00
Steven Clark cfb56105b8
A PKI test to verify our defaults are the same for creates and update apis (#17094) 2022-09-12 09:22:56 -04:00
Alexander Scheel 6d90586ad6
Update issuer usage with ocsp-signing by default (#17087)
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>
2022-09-09 13:28:45 -04:00
Josh Black d8e0a13aae
update gofumpt to 0.3.1 and reformat the repo (#17055)
* update gofumpt to 0.3.1 and reformat the repo

* output the version of the formatter we're using
2022-09-07 17:31:20 -07:00
Steven Clark 96f1443265
Fix various trivial warnings from staticcheck in the PKI plugin (#16946)
* Fix up simple warnings in production code

* Address warnings from static check in the PKI test classes
2022-08-31 16:25:14 -04:00
Alexander Scheel b8576a8de4
Add ability to request manual rebuild of Delta CRLs (#16964)
* 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>
2022-08-31 12:42:59 -07:00
Alexander Scheel f0a127487b
Add ability to cancel PKI tidy operations, pause between tidying certs (#16958)
* 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>
2022-08-31 11:36:12 -07:00
Steven Clark b21e06b917
Add remove_roots_from_chain to sign and issue pki apis (#16935)
* 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
2022-08-31 09:51:26 -04:00
Alexander Scheel e87584f9ba
Fix comment of default value of tidy interval (#16948)
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>
2022-08-31 09:23:17 -04:00
Alexander Scheel a5fafd8163
Add ability to perform automatic tidy operations (#16900)
* 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>
2022-08-30 15:45:54 -04:00
Alexander Scheel f34a93a560 Clean up behavior of If-Modified-Since header (#16929)
* 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>
2022-08-30 13:25:36 -04:00
Gabriel Santos ff5ff849fd
PKI - Honor header If-Modified-Since if present (#16249)
* 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>
2022-08-29 15:28:47 -04:00
Alexander Scheel e03fb14be4
Support for generating Delta CRLs (#16773)
* 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>
2022-08-29 11:37:09 -04:00