* Fix OpenAPI spec definitions for PKI EAB APIs
- Do not generate duplicate operation ids for the various new-eab apis
- Fill out proper operation verb for eab delete call
- Pluralize operation verb for list-eab-keys api
- Fill out proper response data for new-eab and list-eab-keys
* Add cl
* openapi: Fix response schema for PKI Issue requests
* tests
* changelog
* another expiration for generate/rotate root
* more type fixes from @stevendpclark
The upcoming event main plugin will use a very similar pattern
as the database plugin map, so it makes sense to refactor this and move
this map out. It also cleans up the database plugin backend so that
it does not have to keep track of the lock.
Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>
* Replace all time.ParseDurations with testutil.ParseDurationSeconds
* Changelog
* Import formatting
* Import formatting
* Import formatting
* Import formatting
* Semgrep rule that runs as part of CI
* Supporting PR for Enterprise ACME PR cluster tests
- Some changes within the OSS test helpers to help in the ACME Enterprise test cases.
* Don't rename existing helper method to make oss/ent merge easier
* Adds automated ACME tests using Caddy.
* Do not use CheckSignatureFrom method to validate TLS-ALPN-01 challenges
* Uncomment TLS-ALPN test.
* Fix validation of tls-alpn-01 keyAuthz
Surprisingly, this failure was not caught by our earlier, but unmerged
acme.sh tests:
> 2023-06-07T19:35:27.6963070Z [32mPASS[0m builtin/logical/pkiext/pkiext_binary.Test_ACME/group/acme.sh_tls-alpn (33.06s)
from https://github.com/hashicorp/vault/pull/20987.
Notably, we had two failures:
1. The extension's raw value is not used, but is instead an OCTET
STRING encoded version:
> The extension has the following ASN.1 [X.680] format :
>
> Authorization ::= OCTET STRING (SIZE (32))
>
> The extnValue of the id-pe-acmeIdentifier extension is the ASN.1
> DER encoding [X.690] of the Authorization structure, which
> contains the SHA-256 digest of the key authorization for the
> challenge.
2. Unlike DNS, the SHA-256 is directly embedded in the authorization,
as evidenced by the `SIZE (32)` annotation in the quote above: we
were instead expecting this to be url base-64 encoded, which would
have a different size.
This failure was caught by Matt, testing with Caddy. :-)
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Quick gofmt run.
* Fix challenge encoding in TLS-ALPN-01 challenge tests
* Rename a PKI test helper that retrieves the Vault cluster listener's cert to distinguish it from the method that retrieves the PKI mount's CA cert. Combine a couple of Docker file copy commands into one.
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: Steve Clark <steven.clark@hashicorp.com>
Co-authored-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Ignore templated AIA on root generation
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test case verifying that roots generate OK
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add warning on generation
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Correctly validate ACME PoP against public key
ACME's proof of possession based revocation uses a signature from the
private key, but only sends the public copy along with the request.
Ensure the public copy matches the certificate, instead of failing to
cast it to a private key.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add ACME revocation tests
* Clarify commentary in acmeRevocationByPoP
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: Steve Clark <steven.clark@hashicorp.com>
* Allow CSRs with basic constraint extension with IsCA=false
- We previously forbid any CSR with a basic constraint extension within the CSR.
- It was discovered that some ACME clients (Proxmox ACME client) do send us this extension with a value of IsCA to false.
- So allow the extension to be set within the ACME CSR with
a value of IsCA set to false
- Add a test for both the IsCA=true and IsCA=false use-cases and make sure we don't actually set the extension back within the generated certificate.
* PR feedback
- Move basic constraint function to sdk, increase test coverage
- Error out on extra characters being returned from the asn1 unmarshalling.
* make fmt
- When running the SubtestACMEStepDownNode by itself we would be sealing the active node within the cluster too quickly and would end up with the other nodes failing to become an active node with the message: not part of stable configuration, aborting election
- Add an extra check that the raft autopilot state is healthy and that FailureToTolerance has a value of 1 or higher before letting the test continue.
* Signal ACME challenge engine if existing challenges were loaded
- Addresses an issue of existing challenges on disk not being processed until a new challenge is accepted when Vault restarts
- Move loading of existing challenges from the plugin's initialize method into the challenge engine's thread
- Add docker test that validates we addressed the issue and ACME works across standby nodes.
* Add cl
* Adds an ACME validation failure test for certbot that doesn't run in CI unless a particular regression test env var is provided. Also includes a helper function to determine whether or not CI is running and if the regression test env var is provided.
* Rename and move the local or regression test env check. Sinkhole our invalid domain for ACME certbot test to avoid spamming someone's domain if it's registered in the future.
* Add ACME TLS-ALPN-01 Challenge validator to PKI
This adds support for verifying the last missing challenge type,
TLS-ALPN-01 challenges, using Go's TLS library. We wish to add this as
many servers (such as Caddy) support transparently renewing certificates
via this protocol, without influencing the contents of sites served.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Enable suggesting, validating tls-alpn-01 in PKI
Notably, while RFC 8737 is somewhat vague about what identifier types
can be validated with this protocol, it does restrict SANs to be only
DNSSans; from this, we can infer that it is not applicable for IP
typed identifiers. Additionally, since this must resolve to a specific
domain name, we cannot provision it for wildcard identifiers either.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix test expectations to allow ALPN challenges
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tls-alpn-01 as a supported challenge to docs
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test for tls-alpn-01 challenge verifier
This hacks the challenge engine to allow non-standard (non-443) ports,
letting us use a local server listener with custom implementation.
In addition to the standard test cases, we run:
- A test with a longer chain (bad),
- A test without a DNSSan (bad),
- A test with a bad DNSSan (bad),
- A test with some other SANs (bad),
- A test without a CN (good),
- A test without any leaf (bad), and
- A test without the extension (bad).
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update builtin/logical/pki/acme_challenges.go
Co-authored-by: Alexander Scheel <alex.scheel@hashicorp.com>
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: Kit Haines <khaines@mit.edu>
* Allow not specifying CN on CSR
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test case validating behavior
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add notice about failure to validate
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
When writing DNS configs, make sure to push the zone file prior to
writing the reference to the zone in the named.conf.options file.
Otherwise, when adding the initial domain (or any subsequent domains,
which isn't really used by these tests), a race occurs between Docker,
writing the updated config files, and the bind daemon, detecting that
mtime has changed on the named.conf.options file and reloading it
and any referenced zone files.
This should fix the error seen in some tests:
> /etc/bind/named.conf:9: parsing failed: file not found
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Limit ACME issued certificates to a max of 90 days
- If the normal NotAfter date that is calculated from the mount/issuer/role TTL values is greater than 90 days, override the TTL value to a 90-day value.
* Add changelog
* Generate ACME EAB tokens that do not start with -
- To avoid people having issues copying EAB tokens and using them on command lines when they start with - from the base64 encoded values, append a prefix.
- Remove the key_bits data from the eab api, not really useful and now technically wrong
- Fix up some issues with tests not running in parallel.
- Update docs to reflect new EAB apis.
* Add ACME directory to the various EAB output APIs
* Update EAB token prefix to be divisable by 3
- Our decoded prefix was not divisable by 3, which meant the last
character might be tweaked by the rest of the input
* Attempt to resolve flaky test TestAcmeBasicWorkflow test
- Since we can't control the challenge engine, flush the validation records it leverages prior to manually updating the authorization/challenge statuses
```
path_acme_test.go:261: csr: &{[] [] [] [] 0 [] 0 0 <nil> CN=*.localdomain [] [] [] [localhost.localdomain *.localdomain] [] [] []}
path_acme_test.go:300:
Error Trace: /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/builtin/logical/pki/path_acme_test.go:300
Error: Received unexpected error:
403 urn:ietf:params:acme:error:orderNotReady: The request attempted to finalize an order that is not ready to be finalized: order is status pending, needs to be in ready state
Test: TestAcmeBasicWorkflow/role
Messages: failed finalizing order
```
* make fmt
* Change from default_role to default_directory_policy to allow future improvements.
* Helper functions
* Use the helper function and make fmt.
* Do not allow the zero-length role "".
* Semgrep doesn't like shadowing errors that are impossible to hit, so fix that.
* Add default to switch branches.
* Add/fix docs.
* Fix wrong requestedRole
* Fix ACME computed order status
* Return validation errors and status updates for authorizations
- We now populate the error field within challenges with the error results from the challenge
- Update the status of the challenge and authorizations to invalid when we give up on the challenge
- Verify that only a single challenge within a given authorization can be accepted to avoid race conditions.
- Saw a test failure when we generated an EAB key that started with -
```
acme_test.go:249: Certbot Issue Command: [certbot certonly
--no-eff-email --email certbot.client@dadgarcorp.com --eab-kid
0246913b-4382-10fc-bf57-b05f2dad0f13 --eab-hmac-key
-Avt5q_KUWWWL8slYJn_MdmiCA-jzvif6Tpt45gQNR0 --agree-tos --no-verify-ssl
--standalone --non-interactive --server
...
certbot: error: argument --eab-hmac-key: expected one argument
```
* Fix export of HMAC typed keys
When initially implemented, exporting HMAC keys resulted in returning
the unused, internal HMACKey value rather than the main Key value that
is used for HMAC operations.
This is a breaking change.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Consistently handle HMAC keys in keysutil
When generating HMAC-typed keys, set HMACKey = Key consistently, to
allow users of HMAC-typed keys to use them backwards compatibly.
Notably, this could discard the (unused) HMACKey field set today.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test proving export of HMAC keys work
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>
This should help to prevent the issue of missing tidy configurations
in the future, by placing all related configuration options at the
top with common validation logic.
However, short from this approach is ensuring that each config option
can be specified independently. Thus, the test allows (for any added
and properly tracked tidy operations) verifying that we have enabled
proper storage/retention of that attribute.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add WrapKey capabilities to keysutil
This allows one keysutil to wrap another key, assuming that key has an
type matching one of keysutil's allowed KeyTypes. This allows completing
the BYOK import loop with Transit, allowing imported wrapping keys to
export (wrap) other keys in transit, without having them leave in
plaintext.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add /byok-export/:dst/:src[/:version] to Transit
Still respecting exportable, we allow encrypted-only export of transit
keys to another cluster using the BYOK semantics.
In particular, this allows an operator to securely establish key
material between two separate Transit installations. This potentially
allows one cluster to be used as a source cluster (encrypting a large
amount of data) and a second cluster to decrypt this data later. This
might be useful in hybrid or site-specific deployments of Vault for
instance.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add missing dependency to sdk/, vault/
Also updates to a newer version while we're here.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation on BYOK export
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for BYOK export/import
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/transit.mdx
* Update builtin/logical/transit/path_byok.go
Co-authored-by: Matt Schultz <975680+schultz-is@users.noreply.github.com>
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: Matt Schultz <975680+schultz-is@users.noreply.github.com>
* Add Certbot unregister integration test
- Make sure we can unregister ACME accounts through Certbot to provide an additional validation.
* Add vault-crypto as a code owner for builtin/logical/pkiext
* poc: client cert credential type
* go mod tidy
* fix typo
* fix newUserReqToProto
* add changelog
* add newline
* fix changelog
* add test for the client cert generator
* Fix formatting
* unset signing bundle URLs
* set BasicConstraintsValidForNonCA to false
* backdate cert by 30s
* remove empty creation params URLs
* check cert BasicConstraintsValid
* set default key bits in newClientCertificateGenerator
* fix client cert gen test with default values
* Add default for key_type
* fix default key_type
* update test with default key type
* update test
* Update changelog/20425.txt
Co-authored-by: Austin Gebauer <34121980+austingebauer@users.noreply.github.com>
* set default key bits and sig bits
* remove the default for key type ad fix the test
* make fmt + add comments for each exported field
* restart test
---------
Co-authored-by: Austin Gebauer <agebauer@hashicorp.com>
Co-authored-by: Austin Gebauer <34121980+austingebauer@users.noreply.github.com>
* Update EAB management urls underneath pki/eab
- It was decided that for ease of ACLing, the management
paths for EAB apis should be outside of the acme path
prefix
- Delete duplicated tests, rely on the proper cluster
based tests for EAB management.
* Update ACME EAB creation paths to be directory specific
- Make the EAB creation APIs directory specific.
- This commit is still missing the enforcement that
they can be redeemed on a specific path.
* Enforce EAB tokens per ACME directory context like accounts
- Do not allow an EAB from one ACME directory to be used
in another.
- Rework the ACME directory function to simply get the path from the request instead of parsing out the role/issuer name.
- Add some commentary around expectations if operators change issuer names
* Add an EAB certbot integration test
- Verify with the 3rd party certbot cli that our EAB workflow works as expected.
* Fix unit test
- Unit test wasn't setting up r.Path within the request
that we now use to determine the acme directory.
* ACME override issuer's leaf_not_after_behavior to truncate
- To provide a better ACME experience as we don't allow clients to specify TTL times, we will override the issuer's leaf_not_after_behavior setting to 'truncate' if set to the default of 'err' and issue the certificate truncated to the issuer's NotAfter time.
* Only allow ServerAuth ExtKeyUsage from ACME certificates
- Add an enforcement to ACME issued certificates that the only ExtKeyUsage we currently allow is the ServerAuth usage.
* Force ServerAuth as the ExtKeyUsage in ACME roles
- Override a role's values related to ExtKeyUsage when
running in ACME mode to only return the ServerAuth usage.
- We do this as the majority of roles out there will most likely have the ClientAuth set to true which will cause friction using ACME.
* Fix error handling in ACME
- If we don't match a specific ACME error, use ErrServerInternal instead of the last error type from the internal map
- Logger parameters need two params
* Enforce cluster local path is set when enabling ACME
* Add a warning on ACME config read api if enabled but path not set
- This might help expose that the local path configuration on a secondary cluster was not set which would prevent ACME from running.
* Allow public keys to be always exported
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Simplify import key version semantics
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add tests for updated ImportVersion semantics
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix handling of RSA private key pairs
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
---------
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix various EAB related issues
- List API wasn't plumbed through properly so it did not work as expected
- Use random 32 bytes instead of an EC key for EAB key values
- Update OpenAPI definitions
* Clean up unused EAB keys within tidy
* Move Vault EAB creation path to pki/acme/new-eab
* Update eab vault responses to match up with docs
