[WIP] Support custom max Nomad token name length [supersedes hashicor…

…p/vault#4361] (#5117) * Nomad: updating max token length to 256 * Initial support for supporting custom max token name length for Nomad * simplify/correct tests * document nomad max_token_name_length * removed support for max token length env var. Rename field for clarity * cleanups after removing env var support * move RandomWithPrefix to testhelpers * fix spelling * Remove default 256 value. Use zero as a sentinel value and ignore it * update docs
JanMa · Aug 16, 2018 · 4934f42 · 4934f42
1 parent 956e32a
commit 4934f42
Show file tree

Hide file tree

Showing 4 changed files with 185 additions and 10 deletions.
diff --git a/backend.go b/backend.go
@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/vault/logical/framework"
 )
 
+// Factory returns a Nomad backend that satisfies the logical.Backend interface
 func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) {
 	b := Backend()
 	if err := b.Setup(ctx, conf); err != nil {
@@ -16,6 +17,7 @@ func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend,
 	return b, nil
 }
 
+// Backend returns the configured Nomad backend
 func Backend() *backend {
 	var b backend
 	b.Backend = &framework.Backend{
@@ -66,5 +68,6 @@ func (b *backend) client(ctx context.Context, s logical.Storage) (*api.Client, e
 	if err != nil {
 		return nil, err
 	}
+
 	return client, nil
 }
diff --git a/backend_test.go b/backend_test.go
@@ -9,6 +9,7 @@ import (
 	"time"
 
 	nomadapi "github.com/hashicorp/nomad/api"
+	"github.com/hashicorp/vault/helper/testhelpers"
 	"github.com/hashicorp/vault/logical"
 	"github.com/mitchellh/mapstructure"
 	"github.com/ory/dockertest"
@@ -29,8 +30,8 @@ func prepareTestContainer(t *testing.T) (cleanup func(), retAddress string, noma
 	}
 
 	dockerOptions := &dockertest.RunOptions{
-		Repository: "djenriquez/nomad",
-		Tag:        "latest",
+		Repository: "catsby/nomad",
+		Tag:        "0.8.4",
 		Cmd:        []string{"agent", "-dev"},
 		Env:        []string{`NOMAD_LOCAL_CONFIG=bind_addr = "0.0.0.0" acl { enabled = true }`},
 	}
@@ -142,7 +143,8 @@ func TestBackend_config_access(t *testing.T) {
 	}
 
 	expected := map[string]interface{}{
-		"address": connData["address"].(string),
+		"address":               connData["address"].(string),
+		"max_token_name_length": 0,
 	}
 	if !reflect.DeepEqual(expected, resp.Data) {
 		t.Fatalf("bad: expected:%#v\nactual:%#v\n", expected, resp.Data)
@@ -300,3 +302,153 @@ func TestBackend_CredsCreateEnvVar(t *testing.T) {
 		t.Fatalf("resp is error: %v", resp.Error())
 	}
 }
+
+func TestBackend_max_token_name_length(t *testing.T) {
+	config := logical.TestBackendConfig()
+	config.StorageView = &logical.InmemStorage{}
+	b, err := Factory(context.Background(), config)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cleanup, connURL, connToken := prepareTestContainer(t)
+	defer cleanup()
+
+	testCases := []struct {
+		title       string
+		roleName    string
+		tokenLength int
+	}{
+		{
+			title: "Default",
+		},
+		{
+			title:       "ConfigOverride",
+			tokenLength: 64,
+		},
+		{
+			title:       "ConfigOverride-LongName",
+			roleName:    "testlongerrolenametoexceed64charsdddddddddddddddddddddddd",
+			tokenLength: 64,
+		},
+		{
+			title:    "Notrim",
+			roleName: "testlongersubrolenametoexceed64charsdddddddddddddddddddddddd",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.title, func(t *testing.T) {
+			// setup config/access
+			connData := map[string]interface{}{
+				"address":               connURL,
+				"token":                 connToken,
+				"max_token_name_length": tc.tokenLength,
+			}
+			expected := map[string]interface{}{
+				"address":               connURL,
+				"max_token_name_length": tc.tokenLength,
+			}
+
+			expectedMaxTokenNameLength := maxTokenNameLength
+			if tc.tokenLength != 0 {
+				expectedMaxTokenNameLength = tc.tokenLength
+			}
+
+			confReq := logical.Request{
+				Operation: logical.UpdateOperation,
+				Path:      "config/access",
+				Storage:   config.StorageView,
+				Data:      connData,
+			}
+
+			resp, err := b.HandleRequest(context.Background(), &confReq)
+			if err != nil || (resp != nil && resp.IsError()) || resp != nil {
+				t.Fatalf("failed to write configuration: resp:%#v err:%s", resp, err)
+			}
+			confReq.Operation = logical.ReadOperation
+			resp, err = b.HandleRequest(context.Background(), &confReq)
+			if err != nil || (resp != nil && resp.IsError()) {
+				t.Fatalf("failed to write configuration: resp:%#v err:%s", resp, err)
+			}
+
+			// verify token length is returned in the config/access query
+			if !reflect.DeepEqual(expected, resp.Data) {
+				t.Fatalf("bad: expected:%#v\nactual:%#v\n", expected, resp.Data)
+			}
+			// verify token is not returned
+			if resp.Data["token"] != nil {
+				t.Fatalf("token should not be set in the response")
+			}
+
+			// create a role to create nomad credentials with
+			// Seeds random with current timestamp
+
+			if tc.roleName == "" {
+				tc.roleName = "test"
+			}
+			roleTokenName := testhelpers.RandomWithPrefix(tc.roleName)
+
+			confReq.Path = "role/" + roleTokenName
+			confReq.Operation = logical.UpdateOperation
+			confReq.Data = map[string]interface{}{
+				"policies": []string{"policy"},
+				"lease":    "6h",
+			}
+			resp, err = b.HandleRequest(context.Background(), &confReq)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			confReq.Operation = logical.ReadOperation
+			confReq.Path = "creds/" + roleTokenName
+			resp, err = b.HandleRequest(context.Background(), &confReq)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if resp == nil {
+				t.Fatal("resp nil")
+			}
+			if resp.IsError() {
+				t.Fatalf("resp is error: %v", resp.Error())
+			}
+
+			// extract the secret, so we can query nomad directly
+			generatedSecret := resp.Secret
+			generatedSecret.TTL = 6 * time.Hour
+
+			var d struct {
+				Token    string `mapstructure:"secret_id"`
+				Accessor string `mapstructure:"accessor_id"`
+			}
+			if err := mapstructure.Decode(resp.Data, &d); err != nil {
+				t.Fatal(err)
+			}
+
+			// Build a client and verify that the credentials work
+			nomadapiConfig := nomadapi.DefaultConfig()
+			nomadapiConfig.Address = connData["address"].(string)
+			nomadapiConfig.SecretID = d.Token
+			client, err := nomadapi.NewClient(nomadapiConfig)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			// default query options for Nomad queries ... not sure if needed
+			qOpts := &nomadapi.QueryOptions{
+				Namespace: "default",
+			}
+
+			// connect to Nomad and verify the token name does not exceed the
+			// max_token_name_length
+			token, _, err := client.ACLTokens().Self(qOpts)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if len(token.Name) > expectedMaxTokenNameLength {
+				t.Fatalf("token name exceeds max length (%d): %s (%d)", expectedMaxTokenNameLength, token.Name, len(token.Name))
+			}
+		})
+	}
+}
diff --git a/path_config_access.go b/path_config_access.go
@@ -23,6 +23,11 @@ func pathConfigAccess(b *backend) *framework.Path {
 				Type:        framework.TypeString,
 				Description: "Token for API calls",
 			},
+
+			"max_token_name_length": &framework.FieldSchema{
+				Type:        framework.TypeInt,
+				Description: "Max length for name of generated Nomad tokens",
+			},
 		},
 
 		Callbacks: map[logical.Operation]framework.OperationFunc{
@@ -73,7 +78,8 @@ func (b *backend) pathConfigAccessRead(ctx context.Context, req *logical.Request
 
 	return &logical.Response{
 		Data: map[string]interface{}{
-			"address": conf.Address,
+			"address":               conf.Address,
+			"max_token_name_length": conf.MaxTokenNameLength,
 		},
 	}, nil
 }
@@ -96,6 +102,8 @@ func (b *backend) pathConfigAccessWrite(ctx context.Context, req *logical.Reques
 		conf.Token = token.(string)
 	}
 
+	conf.MaxTokenNameLength = data.Get("max_token_name_length").(int)
+
 	entry, err := logical.StorageEntryJSON("config/access", conf)
 	if err != nil {
 		return nil, err
@@ -115,6 +123,7 @@ func (b *backend) pathConfigAccessDelete(ctx context.Context, req *logical.Reque
 }
 
 type accessConfig struct {
-	Address string `json:"address"`
-	Token   string `json:"token"`
+	Address            string `json:"address"`
+	Token              string `json:"token"`
+	MaxTokenNameLength int    `json:"max_token_name_length"`
 }
diff --git a/path_creds_create.go b/path_creds_create.go
@@ -11,6 +11,10 @@ import (
 	"github.com/hashicorp/vault/logical/framework"
 )
 
+// maxTokenNameLength is the maximum length for the name of a Nomad access
+// token
+const maxTokenNameLength = 256
+
 func pathCredsCreate(b *backend) *framework.Path {
 	return &framework.Path{
 		Pattern: "creds/" + framework.GenericNameRegex("name"),
@@ -29,6 +33,12 @@ func pathCredsCreate(b *backend) *framework.Path {
 
 func (b *backend) pathTokenRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
 	name := d.Get("name").(string)
+	conf, _ := b.readConfigAccess(ctx, req.Storage)
+	// establish a default
+	tokenNameLength := maxTokenNameLength
+	if conf != nil && conf.MaxTokenNameLength > 0 {
+		tokenNameLength = conf.MaxTokenNameLength
+	}
 
 	role, err := b.Role(ctx, req.Storage, name)
 	if err != nil {
@@ -56,10 +66,11 @@ func (b *backend) pathTokenRead(ctx context.Context, req *logical.Request, d *fr
 	// Generate a name for the token
 	tokenName := fmt.Sprintf("vault-%s-%s-%d", name, req.DisplayName, time.Now().UnixNano())
 
-	// Handling nomad maximum token length
-	// https://github.com/hashicorp/nomad/blob/d9276e22b3b74674996fb548cdb6bc4c70d5b0e4/nomad/structs/structs.go#L115
-	if len(tokenName) > 64 {
-		tokenName = tokenName[0:63]
+	// Note: if the given role name is sufficiently long, the UnixNano() portion
+	// of the pseudo randomized token name is the part that gets trimmed off,
+	// weakening it's randomness.
+	if len(tokenName) > tokenNameLength {
+		tokenName = tokenName[:tokenNameLength]
 	}
 
 	// Create it