* 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 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 allowed_uri_sans_template
Enables identity templating for the allowed_uri_sans field in PKI cert roles.
Implemented as suggested in #8509
* changelog++
* Update docs with URI SAN templating
* return pkcs8 format for ed25519 curve
convertRespToPKCS8 does not recognize the ed25519 key. Changes
to recognize ed25519 key and return its PKCS8 format
* update azure instructions
Update instructions in regards to azure AD Authentication and OIDC
* Initial pass of ed25519
* Fix typos on marshal function
* test wip
* typo
* fix tests
* missef changelog
* fix mismatch between signature and algo
* added test coverage for ed25519
* remove pkcs1 since does not exist for ed25519
* add ed25519 support to getsigner
* pull request feedback
Signed-off-by: Anner J. Bonilla <abonilla@hoyosintegrity.com>
* typo on key
Signed-off-by: Anner J. Bonilla <abonilla@hoyosintegrity.com>
* cast mistake
Signed-off-by: Anner J. Bonilla <abonilla@hoyosintegrity.com>
Co-authored-by: Jim Kalafut <jkalafut@hashicorp.com>
This change adds the ability to set the signature algorithm of the
CAs that Vault generates and any certificates it signs. This is a
potentially useful stepping stone for a SHA3 transition down the line.
Summary:
* Adds the field "signature_bits" to CA and Sign endpoints
* Adds support for SHA256, SHA384 and SHA512 signatures on EC and RSA
keytypes.
If a CSR contains a SAN of type otherName, encoded in UTF-8, and the signing role specifies use_csr_sans, the otherName SAN will be included in the signed cert's SAN extension.
Allow single star in allowed_other_sans to match any OtherName. Update documentation to clarify globbing behaviour.
Break dataBundle into two pieces: inputBundle, which contains data that
is specific to the pki backend, and creationBundle, which is a more
generic bundle of validated inputs given to certificate creation/signing routines.
Move functions that only take creationBundle to certutil and make them public.
The result will still pass gofmtcheck and won't trigger additional
changes if someone isn't using goimports, but it will avoid the
piecemeal imports changes we've been seeing.
* Set allowed OIDs to any value when generaing a CA.
Also, allow utf-8 in addition to utf8 as the OID type specifier, and
allow `*` to specify any OID of a supported type.
* Update PKI docs
* Add ability to configure the NotBefore property of certificates in role api
* Update index.html.md
* converting field to time.Duration
* setting default back to 30s
* renaming the parameter not_before_duration to differentiate between the NotBefore datetime on the cert
* Update description
* Update PKI to natively use time.Duration
Among other things this now means PKI will output durations in seconds
like other backends, instead of as Go strings.
* Add a warning when refusing to blow away an existing root instead of just returning success
* Fix another issue found while debugging this...
The reason it wasn't caught on tests in the first place is that the ttl
and max ttl were only being compared if in addition to a provided csr, a
role was also provided. This was because the check was in the role !=
nil block instead of outside of it. This has been fixed, which made the
problem occur in all sign-verbatim cases and the changes in this PR have
now verified the fix.
* Max role's max_ttl parameter a TypeDurationString like ttl
* Don't clamp values at write time in favor of evaluating at issue time,
as is the current best practice
* Lots of general cleanup of logic to fix missing cases
