Add the ability to specify token CIDR restrictions on secret IDs.

Fixes #5034

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
 	}

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())
 	}
 }
 

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 {

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"`
 }

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