backport of commit 366693c78dec71212ab344e8d315637722b60d25 (#21887)

Co-authored-by: Laurent <hello@viper61.fr>
This commit is contained in:
hc-github-team-secure-vault-core 2023-07-17 09:50:04 -04:00 committed by GitHub
parent 9c43e232d2
commit ebc05a5221
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 83 additions and 13 deletions

View File

@ -398,13 +398,16 @@ func getAcmeRoleAndIssuer(sc *storageContext, data *framework.FieldData, config
} }
} }
// Override ExtKeyUsage behavior to force it to only be ServerAuth within ACME issued certs // If not allowed in configuration, override ExtKeyUsage behavior to force it to only be
role.ExtKeyUsage = []string{"serverauth"} // ServerAuth within ACME issued certs
role.ExtKeyUsageOIDs = []string{} if !config.AllowRoleExtKeyUsage {
role.ServerFlag = true role.ExtKeyUsage = []string{"serverauth"}
role.ClientFlag = false role.ExtKeyUsageOIDs = []string{}
role.CodeSigningFlag = false role.ServerFlag = true
role.EmailProtectionFlag = false role.ClientFlag = false
role.CodeSigningFlag = false
role.EmailProtectionFlag = false
}
return role, issuer, nil return role, issuer, nil
} }

View File

@ -582,10 +582,18 @@ func issueCertFromCsr(ac *acmeContext, csr *x509.CertificateRequest) (*certutil.
return nil, "", fmt.Errorf("verification of parsed bundle failed: %w", err) return nil, "", fmt.Errorf("verification of parsed bundle failed: %w", err)
} }
// We only allow ServerAuth key usage from ACME issued certs. // We only allow ServerAuth key usage from ACME issued certs
for _, usage := range parsedBundle.Certificate.ExtKeyUsage { // when configuration does not allow usage of ExtKeyusage field.
if usage != x509.ExtKeyUsageServerAuth { config, err := ac.sc.Backend.acmeState.getConfigWithUpdate(ac.sc)
return nil, "", fmt.Errorf("%w: ACME certs only allow ServerAuth key usage", ErrBadCSR) if err != nil {
return nil, "", fmt.Errorf("failed to fetch ACME configuration: %w", err)
}
if !config.AllowRoleExtKeyUsage {
for _, usage := range parsedBundle.Certificate.ExtKeyUsage {
if usage != x509.ExtKeyUsageServerAuth {
return nil, "", fmt.Errorf("%w: ACME certs only allow ServerAuth key usage", ErrBadCSR)
}
} }
} }

View File

@ -795,8 +795,10 @@ func TestAcmeTruncatesToIssuerExpiry(t *testing.T) {
require.Equal(t, shortCa.NotAfter, acmeCert.NotAfter, "certificate times aren't the same") require.Equal(t, shortCa.NotAfter, acmeCert.NotAfter, "certificate times aren't the same")
} }
// TestAcmeIgnoresRoleExtKeyUsage // TestAcmeRoleExtKeyUsage verify that ACME by default ignores the role's various ExtKeyUsage flags,
func TestAcmeIgnoresRoleExtKeyUsage(t *testing.T) { // but if the ACME configuration override of allow_role_ext_key_usage is set that we then honor
// the role's flag.
func TestAcmeRoleExtKeyUsage(t *testing.T) {
t.Parallel() t.Parallel()
cluster, client, _ := setupAcmeBackend(t) cluster, client, _ := setupAcmeBackend(t)
@ -862,6 +864,37 @@ func TestAcmeIgnoresRoleExtKeyUsage(t *testing.T) {
require.Equal(t, 1, len(acmeCert.ExtKeyUsage), "mis-match on expected ExtKeyUsages") require.Equal(t, 1, len(acmeCert.ExtKeyUsage), "mis-match on expected ExtKeyUsages")
require.ElementsMatch(t, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, acmeCert.ExtKeyUsage, require.ElementsMatch(t, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, acmeCert.ExtKeyUsage,
"mismatch of ExtKeyUsage flags") "mismatch of ExtKeyUsage flags")
// Now turn the ACME configuration allow_role_ext_key_usage and retest to make sure we get a certificate
// with them all
_, err = client.Logical().WriteWithContext(context.Background(), "pki/config/acme", map[string]interface{}{
"enabled": true,
"eab_policy": "not-required",
"allow_role_ext_key_usage": true,
})
require.NoError(t, err, "failed updating ACME configuration")
t.Logf("Testing Authorize Order on %s", baseAcmeURL)
order, err = acmeClient.AuthorizeOrder(testCtx, []acme.AuthzID{
{Type: "dns", Value: identifiers[0]},
})
require.NoError(t, err, "failed creating order")
// HACK: Update authorization/challenge to completed as we can't really do it properly in this workflow test.
markAuthorizationSuccess(t, client, acmeClient, acct, order)
certs, _, err = acmeClient.CreateOrderCert(testCtx, order.FinalizeURL, csr, true)
require.NoError(t, err, "order finalization failed")
require.GreaterOrEqual(t, len(certs), 1, "expected at least one cert in bundle")
acmeCert, err = x509.ParseCertificate(certs[0])
require.NoError(t, err, "failed parsing acme cert")
require.Equal(t, 4, len(acmeCert.ExtKeyUsage), "mis-match on expected ExtKeyUsages")
require.ElementsMatch(t, []x509.ExtKeyUsage{
x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth,
x509.ExtKeyUsageCodeSigning, x509.ExtKeyUsageEmailProtection,
},
acmeCert.ExtKeyUsage, "mismatch of ExtKeyUsage flags")
} }
func TestIssuerRoleDirectoryAssociations(t *testing.T) { func TestIssuerRoleDirectoryAssociations(t *testing.T) {

View File

@ -27,6 +27,7 @@ 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"`
AllowRoleExtKeyUsage bool `json:"allow_role_ext_key_usage"`
DefaultDirectoryPolicy string `json:"default_directory_policy"` 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"`
@ -36,6 +37,7 @@ var defaultAcmeConfig = acmeConfigEntry{
Enabled: false, Enabled: false,
AllowedIssuers: []string{"*"}, AllowedIssuers: []string{"*"},
AllowedRoles: []string{"*"}, AllowedRoles: []string{"*"},
AllowRoleExtKeyUsage: false,
DefaultDirectoryPolicy: "sign-verbatim", DefaultDirectoryPolicy: "sign-verbatim",
DNSResolver: "", DNSResolver: "",
EabPolicyName: eabPolicyNotRequired, EabPolicyName: eabPolicyNotRequired,
@ -98,6 +100,11 @@ func pathAcmeConfig(b *backend) *framework.Path {
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.`, 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{"*"},
}, },
"allow_role_ext_key_usage": {
Type: framework.TypeBool,
Description: `whether the ExtKeyUsage field from a role is used, defaults to false meaning that certificate will be signed with ServerAuth.`,
Default: false,
},
"default_directory_policy": { "default_directory_policy": {
Type: framework.TypeString, Type: framework.TypeString,
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`, 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`,
@ -161,6 +168,7 @@ func genResponseFromAcmeConfig(config *acmeConfigEntry, warnings []string) *logi
response := &logical.Response{ response := &logical.Response{
Data: map[string]interface{}{ Data: map[string]interface{}{
"allowed_roles": config.AllowedRoles, "allowed_roles": config.AllowedRoles,
"allow_role_ext_key_usage": config.AllowRoleExtKeyUsage,
"allowed_issuers": config.AllowedIssuers, "allowed_issuers": config.AllowedIssuers,
"default_directory_policy": config.DefaultDirectoryPolicy, "default_directory_policy": config.DefaultDirectoryPolicy,
"enabled": config.Enabled, "enabled": config.Enabled,
@ -194,6 +202,10 @@ func (b *backend) pathAcmeWrite(ctx context.Context, req *logical.Request, d *fr
} }
} }
if allowRoleExtKeyUsageRaw, ok := d.GetOk("allow_role_ext_key_usage"); ok {
config.AllowRoleExtKeyUsage = allowRoleExtKeyUsageRaw.(bool)
}
if defaultDirectoryPolicyRaw, ok := d.GetOk("default_directory_policy"); ok { if defaultDirectoryPolicyRaw, ok := d.GetOk("default_directory_policy"); ok {
config.DefaultDirectoryPolicy = defaultDirectoryPolicyRaw.(string) config.DefaultDirectoryPolicy = defaultDirectoryPolicyRaw.(string)
} }

3
changelog/21702.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
secrets/pki: Add a parameter to allow ExtKeyUsage field usage from a role within ACME.
```

View File

@ -39,6 +39,13 @@ export default class PkiConfigAcmeModel extends Model {
}) })
allowedRoles; allowedRoles;
@attr('boolean', {
label: 'Allow role ExtKeyUsage',
subText:
"When enabled, respect the role's ExtKeyUsage flags. Otherwise, ACME certificates are forced to ServerAuth.",
})
allowRoleExtKeyUsage;
@attr('array', { @attr('array', {
editType: 'stringArray', editType: 'stringArray',
subText: subText:

View File

@ -374,6 +374,10 @@ mount.
an issuer outside this list, it will be allowed. The default value `*` an issuer outside this list, it will be allowed. The default value `*`
allows every issuer within the mount. allows every issuer within the mount.
- `allow_role_ext_key_usage` `(bool: false)` - whether the ExtKeyUsage field
from a role is used, defaults to false meaning that certificate will be
signed with ServerAuth.
- `allowed_roles` `(list: ["*"])` - Specifies a list of roles allowed to - `allowed_roles` `(list: ["*"])` - Specifies a list of roles allowed to
issue certificates via explicit ACME paths. The default value `*` allows issue certificates via explicit ACME paths. The default value `*` allows
every role within the mount to be used. If the `default_directory_policy` every role within the mount to be used. If the `default_directory_policy`