Commit graph

1441 commits

Author SHA1 Message Date
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
Christopher Swenson 2c8e88ab67
Check if plugin version matches running version (#17182)
Check if plugin version matches running version

When registering a plugin, we check if the request version matches the
self-reported version from the plugin. If these do not match, we log a
warning.

This uncovered a few missing pieces for getting the database version
code fully working.

We added an environment variable that helps us unit test the running
version behavior as well, but only for approle, postgresql, and consul
plugins.

Return 400 on plugin not found or version mismatch

Populate the running SHA256 of plugins in the mount and auth tables (#17217)
2022-09-21 12:25:04 -07: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
Steven Clark 0856fa11a3
Fix fmt error (#17241) 2022-09-20 13:33:01 -07: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
vinay-gopalan c548ea39be
Re-initialize v5 backend after a plugin crash (#17140) 2022-09-19 16:48:45 -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
Christopher Swenson b136a7ecd8
Add plugin version to GRPC interface (#17088)
Add plugin version to GRPC interface

Added a version interface in the sdk/logical so that it can be shared between all plugin types, and then wired it up to RunningVersion in the mounts, auth list, and database systems.

I've tested that this works with auth, database, and secrets plugin types, with the following logic to populate RunningVersion:

If a plugin has a PluginVersion() method implemented, then that is used
If not, and the plugin is built into the Vault binary, then the go.mod version is used
Otherwise, the it will be the empty string.
My apologies for the length of this PR.

* Placeholder backend should be external

We use a placeholder backend (previously a framework.Backend) before a
GRPC plugin is lazy-loaded. This makes us later think the plugin is a
builtin plugin.

So we added a `placeholderBackend` type that overrides the
`IsExternal()` method so that later we know that the plugin is external,
and don't give it a default builtin version.
2022-09-15 16:37:59 -07: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 12242d1a97
make fmt (#17131) 2022-09-14 07:45:50 -05:00
Scott Miller 12a8ef1cfd
Implement partial_failure_response_code_override for batch requests (#17118)
* Implement partial_failure_response_code_override for batch requests

* docs

* changelog

* one more test case
2022-09-13 12:51:09 -05: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
Tom Proctor aa50e42fca
Support version selection for database plugins (#16982)
* Support version selection for database plugins
* Don't consider unversioned plugins for version selection algorithm
* Added version to 'plugin not found' error
* Add PluginFactoryVersion function to avoid changing sdk/ API
2022-09-09 17:32:28 +01: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
Matt Schultz d50941ac85
Account for a possible error in Transit BYOK. (#17025) 2022-09-06 11:03:24 -07:00
Steven Clark 98fffbe949
Address failures in FIPS builds around new transit RSA PSS tests (#17024)
- When we added new tests that validate the RSA PSS feature, they
   work properly on normal Go builds, but tests underneath the Boring
   Crypto fips implementations fail due to a lack of SHA3 support in
   FIPS 140-2.
2022-09-06 13:40:13 -04:00
Scott Miller 606edb66d6
Add support for a dedicated HMAC type in Transit. (#16668)
* Get import correct

* limits, docs

* changelog

* unit tests

* And fix import for hmac unit test

* typo

* Update website/content/api-docs/secret/transit.mdx

Co-authored-by: Matt Schultz <975680+schultz-is@users.noreply.github.com>

* Update builtin/logical/transit/path_keys.go

Co-authored-by: Matt Schultz <975680+schultz-is@users.noreply.github.com>

* Validate key sizes a bit more carefully

* Update sdk/helper/keysutil/policy.go

Co-authored-by: Matt Schultz <975680+schultz-is@users.noreply.github.com>

Co-authored-by: Matt Schultz <975680+schultz-is@users.noreply.github.com>
2022-09-06 10:17:58 -05: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
Trishank Karthik Kuppusamy 303f59dce3
Allow configuring the possible salt lengths for RSA PSS signatures (#16549)
* accommodate salt lengths for RSA PSS

* address feedback

* generalise salt length to an int

* fix error reporting

* Revert "fix error reporting"

This reverts commit 8adfc15fe3303b8fdf9f094ea246945ab1364077.

* fix a faulty check

* check for min/max salt lengths

* stringly-typed HTTP param

* unit tests for sign/verify HTTP requests

also, add marshaling for both SDK and HTTP requests

* randomly sample valid salt length

* add changelog

* add documentation
2022-08-31 12:27:03 -04: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
Kit Haines 6ac085dc57
Fix CodeQL Errors - check allocation is smaller than 2^30 (#16869)
* Fix CodeQL Errors - check allocation is smaller than 32 bits.

* make fmt.
2022-08-26 13:26:11 -04:00
Alexander Scheel 43e722c69a
Let PKI tidy associate revoked certs with their issuers (#16871)
* 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>
2022-08-26 10:13:45 -07:00
Brian Shumate 2ab4a58ba9
Transit: update documentation strings (#10027)
- Update descriptions to match field content
  (actually key name, not policy name)
2022-08-26 09:25:02 -07:00
Steven Clark 34ff0154e8
Add ocsp_expiry configuration field to PKI crl config (#16888)
* 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
2022-08-25 16:01:39 -04:00
Steven Clark 35689fa9dc
Add maximums on how much data we will read for OCSP requests (#16879)
* Add maximums on how much data we will read for OCSP requests

* Update max size to 2048
2022-08-25 12:59:49 -04:00
Alexander Scheel f7bc1c8e3c
Cleanup changes around issuer revocation (#16874)
* 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>
2022-08-25 11:36:37 -04:00
Tom Proctor a52fd805dd
Pin MongoDB test container images pre-v6 (#16880)
v6 was released in the last 24h, and our tests fail to connect to the db when v6 is used.
Using v6 needs investigating, but for now I'm pinning to the last known good version.
2022-08-25 08:14:37 -07:00
Alexander Scheel 96ea52343a
Identify issuer on revocation (#16763)
* 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>
2022-08-24 12:23:27 -04:00
Alexander Scheel 6089d2e247
Don't allow crl-signing issuer usage without CRLSign KeyUsage (#16865)
* 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>
2022-08-24 07:45:54 -07:00
Steven Clark 312f7a1882
Handle multiple matching issuers in OCSP requests (#16848)
* 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
2022-08-24 09:00:40 -04:00
Alexander Scheel 231f422822
Finish refactor to remove global crlLifetime (#16835)
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>
2022-08-23 15:19:11 -04:00
Alexander Scheel cacb23bda6
Enable periodic, automatic rebuilding of CRLs (#16762)
* 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>
2022-08-23 13:27:15 -04:00
Kit Haines e9e3b4995b
Add _remaining tidy metrics. (#16702)
* Add _remaining tidy metrics.

* Add two extra metrics during tidy.

* Update test and documentation for remaining tidy metrics.
2022-08-23 12:17:17 -04:00
Kit Haines b3e8098685
Fix LIST issuers endpoint (#16830)
* Fix LIST issuers endpoint ability to access, add a comment.

* Add changelog.
2022-08-23 11:08:23 -04:00
Steven Clark e024324c34
Add an OCSP responder to Vault's PKI plugin (#16723)
* 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
2022-08-22 14:06:15 -04:00
Steven Clark da7fd8f639
Migrate existing PKI mounts that only contains a key (#16813)
* 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
2022-08-22 10:11:21 -07:00
Alexander Scheel 49fd772fcc
Add per-issuer AIA URI information to PKI secrets engine (#16563)
* 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>
2022-08-19 11:43:44 -04:00
Alexander Scheel 0c22c76907
Allow marking issuers as revoked (#16621)
* 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>
2022-08-18 18:08:31 -04:00
Jakob Beckmann 21a10e09b6
fix bug with allowed_users_template and add allowed_domains_template for SSH role (#16056)
* impr(ssh): fix bug with allowed_users_template and add allowed_domains_template field in SSH role configuration, closes #10943

* chore: add changelog entry
2022-08-16 14:59:29 -05:00
Alexander Scheel 1e6730573c
Add proof possession revocation for PKI secrets engine (#16566)
* 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>
2022-08-16 14:01:26 -04:00