From 6b512070dda78ffec06ed9eae0117fa6259ffd6e Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Thu, 7 Jun 2018 11:23:19 -0400 Subject: [PATCH 1/6] Fix panic due to metadata being nil --- builtin/credential/approle/path_login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/credential/approle/path_login.go b/builtin/credential/approle/path_login.go index fce24f540ba5..100e35a2be51 100644 --- a/builtin/credential/approle/path_login.go +++ b/builtin/credential/approle/path_login.go @@ -83,7 +83,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat return logical.ErrorResponse("invalid role ID"), nil } - var metadata map[string]string + metadata := make(map[string]string) if role.BindSecretID { secretID := strings.TrimSpace(data.Get("secret_id").(string)) if secretID == "" { From d223e038bfa34e72a63e13fbd49ab3148c9f1671 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Thu, 7 Jun 2018 14:12:43 -0400 Subject: [PATCH 2/6] added a nil check --- builtin/credential/approle/path_login.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/credential/approle/path_login.go b/builtin/credential/approle/path_login.go index 100e35a2be51..df2f39a4ccf4 100644 --- a/builtin/credential/approle/path_login.go +++ b/builtin/credential/approle/path_login.go @@ -262,7 +262,9 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat } // Always include the role name, for later filtering - metadata["role_name"] = role.name + if metadata != nil { + metadata["role_name"] = role.name + } auth := &logical.Auth{ NumUses: role.TokenNumUses, From 1b8c1d5149618c3ffdd408c41cea330f2c7653e6 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Thu, 7 Jun 2018 14:25:07 -0400 Subject: [PATCH 3/6] Added a test --- builtin/credential/approle/path_login_test.go | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/builtin/credential/approle/path_login_test.go b/builtin/credential/approle/path_login_test.go index b5f5cbf6c278..ec23be6d1a4a 100644 --- a/builtin/credential/approle/path_login_test.go +++ b/builtin/credential/approle/path_login_test.go @@ -8,6 +8,58 @@ import ( "github.com/hashicorp/vault/logical" ) +func TestAppRole_BoundCIDRLogin(t *testing.T) { + var resp *logical.Response + var err error + b, s := createBackendWithStorage(t) + + // Create a role with secret ID binding disabled and only bound cidr list + // enabled + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole", + Operation: logical.CreateOperation, + Data: map[string]interface{}{ + "bind_secret_id": false, + "bound_cidr_list": []string{"127.0.0.1/8"}, + }, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + // Read the role ID + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole/role-id", + Operation: logical.ReadOperation, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + roleID := resp.Data["role_id"] + + // Fill in the connection information and login with just the role ID + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + }, + Storage: s, + Connection: &logical.Connection{RemoteAddr: "127.0.0.1"}, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + // Login should pass + if resp.Auth == nil { + t.Fatalf("expected login to succeed") + } +} + func TestAppRole_RoleLogin(t *testing.T) { var resp *logical.Response var err error From 070e7967b58df88b1338c1620e93c210e6ef395b Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Mon, 11 Jun 2018 10:37:22 -0400 Subject: [PATCH 4/6] ensure metadata is never nil --- builtin/credential/approle/path_login.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/builtin/credential/approle/path_login.go b/builtin/credential/approle/path_login.go index df2f39a4ccf4..cec371e7a63b 100644 --- a/builtin/credential/approle/path_login.go +++ b/builtin/credential/approle/path_login.go @@ -261,11 +261,15 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat } } - // Always include the role name, for later filtering - if metadata != nil { - metadata["role_name"] = role.name + // For some reason, if metadata was set to nil while processing secret ID + // binding, ensure that it is initialized again to avoid a panic. + if metadata == nil { + metadata = make(map[string]string) } + // Always include the role name, for later filtering + metadata["role_name"] = role.name + auth := &logical.Auth{ NumUses: role.TokenNumUses, Period: role.Period, From 5ee44f26c99cdb3ceefee9930bd627b707fc61f4 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Mon, 11 Jun 2018 11:06:21 -0400 Subject: [PATCH 5/6] Remove unnecessary allocation --- builtin/credential/approle/path_login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/credential/approle/path_login.go b/builtin/credential/approle/path_login.go index cec371e7a63b..5dc03759852e 100644 --- a/builtin/credential/approle/path_login.go +++ b/builtin/credential/approle/path_login.go @@ -83,7 +83,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat return logical.ErrorResponse("invalid role ID"), nil } - metadata := make(map[string]string) + var metadata map[string]string if role.BindSecretID { secretID := strings.TrimSpace(data.Get("secret_id").(string)) if secretID == "" { From 401dab1605ee467ea398df43b5cf496d353ae460 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Mon, 11 Jun 2018 11:20:43 -0400 Subject: [PATCH 6/6] revert back to early initialization --- builtin/credential/approle/path_login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/credential/approle/path_login.go b/builtin/credential/approle/path_login.go index 5dc03759852e..cec371e7a63b 100644 --- a/builtin/credential/approle/path_login.go +++ b/builtin/credential/approle/path_login.go @@ -83,7 +83,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat return logical.ErrorResponse("invalid role ID"), nil } - var metadata map[string]string + metadata := make(map[string]string) if role.BindSecretID { secretID := strings.TrimSpace(data.Get("secret_id").(string)) if secretID == "" {