From 8848136bc625eebca46d4a31ade19ce868c214d4 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 3 May 2016 14:24:04 -0400 Subject: [PATCH 1/9] Properly persist auth mount tuning --- vault/logical_system.go | 14 ++++++++++++-- vault/logical_system_helpers.go | 18 +++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index 52f1c7a0b3..89ec1a379d 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -3,6 +3,7 @@ package vault import ( "fmt" "strings" + "sync" "time" "github.com/hashicorp/vault/logical" @@ -845,6 +846,14 @@ func (b *SystemBackend) handleMountTuneWrite( return handleError(err) } + var lock *sync.RWMutex + switch { + case strings.HasPrefix(path, "auth/"): + lock = &b.Core.authLock + default: + lock = &b.Core.mountsLock + } + // Timing configuration parameters { var newDefault, newMax *time.Duration @@ -877,8 +886,9 @@ func (b *SystemBackend) handleMountTuneWrite( } if newDefault != nil || newMax != nil { - b.Core.mountsLock.Lock() - defer b.Core.mountsLock.Unlock() + lock.Lock() + defer lock.Unlock() + if err := b.tuneMountTTLs(path, &mountEntry.Config, newDefault, newMax); err != nil { b.Backend.Logger().Printf("[ERR] sys: tune of path '%s' failed: %v", path, err) return handleError(err) diff --git a/vault/logical_system_helpers.go b/vault/logical_system_helpers.go index b6d35ef76a..a3fb945794 100644 --- a/vault/logical_system_helpers.go +++ b/vault/logical_system_helpers.go @@ -1,8 +1,8 @@ package vault import ( - "errors" "fmt" + "strings" "time" ) @@ -51,6 +51,9 @@ func (b *SystemBackend) tuneMountTTLs(path string, meConfig *MountConfig, newDef } } + origMax := meConfig.MaxLeaseTTL + origDefault := meConfig.DefaultLeaseTTL + if newMax != nil { meConfig.MaxLeaseTTL = *newMax } @@ -59,8 +62,17 @@ func (b *SystemBackend) tuneMountTTLs(path string, meConfig *MountConfig, newDef } // Update the mount table - if err := b.Core.persistMounts(b.Core.mounts); err != nil { - return errors.New("failed to update mount table") + var err error + switch { + case strings.HasPrefix(path, "auth/"): + err = b.Core.persistAuth(b.Core.auth) + default: + err = b.Core.persistMounts(b.Core.mounts) + } + if err != nil { + meConfig.MaxLeaseTTL = origMax + meConfig.DefaultLeaseTTL = origDefault + return fmt.Errorf("failed to update mount table, rolling back TTL changes") } b.Core.logger.Printf("[INFO] core: tuned '%s'", path) From 5aebbcc5661182c22ca77b2e4c14898a0f954c54 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 3 May 2016 15:12:08 -0400 Subject: [PATCH 2/9] changelog++ --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a32ddfe0b8..790bc95d3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,7 @@ BUG FIXES: * command/various: Tell the JSON decoder to not convert all numbers to floats; fixes some various places where numbers were showing up in scientific notation + * core: Properly persist mount-tuned TTLs for auth backends [GH-1371] * core: Don't accidentally crosswire SIGINT to the reload handler [GH-1372] * credential/github: Make organization comparison case-insensitive during login [GH-1359] From 806119f5a1d506a4dcc0cfba34e466b94e02f18c Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 3 May 2016 23:06:57 -0400 Subject: [PATCH 3/9] Fix number of recovery shares output during init --- command/init.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/init.go b/command/init.go index 1f4cc6b2ea..a839973de7 100644 --- a/command/init.go +++ b/command/init.go @@ -92,8 +92,8 @@ func (c *InitCommand) Run(args []string) int { "\n"+ "Recovery key initialized with %d keys and a key threshold of %d. Please\n"+ "securely distribute the above keys.", - shares, - threshold, + recoveryShares, + recoveryThreshold, )) } From 37d425f873f364d474decb6f5f1f100a7119a3ee Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 4 May 2016 02:17:20 -0400 Subject: [PATCH 4/9] Update website docs re token store role period parsing --- website/source/docs/auth/token.html.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/website/source/docs/auth/token.html.md b/website/source/docs/auth/token.html.md index 5a5efe64b3..a3c65e4c55 100644 --- a/website/source/docs/auth/token.html.md +++ b/website/source/docs/auth/token.html.md @@ -599,8 +599,9 @@ of the header should be "X-Vault-Token" and the value should be the token. If set, tokens created against this role will not have a maximum lifetime. Instead, they will have a fixed TTL that is refreshed with each renewal. So long as they continue to be renewed, they will never - expire. The parameter is an integer duration of seconds or a duration - string (e.g. `"72h"`). + expire. The parameter is an integer duration of seconds. Tokens issued + track updates to the role value; the new period takes effect upon next + renew.
  • path_suffix From a74332bb7e01c70d245e62c25f85a7d8e93aee15 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 4 May 2016 01:46:25 -0400 Subject: [PATCH 5/9] Add the steps to generate the CRL test's test-fixture files --- builtin/credential/cert/backend_test.go | 56 ++++++---------- .../cert/test-fixtures/generate.txt | 67 +++++++++++++++++++ 2 files changed, 88 insertions(+), 35 deletions(-) create mode 100644 builtin/credential/cert/test-fixtures/generate.txt diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index d9e98b5e6f..795ff3ff78 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -101,6 +101,15 @@ func connectionState(t *testing.T, serverCAPath, serverCertPath, serverKeyPath, return connState } +func failOnError(t *testing.T, resp *logical.Response, err error) { + if resp != nil && resp.IsError() { + t.Fatalf("error returned in response: %s", resp.Data["error"]) + } + if err != nil { + t.Fatal(err) + } +} + func TestBackend_CRLs(t *testing.T) { config := logical.TestBackendConfig() storage := &logical.InmemStorage{} @@ -130,10 +139,8 @@ func TestBackend_CRLs(t *testing.T) { Data: certData, } - _, err = b.HandleRequest(certReq) - if err != nil { - t.Fatal(err) - } + resp, err := b.HandleRequest(certReq) + failOnError(t, resp, err) // Connection state is presenting the client CA cert and its key. // This is exactly what is registered at the backend. @@ -146,13 +153,8 @@ func TestBackend_CRLs(t *testing.T) { ConnState: &connState, }, } - resp, err := b.HandleRequest(loginReq) - if err != nil { - t.Fatal(err) - } - if resp == nil || resp.IsError() { - t.Fatalf("failed to login") - } + resp, err = b.HandleRequest(loginReq) + failOnError(t, resp, err) // Now, without changing the registered client CA cert, present from // the client side, a cert issued using the registered CA. @@ -161,12 +163,7 @@ func TestBackend_CRLs(t *testing.T) { // Attempt login with the updated connection resp, err = b.HandleRequest(loginReq) - if err != nil { - t.Fatal(err) - } - if resp == nil || resp.IsError() { - t.Fatalf("failed to login") - } + failOnError(t, resp, err) // Register a CRL containing the issued client certificate used above. issuedCRL, err := ioutil.ReadFile(testIssuedCertCRL) @@ -183,10 +180,8 @@ func TestBackend_CRLs(t *testing.T) { Path: "crls/issuedcrl", Data: crlData, } - _, err = b.HandleRequest(crlReq) - if err != nil { - t.Fatal(err) - } + resp, err = b.HandleRequest(crlReq) + failOnError(t, resp, err) // Attempt login with the revoked certificate. resp, err = b.HandleRequest(loginReq) @@ -203,10 +198,8 @@ func TestBackend_CRLs(t *testing.T) { t.Fatal(err) } certData["certificate"] = clientCA2 - _, err = b.HandleRequest(certReq) - if err != nil { - t.Fatal(err) - } + resp, err = b.HandleRequest(certReq) + failOnError(t, resp, err) // Test login using a different client CA cert pair. connState = connectionState(t, serverCAPath, serverCertPath, serverKeyPath, testRootCACertPath2, testRootCAKeyPath2) @@ -214,12 +207,7 @@ func TestBackend_CRLs(t *testing.T) { // Attempt login with the updated connection resp, err = b.HandleRequest(loginReq) - if err != nil { - t.Fatal(err) - } - if resp == nil || resp.IsError() { - t.Fatalf("failed to login") - } + failOnError(t, resp, err) // Register a CRL containing the root CA certificate used above. rootCRL, err := ioutil.ReadFile(testRootCertCRL) @@ -227,10 +215,8 @@ func TestBackend_CRLs(t *testing.T) { t.Fatal(err) } crlData["crl"] = rootCRL - _, err = b.HandleRequest(crlReq) - if err != nil { - t.Fatal(err) - } + resp, err = b.HandleRequest(crlReq) + failOnError(t, resp, err) // Attempt login with the same connection state but with the CRL registered resp, err = b.HandleRequest(loginReq) diff --git a/builtin/credential/cert/test-fixtures/generate.txt b/builtin/credential/cert/test-fixtures/generate.txt new file mode 100644 index 0000000000..7e1e23cbdd --- /dev/null +++ b/builtin/credential/cert/test-fixtures/generate.txt @@ -0,0 +1,67 @@ +vault mount pki +vault mount-tune -max-lease-ttl=438000h pki +vault write pki/root/generate/exported common_name=myvault.com ttl=438000h ip_sans=127.0.0.1 +vi cacert.pem +vi cakey.pem + +vaultcert.hcl +backend "inmem" { +} +disable_mlock = true +default_lease_ttl = "700h" +max_lease_ttl = "720h" +listener "tcp" { + address = "127.0.0.1:8200" + tls_cert_file = "./cacert.pem" + tls_key_file = "./cakey.pem" +} +======================================== +vault mount pki +vault mount-tune -max-lease-ttl=438000h pki +vault write pki/root/generate/exported common_name=myvault.com ttl=438000h max_ttl=438000h ip_sans=127.0.0.1 +vi testcacert1.pem +vi testcakey1.pem +vi testcaserial1 + +vault write pki/config/urls issuing_certificates="http://127.0.0.1:8200/v1/pki/ca" crl_distribution_points="http://127.0.0.1:8200/v1/pki/crl" +vault write pki/roles/myvault-dot-com allowed_domains=myvault.com allow_subdomains=true ttl=437999h max_ttl=438000h allow_ip_sans=true + +vault write pki/issue/myvault-dot-com common_name=cert.myvault.com format=pem ip_sans=127.0.0.1 +vi testissuedserial1 + +vault write pki/issue/myvault-dot-com common_name=cert.myvault.com format=pem ip_sans=127.0.0.1 +vi testissuedcert2.pem +vi testissuedkey2.pem +vi testissuedserial2 + +vault write pki/issue/myvault-dot-com common_name=cert.myvault.com format=pem ip_sans=127.0.0.1 +vi testissuedserial3 + +vault write pki/issue/myvault-dot-com common_name=cert.myvault.com format=pem ip_sans=127.0.0.1 +vi testissuedcert4.pem +vi testissuedkey4.pem +vi testissuedserial4 + +vault write pki/issue/myvault-dot-com common_name=cert.myvault.com format=pem ip_sans=127.0.0.1 +vi testissuedserial5 + +vault write pki/revoke serial_number=$(cat testissuedserial2) +vault write pki/revoke serial_number=$(cat testissuedserial4) +curl -XGET "http://127.0.0.1:8200/v1/pki/crl/pem" -H "x-vault-token:123" > issuedcertcrl +openssl crl -in issuedcertcrl -noout -text + +======================================== +export VAULT_ADDR='http://127.0.0.1:8200' +vault mount pki +vault mount-tune -max-lease-ttl=438000h pki +vault write pki/root/generate/exported common_name=myvault.com ttl=438000h ip_sans=127.0.0.1 +vi testcacert2.pem +vi testcakey2.pem +vi testcaserial2 +vi testcacert2leaseid + +vault write pki/config/urls issuing_certificates="http://127.0.0.1:8200/v1/pki/ca" crl_distribution_points="http://127.0.0.1:8200/v1/pki/crl" +vault revoke $(cat testcacert2leaseid) + +curl -XGET "http://127.0.0.1:8200/v1/pki/crl/pem" -H "x-vault-token:123" > cacert2crl +openssl crl -in cacert2crl -noout -text From 0ea77f7ddac0a9e301ddac3b760c66c7c6449a01 Mon Sep 17 00:00:00 2001 From: Chris Jansen Date: Wed, 4 May 2016 18:04:28 +0100 Subject: [PATCH 6/9] Add scala vault library to list of client libs --- website/source/docs/http/libraries.html.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/website/source/docs/http/libraries.html.md b/website/source/docs/http/libraries.html.md index b400f5331c..716a27f63a 100644 --- a/website/source/docs/http/libraries.html.md +++ b/website/source/docs/http/libraries.html.md @@ -59,3 +59,6 @@ These libraries are provided by the community. * [HVAC](https://github.com/ianunruh/hvac) * `pip install hvac` + +### Scala + * [scala-vault](https://github.com/janstenpickle/scala-vault) From e2927befea6c5dade1c6c05ee31ecb5866e17ff0 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Thu, 5 May 2016 05:22:59 -0400 Subject: [PATCH 7/9] Lower case all policy values in ParsePolicies before processing --- helper/policyutil/policyutil.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helper/policyutil/policyutil.go b/helper/policyutil/policyutil.go index 31f2674d33..09fee92fd5 100644 --- a/helper/policyutil/policyutil.go +++ b/helper/policyutil/policyutil.go @@ -9,7 +9,7 @@ func ParsePolicies(policiesRaw string) []string { policies := strings.Split(policiesRaw, ",") defaultFound := false for i, p := range policies { - policies[i] = strings.TrimSpace(p) + policies[i] = strings.ToLower(strings.TrimSpace(p)) // If 'root' policy is present, ignore all other policies. if policies[i] == "root" { policies = []string{"root"} From 0481976696bcd0d88dd3f2e116fc52caf7f4dd74 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Thu, 5 May 2016 09:45:48 -0400 Subject: [PATCH 8/9] Split SanitizeTTL method to support time.Duration parameters as well --- builtin/credential/github/path_login.go | 2 +- builtin/credential/userpass/path_users.go | 2 +- logical/framework/backend.go | 27 +++++++++++++++-------- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/builtin/credential/github/path_login.go b/builtin/credential/github/path_login.go index 6e6d50e589..eff52efaaa 100644 --- a/builtin/credential/github/path_login.go +++ b/builtin/credential/github/path_login.go @@ -46,7 +46,7 @@ func (b *backend) pathLogin( return nil, err } - ttl, _, err := b.SanitizeTTL(config.TTL.String(), config.MaxTTL.String()) + ttl, _, err := b.SanitizeTTLStr(config.TTL.String(), config.MaxTTL.String()) if err != nil { return logical.ErrorResponse(fmt.Sprintf("[ERR]:%s", err)), nil } diff --git a/builtin/credential/userpass/path_users.go b/builtin/credential/userpass/path_users.go index 62399259a0..a67f25b6ae 100644 --- a/builtin/credential/userpass/path_users.go +++ b/builtin/credential/userpass/path_users.go @@ -176,7 +176,7 @@ func (b *backend) userCreateUpdate(req *logical.Request, d *framework.FieldData) maxTTLStr = maxTTLStrRaw.(string) } - userEntry.TTL, userEntry.MaxTTL, err = b.SanitizeTTL(ttlStr, maxTTLStr) + userEntry.TTL, userEntry.MaxTTL, err = b.SanitizeTTLStr(ttlStr, maxTTLStr) if err != nil { return logical.ErrorResponse(fmt.Sprintf("err: %s", err)), nil } diff --git a/logical/framework/backend.go b/logical/framework/backend.go index e6749935aa..92fccbc581 100644 --- a/logical/framework/backend.go +++ b/logical/framework/backend.go @@ -225,8 +225,7 @@ func (b *Backend) System() logical.SystemView { // compares those with the SystemView values. If they are empty a value of 0 is // set, which will cause initial secret or LeaseExtend operations to use the // mount/system defaults. If they are set, their boundaries are validated. -func (b *Backend) SanitizeTTL(ttlStr, maxTTLStr string) (ttl, maxTTL time.Duration, err error) { - sysMaxTTL := b.System().MaxLeaseTTL() +func (b *Backend) SanitizeTTLStr(ttlStr, maxTTLStr string) (ttl, maxTTL time.Duration, err error) { if len(ttlStr) == 0 || ttlStr == "0" { ttl = 0 } else { @@ -234,10 +233,8 @@ func (b *Backend) SanitizeTTL(ttlStr, maxTTLStr string) (ttl, maxTTL time.Durati if err != nil { return 0, 0, fmt.Errorf("Invalid ttl: %s", err) } - if ttl > sysMaxTTL { - return 0, 0, fmt.Errorf("\"ttl\" value must be less than allowed max lease TTL value '%s'", sysMaxTTL.String()) - } } + if len(maxTTLStr) == 0 || maxTTLStr == "0" { maxTTL = 0 } else { @@ -245,14 +242,26 @@ func (b *Backend) SanitizeTTL(ttlStr, maxTTLStr string) (ttl, maxTTL time.Durati if err != nil { return 0, 0, fmt.Errorf("Invalid max_ttl: %s", err) } - if maxTTL > sysMaxTTL { - return 0, 0, fmt.Errorf("\"max_ttl\" value must be less than allowed max lease TTL value '%s'", sysMaxTTL.String()) - } + } + + ttl, maxTTL, err = b.SanitizeTTL(ttl, maxTTL) + + return +} + +// Caps the boundaries of ttl and max_ttl values to the backend mount's max_ttl value. +func (b *Backend) SanitizeTTL(ttl, maxTTL time.Duration) (time.Duration, time.Duration, error) { + sysMaxTTL := b.System().MaxLeaseTTL() + if ttl > sysMaxTTL { + return 0, 0, fmt.Errorf("\"ttl\" value must be less than allowed max lease TTL value '%s'", sysMaxTTL.String()) + } + if maxTTL > sysMaxTTL { + return 0, 0, fmt.Errorf("\"max_ttl\" value must be less than allowed max lease TTL value '%s'", sysMaxTTL.String()) } if ttl > maxTTL && maxTTL != 0 { ttl = maxTTL } - return + return ttl, maxTTL, nil } // Route looks up the path that would be used for a given path string. From 15f29c6956f052cbf2ae435c63835d877415fa9d Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Thu, 5 May 2016 10:22:28 -0400 Subject: [PATCH 9/9] Updates to policy and string helpers --- helper/policyutil/policyutil.go | 21 +++++++++++++++---- helper/strutil/strutil.go | 37 +++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/helper/policyutil/policyutil.go b/helper/policyutil/policyutil.go index 09fee92fd5..14f25c391f 100644 --- a/helper/policyutil/policyutil.go +++ b/helper/policyutil/policyutil.go @@ -3,13 +3,29 @@ package policyutil import ( "sort" "strings" + + "github.com/hashicorp/vault/helper/strutil" ) func ParsePolicies(policiesRaw string) []string { + if policiesRaw == "" { + return []string{"default"} + } + policies := strings.Split(policiesRaw, ",") + + return SanitizePolicies(policies) +} + +func SanitizePolicies(policies []string) []string { defaultFound := false for i, p := range policies { policies[i] = strings.ToLower(strings.TrimSpace(p)) + // Eliminate unnamed policies. + if policies[i] == "" { + continue + } + // If 'root' policy is present, ignore all other policies. if policies[i] == "root" { policies = []string{"root"} @@ -26,10 +42,7 @@ func ParsePolicies(policiesRaw string) []string { policies = append(policies, "default") } - // Sort to make the computations on policies consistent. - sort.Strings(policies) - - return policies + return strutil.RemoveDuplicates(policies) } // ComparePolicies checks whether the given policy sets are equivalent, as in, diff --git a/helper/strutil/strutil.go b/helper/strutil/strutil.go index de558e8cfd..dd76bea362 100644 --- a/helper/strutil/strutil.go +++ b/helper/strutil/strutil.go @@ -1,5 +1,10 @@ package strutil +import ( + "sort" + "strings" +) + // StrListContains looks for a string in a list of strings. func StrListContains(haystack []string, needle string) bool { for _, item := range haystack { @@ -20,3 +25,35 @@ func StrListSubset(super, sub []string) bool { } return true } + +// Parses a comma separated list of strings into a slice of strings. +// The return slice will be sorted and will not contain duplicate or +// empty items. The values will be converted to lower case. +func ParseStrings(input string) []string { + var parsed []string + if input == "" { + // Don't return nil + return parsed + } + return RemoveDuplicates(strings.Split(input, ",")) +} + +// Removes duplicate and empty elements from a slice of strings. +// This also converts the items in the slice to lower case and +// returns a sorted slice. +func RemoveDuplicates(items []string) []string { + itemsMap := map[string]bool{} + for _, item := range items { + item = strings.ToLower(strings.TrimSpace(item)) + if item == "" { + continue + } + itemsMap[item] = true + } + items = []string{} + for item, _ := range itemsMap { + items = append(items, item) + } + sort.Strings(items) + return items +}