Change from default_role to default_directory_policy (#20935)

* Change from default_role to default_directory_policy to allow future improvements.

* Helper functions

* Use the helper function and make fmt.

* Do not allow the zero-length role "".

* Semgrep doesn't like shadowing errors that are impossible to hit, so fix that.

* Add default to switch branches.

* Add/fix docs.

* Fix wrong requestedRole
This commit is contained in:
Kit Haines 2023-06-01 18:06:30 -04:00 committed by GitHub
parent 8fe7076c02
commit e63dc30507
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 177 additions and 99 deletions

View File

@ -315,59 +315,62 @@ func getAcmeRoleAndIssuer(sc *storageContext, data *framework.FieldData, config
requestedRole := getRequestedAcmeRoleFromPath(data) requestedRole := getRequestedAcmeRoleFromPath(data)
issuerToLoad := requestedIssuer issuerToLoad := requestedIssuer
var wasVerbatim bool
var role *roleEntry var role *roleEntry
var err error
if len(requestedRole) > 0 || len(config.DefaultRole) > 0 { if len(requestedRole) == 0 { // Default Directory
if len(requestedRole) == 0 { policyType, err := getDefaultDirectoryPolicyType(config.DefaultDirectoryPolicy)
requestedRole = config.DefaultRole
}
var err error
role, err = sc.Backend.getRole(sc.Context, sc.Storage, requestedRole)
if err != nil { if err != nil {
return nil, nil, fmt.Errorf("%w: err loading role", ErrServerInternal) return nil, nil, err
}
switch policyType {
case Forbid:
return nil, nil, fmt.Errorf("%w: default directory not allowed by ACME policy", ErrServerInternal)
case SignVerbatim:
role = buildSignVerbatimRoleWithNoData(&roleEntry{
Issuer: requestedIssuer,
NoStore: false,
Name: requestedRole,
})
case Role:
defaultRole, err := getDefaultDirectoryPolicyRole(config.DefaultDirectoryPolicy)
if err != nil {
return nil, nil, err
}
role, err = getAndValidateAcmeRole(sc, defaultRole)
if err != nil {
return nil, nil, err
}
}
} else { // Requested Role
role, err = getAndValidateAcmeRole(sc, requestedRole)
if err != nil {
return nil, nil, err
} }
if role == nil { // Check the Requested Role is Allowed
return nil, nil, fmt.Errorf("%w: role does not exist", ErrMalformed) allowAnyRole := len(config.AllowedRoles) == 1 && config.AllowedRoles[0] == "*"
} if !allowAnyRole {
if role.NoStore { var foundRole bool
return nil, nil, fmt.Errorf("%w: role can not be used as NoStore is set to true", ErrServerInternal) for _, name := range config.AllowedRoles {
} if name == role.Name {
foundRole = true
break
}
}
// If we haven't loaded an issuer directly from our path and the specified if !foundRole {
// role does specify an issuer prefer the role's issuer rather than the default issuer. return nil, nil, fmt.Errorf("%w: specified role not allowed by ACME policy", ErrServerInternal)
if len(role.Issuer) > 0 && len(requestedIssuer) == 0 {
issuerToLoad = role.Issuer
}
} else {
role = buildSignVerbatimRoleWithNoData(&roleEntry{
Issuer: requestedIssuer,
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)
} // If we haven't loaded an issuer directly from our path and the specified (or default)
// role does specify an issuer prefer the role's issuer rather than the default issuer.
if len(role.Issuer) > 0 && len(requestedIssuer) == 0 {
issuerToLoad = role.Issuer
} }
issuer, err := getAcmeIssuer(sc, issuerToLoad) issuer, err := getAcmeIssuer(sc, issuerToLoad)
@ -406,6 +409,24 @@ func getAcmeRoleAndIssuer(sc *storageContext, data *framework.FieldData, config
return role, issuer, nil return role, issuer, nil
} }
func getAndValidateAcmeRole(sc *storageContext, requestedRole string) (*roleEntry, error) {
var err error
role, err := sc.Backend.getRole(sc.Context, sc.Storage, requestedRole)
if err != nil {
return nil, fmt.Errorf("%w: err loading role", ErrServerInternal)
}
if role == nil {
return nil, fmt.Errorf("%w: role does not exist", ErrMalformed)
}
if role.NoStore {
return nil, fmt.Errorf("%w: role can not be used as NoStore is set to true", ErrServerInternal)
}
return role, nil
}
func getRequestedAcmeRoleFromPath(data *framework.FieldData) string { func getRequestedAcmeRoleFromPath(data *framework.FieldData) string {
requestedRole := "" requestedRole := ""
roleNameRaw, present := data.GetOk("role") roleNameRaw, present := data.GetOk("role")

View File

@ -6,6 +6,7 @@ import (
"net" "net"
"os" "os"
"strconv" "strconv"
"strings"
"github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/helper/errutil" "github.com/hashicorp/vault/sdk/helper/errutil"
@ -15,26 +16,26 @@ import (
const ( const (
storageAcmeConfig = "config/acme" storageAcmeConfig = "config/acme"
pathConfigAcmeHelpSyn = "Configuration of ACME Endpoints" 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.,\ndns_resolver=\"\", which specifies a custom DNS resolver to use for all ACME-related DNS lookups" 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_directory_policy=\"\", either \"forbid\", preventing the default directory from being used at all, \"role:<role_name>\" which is the role to be used for non-role-qualified ACME requests; or \"sign-verbatim\", the default 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"
disableAcmeEnvVar = "VAULT_DISABLE_PUBLIC_ACME" disableAcmeEnvVar = "VAULT_DISABLE_PUBLIC_ACME"
) )
type acmeConfigEntry struct { type acmeConfigEntry struct {
Enabled bool `json:"enabled"` Enabled bool `json:"enabled"`
AllowedIssuers []string `json:"allowed_issuers="` AllowedIssuers []string `json:"allowed_issuers="`
AllowedRoles []string `json:"allowed_roles"` AllowedRoles []string `json:"allowed_roles"`
DefaultRole string `json:"default_role"` DefaultDirectoryPolicy string `json:"default_directory_policy"`
DNSResolver string `json:"dns_resolver"` DNSResolver string `json:"dns_resolver"`
EabPolicyName EabPolicyName `json:"eab_policy_name"` EabPolicyName EabPolicyName `json:"eab_policy_name"`
} }
var defaultAcmeConfig = acmeConfigEntry{ var defaultAcmeConfig = acmeConfigEntry{
Enabled: false, Enabled: false,
AllowedIssuers: []string{"*"}, AllowedIssuers: []string{"*"},
AllowedRoles: []string{"*"}, AllowedRoles: []string{"*"},
DefaultRole: "", DefaultDirectoryPolicy: "sign-verbatim",
DNSResolver: "", DNSResolver: "",
EabPolicyName: eabPolicyNotRequired, EabPolicyName: eabPolicyNotRequired,
} }
func (sc *storageContext) getAcmeConfig() (*acmeConfigEntry, error) { func (sc *storageContext) getAcmeConfig() (*acmeConfigEntry, error) {
@ -91,13 +92,13 @@ func pathAcmeConfig(b *backend) *framework.Path {
}, },
"allowed_roles": { "allowed_roles": {
Type: framework.TypeCommaStringSlice, Type: framework.TypeCommaStringSlice,
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.`, 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, any default_directory_policy role must be included to allow usage of the default acme directories under /pki/acme/directory and /pki/issuer/:issuer_id/acme/directory.`,
Default: []string{"*"}, Default: []string{"*"},
}, },
"default_role": { "default_directory_policy": {
Type: framework.TypeString, 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; must be specified in allowed_roles if non-empty`, Description: `the policy to be used for non-role-qualified ACME requests; by default ACME issuance will be otherwise unrestricted, equivalent to the sign-verbatim endpoint; one may also specify a role to use as this policy, as "role:<role_name>", the specified role must be allowed by allowed_roles`,
Default: "", Default: "sign-verbatim",
}, },
"dns_resolver": { "dns_resolver": {
Type: framework.TypeString, Type: framework.TypeString,
@ -156,12 +157,12 @@ func (b *backend) pathAcmeRead(ctx context.Context, req *logical.Request, _ *fra
func genResponseFromAcmeConfig(config *acmeConfigEntry, warnings []string) *logical.Response { func genResponseFromAcmeConfig(config *acmeConfigEntry, warnings []string) *logical.Response {
response := &logical.Response{ response := &logical.Response{
Data: map[string]interface{}{ Data: map[string]interface{}{
"allowed_roles": config.AllowedRoles, "allowed_roles": config.AllowedRoles,
"allowed_issuers": config.AllowedIssuers, "allowed_issuers": config.AllowedIssuers,
"default_role": config.DefaultRole, "default_directory_policy": config.DefaultDirectoryPolicy,
"enabled": config.Enabled, "enabled": config.Enabled,
"dns_resolver": config.DNSResolver, "dns_resolver": config.DNSResolver,
"eab_policy": config.EabPolicyName, "eab_policy": config.EabPolicyName,
}, },
Warnings: warnings, Warnings: warnings,
} }
@ -190,8 +191,8 @@ func (b *backend) pathAcmeWrite(ctx context.Context, req *logical.Request, d *fr
} }
} }
if defaultRoleRaw, ok := d.GetOk("default_role"); ok { if defaultDirectoryPolicyRaw, ok := d.GetOk("default_directory_policy"); ok {
config.DefaultRole = defaultRoleRaw.(string) config.DefaultDirectoryPolicy = defaultDirectoryPolicyRaw.(string)
} }
if allowedIssuersRaw, ok := d.GetOk("allowed_issuers"); ok { if allowedIssuersRaw, ok := d.GetOk("allowed_issuers"); ok {
@ -226,34 +227,50 @@ func (b *backend) pathAcmeWrite(ctx context.Context, req *logical.Request, d *fr
config.EabPolicyName = eabPolicy.Name config.EabPolicyName = eabPolicy.Name
} }
// Validate Default Directory Behavior:
defaultDirectoryPolicyType, err := getDefaultDirectoryPolicyType(config.DefaultDirectoryPolicy)
if err != nil {
return nil, fmt.Errorf("invalid default_directory_policy: %w", err)
}
defaultDirectoryRoleName := ""
switch defaultDirectoryPolicyType {
case Forbid:
case SignVerbatim:
case Role:
defaultDirectoryRoleName, err = getDefaultDirectoryPolicyRole(config.DefaultDirectoryPolicy)
if err != nil {
return nil, fmt.Errorf("failed extracting role name from default directory policy %w", err)
}
_, err := getAndValidateAcmeRole(sc, defaultDirectoryRoleName)
if err != nil {
return nil, fmt.Errorf("default directory policy role %v is not a valid ACME role: %w", defaultDirectoryRoleName, err)
}
default:
return nil, fmt.Errorf("validation for the type of policy defined by %v is undefined", config.DefaultDirectoryPolicy)
}
// Validate Allowed Roles
allowAnyRole := len(config.AllowedRoles) == 1 && config.AllowedRoles[0] == "*" allowAnyRole := len(config.AllowedRoles) == 1 && config.AllowedRoles[0] == "*"
foundDefault := false
if !allowAnyRole { if !allowAnyRole {
foundDefault := len(config.DefaultRole) == 0
for index, name := range config.AllowedRoles { for index, name := range config.AllowedRoles {
if name == "*" { if name == "*" {
return nil, fmt.Errorf("cannot use '*' as role name at index %d", index) return nil, fmt.Errorf("cannot use '*' as role name at index %d", index)
} }
role, err := sc.Backend.getRole(sc.Context, sc.Storage, name) _, err := getAndValidateAcmeRole(sc, name)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed validating allowed_roles: unable to fetch role: %v: %w", name, err) return nil, fmt.Errorf("allowed_role %v is not a valid acme role: %w", name, err)
} }
if role == nil { if defaultDirectoryPolicyType == Role && name == defaultDirectoryRoleName {
return nil, fmt.Errorf("role %v specified in allowed_roles does not exist", name)
}
if role.NoStore {
return nil, fmt.Errorf("role %v specifies no_store=true; this prohibits usage with ACME which requires stored certificates", name)
}
if name == config.DefaultRole {
foundDefault = true foundDefault = true
} }
} }
if !foundDefault { if !foundDefault && defaultDirectoryPolicyType == Role {
return nil, fmt.Errorf("default role %v was not specified in allowed_roles: %v", config.DefaultRole, config.AllowedRoles) return nil, fmt.Errorf("default directory policy %v was not specified in allowed_roles: %v", config.DefaultDirectoryPolicy, config.AllowedRoles)
} }
} }
@ -318,3 +335,38 @@ func isPublicACMEDisabledByEnv() (bool, error) {
return disableAcme, nil return disableAcme, nil
} }
func getDefaultDirectoryPolicyType(defaultDirectoryPolicy string) (DefaultDirectoryPolicyType, error) {
switch {
case defaultDirectoryPolicy == "forbid":
return Forbid, nil
case defaultDirectoryPolicy == "sign-verbatim":
return SignVerbatim, nil
case strings.HasPrefix(defaultDirectoryPolicy, "role:"):
if len(defaultDirectoryPolicy) == 5 {
return Forbid, fmt.Errorf("no role specified by policy %v", defaultDirectoryPolicy)
}
return Role, nil
default:
return Forbid, fmt.Errorf("string %v not a valid Default Directory Policy", defaultDirectoryPolicy)
}
}
func getDefaultDirectoryPolicyRole(defaultDirectoryPolicy string) (string, error) {
policyType, err := getDefaultDirectoryPolicyType(defaultDirectoryPolicy)
if err != nil {
return "", err
}
if policyType != Role {
return "", fmt.Errorf("default directory policy %v is not a role-based-policy", defaultDirectoryPolicy)
}
return defaultDirectoryPolicy[5:], nil
}
type DefaultDirectoryPolicyType int
const (
Forbid DefaultDirectoryPolicyType = iota
SignVerbatim
Role
)

View File

@ -142,17 +142,21 @@ $ certbot certonly --server https://localhost:8200/v1/pki/acme/directory ...
These endpoints are unauthenticated from a Vault authentication model, but These endpoints are unauthenticated from a Vault authentication model, but
internally authenticated via the ACME protocol. internally authenticated via the ACME protocol.
| Method | Path | Issuer | Role | | Method | Path | Default Directory Policy | Issuer | Role |
| :----- | :--------------------------------------------------- | :-------------------- | :----------------------------------- | |:-------|:-----------------------------------------------------|:-------------------------|:----------------------|:--------------|
| `ACME` | `/pki/acme/directory` | `default` | Sign-Verbatim or Specified in Config | | `ACME` | `/pki/acme/directory` | `sign-verbatim` | `default` | Sign-Verbatim |
| `ACME` | `/pki/issuer/:issuer_ref/acme/directory` | `:issuer_ref` | Sign-Verbatim or Specified in Config | | `ACME` | `/pki/acme/directory` | `role:role_ref` | Specified by the role | `:role_ref` |
| `ACME` | `/pki/roles/:role/acme/directory` | Specified by the role | `:role` | | `ACME` | `/pki/issuer/:issuer_ref/acme/directory` | `sign-verbatim` | `:issuer_ref` | Sign-Verbatim |
| `ACME` | `/pki/issuer/:issuer_ref/roles/:role/acme/directory` | `:issuer_ref` | `:role` | | `ACME` | `/pki/issuer/:issuer_ref/acme/directory` | `role:role_ref` | `:issuer_ref` | `:role_ref` |
| `ACME` | `/pki/roles/:role/acme/directory` | (any) | Specified by the role | `:role` |
| `ACME` | `/pki/issuer/:issuer_ref/roles/:role/acme/directory` | (any) | `:issuer_ref` | `:role` |
When a role is not specified (for the first two directory URLs), and no When a role is not specified (for the first two directory URLs, or four lines
`default_role` is specified in the [ACME configuration](#set-acme-configuration), in the table), behavior is specified by the `default_directory_policy` in the
[ACME configuration](#set-acme-configuration). These directories can also be
forbidden by setting that policy as `forbid`. If the policy is `sign-verbatim`
then _any_ identifier for which the client can prove ownership of will be then _any_ identifier for which the client can prove ownership of will be
issued for. This is similar to using the [Sign Verbatim](#sign-verbatim) issued for. This is similar to using the [Sign Verbatim](#sign-verbatim)
endpoint, but with additional verification that the client has proven endpoint, but with additional verification that the client has proven
ownership (within the ACME protocol) of the requested certificate ownership (within the ACME protocol) of the requested certificate
identifiers. identifiers.
@ -340,7 +344,7 @@ $ curl \
"allowed_roles": [ "allowed_roles": [
"*" "*"
], ],
"default_role": "", "default_directory_policy": "sign-verbatim",
"dns_resolver": "", "dns_resolver": "",
"eab_policy": "not-required", "eab_policy": "not-required",
"enabled": true "enabled": true
@ -365,13 +369,14 @@ mount.
allows every issuer within the mount. allows every issuer within the mount.
- `allowed_roles` `(list: ["*"])` - Specifies a list of roles to allow to - `allowed_roles` `(list: ["*"])` - Specifies a list of roles to allow to
issue certificates via explicit ACME paths. If no `default_role` is issue certificates via explicit ACME paths. The default value `*` allows
specified, sign-verbatim-like issuance on the default ACME directory every role within the mount to be used. If the `default_directory_policy`
will still occur. specifies a role, it must be allowed under this configuration.
- `default_role` `(string: "")` - Optionally specifies a role to enforce - `default_directory_policy` `(string: "sign-verbatim")` - Specifies the
on the default ACME directory. Must be present in `allowed_roles` if behavior of the default ACME director. Can be `forbid`, `sign-verbatim`
set. or a role given by `role:<role_name>`. If a role is used, it must be
present in `allowed_roles`.
- `dns_resolver` `(string: "")` - An optional overriding DNS resolver to - `dns_resolver` `(string: "")` - An optional overriding DNS resolver to
use for challenge verification lookups. When not specified, the default use for challenge verification lookups. When not specified, the default
@ -423,7 +428,7 @@ $ curl \
"allowed_roles": [ "allowed_roles": [
"*" "*"
], ],
"default_role": "", "default_directory_policy": "sign-verbatim",
"dns_resolver": "", "dns_resolver": "",
"eab_policy": "not-required", "eab_policy": "not-required",
"enabled": true "enabled": true