From 51b1b6d446f32f5f072cac0755f1907e112f5a76 Mon Sep 17 00:00:00 2001 From: davidadeleon <56207066+davidadeleon@users.noreply.github.com> Date: Fri, 16 Dec 2022 12:09:05 -0500 Subject: [PATCH] Approle: Fix CIDR validation for /32 masks on Token Bound CIDRs (#18145) * Fix CIDR validation for /32 masks * run go fmt * add changelog --- builtin/credential/approle/path_role_test.go | 59 ++++++++++++++++++++ builtin/credential/approle/validation.go | 9 +++ changelog/18145.txt | 3 + 3 files changed, 71 insertions(+) create mode 100644 changelog/18145.txt diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go index 06706dda5..885cb8e38 100644 --- a/builtin/credential/approle/path_role_test.go +++ b/builtin/credential/approle/path_role_test.go @@ -619,6 +619,65 @@ func TestAppRole_CIDRSubset(t *testing.T) { } } +func TestAppRole_TokenBoundCIDRSubset32Mask(t *testing.T) { + var resp *logical.Response + var err error + + b, storage := createBackendWithStorage(t) + + roleData := map[string]interface{}{ + "role_id": "role-id-123", + "policies": "a,b", + "token_bound_cidrs": "127.0.0.1/32", + } + + roleReq := &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/testrole1", + 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) + } + + secretIDData := map[string]interface{}{ + "token_bound_cidrs": "127.0.0.1/32", + } + secretIDReq := &logical.Request{ + Operation: logical.UpdateOperation, + Storage: storage, + Path: "role/testrole1/secret-id", + Data: secretIDData, + } + + resp, err = b.HandleRequest(context.Background(), secretIDReq) + if err != nil { + t.Fatalf("err: %v resp: %#v", err, resp) + } + + secretIDData = map[string]interface{}{ + "token_bound_cidrs": "127.0.0.1/24", + } + secretIDReq = &logical.Request{ + Operation: logical.UpdateOperation, + Storage: storage, + Path: "role/testrole1/secret-id", + Data: secretIDData, + } + + resp, err = b.HandleRequest(context.Background(), secretIDReq) + if resp != nil { + t.Fatalf("resp:%#v", resp) + } + + if err == nil { + t.Fatal("expected an error") + } +} + func TestAppRole_RoleConstraints(t *testing.T) { var resp *logical.Response var err error diff --git a/builtin/credential/approle/validation.go b/builtin/credential/approle/validation.go index 25f343675..9b3f87827 100644 --- a/builtin/credential/approle/validation.go +++ b/builtin/credential/approle/validation.go @@ -6,6 +6,7 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "strings" "time" uuid "github.com/hashicorp/go-uuid" @@ -77,6 +78,14 @@ func verifyCIDRRoleSecretIDSubset(secretIDCIDRs []string, roleBoundCIDRList []st // If there are no CIDR blocks on the role, then the subset // requirement would be satisfied if len(roleBoundCIDRList) != 0 { + // Address blocks with /32 mask do not get stored with the CIDR mask + // Check if there are any /32 addresses and append CIDR mask + for i, block := range roleBoundCIDRList { + if !strings.Contains(block, "/") { + roleBoundCIDRList[i] = fmt.Sprint(block, "/32") + } + } + subset, err := cidrutil.SubsetBlocks(roleBoundCIDRList, secretIDCIDRs) if !subset || err != nil { return fmt.Errorf( diff --git a/changelog/18145.txt b/changelog/18145.txt new file mode 100644 index 000000000..2e7172e57 --- /dev/null +++ b/changelog/18145.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth/approle: Fix `token_bound_cidrs` validation when using /32 blocks for role and secret ID +```