Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve TF state for PKI Multi-Issuer workflows #1973

Merged
merged 7 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ require (
github.com/denisenkom/go-mssqldb v0.12.0
github.com/docker/distribution v2.8.1+incompatible // indirect
github.com/go-sql-driver/mysql v1.6.0
github.com/google/uuid v1.3.0
github.com/googleapis/enterprise-certificate-proxy v0.2.0 // indirect
github.com/gosimple/slug v1.11.0
github.com/hashicorp/errwrap v1.1.0
Expand Down
12 changes: 12 additions & 0 deletions util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ func ToStringArray(input []interface{}) []string {
return output
}

func Is500(err error) bool {
return ErrorContainsHTTPCode(err, http.StatusInternalServerError)
}

func Is404(err error) bool {
return ErrorContainsHTTPCode(err, http.StatusNotFound)
}
Expand All @@ -64,6 +68,14 @@ func ErrorContainsHTTPCode(err error, codes ...int) bool {
return false
}

func ErrorContainsString(err error, s string) bool {
if strings.Contains(err.Error(), s) {
return true
}

return false
vinay-gopalan marked this conversation as resolved.
Show resolved Hide resolved
}

// CalculateConflictsWith returns a slice of field names that conflict with
// a single field (self).
func CalculateConflictsWith(self string, group []string) []string {
Expand Down
34 changes: 26 additions & 8 deletions vault/resource_pki_secret_backend_intermediate_cert_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ package vault

import (
"context"
"fmt"
"log"
"strings"

"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
Expand Down Expand Up @@ -231,7 +233,8 @@ func pkiSecretBackendIntermediateCertRequestCreate(ctx context.Context, d *schem

backend := d.Get(consts.FieldBackend).(string)
intermediateType := d.Get(consts.FieldType).(string)
path := pkiSecretBackendIntermediateGeneratePath(backend, intermediateType)

path := pkiSecretBackendIntermediateGeneratePath(backend, intermediateType, provider.IsAPISupported(meta, provider.VaultVersion111))

intermediateCertAPIFields := []string{
consts.FieldCommonName,
Expand Down Expand Up @@ -262,15 +265,19 @@ func pkiSecretBackendIntermediateCertRequestCreate(ctx context.Context, d *schem

// add multi-issuer write API fields if supported
isIssuerAPISupported := provider.IsAPISupported(meta, provider.VaultVersion111)
isKeyBeingGenerated := !(intermediateType == keyTypeKMS || intermediateType == consts.FieldExisting)
vinay-gopalan marked this conversation as resolved.
Show resolved Hide resolved

if !(intermediateType == keyTypeKMS || intermediateType == consts.FieldExisting) {
// if kms or existing type,
if isKeyBeingGenerated {
intermediateCertAPIFields = append(intermediateCertAPIFields, consts.FieldKeyType, consts.FieldKeyBits)
if isIssuerAPISupported {
intermediateCertAPIFields = append(intermediateCertAPIFields, consts.FieldKeyRef)
intermediateCertAPIFields = append(intermediateCertAPIFields, consts.FieldKeyName)
}
} else if isIssuerAPISupported {
intermediateCertAPIFields = append(intermediateCertAPIFields, consts.FieldKeyName)
} else {
intermediateCertAPIFields = append(intermediateCertAPIFields, consts.FieldKeyRef)
}

if isIssuerAPISupported {
intermediateCertAPIFields = append(intermediateCertAPIFields, consts.FieldIssuerName)
vinay-gopalan marked this conversation as resolved.
Show resolved Hide resolved
}

data := map[string]interface{}{}
Expand Down Expand Up @@ -329,7 +336,15 @@ func pkiSecretBackendIntermediateCertRequestCreate(ctx context.Context, d *schem

}

d.SetId(path)
id := path
if provider.IsAPISupported(meta, provider.VaultVersion111) {
// multiple CSRs can be generated
// ensure unique IDs
uniqueSuffix := uuid.New()
id = fmt.Sprintf("%s/%s", path, uniqueSuffix)
}

d.SetId(id)
return pkiSecretBackendIntermediateCertRequestRead(ctx, d, meta)
}

Expand All @@ -341,6 +356,9 @@ func pkiSecretBackendIntermediateCertRequestDelete(ctx context.Context, d *schem
return nil
}

func pkiSecretBackendIntermediateGeneratePath(backend string, intermediateType string) string {
func pkiSecretBackendIntermediateGeneratePath(backend, intermediateType string, isMultiIssuerSupported bool) string {
if isMultiIssuerSupported {
return strings.Trim(backend, "/") + "/issuers/generate/intermediate/" + strings.Trim(intermediateType, "/")
}
return strings.Trim(backend, "/") + "/intermediate/generate/" + strings.Trim(intermediateType, "/")
}
13 changes: 10 additions & 3 deletions vault/resource_pki_secret_backend_issuer.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,11 @@ func pkiSecretBackendIssuerRead(ctx context.Context, d *schema.ResourceData, met

log.Printf("[DEBUG] Reading %s from Vault", path)
resp, err := client.Logical().ReadWithContext(ctx, path)
if resp == nil {
d.SetId("")
return nil
}

if err != nil {
return diag.Errorf("error reading from Vault: %s", err)
}
Expand Down Expand Up @@ -232,9 +237,11 @@ func pkiSecretBackendIssuerRead(ctx context.Context, d *schema.ResourceData, met
}

for _, k := range fields {
if err := d.Set(k, resp.Data[k]); err != nil {
return diag.Errorf("error setting state key %q for issuer, err=%s",
k, err)
if v, ok := resp.Data[k]; ok {
if err := d.Set(k, v); err != nil {
return diag.Errorf("error setting state key %q for issuer, err=%s",
k, err)
}
}
}

Expand Down
91 changes: 77 additions & 14 deletions vault/resource_pki_secret_backend_root_cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ import (
"github.com/hashicorp/terraform-provider-vault/util"
)

const (
issuerNotFoundErr = "unable to find PKI issuer for reference"
)

func pkiSecretBackendRootCertResource() *schema.Resource {
return &schema.Resource{
CreateContext: pkiSecretBackendRootCertCreate,
Expand Down Expand Up @@ -53,9 +57,38 @@ func pkiSecretBackendRootCertResource() *schema.Resource {
return e
}

cert, err := getCACertificate(client, d.Get(consts.FieldBackend).(string))
if err != nil {
return err
var cert *x509.Certificate
isIssuerAPISupported := provider.IsAPISupported(meta, provider.VaultVersion111)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prior to multi-issuer support (<= Vault 1.10), this customize diff would read the default CA in PEM format and compare to what was in the TF state. When there is only one issuer, this makes sense since it will be the default. However, for multiple issuers, all Root Cert resources that are not the default issuer will be forced to be recreated. This was leading users into a constant create-destroy cycle when multiple root certs were added in a TF file.

This update now correctly read the specific issuer's certificate in PEM format, and compare to what is in the TF state. If there is a change in the certificate, the Root Cert is recreated. Also, if an issuer is deleted outside of TF, this update correctly resolves the state by recreating the resource.

if isIssuerAPISupported {
// get the specific certificate for issuer with issuer_id
cert, e = getIssuerPEM(client, d.Get(consts.FieldBackend).(string), d.Get(consts.FieldIssuerID).(string))
if e != nil {
// Check if this is an out-of-band change on the issuer
if util.Is500(e) && util.ErrorContainsString(e, issuerNotFoundErr) {
log.Printf("issuer deleted out-of-band. re-creating root cert")
vinay-gopalan marked this conversation as resolved.
Show resolved Hide resolved
// Force a change on the issuer ID field since
// it no longer exists and must be re-created
if e = d.SetNewComputed(consts.FieldIssuerID); e != nil {
return e
}

if e := d.ForceNew(consts.FieldIssuerID); e != nil {
return e
}

return nil
}

// not an out-of-band issuer error
return e
}
} else {
// get the 'default' issuer's certificate
// default behavior for non multi-issuer API
cert, e = getDefaultCAPEM(client, d.Get(consts.FieldBackend).(string))
if e != nil {
return e
}
}

if cert != nil {
Expand Down Expand Up @@ -316,7 +349,7 @@ func pkiSecretBackendRootCertCreate(_ context.Context, d *schema.ResourceData, m
backend := d.Get(consts.FieldBackend).(string)
rootType := d.Get(consts.FieldType).(string)

path := pkiSecretBackendIntermediateSetSignedReadPath(backend, rootType)
path := pkiSecretBackendGenerateRootPath(backend, rootType, provider.IsAPISupported(meta, provider.VaultVersion111))

rootCertAPIFields := []string{
consts.FieldCommonName,
Expand Down Expand Up @@ -349,14 +382,19 @@ func pkiSecretBackendRootCertCreate(_ context.Context, d *schema.ResourceData, m

// add multi-issuer write API fields if supported
isIssuerAPISupported := provider.IsAPISupported(meta, provider.VaultVersion111)
isKeyBeingGenerated := !(rootType == keyTypeKMS || rootType == consts.FieldExisting)

if !(rootType == keyTypeKMS || rootType == consts.FieldExisting) {
if isKeyBeingGenerated {
rootCertAPIFields = append(rootCertAPIFields, consts.FieldKeyType, consts.FieldKeyBits)
if isIssuerAPISupported {
rootCertAPIFields = append(rootCertAPIFields, consts.FieldKeyRef, consts.FieldIssuerName)
rootCertAPIFields = append(rootCertAPIFields, consts.FieldKeyName)
}
} else if isIssuerAPISupported {
rootCertAPIFields = append(rootCertAPIFields, consts.FieldKeyName, consts.FieldIssuerName)
} else {
rootCertAPIFields = append(rootCertAPIFields, consts.FieldKeyRef)
}

if isIssuerAPISupported {
rootCertAPIFields = append(rootCertAPIFields, consts.FieldIssuerName)
}

data := map[string]interface{}{}
Expand Down Expand Up @@ -416,13 +454,20 @@ func pkiSecretBackendRootCertCreate(_ context.Context, d *schema.ResourceData, m
}
}

d.SetId(path)
id := path
if isIssuerAPISupported {
// multiple root certs can be issued
// ensure ID for each root_cert resource is unique
issuerID := resp.Data[consts.FieldIssuerID]
id = fmt.Sprintf("%s/issuer/%s", backend, issuerID)
}

d.SetId(id)

return nil
}

func getCACertificate(client *api.Client, mount string) (*x509.Certificate, error) {
path := fmt.Sprintf("/v1/%s/ca/pem", mount)
func getCACertificate(client *api.Client, path string) (*x509.Certificate, error) {
req := client.NewRequest(http.MethodGet, path)
req.ClientToken = ""
resp, err := client.RawRequest(req)
Expand Down Expand Up @@ -455,6 +500,16 @@ func getCACertificate(client *api.Client, mount string) (*x509.Certificate, erro
return nil, nil
}

func getDefaultCAPEM(client *api.Client, mount string) (*x509.Certificate, error) {
path := fmt.Sprintf("/v1/%s/ca/pem", mount)
return getCACertificate(client, path)
}

func getIssuerPEM(client *api.Client, mount, issuerID string) (*x509.Certificate, error) {
path := fmt.Sprintf("/v1/%s/issuer/%s/pem", mount, issuerID)
return getCACertificate(client, path)
}

func pkiSecretBackendRootCertDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
client, e := provider.GetClient(d, meta)
if e != nil {
Expand All @@ -463,7 +518,12 @@ func pkiSecretBackendRootCertDelete(ctx context.Context, d *schema.ResourceData,

backend := d.Get(consts.FieldBackend).(string)

path := pkiSecretBackendIntermediateSetSignedDeletePath(backend)
var path string
if provider.IsAPISupported(meta, provider.VaultVersion111) {
path = d.Id()
} else {
path = pkiSecretBackendDeleteRootPath(backend)
}

log.Printf("[DEBUG] Deleting root cert from PKI secret backend %q", path)
if _, err := client.Logical().Delete(path); err != nil {
Expand All @@ -473,11 +533,14 @@ func pkiSecretBackendRootCertDelete(ctx context.Context, d *schema.ResourceData,
return nil
}

func pkiSecretBackendIntermediateSetSignedReadPath(backend string, rootType string) string {
func pkiSecretBackendGenerateRootPath(backend, rootType string, isMultiIssuerSupported bool) string {
if isMultiIssuerSupported {
return strings.Trim(backend, "/") + "/issuers/generate/root/" + strings.Trim(rootType, "/")
}
return strings.Trim(backend, "/") + "/root/generate/" + strings.Trim(rootType, "/")
}

func pkiSecretBackendIntermediateSetSignedDeletePath(backend string) string {
func pkiSecretBackendDeleteRootPath(backend string) string {
return strings.Trim(backend, "/") + "/root"
}

Expand Down
59 changes: 58 additions & 1 deletion vault/resource_pki_secret_backend_root_cert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ func TestPkiSecretBackendRootCertificate_basic(t *testing.T) {
if err != nil {
t.Fatal(err)
}
genPath := pkiSecretBackendIntermediateSetSignedReadPath(path, "internal")

isMultiIssuerSupported := testProvider.Meta().(*provider.ProviderMeta).IsAPISupported(provider.VaultVersion111)
genPath := pkiSecretBackendGenerateRootPath(path, "internal", isMultiIssuerSupported)
resp, err := client.Logical().Write(genPath,
map[string]interface{}{
consts.FieldCommonName: "out-of-band",
Expand Down Expand Up @@ -145,6 +147,38 @@ func TestPkiSecretBackendRootCertificate_multiIssuer(t *testing.T) {
})
}

// Ensures that TF state is cleanly resolved whenever
// multiple root certs are generated
func TestAccPKISecretBackendRootCert_multiple(t *testing.T) {
backend := acctest.RandomWithPrefix("tf-test-pki")
resourceType := "vault_pki_secret_backend_root_cert"
resourceCurrentIssuer := resourceType + ".current"
resourceNextIssuer := resourceType + ".next"

resource.Test(t, resource.TestCase{
ProviderFactories: providerFactories,
PreCheck: func() {
testutil.TestAccPreCheck(t)
SkipIfAPIVersionLT(t, testProvider.Meta(), provider.VaultVersion111)
},
CheckDestroy: testCheckMountDestroyed(resourceType, consts.MountTypePKI, consts.FieldBackend),
Steps: []resource.TestStep{
{
Config: testAccPKISecretBackendRootCert_multiple(backend),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceCurrentIssuer, consts.FieldBackend, backend),
resource.TestCheckResourceAttr(resourceCurrentIssuer, consts.FieldType, "internal"),
resource.TestCheckResourceAttrSet(resourceCurrentIssuer, consts.FieldIssuerID),

resource.TestCheckResourceAttr(resourceNextIssuer, consts.FieldBackend, backend),
resource.TestCheckResourceAttr(resourceNextIssuer, consts.FieldType, "internal"),
resource.TestCheckResourceAttrSet(resourceNextIssuer, consts.FieldIssuerID),
),
},
},
})
}

func TestPkiSecretBackendRootCertificate_managedKeys(t *testing.T) {
path := "pki-" + strconv.Itoa(acctest.RandInt())

Expand Down Expand Up @@ -228,6 +262,29 @@ resource "vault_pki_secret_backend_root_cert" "test" {
return config
}

func testAccPKISecretBackendRootCert_multiple(path string) string {
return fmt.Sprintf(`
resource "vault_mount" "test" {
path = "%s"
type = "pki"
description = "PKI secret engine mount"
}

resource "vault_pki_secret_backend_root_cert" "current" {
backend = vault_mount.test.path
type = "internal"
common_name = "test"
ttl = "86400"
}

resource "vault_pki_secret_backend_root_cert" "next" {
backend = vault_mount.test.path
type = "internal"
common_name = "test"
ttl = "86400"
}`, path)
}

func testPkiSecretBackendRootCertificateConfig_managedKeys(path, managedKeyName, accessKey, secretKey string) string {
config := fmt.Sprintf(`
resource "vault_managed_keys" "test" {
Expand Down