From 3c35a25d363d255571378dbd06228c850974b382 Mon Sep 17 00:00:00 2001 From: Marc Boudreau Date: Thu, 24 Jun 2021 13:03:41 -0400 Subject: [PATCH] Fix for Issue 11863 - Panic when creating/updating approle role with token_type (#11864) * initializing resp variable with aa *logical.Response before using it to add warning for default-service or default-batch token type. Also adding guard around code that sets resp to a new logical.Response further on in the function. * adding changelog entry * renaming changelog file to match PR number --- builtin/credential/approle/path_role.go | 6 +- builtin/credential/approle/path_role_test.go | 130 +++++++++++++++++++ changelog/11864.txt | 3 + 3 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 changelog/11864.txt diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go index 212ed7f32..d6e30e209 100644 --- a/builtin/credential/approle/path_role.go +++ b/builtin/credential/approle/path_role.go @@ -887,9 +887,11 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request switch tokenTypeRaw.(string) { case "default-service": data.Raw["token_type"] = "service" + resp = &logical.Response{} resp.AddWarning("default-service has no useful meaning; adjusting to service") case "default-batch": data.Raw["token_type"] = "batch" + resp = &logical.Response{} resp.AddWarning("default-batch has no useful meaning; adjusting to batch") } } @@ -976,7 +978,9 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request } if role.TokenMaxTTL > b.System().MaxLeaseTTL() { - resp = &logical.Response{} + if resp == nil { + resp = &logical.Response{} + } resp.AddWarning("token_max_ttl is greater than the backend mount's maximum TTL value; issued tokens' max TTL value will be truncated") } diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go index 215d15c57..de9bd6097 100644 --- a/builtin/credential/approle/path_role_test.go +++ b/builtin/credential/approle/path_role_test.go @@ -1820,6 +1820,136 @@ func TestAppRole_RoleWithTokenBoundCIDRsCRUD(t *testing.T) { } } +func TestAppRole_RoleWithTokenTypeCRUD(t *testing.T) { + var resp *logical.Response + var err error + b, storage := createBackendWithStorage(t) + + roleData := map[string]interface{}{ + "policies": "p,q,r,s", + "secret_id_num_uses": 10, + "secret_id_ttl": 300, + "token_ttl": 400, + "token_max_ttl": 500, + "token_num_uses": 600, + "token_type": "default-service", + } + roleReq := &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/role1", + Storage: storage, + Data: roleData, + } + + resp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + if 0 == len(resp.Warnings) { + t.Fatalf("bad:\nexpected warning in resp:%#v\n", resp.Warnings) + } + + roleReq.Operation = logical.ReadOperation + resp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + expected := map[string]interface{}{ + "bind_secret_id": true, + "policies": []string{"p", "q", "r", "s"}, + "secret_id_num_uses": 10, + "secret_id_ttl": 300, + "token_ttl": 400, + "token_max_ttl": 500, + "token_num_uses": 600, + "token_type": "service", + } + + var expectedStruct roleStorageEntry + err = mapstructure.Decode(expected, &expectedStruct) + if err != nil { + t.Fatal(err) + } + + var actualStruct roleStorageEntry + err = mapstructure.Decode(resp.Data, &actualStruct) + if err != nil { + t.Fatal(err) + } + + expectedStruct.RoleID = actualStruct.RoleID + if !reflect.DeepEqual(expectedStruct, actualStruct) { + t.Fatalf("bad:\nexpected:%#v\nactual:%#v\n", expectedStruct, actualStruct) + } + + roleData = map[string]interface{}{ + "role_id": "test_role_id", + "policies": "a,b,c,d", + "secret_id_num_uses": 100, + "secret_id_ttl": 3000, + "token_ttl": 4000, + "token_max_ttl": 5000, + "token_type": "default-service", + } + roleReq.Data = roleData + roleReq.Operation = logical.UpdateOperation + + resp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + if 0 == len(resp.Warnings) { + t.Fatalf("bad:\nexpected a warning in resp:%#v\n", resp.Warnings) + } + + roleReq.Operation = logical.ReadOperation + resp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + expected = map[string]interface{}{ + "policies": []string{"a", "b", "c", "d"}, + "secret_id_num_uses": 100, + "secret_id_ttl": 3000, + "token_ttl": 4000, + "token_max_ttl": 5000, + "token_type": "service", + } + err = mapstructure.Decode(expected, &expectedStruct) + if err != nil { + t.Fatal(err) + } + + err = mapstructure.Decode(resp.Data, &actualStruct) + if err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(expectedStruct, actualStruct) { + t.Fatalf("bad:\nexpected:%#v\nactual:%#v\n", expectedStruct, actualStruct) + } + + // Delete test for role + roleReq.Path = "role/role1" + roleReq.Operation = logical.DeleteOperation + resp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + roleReq.Operation = logical.ReadOperation + resp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + if resp != nil { + t.Fatalf("expected a nil response") + } +} + func createRole(t *testing.T, b *backend, s logical.Storage, roleName, policies string) { roleData := map[string]interface{}{ "policies": policies, diff --git a/changelog/11864.txt b/changelog/11864.txt new file mode 100644 index 000000000..0773ab020 --- /dev/null +++ b/changelog/11864.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth/approle: fixing dereference of nil pointer +``` \ No newline at end of file