Commit graph

421 commits

Author SHA1 Message Date
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
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
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
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
Alexander Scheel b8cfb4dde5
Ignore EC PARAMETER blocks during issuer import (#16721)
* 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>
2022-08-15 08:59:10 -07:00
Alexander Scheel e388cfec64
Add BYOC-based revocation to PKI secrets engine (#16564)
* 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>
2022-08-15 08:50:57 -05:00
Alexander Scheel a259978a3d
Add warning when generate_lease=true (#16398)
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>
2022-08-08 13:26:10 -04:00
Eng Zer Jun 61262ad98e
refactor: replace strings.Replace with strings.ReplaceAll (#15392)
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>
2022-08-03 15:22:48 -04:00
swayne275 4632a26a09
Use %q for quoted strings where appropriate (#15216)
* change '%s' to %q where single vs double quotes shouldn't matter

* replace double quotes with %q in logs and errors
2022-08-03 12:32:45 -06:00
Alexander Scheel 8acbf7f480
Add PSS support to PKI Secrets Engine (#16519)
* 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>
2022-08-03 12:42:24 -04:00
Alexander Scheel cf7105929f
Allow old certs to be cross-signed (#16494)
* 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>
2022-08-03 06:34:21 -07:00
Alexander Scheel 4dbbd3e1f8
Make PKI tests run in parallel (#16514)
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>
2022-08-01 16:43:38 -04:00
Alexander Scheel aba72d7f7a
Add next-step warning on import without AIA URLs (#16392)
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>
2022-07-21 11:05:19 -04:00
Steven Clark d04b143bd5
pki: When a role sets key_type to any ignore key_bits value when signing a csr (#16246)
* 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.
2022-07-08 10:56:15 -04:00
Alexander Scheel d353245af3
Remove structs, mapstructure from PKI storage (#16190)
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>
2022-06-29 15:05:31 -04:00
Alexander Scheel 920ec37b21
Refactor PKI storage calls to take a shared struct (#16019)
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>
2022-06-29 12:00:44 -04:00
Alexander Scheel 75eedf1b97
Add warning on missing tidy targets (#16164)
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>
2022-06-27 17:42:41 -04:00
Alexander Scheel 327963af03
Return errors on short PEM bundles (keys, issuers) (#16142)
* 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>
2022-06-24 15:32:56 -04:00
Alexander Scheel eeb4029eb1
Add signature_bits to sign-intermediate, sign-verbatim (#16124)
* 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>
2022-06-23 14:07:27 -04:00
Alexander Scheel 248990a1ec
Fix leaf revocation under intermediate CAs (#16052)
* 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>
2022-06-17 18:04:51 -04:00
Alexander Scheel b00e32fec7
Fix format errors in PKI tests (#16015)
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
2022-06-16 07:41:05 -07:00
Alexander Scheel 4e6a9741ee
Add explicit cn_validations field to PKI Roles (#15996)
* 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>
2022-06-16 06:53:27 -07:00
Alexander Scheel 3496bc0416
Refactor PKI tests for speed (#15999)
* 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>
2022-06-16 09:11:22 -04:00
Steven Clark 3b9f29fedd
pki: Do not use a static issuer/key name within the migration (#15886)
- 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.
2022-06-08 15:31:30 -04:00
Kit Haines 4f532ecc4d
Support for CPS URLs in Custom Policy Identifiers. (#15751)
* 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>
2022-06-03 14:50:46 -04:00
Steven Clark 0b6781e3b9
PKI: Only set issuers with an associated key as default on import (#15754)
- 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.
2022-06-02 12:59:07 -04:00
Steven Clark 2e215975ff
Add integration tests for aliased PKI paths (root/rotate, root/replace) (#15703)
* 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.
2022-05-31 15:00:20 -04:00
Alexander Scheel 3166d1ff78
Allow issuer/:issuer_ref/sign-verbatim/:role, add error on missing role (#15543)
* 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>
2022-05-23 13:09:18 -04:00
Alexander Scheel 69b870d675
Add role patching test case (#15545)
* 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>
2022-05-20 15:30:22 -04:00
kitography 024716421e
Vault 5917 allow patch operations to pki roles issuers (#15510)
* 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.
2022-05-20 13:34:55 -04:00
Steven Clark 0e18a68691
PKI: Do not error out on unknown issuers/keys on delete api calls. (#15541)
- No longer error out when we fail to lookup the passed in issuer_ref
   or key_ref values on delete apis.
 - Add more key related unit tests
2022-05-20 13:33:26 -04:00
Steven Clark 892d4d1e37
Return the signed ca in the ca_chain response field within sign-intermediate api call. (#15524)
* 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.
2022-05-20 11:06:44 -04:00
Alexander Scheel faea196991
Rebase #14178 / Add not_before_duration API parameter to Root/Intermediate CA generation (#15511)
* 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>
2022-05-19 12:35:08 -04:00
Alexander Scheel c7efb97f08
Add warning on missing AIA info fields (#15509)
* 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>
2022-05-19 11:12:10 -04:00