scottk/vault
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/vault.git synced 2025-10-29 09:42:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Gracefully handle CSRs without CNs (#20982)

Browse Source 
* 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>
This commit is contained in:
Alexander Scheel
2023-06-07 08:15:19 -04:00
committed by GitHub
parent 9f87bcfb3b
commit 08c1efa4d4
3 changed files with 39 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
builtin/logical/pki/acme_challenge_engine.go
View File

@@ -14,6 +14,8 @@ var MaxChallengeTimeout = 1 * time.Minute
const MaxRetryAttempts = 5
const ChallengeAttemptFailedMsg = "this may occur if the validation target was misconfigured: check that challenge responses are available at the required locations and retry."
type ChallengeValidation struct {
// Account KID that this validation attempt is recorded under.
Account string `json:"account"`
@@ -401,7 +403,7 @@ func (ace *ACMEChallengeEngine) _verifyChallenge(sc *storageContext, id string,
valid, err = ValidateHTTP01Challenge(authz.Identifier.Value, cv.Token, cv.Thumbprint, config)
if err != nil {
err = fmt.Errorf("%w: error validating http-01 challenge %v: %s", ErrIncorrectResponse, id, err.Error())
err = fmt.Errorf("%w: error validating http-01 challenge %v: %v; %v", ErrIncorrectResponse, id, err, ChallengeAttemptFailedMsg)
return ace._verifyChallengeRetry(sc, cv, authzPath, authz, challenge, err, id)
}
case ACMEDNSChallenge:
@@ -412,7 +414,7 @@ func (ace *ACMEChallengeEngine) _verifyChallenge(sc *storageContext, id string,
valid, err = ValidateDNS01Challenge(authz.Identifier.Value, cv.Token, cv.Thumbprint, config)
if err != nil {
err = fmt.Errorf("%w: error validating dns-01 challenge %v: %s", ErrIncorrectResponse, id, err.Error())
err = fmt.Errorf("%w: error validating dns-01 challenge %v: %v; %v", ErrIncorrectResponse, id, err, ChallengeAttemptFailedMsg)
return ace._verifyChallengeRetry(sc, cv, authzPath, authz, challenge, err, id)
}
default:

34
builtin/logical/pki/path_acme_order.go
View File

@@ -11,6 +11,7 @@ import (
"net"
"net/http"
"sort"
"strings"
"time"
"github.com/hashicorp/vault/sdk/helper/strutil"
@@ -480,6 +481,34 @@ func storeCertificate(sc *storageContext, signedCertBundle *certutil.ParsedCertB
return nil
}
func maybeAugmentReqDataWithSuitableCN(ac *acmeContext, csr *x509.CertificateRequest, data *framework.FieldData) {
// Role doesn't require a CN, so we don't care.
if !ac.role.RequireCN {
return
}
// CSR contains a CN, so use that one.
if csr.Subject.CommonName != "" {
return
}
// Choose a CN in the order wildcard -> DNS -> IP -> fail.
for _, name := range csr.DNSNames {
if strings.Contains(name, "*") {
data.Raw["common_name"] = name
return
}
}
if len(csr.DNSNames) > 0 {
data.Raw["common_name"] = csr.DNSNames[0]
return
}
if len(csr.IPAddresses) > 0 {
data.Raw["common_name"] = csr.IPAddresses[0].String()
return
}
}
func issueCertFromCsr(ac *acmeContext, csr *x509.CertificateRequest) (*certutil.ParsedCertBundle, issuerID, error) {
pemBlock := &pem.Block{
Type: "CERTIFICATE REQUEST",
@@ -495,6 +524,11 @@ func issueCertFromCsr(ac *acmeContext, csr *x509.CertificateRequest) (*certutil.
Schema: getCsrSignVerbatimSchemaFields(),
}
// XXX: Usability hack: by default, minimalist roles have require_cn=true,
// but some ACME clients do not provision one in the certificate as modern
// (TLS) clients are mostly verifying against server's DNS SANs.
maybeAugmentReqDataWithSuitableCN(ac, csr, data)
signingBundle, issuerId, err := ac.sc.fetchCAInfoWithIssuer(ac.issuer.ID.String(), IssuanceUsage)
if err != nil {
return nil, "", fmt.Errorf("failed loading CA %s: %w", ac.issuer.ID.String(), err)

2
builtin/logical/pki/path_acme_test.go
View File

@@ -784,7 +784,7 @@ func TestAcmeIgnoresRoleExtKeyUsage(t *testing.T) {
"allowed_domains": "localdomain",
"allow_subdomains": "true",
"allow_wildcard_certificates": "true",
"require_cn": "false",
"require_cn": "true", /* explicit default */
"server_flag": "true",
"client_flag": "true",
"code_signing_flag": "true",