Skip to content

Commit

Permalink
Perform validation when issuing or signing certificates.
Browse files Browse the repository at this point in the history
Add environment variable VAULT_DISABLE_ISSUING_VERIFICATION.

Setting VAULT_DISABLE_ISSUING_VERIFICATION=true will disable the cert
issuance/signing verification.
  • Loading branch information
victorr committed Nov 15, 2024
1 parent 52ba156 commit 5e43634
Show file tree
Hide file tree
Showing 7 changed files with 350 additions and 128 deletions.
187 changes: 147 additions & 40 deletions builtin/logical/pki/cert_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"fmt"
"net"
"net/url"
"os"
"reflect"
"strings"
"testing"
Expand Down Expand Up @@ -275,6 +276,111 @@ type parseCertificateTestCase struct {
wantErr bool
}

func TestDisableVerifyCertificateEnvVar(t *testing.T) {

Check failure on line 279 in builtin/logical/pki/cert_util_test.go

View workflow job for this annotation

GitHub Actions / Code checks

Test TestDisableVerifyCertificateEnvVar is missing a go doc
caData := map[string]any{
// Copied from the "full CA" test case of TestParseCertificate,
// with tweaked permitted_dns_domains and ttl
"common_name": "the common name",
"alt_names": "[email protected],[email protected],example.com,www.example.com",
"ip_sans": "1.2.3.4,1.2.3.5",
"uri_sans": "https://example.com,https://www.example.com",
"other_sans": "1.3.6.1.4.1.311.20.2.3;utf8:[email protected]",
"ttl": "3h",
"max_path_length": 2,
"permitted_dns_domains": ".example.com,.www.example.com",
"ou": "unit1, unit2",
"organization": "org1, org2",
"country": "US, CA",
"locality": "locality1, locality2",
"province": "province1, province2",
"street_address": "street_address1, street_address2",
"postal_code": "postal_code1, postal_code2",
"not_before_duration": "45s",
"key_type": "rsa",
"use_pss": true,
"key_bits": 2048,
"signature_bits": 384,
}

roleData := map[string]any{
"allow_any_name": true,
"cn_validations": "disabled",
"allow_ip_sans": true,
"allowed_other_sans": "1.3.6.1.4.1.311.20.2.3;utf8:*@example.com",
"allowed_uri_sans": "https://example.com,https://www.example.com",
"allowed_user_ids": "*",
"not_before_duration": "45s",
"signature_bits": 384,
"key_usage": "KeyAgreement",
"ext_key_usage": "ServerAuth",
"ext_key_usage_oids": "1.3.6.1.5.5.7.3.67,1.3.6.1.5.5.7.3.68",
"client_flag": false,
"server_flag": false,
"policy_identifiers": "1.2.3.4.5.6.7.8.9.0",
}

certData := map[string]any{
// using the same order as in https://developer.hashicorp.com/vault/api-docs/secret/pki#generate-certificate-and-key
"common_name": "the common name non ca",
"alt_names": "[email protected],[email protected],example.com,www.example.com",
"ip_sans": "1.2.3.4,1.2.3.5",
"uri_sans": "https://example.com,https://www.example.com",
"other_sans": "1.3.6.1.4.1.311.20.2.3;utf8:[email protected]",
"ttl": "2h",
// format
// private_key_format
"exclude_cn_from_sans": true,
// not_after
// remove_roots_from_chain
"user_ids": "humanoid,robot",
}

defer func() {
os.Unsetenv("VAULT_DISABLE_ISSUING_VERIFICATION")
}()

b, s := CreateBackendWithStorage(t)

// Create the CA
resp, err := CBWrite(b, s, "root/generate/internal", caData)
require.NoError(t, err)
require.NotNil(t, resp)

// Create the role
resp, err = CBWrite(b, s, "roles/test", roleData)
require.NoError(t, err)
require.NotNil(t, resp)

// Try to create the cert -- should fail verification, since example.com is not allowed
t.Run("no VAULT_DISABLE_ISSUING_VERIFICATION env var", func(t *testing.T) {
resp, err = CBWrite(b, s, "issue/test", certData)
require.ErrorContains(t, err, `DNS name "example.com" is not permitted by any constraint`)
})

// Try to create the cert -- should fail verification, since example.com is not allowed
t.Run("VAULT_DISABLE_ISSUING_VERIFICATION=false", func(t *testing.T) {
os.Setenv("VAULT_DISABLE_ISSUING_VERIFICATION", "false")
resp, err = CBWrite(b, s, "issue/test", certData)
require.ErrorContains(t, err, `DNS name "example.com" is not permitted by any constraint`)
})

// Create the cert, should succeed with the disable env var set
t.Run("VAULT_DISABLE_ISSUING_VERIFICATION=true", func(t *testing.T) {
os.Setenv("VAULT_DISABLE_ISSUING_VERIFICATION", "true")
resp, err = CBWrite(b, s, "issue/test", certData)
require.NoError(t, err)
require.NotNil(t, resp)
})

// Invalid env var
t.Run("invalid VAULT_DISABLE_ISSUING_VERIFICATION", func(t *testing.T) {
os.Setenv("VAULT_DISABLE_ISSUING_VERIFICATION", "invalid")
resp, err = CBWrite(b, s, "issue/test", certData)
require.ErrorContains(t, err, "failed parsing environment variable VAULT_DISABLE_ISSUING_VERIFICATION")
})

}

func TestParseCertificate(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -364,7 +470,7 @@ func TestParseCertificate(t *testing.T) {
"other_sans": "1.3.6.1.4.1.311.20.2.3;utf8:[email protected]",
"ttl": "2h",
"max_path_length": 2,
"permitted_dns_domains": ".example.com,.www.example.com",
"permitted_dns_domains": "example.com,.example.com,.www.example.com",
"ou": "unit1, unit2",
"organization": "org1, org2",
"country": "US, CA",
Expand Down Expand Up @@ -409,7 +515,7 @@ func TestParseCertificate(t *testing.T) {
UsePSS: true,
ForceAppendCaChain: false,
UseCSRValues: false,
PermittedDNSDomains: []string{".example.com", ".www.example.com"},
PermittedDNSDomains: []string{"example.com", ".example.com", ".www.example.com"},
URLs: nil,
MaxPathLength: 2,
NotBeforeDuration: 45 * time.Second,
Expand All @@ -433,7 +539,7 @@ func TestParseCertificate(t *testing.T) {
"serial_number": "",
"ttl": "2h0m45s",
"max_path_length": 2,
"permitted_dns_domains": ".example.com,.www.example.com",
"permitted_dns_domains": "example.com,.example.com,.www.example.com",
"use_pss": true,
"key_type": "rsa",
"key_bits": 2048,
Expand Down Expand Up @@ -532,49 +638,50 @@ func TestParseCertificate(t *testing.T) {
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
b, s := CreateBackendWithStorage(t)

b, s := CreateBackendWithStorage(t)

var cert *x509.Certificate
issueTime := time.Now()
if tt.wantParams.IsCA {
resp, err := CBWrite(b, s, "root/generate/internal", tt.data)
require.NoError(t, err)
require.NotNil(t, resp)
var cert *x509.Certificate
issueTime := time.Now()
if tt.wantParams.IsCA {
resp, err := CBWrite(b, s, "root/generate/internal", tt.data)
require.NoError(t, err)
require.NotNil(t, resp)

certData := resp.Data["certificate"].(string)
cert, err = parsing.ParseCertificateFromString(certData)
require.NoError(t, err)
require.NotNil(t, cert)
} else {
// use the "simple CA" data to create the internal CA
caData := tests[1].data
caData["ttl"] = "3h"
resp, err := CBWrite(b, s, "root/generate/internal", caData)
require.NoError(t, err)
require.NotNil(t, resp)
certData := resp.Data["certificate"].(string)
cert, err = parsing.ParseCertificateFromString(certData)
require.NoError(t, err)
require.NotNil(t, cert)
} else {
// use the "simple CA" data to create the internal CA
caData := tests[1].data
caData["ttl"] = "3h"
resp, err := CBWrite(b, s, "root/generate/internal", caData)
require.NoError(t, err)
require.NotNil(t, resp)

// create a role
resp, err = CBWrite(b, s, "roles/test", tt.roleData)
require.NoError(t, err)
require.NotNil(t, resp)
// create a role
resp, err = CBWrite(b, s, "roles/test", tt.roleData)
require.NoError(t, err)
require.NotNil(t, resp)

// create the cert
resp, err = CBWrite(b, s, "issue/test", tt.data)
require.NoError(t, err)
require.NotNil(t, resp)
// create the cert
resp, err = CBWrite(b, s, "issue/test", tt.data)
require.NoError(t, err)
require.NotNil(t, resp)

certData := resp.Data["certificate"].(string)
cert, err = parsing.ParseCertificateFromString(certData)
require.NoError(t, err)
require.NotNil(t, cert)
}
certData := resp.Data["certificate"].(string)
cert, err = parsing.ParseCertificateFromString(certData)
require.NoError(t, err)
require.NotNil(t, cert)
}

t.Run(tt.name+" parameters", func(t *testing.T) {
testParseCertificateToCreationParameters(t, issueTime, tt, cert)
})
t.Run(tt.name+" fields", func(t *testing.T) {
testParseCertificateToFields(t, issueTime, tt, cert)
t.Run(tt.name+" parameters", func(t *testing.T) {
testParseCertificateToCreationParameters(t, issueTime, tt, cert)
})
t.Run(tt.name+" fields", func(t *testing.T) {
testParseCertificateToFields(t, issueTime, tt, cert)
})
})
}
}
Expand Down
91 changes: 91 additions & 0 deletions builtin/logical/pki/issuing/cert_verify.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package issuing

import (
"context"
"fmt"
ctx509 "github.com/google/certificate-transparency-go/x509"
"github.com/hashicorp/vault/sdk/helper/certutil"
"github.com/hashicorp/vault/sdk/logical"
"os"
"strconv"
"time"
)

// disableVerifyCertificateEnvVar is an environment variable that can be used to disable the
// verification done when issuing or signing certificates that was added by VAULT-22013. It
// is meant as a scape hatch to avoid breaking deployments that the new verification would
// break.
const disableVerifyCertificateEnvVar = "VAULT_DISABLE_ISSUING_VERIFICATION"

func isCertificateVerificationDisabled() (bool, error) {
disableRaw, ok := os.LookupEnv(disableVerifyCertificateEnvVar)
if !ok {
return false, nil
}

disable, err := strconv.ParseBool(disableRaw)
if err != nil {
return false, fmt.Errorf("failed parsing environment variable %s: %w", disableVerifyCertificateEnvVar, err)
}

return disable, nil
}

func VerifyCertificate(ctx context.Context, storage logical.Storage, issuerId IssuerID, parsedBundle *certutil.ParsedCertBundle) error {
if verificationDisabled, err := isCertificateVerificationDisabled(); err != nil {
return err
} else if verificationDisabled {
return nil
}

certChainPool := ctx509.NewCertPool()
for _, certificate := range parsedBundle.CAChain {
cert, err := convertCertificate(certificate.Bytes)
if err != nil {
return err
}
certChainPool.AddCert(cert)
}

// Validation Code, assuming we need to validate the entire chain of constraints

// Note that we use github.com/google/certificate-transparency-go/x509 to perform certificate verification,
// since that library provides options to disable checks that the standard library does not.

options := ctx509.VerifyOptions{
Intermediates: nil, // We aren't verifying the chain here, this would do more work
Roots: certChainPool,
CurrentTime: time.Time{},
KeyUsages: nil,
MaxConstraintComparisions: 0, // This means infinite
DisableTimeChecks: true,
DisableEKUChecks: true,
DisableCriticalExtensionChecks: false,
DisableNameChecks: false,
DisablePathLenChecks: false,
DisableNameConstraintChecks: false,
}

if err := entSetCertVerifyOptions(ctx, storage, issuerId, &options); err != nil {
return err
}

certificate, err := convertCertificate(parsedBundle.CertificateBytes)
if err != nil {
return err
}

_, err = certificate.Verify(options)
return err
}

func convertCertificate(certBytes []byte) (*ctx509.Certificate, error) {
ret, err := ctx509.ParseCertificate(certBytes)
if err != nil {
return nil, fmt.Errorf("cannot convert certificate for validation: %w", err)
}
return ret, nil
}
18 changes: 18 additions & 0 deletions builtin/logical/pki/issuing/issuing_stubs_oss.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

//go:build !enterprise

package issuing

import (
"context"
ctx509 "github.com/google/certificate-transparency-go/x509"
"github.com/hashicorp/vault/sdk/logical"
)

//go:generate go run github.com/hashicorp/vault/tools/stubmaker

func entSetCertVerifyOptions(ctx context.Context, storage logical.Storage, issuerId IssuerID, options *ctx509.VerifyOptions) error {
return nil
}
4 changes: 4 additions & 0 deletions builtin/logical/pki/path_issue_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,10 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d
}
}

if err := issuing.VerifyCertificate(sc.GetContext(), sc.GetStorage(), issuerId, parsedBundle); err != nil {
return nil, err
}

generateLease := false
if role.GenerateLease != nil && *role.GenerateLease {
generateLease = true
Expand Down
3 changes: 3 additions & 0 deletions changelog/28921.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
secrets/pki: Perform certificate validation when issuing or signing leaf certificates.
```
Loading

0 comments on commit 5e43634

Please sign in to comment.