diff --git a/builtin/credential/approle/path_login.go b/builtin/credential/approle/path_login.go index 33b5c218b2a7..24a2a9fc1c0d 100644 --- a/builtin/credential/approle/path_login.go +++ b/builtin/credential/approle/path_login.go @@ -85,6 +85,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat } metadata := make(map[string]string) + var entry *secretIDStorageEntry if role.BindSecretID { secretID := strings.TrimSpace(data.Get("secret_id").(string)) if secretID == "" { @@ -111,10 +112,11 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat unlockFunc() }() - entry, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) + entry, err = b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) if err != nil { return nil, err - } else if entry == nil { + } + if entry == nil { return logical.ErrorResponse("invalid secret id"), nil } @@ -130,7 +132,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat secretIDLock.Lock() unlockFunc = secretIDLock.Unlock - entry, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) + entry, err = b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC) if err != nil { return nil, err } @@ -262,7 +264,14 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat } // Parse the CIDRs we should be binding the token to. - tokenBoundCIDRs, err := parseutil.ParseAddrs(role.TokenBoundCIDRs) + var tokenBoundCIDRStrings []string + if entry != nil { + tokenBoundCIDRStrings = entry.TokenBoundCIDRs + } + if len(tokenBoundCIDRStrings) == 0 { + tokenBoundCIDRStrings = role.TokenBoundCIDRs + } + tokenBoundCIDRs, err := parseutil.ParseAddrs(tokenBoundCIDRStrings) if err != nil { return logical.ErrorResponse(err.Error()), nil } diff --git a/builtin/credential/approle/path_login_test.go b/builtin/credential/approle/path_login_test.go index ec23be6d1a4a..b1c7af715d22 100644 --- a/builtin/credential/approle/path_login_test.go +++ b/builtin/credential/approle/path_login_test.go @@ -19,8 +19,9 @@ func TestAppRole_BoundCIDRLogin(t *testing.T) { Path: "role/testrole", Operation: logical.CreateOperation, Data: map[string]interface{}{ - "bind_secret_id": false, - "bound_cidr_list": []string{"127.0.0.1/8"}, + "bind_secret_id": false, + "bound_cidr_list": []string{"127.0.0.1/8"}, + "token_bound_cidrs": []string{"10.0.0.0/8"}, }, Storage: s, }) @@ -53,10 +54,69 @@ func TestAppRole_BoundCIDRLogin(t *testing.T) { if err != nil || (resp != nil && resp.IsError()) { t.Fatalf("err:%v resp:%#v", err, resp) } + if resp.Auth == nil { + t.Fatal("expected login to succeed") + } + if len(resp.Auth.BoundCIDRs) != 1 { + t.Fatal("bad token bound cidrs") + } + if resp.Auth.BoundCIDRs[0].String() != "10.0.0.0/8" { + t.Fatalf("bad: %s", resp.Auth.BoundCIDRs[0].String()) + } + + // Override with a secret-id value, verify it doesn't pass + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "role/testrole", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "bind_secret_id": true, + }, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + roleSecretIDReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "role/testrole/secret-id", + Storage: s, + Data: map[string]interface{}{ + "token_bound_cidrs": []string{"11.0.0.0/24"}, + }, + } + resp, err = b.HandleRequest(context.Background(), roleSecretIDReq) + if err == nil { + t.Fatal("expected error due to mismatching subnet relationship") + } + roleSecretIDReq.Data["token_bound_cidrs"] = "10.0.0.0/24" + resp, err = b.HandleRequest(context.Background(), roleSecretIDReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + secretID := resp.Data["secret_id"] - // Login should pass + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Path: "login", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "role_id": roleID, + "secret_id": secretID, + }, + 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) + } if resp.Auth == nil { - t.Fatalf("expected login to succeed") + t.Fatal("expected login to succeed") + } + if len(resp.Auth.BoundCIDRs) != 1 { + t.Fatal("bad token bound cidrs") + } + if resp.Auth.BoundCIDRs[0].String() != "10.0.0.0/24" { + t.Fatalf("bad: %s", resp.Auth.BoundCIDRs[0].String()) } } diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go index 057bae4cb48f..a21261ceb781 100644 --- a/builtin/credential/approle/path_role.go +++ b/builtin/credential/approle/path_role.go @@ -491,6 +491,11 @@ specific set of IP addresses. If 'bound_cidr_list' is set on the role, then the list of CIDR blocks listed here should be a subset of the CIDR blocks listed on the role.`, }, + "token_bound_cidrs": &framework.FieldSchema{ + Type: framework.TypeCommaStringSlice, + Description: `Comma separated string or list of CIDR blocks. If set, specifies the blocks of +IP addresses which can use the returned token. Should be a subset of the token CIDR blocks listed on the role, if any.`, + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ logical.UpdateOperation: b.pathRoleSecretIDUpdate, @@ -596,6 +601,11 @@ specific set of IP addresses. If 'bound_cidr_list' is set on the role, then the list of CIDR blocks listed here should be a subset of the CIDR blocks listed on the role.`, }, + "token_bound_cidrs": &framework.FieldSchema{ + Type: framework.TypeCommaStringSlice, + Description: `Comma separated string or list of CIDR blocks. If set, specifies the blocks of +IP addresses which can use the returned token. Should be a subset of the token CIDR blocks listed on the role, if any.`, + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ logical.UpdateOperation: b.pathRoleCustomSecretIDUpdate, @@ -1222,6 +1232,7 @@ func (entry *secretIDStorageEntry) ToResponseData() map[string]interface{} { "last_updated_time": entry.LastUpdatedTime, "metadata": entry.Metadata, "cidr_list": entry.CIDRList, + "token_bound_cidrs": entry.TokenBoundCIDRs, } } @@ -2278,17 +2289,32 @@ func (b *backend) handleRoleSecretIDCommon(ctx context.Context, req *logical.Req return logical.ErrorResponse("failed to validate CIDR blocks"), nil } } - // Ensure that the CIDRs on the secret ID are a subset of that of role's if err := verifyCIDRRoleSecretIDSubset(secretIDCIDRs, role.SecretIDBoundCIDRs); err != nil { return nil, err } + secretIDTokenCIDRs := data.Get("token_bound_cidrs").([]string) + if len(secretIDTokenCIDRs) != 0 { + valid, err := cidrutil.ValidateCIDRListSlice(secretIDTokenCIDRs) + if err != nil { + return nil, errwrap.Wrapf("failed to validate token CIDR blocks: {{err}}", err) + } + if !valid { + return logical.ErrorResponse("failed to validate token CIDR blocks"), nil + } + } + // Ensure that the token CIDRs on the secret ID are a subset of that of role's + if err := verifyCIDRRoleSecretIDSubset(secretIDTokenCIDRs, role.TokenBoundCIDRs); err != nil { + return nil, err + } + secretIDStorage := &secretIDStorageEntry{ SecretIDNumUses: role.SecretIDNumUses, SecretIDTTL: role.SecretIDTTL, Metadata: make(map[string]string), CIDRList: secretIDCIDRs, + TokenBoundCIDRs: secretIDTokenCIDRs, } if err = strutil.ParseArbitraryKeyValues(data.Get("metadata").(string), secretIDStorage.Metadata, ","); err != nil { diff --git a/builtin/credential/approle/validation.go b/builtin/credential/approle/validation.go index c7cadf6bc4d3..99e9bddc397f 100644 --- a/builtin/credential/approle/validation.go +++ b/builtin/credential/approle/validation.go @@ -52,6 +52,10 @@ type secretIDStorageEntry struct { // restrictions on the usage of SecretID CIDRList []string `json:"cidr_list" mapstructure:"cidr_list"` + // TokenBoundCIDRs is a set of CIDR blocks that impose source address + // restrictions on the usage of the token generated by this SecretID + TokenBoundCIDRs []string `json:"token_cidr_list" mapstructure:"token_bound_cidrs"` + // This is a deprecated field SecretIDNumUsesDeprecated int `json:"SecretIDNumUses" mapstructure:"SecretIDNumUses"` } diff --git a/website/source/api/auth/approle/index.html.md b/website/source/api/auth/approle/index.html.md index d20b812bdc41..95c6809ca228 100644 --- a/website/source/api/auth/approle/index.html.md +++ b/website/source/api/auth/approle/index.html.md @@ -296,6 +296,9 @@ itself, and also to delete the SecretID from the AppRole. enforcing secret IDs to be used from specific set of IP addresses. If `bound_cidr_list` is set on the role, then the list of CIDR blocks listed here should be a subset of the CIDR blocks listed on the role. +- `token_bound_cidrs` `(array: [])` - Comma-separated string or list of CIDR + blocks; if set, specifies blocks of IP addresses which can use the auth tokens + generated by this SecretID. Overrides any role-set value but must be a subset. ### Sample Payload @@ -521,6 +524,9 @@ Assigns a "custom" SecretID against an existing AppRole. This is used in the enforcing secret IDs to be used from specific set of IP addresses. If `bound_cidr_list` is set on the role, then the list of CIDR blocks listed here should be a subset of the CIDR blocks listed on the role. +- `token_bound_cidrs` `(array: [])` - Comma-separated string or list of CIDR + blocks; if set, specifies blocks of IP addresses which can use the auth tokens + generated by this SecretID. Overrides any role-set value but must be a subset. ### Sample Payload