* Add PKI test for delete role
- Create a role, validate that defaults are what we expect
and delete the role, verifying it is gone on subsequent read
attempts.
* Add PKI test for crl/rotate command
- Missing a unit test that validates the crl/rotate command works. The test validates the rotate command was successful
by checking if we have a different/new update time on the CRL.
* Rework PKI TestBackend_PathFetchValidRaw test to not write directly to storage
- Rework the existing test to not write directly to storage as we might change that in the future.
- Add tests that validate the ca_chain behaviour of not returning the root authority cert
* PR Feedback
* Additional PR feedback
* Correctly handle minimums, default SignatureBits
When using KeyType = "any" on a role (whether explicitly or implicitly
via a sign-verbatim like operation), we need to update the value of
SignatureBits from its new value 0 to a per-key-type default value. This
will allow sign operations on these paths to function correctly, having
the correctly inferred default signature bit length.
Additionally, this allows the computed default value for key type to be
used for minimum size validation in the RSA/ECDSA paths. We additionally
enforce the 2048-minimum in this case as well.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix defaults and validation of "any" KeyType
When certutil is given the placeholder any keytype, it attempts to
validate and update the default zero value. However, in lacking a
default value for SignatureBits, it cannot update the value from the
zero value, thus causing validation to fail.
Add more awareness to the placeholder "any" value to certutil.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add role-based regression tests for key bits
This adds regression tests for Key Type, Key Bits, and Signature Bits
parameters on the role. We test several values, including the "any"
value to ensure it correctly restricts key sizes.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add sign-verbatim test for key type
This ensures that we test sign-verbatim against a variety of key types.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
* Fix semgrep 0.86.5 parsing failures
- semgrep https://github.com/returntocorp/semgrep/pull/4671 seems to have
introduce this parsing failure within version 0.86.0 and higher
- Workaround parsing failure by breaking out the if error check.
* Pin semgrep version to 0.86.5
* Fix formatting issues
The URL password redaction operation did not handle the case where the
database connection URL was provided as a percent-encoded string, and
its password component contained reserved characters. It attempted to
redact the password by replacing the unescaped password in the
percent-encoded URL. This resulted in the password being revealed when
reading the configuration from Vault.
* Misc PKI code fixes.
- Harden the code base a bit adding default's to switch statements
to various error handlers and processing statements.
- Fixup some error messages to include proper values we support.
* Additional default case missing within PKI
* Fix typo in PKI error message
* Add error check when looking up public key info for a managed key within PKI
* Trap use case that an unknown public key is returned to PKI through managed keys
Instead of using the field FieldData.Raw, use method GetOkError() which does
type conversion but still allows to check whether a value for the parameter was
provided. Note that GetOkError() converts nil values to default or zero values,
so, for example, a nil plaintext value will result in the empty string being
encrypted.
* Update description of certificate fetch API
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Clarify /config/crl and /config/url PKI are empty
GET-ing these URLs will return 404 until such time as a config is posted
to them, even though (in the case of CRL), default values will be used.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Clarify usage of /pki/crl/rotate
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update documentation around PKI key_bits
This unifies the description of key_bits to match the API description
(which is consistent across all usages).
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix indented field descriptions in PKI paths
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Clarify documentation around serial_number
Note that this field has no impact on the actual Serial Number field and
only an attribute in the requested certificate's Subject.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix spelling of localdomain
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
As reported by Steve Clark, building an intermediate mount in PKI (and
calling /intermediate/set-signed) results in a duplicate intermediate CA
certificate in the full chain output (ca_chain field of the
/cert/ca_chain API endpoint response).
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add warning when generate_lease=no_store=true
When no_store=true, the value of generate_lease is ignored completely
(and set to false). This means that when generate_lease=true is
specified by the caller of the API, it is silently swallowed. While
changing the behavior could break callers, setting a warning on the
response (changing from a 204->200 in the process) seems to make the
most sense.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
As pointed out internally, a lot of the API docs and FrameworkField
descriptions of parameters were out of date. This syncs a number of
them, updating their descriptions where relevant.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add new AllowWildcardCertificate field to PKI role
This field allows the PKI role to control whether or not issuance of
wildcard certificates are allowed. We default (both on migration and
new role creation) to the less secure true value for backwards
compatibility with existing Vault versions.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Refactor sanitizedName to reducedName
Per comment, this variable name was confusing during the reproduction
and subsequent fix of the earlier vulnerability and associated bug
report. Because the common name isn't necessarily _sanitized_ in any way
(and indeed must be considered in relation to other parts or the whole),
but portions of the entire name are removed, reducedName appears to make
the most sense.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Enforce AllowWildcardCertificates during issuance
This commit adds the bulk of correctly validating wildcard certificate
Common Names during issuance according to RFC 6125 Section 6.4.3
semantics. As part of this, support for RFC 2818-conforming wildcard
certificates (wherein there are almost no restrictions on issuance) has
been removed.
Note that this flag does take precedence over AllowAnyName, giving a
little more safety in wildcard issuance in this case.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update test cases to conform with RFC 6125
Test cases 19, 70+71, and 83+84 didn't conform with the RFC 6125, and so
should've been rejected under strict conformance. For 70+71 and 83+84,
we previously conditioned around the value of AllowSubdomains (allowing
issuance when true), but they likely should've been rejected either way.
Additionally, update the notes about globs matching wildcard
certificates to notate this is indeed the case.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Check AllowWildcardCertifciates in issuance tests
This allows for regression tests to cover the new
AllowWildcardCertificate conditional. We add additional test cases
ensuring that wildcard issuance is properly forbidden in all relevant
scenarios, while allowing the existing test cases to validate that
wildcard status doesn't affect non-wildcard certificates.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add Wildcard allowance during signing operations
When using sign-verbatim, sign-intermediate, or getting certificate
generation parameters, set AllowWildcardCertificates to mirror existing
policies.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow issuance of wildcard via glob match
From Vault v1.8.0 onwards, we would incorrectly disallow issuance of a
wildcard certificate when allow_glob_domain was enabled with a
multi-part glob domain in allowed_domains (such as *.*.foo) when
attempting to issue a wildcard for a subdomain (such as *.bar.foo).
This fixes that by reverting an errant change in the case insensitivity
patch. Here, when validating against a very powerful glob construct, we
leave the wildcard prefix (*.) from the raw common_name element, to
allow multi-part globs to match wildcard entries.
It is important to note that "sanitizedName" is an incorrect variable
naming here. Wildcard parsing (per RFC 6125 which supercedes RFC 2818)
must be in the left-most segment of the domain, but we lack validation
to ensure no internal wildcards exist. Additionally per item 3 of
section 6.4.3 of RFC 6125, wildcards MAY be internal to a domain
segment, in which case sanitizedName again leaves the wildcard in place.
Resolves: #13530
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Remove duplicate email address check
As pointed out by Steven Clark (author of the removed conditional in
70012cd865b3dcdab376dba0c0e0abc88c48f508), this is duplicate from the
now-reintroduced comparison against name (versus the erroneous
sanitizedName at the time of his commit).
This is a reversion of the changes to builtin/logical/pki/cert_util.go,
but keeping the additional valuable test cases.
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add multi-dimensional PKI role issuance tests
This commit introduces multi-dimensional testing of PKI secrets engine's
role-based certificate issuance with the intent of preventing future
regressions.
Here, dimensions of testing include:
- AllowedDomains to decide which domains are approved for issuance,
- AllowBareDomains to decide if raw entries of AllowedDomains are
permitted,
- AllowGlobDomains to decide if glob patterns in AllowedDomains are
parsed,
- AllowSubdomains to decide if subdomains of AllowedDomains are
permitted,
- AllowLocalhost to decide if localhost identifiers are permitted, and
- CommonName of the certificate to request.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
* Add checks for other error types within the PKI plugin
- The PKI plugin assumes the code it is calling always returns an error
of type errutil.UserError or errutil.InternalError. While I believe
so far this is still true, it would be easy to add a code path that
just returns a generic error and we would completely ignore it.
- This was found within some managed key testing where I forgot to wrap
an error within one of the expected types
* Add changelog
* Allow OpenSSH-style key type identifiers
To bring better parity with the changes of #14008, wherein we allowed
OpenSSH-style key identifiers during generation. When specifying a list
of allowed keys, validate against both OpenSSH-style key identifiers
and the usual simplified names as well ("rsa" or "ecdsa"). Notably, the
PKI secrets engine prefers "ec" over "ecdsa", so we permit both as well.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix missing quote in docs
* Explicitly call out SSH algorithm_signer default
Related: #11608
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Use rsa-sha2-256 as the default SSH CA hash algo
As mentioned in the OpenSSH 8.2 release notes, OpenSSH will no longer be
accepting ssh-rsa signatures by default as these use the insecure SHA-1
algorithm.
For roles in which an explicit signature type wasn't specified, we
should change the default from SHA-1 to SHA-256 for security and
compatibility with modern OpenSSH releases.
See also: https://www.openssh.com/txt/release-8.2
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update docs mentioning new algorithm change
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Fix missing parenthesis, clarify new default value
* Add to side bar
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* plugin/catalog: support plugin registration whe type is explicitly provided
* don't use database type on plugin backend test; mock doesn't satisfy the DB interface
* check multiplexing support from plugin directly on newPluginClient
* do not return mutiplexed bool on catalog helper funcs
* Allow specifying multiple allowed SSH key lengths
In the ssh secrets engine, only a single allowed key length was allowed
for each algorithm type. However, many algorithms have multiple safe
values (such as RSA and ECDSA); allowing a single role to have multiple
values for a single algorithm is thus helpful.
On creation or update, roles can now specify multiple types using a list
or comma separated string of allowed values:
allowed_user_key_lengths: map[string][]int{"rsa": []int{2048, 4096}}
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Break out ssh upgrade logic into separate function
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Update parseutil for optional lists of integers
go get -u github.com/hashicorp/go-secure-stdlib/parseutil
go mod tidy
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Simplify parse logic using new parseutil
The newly introduced parseutil.ParseIntSlice handles the more
complicated optional int-like slice logic for us.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* adds development workflow to mirage config
* adds mirage handler and factory for mfa workflow
* adds mfa handling to auth service and cluster adapter
* moves auth success logic from form to controller
* adds mfa form component
* shows delayed auth message for all methods
* adds new code delay to mfa form
* adds error views
* fixes merge conflict
* adds integration tests for mfa-form component
* fixes auth tests
* updates mfa response handling to align with backend
* updates mfa-form to handle multiple methods and constraints
* adds noDefault arg to Select component
* updates mirage mfa handler to align with backend and adds generator for various mfa scenarios
* adds tests
* flaky test fix attempt
* reverts test fix attempt
* adds changelog entry
* updates comments for todo items
* removes faker from mfa mirage factory and handler
* adds number to word helper
* fixes tests
* Revert "Merge branch 'main' into ui/mfa"
This reverts commit 8ee6a6aaa1b6c9ec16b985c10d91c3806819ec40, reversing
changes made to 2428dd6cca07bb41cda3f453619646ca3a88bfd0.
* format-ttl helper fix from main
* feat: DB plugin multiplexing (#13734)
* WIP: start from main and get a plugin runner from core
* move MultiplexedClient map to plugin catalog
- call sys.NewPluginClient from PluginFactory
- updates to getPluginClient
- thread through isMetadataMode
* use go-plugin ClientProtocol interface
- call sys.NewPluginClient from dbplugin.NewPluginClient
* move PluginSets to dbplugin package
- export dbplugin HandshakeConfig
- small refactor of PluginCatalog.getPluginClient
* add removeMultiplexedClient; clean up on Close()
- call client.Kill from plugin catalog
- set rpcClient when muxed client exists
* add ID to dbplugin.DatabasePluginClient struct
* only create one plugin process per plugin type
* update NewPluginClient to return connection ID to sdk
- wrap grpc.ClientConn so we can inject the ID into context
- get ID from context on grpc server
* add v6 multiplexing protocol version
* WIP: backwards compat for db plugins
* Ensure locking on plugin catalog access
- Create public GetPluginClient method for plugin catalog
- rename postgres db plugin
* use the New constructor for db plugins
* grpc server: use write lock for Close and rlock for CRUD
* cleanup MultiplexedClients on Close
* remove TODO
* fix multiplexing regression with grpc server connection
* cleanup grpc server instances on close
* embed ClientProtocol in Multiplexer interface
* use PluginClientConfig arg to make NewPluginClient plugin type agnostic
* create a new plugin process for non-muxed plugins
* feat: plugin multiplexing: handle plugin client cleanup (#13896)
* use closure for plugin client cleanup
* log and return errors; add comments
* move rpcClient wrapping to core for ID injection
* refactor core plugin client and sdk
* remove unused ID method
* refactor and only wrap clientConn on multiplexed plugins
* rename structs and do not export types
* Slight refactor of system view interface
* Revert "Slight refactor of system view interface"
This reverts commit 73d420e5cd2f0415e000c5a9284ea72a58016dd6.
* Revert "Revert "Slight refactor of system view interface""
This reverts commit f75527008a1db06d04a23e04c3059674be8adb5f.
* only provide pluginRunner arg to the internal newPluginClient method
* embed ClientProtocol in pluginClient and name logger
* Add back MLock support
* remove enableMlock arg from setupPluginCatalog
* rename plugin util interface to PluginClient
Co-authored-by: Brian Kassouf <bkassouf@hashicorp.com>
* feature: multiplexing: fix unit tests (#14007)
* fix grpc_server tests and add coverage
* update run_config tests
* add happy path test case for grpc_server ID from context
* update test helpers
* feat: multiplexing: handle v5 plugin compiled with new sdk
* add mux supported flag and increase test coverage
* set multiplexingSupport field in plugin server
* remove multiplexingSupport field in sdk
* revert postgres to non-multiplexed
* add comments on grpc server fields
* use pointer receiver on grpc server methods
* add changelog
* use pointer for grpcserver instance
* Use a gRPC server to determine if a plugin should be multiplexed
* Apply suggestions from code review
Co-authored-by: Brian Kassouf <briankassouf@users.noreply.github.com>
* add lock to removePluginClient
* add multiplexingSupport field to externalPlugin struct
* do not send nil to grpc MultiplexingSupport
* check err before logging
* handle locking scenario for cleanupFunc
* allow ServeConfigMultiplex to dispense v5 plugin
* reposition structs, add err check and comments
* add comment on locking for cleanupExternalPlugin
Co-authored-by: Brian Kassouf <bkassouf@hashicorp.com>
Co-authored-by: Brian Kassouf <briankassouf@users.noreply.github.com>
* Ensure we can issue against generated SSH CA keys
This adds a test to ensure that we can issue leaf SSH certificates using
the newly generated SSH CA keys. Presently this fails because the
ed25519 key private is stored using PKIX's PKCS8 PrivateKey object
format rather than using OpenSSH's desired private key format:
> path_config_ca_test.go:211: bad case 12: err: failed to parse stored CA private key: ssh: invalid openssh private key format, resp: <nil>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add dependency on edkey for OpenSSH ed25519 keys
As mentioned in various terraform-provider-tls discussions, OpenSSH
doesn't understand the standard OpenSSL/PKIX ed25519 key structure (as
generated by PKCS8 marshalling). Instead, we need to place it into the
OpenSSH RFC 8709 format. As mentioned in this dependency's README,
support in golang.org/x/crypto/ssh is presently lacking for this.
When the associated CL is merged, we should be able to remove this dep
and rely on the (extended) standard library, however, no review progress
appears to have been made since the CL was opened by the author.
See also: https://go-review.googlesource.com/c/crypto/+/218620/
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add generation support for other SSH CA key types
This adds two new arguments to config/ca, mirroring the values of PKI
secrets engine but tailored towards SSH mounts. Key types are specified
as x/crypto/ssh KeyAlgo identifiers (e.g., ssh-rsa or ssh-ed25519)
and respect current defaults (ssh-rsa/4096). Key bits defaults to 0,
which for ssh-rsa then takes a value of 4096.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation on key_type, key_bits for ssh/config/ca
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow all other_sans in sign-intermediate and sign-verbatim
/sign-verbatim and /sign-intermediate are more dangerous endpoints in
that they (usually) do not have an associated role. In this case, a
permissive role is constructed during execution of these tests. However,
the AllowedOtherSANs field was missing from this, prohibiting its use
when issuing certificates.
Resolves: #13157
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add duration/count metrics to PKI issue and revoke flows
* docs, changelog
* tidy
* last tidy
* remove err
* Update callsites
* Simple returns
* Handle the fact that test cases don't have namespaces
* Add mount point to the request
* fmt
* Handle empty mount point, and add it to unit tests
* improvement
* Turns out sign-verbatim is tricky, it can take a role but doesn't have to
* Get around the field schema problem
* Use application/pem-certificate-chain for PEMs
As mentioned in #10948, it appears we're incorrectly using the
`application/pkix-cert` media type for PEM blobs, when
`application/x-pem-file` is more appropriate. Per RFC 5280 Section
4.2.1.13, `application/pkix-crl` is only appropriate when the CRL is in
DER form. Likewise, Section 4.2.2.1 states that `application/pkix-cert`
is only applicable when a single DER certificate is used.
Per recommendation in RFC 8555 ("ACME"), Section 7.4.2 and 9.1, we use
the newer `application/pem-certificate-chain` media type for
certificates. However, this is not applicable for CRLs, so we use fall
back to `application/x-pem-file` for these. Notably, no official IETF
source is present for the latter. On the OpenSSL PKI tutorial
(https://pki-tutorial.readthedocs.io/en/latest/mime.html), this type is
cited as coming from S/MIME's predecessor, PEM, but neither of the main
PEM RFCs (RFC 934, 1421, 1422, 1423, or 1424) mention this type.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Include full chain in /cert/ca_chain response
This allows callers to get the full chain (including issuing
certificates) from a call to /cert/ca_chain. Previously, most endpoints
(including during issuance) do not include the root authority, requiring
an explicit call to /cert/ca to fetch. This allows full chains to be
constructed without without needing multiple calls to the API.
Resolves: #13489
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test case for full CA issuance
We test three main scenarios:
1. A root-only CA's `/cert/ca_chain`'s `.data.ca_chain` field should
contain only the root,
2. An intermediate CA (with root provide) should contain both the root
and the intermediate.
3. An external (e.g., `/config/ca`-provided) CA with both root and
intermediate should contain both certs.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add documentation for new ca_chain field
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add changelog entry
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add note about where to find the entire chain
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Support Y10K value in notAfter field when signing non-CA certificates
* Add changelog entry for 13736
* Add test for using not_after parameter for non-CA certificates that are being signed
* Fix CA value for test for not_after value when signing non-CA certs
* Address formatting
* Add changelog file
* Revert changelog entry commit f28b54e7b5ad21144c8a2da942d766e64a332caf
Steven Clark