Integrate acme config enable/disable into tests (#20407)

* Add default ACME configuration, invalidate on write

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add enforcment of ACME enabled

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Validate requested role against ACME config

Co-authored-by: kitography <khaines@mit.edu>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add validation of issuer restrictions with ACME

Co-authored-by: kitography <khaines@mit.edu>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add validation around allowed config lenghts

Co-authored-by: kitography <khaines@mit.edu>
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Prune later deemed unnecessary config options

Co-authored-by: kitography <khaines@mit.edu>
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* make fmt

---------

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: kitography <khaines@mit.edu>
Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
This commit is contained in:
Alexander Scheel 2023-04-27 16:31:13 -04:00 committed by GitHub
parent dcff8c2a07
commit 364a639cca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 167 additions and 51 deletions

View File

@ -55,7 +55,12 @@ func (b *backend) acmeWrapper(op acmeOperation) framework.OperationFunc {
return acmeErrorWrapper(func(ctx context.Context, r *logical.Request, data *framework.FieldData) (*logical.Response, error) {
sc := b.makeStorageContext(ctx, r.Storage)
if isAcmeDisabled(sc) {
config, err := sc.Backend.acmeState.getConfigWithUpdate(sc)
if err != nil {
return nil, fmt.Errorf("failed to fetch ACME configuration: %w", err)
}
if isAcmeDisabled(sc, config) {
return nil, ErrAcmeDisabled
}
@ -68,7 +73,7 @@ func (b *backend) acmeWrapper(op acmeOperation) framework.OperationFunc {
return nil, err
}
role, issuer, err := getAcmeRoleAndIssuer(sc, data)
role, issuer, err := getAcmeRoleAndIssuer(sc, data, config)
if err != nil {
return nil, err
}
@ -262,14 +267,19 @@ func getAcmeDirectory(data *framework.FieldData) string {
return fmt.Sprintf("issuer-%s::role-%s", requestedIssuer, requestedRole)
}
func getAcmeRoleAndIssuer(sc *storageContext, data *framework.FieldData) (*roleEntry, *issuerEntry, error) {
func getAcmeRoleAndIssuer(sc *storageContext, data *framework.FieldData, config *acmeConfigEntry) (*roleEntry, *issuerEntry, error) {
requestedIssuer := getRequestedAcmeIssuerFromPath(data)
requestedRole := getRequestedAcmeRoleFromPath(data)
issuerToLoad := requestedIssuer
var wasVerbatim bool
var role *roleEntry
if len(requestedRole) > 0 {
if len(requestedRole) > 0 || len(config.DefaultRole) > 0 {
if len(requestedRole) == 0 {
requestedRole = config.DefaultRole
}
var err error
role, err = sc.Backend.getRole(sc.Context, sc.Storage, requestedRole)
if err != nil {
@ -295,6 +305,26 @@ func getAcmeRoleAndIssuer(sc *storageContext, data *framework.FieldData) (*roleE
NoStore: false,
Name: requestedRole,
})
wasVerbatim = true
}
allowAnyRole := len(config.AllowedRoles) == 1 && config.AllowedRoles[0] == "*"
if !allowAnyRole {
if wasVerbatim {
return nil, nil, fmt.Errorf("%w: using the default directory without specifying a role is not supported by this configuration; specify 'default_role' in the acme config to the default directories", ErrServerInternal)
}
var foundRole bool
for _, name := range config.AllowedRoles {
if name == role.Name {
foundRole = true
break
}
}
if !foundRole {
return nil, nil, fmt.Errorf("%w: specified role not allowed by ACME policy", ErrServerInternal)
}
}
issuer, err := getAcmeIssuer(sc, issuerToLoad)
@ -302,7 +332,25 @@ func getAcmeRoleAndIssuer(sc *storageContext, data *framework.FieldData) (*roleE
return nil, nil, err
}
// TODO: Need additional configuration validation here, for allowed roles/issuers.
allowAnyIssuer := len(config.AllowedIssuers) == 1 && config.AllowedIssuers[0] == "*"
if !allowAnyIssuer {
var foundIssuer bool
for index, name := range config.AllowedIssuers {
candidateId, err := sc.resolveIssuerReference(name)
if err != nil {
return nil, nil, fmt.Errorf("failed to resolve reference for allowed_issuer entry %d: %w", index, err)
}
if candidateId == issuer.ID {
foundIssuer = true
break
}
}
if !foundIssuer {
return nil, nil, fmt.Errorf("%w: specified issuer not allowed by ACME policy", ErrServerInternal)
}
}
return role, issuer, nil
}
@ -325,7 +373,11 @@ func getRequestedAcmeIssuerFromPath(data *framework.FieldData) string {
return requestedIssuer
}
func isAcmeDisabled(sc *storageContext) bool {
func isAcmeDisabled(sc *storageContext, config *acmeConfigEntry) bool {
if !config.Enabled {
return true
}
if disableAcmeRaw := os.Getenv("VAULT_DISABLE_PUBLIC_ACME"); disableAcmeRaw != "" {
disableAcme, err := strconv.ParseBool(disableAcmeRaw)
if err != nil {
@ -335,11 +387,11 @@ func isAcmeDisabled(sc *storageContext) bool {
// The OS environment if true will override any configuration option.
if disableAcme {
// TODO: If EAB is enforced in the configuration, don't mark
// ACME as disabled.
return true
}
}
// TODO: Implement configuration based check here.
return false
}

View File

@ -24,6 +24,11 @@ func TestACMEIssuerRoleLoading(t *testing.T) {
})
require.NoError(t, err)
_, err = CBWrite(b, s, "config/acme", map[string]interface{}{
"enabled": true,
})
require.NoError(t, err)
_, err = CBWrite(b, s, "root/generate/internal", map[string]interface{}{
"common_name": "myvault1.com",
"issuer_name": "issuer-1",

View File

@ -334,6 +334,12 @@ func TestAcmeClusterPathNotConfigured(t *testing.T) {
cluster, client := setupTestPkiCluster(t)
defer cluster.Cleanup()
// Enable ACME but don't set a path.
_, err := client.Logical().WriteWithContext(context.Background(), "pki/config/acme", map[string]interface{}{
"enabled": true,
})
require.NoError(t, err)
// Do not fill in the path option within the local cluster configuration
cases := []struct {
name string
@ -429,6 +435,11 @@ func setupAcmeBackend(t *testing.T) (*vault.TestCluster, *api.Client, string) {
})
require.NoError(t, err)
_, err = client.Logical().WriteWithContext(context.Background(), "pki/config/acme", map[string]interface{}{
"enabled": true,
})
require.NoError(t, err)
// Allow certain headers to pass through for ACME support
_, err = client.Logical().WriteWithContext(context.Background(), "sys/mounts/pki/tune", map[string]interface{}{
"allowed_response_headers": []string{"Last-Modified", "Replay-Nonce", "Link", "Location"},

View File

@ -13,7 +13,7 @@ import (
const (
storageAcmeConfig = "config/acme"
pathConfigAcmeHelpSyn = "Configuration of ACME Endpoints"
pathConfigAcmeHelpDesc = "Here we configure:\n\nenabled=false, whether ACME is enabled, defaults to false meaning that clusters will by default not get ACME support,\nallowed_issuers=\"default\", which issuers are allowed for use with ACME; by default, this will only be the primary (default) issuer,\nallowed_roles=\"*\", which roles are allowed for use with ACME; by default these will be all roles matching our selection criteria,\ndefault_role=\"\", if not empty, the role to be used for non-role-qualified ACME requests; by default this will be empty, meaning ACME issuance will be equivalent to sign-verbatim,\nallow_no_allowed_domains=false, whether ACME will allow the use of roles without any explicit list of allowed domains, and\nallow_any_domain=false, whether ACME will allow the use of roles with allow_any_name=true set."
pathConfigAcmeHelpDesc = "Here we configure:\n\nenabled=false, whether ACME is enabled, defaults to false meaning that clusters will by default not get ACME support,\nallowed_issuers=\"default\", which issuers are allowed for use with ACME; by default, this will only be the primary (default) issuer,\nallowed_roles=\"*\", which roles are allowed for use with ACME; by default these will be all roles matching our selection criteria,\ndefault_role=\"\", if not empty, the role to be used for non-role-qualified ACME requests; by default this will be empty, meaning ACME issuance will be equivalent to sign-verbatim.,\ndns_resolver=\"\", which specifies a custom DNS resolver to use for all ACME-related DNS lookups"
)
type acmeConfigEntry struct {
@ -21,34 +21,48 @@ type acmeConfigEntry struct {
AllowedIssuers []string `json:"allowed_issuers="`
AllowedRoles []string `json:"allowed_roles"`
DefaultRole string `json:"default_role"`
AllowNoAllowedDomains bool `json:"allow_no_allowed_domains"`
AllowAnyDomain bool `json:"allow_any_domain"`
DNSResolver string `json:"dns_resolver"`
}
var defaultAcmeConfig = acmeConfigEntry{
Enabled: false,
AllowedIssuers: []string{"*"},
AllowedRoles: []string{"*"},
DefaultRole: "",
DNSResolver: "",
}
func (sc *storageContext) getAcmeConfig() (*acmeConfigEntry, error) {
entry, err := sc.Storage.Get(sc.Context, storageAcmeConfig)
if err != nil {
return nil, err
}
mapping := &acmeConfigEntry{}
if entry != nil {
if err := entry.DecodeJSON(mapping); err != nil {
return nil, errutil.InternalError{Err: fmt.Sprintf("unable to decode ACME configuration: %v", err)}
}
var mapping acmeConfigEntry
if entry == nil {
mapping = defaultAcmeConfig
return &mapping, nil
}
return mapping, nil
if err := entry.DecodeJSON(&mapping); err != nil {
return nil, errutil.InternalError{Err: fmt.Sprintf("unable to decode ACME configuration: %v", err)}
}
return &mapping, nil
}
func (sc *storageContext) setAcmeConfig(entry *acmeConfigEntry) error {
json, err := logical.StorageEntryJSON(storageAcmeConfig, entry)
if err != nil {
return err
return fmt.Errorf("failed creating storage entry: %w", err)
}
return sc.Storage.Put(sc.Context, json)
if err := sc.Storage.Put(sc.Context, json); err != nil {
return fmt.Errorf("failed writing storage entry: %w", err)
}
sc.Backend.acmeState.markConfigDirty()
return nil
}
func pathAcmeConfig(b *backend) *framework.Path {
@ -68,30 +82,22 @@ func pathAcmeConfig(b *backend) *framework.Path {
"allowed_issuers": {
Type: framework.TypeCommaStringSlice,
Description: `which issuers are allowed for use with ACME; by default, this will only be the primary (default) issuer`,
Default: "default",
Default: "*",
},
"allowed_roles": {
Type: framework.TypeCommaStringSlice,
Description: `which roles are allowed for use with ACME; by default these will be all roles matching our selection criteria`,
Description: `which roles are allowed for use with ACME; by default via '*', these will be all roles including sign-verbatim; when concrete role names are specified, sign-verbatim is not allowed and a default_role must be specified in order to allow usage of the default acme directories under /pki/acme/directory and /pki/issuer/:issuer_id/acme/directory.`,
Default: "*",
},
"default_role": {
Type: framework.TypeString,
Description: `if not empty, the role to be used for non-role-qualified ACME requests; by default this will be empty, meaning ACME issuance will be equivalent to sign-verbatim,`,
Description: `if not empty, the role to be used for non-role-qualified ACME requests; by default this will be empty, meaning ACME issuance will be equivalent to sign-verbatim; must be specified in allowed_roles if non-empty`,
Default: "",
},
"allow_no_allowed_domains": {
Type: framework.TypeBool,
Description: `whether ACME will allow the use of roles without any explicit list of allowed domains`,
Default: false,
},
"allow_any_domain": {
Type: framework.TypeBool,
Description: `whether ACME will allow the use of roles with allow_any_name=true set.`,
},
"dns_resolver": {
Type: framework.TypeString,
Description: `DNS resolver to use for domain resolution on this mount. Defaults to using the default system resolver. Must be in the format <host>:<port>, with both parts mandatory.`,
Default: "",
},
},
@ -132,9 +138,7 @@ func (b *backend) pathAcmeRead(ctx context.Context, req *logical.Request, _ *fra
func genResponseFromAcmeConfig(config *acmeConfigEntry) *logical.Response {
response := &logical.Response{
Data: map[string]interface{}{
"allow_any_domain": config.AllowAnyDomain,
"allowed_roles": config.AllowedRoles,
"allow_no_allowed_domains": config.AllowNoAllowedDomains,
"allowed_issuers": config.AllowedIssuers,
"default_role": config.DefaultRole,
"enabled": config.Enabled,
@ -159,24 +163,22 @@ func (b *backend) pathAcmeWrite(ctx context.Context, req *logical.Request, d *fr
config.Enabled = enabledRaw.(bool)
}
if allowAnyDomainRaw, ok := d.GetOk("allow_any_domain"); ok {
config.AllowAnyDomain = allowAnyDomainRaw.(bool)
}
if allowedRolesRaw, ok := d.GetOk("allowed_roles"); ok {
config.AllowedRoles = allowedRolesRaw.([]string)
if len(config.AllowedRoles) == 0 {
return nil, fmt.Errorf("allowed_roles must take a non-zero length value; specify '*' as the value to allow anything or specify enabled=false to disable ACME entirely")
}
}
if defaultRoleRaw, ok := d.GetOk("default_role"); ok {
config.DefaultRole = defaultRoleRaw.(string)
}
if allowNoAllowedDomainsRaw, ok := d.GetOk("allow_no_allowed_domains"); ok {
config.AllowNoAllowedDomains = allowNoAllowedDomainsRaw.(bool)
}
if allowedIssuersRaw, ok := d.GetOk("allowed_issuers"); ok {
config.AllowedIssuers = allowedIssuersRaw.([]string)
if len(config.AllowedIssuers) == 0 {
return nil, fmt.Errorf("allowed_issuers must take a non-zero length value; specify '*' as the value to allow anything or specify enabled=false to disable ACME entirely")
}
}
if dnsResolverRaw, ok := d.GetOk("dns_resolver"); ok {
@ -195,6 +197,47 @@ func (b *backend) pathAcmeWrite(ctx context.Context, req *logical.Request, d *fr
}
}
allowAnyRole := len(config.AllowedRoles) == 1 && config.AllowedRoles[0] == "*"
if !allowAnyRole {
foundDefault := len(config.DefaultRole) == 0
for index, name := range config.AllowedRoles {
if name == "*" {
return nil, fmt.Errorf("cannot use '*' as role name at index %d", index)
}
role, err := sc.Backend.getRole(sc.Context, sc.Storage, name)
if err != nil {
return nil, fmt.Errorf("failed validating allowed_roles: unable to fetch role: %v: %w", name, err)
}
if role == nil {
return nil, fmt.Errorf("role %v specified in allowed_roles does not exist", name)
}
if name == config.DefaultRole {
foundDefault = true
}
}
if !foundDefault {
return nil, fmt.Errorf("default role %v was not specified in allowed_roles: %v", config.DefaultRole, config.AllowedRoles)
}
}
allowAnyIssuer := len(config.AllowedIssuers) == 1 && config.AllowedIssuers[0] == "*"
if !allowAnyIssuer {
for index, name := range config.AllowedIssuers {
if name == "*" {
return nil, fmt.Errorf("cannot use '*' as issuer name at index %d", index)
}
_, err := sc.resolveIssuerReference(name)
if err != nil {
return nil, fmt.Errorf("failed validating allowed_issuers: unable to fetch issuer: %v: %w", name, err)
}
}
}
err = sc.setAcmeConfig(config)
if err != nil {
return nil, err

View File

@ -164,6 +164,11 @@ func setupAcme(t *testing.T, cluster *tcDocker.DockerCluster, vaultAddr string,
})
require.NoError(t, err)
_, err = client.Logical().Write("pki/config/acme", map[string]interface{}{
"enabled": true,
})
require.NoError(t, err)
// Setup root+intermediate CA hierarchy within this mount.
resp, err := client.Logical().Write("pki/root/generate/internal", map[string]interface{}{
"common_name": "Root X1" + testSuffix,