From 6b0c6923850c442c5fdb3855a89b4ff17d287ce9 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Fri, 26 Feb 2016 19:43:55 -0500 Subject: [PATCH 01/42] Provide 'sys/step-down' and 'vault step-down' This endpoint causes the node it's hit to step down from active duty. It's a noop if the node isn't active or not running in HA mode. The node will wait one second before attempting to reacquire the lock, to give other nodes a chance to grab it. Fixes #1093 --- api/sys_stepdown.go | 10 + cli/commands.go | 6 + command/step-down.go | 54 ++++++ http/handler.go | 1 + http/sys_seal.go | 23 +++ http/sys_seal_test.go | 10 + vault/core.go | 69 +++++-- vault/core_test.go | 176 ++++++++++++++++++ website/source/docs/http/sys-seal.html.md | 4 +- .../source/docs/http/sys-step-down.html.md | 33 ++++ website/source/layouts/http.erb | 3 + 11 files changed, 374 insertions(+), 15 deletions(-) create mode 100644 api/sys_stepdown.go create mode 100644 command/step-down.go create mode 100644 website/source/docs/http/sys-step-down.html.md diff --git a/api/sys_stepdown.go b/api/sys_stepdown.go new file mode 100644 index 0000000000..421e5f19fb --- /dev/null +++ b/api/sys_stepdown.go @@ -0,0 +1,10 @@ +package api + +func (c *Sys) StepDown() error { + r := c.c.NewRequest("PUT", "/v1/sys/step-down") + resp, err := c.c.RawRequest(r) + if err == nil { + defer resp.Body.Close() + } + return err +} diff --git a/cli/commands.go b/cli/commands.go index 05f5c74795..1f5b89f911 100644 --- a/cli/commands.go +++ b/cli/commands.go @@ -224,6 +224,12 @@ func Commands(metaPtr *command.Meta) map[string]cli.CommandFactory { }, nil }, + "step-down": func() (cli.Command, error) { + return &command.StepDownCommand{ + Meta: meta, + }, nil + }, + "mount": func() (cli.Command, error) { return &command.MountCommand{ Meta: meta, diff --git a/command/step-down.go b/command/step-down.go new file mode 100644 index 0000000000..1f2448e560 --- /dev/null +++ b/command/step-down.go @@ -0,0 +1,54 @@ +package command + +import ( + "fmt" + "strings" +) + +// StepDownCommand is a Command that seals the vault. +type StepDownCommand struct { + Meta +} + +func (c *StepDownCommand) Run(args []string) int { + flags := c.Meta.FlagSet("step-down", FlagSetDefault) + flags.Usage = func() { c.Ui.Error(c.Help()) } + if err := flags.Parse(args); err != nil { + return 1 + } + + client, err := c.Client() + if err != nil { + c.Ui.Error(fmt.Sprintf( + "Error initializing client: %s", err)) + return 2 + } + + if err := client.Sys().StepDown(); err != nil { + c.Ui.Error(fmt.Sprintf("Error stepping down: %s", err)) + return 1 + } + + return 0 +} + +func (c *StepDownCommand) Synopsis() string { + return "Force the Vault node to give up active duty" +} + +func (c *StepDownCommand) Help() string { + helpText := ` +Usage: vault step-down [options] + + Force the Vault node to step down from active duty. + + This causes the indicated node to give up active status. Note that while the + affected node will have a short delay before attempting to grab the lock + again, if no other node grabs the lock beforehand, it is possible for the + same node to re-grab the lock and become active again. + +General Options: + + ` + generalOptionsUsage() + return strings.TrimSpace(helpText) +} diff --git a/http/handler.go b/http/handler.go index bd2f2dafc7..5508a9539a 100644 --- a/http/handler.go +++ b/http/handler.go @@ -23,6 +23,7 @@ func Handler(core *vault.Core) http.Handler { mux.Handle("/v1/sys/init", handleSysInit(core)) mux.Handle("/v1/sys/seal-status", handleSysSealStatus(core)) mux.Handle("/v1/sys/seal", handleSysSeal(core)) + mux.Handle("/v1/sys/step-down", handleSysStepDown(core)) mux.Handle("/v1/sys/unseal", handleSysUnseal(core)) mux.Handle("/v1/sys/mounts", proxySysRequest(core)) mux.Handle("/v1/sys/mounts/", proxySysRequest(core)) diff --git a/http/sys_seal.go b/http/sys_seal.go index d5ac76624f..a11a2078b7 100644 --- a/http/sys_seal.go +++ b/http/sys_seal.go @@ -34,6 +34,29 @@ func handleSysSeal(core *vault.Core) http.Handler { }) } +func handleSysStepDown(core *vault.Core) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case "PUT": + case "POST": + default: + respondError(w, http.StatusMethodNotAllowed, nil) + return + } + + // Get the auth for the request so we can access the token directly + req := requestAuth(r, &logical.Request{}) + + // Seal with the token above + if err := core.StepDown(req.ClientToken); err != nil { + respondError(w, http.StatusInternalServerError, err) + return + } + + respondOk(w, nil) + }) +} + func handleSysUnseal(core *vault.Core) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.Method { diff --git a/http/sys_seal_test.go b/http/sys_seal_test.go index 4b30082760..e1cca89a6f 100644 --- a/http/sys_seal_test.go +++ b/http/sys_seal_test.go @@ -304,3 +304,13 @@ func TestSysSeal_Permissions(t *testing.T) { httpResp = testHttpPut(t, "child", addr+"/v1/sys/seal", nil) testResponseStatus(t, httpResp, 204) } + +func TestSysStepDown(t *testing.T) { + core, _, token := vault.TestCoreUnsealed(t) + ln, addr := TestServer(t, core) + defer ln.Close() + TestServerAuth(t, addr, token) + + resp := testHttpPut(t, token, addr+"/v1/sys/step-down", nil) + testResponseStatus(t, resp, 204) +} diff --git a/vault/core.go b/vault/core.go index 75df726016..fa54058430 100644 --- a/vault/core.go +++ b/vault/core.go @@ -1157,22 +1157,45 @@ func (c *Core) Unseal(key []byte) (bool, error) { return true, nil } -// Seal is used to re-seal the Vault. This requires the Vault to -// be unsealed again to perform any further operations. -func (c *Core) Seal(token string) (retErr error) { - defer metrics.MeasureSince([]string{"core", "seal"}, time.Now()) +// Seal is used to seal the vault +func (c *Core) Seal(token string) error { + return c.stepDownAndSeal(token, true) +} + +// StepDown is used to step down from leadership +func (c *Core) StepDown(token string) error { + return c.stepDownAndSeal(token, false) +} + +// stepDownAndSeal is used to step down from leadership and, optionally, +// re-seal the Vault. If sealed, this requires the Vault to be unsealed again +// to perform any further operations. +func (c *Core) stepDownAndSeal(token string, seal bool) (retErr error) { + if seal { + defer metrics.MeasureSince([]string{"core", "seal"}, time.Now()) + } else { + defer metrics.MeasureSince([]string{"core", "step_down"}, time.Now()) + } + c.stateLock.Lock() defer c.stateLock.Unlock() if c.sealed { return nil } + if !seal && (c.ha == nil || c.standby) { + return nil + } // Validate the token is a root token req := &logical.Request{ Operation: logical.UpdateOperation, - Path: "sys/seal", ClientToken: token, } + if seal { + req.Path = "sys/seal" + } else { + req.Path = "sys/step-down" + } acl, te, err := c.fetchACLandTokenEntry(req) // Attempt to use the token (decrement num_uses) @@ -1189,8 +1212,8 @@ func (c *Core) Seal(token string) (retErr error) { // just returning with an error and recommending a vault restart, which // essentially does the same thing. if c.standby { - c.logger.Printf("[ERR] core: vault cannot be sealed when in standby mode; please restart instead") - return errors.New("vault cannot be sealed when in standby mode; please restart instead") + c.logger.Printf("[ERR] core: vault cannot step down or be sealed when in standby mode; please restart instead") + return errors.New("vault cannot step down or be sealed when in standby mode; please restart instead") } return err } @@ -1207,19 +1230,22 @@ func (c *Core) Seal(token string) (retErr error) { } // Seal the Vault - err = c.sealInternal() - if err == nil && retErr == ErrInternalError { - c.logger.Printf("[ERR] core: core is successfully sealed but another error occurred during the operation") + if seal { + err = c.sealInternal() + if err == nil && retErr == ErrInternalError { + c.logger.Printf("[ERR] core: core is successfully sealed but another error occurred during the operation") + } else { + retErr = err + } } else { - retErr = err + c.stepDownInternal() } return } -// sealInternal is an internal method used to seal the vault. -// It does not do any authorization checking. The stateLock must -// be held prior to calling. +// sealInternal is an internal method used to seal the vault. It does not do +// any authorization checking. The stateLock must be held prior to calling. func (c *Core) sealInternal() error { // Enable that we are sealed to prevent furthur transactions c.sealed = true @@ -1244,9 +1270,20 @@ func (c *Core) sealInternal() error { return err } c.logger.Printf("[INFO] core: vault is sealed") + return nil } +// stepDownInternal is an internal method used to step down from active duty. +// It does not do any authorization checking. +func (c *Core) stepDownInternal() { + // Merely trigger the loop to re-run. This value will cause the + // loop to run through giving up leadership, but without triggering + // the return at the end of the next loop run, since it's not + // closed + c.standbyStopCh <- struct{}{} +} + // postUnseal is invoked after the barrier is unsealed, but before // allowing any user operations. This allows us to setup any state that // requires the Vault to be unsealed such as mount tables, logical backends, @@ -1443,6 +1480,10 @@ func (c *Core) runStandby(doneCh, stopCh chan struct{}) { if preSealErr != nil { c.logger.Printf("[ERR] core: pre-seal teardown failed: %v", err) } + + // If we've merely stepped down, we could instantly grab the lock + // again. Give the other nodes a chance. + time.Sleep(time.Second) } } diff --git a/vault/core_test.go b/vault/core_test.go index 1f9f80bd0b..e597d767e2 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -1183,6 +1183,182 @@ func TestCore_Standby_Seal(t *testing.T) { } } +func TestCore_StepDown(t *testing.T) { + // Create the first core and initialize it + inm := physical.NewInmem() + inmha := physical.NewInmemHA() + advertiseOriginal := "http://127.0.0.1:8200" + core, err := NewCore(&CoreConfig{ + Physical: inm, + HAPhysical: inmha, + AdvertiseAddr: advertiseOriginal, + DisableMlock: true, + }) + if err != nil { + t.Fatalf("err: %v", err) + } + key, root := TestCoreInit(t, core) + if _, err := core.Unseal(TestKeyCopy(key)); err != nil { + t.Fatalf("unseal err: %s", err) + } + + // Verify unsealed + sealed, err := core.Sealed() + if err != nil { + t.Fatalf("err checking seal status: %s", err) + } + if sealed { + t.Fatal("should not be sealed") + } + + // Wait for core to become active + testWaitActive(t, core) + + // Ensure that the original clean function has stopped running + time.Sleep(2 * time.Second) + + // Check the leader is local + isLeader, advertise, err := core.Leader() + if err != nil { + t.Fatalf("err: %v", err) + } + if !isLeader { + t.Fatalf("should be leader") + } + if advertise != advertiseOriginal { + t.Fatalf("Bad advertise: %v", advertise) + } + + // Create the second core and initialize it + advertiseOriginal2 := "http://127.0.0.1:8500" + core2, err := NewCore(&CoreConfig{ + Physical: inm, + HAPhysical: inmha, + AdvertiseAddr: advertiseOriginal2, + DisableMlock: true, + }) + if err != nil { + t.Fatalf("err: %v", err) + } + if _, err := core2.Unseal(TestKeyCopy(key)); err != nil { + t.Fatalf("unseal err: %s", err) + } + + // Verify unsealed + sealed, err = core2.Sealed() + if err != nil { + t.Fatalf("err checking seal status: %s", err) + } + if sealed { + t.Fatal("should not be sealed") + } + + // Core2 should be in standby + standby, err := core2.Standby() + if err != nil { + t.Fatalf("err: %v", err) + } + if !standby { + t.Fatalf("should be standby") + } + + // Check the leader is not local + isLeader, advertise, err = core2.Leader() + if err != nil { + t.Fatalf("err: %v", err) + } + if isLeader { + t.Fatalf("should not be leader") + } + if advertise != advertiseOriginal { + t.Fatalf("Bad advertise: %v", advertise) + } + + // Step down core + err = core.StepDown(root) + if err != nil { + t.Fatal("error stepping down core 1") + } + + // Give time to switch leaders + time.Sleep(2 * time.Second) + + // Core1 should be in standby + standby, err = core.Standby() + if err != nil { + t.Fatalf("err: %v", err) + } + if !standby { + t.Fatalf("should be standby") + } + + // Check the leader is core2 + isLeader, advertise, err = core2.Leader() + if err != nil { + t.Fatalf("err: %v", err) + } + if !isLeader { + t.Fatalf("should be leader") + } + if advertise != advertiseOriginal2 { + t.Fatalf("Bad advertise: %v", advertise) + } + + // Check the leader is not local + isLeader, advertise, err = core.Leader() + if err != nil { + t.Fatalf("err: %v", err) + } + if isLeader { + t.Fatalf("should not be leader") + } + if advertise != advertiseOriginal2 { + t.Fatalf("Bad advertise: %v", advertise) + } + + // Step down core2 + err = core2.StepDown(root) + if err != nil { + t.Fatal("error stepping down core 1") + } + + // Give time to switch leaders + time.Sleep(2 * time.Second) + + // Core2 should be in standby + standby, err = core2.Standby() + if err != nil { + t.Fatalf("err: %v", err) + } + if !standby { + t.Fatalf("should be standby") + } + + // Check the leader is core1 + isLeader, advertise, err = core.Leader() + if err != nil { + t.Fatalf("err: %v", err) + } + if !isLeader { + t.Fatalf("should be leader") + } + if advertise != advertiseOriginal { + t.Fatalf("Bad advertise: %v", advertise) + } + + // Check the leader is not local + isLeader, advertise, err = core2.Leader() + if err != nil { + t.Fatalf("err: %v", err) + } + if isLeader { + t.Fatalf("should not be leader") + } + if advertise != advertiseOriginal { + t.Fatalf("Bad advertise: %v", advertise) + } +} + func TestCore_CleanLeaderPrefix(t *testing.T) { // Create the first core and initialize it inm := physical.NewInmem() diff --git a/website/source/docs/http/sys-seal.html.md b/website/source/docs/http/sys-seal.html.md index 55d5a81a9c..d82b9af386 100644 --- a/website/source/docs/http/sys-seal.html.md +++ b/website/source/docs/http/sys-seal.html.md @@ -11,7 +11,9 @@ description: |-
Description
- Seals the Vault. In HA mode, only an active node can be sealed. Standby nodes should be restarted to get the same effect. + Seals the Vault. In HA mode, only an active node can be sealed. Standby + nodes should be restarted to get the same effect. Requires a token with + `root` policy or `sudo` capability on the path.
Method
diff --git a/website/source/docs/http/sys-step-down.html.md b/website/source/docs/http/sys-step-down.html.md new file mode 100644 index 0000000000..94f5aa4c29 --- /dev/null +++ b/website/source/docs/http/sys-step-down.html.md @@ -0,0 +1,33 @@ +--- +layout: "http" +page_title: "HTTP API: /sys/step-down" +sidebar_current: "docs-http-ha-step-down" +description: |- + The '/sys/step-down' endpoint causes the node to give up active status. +--- + +# /sys/seal + +
+
Description
+
+ Forces the node to give up active status. If the node does not have active + status, this endpoint does nothing. Note that the node will sleep for a + second before attempting to grab the active lock again, but if no standby + nodes grab the active lock in the interim, the same node may become the + active node again. Requires a token with `root` policy or `sudo` capability + on the path. +
+ +
Method
+
PUT
+ +
Parameters
+
+ None +
+ +
Returns
+
A `204` response code. +
+
diff --git a/website/source/layouts/http.erb b/website/source/layouts/http.erb index ac69d5651e..ee75b37f9c 100644 --- a/website/source/layouts/http.erb +++ b/website/source/layouts/http.erb @@ -107,6 +107,9 @@ > /sys/leader + > + /sys/step-down + From ef4466d6d3b1ea81b4d0ce4c9aa6a06ccf34166a Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Sun, 28 Feb 2016 21:35:32 -0500 Subject: [PATCH 02/42] Address review feedback --- vault/core.go | 151 ++++++++++++++++++++++++++------------------- vault/core_test.go | 5 +- 2 files changed, 92 insertions(+), 64 deletions(-) diff --git a/vault/core.go b/vault/core.go index fa54058430..1340c0f93c 100644 --- a/vault/core.go +++ b/vault/core.go @@ -64,6 +64,10 @@ const ( // leaderPrefixCleanDelay is how long to wait between deletions // of orphaned leader keys, to prevent slamming the backend. leaderPrefixCleanDelay = 200 * time.Millisecond + + // manualStepDownSleepPeriod is how long to sleep after a user-initiated + // step down of the active node, to prevent instantly regrabbing the lock + manualStepDownSleepPeriod = 10 * time.Second ) var ( @@ -206,9 +210,10 @@ type Core struct { stateLock sync.RWMutex sealed bool - standby bool - standbyDoneCh chan struct{} - standbyStopCh chan struct{} + standby bool + standbyDoneCh chan struct{} + standbyStopCh chan struct{} + manualStepDownCh chan struct{} // unlockParts has the keys provided to Unseal until // the threshold number of parts is available. @@ -1149,7 +1154,8 @@ func (c *Core) Unseal(key []byte) (bool, error) { // Go to standby mode, wait until we are active to unseal c.standbyDoneCh = make(chan struct{}) c.standbyStopCh = make(chan struct{}) - go c.runStandby(c.standbyDoneCh, c.standbyStopCh) + c.manualStepDownCh = make(chan struct{}) + go c.runStandby(c.standbyDoneCh, c.standbyStopCh, c.manualStepDownCh) } // Success! @@ -1157,54 +1163,25 @@ func (c *Core) Unseal(key []byte) (bool, error) { return true, nil } -// Seal is used to seal the vault -func (c *Core) Seal(token string) error { - return c.stepDownAndSeal(token, true) -} - -// StepDown is used to step down from leadership -func (c *Core) StepDown(token string) error { - return c.stepDownAndSeal(token, false) -} - -// stepDownAndSeal is used to step down from leadership and, optionally, -// re-seal the Vault. If sealed, this requires the Vault to be unsealed again -// to perform any further operations. -func (c *Core) stepDownAndSeal(token string, seal bool) (retErr error) { - if seal { - defer metrics.MeasureSince([]string{"core", "seal"}, time.Now()) - } else { - defer metrics.MeasureSince([]string{"core", "step_down"}, time.Now()) - } +// Seal is used to re-seal the Vault. This requires the Vault to +// be unsealed again to perform any further operations. +func (c *Core) Seal(token string) (retErr error) { + defer metrics.MeasureSince([]string{"core", "seal"}, time.Now()) c.stateLock.Lock() defer c.stateLock.Unlock() if c.sealed { return nil } - if !seal && (c.ha == nil || c.standby) { - return nil - } // Validate the token is a root token req := &logical.Request{ Operation: logical.UpdateOperation, + Path: "sys/seal", ClientToken: token, } - if seal { - req.Path = "sys/seal" - } else { - req.Path = "sys/step-down" - } - acl, te, err := c.fetchACLandTokenEntry(req) - // Attempt to use the token (decrement num_uses) - if te != nil { - if err := c.tokenStore.UseToken(te); err != nil { - c.logger.Printf("[ERR] core: failed to use token: %v", err) - retErr = ErrInternalError - } - } + acl, te, err := c.fetchACLandTokenEntry(req) if err != nil { // Since there is no token store in standby nodes, sealing cannot // be done. Ideally, the request has to be forwarded to leader node @@ -1212,11 +1189,20 @@ func (c *Core) stepDownAndSeal(token string, seal bool) (retErr error) { // just returning with an error and recommending a vault restart, which // essentially does the same thing. if c.standby { - c.logger.Printf("[ERR] core: vault cannot step down or be sealed when in standby mode; please restart instead") - return errors.New("vault cannot step down or be sealed when in standby mode; please restart instead") + c.logger.Printf("[ERR] core: vault cannot seal when in standby mode; please restart instead") + return errors.New("vault cannot seal when in standby mode; please restart instead") } return err } + // Attempt to use the token (decrement num_uses) + // If we can't, we still continue attempting the seal, so long as the token + // has appropriate permissions + if te != nil { + if err := c.tokenStore.UseToken(te); err != nil { + c.logger.Printf("[ERR] core: failed to use token: %v", err) + retErr = ErrInternalError + } + } // Verify that this operation is allowed allowed, rootPrivs := acl.AllowOperation(req.Operation, req.Path) @@ -1229,21 +1215,65 @@ func (c *Core) stepDownAndSeal(token string, seal bool) (retErr error) { return logical.ErrPermissionDenied } - // Seal the Vault - if seal { - err = c.sealInternal() - if err == nil && retErr == ErrInternalError { - c.logger.Printf("[ERR] core: core is successfully sealed but another error occurred during the operation") - } else { - retErr = err - } + //Seal the Vault + err = c.sealInternal() + if err == nil && retErr == ErrInternalError { + c.logger.Printf("[ERR] core: core is successfully sealed but another error occurred during the operation") } else { - c.stepDownInternal() + retErr = err } return } +// StepDown is used to step down from leadership +func (c *Core) StepDown(token string) error { + defer metrics.MeasureSince([]string{"core", "step_down"}, time.Now()) + + c.stateLock.Lock() + defer c.stateLock.Unlock() + if c.sealed { + return nil + } + if c.ha == nil || c.standby { + return nil + } + + // Validate the token is a root token + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "sys/step-down", + ClientToken: token, + } + + acl, te, err := c.fetchACLandTokenEntry(req) + if err != nil { + return err + } + // Attempt to use the token (decrement num_uses) + if te != nil { + if err := c.tokenStore.UseToken(te); err != nil { + c.logger.Printf("[ERR] core: failed to use token: %v", err) + return err + } + } + + // Verify that this operation is allowed + allowed, rootPrivs := acl.AllowOperation(req.Operation, req.Path) + if !allowed { + return logical.ErrPermissionDenied + } + + // We always require root privileges for this operation + if !rootPrivs { + return logical.ErrPermissionDenied + } + + c.manualStepDownCh <- struct{}{} + + return nil +} + // sealInternal is an internal method used to seal the vault. It does not do // any authorization checking. The stateLock must be held prior to calling. func (c *Core) sealInternal() error { @@ -1274,16 +1304,6 @@ func (c *Core) sealInternal() error { return nil } -// stepDownInternal is an internal method used to step down from active duty. -// It does not do any authorization checking. -func (c *Core) stepDownInternal() { - // Merely trigger the loop to re-run. This value will cause the - // loop to run through giving up leadership, but without triggering - // the return at the end of the next loop run, since it's not - // closed - c.standbyStopCh <- struct{}{} -} - // postUnseal is invoked after the barrier is unsealed, but before // allowing any user operations. This allows us to setup any state that // requires the Vault to be unsealed such as mount tables, logical backends, @@ -1390,8 +1410,9 @@ func (c *Core) preSeal() error { // runStandby is a long running routine that is used when an HA backend // is enabled. It waits until we are leader and switches this Vault to // active. -func (c *Core) runStandby(doneCh, stopCh chan struct{}) { +func (c *Core) runStandby(doneCh, stopCh, manualStepDownCh chan struct{}) { defer close(doneCh) + defer close(manualStepDownCh) c.logger.Printf("[INFO] core: entering standby mode") // Monitor for key rotation @@ -1455,11 +1476,15 @@ func (c *Core) runStandby(doneCh, stopCh chan struct{}) { } // Monitor a loss of leadership + var manualStepDown bool select { case <-leaderLostCh: c.logger.Printf("[WARN] core: leadership lost, stopping active operation") case <-stopCh: c.logger.Printf("[WARN] core: stopping active operation") + case <-manualStepDownCh: + c.logger.Printf("[WARN] core: stepping down from active operation to standby") + manualStepDown = true } // Clear ourself as leader @@ -1483,7 +1508,9 @@ func (c *Core) runStandby(doneCh, stopCh chan struct{}) { // If we've merely stepped down, we could instantly grab the lock // again. Give the other nodes a chance. - time.Sleep(time.Second) + if manualStepDown { + time.Sleep(manualStepDownSleepPeriod) + } } } diff --git a/vault/core_test.go b/vault/core_test.go index e597d767e2..c66cf1fa82 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -1322,8 +1322,9 @@ func TestCore_StepDown(t *testing.T) { t.Fatal("error stepping down core 1") } - // Give time to switch leaders - time.Sleep(2 * time.Second) + // Give time to switch leaders -- core 1 will still be waiting on its + // cooling off period so give it a full 10 seconds to recover + time.Sleep(10 * time.Second) // Core2 should be in standby standby, err = core2.Standby() From d0ec85f4ba5d9f033a190a759fb791a6a60acde7 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 29 Feb 2016 10:09:11 -0500 Subject: [PATCH 03/42] Update doc, it's now 10 seconds --- website/source/docs/http/sys-step-down.html.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/source/docs/http/sys-step-down.html.md b/website/source/docs/http/sys-step-down.html.md index 94f5aa4c29..ee6b1d82fd 100644 --- a/website/source/docs/http/sys-step-down.html.md +++ b/website/source/docs/http/sys-step-down.html.md @@ -12,8 +12,8 @@ description: |-
Description
Forces the node to give up active status. If the node does not have active - status, this endpoint does nothing. Note that the node will sleep for a - second before attempting to grab the active lock again, but if no standby + status, this endpoint does nothing. Note that the node will sleep for ten + seconds before attempting to grab the active lock again, but if no standby nodes grab the active lock in the interim, the same node may become the active node again. Requires a token with `root` policy or `sudo` capability on the path. From a40e0fc8d4e5521916c1f0145663a9d81847e022 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 1 Mar 2016 10:12:50 -0500 Subject: [PATCH 04/42] zeroaddress documentation fix --- website/source/docs/secrets/ssh/index.html.md | 105 +++++++++++++++++- 1 file changed, 102 insertions(+), 3 deletions(-) diff --git a/website/source/docs/secrets/ssh/index.html.md b/website/source/docs/secrets/ssh/index.html.md index 3266cea035..8344b44833 100644 --- a/website/source/docs/secrets/ssh/index.html.md +++ b/website/source/docs/secrets/ssh/index.html.md @@ -206,8 +206,12 @@ $ vault write ssh/roles/dynamic_key_role \ Success! Data written to: ssh/roles/dynamic_key_role ``` -`cidr_list` is optional and defaults to the zero address (0.0.0.0/0), e.g. all -hosts. +`cidr_list` is a comma separated list of CIDR blocks for which a role can generate +credentials for. If this is empty, the role can only generate credentials if it belongs +to the set of zero-address roles. + +Zero-address roles, configured via `/ssh/config/zeroaddress` endpoint, takes comma separated list +of role names that can generate credentials for any IP address. Use the `install_script` option to provide an install script if the remote hosts do not resemble a typical Linux machine. The default script is compiled @@ -388,7 +392,6 @@ username@ip:~$ (String) Comma separated list of CIDR blocks for which the role is applicable for. CIDR blocks can belong to more than one role. - Defaults to the zero address (0.0.0.0/0).
  • exclude_cidr_list @@ -559,6 +562,102 @@ username@ip:~$
    A `204` response code.
    + +### /ssh/config/zeroaddress + +#### GET + +
    +
    Description
    +
    + Returns the list of configured zero-address roles. +
    + +
    Method
    +
    GET
    + +
    URL
    +
    `/ssh/config/zeroaddress`
    + +
    Parameters
    +
    None
    + +
    Returns
    +
    + +```json +{ + "lease_id":"", + "renewable":false, + "lease_duration":0, + "data":{ + "roles":[ + "otp_key_role" + ] + }, + "warnings":null, + "auth":null +} +``` + +
    +#### POST + +
    +
    Description
    +
    + Configures zero-address roles. +
    + +
    Method
    +
    POST
    + +
    URL
    +
    `/ssh/config/zeroaddress`
    + +
    Parameters
    +
    +
      +
    • + roles + required + (String) + Comma separated list of role names which allows credentials to be requested + for any IP address. CIDR blocks previously registered under these roles will + be ignored. +
    • +
    +
    + +
    Returns
    +
    + A `204` response code. +
    + +#### DELETE + +
    +
    Description
    +
    + Deletes the zero-address roles configuration. +
    + +
    Method
    +
    DELETE
    + +
    URL
    +
    `/ssh/config/zeroaddress`
    + +
    Parameters
    +
    None
    + +
    Returns
    +
    + A `204` response code. +
    + + + ### /ssh/creds/ #### POST From 8feae7eb1fc93b997688ee192089be74b6dbb229 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 1 Mar 2016 11:21:29 -0500 Subject: [PATCH 05/42] removed datatype and corrected a sentense --- website/source/docs/secrets/ssh/index.html.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/website/source/docs/secrets/ssh/index.html.md b/website/source/docs/secrets/ssh/index.html.md index 8344b44833..4f5d386493 100644 --- a/website/source/docs/secrets/ssh/index.html.md +++ b/website/source/docs/secrets/ssh/index.html.md @@ -207,7 +207,7 @@ Success! Data written to: ssh/roles/dynamic_key_role ``` `cidr_list` is a comma separated list of CIDR blocks for which a role can generate -credentials for. If this is empty, the role can only generate credentials if it belongs +credentials. If this is empty, the role can only generate credentials if it belongs to the set of zero-address roles. Zero-address roles, configured via `/ssh/config/zeroaddress` endpoint, takes comma separated list @@ -621,10 +621,8 @@ username@ip:~$
  • roles required - (String) - Comma separated list of role names which allows credentials to be requested - for any IP address. CIDR blocks previously registered under these roles will - be ignored. + A string containing comma separated list of role names which allows credentials to be requested + for any IP address. CIDR blocks previously registered under these roles will be ignored.
  • From 01d61f6f0cbe6fb93b53289adf4405008fc9527b Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 1 Mar 2016 11:48:17 -0500 Subject: [PATCH 06/42] fix typo --- command/policy_delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/policy_delete.go b/command/policy_delete.go index daa6963f6f..046d88c17d 100644 --- a/command/policy_delete.go +++ b/command/policy_delete.go @@ -53,7 +53,7 @@ Usage: vault policy-delete [options] name Delete a policy with the given name. - One the policy is deleted, all users associated with the policy will + Once the policy is deleted, all users associated with the policy will be affected immediately. When a user is associated with a policy that doesn't exist, it is identical to not being associated with that policy. From c506988cde55dc2fa23cfc7645c7b32cf9a52954 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Mon, 29 Feb 2016 10:40:15 -0500 Subject: [PATCH 07/42] supporting non-ca certs for verification --- builtin/credential/cert/path_certs.go | 20 +++++- builtin/credential/cert/path_login.go | 90 +++++++++++++++++++-------- 2 files changed, 82 insertions(+), 28 deletions(-) diff --git a/builtin/credential/cert/path_certs.go b/builtin/credential/cert/path_certs.go index c59429c5cb..b11c4237b7 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -1,6 +1,7 @@ package cert import ( + "crypto/x509" "fmt" "strings" "time" @@ -51,7 +52,7 @@ Defaults to system/backend default TTL time.`, Callbacks: map[logical.Operation]framework.OperationFunc{ logical.DeleteOperation: b.pathCertDelete, logical.ReadOperation: b.pathCertRead, - logical.UpdateOperation: b.pathCertWrite, + logical.UpdateOperation: b.pathCertWrite, }, HelpSynopsis: pathCertHelpSyn, @@ -132,6 +133,22 @@ func (b *backend) pathCertWrite( return logical.ErrorResponse("failed to parse certificate"), nil } + // If the certificate is not a CA cert, then ensure that x509.ExtKeyUsageClientAuth is set + if !parsed[0].IsCA { + if parsed[0].ExtKeyUsage == nil { + return logical.ErrorResponse("non-CA certificates should have TLS web client authentication set as an extended key usage"), nil + } + var bool clientAuth + for _, usage := range parsed[0].ExtKeyUsage { + if usage == x509.ExtKeyUsageClientAuth { + clientAuth = true + } + } + if !clientAuth { + return logical.ErrorResponse("non-CA certificates should have TLS web client authentication set as an extended key usage"), nil + } + } + certEntry := &CertEntry{ Name: name, Certificate: certificate, @@ -140,7 +157,6 @@ func (b *backend) pathCertWrite( } // Parse the lease duration or default to backend/system default - var err error maxTTL := b.System().MaxLeaseTTL() ttl := time.Duration(d.Get("ttl").(int)) * time.Second if ttl == time.Duration(0) { diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index ad27fb3fb5..541c4d1fcf 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -134,28 +134,54 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical connState := req.Connection.ConnState // Load the trusted certificates - roots, trusted := b.loadTrustedCerts(req.Storage) + roots, trusted, nonCAPool, trustedNonCAs := b.loadTrustedCerts(req.Storage) // Validate the connection state is trusted - trustedChains, err := validateConnState(roots, connState) - if err != nil { - return nil, nil, err - } + // If trustedNonCAs is not empty it means that client had registered a non-CA cert + // with the backend. + if len(trustedNonCAs) != 0 { + _, err := validateConnState(nonCAPool, connState) + if err != nil { + return nil, nil, err + } + // Match the trusted non-CA chain with the policy + return b.matchNonCAPolicy(connState.PeerCertificates, trustedNonCAs), nil, nil + } else { + trustedChains, err := validateConnState(roots, connState) + if err != nil { + return nil, nil, err + } - // If no trusted chain was found, client is not authenticated - if len(trustedChains) == 0 { - return nil, logical.ErrorResponse("invalid certificate or no client certificate supplied"), nil - } + // If no trusted chain was found, client is not authenticated + if len(trustedChains) == 0 { + return nil, logical.ErrorResponse("invalid certificate or no client certificate supplied"), nil + } - validChain := b.checkForValidChain(req.Storage, trustedChains) - if !validChain { - return nil, logical.ErrorResponse( - "no chain containing non-revoked certificates could be found for this login certificate", - ), nil - } + validChain := b.checkForValidChain(req.Storage, trustedChains) + if !validChain { + return nil, logical.ErrorResponse( + "no chain containing non-revoked certificates could be found for this login certificate", + ), nil + } - // Match the trusted chain with the policy - return b.matchPolicy(trustedChains, trusted), nil, nil + // Match the trusted chain with the policy + return b.matchPolicy(trustedChains, trusted), nil, nil + } +} + +// matchNonCAPolicy is used to match the client cert with the registered non-CA +// policy to establish client identity. +func (b *backend) matchNonCAPolicy(peerCertificates []*x509.Certificate, trustedNonCAs []*ParsedCert) *ParsedCert { + for _, trustedNonCA := range trustedNonCAs { + for _, tCert := range trustedNonCA.Certificates { + for _, cCert := range peerCertificates { + if tCert.Equal(cCert) { + return trustedNonCA + } + } + } + } + return nil } // matchPolicy is used to match the associated policy with the certificate that @@ -177,8 +203,9 @@ func (b *backend) matchPolicy(chains [][]*x509.Certificate, trusted []*ParsedCer } // loadTrustedCerts is used to load all the trusted certificates from the backend -func (b *backend) loadTrustedCerts(store logical.Storage) (pool *x509.CertPool, trusted []*ParsedCert) { +func (b *backend) loadTrustedCerts(store logical.Storage) (pool *x509.CertPool, trusted []*ParsedCert, nonCAPool *x509.CertPool, trustedNonCAs []*ParsedCert) { pool = x509.NewCertPool() + nonCAPool = x509.NewCertPool() names, err := store.List("cert/") if err != nil { b.Logger().Printf("[ERR] cert: failed to list trusted certs: %v", err) @@ -195,15 +222,25 @@ func (b *backend) loadTrustedCerts(store logical.Storage) (pool *x509.CertPool, b.Logger().Printf("[ERR] cert: failed to parse certificate for '%s'", name) continue } - for _, p := range parsed { - pool.AddCert(p) - } + if !parsed[0].IsCA { + for _, p := range parsed { + nonCAPool.AddCert(p) + } + trustedNonCAs = append(trustedNonCAs, &ParsedCert{ + Entry: entry, + Certificates: parsed, + }) + } else { + for _, p := range parsed { + pool.AddCert(p) + } - // Create a ParsedCert entry - trusted = append(trusted, &ParsedCert{ - Entry: entry, - Certificates: parsed, - }) + // Create a ParsedCert entry + trusted = append(trusted, &ParsedCert{ + Entry: entry, + Certificates: parsed, + }) + } } return } @@ -257,6 +294,7 @@ func validateConnState(roots *x509.CertPool, cs *tls.ConnectionState) ([][]*x509 Intermediates: x509.NewCertPool(), KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, } + certs := cs.PeerCertificates if len(certs) == 0 { return nil, nil From 9e610f64171417b52a086025ebd50f9d89d5d313 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Mon, 29 Feb 2016 16:02:19 -0500 Subject: [PATCH 08/42] Added testcase for cert writes --- builtin/credential/cert/backend_test.go | 40 +++++++++++++++++-- builtin/credential/cert/path_certs.go | 2 +- .../cert/test-fixtures/noclientauthcert.pem | 19 +++++++++ 3 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 builtin/credential/cert/test-fixtures/noclientauthcert.pem diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index 341c288dc1..2d03041209 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -27,6 +27,33 @@ func testFactory(t *testing.T) logical.Backend { return b } +// Test the certificates being registered to the backend +func TestBackend_CertWrites(t *testing.T) { + // CA cert + ca1, err := ioutil.ReadFile("test-fixtures/root/rootcacert.pem") + if err != nil { + t.Fatalf("err: %v", err) + } + // Non CA Cert + ca2, err := ioutil.ReadFile("test-fixtures/keys/cert.pem") + if err != nil { + t.Fatalf("err: %v", err) + } + // Non CA cert without TLS web client authentication + ca3, err := ioutil.ReadFile("test-fixtures/noclientauthcert.pem") + if err != nil { + t.Fatalf("err: %v", err) + } + logicaltest.Test(t, logicaltest.TestCase{ + Backend: testFactory(t), + Steps: []logicaltest.TestStep{ + testAccStepCert(t, "web", ca1, "foo", false), + testAccStepCert(t, "web", ca2, "foo", false), + testAccStepCert(t, "web", ca3, "foo", true), + }, + }) +} + // Test a client trusted by a CA func TestBackend_basic_CA(t *testing.T) { connState := testConnState(t, "test-fixtures/keys/cert.pem", @@ -38,7 +65,7 @@ func TestBackend_basic_CA(t *testing.T) { logicaltest.Test(t, logicaltest.TestCase{ Backend: testFactory(t), Steps: []logicaltest.TestStep{ - testAccStepCert(t, "web", ca, "foo"), + testAccStepCert(t, "web", ca, "foo", false), testAccStepLogin(t, connState), testAccStepCertLease(t, "web", ca, "foo"), testAccStepCertTTL(t, "web", ca, "foo"), @@ -86,7 +113,7 @@ func TestBackend_basic_singleCert(t *testing.T) { logicaltest.Test(t, logicaltest.TestCase{ Backend: testFactory(t), Steps: []logicaltest.TestStep{ - testAccStepCert(t, "web", ca, "foo"), + testAccStepCert(t, "web", ca, "foo", false), testAccStepLogin(t, connState), }, }) @@ -196,16 +223,23 @@ func testAccStepLoginInvalid(t *testing.T, connState tls.ConnectionState) logica } func testAccStepCert( - t *testing.T, name string, cert []byte, policies string) logicaltest.TestStep { + t *testing.T, name string, cert []byte, policies string, expectError bool) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.UpdateOperation, Path: "certs/" + name, + ErrorOk: true, Data: map[string]interface{}{ "certificate": string(cert), "policies": policies, "display_name": name, "lease": 1000, }, + Check: func(resp *logical.Response) error { + if resp == nil && expectError { + return fmt.Errorf("expected error but received nil") + } + return nil + }, } } diff --git a/builtin/credential/cert/path_certs.go b/builtin/credential/cert/path_certs.go index b11c4237b7..8d941275fc 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -138,7 +138,7 @@ func (b *backend) pathCertWrite( if parsed[0].ExtKeyUsage == nil { return logical.ErrorResponse("non-CA certificates should have TLS web client authentication set as an extended key usage"), nil } - var bool clientAuth + var clientAuth bool for _, usage := range parsed[0].ExtKeyUsage { if usage == x509.ExtKeyUsageClientAuth { clientAuth = true diff --git a/builtin/credential/cert/test-fixtures/noclientauthcert.pem b/builtin/credential/cert/test-fixtures/noclientauthcert.pem new file mode 100644 index 0000000000..1e79f00ce0 --- /dev/null +++ b/builtin/credential/cert/test-fixtures/noclientauthcert.pem @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDBDCCAeygAwIBAgIBAjANBgkqhkiG9w0BAQUFADBxMQowCAYDVQQDFAEqMQsw +CQYDVQQIEwJHQTELMAkGA1UEBhMCVVMxJTAjBgkqhkiG9w0BCQEWFnZpc2hhbG5h +eWFrdkBnbWFpbC5jb20xEjAQBgNVBAoTCUhhc2hpQ29ycDEOMAwGA1UECxMFVmF1 +bHQwHhcNMTYwMjEyMTcyNjQwWhcNMjEwMjEwMTcyNjQwWjBxMQowCAYDVQQDFAEq +MQswCQYDVQQIEwJHQTELMAkGA1UEBhMCVVMxJTAjBgkqhkiG9w0BCQEWFnZpc2hh +bG5heWFrdkBnbWFpbC5jb20xEjAQBgNVBAoTCUhhc2hpQ29ycDEOMAwGA1UECxMF +VmF1bHQwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMoJH9PTsuS/sHy604Tb +BISlTT2WSgEGkkyCkrr0XnP26fzjFejRU8ReBf4O2CEBchaAo4vMOdtcMktXNseG +j2mhezEjkT6cnjjwdKiwlOrDAgYN4JvXJcZwfVmmO9uJM6kFYVfIQSI3f3fnmuYa +GYykuOfc1IXeFRiyArQqpRstAgMBAAGjKzApMAkGA1UdEwQCMAAwCwYDVR0PBAQD +AgXgMA8GA1UdEQQIMAaHBH8AAAEwDQYJKoZIhvcNAQEFBQADggEBAHm1q/LWikiV +O8ADqqerawOZUjbLrGB0+IXDvUBYWz5I2GoY6629Nw4ySF9ePAotJ7A+4My2Sdo+ +oHyZE0Ww3hWeuXkLqTbVR9l0+7zS5oHhAFAzVdcgW8aOI0wi2GCuQ0u160V0LARt +9kuYy8V4v8wznpzljdXU/Q2DmlnhW+M060KBwsf699UkheWJ698FyT9CtSitBqox +zSLtMSuzvRc20Hksjan3leaUYgseMw/rZhQpAbUdHxEocoY02kiWViqdroCzfqlp +RoLrr4/mFKDqEgv63ogFkG8cFb2v86Y6UcpSBjekWrzM4ZFv71ISZwqmi1wXi/hQ +M3obAeGTHc4= +-----END CERTIFICATE----- From d8213e8094719eb9a7981e47d4efeaa886cdbfb6 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Mon, 29 Feb 2016 18:29:41 -0500 Subject: [PATCH 09/42] corrections, policy matching changes and test cert changes --- builtin/credential/cert/backend_test.go | 2 +- builtin/credential/cert/path_certs.go | 7 ++-- builtin/credential/cert/path_login.go | 32 +++++++------------ .../cert/test-fixtures/noclientauthcert.pem | 26 +++++++-------- 4 files changed, 27 insertions(+), 40 deletions(-) diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index 2d03041209..9876261529 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -227,7 +227,7 @@ func testAccStepCert( return logicaltest.TestStep{ Operation: logical.UpdateOperation, Path: "certs/" + name, - ErrorOk: true, + ErrorOk: expectError, Data: map[string]interface{}{ "certificate": string(cert), "policies": policies, diff --git a/builtin/credential/cert/path_certs.go b/builtin/credential/cert/path_certs.go index 8d941275fc..394ea50fdb 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -134,10 +134,7 @@ func (b *backend) pathCertWrite( } // If the certificate is not a CA cert, then ensure that x509.ExtKeyUsageClientAuth is set - if !parsed[0].IsCA { - if parsed[0].ExtKeyUsage == nil { - return logical.ErrorResponse("non-CA certificates should have TLS web client authentication set as an extended key usage"), nil - } + if !parsed[0].IsCA && parsed[0].ExtKeyUsage != nil { var clientAuth bool for _, usage := range parsed[0].ExtKeyUsage { if usage == x509.ExtKeyUsageClientAuth { @@ -145,7 +142,7 @@ func (b *backend) pathCertWrite( } } if !clientAuth { - return logical.ErrorResponse("non-CA certificates should have TLS web client authentication set as an extended key usage"), nil + return logical.ErrorResponse("non-CA certificates should have TLS client authentication set as an extended key usage"), nil } } diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 541c4d1fcf..44cd5d474c 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -1,6 +1,7 @@ package cert import ( + "bytes" "crypto/tls" "crypto/x509" "encoding/base64" @@ -134,19 +135,15 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical connState := req.Connection.ConnState // Load the trusted certificates - roots, trusted, nonCAPool, trustedNonCAs := b.loadTrustedCerts(req.Storage) + roots, trusted, trustedNonCAs := b.loadTrustedCerts(req.Storage) - // Validate the connection state is trusted // If trustedNonCAs is not empty it means that client had registered a non-CA cert // with the backend. if len(trustedNonCAs) != 0 { - _, err := validateConnState(nonCAPool, connState) - if err != nil { - return nil, nil, err - } - // Match the trusted non-CA chain with the policy - return b.matchNonCAPolicy(connState.PeerCertificates, trustedNonCAs), nil, nil + // Match the trusted chain with the policy + return b.matchNonCAPolicy(connState.PeerCertificates[0], trustedNonCAs), nil, nil } else { + // Validate the connection state is trusted trustedChains, err := validateConnState(roots, connState) if err != nil { return nil, nil, err @@ -170,15 +167,12 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical } // matchNonCAPolicy is used to match the client cert with the registered non-CA -// policy to establish client identity. -func (b *backend) matchNonCAPolicy(peerCertificates []*x509.Certificate, trustedNonCAs []*ParsedCert) *ParsedCert { +// policies to establish client identity. +func (b *backend) matchNonCAPolicy(clientCert *x509.Certificate, trustedNonCAs []*ParsedCert) *ParsedCert { for _, trustedNonCA := range trustedNonCAs { - for _, tCert := range trustedNonCA.Certificates { - for _, cCert := range peerCertificates { - if tCert.Equal(cCert) { - return trustedNonCA - } - } + tCert := trustedNonCA.Certificates[0] + if tCert.SerialNumber.String() == clientCert.SerialNumber.String() && bytes.Equal(tCert.RawIssuer, clientCert.RawIssuer) { + return trustedNonCA } } return nil @@ -203,9 +197,8 @@ func (b *backend) matchPolicy(chains [][]*x509.Certificate, trusted []*ParsedCer } // loadTrustedCerts is used to load all the trusted certificates from the backend -func (b *backend) loadTrustedCerts(store logical.Storage) (pool *x509.CertPool, trusted []*ParsedCert, nonCAPool *x509.CertPool, trustedNonCAs []*ParsedCert) { +func (b *backend) loadTrustedCerts(store logical.Storage) (pool *x509.CertPool, trusted []*ParsedCert, trustedNonCAs []*ParsedCert) { pool = x509.NewCertPool() - nonCAPool = x509.NewCertPool() names, err := store.List("cert/") if err != nil { b.Logger().Printf("[ERR] cert: failed to list trusted certs: %v", err) @@ -223,9 +216,6 @@ func (b *backend) loadTrustedCerts(store logical.Storage) (pool *x509.CertPool, continue } if !parsed[0].IsCA { - for _, p := range parsed { - nonCAPool.AddCert(p) - } trustedNonCAs = append(trustedNonCAs, &ParsedCert{ Entry: entry, Certificates: parsed, diff --git a/builtin/credential/cert/test-fixtures/noclientauthcert.pem b/builtin/credential/cert/test-fixtures/noclientauthcert.pem index 1e79f00ce0..3948f22032 100644 --- a/builtin/credential/cert/test-fixtures/noclientauthcert.pem +++ b/builtin/credential/cert/test-fixtures/noclientauthcert.pem @@ -1,19 +1,19 @@ -----BEGIN CERTIFICATE----- -MIIDBDCCAeygAwIBAgIBAjANBgkqhkiG9w0BAQUFADBxMQowCAYDVQQDFAEqMQsw +MIIDGTCCAgGgAwIBAgIBBDANBgkqhkiG9w0BAQUFADBxMQowCAYDVQQDFAEqMQsw CQYDVQQIEwJHQTELMAkGA1UEBhMCVVMxJTAjBgkqhkiG9w0BCQEWFnZpc2hhbG5h eWFrdkBnbWFpbC5jb20xEjAQBgNVBAoTCUhhc2hpQ29ycDEOMAwGA1UECxMFVmF1 -bHQwHhcNMTYwMjEyMTcyNjQwWhcNMjEwMjEwMTcyNjQwWjBxMQowCAYDVQQDFAEq +bHQwHhcNMTYwMjI5MjE0NjE2WhcNMjEwMjI3MjE0NjE2WjBxMQowCAYDVQQDFAEq MQswCQYDVQQIEwJHQTELMAkGA1UEBhMCVVMxJTAjBgkqhkiG9w0BCQEWFnZpc2hh bG5heWFrdkBnbWFpbC5jb20xEjAQBgNVBAoTCUhhc2hpQ29ycDEOMAwGA1UECxMF -VmF1bHQwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMoJH9PTsuS/sHy604Tb -BISlTT2WSgEGkkyCkrr0XnP26fzjFejRU8ReBf4O2CEBchaAo4vMOdtcMktXNseG -j2mhezEjkT6cnjjwdKiwlOrDAgYN4JvXJcZwfVmmO9uJM6kFYVfIQSI3f3fnmuYa -GYykuOfc1IXeFRiyArQqpRstAgMBAAGjKzApMAkGA1UdEwQCMAAwCwYDVR0PBAQD -AgXgMA8GA1UdEQQIMAaHBH8AAAEwDQYJKoZIhvcNAQEFBQADggEBAHm1q/LWikiV -O8ADqqerawOZUjbLrGB0+IXDvUBYWz5I2GoY6629Nw4ySF9ePAotJ7A+4My2Sdo+ -oHyZE0Ww3hWeuXkLqTbVR9l0+7zS5oHhAFAzVdcgW8aOI0wi2GCuQ0u160V0LARt -9kuYy8V4v8wznpzljdXU/Q2DmlnhW+M060KBwsf699UkheWJ698FyT9CtSitBqox -zSLtMSuzvRc20Hksjan3leaUYgseMw/rZhQpAbUdHxEocoY02kiWViqdroCzfqlp -RoLrr4/mFKDqEgv63ogFkG8cFb2v86Y6UcpSBjekWrzM4ZFv71ISZwqmi1wXi/hQ -M3obAeGTHc4= +VmF1bHQwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMfRkLfIGHt1r2jjnV0N +LqRCu3oB+J1dqpM03vQt3qzIiqtuQuIA2ba7TJm2HwU3W3+rtfFcS+hkBR/LZM+u +cBPB+9b9+7i08vHjgy2P3QH/Ebxa8j1v7JtRMT2qyxWK8NlT/+wZSH82Cr812aS/ +zNT56FbBo2UAtzpqeC4eiv6NAgMBAAGjQDA+MAkGA1UdEwQCMAAwCwYDVR0PBAQD +AgXgMBMGA1UdJQQMMAoGCCsGAQUFBwMBMA8GA1UdEQQIMAaHBH8AAAEwDQYJKoZI +hvcNAQEFBQADggEBAG2mUwsZ6+R8qqyNjzMk7mgpsRZv9TEl6c1IiQdyjaCOPaYH +vtZpLX20um36cxrLuOUtZLllG/VJEhRZW5mXWxuOk4QunWMBXQioCDJG1ktcZAcQ +QqYv9Dzy2G9lZHjLztEac37T75RXW7OEeQREgwP11c8sQYiS9jf+7ITYL7nXjoKq +gEuH0h86BOH2O/BxgMelt9O0YCkvkLLHnE27xuNelRRZcBLSuE1GxdUi32MDJ+ff +25GUNM0zzOEaJAFE/USUBEdQqN1gvJidNXkAiMtIK7T8omQZONRaD2ZnSW8y2krh +eUg+rKis9RinqFlahLPfI5BlyQsNMEnsD07Q85E= -----END CERTIFICATE----- From 86df49b9927a744d0000e22e455072fc8a9b741a Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 1 Mar 2016 10:24:22 -0500 Subject: [PATCH 10/42] Added ExtKeyUsageAny, changed big.Int comparison and fixed code flow --- builtin/credential/cert/path_certs.go | 3 +- builtin/credential/cert/path_login.go | 44 +++++++++++++-------------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/builtin/credential/cert/path_certs.go b/builtin/credential/cert/path_certs.go index 394ea50fdb..c94dfdff60 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -137,8 +137,9 @@ func (b *backend) pathCertWrite( if !parsed[0].IsCA && parsed[0].ExtKeyUsage != nil { var clientAuth bool for _, usage := range parsed[0].ExtKeyUsage { - if usage == x509.ExtKeyUsageClientAuth { + if usage == x509.ExtKeyUsageClientAuth || usage == x509.ExtKeyUsageAny { clientAuth = true + break } } if !clientAuth { diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 44cd5d474c..2ecdbdd467 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -142,28 +142,28 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical if len(trustedNonCAs) != 0 { // Match the trusted chain with the policy return b.matchNonCAPolicy(connState.PeerCertificates[0], trustedNonCAs), nil, nil - } else { - // Validate the connection state is trusted - trustedChains, err := validateConnState(roots, connState) - if err != nil { - return nil, nil, err - } - - // If no trusted chain was found, client is not authenticated - if len(trustedChains) == 0 { - return nil, logical.ErrorResponse("invalid certificate or no client certificate supplied"), nil - } - - validChain := b.checkForValidChain(req.Storage, trustedChains) - if !validChain { - return nil, logical.ErrorResponse( - "no chain containing non-revoked certificates could be found for this login certificate", - ), nil - } - - // Match the trusted chain with the policy - return b.matchPolicy(trustedChains, trusted), nil, nil } + + // Validate the connection state is trusted + trustedChains, err := validateConnState(roots, connState) + if err != nil { + return nil, nil, err + } + + // If no trusted chain was found, client is not authenticated + if len(trustedChains) == 0 { + return nil, logical.ErrorResponse("invalid certificate or no client certificate supplied"), nil + } + + validChain := b.checkForValidChain(req.Storage, trustedChains) + if !validChain { + return nil, logical.ErrorResponse( + "no chain containing non-revoked certificates could be found for this login certificate", + ), nil + } + + // Match the trusted chain with the policy + return b.matchPolicy(trustedChains, trusted), nil, nil } // matchNonCAPolicy is used to match the client cert with the registered non-CA @@ -171,7 +171,7 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical func (b *backend) matchNonCAPolicy(clientCert *x509.Certificate, trustedNonCAs []*ParsedCert) *ParsedCert { for _, trustedNonCA := range trustedNonCAs { tCert := trustedNonCA.Certificates[0] - if tCert.SerialNumber.String() == clientCert.SerialNumber.String() && bytes.Equal(tCert.RawIssuer, clientCert.RawIssuer) { + if tCert.SerialNumber.Cmp(clientCert.SerialNumber) == 0 && bytes.Equal(tCert.AuthorityKeyId, clientCert.AuthorityKeyId) { return trustedNonCA } } From 4d5634528c3b099c4180c252e2ff7c1ef1025991 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 1 Mar 2016 16:43:51 -0500 Subject: [PATCH 11/42] continue if non-CA policy is not found --- builtin/credential/cert/path_login.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 2ecdbdd467..6ad76c6f86 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -140,8 +140,10 @@ func (b *backend) verifyCredentials(req *logical.Request) (*ParsedCert, *logical // If trustedNonCAs is not empty it means that client had registered a non-CA cert // with the backend. if len(trustedNonCAs) != 0 { - // Match the trusted chain with the policy - return b.matchNonCAPolicy(connState.PeerCertificates[0], trustedNonCAs), nil, nil + policy := b.matchNonCAPolicy(connState.PeerCertificates[0], trustedNonCAs) + if policy != nil { + return policy, nil, nil + } } // Validate the connection state is trusted From c3a70bc1bfcd8528c4e96802afce44587d8cb688 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 1 Mar 2016 17:02:48 -0500 Subject: [PATCH 12/42] Allow `token-renew` to not be given a token; it will then use the renew-self endpoint. Otherwise it will use the renew endpoint, even if the token matches the client token. Adds an -increment flag to allow increments even with no token passed in. Fixes #1150 --- command/token_renew.go | 61 +++++++++++------ command/token_renew_test.go | 129 +++++++++++++++++++++++++++++++++++- 2 files changed, 169 insertions(+), 21 deletions(-) diff --git a/command/token_renew.go b/command/token_renew.go index 776f513569..a3404e5821 100644 --- a/command/token_renew.go +++ b/command/token_renew.go @@ -2,8 +2,8 @@ package command import ( "fmt" - "strconv" "strings" + "time" "github.com/hashicorp/vault/api" ) @@ -14,32 +14,45 @@ type TokenRenewCommand struct { } func (c *TokenRenewCommand) Run(args []string) int { - var format string + var format, increment string flags := c.Meta.FlagSet("token-renew", FlagSetDefault) flags.StringVar(&format, "format", "table", "") + flags.StringVar(&increment, "increment", "", "") flags.Usage = func() { c.Ui.Error(c.Help()) } if err := flags.Parse(args); err != nil { return 1 } args = flags.Args() - if len(args) < 1 { + if len(args) > 2 { flags.Usage() c.Ui.Error(fmt.Sprintf( - "\ntoken-renew expects at least one argument")) + "\ntoken-renew expects at most two arguments")) return 1 } - var increment int - token := args[0] - if len(args) > 1 { - value, err := strconv.ParseInt(args[1], 10, 0) + var token string + if len(args) > 0 { + token = args[0] + } + + var inc int + if len(args) == 2 { + dur, err := time.ParseDuration(args[1]) if err != nil { c.Ui.Error(fmt.Sprintf("Invalid increment: %s", err)) return 1 } - increment = int(value) + inc = int(dur / time.Second) + } else if increment != "" { + dur, err := time.ParseDuration(increment) + if err != nil { + c.Ui.Error(fmt.Sprintf("Invalid increment: %s", err)) + return 1 + } + + inc = int(dur / time.Second) } client, err := c.Client() @@ -52,10 +65,10 @@ func (c *TokenRenewCommand) Run(args []string) int { // If the given token is the same as the client's, use renew-self instead // as this is far more likely to be allowed via policy var secret *api.Secret - if client.Token() == token { - secret, err = client.Auth().Token().RenewSelf(increment) + if token == "" { + secret, err = client.Auth().Token().RenewSelf(inc) } else { - secret, err = client.Auth().Token().Renew(token, increment) + secret, err = client.Auth().Token().Renew(token, inc) } if err != nil { c.Ui.Error(fmt.Sprintf( @@ -72,17 +85,20 @@ func (c *TokenRenewCommand) Synopsis() string { func (c *TokenRenewCommand) Help() string { helpText := ` -Usage: vault token-renew [options] token [increment] +Usage: vault token-renew [options] [token] [increment] - Renew an auth token, extending the amount of time it can be used. - Token is renewable only if there is a lease associated with it. + Renew an auth token, extending the amount of time it can be used. If a token + is given to the command, '/auth/token/renew' will be called with the given + token; otherwise, '/auth/token/renew-self' will be called with the client + token. - This command is similar to "renew", but "renew" is only for lease IDs. - This command is only for tokens. + This command is similar to "renew", but "renew" is only for leases; this + command is only for tokens. - An optional increment can be given to request a certain number of - seconds to increment the lease. This request is advisory; Vault may not - adhere to it at all. + An optional increment can be given to request a certain number of seconds to + increment the lease. This request is advisory; Vault may not adhere to it at + all. If a token is being passed in on the command line, the increment can as + well; otherwise it must be passed in via the '-increment' flag. General Options: @@ -90,6 +106,11 @@ General Options: Token Renew Options: + -increment=3600 The desired increment. If not supplied, Vault will + use the default TTL. If supplied, it may still be + ignored. This can be submitted as an integer number + of seconds or a string duration (e.g. "72h"). + -format=table The format for output. By default it is a whitespace- delimited table. This can also be json or yaml. diff --git a/command/token_renew_test.go b/command/token_renew_test.go index 34ebec7e13..0dbd53d621 100644 --- a/command/token_renew_test.go +++ b/command/token_renew_test.go @@ -41,9 +41,136 @@ func TestTokenRenew(t *testing.T) { t.Fatalf("err: %s", err) } - // Verify it worked + // Renew, passing in the token args = append(args, resp.Auth.ClientToken) if code := c.Run(args); code != 0 { t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) } } + +func TestTokenRenewWithIncrement(t *testing.T) { + core, _, token := vault.TestCoreUnsealed(t) + ln, addr := http.TestServer(t, core) + defer ln.Close() + + ui := new(cli.MockUi) + c := &TokenRenewCommand{ + Meta: Meta{ + ClientToken: token, + Ui: ui, + }, + } + + args := []string{ + "-address", addr, + } + + // Run it once for client + c.Run(args) + + // Create a token + client, err := c.Client() + if err != nil { + t.Fatalf("err: %s", err) + } + resp, err := client.Auth().Token().Create(&api.TokenCreateRequest{ + Lease: "1h", + }) + if err != nil { + t.Fatalf("err: %s", err) + } + + // Renew, passing in the token + args = append(args, resp.Auth.ClientToken) + args = append(args, "72h") + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } +} + +func TestTokenRenewSelf(t *testing.T) { + core, _, token := vault.TestCoreUnsealed(t) + ln, addr := http.TestServer(t, core) + defer ln.Close() + + ui := new(cli.MockUi) + c := &TokenRenewCommand{ + Meta: Meta{ + ClientToken: token, + Ui: ui, + }, + } + + args := []string{ + "-address", addr, + } + + // Run it once for client + c.Run(args) + + // Create a token + client, err := c.Client() + if err != nil { + t.Fatalf("err: %s", err) + } + resp, err := client.Auth().Token().Create(&api.TokenCreateRequest{ + Lease: "1h", + }) + if err != nil { + t.Fatalf("err: %s", err) + } + if resp.Auth.ClientToken == "" { + t.Fatal("returned client token is empty") + } + + c.Meta.ClientToken = resp.Auth.ClientToken + + // Renew using the self endpoint + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } +} + +func TestTokenRenewSelfWithIncrement(t *testing.T) { + core, _, token := vault.TestCoreUnsealed(t) + ln, addr := http.TestServer(t, core) + defer ln.Close() + + ui := new(cli.MockUi) + c := &TokenRenewCommand{ + Meta: Meta{ + ClientToken: token, + Ui: ui, + }, + } + + args := []string{ + "-address", addr, + } + + // Run it once for client + c.Run(args) + + // Create a token + client, err := c.Client() + if err != nil { + t.Fatalf("err: %s", err) + } + resp, err := client.Auth().Token().Create(&api.TokenCreateRequest{ + Lease: "1h", + }) + if err != nil { + t.Fatalf("err: %s", err) + } + if resp.Auth.ClientToken == "" { + t.Fatal("returned client token is empty") + } + + c.Meta.ClientToken = resp.Auth.ClientToken + + args = append(args, "-increment=72h") + // Renew using the self endpoint + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } +} From e7765993155dbe9a4bd9a3b618b34750a9fad009 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 1 Mar 2016 17:12:14 -0500 Subject: [PATCH 13/42] changelog++ --- CHANGELOG.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f52f4ec30..0fef0b8f79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,18 @@ ## 0.5.2 (Unreleased) +IMPROVEMENTS: + + * credential/cert: Non-CA certificates can be used for authentication. They + must be matched exactly (issuer and serial number) for authentication, and + the certificate must carry the client authentication or 'any' extended usage + attributes. [GH-1148] + BUG FIXES: -* logical/cassandra: Apply hyphen/underscore replacement to the entire - generated username, not just the UUID, in order to handle token display name - hyphens [GH-1140] -* physical/etcd: Output actual error when cluster sync fails [GH-1141] + * logical/cassandra: Apply hyphen/underscore replacement to the entire + generated username, not just the UUID, in order to handle token display name + hyphens [GH-1140] + * physical/etcd: Output actual error when cluster sync fails [GH-1141] ## 0.5.1 (February 25th, 2016) From 8f728f8ed371fe57b566e794c03cdded19b064a5 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 1 Mar 2016 17:18:20 -0500 Subject: [PATCH 14/42] changelog++ --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fef0b8f79..543c6f67c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,11 @@ ## 0.5.2 (Unreleased) IMPROVEMENTS: - * credential/cert: Non-CA certificates can be used for authentication. They must be matched exactly (issuer and serial number) for authentication, and the certificate must carry the client authentication or 'any' extended usage - attributes. [GH-1148] + attributes. [GH-1153] + * secret/ssh: Added documentation for `ssh/config/zeroaddress` endpoint. [GH-1154] BUG FIXES: From 143d876c99cf9f8e4ce30eabf349849a70f6c659 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 1 Mar 2016 20:25:40 -0500 Subject: [PATCH 15/42] Address review feedback --- command/token_renew.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/command/token_renew.go b/command/token_renew.go index a3404e5821..3017f71624 100644 --- a/command/token_renew.go +++ b/command/token_renew.go @@ -37,15 +37,11 @@ func (c *TokenRenewCommand) Run(args []string) int { } var inc int + // If both are specified prefer the argument if len(args) == 2 { - dur, err := time.ParseDuration(args[1]) - if err != nil { - c.Ui.Error(fmt.Sprintf("Invalid increment: %s", err)) - return 1 - } - - inc = int(dur / time.Second) - } else if increment != "" { + increment = args[1] + } + if increment != "" { dur, err := time.ParseDuration(increment) if err != nil { c.Ui.Error(fmt.Sprintf("Invalid increment: %s", err)) From e7f41004376d02c6dcf24484f6edba30f81952ac Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 1 Mar 2016 20:27:08 -0500 Subject: [PATCH 16/42] changelog++ --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 543c6f67c4..7d73bde769 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ IMPROVEMENTS: the certificate must carry the client authentication or 'any' extended usage attributes. [GH-1153] * secret/ssh: Added documentation for `ssh/config/zeroaddress` endpoint. [GH-1154] + * command/token-renew: Allow no token to be passed in; use `renew-self` in + this case. Change the behavior for any token being passed in to use `renew`. + [GH-1150] BUG FIXES: From c19641887d52c2a42f6fece4c07ef637860a7425 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 2 Mar 2016 11:53:23 -0500 Subject: [PATCH 17/42] Allow specifying an initial root token ID in dev mode. Ping #1160 --- command/server.go | 58 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 7 deletions(-) diff --git a/command/server.go b/command/server.go index f8e985ad0a..aa33a686d7 100644 --- a/command/server.go +++ b/command/server.go @@ -41,9 +41,10 @@ type ServerCommand struct { func (c *ServerCommand) Run(args []string) int { var dev, verifyOnly bool var configPath []string - var logLevel string + var logLevel, rootTokenID string flags := c.Meta.FlagSet("server", FlagSetDefault) flags.BoolVar(&dev, "dev", false, "") + flags.StringVar(&rootTokenID, "root-token-id", "", "") flags.StringVar(&logLevel, "log-level", "info", "") flags.BoolVar(&verifyOnly, "verify-only", false, "") flags.Usage = func() { c.Ui.Error(c.Help()) } @@ -53,10 +54,17 @@ func (c *ServerCommand) Run(args []string) int { } // Validation - if !dev && len(configPath) == 0 { - c.Ui.Error("At least one config path must be specified with -config") - flags.Usage() - return 1 + if !dev { + switch { + case len(configPath) == 0: + c.Ui.Error("At least one config path must be specified with -config") + flags.Usage() + return 1 + case rootTokenID != "": + c.Ui.Error("Root token ID can only be specified with -dev") + flags.Usage() + return 1 + } } // Load the configuration @@ -193,7 +201,7 @@ func (c *ServerCommand) Run(args []string) int { // If we're in dev mode, then initialize the core if dev { - init, err := c.enableDev(core) + init, err := c.enableDev(core, rootTokenID) if err != nil { c.Ui.Error(fmt.Sprintf( "Error initializing dev mode: %s", err)) @@ -319,7 +327,7 @@ func (c *ServerCommand) Run(args []string) int { return 0 } -func (c *ServerCommand) enableDev(core *vault.Core) (*vault.InitResult, error) { +func (c *ServerCommand) enableDev(core *vault.Core, rootTokenID string) (*vault.InitResult, error) { // Initialize it with a basic single key init, err := core.Initialize(&vault.SealConfig{ SecretShares: 1, @@ -342,6 +350,39 @@ func (c *ServerCommand) enableDev(core *vault.Core) (*vault.InitResult, error) { return nil, fmt.Errorf("failed to unseal Vault for dev mode") } + if rootTokenID != "" { + req := &logical.Request{ + Operation: logical.UpdateOperation, + ClientToken: init.RootToken, + Path: "auth/token/create", + Data: map[string]interface{}{ + "id": rootTokenID, + "policies": []string{"root"}, + "no_parent": true, + "no_default_policy": true, + }, + } + resp, err := core.HandleRequest(req) + if err != nil { + return nil, fmt.Errorf("failed to create root token with ID %s: %s", rootTokenID, err) + } + if resp == nil { + return nil, fmt.Errorf("nil response when creating root token with ID %s", rootTokenID) + } + if resp.Auth == nil { + return nil, fmt.Errorf("nil auth when creating root token with ID %s", rootTokenID) + } + + init.RootToken = resp.Auth.ClientToken + + req.Path = "auth/token/revoke-self" + req.Data = nil + resp, err = core.HandleRequest(req) + if err != nil { + return nil, fmt.Errorf("failed to revoke initial root token: %s", err) + } + } + // Set the token tokenHelper, err := c.TokenHelper() if err != nil { @@ -507,6 +548,9 @@ General Options: to stderr. Supported values: "trace", "debug", "info", "warn", "err" + -root-token-id="" If set, the root token returned in Dev mode will have the + given ID. This *only* has an effect when running in Dev + mode. ` return strings.TrimSpace(helpText) } From 278b4f9f8a75648ba5e8631b483f9c34a63bda7a Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 2 Mar 2016 12:05:16 -0500 Subject: [PATCH 18/42] changelog++ --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d73bde769..429512e16d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ IMPROVEMENTS: the certificate must carry the client authentication or 'any' extended usage attributes. [GH-1153] * secret/ssh: Added documentation for `ssh/config/zeroaddress` endpoint. [GH-1154] + * command/server: The initial root token ID when running in `-dev` mode can + now be specified via `-root-token-id` [GH-1162] * command/token-renew: Allow no token to be passed in; use `renew-self` in this case. Change the behavior for any token being passed in to use `renew`. [GH-1150] From 46a71bd6481d35b3f1d05ee1e8edbeb6f32aae7c Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 2 Mar 2016 12:06:16 -0500 Subject: [PATCH 19/42] Add a sleep in the RedirectStandby test to try to fix raciness --- http/logical_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/http/logical_test.go b/http/logical_test.go index fc2a0830e3..de124f7a78 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -85,6 +85,10 @@ func TestLogical_StandbyRedirect(t *testing.T) { t.Fatalf("unseal err: %s", err) } + // Attempt to fix raciness in this test by giving the first core a chance + // to grab the lock + time.Sleep(time.Second) + // Create a second HA Vault conf2 := &vault.CoreConfig{ Physical: inmha, From f85c3f48afcd7b854c301fe4cbcf27ddff6fa00b Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 2 Mar 2016 14:16:54 -0500 Subject: [PATCH 20/42] Remove sys_policy from special handling as it's implemented in logical_system too. Clean up the mux handlers. --- http/handler.go | 16 +--- http/sys_policy.go | 150 ----------------------------------- http/sys_policy_test.go | 11 ++- vault/logical_system.go | 8 +- vault/logical_system_test.go | 9 ++- 5 files changed, 21 insertions(+), 173 deletions(-) delete mode 100644 http/sys_policy.go diff --git a/http/handler.go b/http/handler.go index bd2f2dafc7..360b3a3451 100644 --- a/http/handler.go +++ b/http/handler.go @@ -24,28 +24,14 @@ func Handler(core *vault.Core) http.Handler { mux.Handle("/v1/sys/seal-status", handleSysSealStatus(core)) mux.Handle("/v1/sys/seal", handleSysSeal(core)) mux.Handle("/v1/sys/unseal", handleSysUnseal(core)) - mux.Handle("/v1/sys/mounts", proxySysRequest(core)) - mux.Handle("/v1/sys/mounts/", proxySysRequest(core)) - mux.Handle("/v1/sys/remount", proxySysRequest(core)) - mux.Handle("/v1/sys/policy", handleSysListPolicies(core)) - mux.Handle("/v1/sys/policy/", handleSysPolicy(core)) mux.Handle("/v1/sys/renew/", handleLogical(core, false)) - mux.Handle("/v1/sys/revoke/", proxySysRequest(core)) - mux.Handle("/v1/sys/revoke-prefix/", proxySysRequest(core)) - mux.Handle("/v1/sys/auth", proxySysRequest(core)) - mux.Handle("/v1/sys/auth/", proxySysRequest(core)) - mux.Handle("/v1/sys/audit-hash/", proxySysRequest(core)) - mux.Handle("/v1/sys/audit", proxySysRequest(core)) - mux.Handle("/v1/sys/audit/", proxySysRequest(core)) mux.Handle("/v1/sys/leader", handleSysLeader(core)) mux.Handle("/v1/sys/health", handleSysHealth(core)) - mux.Handle("/v1/sys/rotate", proxySysRequest(core)) - mux.Handle("/v1/sys/key-status", proxySysRequest(core)) mux.Handle("/v1/sys/generate-root/attempt", handleSysGenerateRootAttempt(core)) mux.Handle("/v1/sys/generate-root/update", handleSysGenerateRootUpdate(core)) mux.Handle("/v1/sys/rekey/init", handleSysRekeyInit(core)) - mux.Handle("/v1/sys/rekey/backup", proxySysRequest(core)) mux.Handle("/v1/sys/rekey/update", handleSysRekeyUpdate(core)) + mux.Handle("/v1/sys/", proxySysRequest(core)) mux.Handle("/v1/", handleLogical(core, false)) // Wrap the handler in another handler to trigger all help paths. diff --git a/http/sys_policy.go b/http/sys_policy.go deleted file mode 100644 index b6234c6836..0000000000 --- a/http/sys_policy.go +++ /dev/null @@ -1,150 +0,0 @@ -package http - -import ( - "net/http" - "strings" - - "github.com/hashicorp/vault/logical" - "github.com/hashicorp/vault/vault" -) - -func handleSysListPolicies(core *vault.Core) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != "GET" { - respondError(w, http.StatusMethodNotAllowed, nil) - return - } - - resp, ok := request(core, w, r, requestAuth(r, &logical.Request{ - Operation: logical.ReadOperation, - Path: "sys/policy", - Connection: getConnection(r), - })) - if !ok { - return - } - - var policies []string - policiesRaw, ok := resp.Data["keys"] - if ok { - policies = policiesRaw.([]string) - } - - respondOk(w, &listPolicyResponse{Policies: policies}) - }) -} - -func handleSysPolicy(core *vault.Core) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.Method { - case "GET": - handleSysReadPolicy(core, w, r) - case "PUT": - fallthrough - case "POST": - handleSysWritePolicy(core, w, r) - case "DELETE": - handleSysDeletePolicy(core, w, r) - default: - respondError(w, http.StatusMethodNotAllowed, nil) - return - } - }) -} - -func handleSysDeletePolicy(core *vault.Core, w http.ResponseWriter, r *http.Request) { - // Determine the path... - prefix := "/v1/sys/policy/" - if !strings.HasPrefix(r.URL.Path, prefix) { - respondError(w, http.StatusNotFound, nil) - return - } - path := r.URL.Path[len(prefix):] - if path == "" { - respondError(w, http.StatusNotFound, nil) - return - } - - _, ok := request(core, w, r, requestAuth(r, &logical.Request{ - Operation: logical.DeleteOperation, - Path: "sys/policy/" + path, - Connection: getConnection(r), - })) - if !ok { - return - } - - respondOk(w, nil) -} - -func handleSysReadPolicy(core *vault.Core, w http.ResponseWriter, r *http.Request) { - // Determine the path... - prefix := "/v1/sys/policy/" - if !strings.HasPrefix(r.URL.Path, prefix) { - respondError(w, http.StatusNotFound, nil) - return - } - path := r.URL.Path[len(prefix):] - if path == "" { - respondError(w, http.StatusNotFound, nil) - return - } - - resp, ok := request(core, w, r, requestAuth(r, &logical.Request{ - Operation: logical.ReadOperation, - Path: "sys/policy/" + path, - Connection: getConnection(r), - })) - if !ok { - return - } - if resp == nil { - respondError(w, http.StatusNotFound, nil) - return - } - - respondOk(w, resp.Data) -} - -func handleSysWritePolicy(core *vault.Core, w http.ResponseWriter, r *http.Request) { - // Determine the path... - prefix := "/v1/sys/policy/" - if !strings.HasPrefix(r.URL.Path, prefix) { - respondError(w, http.StatusNotFound, nil) - return - } - path := r.URL.Path[len(prefix):] - if path == "" { - respondError(w, http.StatusNotFound, nil) - return - } - - // Parse the request if we can - var req writePolicyRequest - if err := parseRequest(r, &req); err != nil { - respondError(w, http.StatusBadRequest, err) - return - } - - _, ok := request(core, w, r, requestAuth(r, &logical.Request{ - Operation: logical.UpdateOperation, - Path: "sys/policy/" + path, - Connection: getConnection(r), - Data: map[string]interface{}{ - "rules": req.Rules, - }, - })) - if !ok { - return - } - - respondOk(w, nil) -} - -type listPolicyResponse struct { - Policies []string `json:"policies"` -} - -type writePolicyRequest struct { - Rules string `json:"rules"` -} diff --git a/http/sys_policy_test.go b/http/sys_policy_test.go index 281572e141..27495ae496 100644 --- a/http/sys_policy_test.go +++ b/http/sys_policy_test.go @@ -18,11 +18,12 @@ func TestSysPolicies(t *testing.T) { var actual map[string]interface{} expected := map[string]interface{}{ "policies": []interface{}{"default", "root"}, + "keys": []interface{}{"default", "root"}, } testResponseStatus(t, resp, 200) testResponseBody(t, resp, &actual) if !reflect.DeepEqual(actual, expected) { - t.Fatalf("bad: %#v", actual) + t.Fatalf("bad: got\n%#v\nexpected\n%#v\n", actual, expected) } } @@ -42,7 +43,7 @@ func TestSysReadPolicy(t *testing.T) { testResponseStatus(t, resp, 200) testResponseBody(t, resp, &actual) if !reflect.DeepEqual(actual, expected) { - t.Fatalf("bad: %#v", actual) + t.Fatalf("bad: got\n%#v\nexpected\n%#v\n", actual, expected) } } @@ -62,11 +63,12 @@ func TestSysWritePolicy(t *testing.T) { var actual map[string]interface{} expected := map[string]interface{}{ "policies": []interface{}{"default", "foo", "root"}, + "keys": []interface{}{"default", "foo", "root"}, } testResponseStatus(t, resp, 200) testResponseBody(t, resp, &actual) if !reflect.DeepEqual(actual, expected) { - t.Fatalf("bad: %#v", actual) + t.Fatalf("bad: got\n%#v\nexpected\n%#v\n", actual, expected) } } @@ -89,10 +91,11 @@ func TestSysDeletePolicy(t *testing.T) { var actual map[string]interface{} expected := map[string]interface{}{ "policies": []interface{}{"default", "root"}, + "keys": []interface{}{"default", "root"}, } testResponseStatus(t, resp, 200) testResponseBody(t, resp, &actual) if !reflect.DeepEqual(actual, expected) { - t.Fatalf("bad: %#v", actual) + t.Fatalf("bad: got\n%#v\nexpected\n%#v\n", actual, expected) } } diff --git a/vault/logical_system.go b/vault/logical_system.go index e4b1ca236c..e2eac88e39 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -246,6 +246,7 @@ func NewSystemBackend(core *Core, config *logical.BackendConfig) logical.Backend Callbacks: map[logical.Operation]framework.OperationFunc{ logical.ReadOperation: b.handlePolicyList, + logical.ListOperation: b.handlePolicyList, }, HelpSynopsis: strings.TrimSpace(sysHelp["policy-list"][0]), @@ -815,7 +816,12 @@ func (b *SystemBackend) handlePolicyList( // Add the special "root" policy policies = append(policies, "root") - return logical.ListResponse(policies), err + resp := logical.ListResponse(policies) + + // Backwords compatibility + resp.Data["policies"] = resp.Data["keys"] + + return resp, err } // handlePolicyRead handles the "policy/" endpoint to read a policy diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index a49b92bc1b..633079dafc 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -431,7 +431,8 @@ func TestSystemBackend_policyList(t *testing.T) { } exp := map[string]interface{}{ - "keys": []string{"default", "root"}, + "keys": []string{"default", "root"}, + "policies": []string{"default", "root"}, } if !reflect.DeepEqual(resp.Data, exp) { t.Fatalf("got: %#v expect: %#v", resp.Data, exp) @@ -483,7 +484,8 @@ func TestSystemBackend_policyCRUD(t *testing.T) { } exp = map[string]interface{}{ - "keys": []string{"default", "foo", "root"}, + "keys": []string{"default", "foo", "root"}, + "policies": []string{"default", "foo", "root"}, } if !reflect.DeepEqual(resp.Data, exp) { t.Fatalf("got: %#v expect: %#v", resp.Data, exp) @@ -517,7 +519,8 @@ func TestSystemBackend_policyCRUD(t *testing.T) { } exp = map[string]interface{}{ - "keys": []string{"default", "root"}, + "keys": []string{"default", "root"}, + "policies": []string{"default", "root"}, } if !reflect.DeepEqual(resp.Data, exp) { t.Fatalf("got: %#v expect: %#v", resp.Data, exp) From f88c6c16db3b3428c6cdc80519fe36a8409c4559 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 2 Mar 2016 14:55:51 -0500 Subject: [PATCH 21/42] Remove proxy function as it's unneeded now --- http/handler.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/http/handler.go b/http/handler.go index 360b3a3451..4087ef6549 100644 --- a/http/handler.go +++ b/http/handler.go @@ -31,7 +31,7 @@ func Handler(core *vault.Core) http.Handler { mux.Handle("/v1/sys/generate-root/update", handleSysGenerateRootUpdate(core)) mux.Handle("/v1/sys/rekey/init", handleSysRekeyInit(core)) mux.Handle("/v1/sys/rekey/update", handleSysRekeyUpdate(core)) - mux.Handle("/v1/sys/", proxySysRequest(core)) + mux.Handle("/v1/sys/", handleLogical(core, true)) mux.Handle("/v1/", handleLogical(core, false)) // Wrap the handler in another handler to trigger all help paths. @@ -200,10 +200,6 @@ func respondOk(w http.ResponseWriter, body interface{}) { } } -func proxySysRequest(core *vault.Core) http.Handler { - return handleLogical(core, true) -} - type ErrorResponse struct { Errors []string `json:"errors"` } From a05ea4720cc1131919c26eb177be63a08cf35c02 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 10:23:43 -0500 Subject: [PATCH 22/42] Add ability to control dev root token id with VAULT_DEV_ROOT_TOKEN_ID env var, and change the CLI flag to match. Ping #1160 --- CHANGELOG.md | 3 ++- command/server.go | 37 +++++++++++++++++++++---------------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 429512e16d..628450754b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,8 @@ IMPROVEMENTS: attributes. [GH-1153] * secret/ssh: Added documentation for `ssh/config/zeroaddress` endpoint. [GH-1154] * command/server: The initial root token ID when running in `-dev` mode can - now be specified via `-root-token-id` [GH-1162] + now be specified via `-dev-root-token-id` or the environment variable + `VAULT_DEV_ROOT_TOKEN_ID` [GH-1162] * command/token-renew: Allow no token to be passed in; use `renew-self` in this case. Change the behavior for any token being passed in to use `renew`. [GH-1150] diff --git a/command/server.go b/command/server.go index aa33a686d7..583d281537 100644 --- a/command/server.go +++ b/command/server.go @@ -41,10 +41,11 @@ type ServerCommand struct { func (c *ServerCommand) Run(args []string) int { var dev, verifyOnly bool var configPath []string - var logLevel, rootTokenID string + var logLevel, devRootTokenID, devAddress string flags := c.Meta.FlagSet("server", FlagSetDefault) flags.BoolVar(&dev, "dev", false, "") - flags.StringVar(&rootTokenID, "root-token-id", "", "") + flags.StringVar(&devRootTokenID, "dev-root-token-id", "", "") + flags.StringVar(&devAddress, "dev-address", "", "") flags.StringVar(&logLevel, "log-level", "info", "") flags.BoolVar(&verifyOnly, "verify-only", false, "") flags.Usage = func() { c.Ui.Error(c.Help()) } @@ -53,6 +54,10 @@ func (c *ServerCommand) Run(args []string) int { return 1 } + if len(os.Getenv("VAULT_DEV_ROOT_TOKEN_ID")) > 0 { + devRootTokenID = os.Getenv("VAULT_DEV_ROOT_TOKEN_ID") + } + // Validation if !dev { switch { @@ -60,7 +65,7 @@ func (c *ServerCommand) Run(args []string) int { c.Ui.Error("At least one config path must be specified with -config") flags.Usage() return 1 - case rootTokenID != "": + case devRootTokenID != "": c.Ui.Error("Root token ID can only be specified with -dev") flags.Usage() return 1 @@ -201,7 +206,7 @@ func (c *ServerCommand) Run(args []string) int { // If we're in dev mode, then initialize the core if dev { - init, err := c.enableDev(core, rootTokenID) + init, err := c.enableDev(core, devRootTokenID) if err != nil { c.Ui.Error(fmt.Sprintf( "Error initializing dev mode: %s", err)) @@ -536,21 +541,21 @@ Usage: vault server [options] General Options: - -config= Path to the configuration file or directory. This can be - specified multiple times. If it is a directory, all - files with a ".hcl" or ".json" suffix will be loaded. + -config= Path to the configuration file or directory. This can be + specified multiple times. If it is a directory, all + files with a ".hcl" or ".json" suffix will be loaded. - -dev Enables Dev mode. In this mode, Vault is completely - in-memory and unsealed. Do not run the Dev server in - production! + -dev Enables Dev mode. In this mode, Vault is completely + in-memory and unsealed. Do not run the Dev server in + production! - -log-level=info Log verbosity. Defaults to "info", will be outputted - to stderr. Supported values: "trace", "debug", "info", - "warn", "err" + -dev-root-token-id="" If set, the root token returned in Dev mode will have the + given ID. This *only* has an effect when running in Dev + mode. - -root-token-id="" If set, the root token returned in Dev mode will have the - given ID. This *only* has an effect when running in Dev - mode. + -log-level=info Log verbosity. Defaults to "info", will be outputted + to stderr. Supported values: "trace", "debug", "info", + "warn", "err" ` return strings.TrimSpace(helpText) } From 00721af2c15c40d7ee5b3de2573f6bb482b33793 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 10:48:52 -0500 Subject: [PATCH 23/42] Add the ability to specify dev mode address via CLI flag and envvar. Fixes #1160 --- command/server.go | 48 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/command/server.go b/command/server.go index 583d281537..860fc5ce40 100644 --- a/command/server.go +++ b/command/server.go @@ -41,11 +41,11 @@ type ServerCommand struct { func (c *ServerCommand) Run(args []string) int { var dev, verifyOnly bool var configPath []string - var logLevel, devRootTokenID, devAddress string + var logLevel, devRootTokenID, devListenAddress string flags := c.Meta.FlagSet("server", FlagSetDefault) flags.BoolVar(&dev, "dev", false, "") flags.StringVar(&devRootTokenID, "dev-root-token-id", "", "") - flags.StringVar(&devAddress, "dev-address", "", "") + flags.StringVar(&devListenAddress, "dev-listen-address", "", "") flags.StringVar(&logLevel, "log-level", "info", "") flags.BoolVar(&verifyOnly, "verify-only", false, "") flags.Usage = func() { c.Ui.Error(c.Help()) } @@ -54,10 +54,14 @@ func (c *ServerCommand) Run(args []string) int { return 1 } - if len(os.Getenv("VAULT_DEV_ROOT_TOKEN_ID")) > 0 { + if os.Getenv("VAULT_DEV_ROOT_TOKEN_ID") != "" { devRootTokenID = os.Getenv("VAULT_DEV_ROOT_TOKEN_ID") } + if os.Getenv("VAULT_DEV_LISTEN_ADDRESS") != "" { + devListenAddress = os.Getenv("VAULT_DEV_LISTEN_ADDRESS") + } + // Validation if !dev { switch { @@ -69,6 +73,10 @@ func (c *ServerCommand) Run(args []string) int { c.Ui.Error("Root token ID can only be specified with -dev") flags.Usage() return 1 + case devListenAddress != "": + c.Ui.Error("Development address can only be specified with -dev") + flags.Usage() + return 1 } } @@ -76,6 +84,9 @@ func (c *ServerCommand) Run(args []string) int { var config *server.Config if dev { config = server.DevConfig() + if devListenAddress != "" { + config.Listeners[0].Config["address"] = devListenAddress + } } for _, path := range configPath { current, err := server.LoadConfig(path) @@ -541,21 +552,28 @@ Usage: vault server [options] General Options: - -config= Path to the configuration file or directory. This can be - specified multiple times. If it is a directory, all - files with a ".hcl" or ".json" suffix will be loaded. + -config= Path to the configuration file or directory. This can + be specified multiple times. If it is a directory, + all files with a ".hcl" or ".json" suffix will be + loaded. - -dev Enables Dev mode. In this mode, Vault is completely - in-memory and unsealed. Do not run the Dev server in - production! + -dev Enables Dev mode. In this mode, Vault is completely + in-memory and unsealed. Do not run the Dev server in + production! - -dev-root-token-id="" If set, the root token returned in Dev mode will have the - given ID. This *only* has an effect when running in Dev - mode. + -dev-root-token-id="" If set, the root token returned in Dev mode will have + the given ID. This *only* has an effect when running + in Dev mode. Can also be specified with the + VAULT_DEV_ROOT_TOKEN_ID environment variable. - -log-level=info Log verbosity. Defaults to "info", will be outputted - to stderr. Supported values: "trace", "debug", "info", - "warn", "err" + -dev-listen-address="" If set, this overrides the normal Dev mode listen + address of "127.0.0.1:8200". Can also be specified + with the VAULT_DEV_LISTEN_ADDRESS environment + variable. + + -log-level=info Log verbosity. Defaults to "info", will be output to + stderr. Supported values: "trace", "debug", "info", + "warn", "err" ` return strings.TrimSpace(helpText) } From 2b7edf6bfd912abf56797a3ecc469e1f7d44c1c3 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 10:58:58 -0500 Subject: [PATCH 24/42] Update cubbyhole text to be more explicit. Fixes #1165 --- .../source/docs/secrets/cubbyhole/index.html.md | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/website/source/docs/secrets/cubbyhole/index.html.md b/website/source/docs/secrets/cubbyhole/index.html.md index 01d1aeb218..443424e48e 100644 --- a/website/source/docs/secrets/cubbyhole/index.html.md +++ b/website/source/docs/secrets/cubbyhole/index.html.md @@ -15,13 +15,14 @@ the configured physical storage for Vault. It is mounted at the `cubbyhole/` prefix by default and cannot be mounted elsewhere or removed. This backend differs from the `generic` backend in that the `generic` backend's -values are accessible to any token with read privileges on that path. In this -backend, paths are scoped per token; no token can read secrets placed in -another token's cubbyhole. When the token expires, its cubbyhole is destroyed. +values are accessible to any token with read privileges on that path. In +`cubbyhole`, paths are scoped per token; no token can access another token's +cubbyhole, whether to read, write, list, or for any other operation. When the +token expires, its cubbyhole is destroyed. Also unlike the `generic` backend, because the cubbyhole's lifetime is linked -to an authentication token, there is no concept of a lease or lease TTL for -values contained in the token's cubbyhole. +to that of an authentication token, there is no concept of a TTL for values +contained in the token's cubbyhole. Writing to a key in the `cubbyhole` backend will replace the old value; the sub-fields are not merged together. @@ -96,9 +97,7 @@ As expected, the value previously set is returned to us.
    Returns a list of secret entries at the specified location. Folders are suffixed with `/`. The input must be a folder; list on a file will not - return a value. Note that no policy-based filtering is performed on - returned keys; it is not recommended to put sensitive or secret values as - key names. The values themselves are not accessible via this command. + return a value. The values themselves are not accessible via this command.
    Method
    From 1ae2f2fd34eee07542ab0f25b29ec3497a6110a0 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 11:09:27 -0500 Subject: [PATCH 25/42] Remove unneeded sleeps in test code --- vault/core_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/vault/core_test.go b/vault/core_test.go index c66cf1fa82..ec54585736 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -1106,9 +1106,6 @@ func TestCore_Standby_Seal(t *testing.T) { // Wait for core to become active testWaitActive(t, core) - // Ensure that the original clean function has stopped running - time.Sleep(2 * time.Second) - // Check the leader is local isLeader, advertise, err := core.Leader() if err != nil { @@ -1214,9 +1211,6 @@ func TestCore_StepDown(t *testing.T) { // Wait for core to become active testWaitActive(t, core) - // Ensure that the original clean function has stopped running - time.Sleep(2 * time.Second) - // Check the leader is local isLeader, advertise, err := core.Leader() if err != nil { From 93e8ce28c35aacfa9496960d2213f0d749a7d90f Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 11:12:15 -0500 Subject: [PATCH 26/42] changelog++ --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 628450754b..5dbb5cb7eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ IMPROVEMENTS: * command/server: The initial root token ID when running in `-dev` mode can now be specified via `-dev-root-token-id` or the environment variable `VAULT_DEV_ROOT_TOKEN_ID` [GH-1162] + * command/server: The listen address when running in `-dev` mode can now be + specified via `-dev-listen-address` or the environment variable + `VAULT_DEV_LISTEN_ADDRESS` [GH-1169] * command/token-renew: Allow no token to be passed in; use `renew-self` in this case. Change the behavior for any token being passed in to use `renew`. [GH-1150] From 3b84ae6ccabd159b8ac625ef01779d3d9ba6e5ca Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 11:32:48 -0500 Subject: [PATCH 27/42] Strip leading paths in policies. It appears to be a common mistake, but they won't ever match. Fixes #1167 --- vault/policy.go | 6 ++++++ vault/policy_test.go | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/vault/policy.go b/vault/policy.go index 2bb5dd31f6..6f856e7b72 100644 --- a/vault/policy.go +++ b/vault/policy.go @@ -74,6 +74,12 @@ func Parse(rules string) (*Policy, error) { // Validate the path policy for _, pc := range p.Paths { + // Strip a leading '/' as paths in Vault start after the / in the API + // path + if len(pc.Prefix) > 0 && pc.Prefix[0] == '/' { + pc.Prefix = pc.Prefix[1:] + } + // Strip the glob character if found if strings.HasSuffix(pc.Prefix, "*") { pc.Prefix = strings.TrimSuffix(pc.Prefix, "*") diff --git a/vault/policy_test.go b/vault/policy_test.go index 766a531c6a..7ba6eb9f7c 100644 --- a/vault/policy_test.go +++ b/vault/policy_test.go @@ -80,7 +80,8 @@ path "prod/version" { } # Read access to foobar -path "foo/bar" { +# Also tests stripping of leading slash +path "/foo/bar" { policy = "read" } From d0e0cdc66313474d1403d7ada5f5ca7cf73ac2ea Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 12:04:48 -0500 Subject: [PATCH 28/42] changelog++ --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dbb5cb7eb..bf06ecb4ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,6 @@ ## 0.5.2 (Unreleased) IMPROVEMENTS: - * credential/cert: Non-CA certificates can be used for authentication. They - must be matched exactly (issuer and serial number) for authentication, and - the certificate must carry the client authentication or 'any' extended usage - attributes. [GH-1153] - * secret/ssh: Added documentation for `ssh/config/zeroaddress` endpoint. [GH-1154] * command/server: The initial root token ID when running in `-dev` mode can now be specified via `-dev-root-token-id` or the environment variable `VAULT_DEV_ROOT_TOKEN_ID` [GH-1162] @@ -15,6 +10,11 @@ IMPROVEMENTS: * command/token-renew: Allow no token to be passed in; use `renew-self` in this case. Change the behavior for any token being passed in to use `renew`. [GH-1150] + * credential/cert: Non-CA certificates can be used for authentication. They + must be matched exactly (issuer and serial number) for authentication, and + the certificate must carry the client authentication or 'any' extended usage + attributes. [GH-1153] + * secret/ssh: Added documentation for `ssh/config/zeroaddress` endpoint. [GH-1154] BUG FIXES: From 1611a39c01d6f24273abecb494cb9a243b3e65bb Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 12:06:42 -0500 Subject: [PATCH 29/42] changelog++ --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf06ecb4ca..0092e3aa04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## 0.5.2 (Unreleased) IMPROVEMENTS: + * core: Ignore leading `/` in policy paths [GH-1170] * command/server: The initial root token ID when running in `-dev` mode can now be specified via `-dev-root-token-id` or the environment variable `VAULT_DEV_ROOT_TOKEN_ID` [GH-1162] From 4e964a62f6c7d7c5f002399d987579a41eaa2932 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 12:29:30 -0500 Subject: [PATCH 30/42] Add default case for if the step down channel is blocked --- vault/core.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/vault/core.go b/vault/core.go index 1340c0f93c..ff2a79339e 100644 --- a/vault/core.go +++ b/vault/core.go @@ -1269,7 +1269,11 @@ func (c *Core) StepDown(token string) error { return logical.ErrPermissionDenied } - c.manualStepDownCh <- struct{}{} + select { + case c.manualStepDownCh <- struct{}{}: + default: + c.logger.Printf("[WARN] core: manual step-down operation already queued") + } return nil } From 161413e4c95077b9e5c76d7d67e2ecff4707ed49 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 12:33:38 -0500 Subject: [PATCH 31/42] changelog++ --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0092e3aa04..5b423ba25e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ IMPROVEMENTS: * command/server: The listen address when running in `-dev` mode can now be specified via `-dev-listen-address` or the environment variable `VAULT_DEV_LISTEN_ADDRESS` [GH-1169] + * command/step-down: New `vault step-down` command and API endpoint to force + the targeted node to give up active status, but without sealing. The node + will wait ten seconds before attempting too grab the lock again. [GH-1146] * command/token-renew: Allow no token to be passed in; use `renew-self` in this case. Change the behavior for any token being passed in to use `renew`. [GH-1150] From 5f0beb733099aa6c127d2c4eb528120c3fecb06f Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 13:13:47 -0500 Subject: [PATCH 32/42] Create a unified function to sanitize mount paths. This allows mount paths to start with '/' in addition to ensuring they end in '/' before leaving the system backend. --- api/sys_auth.go | 16 ---------------- api/sys_mounts.go | 31 ------------------------------- vault/logical_system.go | 35 ++++++++++++++++++++++++++--------- 3 files changed, 26 insertions(+), 56 deletions(-) diff --git a/api/sys_auth.go b/api/sys_auth.go index bdc1948918..8e1cdec390 100644 --- a/api/sys_auth.go +++ b/api/sys_auth.go @@ -18,10 +18,6 @@ func (c *Sys) ListAuth() (map[string]*AuthMount, error) { } func (c *Sys) EnableAuth(path, authType, desc string) error { - if err := c.checkAuthPath(path); err != nil { - return err - } - body := map[string]string{ "type": authType, "description": desc, @@ -42,10 +38,6 @@ func (c *Sys) EnableAuth(path, authType, desc string) error { } func (c *Sys) DisableAuth(path string) error { - if err := c.checkAuthPath(path); err != nil { - return err - } - r := c.c.NewRequest("DELETE", fmt.Sprintf("/v1/sys/auth/%s", path)) resp, err := c.c.RawRequest(r) if err == nil { @@ -54,14 +46,6 @@ func (c *Sys) DisableAuth(path string) error { return err } -func (c *Sys) checkAuthPath(path string) error { - if path[0] == '/' { - return fmt.Errorf("path must not start with /: %s", path) - } - - return nil -} - // Structures for the requests/resposne are all down here. They aren't // individually documentd because the map almost directly to the raw HTTP API // documentation. Please refer to that documentation for more details. diff --git a/api/sys_mounts.go b/api/sys_mounts.go index 4fd5e8b169..623a146286 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -20,10 +20,6 @@ func (c *Sys) ListMounts() (map[string]*MountOutput, error) { } func (c *Sys) Mount(path string, mountInfo *MountInput) error { - if err := c.checkMountPath(path); err != nil { - return err - } - body := structs.Map(mountInfo) r := c.c.NewRequest("POST", fmt.Sprintf("/v1/sys/mounts/%s", path)) @@ -41,10 +37,6 @@ func (c *Sys) Mount(path string, mountInfo *MountInput) error { } func (c *Sys) Unmount(path string) error { - if err := c.checkMountPath(path); err != nil { - return err - } - r := c.c.NewRequest("DELETE", fmt.Sprintf("/v1/sys/mounts/%s", path)) resp, err := c.c.RawRequest(r) if err == nil { @@ -54,13 +46,6 @@ func (c *Sys) Unmount(path string) error { } func (c *Sys) Remount(from, to string) error { - if err := c.checkMountPath(from); err != nil { - return err - } - if err := c.checkMountPath(to); err != nil { - return err - } - body := map[string]interface{}{ "from": from, "to": to, @@ -79,10 +64,6 @@ func (c *Sys) Remount(from, to string) error { } func (c *Sys) TuneMount(path string, config MountConfigInput) error { - if err := c.checkMountPath(path); err != nil { - return err - } - body := structs.Map(config) r := c.c.NewRequest("POST", fmt.Sprintf("/v1/sys/mounts/%s/tune", path)) if err := r.SetJSONBody(body); err != nil { @@ -97,10 +78,6 @@ func (c *Sys) TuneMount(path string, config MountConfigInput) error { } func (c *Sys) MountConfig(path string) (*MountConfigOutput, error) { - if err := c.checkMountPath(path); err != nil { - return nil, err - } - r := c.c.NewRequest("GET", fmt.Sprintf("/v1/sys/mounts/%s/tune", path)) resp, err := c.c.RawRequest(r) @@ -113,14 +90,6 @@ func (c *Sys) MountConfig(path string) (*MountConfigOutput, error) { return &result, err } -func (c *Sys) checkMountPath(path string) error { - if path[0] == '/' { - return fmt.Errorf("path must not start with /: %s", path) - } - - return nil -} - type MountInput struct { Type string `json:"type" structs:"type"` Description string `json:"description" structs:"description"` diff --git a/vault/logical_system.go b/vault/logical_system.go index e2eac88e39..5217c97e7a 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -466,6 +466,8 @@ func (b *SystemBackend) handleMount( logicalType := data.Get("type").(string) description := data.Get("description").(string) + path = sanitizeMountPath(path) + var config MountConfig var apiConfig struct { @@ -562,6 +564,8 @@ func (b *SystemBackend) handleUnmount( return logical.ErrorResponse("path cannot be blank"), logical.ErrInvalidRequest } + suffix = sanitizeMountPath(suffix) + // Attempt unmount if err := b.Core.unmount(suffix); err != nil { b.Backend.Logger().Printf("[ERR] sys: unmount '%s' failed: %v", suffix, err) @@ -583,6 +587,9 @@ func (b *SystemBackend) handleRemount( logical.ErrInvalidRequest } + fromPath = sanitizeMountPath(fromPath) + toPath = sanitizeMountPath(toPath) + // Attempt remount if err := b.Core.remount(fromPath, toPath); err != nil { b.Backend.Logger().Printf("[ERR] sys: remount '%s' to '%s' failed: %v", fromPath, toPath, err) @@ -602,9 +609,7 @@ func (b *SystemBackend) handleMountTuneRead( logical.ErrInvalidRequest } - if !strings.HasSuffix(path, "/") { - path += "/" - } + path = sanitizeMountPath(path) sysView := b.Core.router.MatchingSystemView(path) if sysView == nil { @@ -633,9 +638,7 @@ func (b *SystemBackend) handleMountTuneWrite( logical.ErrInvalidRequest } - if !strings.HasSuffix(path, "/") { - path += "/" - } + path = sanitizeMountPath(path) // Prevent protected paths from being changed for _, p := range untunableMounts { @@ -777,6 +780,8 @@ func (b *SystemBackend) handleEnableAuth( logical.ErrInvalidRequest } + path = sanitizeMountPath(path) + // Create the mount entry me := &MountEntry{ Path: path, @@ -800,6 +805,8 @@ func (b *SystemBackend) handleDisableAuth( return logical.ErrorResponse("path cannot be blank"), logical.ErrInvalidRequest } + suffix = sanitizeMountPath(suffix) + // Attempt disable if err := b.Core.disableCredential(suffix); err != nil { b.Backend.Logger().Printf("[ERR] sys: disable auth '%s' failed: %v", suffix, err) @@ -908,9 +915,7 @@ func (b *SystemBackend) handleAuditHash( return logical.ErrorResponse("the \"input\" parameter is empty"), nil } - if !strings.HasSuffix(path, "/") { - path += "/" - } + path = sanitizeMountPath(path) hash, err := b.Core.auditBroker.GetHash(path, input) if err != nil { @@ -1089,6 +1094,18 @@ func (b *SystemBackend) handleRotate( return nil, nil } +func sanitizeMountPath(path string) string { + if !strings.HasSuffix(path, "/") { + path += "/" + } + + if strings.HasPrefix(path, "/") { + path = path[1:] + } + + return path +} + const sysHelpRoot = ` The system backend is built-in to Vault and cannot be remounted or unmounted. It contains the paths that are used to configure Vault itself From f6840bea61c2c69d290fbddb23ff6ab377d937bb Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 13:37:51 -0500 Subject: [PATCH 33/42] Fix out-of-date comment --- vault/acl.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vault/acl.go b/vault/acl.go index 51800ae2db..ac4922407b 100644 --- a/vault/acl.go +++ b/vault/acl.go @@ -63,8 +63,7 @@ func NewACL(policies []*Policy) (*ACL, error) { default: // Insert the capabilities in this new policy into the existing - // value; since it's a pointer we can just modify the - // underlying data + // value tree.Insert(pc.Prefix, existing|pc.CapabilitiesBitmap) } } From f538393d24b50d5e833e740dd2cbbcbb29f376de Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 13:47:00 -0500 Subject: [PATCH 34/42] changelog++ --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b423ba25e..9a02603467 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ IMPROVEMENTS: * core: Ignore leading `/` in policy paths [GH-1170] + * core: Ignore leading `/` in mount paths [GH-1172] * command/server: The initial root token ID when running in `-dev` mode can now be specified via `-dev-root-token-id` or the environment variable `VAULT_DEV_ROOT_TOKEN_ID` [GH-1162] From 67b8eab20472e42ce6f4d7d7461f610fa2674ec8 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 3 Mar 2016 18:10:14 -0500 Subject: [PATCH 35/42] Update help text exporting dev mode listen address. Ping #1160 --- command/server.go | 2 +- command/server/config.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/command/server.go b/command/server.go index 860fc5ce40..3fc103304b 100644 --- a/command/server.go +++ b/command/server.go @@ -239,7 +239,7 @@ func (c *ServerCommand) Run(args []string) int { "immediately begin using the Vault CLI.\n\n"+ "The only step you need to take is to set the following\n"+ "environment variables:\n\n"+ - " "+export+" VAULT_ADDR="+quote+"http://127.0.0.1:8200"+quote+"\n\n"+ + " "+export+" VAULT_ADDR="+quote+"http://"+config.Listeners[0].Config["address"]+quote+"\n\n"+ "The unseal key and root token are reproduced below in case you\n"+ "want to seal/unseal the Vault or play with authentication.\n\n"+ "Unseal Key: %s\nRoot Token: %s\n", diff --git a/command/server/config.go b/command/server/config.go index 253bfc55f2..cce2d36f3a 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -44,6 +44,7 @@ func DevConfig() *Config { &Listener{ Type: "tcp", Config: map[string]string{ + "address": "127.0.0.1:8200", "tls_disable": "1", }, }, From ea52b6ec8be42ec28a5e862b9b1076912025c828 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Fri, 4 Mar 2016 14:56:51 -0500 Subject: [PATCH 36/42] changed response of expiration manager's renewtoken to logical.response --- vault/expiration.go | 9 ++++++--- vault/token_store.go | 11 +---------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/vault/expiration.go b/vault/expiration.go index 3e06d2591f..ec55bf9f68 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -310,7 +310,7 @@ func (m *ExpirationManager) Renew(leaseID string, increment time.Duration) (*log // RenewToken is used to renew a token which does not need to // invoke a logical backend. func (m *ExpirationManager) RenewToken(req *logical.Request, source string, token string, - increment time.Duration) (*logical.Auth, error) { + increment time.Duration) (*logical.Response, error) { defer metrics.MeasureSince([]string{"expire", "renew-token"}, time.Now()) // Compute the Lease ID leaseID := path.Join(source, m.tokenStore.SaltID(token)) @@ -331,13 +331,16 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke if err != nil { return nil, err } + if resp.IsError() { + return resp, nil + } // Fast-path if there is no renewal if resp == nil { return nil, nil } if resp.Auth == nil || !resp.Auth.LeaseEnabled() { - return resp.Auth, nil + return resp, nil } // Attach the ClientToken @@ -354,7 +357,7 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke // Update the expiration time m.updatePending(le, resp.Auth.LeaseTotal()) - return resp.Auth, nil + return resp, nil } // Register is used to take a request and response with an associated diff --git a/vault/token_store.go b/vault/token_store.go index 0e8e5dcff9..b319e04a6e 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -895,16 +895,7 @@ func (ts *TokenStore) handleRenew( } // Renew the token and its children - auth, err := ts.expiration.RenewToken(req, te.Path, te.ID, increment) - if err != nil { - return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest - } - - // Generate the response - resp := &logical.Response{ - Auth: auth, - } - return resp, nil + return ts.expiration.RenewToken(req, te.Path, te.ID, increment) } func (ts *TokenStore) destroyCubbyhole(saltedID string) error { From 4f5f2a4376f55b58be77fee3734bb635415b92dd Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Fri, 4 Mar 2016 15:03:01 -0500 Subject: [PATCH 37/42] Fix testcase --- vault/expiration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/expiration_test.go b/vault/expiration_test.go index e962947ae5..c086cd05e9 100644 --- a/vault/expiration_test.go +++ b/vault/expiration_test.go @@ -424,7 +424,7 @@ func TestExpiration_RenewToken(t *testing.T) { t.Fatalf("err: %v", err) } - if auth.ClientToken != out.ClientToken { + if auth.ClientToken != out.Auth.ClientToken { t.Fatalf("Bad: %#v", out) } } From a94a7a8c95817dd5bae626f2c3c6c707e3c4ecc6 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Fri, 4 Mar 2016 15:13:04 -0500 Subject: [PATCH 38/42] Place the response nil check before resp.IsError() --- vault/expiration.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/vault/expiration.go b/vault/expiration.go index ec55bf9f68..6f50112ddb 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -331,14 +331,15 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke if err != nil { return nil, err } + + if resp == nil { + return nil, nil + } + if resp.IsError() { return resp, nil } - // Fast-path if there is no renewal - if resp == nil { - return nil, nil - } if resp.Auth == nil || !resp.Auth.LeaseEnabled() { return resp, nil } From 295142846858255c6451b40e559468be716498b7 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Fri, 4 Mar 2016 15:35:58 -0500 Subject: [PATCH 39/42] review rework --- vault/expiration.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/vault/expiration.go b/vault/expiration.go index 6f50112ddb..fad33c6c0c 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -337,11 +337,13 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke } if resp.IsError() { - return resp, nil + return &logical.Response{ + Data: resp.Data, + }, nil } if resp.Auth == nil || !resp.Auth.LeaseEnabled() { - return resp, nil + return &logical.Response{}, nil } // Attach the ClientToken @@ -358,7 +360,9 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke // Update the expiration time m.updatePending(le, resp.Auth.LeaseTotal()) - return resp, nil + return &logical.Response{ + Auth: resp.Auth, + }, nil } // Register is used to take a request and response with an associated From ee71f8198a17a050e960d5ae5db8d05474bda45f Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Fri, 4 Mar 2016 18:08:13 -0500 Subject: [PATCH 40/42] review rework 2 --- vault/expiration.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vault/expiration.go b/vault/expiration.go index fad33c6c0c..cf27549021 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -343,7 +343,9 @@ func (m *ExpirationManager) RenewToken(req *logical.Request, source string, toke } if resp.Auth == nil || !resp.Auth.LeaseEnabled() { - return &logical.Response{}, nil + return &logical.Response{ + Auth: resp.Auth, + }, nil } // Attach the ClientToken From 4f672fb1e08817373ed6eec2433e8ef9d3dbfcbd Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Fri, 4 Mar 2016 18:23:08 -0500 Subject: [PATCH 41/42] changelog++ --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a02603467..902a01c12f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ BUG FIXES: generated username, not just the UUID, in order to handle token display name hyphens [GH-1140] * physical/etcd: Output actual error when cluster sync fails [GH-1141] + * vault/expiration: Not letting the error responses from the backends to skip + during renewals [GH-1176] ## 0.5.1 (February 25th, 2016) From 7ef904b930e6010aeeadc66123cbc624c26badf7 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 7 Mar 2016 09:34:16 -0500 Subject: [PATCH 42/42] Use better error message on LDAP renew failure --- builtin/credential/ldap/path_login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/credential/ldap/path_login.go b/builtin/credential/ldap/path_login.go index 1e3a90451f..58e36cbfaf 100644 --- a/builtin/credential/ldap/path_login.go +++ b/builtin/credential/ldap/path_login.go @@ -76,7 +76,7 @@ func (b *backend) pathLoginRenew( sort.Strings(policies) if strings.Join(policies, ",") != prevpolicies { - return logical.ErrorResponse("policies have changed, revoking login"), nil + return logical.ErrorResponse("policies have changed, not renewing"), nil } return framework.LeaseExtend(0, 0, b.System())(req, d)