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

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

Merged
merged 4 commits into from
Aug 15, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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:

* plugins: Send logs through Vault's logger rather than stdout [GH-3142]
Expand Down
7 changes: 6 additions & 1 deletion builtin/logical/pki/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
124 changes: 116 additions & 8 deletions builtin/logical/pki/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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))
}

Expand All @@ -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))
}

Expand Down Expand Up @@ -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))
}

Expand All @@ -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))
}

Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions builtin/logical/pki/path_config_ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.`,
},
},

Expand Down
9 changes: 7 additions & 2 deletions builtin/logical/pki/path_fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
38 changes: 37 additions & 1 deletion builtin/logical/pki/path_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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.
`
Expand Down
Loading