Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Aug 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions builtin/credential/approle/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand All @@ -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
}

Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
68 changes: 64 additions & 4 deletions builtin/credential/approle/path_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down Expand Up @@ -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())
}
}

Expand Down
28 changes: 27 additions & 1 deletion builtin/credential/approle/path_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions builtin/credential/approle/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down
6 changes: 6 additions & 0 deletions website/source/api/auth/approle/index.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down