* Refactor plugin catalog into its own package
* Fix some unnecessarily slow tests due to accidentally running multiple plugin processes
* Clean up MakeTestPluginDir helper
* Move getBackendVersion tests to plugin catalog package
* Use corehelpers.MakeTestPlugin consistently
* Fix semgrep failure: check for nil value from logical.Storage
- Noticed that our documentation was out of date, we allow 8192
bit RSA keys to be used as an argument to the various PKI
issuer/key creation APIs.
- Augument some unit tests to verify this continues to work
* allow to skip TLS check in acme http-01 challenge
* remove configurable logic, just ignore TLS
* add changelog
* Add test case
---------
Co-authored-by: Steve Clark <steven.clark@hashicorp.com>
* Address a panic export RSA public keys in transit
- When attempting to export the public key for an RSA key that
we only have a private key for, the export panics with a nil
deference.
- Add additional tests around Transit key exporting
* Add cl
- Try to avoid these build failures as our proxy does seem to have
issues around pulling images with the 'latest' tag at times.
```
acme_test.go:206:
Error Trace: /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/builtin/logical/pkiext/pkiext_binary/acme_test.go:206
/home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/builtin/logical/pkiext/pkiext_binary/acme_test.go:75
Error: Received unexpected error:
container create failed: Error response from daemon: No such image: docker.mirror.hashicorp.services/curlimages/curl:latest
Test: Test_ACME/group/caddy_http_eab
Messages: could not start cURL container
```
* wip
* more pruning
* Integrate OCSP into binary paths PoC
- Simplify some of the changes to the router
- Remove the binary test PKI endpoint
- Switch OCSP to use the new binary paths backend variable
* Fix proto generation and test compilation
* Add unit test for binary request handling
---------
Co-authored-by: Scott G. Miller <smiller@hashicorp.com>
* fix lease revocation when config token exists in one namespace but can create tokens in another
* add test
* Add similar check for admin partition
* Add admin partition test
---------
Co-authored-by: robmonte <17119716+robmonte@users.noreply.github.com>
* Forbid setting auto_rotate_period on transit managed keys
- Prevent and guard against auto-rotating managed keys as we
generate an invalid key version without the uuid field set.
- Hook in the datakey generation api into managed key encryption.
* Add cl
- Do not load existing ACME challenges persisted within storage on non-active nodes. This was the main culprit of the issues, secondary nodes would load existing persisted challenges trying to resolve them but writes would fail leading to the excessive logging.
- We now handle this by not starting the ACME background thread on non-active nodes, while also checking within the scheduling loop and breaking out. That will force a re-reading of the Closing channel that should have been called by the PKI plugin's Cleanup method.
- If a node is stepped down from being the active node while it is actively processing a verification, we could get into an infinite loop due to an ErrReadOnly error attempting to clean up a challenge entry
- Add a maximum number of retries for errors around attempting to decode,fetch challenge/authorization entries from disk. We use double the number of "normal" max attempts for these types of errors, than we would for normal ACME retry attempts to avoid collision issues. Note that these additional retry attempts are not persisted to disk and will restart on every node start
- Add a 1 second backoff to any disk related error to not immediately spin on disk/io errors for challenges.
* secrets/db: add rotation error path test
We add a test to verify that failed rotations can successfully recover
and that they do not occur outside of a rotation window. Additionally,
we remove registering some external plugins in getCluster() that shaves
off about 5 minutes the database package tests.
* remove dead code and add test comment
* revert to original container helper after refactor
This fixes the enterprise failure of the test
```
=== FAIL: builtin/logical/pki TestCRLIssuerRemoval (0.00s)
crl_test.go:1456:
Error Trace: /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/builtin/logical/pki/crl_test.go:1456
Error: Received unexpected error:
Global, cross-cluster revocation queue cannot be enabled when auto rebuilding is disabled as the local cluster may not have the certificate entry!
Test: TestCRLIssuerRemoval
Messages: failed enabling unified CRLs on enterprise
```
* Clean up unused CRL entries when issuer is removed
When a issuer is removed, the space utilized by its CRL was not freed,
both from the CRL config mapping issuer IDs to CRL IDs and from the
CRL storage entry. We thus implement a two step cleanup, wherein
orphaned CRL IDs are removed from the config and any remaining full
CRL entries are removed from disk.
This relates to a Consul<->Vault interop issue (#22980), wherein Consul
creates a new issuer on every leadership election, causing this config
to grow. Deleting issuers manually does not entirely solve this problem
as the config does not fully reclaim space used in this entry.
Notably, an observation that when deleting issuers, the CRL was rebuilt
on secondary clusters (due to the invalidation not caring about type of
the operation); for consistency and to clean up the unified CRLs, we
also need to run the rebuild on the active primary cluster that deleted
the issuer as well.
This approach does allow cleanup on existing impacted clusters by simply
rebuilding the CRL.
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test case on CRL removal
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>
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
* Ignore nonces when encrypting without convergence or with convergence versions > 1
* Honor nonce use warning in non-FIPS modes
* Revert "Honor nonce use warning in non-FIPS modes"
This reverts commit 2aee3dbdc11c4e333ecb20503539c7993b24ee57.
* Add a test func that removes a nonce when not needed
* err out rather than ignore the nonce
* Alter unit test to cover, also cover convergent version 3
* More unit test work
* Fix test 14
* changelog
* tests not already in a nonce present path
* Update unit test to not assume warning when nonce provided incorrectly
* remove unused test field
* Fix auto-squash events experiments
When #22835 was merged, it was auto-squashed, so the `experiments`
import was removed, but the test still referenced it.
This removes the (now unnecessary) experiment from the test.
* Allow nonces for managed keys, because we have no way of knowing if the backing cipher/mode needs one
---------
Co-authored-by: Christopher Swenson <christopher.swenson@hashicorp.com>
The recent additions to the transit secret engine have created two new
endpoints which both have the incorrect (and duplicate) operation ID of
just `"update"`.
Amend to unique meaningful values.
* add rotation_schedule field to db backend
* add cron schedule field
* use priority queue with scheduled rotation types
* allow marshalling of cron schedule type
* return warning on use of mutually exclusive fields
* handle mutual exclusion of rotation fields (#22306)
* handle mutual exclusion of rotation fields
* fix import
* adv ttl mgmt: add rotation_window field (#22303)
* adv ttl mgmt: add rotation_window field
* do some rotation_window validation and add unit tests
* adv ttl mgmt: Ensure initialization sets appropriate rotation schedule (#22341)
* general cleanup and refactor rotation type checks
* make NextRotationTime account for the rotation type
* add comments
* add unit tests to handle mutual exclusion (#22352)
* add unit tests to handle mutual exclusion
* revert rotation_test.go and add missing test case to path_roles_test.go
* adv ttl mgmt: add tests for init queue (#22376)
* Vault 18908/handle manual rotation (#22389)
* support manual rotation for schedule based roles
* update description and naming
* adv ttl mgmt: consider rotation window (#22448)
* consider rotation window
ensure rotations only occur within a rotation window for schedule-based
rotations
* use helper method to set priority in rotateCredential
* fix bug with priority check
* remove test for now
* add and remove comments
* add unit tests for manual rotation (#22453)
* adv ttl mgmt: add tests for rotation_window
* adv ttl mgmt: refactor window tests (#22472)
* Handle GET static-creds endpoint (#22476)
* update read static-creds endpoint to include correct resp data
* return rotation_window if set
* update
* add changelog
* add unit test for static-creds read endpoint (#22505)
* Add the ability to set seconds in cron schedule for testing purposes
* update test so we don't use global var
* update with suggestions
---------
Co-authored-by: JM Faircloth <jmfaircloth@hashicorp.com>
Co-authored-by: John-Michael Faircloth <fairclothjm@users.noreply.github.com>
* add rotation_schedule field to db backend
* add cron schedule field
* use priority queue with scheduled rotation types
* allow marshalling of cron schedule type
* return warning on use of mutually exclusive fields
* handle mutual exclusion of rotation fields (#22306)
* handle mutual exclusion of rotation fields
* fix import
* adv ttl mgmt: add rotation_window field (#22303)
* adv ttl mgmt: add rotation_window field
* do some rotation_window validation and add unit tests
* adv ttl mgmt: Ensure initialization sets appropriate rotation schedule (#22341)
* general cleanup and refactor rotation type checks
* make NextRotationTime account for the rotation type
* add comments
* add unit tests to handle mutual exclusion (#22352)
* add unit tests to handle mutual exclusion
* revert rotation_test.go and add missing test case to path_roles_test.go
* adv ttl mgmt: add tests for init queue (#22376)
* Vault 18908/handle manual rotation (#22389)
* support manual rotation for schedule based roles
* update description and naming
* adv ttl mgmt: consider rotation window (#22448)
* consider rotation window
ensure rotations only occur within a rotation window for schedule-based
rotations
* use helper method to set priority in rotateCredential
* fix bug with priority check
* remove test for now
* add and remove comments
* add unit tests for manual rotation (#22453)
* adv ttl mgmt: add tests for rotation_window
* adv ttl mgmt: refactor window tests (#22472)
* Handle GET static-creds endpoint (#22476)
* update read static-creds endpoint to include correct resp data
* return rotation_window if set
* update
* add changelog
* add unit test for static-creds read endpoint (#22505)
---------
Co-authored-by: Milena Zlaticanin <60530402+Zlaticanin@users.noreply.github.com>
* setup initial boilerplate code for sign csr endpoint
* add function to sign csr
* working version of sign csr endpoint
* improving errors for csr create and sign endpoint
* initial implementation for import leaf certificate endpoint
* check if more than one certificate was provided in the ceritificate chain
* improve validate cert public key matches transit key
* convert provided cert chain from PEM to DER so it can be parsed by
x509.ParseCertificates and fixing other bugs
* fix creation of csr from csrTemplate
* add missing persist of certificate chain after validations in set-certificate endpoint
* allow exporting a certificate-chain
* move function declaration to end of page
* improving variable and function names, removing comments
* fix certificate chain parsing - work in progress
* test for signCsr endpoint
* use Operations instead of Callbacks in framework.Path
* setup test for set-certificate endpoint
fix problems with sign-csr endpoint returning base64
* finish set-certificate endpoint test
* use public key KeyEntry fields instead of retrieving public key from private
* improve error message and make better distinction between client and server error
also moved check of key types before checking if key match to endpoint handler
* check if private key has been imported for key version selected when signing a csr
* improve errors
* add endpoint description and synopsis
* fix functions calls in backend as function names changed
* improve import cert chain test
* trim whitespaces on export certificate chain
* changelog
* pass context from handler function to policy Persist
* make fmt run
* fix: assign returned error from PersistCertificateChain to err so it can be evaluated
* additional validations and improvements to parseCertificateChain function
* add validation to check if there is only one certificate in the certificate chain and it is in the first position
* import cert chain test: move creation of cluster to exported test function
* move check of end-cert pub key algorithm and key transit algorithm match into a separate function
* test export certificate chain
* Update sdk/helper/keysutil/policy.go
Co-authored-by: Alexander Scheel <alexander.m.scheel@gmail.com>
* fix validateLeafCertPosition
* reject certificate actions on policies that allow key derivation and remove derived checks
* return UserError from CreateCSR SDK function as 400 in transit API handler
* add derived check for ED5519 keys on CreateCSR SDK func
* remove unecessary calls of x509.CreateCertificateRequest
* move validate key type match back into SDK ValidateLeafCertMatch function
* add additional validations (ValidateLeafCertKeyMatch, etc) in SDK PersistCertificateChain function
* remove uncessary call of ValidateLeafCertKeyMatch in parseImportCertChainWrite
* store certificate chain as a [][]byte instead of []*x509.Certificate
* include persisted ca chain in import cert-chain response
* remove NOTE comment
* allow exporting cert-chain even if exportable is set as false
* remove NOTE comment
* add certifcate chain to formatKeyPublic if present
also added an additional check to validate if field is added when
certchain is present
---------
Co-authored-by: Alexander Scheel <alexander.m.scheel@gmail.com>
* Adding explicit MPL license for sub-package.
This directory and its subdirectories (packages) contain files licensed with the MPLv2 `LICENSE` file in this directory and are intentionally licensed separately from the BSL `LICENSE` file at the root of this repository.
* Adding explicit MPL license for sub-package.
This directory and its subdirectories (packages) contain files licensed with the MPLv2 `LICENSE` file in this directory and are intentionally licensed separately from the BSL `LICENSE` file at the root of this repository.
* Updating the license from MPL to Business Source License.
Going forward, this project will be licensed under the Business Source License v1.1. Please see our blog post for more details at https://hashi.co/bsl-blog, FAQ at www.hashicorp.com/licensing-faq, and details of the license at www.hashicorp.com/bsl.
* add missing license headers
* Update copyright file headers to BUS-1.1
* Fix test that expected exact offset on hcl file
---------
Co-authored-by: hashicorp-copywrite[bot] <110428419+hashicorp-copywrite[bot]@users.noreply.github.com>
Co-authored-by: Sarah Thompson <sthompson@hashicorp.com>
Co-authored-by: Brian Kassouf <bkassouf@hashicorp.com>
* Refactor sign-intermediate API response
- Allow the sign-intermediate response handling code to be shared
across different API calls.
* Add missing cieps.go
- The Golang ACME library will retry on failures until the passed in
context times out. So if a test is broken it will never end until
the context expires.
- The timeouts are rather large to avoid some issues with slow CI
runners, that have already triggered on a nightly 32 bit run
* Use Hashicorp docker proxy for CI docker images
* Use library/consul as the mirror path instead of hashicorp/consul
- Looks like the older 1.4.4 image was not published within the
hashicorp/consul space, only newer images are.
- Switch to library/consul which seems to have both versions
* OpenAPI: Define default response structure for ListOperations
Almost all Vault ListOperation responses have an identical response
schema. Update the OpenAPI generator to know this, and remove a few
instances where that standard response schema had been manually
copy/pasted into place in individual endpoints.
* changelog
* Only render StandardListResponse schema, if an operation uses it
* Teach the response schema validation test helper about the default list schema too
---------
Co-authored-by: Anton Averchenkov <84287187+averche@users.noreply.github.com>
* Add missing `Query: true` metadata to API definitions
Also improve the documentation comment for `Query` to guide people better how they should be setting `Query` in the future.
Endpoints affected:
- auth/approle/role/{role_name}/secret-id/destroy
- auth/approle/role/{role_name}/secret-id-accessor/destroy
- auth/token/lookup
- auth/token/lookup-self
- sys/internal/specs/openapi
- sys/wrapping/lookup
- identity/oidc/provider/{name}/authorize
There are also endpoints in the `aws` and `gcp` secrets engines which need the same treatment in their own PRs.
When working on the `auth/token/lookup-self` path, I discovered that it
had a parameter which was completely pointless - it was even documented
as unused. It only existed because the `auth/token/lookup-self` code
path was implemented by bodging the current token into the request data
and passing control to the `auth/token/lookup` handler directly -
instead of just factoring out the common code to a reusable function -
so I fixed that whilst I was there.
Note that two of the affected endpoints currently have one form of their
OpenAPI operation ID set to something mentioning "with-parameters":
- identity/oidc/provider/{name}/authorize
- sys/internal/specs/openapi
These operation IDs should be changed, as they perpetuate
a misunderstanding - both read (GET) and update (POST/PUT) forms of
these APIs are **equally** capable of being used with parameters.
* I failed to spot that the aws plugin is in-repo! Update that too.
* Remove code cleanup changes from this PR
* Wording and wrapping adjustment as requested.
* CreateOperation should only be implemented alongside ExistenceCheck
Closes#12329
Vault treats all POST or PUT HTTP requests equally - they default to
being treated as UpdateOperations, but, if a backend implements an
ExistenceCheck function, CreateOperations can be separated out when the
existence check returns false.
It follows, then, that if a CreateOperation handler is implemented
without an ExistenceCheck function, this is unreachable code - a coding
error. It's a fairly minor error in the grand scheme of things, but it
causes the generated OpenAPI spec to include x-vault-createSupported for
operations on which create can never actually be invoked - and promotes
muddled understanding of the create/update feature.
In this PR:
1) Implement a new test, which checks all builtin auth methods and
secrets engines can be successfully initialized. (This is important
to validate the next part.)
2) Expand upon the existing coding error checks built in to
framework.Backend, adding a check for this misuse of CreateOperation.
3) Fix up instances of improper CreateOperation within the Vault
repository - just two, transit and mock.
Note: At this point, the newly added test will **fail**.
There are improper uses of CreateOperation in all of the following:
vault-plugin-auth-cf
vault-plugin-auth-kerberos
vault-plugin-auth-kubernetes
vault-plugin-secrets-ad
vault-plugin-secrets-gcpkms
vault-plugin-secrets-kubernetes
vault-plugin-secrets-kv
vault-plugin-secrets-openldap
vault-plugin-secrets-terraform
each of which needs to be fixed and updated in go.mod here, before this
new check can be added.
* Add subtests
* Add in testing of KV v2, which otherwise doesn't get tested
This is a surprisingly complicated special case
* The database plugin needs special handling as well, and add in help invocations of the builtin backends too
* Fix extra package prefix
* Add changelog
* Update 6 out of 9 plugins to needed new versions
Note, this IS an upgrade despite the apparent version numbers going
down. (That's a consequence of slightly odd release management occurring
in the plugin repositories.)
* Update to deal with code changes since branch originally created
* Perform necessary update of vault-plugin-secrets-kubernetes so that CI checks on PR can run
* Fix another instance of incorrect CreateOperation, for a test-only endpoint
By being hidden behind a Go build constraint, it had evaded notice until
now.
* Add an opportunistic test of sys/internal/specs/openapi too
* Fix ACME tidy to not reference acmeCtx
acmeContext is useful for when we need to reference things with a ACME
base URL, but everything used in tidy doesn't need this URL as it is not
coming from an ACME request.
Refactor tidy to remove references to acmeContext, including dependent
functions in acme_state.go.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Remove spurious log message
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Draft Tidy Acme Test with Backdate Storage + Backdate Sysxsx
* Fixes to ACME tidy testing
Co-authored-by: kitography <khaines@mit.edu>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Correctly set account kid to update account status
Co-authored-by: kitography <khaines@mit.edu>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add TestTidyAcmeWithSafetyBuffer
Co-authored-by: kitography <khaines@mit.edu>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test for disabling tidy operation
Co-authored-by: kitography <khaines@mit.edu>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add acme_account_safety_buffer to auto-tidy config
Resolve: #21872
Co-authored-by: kitography <khaines@mit.edu>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests verifying tidy safety buffers
Co-authored-by: kitography <khaines@mit.edu>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add account status validations and order cleanup tests
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: kitography <khaines@mit.edu>
Co-authored-by: Steve Clark <steven.clark@hashicorp.com>
* imprv: Add a parameter to allow ExtKeyUsage field usage from a role
* chore: Add the changelog entry
* imprv: Reword UI and changelog
* doc: Add allow_role_extkeyusage in parameter list
* imprv: Align variable names with existing fields/codebase
* Add unit test and tweak some labels
---------
Co-authored-by: Steve Clark <steven.clark@hashicorp.com>
* Refactor CRL writing config to passthrough cache
When reading the CRL config via API endpoint, always read through to the
disk, updating the cache in the process. Similarly, when writing to the
CRL config, read first from disk (updating the cache), and on write,
also write back through the cache, providing consistency without the
need to invalidate through markConfigDirty(...).
This will form the basis of the new pattern for config caching.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor ACME writing config to passthrough cache
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
- Updating existing entries in OSS so that this can be backported.
- The proper fix on main going forward it to remove these ENT only
entries and move them into the enterprise only file
* OpenAPI: Separate ListOperation from ReadOperation
Historically, since Vault's ReadOperation and ListOperation both map to
the HTTP GET method, their representation in the generated OpenAPI has
been a bit confusing.
This was partially mitigated some time ago, by making the `list` query
parameter express whether it was required or optional - but only in
a way useful to human readers - the human had to know, for example, that
the schema of the response body would change depending on whether `list`
was selected.
Now that there is an effort underway to automatically generate API
clients from the OpenAPI spec, we have a need to fix this more
comprehensively. Fortunately, we do have a means to do so - since Vault
has opinionated treatment of trailing slashes, linked to operations
being list or not, we can use an added trailing slash on the URL path to
separate list operations in the OpenAPI spec.
This PR implements that, and then fixes an operation ID which becomes
duplicated, with this change applied.
See also hashicorp/vault-client-go#174, a bug which will be fixed by
this work.
* Set further DisplayAttrs in auth/github plugin
To mask out more duplicate read/list functionality, now being separately
generated to OpenAPI client libraries as a result of this change.
* Apply requested changes to operation IDs
I'm not totally convinced its worth the extra lines of code, but
equally, I don't have strong feelings about it, so I'll just make the
change.
* Adjust logic to prevent any possibility of generating OpenAPI paths with doubled final slashes
Even in the edge case of improper use of regex patterns and operations.
* changelog
* Fix TestSudoPaths to pass again... which snowballed a bit...
Once I looked hard at it, I found it was missing several sudo paths,
which led to additional bug fixing elsewhere.
I might need to pull some parts of this change out into a separate PR
for ease of review...
* Fix other tests
* More test fixing
* Undo scope creep - back away from fixing sudo paths not shown as such in OpenAPI, at least within this PR
Just add TODO comments for now.
- Add a little more debug to try and understand how we could get the
following error I saw within GHA
```
path_tidy_test.go:769: Created root and leaf certificate, deleted leaf, but a got a certificate count of 2
```
- Fix some other issues discovered while running the test through
stress locally
