mirrors/vault
1
0
Fork 0
mirror of https://github.com/hashicorp/vault.git synced 2026-05-12 16:16:47 +02:00
Code Issues Projects Releases Wiki Activity

Validate 2 character ISO 3166 codes for the Country role field (#13346) (#13496)

Browse Source 
* Validate 2 character ISO 3166 codes for the Country role field

* stricter validation

* dont validate on GetRole unless it was modified by upgrade.  This should help not error out on existing roles with a bad validation

* Validate, but only warn rather than fail

* fix unit tests that were using invalid country codes

* fix more tests

* lint

* Update changelog/_13346.txt



* simply slice searches

* rephrase changelog

---------

Co-authored-by: Scott Miller <smiller@hashicorp.com>
Co-authored-by: Nick Cabatoff <ncabatoff@hashicorp.com>
This commit is contained in:
Vault Automation 2026-05-08 13:04:21 -06:00 committed by GitHub
parent b9941e1e8d
commit a419e96d4e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 137 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -936,7 +936,7 @@ func generateTestCsr(t *testing.T, keyType certutil.PrivateKeyType, keyBits int)
csrTemplate := x509.CertificateRequest{
Subject: pkix.Name{
Country: []string{"MyCountry"},
Country: []string{"MC"},
PostalCode: []string{"MyPostalCode"},
SerialNumber: "MySerialNumber",
CommonName: "my@example.com",
@ -1561,10 +1561,10 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
}
// Country tests
{
roleVals.Country = []string{"foo"}
roleVals.Country = []string{"jp"}
addTests(getCountryCheck(roleVals))
roleVals.Country = []string{"foo", "bar"}
roleVals.Country = []string{"us", "ca"}
addTests(getCountryCheck(roleVals))
}
// OU tests

24
builtin/logical/pki/ca_util.go
View File

@ -21,11 +21,17 @@ import (
"golang.org/x/crypto/ed25519"
)
func getGenerationParams(sc *storageContext, data *framework.FieldData, isRoot bool) (exported bool, format string, role *issuing.RoleEntry, errorResp *logical.Response) {
type generationParams struct {
exported bool
format string
role *issuing.RoleEntry
}
func getGenerationParams(sc *storageContext, data *framework.FieldData, isRoot bool) (params generationParams, warnings []string, errorResp *logical.Response) {
exportedStr := data.Get("exported").(string)
switch exportedStr {
case "exported":
exported = true
params.exported = true
case "internal":
case "existing":
case "kms":
@ -35,8 +41,8 @@ func getGenerationParams(sc *storageContext, data *framework.FieldData, isRoot b
return
}
format = getFormat(data)
if format == "" {
params.format = getFormat(data)
if params.format == "" {
errorResp = logical.ErrorResponse(
`the "format" path parameter must be "pem", "der", or "pem_bundle"`)
return
@ -48,7 +54,12 @@ func getGenerationParams(sc *storageContext, data *framework.FieldData, isRoot b
return
}
role = &issuing.RoleEntry{
country := data.Get("country").([]string)
if err := validateCountry(country); err != nil {
warnings = append(warnings, err.Error())
}
role := &issuing.RoleEntry{
TTL: time.Duration(data.Get("ttl").(int)) * time.Second,
KeyType: keyType,
KeyBits: keyBits,
@ -65,7 +76,7 @@ func getGenerationParams(sc *storageContext, data *framework.FieldData, isRoot b
AllowedUserIDs: []string{"*"},
OU: data.Get("ou").([]string),
Organization: data.Get("organization").([]string),
Country: data.Get("country").([]string),
Country: country,
Locality: data.Get("locality").([]string),
Province: data.Get("province").([]string),
StreetAddress: data.Get("street_address").([]string),
@ -74,6 +85,7 @@ func getGenerationParams(sc *storageContext, data *framework.FieldData, isRoot b
CNValidations: []string{"disabled"},
KeyUsage: data.Get("key_usage").([]string),
}
params.role = role
*role.AllowWildcardCertificates = true
if role.KeyBits, err = certutil.ValidateDefaultOrValueKeyType(role.KeyType, role.KeyBits); err != nil {

16
builtin/logical/pki/cert_util.go
View File

@ -18,6 +18,7 @@ import (
"strconv"
"strings"
"time"
"unicode"
"github.com/hashicorp/vault/builtin/logical/pki/issuing"
"github.com/hashicorp/vault/builtin/logical/pki/parsing"
@ -769,3 +770,18 @@ func (i CertNotAfterInputFromFieldData) GetTTL() int {
func (i CertNotAfterInputFromFieldData) GetOptionalNotAfter() (interface{}, bool) {
return i.data.GetOk("not_after")
}
func validateCountry(country []string) error {
for _, c := range country {
if len(c) != 2 {
return fmt.Errorf("country must be a 2 character ISO 3166 code, have: %v", c)
}
for _, cc := range c {
cc = unicode.ToUpper(cc)
if cc < 'A' || cc > 'Z' {
return fmt.Errorf("country code characters must be between A and Z, have: %v", c)
}
}
}
return nil
}

32
builtin/logical/pki/path_generate_root_test.go
View File

@ -10,21 +10,23 @@ import (
"github.com/stretchr/testify/require"
)
// TestGenerateRoot_MaxPathLengthValidation validates the behavior of the max_path_length
// parameter on the POST /root/generate/:exported API handler.
//
// The handler logic (pathCAGenerateRoot in path_root.go) is:
// - If max_path_length is not provided in the request, the field is left
// unset on the role and the certificate generation code picks a default
// (no BasicConstraints pathLenConstraint, i.e. MaxPathLen == -1 in Go's
// x509 representation).
// - If max_path_length is provided and is < -1, the request is rejected with
// an error.
// - If max_path_length is provided and is >= -1, the generated certificate
// will carry that pathLenConstraint.
// - When the generated certificate has MaxPathLen == 0 (pathLenConstraint=0),
// Vault adds a warning that the certificate cannot be used to issue
// intermediate CAs.
// TestGenerateRoot_InvalidCountry validates that ISO 3166 is followed for the Country field
func TestGenerateRoot_InvalidCountry(t *testing.T) {
t.Parallel()
b, s := CreateBackendWithStorage(t)
params := map[string]interface{}{
"common_name": "root.example.com",
"ttl": "87600h",
"key_type": "ec",
"country": "Japan",
}
resp, err := CBWrite(b, s, "root/generate/internal", params)
require.NoError(t, err)
require.True(t, stringSliceContainsAny(resp.Warnings, "3166"))
}
func TestGenerateRoot_MaxPathLengthValidation(t *testing.T) {
t.Parallel()

32
builtin/logical/pki/path_intermediate.go
View File

@ -121,19 +121,25 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req
}
data.Raw["use_pss"] = false
resp := &logical.Response{
Data: map[string]interface{}{},
}
sc := b.makeStorageContext(ctx, req.Storage)
exported, format, role, errorResp := getGenerationParams(sc, data, false)
genParams, warnings, errorResp := getGenerationParams(sc, data, false)
if errorResp != nil {
return errorResp, nil
}
if len(warnings) > 0 {
resp.Warnings = append(resp.Warnings, warnings...)
}
keyName, err := getKeyName(sc, data)
if err != nil {
return logical.ErrorResponse(err.Error()), nil
}
var resp *logical.Response
input := &inputBundle{
role: role,
role: genParams.role,
req: req,
apiData: data,
}
@ -153,10 +159,6 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req
return nil, fmt.Errorf("error converting raw CSR bundle to CSR bundle: %w", err)
}
resp = &logical.Response{
Data: map[string]interface{}{},
}
entries, err := getGlobalAIAURLs(ctx, req.Storage)
if err == nil && len(entries.OCSPServers) == 0 && len(entries.IssuingCertificates) == 0 &&
len(entries.CRLDistributionPoints) == 0 && len(entries.DeltaCRLDistributionPoints) == 0 {
@ -170,17 +172,17 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req
resp.AddWarning("This mount has configured delta crl distribution points but no base crl distribution points were set.")
}
switch format {
switch genParams.format {
case "pem":
resp.Data["csr"] = csrb.CSR
if exported {
if genParams.exported {
resp.Data["private_key"] = csrb.PrivateKey
resp.Data["private_key_type"] = csrb.PrivateKeyType
}
case "pem_bundle":
resp.Data["csr"] = csrb.CSR
if exported {
if genParams.exported {
resp.Data["csr"] = fmt.Sprintf("%s\n%s", csrb.PrivateKey, csrb.CSR)
resp.Data["private_key"] = csrb.PrivateKey
resp.Data["private_key_type"] = csrb.PrivateKeyType
@ -188,12 +190,12 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req
case "der":
resp.Data["csr"] = base64.StdEncoding.EncodeToString(parsedBundle.CSRBytes)
if exported {
if genParams.exported {
resp.Data["private_key"] = base64.StdEncoding.EncodeToString(parsedBundle.PrivateKeyBytes)
resp.Data["private_key_type"] = csrb.PrivateKeyType
}
default:
return nil, fmt.Errorf("unsupported format argument: %s", format)
return nil, fmt.Errorf("unsupported format argument: %s", genParams.format)
}
if data.Get("private_key_format").(string) == "pkcs8" {
@ -215,9 +217,9 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req
observe.NewAdditionalPKIMetadata("key_id", myKey.ID),
observe.NewAdditionalPKIMetadata("key_name", myKey.Name),
observe.NewAdditionalPKIMetadata("key_type", myKey.PrivateKeyType),
observe.NewAdditionalPKIMetadata("role_name", role.Name),
observe.NewAdditionalPKIMetadata("exported", exported),
observe.NewAdditionalPKIMetadata("type", format))
observe.NewAdditionalPKIMetadata("role_name", genParams.role.Name),
observe.NewAdditionalPKIMetadata("exported", genParams.exported),
observe.NewAdditionalPKIMetadata("type", genParams.format))
return resp, nil
}

19
builtin/logical/pki/path_roles.go
View File

@ -1045,27 +1045,28 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
}
*entry.AllowWildcardCertificates = allowWildcardCertificates.(bool)
warning := ""
var warnings []string
// no_store implies generate_lease := false
if entry.NoStore {
*entry.GenerateLease = false
if data.Get("generate_lease").(bool) {
warning = "mutually exclusive values no_store=true and generate_lease=true were both specified; no_store=true takes priority"
warnings = append(warnings, "mutually exclusive values no_store=true and generate_lease=true were both specified; no_store=true takes priority")
}
} else {
*entry.GenerateLease = data.Get("generate_lease").(bool)
if *entry.GenerateLease {
warning = "it is encouraged to disable generate_lease and rely on PKI's native capabilities when possible; this option can cause Vault-wide issues with large numbers of issued certificates"
warnings = append(warnings, "it is encouraged to disable generate_lease and rely on PKI's native capabilities when possible; this option can cause Vault-wide issues with large numbers of issued certificates")
}
}
userError, warnings, err := validateRole(b, entry, ctx, req.Storage)
userError, vrWarns, err := validateRole(b, entry, ctx, req.Storage)
if err != nil {
return nil, err
}
if warning != "" {
warnings = append(warnings, warning)
if len(vrWarns) > 0 {
warnings = append(warnings, vrWarns...)
}
if userError != "" {
return logical.ErrorResponse(userError), nil
}
@ -1112,6 +1113,11 @@ func validateRole(b *backend, entry *issuing.RoleEntry, ctx context.Context, s l
return fmt.Sprintf("error setting keyBits %v on role for keyType %v: %v", entry.KeyBits, entry.KeyType, err.Error()), nil, nil
}
var warnings []string
if err := validateCountry(entry.Country); err != nil {
warnings = append(warnings, err.Error())
}
if entry.SerialNumberSource != "" &&
entry.SerialNumberSource != "json-csr" &&
entry.SerialNumberSource != "json" {
@ -1142,7 +1148,6 @@ func validateRole(b *backend, entry *issuing.RoleEntry, ctx context.Context, s l
issuer = defaultRef
}
var warnings []string
// Check that the issuers reference set resolves to something
if !b.UseLegacyBundleCaStorage() {
sc := b.makeStorageContext(ctx, s)

33
builtin/logical/pki/path_roles_test.go
View File

@ -219,6 +219,37 @@ func TestPki_RoleKeyUsage(t *testing.T) {
}
}
// TestPki_RoleBadCountry validates that ISO 3166 is followed for the Country field
func TestPki_RoleBadCountry(t *testing.T) {
t.Parallel()
b, storage := CreateBackendWithStorage(t)
roleData := map[string]interface{}{
"allowed_domains": "myvault.com",
"ttl": "5h",
"country": "Japan",
}
roleReq := &logical.Request{
Operation: logical.UpdateOperation,
Path: "roles/testrole",
Storage: storage,
Data: roleData,
}
resp, err := b.HandleRequest(context.Background(), roleReq)
require.NoError(t, err)
require.False(t, resp.IsError())
require.True(t, stringSliceContainsAny(resp.Warnings, "3166"))
roleData["country"] = "3p"
resp, err = b.HandleRequest(context.Background(), roleReq)
require.NoError(t, err)
require.False(t, resp.IsError())
require.True(t, stringSliceContainsAny(resp.Warnings, "country code"))
}
func TestPki_RoleOUOrganizationUpgrade(t *testing.T) {
t.Parallel()
var resp *logical.Response
@ -897,7 +928,7 @@ func TestPki_RolePatch(t *testing.T) {
{
Field: "country",
Before: []string{"US"},
Patched: []string{"USA"},
Patched: []string{"CA"},
},
{
Field: "locality",

30
builtin/logical/pki/path_root.go
View File

@ -141,11 +141,18 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
sc := b.makeStorageContext(ctx, req.Storage)
resp := &logical.Response{
Data: make(map[string]any),
}
// Tell getGenerationParams we are generating a root, so that we can validate signatureBits against KeyType
exported, format, role, errorResp := getGenerationParams(sc, data, true)
genParams, warnings, errorResp := getGenerationParams(sc, data, true)
if errorResp != nil {
return errorResp, nil
}
if len(warnings) > 0 {
resp.Warnings = append(resp.Warnings, warnings...)
}
maxPathLengthIface, ok := data.GetOk("max_path_length")
if ok {
@ -153,7 +160,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
if maxPathLength < -1 {
return logical.ErrorResponse("requested max_path_length %d is invalid: must be a non-negative integer or -1 for no constraint", maxPathLength), nil
}
role.MaxPathLength = &maxPathLength
genParams.role.MaxPathLength = &maxPathLength
}
issuerName, err := getIssuerName(sc, data)
@ -178,7 +185,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
input := &inputBundle{
req: req,
apiData: data,
role: role,
role: genParams.role,
}
b.adjustInputBundle(input)
parsedBundle, warnings, err := generateCert(sc, input, nil, true, b.Backend.GetRandomReader())
@ -196,12 +203,8 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
return nil, fmt.Errorf("error converting raw cert bundle to cert bundle: %w", err)
}
resp := &logical.Response{
Data: map[string]interface{}{
"expiration": int64(parsedBundle.Certificate.NotAfter.Unix()),
"serial_number": cb.SerialNumber,
},
}
resp.Data["expiration"] = int64(parsedBundle.Certificate.NotAfter.Unix())
resp.Data["serial_number"] = cb.SerialNumber
if keyUsages, ok := data.GetOk("key_usage"); ok {
err = validateCaKeyUsages(keyUsages.([]string))
@ -237,11 +240,12 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
resp.AddWarning("This mount has configured Delta CRL distribution points are set but no CRL Distribution Points.")
}
format := genParams.format
switch format {
case "pem":
resp.Data["certificate"] = cb.Certificate
resp.Data["issuing_ca"] = cb.Certificate
if exported {
if genParams.exported {
resp.Data["private_key"] = cb.PrivateKey
resp.Data["private_key_type"] = cb.PrivateKeyType
}
@ -249,7 +253,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
case "pem_bundle":
resp.Data["issuing_ca"] = cb.Certificate
if exported {
if genParams.exported {
resp.Data["private_key"] = cb.PrivateKey
resp.Data["private_key_type"] = cb.PrivateKeyType
resp.Data["certificate"] = fmt.Sprintf("%s\n%s", cb.PrivateKey, cb.Certificate)
@ -260,7 +264,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
case "der":
resp.Data["certificate"] = base64.StdEncoding.EncodeToString(parsedBundle.CertificateBytes)
resp.Data["issuing_ca"] = base64.StdEncoding.EncodeToString(parsedBundle.CertificateBytes)
if exported {
if genParams.exported {
resp.Data["private_key"] = base64.StdEncoding.EncodeToString(parsedBundle.PrivateKeyBytes)
resp.Data["private_key_type"] = cb.PrivateKeyType
}
@ -344,7 +348,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
observe.NewAdditionalPKIMetadata("key_id", myKey.ID),
observe.NewAdditionalPKIMetadata("key_name", myKey.Name),
observe.NewAdditionalPKIMetadata("key_type", myKey.PrivateKeyType),
observe.NewAdditionalPKIMetadata("role_name", role.Name),
observe.NewAdditionalPKIMetadata("role_name", genParams.role.Name),
observe.NewAdditionalPKIMetadata("serial_number", parsing.SerialFromCert(parsedBundle.Certificate)),
observe.NewAdditionalPKIMetadata("type", format),
observe.NewAdditionalPKIMetadata("common_name", parsedBundle.Certificate.Subject.CommonName),

4
builtin/logical/pki/storage_test.go
View File

@ -263,7 +263,7 @@ func genCertBundle(t *testing.T, b *backend, s logical.Storage) *certutil.CertBu
},
}
sc := b.makeStorageContext(ctx, s)
_, _, role, respErr := getGenerationParams(sc, apiData, true)
genParams, _, respErr := getGenerationParams(sc, apiData, true)
require.Nil(t, respErr)
input := &inputBundle{
@ -273,7 +273,7 @@ func genCertBundle(t *testing.T, b *backend, s logical.Storage) *certutil.CertBu
Storage: s,
},
apiData: apiData,
role: role,
role: genParams.role,
}
parsedCertBundle, _, err := generateCert(sc, input, nil, true, b.GetRandomReader())

3
changelog/_13346.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
secrets/pki: Warn if the Country field on roles and when generating CAs is not ISO 3166 compliant
```