From b9aa628197d4d3e37614bb6426deca359812910d Mon Sep 17 00:00:00 2001 From: Andrei Burd Date: Sun, 15 Apr 2018 15:07:10 +0300 Subject: [PATCH 01/10] Nomad: updating max token length to 256 --- builtin/logical/nomad/path_creds_create.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/logical/nomad/path_creds_create.go b/builtin/logical/nomad/path_creds_create.go index 43922b45b006..1b864e87f84d 100644 --- a/builtin/logical/nomad/path_creds_create.go +++ b/builtin/logical/nomad/path_creds_create.go @@ -58,8 +58,9 @@ func (b *backend) pathTokenRead(ctx context.Context, req *logical.Request, d *fr // 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] + // size increased in https://github.com/hashicorp/nomad/pull/3888 + if len(tokenName) > 256 { + tokenName = tokenName[0:255] } // Create it From b1ad2043912d0ac3d52a4f011b1214b19a8342b6 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Wed, 15 Aug 2018 11:14:04 -0500 Subject: [PATCH 02/10] Initial support for supporting custom max token name length for Nomad --- builtin/logical/nomad/backend.go | 7 + builtin/logical/nomad/backend_test.go | 197 +++++++++++++++++++- builtin/logical/nomad/path_config_access.go | 33 +++- builtin/logical/nomad/path_creds_create.go | 14 +- 4 files changed, 242 insertions(+), 9 deletions(-) diff --git a/builtin/logical/nomad/backend.go b/builtin/logical/nomad/backend.go index 9b4d5aec3f90..6e84188e1f2a 100644 --- a/builtin/logical/nomad/backend.go +++ b/builtin/logical/nomad/backend.go @@ -8,6 +8,11 @@ import ( "github.com/hashicorp/vault/logical/framework" ) +// maxTokenNameLength is the maximum length for the name of a Nomad access +// token +const maxTokenNameLength = 256 + +// 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 +21,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 +72,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/builtin/logical/nomad/backend_test.go b/builtin/logical/nomad/backend_test.go index e22ec90cfca5..97b5fa083f21 100644 --- a/builtin/logical/nomad/backend_test.go +++ b/builtin/logical/nomad/backend_test.go @@ -3,8 +3,10 @@ package nomad import ( "context" "fmt" + "math/rand" "os" "reflect" + "strconv" "testing" "time" @@ -14,6 +16,18 @@ import ( "github.com/ory/dockertest" ) +// randomWithPrefix is used to generate a unique name with a prefix, for +// randomizing names in acceptance tests +func randomWithPrefix(name string) string { + reseed() + return fmt.Sprintf("%s-%d", name, rand.New(rand.NewSource(time.Now().UnixNano())).Int()) +} + +// Seeds random with current timestamp +func reseed() { + rand.Seed(time.Now().UTC().UnixNano()) +} + func prepareTestContainer(t *testing.T) (cleanup func(), retAddress string, nomadToken string) { nomadToken = os.Getenv("NOMAD_TOKEN") @@ -29,8 +43,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 +156,8 @@ func TestBackend_config_access(t *testing.T) { } expected := map[string]interface{}{ - "address": connData["address"].(string), + "address": connData["address"].(string), + "max_token_length": maxTokenNameLength, } if !reflect.DeepEqual(expected, resp.Data) { t.Fatalf("bad: expected:%#v\nactual:%#v\n", expected, resp.Data) @@ -300,3 +315,179 @@ func TestBackend_CredsCreateEnvVar(t *testing.T) { t.Fatalf("resp is error: %v", resp.Error()) } } + +func TestBackend_max_token_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 + envLengthString string + }{ + { + title: "Default", + }, + { + title: "ConfigOverride", + tokenLength: 64, + }, + { + title: "ConfigOverride-LongName", + roleName: "testlongerrolenametoexceed64charsdddddddddddddddddddddddd", + tokenLength: 64, + }, + { + title: "ConfigOverride-LongName-notrim", + roleName: "testlongerrolenametoexceed64charsdddddddddddddddddddddddd", + }, + { + title: "ConfigOverride-LongName-envtrim", + roleName: "testlongerrolenametoexceed64charsdddddddddddddddddddddddd", + envLengthString: "16", + }, + } + + for _, tc := range testCases { + t.Run(tc.title, func(t *testing.T) { + // setup config/access + connData := map[string]interface{}{ + "address": connURL, + "token": connToken, + } + expected := map[string]interface{}{ + "address": connURL, + "max_token_length": maxTokenNameLength, + } + if tc.tokenLength != 0 { + connData["max_token_length"] = tc.tokenLength + expected["max_token_length"] = tc.tokenLength + } else { + // Set tc.tokenLength to default, but don't send to config/access unless + // it's been explicitly set + // Normally this is handled by the Default specified in the schema + tc.tokenLength = maxTokenNameLength + } + if tc.envLengthString != "" { + i, _ := strconv.Atoi(tc.envLengthString) + expected["max_token_length"] = i + } else { + // Set tc.tokenLength to default, but don't send to config/access unless + // it's been explicitly set + // Normally this is handled by the Default specified in the schema + tc.tokenLength = maxTokenNameLength + } + + if tc.envLengthString != "" { + os.Setenv("NOMAD_MAX_TOKEN_LENGTH", tc.envLengthString) + defer os.Unsetenv("NOMAD_MAX_TOKEN_LENGTH") + i, _ := strconv.Atoi(tc.envLengthString) + tc.tokenLength = i + expected["max_token_length"] = i + } + + 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 := 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_length + token, _, err := client.ACLTokens().Self(qOpts) + if err != nil { + t.Fatal(err) + } + + if len(token.Name) > tc.tokenLength { + t.Fatalf("token name exceeds max length (%d): %s (%d)", tc.tokenLength, token.Name, len(token.Name)) + } + }) + } +} diff --git a/builtin/logical/nomad/path_config_access.go b/builtin/logical/nomad/path_config_access.go index 0c7a2d8b2d45..064b58f5a336 100644 --- a/builtin/logical/nomad/path_config_access.go +++ b/builtin/logical/nomad/path_config_access.go @@ -2,6 +2,9 @@ package nomad import ( "context" + "log" + "os" + "strconv" "github.com/hashicorp/errwrap" "github.com/hashicorp/vault/logical" @@ -23,6 +26,14 @@ func pathConfigAccess(b *backend) *framework.Path { Type: framework.TypeString, Description: "Token for API calls", }, + + "max_token_length": &framework.FieldSchema{ + Type: framework.TypeInt, + Description: "Max length for generated Nomad tokens", + // Default length is 256 as of + // https://github.com/hashicorp/nomad/blob/21682427f3474f92cc589832efe72850a61c83a7/nomad/structs/structs.go#L116 + Default: maxTokenNameLength, + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ @@ -73,7 +84,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_length": conf.MaxTokenLength, }, }, nil } @@ -96,6 +108,20 @@ func (b *backend) pathConfigAccessWrite(ctx context.Context, req *logical.Reques conf.Token = token.(string) } + // max_token_length has default of 256 + conf.MaxTokenLength = data.Get("max_token_length").(int) + envMaxTokenLength := os.Getenv("NOMAD_MAX_TOKEN_LENGTH") + if envMaxTokenLength != "" { + // if we find NOMAD_MAX_max_token_length in the env and can parse it, override + // the default length + i, err := strconv.Atoi(envMaxTokenLength) + if err != nil { + log.Printf("[WARN] error parsing NOMAD_MAX_TOKEN_LENGTH, using default 256") + } else { + conf.MaxTokenLength = i + } + } + entry, err := logical.StorageEntryJSON("config/access", conf) if err != nil { return nil, err @@ -115,6 +141,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"` + MaxTokenLength int `json:"max_token_length"` } diff --git a/builtin/logical/nomad/path_creds_create.go b/builtin/logical/nomad/path_creds_create.go index 1b864e87f84d..bda0d052cb77 100644 --- a/builtin/logical/nomad/path_creds_create.go +++ b/builtin/logical/nomad/path_creds_create.go @@ -29,6 +29,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 { + tokenNameLength = conf.MaxTokenLength + } role, err := b.Role(ctx, req.Storage, name) if err != nil { @@ -58,9 +64,11 @@ func (b *backend) pathTokenRead(ctx context.Context, req *logical.Request, d *fr // Handling nomad maximum token length // https://github.com/hashicorp/nomad/blob/d9276e22b3b74674996fb548cdb6bc4c70d5b0e4/nomad/structs/structs.go#L115 - // size increased in https://github.com/hashicorp/nomad/pull/3888 - if len(tokenName) > 256 { - tokenName = tokenName[0:255] + // Note: if the given role name is suffeciently long, the UnixNano() portion + // of the pseudo randomized token name is the part that gets trimmed off, + // weaking it's randomness. + if len(tokenName) > tokenNameLength { + tokenName = tokenName[0:tokenNameLength] } // Create it From 3bd07ee42c10aee8cfd7e49001a5ea69546befe6 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Wed, 15 Aug 2018 16:57:49 -0500 Subject: [PATCH 03/10] simplify/correct tests --- builtin/logical/nomad/backend_test.go | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/builtin/logical/nomad/backend_test.go b/builtin/logical/nomad/backend_test.go index 97b5fa083f21..dd039b725b16 100644 --- a/builtin/logical/nomad/backend_test.go +++ b/builtin/logical/nomad/backend_test.go @@ -367,31 +367,21 @@ func TestBackend_max_token_length(t *testing.T) { "address": connURL, "max_token_length": maxTokenNameLength, } + + expectedTokenNameLength := maxTokenNameLength + if tc.tokenLength != 0 { connData["max_token_length"] = tc.tokenLength expected["max_token_length"] = tc.tokenLength - } else { - // Set tc.tokenLength to default, but don't send to config/access unless - // it's been explicitly set - // Normally this is handled by the Default specified in the schema - tc.tokenLength = maxTokenNameLength - } - if tc.envLengthString != "" { - i, _ := strconv.Atoi(tc.envLengthString) - expected["max_token_length"] = i - } else { - // Set tc.tokenLength to default, but don't send to config/access unless - // it's been explicitly set - // Normally this is handled by the Default specified in the schema - tc.tokenLength = maxTokenNameLength + expectedTokenNameLength = tc.tokenLength } if tc.envLengthString != "" { os.Setenv("NOMAD_MAX_TOKEN_LENGTH", tc.envLengthString) defer os.Unsetenv("NOMAD_MAX_TOKEN_LENGTH") i, _ := strconv.Atoi(tc.envLengthString) - tc.tokenLength = i expected["max_token_length"] = i + expectedTokenNameLength = i } confReq := logical.Request{ @@ -485,8 +475,8 @@ func TestBackend_max_token_length(t *testing.T) { t.Fatal(err) } - if len(token.Name) > tc.tokenLength { - t.Fatalf("token name exceeds max length (%d): %s (%d)", tc.tokenLength, token.Name, len(token.Name)) + if len(token.Name) > expectedTokenNameLength { + t.Fatalf("token name exceeds max length (%d): %s (%d)", expectedTokenNameLength, token.Name, len(token.Name)) } }) } From 024fc962cbfa3f81fe28bc8eef40b7b5c064d47d Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 16 Aug 2018 10:14:09 -0500 Subject: [PATCH 04/10] document nomad max_token_name_length --- website/source/api/secret/nomad/index.html.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/website/source/api/secret/nomad/index.html.md b/website/source/api/secret/nomad/index.html.md index fa3cd94e0caa..0be346773e58 100644 --- a/website/source/api/secret/nomad/index.html.md +++ b/website/source/api/secret/nomad/index.html.md @@ -37,12 +37,16 @@ Nomad tokens. This value can also be provided on individual calls with the NOMAD_TOKEN environment variable. +- `max_token_name_length` `(int: )` – Specifies the maximum length to use for +the name of the Nomad token generated with [Generate Credential](#generate-credential). Default `256`. + ### Sample Payload ```json { "address": "http://127.0.0.1:4646", - "token": "adha..." + "token": "adha...", + "max_token_name_length": 256 } ``` From 73e77f4cf9bc52b3bcf6b97444794a6ed209c12f Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 16 Aug 2018 10:26:25 -0500 Subject: [PATCH 05/10] removed support for max token length env var. Rename field for clarity --- builtin/logical/nomad/backend.go | 4 --- builtin/logical/nomad/path_config_access.go | 36 +++++++-------------- builtin/logical/nomad/path_creds_create.go | 8 ++--- 3 files changed, 15 insertions(+), 33 deletions(-) diff --git a/builtin/logical/nomad/backend.go b/builtin/logical/nomad/backend.go index 6e84188e1f2a..a8b4b99287b8 100644 --- a/builtin/logical/nomad/backend.go +++ b/builtin/logical/nomad/backend.go @@ -8,10 +8,6 @@ import ( "github.com/hashicorp/vault/logical/framework" ) -// maxTokenNameLength is the maximum length for the name of a Nomad access -// token -const maxTokenNameLength = 256 - // Factory returns a Nomad backend that satisfies the logical.Backend interface func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) { b := Backend() diff --git a/builtin/logical/nomad/path_config_access.go b/builtin/logical/nomad/path_config_access.go index 064b58f5a336..d954d706a0bd 100644 --- a/builtin/logical/nomad/path_config_access.go +++ b/builtin/logical/nomad/path_config_access.go @@ -2,9 +2,6 @@ package nomad import ( "context" - "log" - "os" - "strconv" "github.com/hashicorp/errwrap" "github.com/hashicorp/vault/logical" @@ -13,6 +10,10 @@ import ( const configAccessKey = "config/access" +// maxTokenNameLength is the maximum length for the name of a Nomad access +// token +const maxTokenNameLength = 256 + func pathConfigAccess(b *backend) *framework.Path { return &framework.Path{ Pattern: "config/access", @@ -27,12 +28,10 @@ func pathConfigAccess(b *backend) *framework.Path { Description: "Token for API calls", }, - "max_token_length": &framework.FieldSchema{ + "max_token_name_length": &framework.FieldSchema{ Type: framework.TypeInt, - Description: "Max length for generated Nomad tokens", - // Default length is 256 as of - // https://github.com/hashicorp/nomad/blob/21682427f3474f92cc589832efe72850a61c83a7/nomad/structs/structs.go#L116 - Default: maxTokenNameLength, + Description: "Max length for name of generated Nomad tokens", + Default: maxTokenNameLength, }, }, @@ -108,19 +107,8 @@ func (b *backend) pathConfigAccessWrite(ctx context.Context, req *logical.Reques conf.Token = token.(string) } - // max_token_length has default of 256 - conf.MaxTokenLength = data.Get("max_token_length").(int) - envMaxTokenLength := os.Getenv("NOMAD_MAX_TOKEN_LENGTH") - if envMaxTokenLength != "" { - // if we find NOMAD_MAX_max_token_length in the env and can parse it, override - // the default length - i, err := strconv.Atoi(envMaxTokenLength) - if err != nil { - log.Printf("[WARN] error parsing NOMAD_MAX_TOKEN_LENGTH, using default 256") - } else { - conf.MaxTokenLength = i - } - } + // max_token_name_length has default of 256 + conf.MaxTokenNameLength = data.Get("max_token_name_length").(int) entry, err := logical.StorageEntryJSON("config/access", conf) if err != nil { @@ -141,7 +129,7 @@ func (b *backend) pathConfigAccessDelete(ctx context.Context, req *logical.Reque } type accessConfig struct { - Address string `json:"address"` - Token string `json:"token"` - MaxTokenLength int `json:"max_token_length"` + Address string `json:"address"` + Token string `json:"token"` + MaxTokenNameLength int `json:"max_token_name_length"` } diff --git a/builtin/logical/nomad/path_creds_create.go b/builtin/logical/nomad/path_creds_create.go index bda0d052cb77..400cff27b908 100644 --- a/builtin/logical/nomad/path_creds_create.go +++ b/builtin/logical/nomad/path_creds_create.go @@ -32,8 +32,8 @@ func (b *backend) pathTokenRead(ctx context.Context, req *logical.Request, d *fr conf, _ := b.readConfigAccess(ctx, req.Storage) // establish a default tokenNameLength := maxTokenNameLength - if conf != nil { - tokenNameLength = conf.MaxTokenLength + if conf != nil && conf.MaxTokenNameLength > 0 { + tokenNameLength = conf.MaxTokenNameLength } role, err := b.Role(ctx, req.Storage, name) @@ -62,13 +62,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 // Note: if the given role name is suffeciently long, the UnixNano() portion // of the pseudo randomized token name is the part that gets trimmed off, // weaking it's randomness. if len(tokenName) > tokenNameLength { - tokenName = tokenName[0:tokenNameLength] + tokenName = tokenName[:tokenNameLength] } // Create it From aba959451eb1c192b2ec0d8e6663fbf06a7ae3a1 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 16 Aug 2018 10:31:23 -0500 Subject: [PATCH 06/10] cleanups after removing env var support --- builtin/logical/nomad/backend_test.go | 37 ++++++--------------- builtin/logical/nomad/path_config_access.go | 4 +-- 2 files changed, 13 insertions(+), 28 deletions(-) diff --git a/builtin/logical/nomad/backend_test.go b/builtin/logical/nomad/backend_test.go index dd039b725b16..fbfc0606a310 100644 --- a/builtin/logical/nomad/backend_test.go +++ b/builtin/logical/nomad/backend_test.go @@ -6,7 +6,6 @@ import ( "math/rand" "os" "reflect" - "strconv" "testing" "time" @@ -156,8 +155,8 @@ func TestBackend_config_access(t *testing.T) { } expected := map[string]interface{}{ - "address": connData["address"].(string), - "max_token_length": maxTokenNameLength, + "address": connData["address"].(string), + "max_token_name_length": maxTokenNameLength, } if !reflect.DeepEqual(expected, resp.Data) { t.Fatalf("bad: expected:%#v\nactual:%#v\n", expected, resp.Data) @@ -316,7 +315,7 @@ func TestBackend_CredsCreateEnvVar(t *testing.T) { } } -func TestBackend_max_token_length(t *testing.T) { +func TestBackend_max_token_name_length(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} b, err := Factory(context.Background(), config) @@ -328,10 +327,9 @@ func TestBackend_max_token_length(t *testing.T) { defer cleanup() testCases := []struct { - title string - roleName string - tokenLength int - envLengthString string + title string + roleName string + tokenLength int }{ { title: "Default", @@ -349,11 +347,6 @@ func TestBackend_max_token_length(t *testing.T) { title: "ConfigOverride-LongName-notrim", roleName: "testlongerrolenametoexceed64charsdddddddddddddddddddddddd", }, - { - title: "ConfigOverride-LongName-envtrim", - roleName: "testlongerrolenametoexceed64charsdddddddddddddddddddddddd", - envLengthString: "16", - }, } for _, tc := range testCases { @@ -364,26 +357,18 @@ func TestBackend_max_token_length(t *testing.T) { "token": connToken, } expected := map[string]interface{}{ - "address": connURL, - "max_token_length": maxTokenNameLength, + "address": connURL, + "max_token_name_length": maxTokenNameLength, } expectedTokenNameLength := maxTokenNameLength if tc.tokenLength != 0 { - connData["max_token_length"] = tc.tokenLength - expected["max_token_length"] = tc.tokenLength + connData["max_token_name_length"] = tc.tokenLength + expected["max_token_name_length"] = tc.tokenLength expectedTokenNameLength = tc.tokenLength } - if tc.envLengthString != "" { - os.Setenv("NOMAD_MAX_TOKEN_LENGTH", tc.envLengthString) - defer os.Unsetenv("NOMAD_MAX_TOKEN_LENGTH") - i, _ := strconv.Atoi(tc.envLengthString) - expected["max_token_length"] = i - expectedTokenNameLength = i - } - confReq := logical.Request{ Operation: logical.UpdateOperation, Path: "config/access", @@ -469,7 +454,7 @@ func TestBackend_max_token_length(t *testing.T) { } // connect to Nomad and verify the token name does not exceed the - // max_token_length + // max_token_name_length token, _, err := client.ACLTokens().Self(qOpts) if err != nil { t.Fatal(err) diff --git a/builtin/logical/nomad/path_config_access.go b/builtin/logical/nomad/path_config_access.go index d954d706a0bd..a2ce9738eaef 100644 --- a/builtin/logical/nomad/path_config_access.go +++ b/builtin/logical/nomad/path_config_access.go @@ -83,8 +83,8 @@ func (b *backend) pathConfigAccessRead(ctx context.Context, req *logical.Request return &logical.Response{ Data: map[string]interface{}{ - "address": conf.Address, - "max_token_length": conf.MaxTokenLength, + "address": conf.Address, + "max_token_name_length": conf.MaxTokenNameLength, }, }, nil } From ea46e620b03c941d0a04782c55312c8adc52d054 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 16 Aug 2018 10:42:27 -0500 Subject: [PATCH 07/10] move RandomWithPrefix to testhelpers --- builtin/logical/nomad/backend_test.go | 16 ++-------------- helper/testhelpers/testhelpers.go | 7 +++++++ 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/builtin/logical/nomad/backend_test.go b/builtin/logical/nomad/backend_test.go index fbfc0606a310..3b723b849857 100644 --- a/builtin/logical/nomad/backend_test.go +++ b/builtin/logical/nomad/backend_test.go @@ -3,30 +3,18 @@ package nomad import ( "context" "fmt" - "math/rand" "os" "reflect" "testing" "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" ) -// randomWithPrefix is used to generate a unique name with a prefix, for -// randomizing names in acceptance tests -func randomWithPrefix(name string) string { - reseed() - return fmt.Sprintf("%s-%d", name, rand.New(rand.NewSource(time.Now().UnixNano())).Int()) -} - -// Seeds random with current timestamp -func reseed() { - rand.Seed(time.Now().UTC().UnixNano()) -} - func prepareTestContainer(t *testing.T) (cleanup func(), retAddress string, nomadToken string) { nomadToken = os.Getenv("NOMAD_TOKEN") @@ -401,7 +389,7 @@ func TestBackend_max_token_name_length(t *testing.T) { if tc.roleName == "" { tc.roleName = "test" } - roleTokenName := randomWithPrefix(tc.roleName) + roleTokenName := testhelpers.RandomWithPrefix(tc.roleName) confReq.Path = "role/" + roleTokenName confReq.Operation = logical.UpdateOperation diff --git a/helper/testhelpers/testhelpers.go b/helper/testhelpers/testhelpers.go index b385b622baaa..08428ec39e11 100644 --- a/helper/testhelpers/testhelpers.go +++ b/helper/testhelpers/testhelpers.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "math/rand" + "time" uuid "github.com/hashicorp/go-uuid" "github.com/hashicorp/vault/helper/xor" @@ -80,3 +81,9 @@ func GenerateRootWithError(t testing.T, cluster *vault.TestCluster, drToken bool } return token, nil } + +// RandomWithPrefix is used to generate a unique name with a prefix, for +// randomizing names in acceptance tests +func RandomWithPrefix(name string) string { + return fmt.Sprintf("%s-%d", name, rand.New(rand.NewSource(time.Now().UnixNano())).Int()) +} From 3c3aa32967a8ee9cbddbd9d5c90ac12c4577b79c Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 16 Aug 2018 10:59:25 -0500 Subject: [PATCH 08/10] fix spelling --- builtin/logical/nomad/path_creds_create.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/logical/nomad/path_creds_create.go b/builtin/logical/nomad/path_creds_create.go index 400cff27b908..da741930dccf 100644 --- a/builtin/logical/nomad/path_creds_create.go +++ b/builtin/logical/nomad/path_creds_create.go @@ -62,9 +62,9 @@ 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()) - // Note: if the given role name is suffeciently long, the UnixNano() portion + // 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, - // weaking it's randomness. + // weakening it's randomness. if len(tokenName) > tokenNameLength { tokenName = tokenName[:tokenNameLength] } From 5e547008360dc4d235becd791f308518f6e756a2 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 16 Aug 2018 11:59:47 -0500 Subject: [PATCH 09/10] Remove default 256 value. Use zero as a sentinel value and ignore it --- builtin/logical/nomad/backend_test.go | 24 ++++++++++----------- builtin/logical/nomad/path_config_access.go | 6 ------ builtin/logical/nomad/path_creds_create.go | 4 ++++ 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/builtin/logical/nomad/backend_test.go b/builtin/logical/nomad/backend_test.go index 3b723b849857..4f4afe25e96d 100644 --- a/builtin/logical/nomad/backend_test.go +++ b/builtin/logical/nomad/backend_test.go @@ -144,7 +144,7 @@ func TestBackend_config_access(t *testing.T) { expected := map[string]interface{}{ "address": connData["address"].(string), - "max_token_name_length": maxTokenNameLength, + "max_token_name_length": 0, } if !reflect.DeepEqual(expected, resp.Data) { t.Fatalf("bad: expected:%#v\nactual:%#v\n", expected, resp.Data) @@ -332,8 +332,8 @@ func TestBackend_max_token_name_length(t *testing.T) { tokenLength: 64, }, { - title: "ConfigOverride-LongName-notrim", - roleName: "testlongerrolenametoexceed64charsdddddddddddddddddddddddd", + title: "Notrim", + roleName: "testlongersubrolenametoexceed64charsdddddddddddddddddddddddd", }, } @@ -341,20 +341,18 @@ func TestBackend_max_token_name_length(t *testing.T) { t.Run(tc.title, func(t *testing.T) { // setup config/access connData := map[string]interface{}{ - "address": connURL, - "token": connToken, + "address": connURL, + "token": connToken, + "max_token_name_length": tc.tokenLength, } expected := map[string]interface{}{ "address": connURL, - "max_token_name_length": maxTokenNameLength, + "max_token_name_length": tc.tokenLength, } - expectedTokenNameLength := maxTokenNameLength - + expectedMaxTokenNameLength := maxTokenNameLength if tc.tokenLength != 0 { - connData["max_token_name_length"] = tc.tokenLength - expected["max_token_name_length"] = tc.tokenLength - expectedTokenNameLength = tc.tokenLength + expectedMaxTokenNameLength = tc.tokenLength } confReq := logical.Request{ @@ -448,8 +446,8 @@ func TestBackend_max_token_name_length(t *testing.T) { t.Fatal(err) } - if len(token.Name) > expectedTokenNameLength { - t.Fatalf("token name exceeds max length (%d): %s (%d)", expectedTokenNameLength, token.Name, len(token.Name)) + if len(token.Name) > expectedMaxTokenNameLength { + t.Fatalf("token name exceeds max length (%d): %s (%d)", expectedMaxTokenNameLength, token.Name, len(token.Name)) } }) } diff --git a/builtin/logical/nomad/path_config_access.go b/builtin/logical/nomad/path_config_access.go index a2ce9738eaef..c1edd6bad133 100644 --- a/builtin/logical/nomad/path_config_access.go +++ b/builtin/logical/nomad/path_config_access.go @@ -10,10 +10,6 @@ import ( const configAccessKey = "config/access" -// maxTokenNameLength is the maximum length for the name of a Nomad access -// token -const maxTokenNameLength = 256 - func pathConfigAccess(b *backend) *framework.Path { return &framework.Path{ Pattern: "config/access", @@ -31,7 +27,6 @@ func pathConfigAccess(b *backend) *framework.Path { "max_token_name_length": &framework.FieldSchema{ Type: framework.TypeInt, Description: "Max length for name of generated Nomad tokens", - Default: maxTokenNameLength, }, }, @@ -107,7 +102,6 @@ func (b *backend) pathConfigAccessWrite(ctx context.Context, req *logical.Reques conf.Token = token.(string) } - // max_token_name_length has default of 256 conf.MaxTokenNameLength = data.Get("max_token_name_length").(int) entry, err := logical.StorageEntryJSON("config/access", conf) diff --git a/builtin/logical/nomad/path_creds_create.go b/builtin/logical/nomad/path_creds_create.go index da741930dccf..deca9e490c7e 100644 --- a/builtin/logical/nomad/path_creds_create.go +++ b/builtin/logical/nomad/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"), From 39ff353e89ec00658859a811364a28fe67d36058 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 16 Aug 2018 12:05:09 -0500 Subject: [PATCH 10/10] update docs --- website/source/api/secret/nomad/index.html.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/website/source/api/secret/nomad/index.html.md b/website/source/api/secret/nomad/index.html.md index 0be346773e58..d7e84c2b47a3 100644 --- a/website/source/api/secret/nomad/index.html.md +++ b/website/source/api/secret/nomad/index.html.md @@ -37,8 +37,12 @@ Nomad tokens. This value can also be provided on individual calls with the NOMAD_TOKEN environment variable. -- `max_token_name_length` `(int: )` – Specifies the maximum length to use for -the name of the Nomad token generated with [Generate Credential](#generate-credential). Default `256`. +- `max_token_name_length` `(int: )` – Specifies the maximum length to + use for the name of the Nomad token generated with [Generate + Credential](#generate-credential). If omitted, `0` is used and ignored, + defaulting to the max value allowed by the Nomad version. For Nomad versions + 0.8.3 and earlier, the default is `64`. For Nomad version 0.8.4 and later, the default is + `256`. ### Sample Payload