Merge pull request #1854 from hashicorp/f-vault-role-validate

Validate the Vault role being used

nomad/vault.go

@@ -13,6 +13,7 @@ import (
 
 	"gopkg.in/tomb.v2"
 
+	multierror "github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/nomad/structs/config"
 	vapi "github.com/hashicorp/vault/api"
@@ -204,6 +205,12 @@ func (v *vaultClient) Stop() {
 // cancelled if set un-active as it is assumed another instances is taking over
 func (v *vaultClient) SetActive(active bool) {
 	atomic.StoreInt32(&v.active, 1)
+
+	// Clear out the revoking tokens
+	v.revLock.Lock()
+	v.revoking = make(map[*structs.VaultAccessor]time.Time)
+	v.revLock.Unlock()
+
 	return
 }
 
@@ -335,7 +342,7 @@ OUTER:
 
 	// Retrieve our token, validate it and parse the lease duration
 	if err := v.parseSelfToken(); err != nil {
-		v.logger.Printf("[ERR] vault: failed to lookup self token and not retrying: %v", err)
+		v.logger.Printf("[ERR] vault: failed to validate self token/role and not retrying: %v", err)
 		v.l.Lock()
 		v.connEstablished = false
 		v.connEstablishedErr = err
@@ -508,39 +515,93 @@ func (v *vaultClient) parseSelfToken() error {
 		}
 	}
 
+	var mErr multierror.Error
 	if !root {
 		// All non-root tokens must be renewable
 		if !data.Renewable {
-			return fmt.Errorf("Vault token is not renewable or root")
+			multierror.Append(&mErr, fmt.Errorf("Vault token is not renewable or root"))
 		}
 
 		// All non-root tokens must have a lease duration
 		if data.CreationTTL == 0 {
-			return fmt.Errorf("invalid lease duration of zero")
+			multierror.Append(&mErr, fmt.Errorf("invalid lease duration of zero"))
 		}
 
 		// The lease duration can not be expired
 		if data.TTL == 0 {
-			return fmt.Errorf("token TTL is zero")
+			multierror.Append(&mErr, fmt.Errorf("token TTL is zero"))
 		}
 
-		// There must be a valid role
+		// There must be a valid role since we aren't root
 		if data.Role == "" {
-			return fmt.Errorf("token role name must be set when not using a root token")
+			multierror.Append(&mErr, fmt.Errorf("token role name must be set when not using a root token"))
 		}
+
 	} else if data.CreationTTL != 0 {
 		// If the root token has a TTL it must be renewable
 		if !data.Renewable {
-			return fmt.Errorf("Vault token has a TTL but is not renewable")
+			multierror.Append(&mErr, fmt.Errorf("Vault token has a TTL but is not renewable"))
 		} else if data.TTL == 0 {
 			// If the token has a TTL make sure it has not expired
-			return fmt.Errorf("token TTL is zero")
+			multierror.Append(&mErr, fmt.Errorf("token TTL is zero"))
+		}
+	}
+
+	// If given a role validate it
+	if data.Role != "" {
+		if err := v.validateRole(data.Role); err != nil {
+			multierror.Append(&mErr, err)
 		}
 	}
 
 	data.Root = root
 	v.tokenData = &data
-	return nil
+	return mErr.ErrorOrNil()
+}
+
+// validateRole contacts Vault and checks that the given Vault role is valid for
+// the purposes of being used by Nomad
+func (v *vaultClient) validateRole(role string) error {
+	if role == "" {
+		return fmt.Errorf("Invalid empty role name")
+	}
+
+	// Validate the role
+	rsecret, err := v.client.Logical().Read(fmt.Sprintf("auth/token/roles/%s", role))
+	if err != nil {
+		return fmt.Errorf("failed to lookup role %q: %v", role, err)
+	}
+
+	// Read and parse the fields
+	var data struct {
+		ExplicitMaxTtl int `mapstructure:"explicit_max_ttl"`
+		Orphan         bool
+		Period         int
+		Renewable      bool
+	}
+	if err := mapstructure.WeakDecode(rsecret.Data, &data); err != nil {
+		return fmt.Errorf("failed to parse Vault role's data block: %v", err)
+	}
+
+	// Validate the role is acceptable
+	var mErr multierror.Error
+	if data.Orphan {
+		multierror.Append(&mErr, fmt.Errorf("Role must not allow orphans"))
+	}
+
+	if !data.Renewable {
+		multierror.Append(&mErr, fmt.Errorf("Role must allow tokens to be renewed"))
+	}
+
+	if data.ExplicitMaxTtl != 0 {
+		multierror.Append(&mErr, fmt.Errorf("Role can not use an explicit max ttl. Token must be periodic."))
+	}
+
+	if data.Period == 0 {
+		multierror.Append(&mErr, fmt.Errorf("Role must have a non-zero period to make tokens periodic."))
+	}
+
+	return mErr.ErrorOrNil()
 }
 
 // ConnectionEstablished returns whether a connection to Vault has been

nomad/vault_test.go

@@ -24,7 +24,12 @@ const (
 	// authPolicy is a policy that allows token creation operations
 	authPolicy = `path "auth/token/create/*" {
 	capabilities = ["create", "read", "update", "delete", "list"]
-}`
+}
+
+path "auth/token/roles/*" {
+	capabilities = ["create", "read", "update", "delete", "list"]
+}
+`
 )
 
 func TestVaultClient_BadConfig(t *testing.T) {
@@ -78,6 +83,54 @@ func TestVaultClient_EstablishConnection(t *testing.T) {
 	waitForConnection(client, t)
 }
 
+func TestVaultClient_ValidateRole(t *testing.T) {
+	v := testutil.NewTestVault(t).Start()
+	defer v.Stop()
+
+	// Set the configs token in a new test role
+	data := map[string]interface{}{
+		"allowed_policies": "default,root",
+		"orphan":           true,
+		"renewable":        true,
+		"explicit_max_ttl": 10,
+	}
+	v.Config.Token = testVaultRoleAndToken(v, t, data)
+
+	logger := log.New(os.Stderr, "", log.LstdFlags)
+	v.Config.ConnectionRetryIntv = 100 * time.Millisecond
+	client, err := NewVaultClient(v.Config, logger, nil)
+	if err != nil {
+		t.Fatalf("failed to build vault client: %v", err)
+	}
+	defer client.Stop()
+
+	// Wait for an error
+	var conn bool
+	var connErr error
+	testutil.WaitForResult(func() (bool, error) {
+		conn, connErr = client.ConnectionEstablished()
+		if conn {
+			return false, fmt.Errorf("Should not connect")
+		}
+
+		if connErr == nil {
+			return false, fmt.Errorf("expect an error")
+		}
+
+		return true, nil
+	}, func(err error) {
+		t.Fatalf("bad: %v", err)
+	})
+
+	errStr := connErr.Error()
+	if !strings.Contains(errStr, "not allow orphans") {
+		t.Fatalf("Expect orphan error")
+	}
+	if !strings.Contains(errStr, "explicit max ttl") {
+		t.Fatalf("Expect explicit max ttl error")
+	}
+}
+
 func TestVaultClient_SetActive(t *testing.T) {
 	v := testutil.NewTestVault(t).Start()
 	defer v.Stop()
@@ -115,7 +168,7 @@ func TestVaultClient_SetConfig(t *testing.T) {
 	defer v2.Stop()
 
 	// Set the configs token in a new test role
-	v2.Config.Token = testVaultRoleAndToken(v2, t, 20)
+	v2.Config.Token = defaultTestVaultRoleAndToken(v2, t, 20)
 
 	logger := log.New(os.Stderr, "", log.LstdFlags)
 	client, err := NewVaultClient(v.Config, logger, nil)
@@ -142,9 +195,18 @@ func TestVaultClient_SetConfig(t *testing.T) {
 	}
 }
 
-// testVaultRoleAndToken creates a test Vault role where children are created
-// with the passed period. A token created in that role is returned
-func testVaultRoleAndToken(v *testutil.TestVault, t *testing.T, rolePeriod int) string {
+// defaultTestVaultRoleAndToken creates a test Vault role and returns a token
+// created in that role
+func defaultTestVaultRoleAndToken(v *testutil.TestVault, t *testing.T, rolePeriod int) string {
+	d := make(map[string]interface{}, 2)
+	d["allowed_policies"] = "default,auth"
+	d["period"] = rolePeriod
+	return testVaultRoleAndToken(v, t, d)
+}
+
+// testVaultRoleAndToken creates a test Vault role with the specified data and
+// returns a token created in that role
+func testVaultRoleAndToken(v *testutil.TestVault, t *testing.T, data map[string]interface{}) string {
 	// Build the auth policy
 	sys := v.Client.Sys()
 	if err := sys.PutPolicy("auth", authPolicy); err != nil {
@@ -153,10 +215,7 @@ func testVaultRoleAndToken(v *testutil.TestVault, t *testing.T, rolePeriod int)
 
 	// Build a role
 	l := v.Client.Logical()
-	d := make(map[string]interface{}, 2)
-	d["allowed_policies"] = "default,auth"
-	d["period"] = rolePeriod
-	l.Write("auth/token/roles/test", d)
+	l.Write("auth/token/roles/test", data)
 
 	// Create a new token with the role
 	a := v.Client.Auth().Token()
@@ -179,7 +238,7 @@ func TestVaultClient_RenewalLoop(t *testing.T) {
 	defer v.Stop()
 
 	// Set the configs token in a new test role
-	v.Config.Token = testVaultRoleAndToken(v, t, 5)
+	v.Config.Token = defaultTestVaultRoleAndToken(v, t, 5)
 
 	// Start the client
 	logger := log.New(os.Stderr, "", log.LstdFlags)
@@ -417,7 +476,7 @@ func TestVaultClient_CreateToken_Role(t *testing.T) {
 	defer v.Stop()
 
 	// Set the configs token in a new test role
-	v.Config.Token = testVaultRoleAndToken(v, t, 5)
+	v.Config.Token = defaultTestVaultRoleAndToken(v, t, 5)
 
 	// Start the client
 	logger := log.New(os.Stderr, "", log.LstdFlags)
@@ -464,7 +523,7 @@ func TestVaultClient_CreateToken_Role_InvalidToken(t *testing.T) {
 	defer v.Stop()
 
 	// Set the configs token in a new test role
-	testVaultRoleAndToken(v, t, 5)
+	defaultTestVaultRoleAndToken(v, t, 5)
 	v.Config.Token = "foo-bar"
 
 	// Start the client