mirror of
				https://github.com/optim-enterprises-bv/vault.git
				synced 2025-10-30 18:17:55 +00:00 
			
		
		
		
	Add a parameter to allow ExtKeyUsage field usage from a role within ACME (#21702)
* imprv: Add a parameter to allow ExtKeyUsage field usage from a role * chore: Add the changelog entry * imprv: Reword UI and changelog * doc: Add allow_role_extkeyusage in parameter list * imprv: Align variable names with existing fields/codebase * Add unit test and tweak some labels --------- Co-authored-by: Steve Clark <steven.clark@hashicorp.com>
This commit is contained in:
		| @@ -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 | ||||||
| } | } | ||||||
|   | |||||||
| @@ -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) | ||||||
|  | 			} | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|   | |||||||
| @@ -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) { | ||||||
|   | |||||||
| @@ -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, | ||||||
| @@ -97,6 +99,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`, | ||||||
| @@ -160,6 +167,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, | ||||||
| @@ -193,6 +201,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
									
								
							
							
						
						
									
										3
									
								
								changelog/21702.txt
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,3 @@ | |||||||
|  | ```release-note:improvement | ||||||
|  | secrets/pki: Add a parameter to allow ExtKeyUsage field usage from a role within ACME. | ||||||
|  | ``` | ||||||
| @@ -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: | ||||||
|   | |||||||
| @@ -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` | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Laurent
					Laurent