From 3ab5de08a7562e45fc575908d709fa85be1102e3 Mon Sep 17 00:00:00 2001 From: Clint Date: Thu, 16 Aug 2018 14:48:23 -0500 Subject: [PATCH] [WIP] Support custom max Nomad token name length [supersedes https://github.com/hashicorp/vault/pull/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 --- nomad/backend.go | 3 + nomad/backend_test.go | 158 +++++++++++++++++++++++++++++++++++- nomad/path_config_access.go | 15 +++- nomad/path_creds_create.go | 19 ++++- 4 files changed, 185 insertions(+), 10 deletions(-) diff --git a/nomad/backend.go b/nomad/backend.go index 9b4d5ae..a8b4b99 100644 --- a/nomad/backend.go +++ b/nomad/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/nomad/backend_test.go b/nomad/backend_test.go index e22ec90..4f4afe2 100644 --- a/nomad/backend_test.go +++ b/nomad/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/nomad/path_config_access.go b/nomad/path_config_access.go index 0c7a2d8..c1edd6b 100644 --- a/nomad/path_config_access.go +++ b/nomad/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/nomad/path_creds_create.go b/nomad/path_creds_create.go index 43922b4..deca9e4 100644 --- a/nomad/path_creds_create.go +++ b/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"), @@ -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