Make PKI root generation idempotent-ish and add delete endpoint. (#3165)

.travis.yml

@@ -20,5 +20,5 @@ branches:
 
 script:
   - make bootstrap
-  - travis_wait 30 make test
-  - travis_wait 30 make testrace
+  - travis_wait 45 make test
+  - travis_wait 45 make testrace

CHANGELOG.md

@@ -1,5 +1,12 @@
 ## 0.8.1 (Unreleased)
 
+DEPRECATIONS/CHANGES:
+
+ * PKI Root Generation: Calling `pki/root/generate` when a CA cert/key already
+   exists will now return a `204` instead of overwriting an existing root. If
+   you want to recreate the root, first run a delete operation on `pki/root`
+   (requires `sudo` capability), then generate it again.
+
 IMPROVEMENTS:
 
  * auth/approle: Allow array input for policies in addition to comma-delimited

builtin/logical/pki/backend.go

@@ -39,15 +39,20 @@ func Backend() *backend {
 				"crl",
 				"certs/",
 			},
+
+			Root: []string{
+				"root",
+			},
 		},
 
 		Paths: []*framework.Path{
 			pathListRoles(&b),
 			pathRoles(&b),
 			pathGenerateRoot(&b),
+			pathSignIntermediate(&b),
+			pathDeleteRoot(&b),
 			pathGenerateIntermediate(&b),
 			pathSetSignedIntermediate(&b),
-			pathSignIntermediate(&b),
 			pathConfigCA(&b),
 			pathConfigCRL(&b),
 			pathConfigURLs(&b),

builtin/logical/pki/backend_test.go

@@ -22,10 +22,13 @@ import (
 	"time"
 
 	"github.com/fatih/structs"
+	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/helper/certutil"
 	"github.com/hashicorp/vault/helper/strutil"
+	vaulthttp "github.com/hashicorp/vault/http"
 	"github.com/hashicorp/vault/logical"
 	logicaltest "github.com/hashicorp/vault/logical/testing"
+	"github.com/hashicorp/vault/vault"
 	"github.com/mitchellh/mapstructure"
 )
 
@@ -648,6 +651,11 @@ func generateCSRSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s
 			ErrorOk: true,
 		},
 
+		logicaltest.TestStep{
+			Operation: logical.DeleteOperation,
+			Path:      "root",
+		},
+
 		logicaltest.TestStep{
 			Operation: logical.UpdateOperation,
 			Path:      "root/generate/exported",
@@ -865,6 +873,11 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 		},
 
 		// Test a bunch of generation stuff
+		logicaltest.TestStep{
+			Operation: logical.DeleteOperation,
+			Path:      "root",
+		},
+
 		logicaltest.TestStep{
 			Operation: logical.UpdateOperation,
 			Path:      "root/generate/exported",
@@ -997,6 +1010,11 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 		},
 
 		// Do it all again, with EC keys and DER format
+		logicaltest.TestStep{
+			Operation: logical.DeleteOperation,
+			Path:      "root",
+		},
+
 		logicaltest.TestStep{
 			Operation: logical.UpdateOperation,
 			Path:      "root/generate/exported",
@@ -1218,7 +1236,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 			Operation: logical.ReadOperation,
 			PreFlight: setSerialUnderTest,
 			Check: func(resp *logical.Response) error {
-				if resp.Data["error"] != nil && resp.Data["error"].(string) != "" {
+				if resp != nil && resp.Data["error"] != nil && resp.Data["error"].(string) != "" {
 					return fmt.Errorf("got an error: %s", resp.Data["error"].(string))
 				}
 
@@ -1232,7 +1250,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 			Operation: logical.ReadOperation,
 			PreFlight: setSerialUnderTest,
 			Check: func(resp *logical.Response) error {
-				if resp.Data["error"] != nil && resp.Data["error"].(string) != "" {
+				if resp != nil && resp.Data["error"] != nil && resp.Data["error"].(string) != "" {
 					return fmt.Errorf("got an error: %s", resp.Data["error"].(string))
 				}
 
@@ -1290,7 +1308,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 			Operation: logical.ReadOperation,
 			PreFlight: setSerialUnderTest,
 			Check: func(resp *logical.Response) error {
-				if resp.Data["error"] != nil && resp.Data["error"].(string) != "" {
+				if resp != nil && resp.Data["error"] != nil && resp.Data["error"].(string) != "" {
 					return fmt.Errorf("got an error: %s", resp.Data["error"].(string))
 				}
 
@@ -1304,7 +1322,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 			Operation: logical.ReadOperation,
 			PreFlight: setSerialUnderTest,
 			Check: func(resp *logical.Response) error {
-				if resp.Data["error"] != nil && resp.Data["error"].(string) != "" {
+				if resp != nil && resp.Data["error"] != nil && resp.Data["error"].(string) != "" {
 					return fmt.Errorf("got an error: %s", resp.Data["error"].(string))
 				}
 
@@ -1330,8 +1348,8 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 			Operation: logical.ReadOperation,
 			PreFlight: setSerialUnderTest,
 			Check: func(resp *logical.Response) error {
-				if resp.Data["error"] == nil || resp.Data["error"].(string) == "" {
-					return fmt.Errorf("didn't get an expected error")
+				if resp != nil {
+					return fmt.Errorf("expected no response")
 				}
 
 				serialUnderTest = "cert/" + reqdata["ec_int_serial_number"].(string)
@@ -1344,8 +1362,8 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
 			Operation: logical.ReadOperation,
 			PreFlight: setSerialUnderTest,
 			Check: func(resp *logical.Response) error {
-				if resp.Data["error"] == nil || resp.Data["error"].(string) == "" {
-					return fmt.Errorf("didn't get an expected error")
+				if resp != nil {
+					return fmt.Errorf("expected no response")
 				}
 
 				serialUnderTest = "cert/" + reqdata["rsa_int_serial_number"].(string)
@@ -2156,6 +2174,96 @@ func TestBackend_SignVerbatim(t *testing.T) {
 	}
 }
 
+func TestBackend_Root_Idempotentcy(t *testing.T) {
+	coreConfig := &vault.CoreConfig{
+		LogicalBackends: map[string]logical.Factory{
+			"pki": Factory,
+		},
+	}
+	cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
+		HandlerFunc: vaulthttp.Handler,
+	})
+	cluster.Start()
+	defer cluster.Cleanup()
+
+	client := cluster.Cores[0].Client
+	var err error
+	err = client.Sys().Mount("pki", &api.MountInput{
+		Type: "pki",
+		Config: api.MountConfigInput{
+			DefaultLeaseTTL: "16h",
+			MaxLeaseTTL:     "32h",
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	resp, err := client.Logical().Write("pki/root/generate/internal", map[string]interface{}{
+		"common_name": "myvault.com",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp == nil {
+		t.Fatal("expected ca info")
+	}
+	resp, err = client.Logical().Read("pki/cert/ca_chain")
+	r1Data := resp.Data
+
+	// Try again, make sure it's a 204 and same CA
+	resp, err = client.Logical().Write("pki/root/generate/internal", map[string]interface{}{
+		"common_name": "myvault.com",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp != nil {
+		t.Fatal("expected no ca info")
+	}
+	resp, err = client.Logical().Read("pki/cert/ca_chain")
+	r2Data := resp.Data
+	if !reflect.DeepEqual(r1Data, r2Data) {
+		t.Fatal("got different ca certs")
+	}
+
+	resp, err = client.Logical().Delete("pki/root")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp != nil {
+		t.Fatal("expected nil response")
+	}
+	// Make sure it behaves the same
+	resp, err = client.Logical().Delete("pki/root")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp != nil {
+		t.Fatal("expected nil response")
+	}
+
+	_, err = client.Logical().Read("pki/cert/ca_chain")
+	if err == nil {
+		t.Fatal("expected error")
+	}
+
+	resp, err = client.Logical().Write("pki/root/generate/internal", map[string]interface{}{
+		"common_name": "myvault.com",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp == nil {
+		t.Fatal("expected ca info")
+	}
+
+	_, err = client.Logical().Read("pki/cert/ca_chain")
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
 const (
 	rsaCAKey string = `-----BEGIN RSA PRIVATE KEY-----
 MIIEogIBAAKCAQEAmPQlK7xD5p+E8iLQ8XlVmll5uU2NKMxKY3UF5tbh+0vkc+Fy

builtin/logical/pki/path_config_ca.go

@@ -16,9 +16,7 @@ func pathConfigCA(b *backend) *framework.Path {
 			"pem_bundle": &framework.FieldSchema{
 				Type: framework.TypeString,
 				Description: `PEM-format, concatenated unencrypted
-secret key and certificate, or, if a
-CSR was generated with the "generate"
-endpoint, just the signed certificate.`,
+secret key and certificate.`,
 			},
 		},
 

builtin/logical/pki/path_fetch.go

@@ -159,7 +159,7 @@ func (b *backend) pathFetchRead(req *logical.Request, data *framework.FieldData)
 		caInfo, err := fetchCAInfo(req)
 		switch err.(type) {
 		case errutil.UserError:
-			response = logical.ErrorResponse(funcErr.Error())
+			response = logical.ErrorResponse(err.Error())
 			goto reply
 		case errutil.InternalError:
 			retErr = err
@@ -189,7 +189,7 @@ func (b *backend) pathFetchRead(req *logical.Request, data *framework.FieldData)
 		}
 	}
 	if certEntry == nil {
-		response = logical.ErrorResponse(fmt.Sprintf("certificate with serial %s not found", serial))
+		response = nil
 		goto reply
 	}
 
@@ -244,6 +244,11 @@ reply:
 		}
 	case retErr != nil:
 		response = nil
+		return
+	case response == nil:
+		return
+	case response.IsError():
+		return response, nil
 	default:
 		response.Data["certificate"] = string(certificate)
 		response.Data["revocation_time"] = revocationTime

builtin/logical/pki/path_root.go

@@ -28,6 +28,21 @@ func pathGenerateRoot(b *backend) *framework.Path {
 	return ret
 }
 
+func pathDeleteRoot(b *backend) *framework.Path {
+	ret := &framework.Path{
+		Pattern: "root",
+
+		Callbacks: map[logical.Operation]framework.OperationFunc{
+			logical.DeleteOperation: b.pathCADeleteRoot,
+		},
+
+		HelpSynopsis:    pathDeleteRootHelpSyn,
+		HelpDescription: pathDeleteRootHelpDesc,
+	}
+
+	return ret
+}
+
 func pathSignIntermediate(b *backend) *framework.Path {
 	ret := &framework.Path{
 		Pattern: "root/sign-intermediate",
@@ -66,10 +81,23 @@ the non-repudiation flag.`,
 	return ret
 }
 
+func (b *backend) pathCADeleteRoot(
+	req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
+	return nil, req.Storage.Delete("config/ca_bundle")
+}
+
 func (b *backend) pathCAGenerateRoot(
 	req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
 	var err error
 
+	entry, err := req.Storage.Get("config/ca_bundle")
+	if err != nil {
+		return nil, err
+	}
+	if entry != nil {
+		return nil, nil
+	}
+
 	exported, format, role, errorResp := b.getGenerationParams(data)
 	if errorResp != nil {
 		return errorResp, nil
@@ -133,7 +161,7 @@ func (b *backend) pathCAGenerateRoot(
 	}
 
 	// Store it as the CA bundle
-	entry, err := logical.StorageEntryJSON("config/ca_bundle", cb)
+	entry, err = logical.StorageEntryJSON("config/ca_bundle", cb)
 	if err != nil {
 		return nil, err
 	}
@@ -299,6 +327,14 @@ const pathGenerateRootHelpDesc = `
 See the API documentation for more information.
 `
 
+const pathDeleteRootHelpSyn = `
+Deletes the root CA key to allow a new one to be generated.
+`
+
+const pathDeleteRootHelpDesc = `
+See the API documentation for more information.
+`
+
 const pathSignIntermediateHelpSyn = `
 Issue an intermediate CA certificate based on the provided CSR.
 `