Store the CIDR list in the secret ID storage entry.

Use the stored information to validate the source address and credential issue time.
Correct the logic used to verify BoundCIDRList on the role.
Reverify the subset requirements between secret ID and role during credential issue time.
This commit is contained in:
vishalnayak 2016-09-21 19:41:08 -04:00
parent 578b82acf5
commit aaadd4ad97
7 changed files with 121 additions and 66 deletions

View File

@ -33,7 +33,11 @@ func pathLogin(b *backend) *framework.Path {
// Returns the Auth object indicating the authentication and authorization information
// if the credentials provided are validated by the backend.
func (b *backend) pathLoginUpdate(req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
role, roleName, metadata, err := b.validateCredentials(req, data)
if req.Connection == nil || req.Connection.RemoteAddr == "" {
return nil, fmt.Errorf("failed to get connection information")
}
role, roleName, metadata, err := b.validateCredentials(req, data, req.Connection.RemoteAddr)
if err != nil || role == nil {
return logical.ErrorResponse(fmt.Sprintf("failed to validate SecretID: %s", err)), nil
}

View File

@ -43,6 +43,9 @@ func TestAppRole_RoleLogin(t *testing.T) {
Path: "login",
Storage: storage,
Data: loginData,
Connection: &logical.Connection{
RemoteAddr: "127.0.0.1",
},
}
resp, err = b.HandleRequest(loginReq)
if err != nil || (resp != nil && resp.IsError()) {

View File

@ -1738,39 +1738,17 @@ func (b *backend) handleRoleSecretIDCommon(req *logical.Request, data *framework
// Parse the CIDR blocks into a slice
secretIDCIDRs := strutil.ParseDedupAndSortStrings(cidrList, ",")
if len(secretIDCIDRs) != 0 {
// If role has bound_cidr_list set, check if each CIDR block is
// a subset of at least one CIDR block registered under the
// role. If bound_cidr_list is not set on the role, then it
// means that the role allows any IP address, implying that the
// CIDR blocks on the secret ID are a subset of that of role's.
roleCIDRs := strutil.ParseDedupAndSortStrings(role.BoundCIDRList, ",")
if len(roleCIDRs) != 0 {
subset, err := cidrutil.SubsetBlocks(roleCIDRs, secretIDCIDRs)
if err != nil {
return nil, fmt.Errorf("failed to verify subset relationship between CIDR blocks on the role %q and CIDR blocks on the secret ID %q: %v", roleCIDRs, secretIDCIDRs, err)
}
if !subset {
return logical.ErrorResponse(fmt.Sprintf("CIDR blocks on the secret ID %q should be a subset of CIDR blocks on the role %q", secretIDCIDRs, roleCIDRs)), nil
}
}
if req.Connection == nil || req.Connection.RemoteAddr == "" {
return nil, fmt.Errorf("failed to get connection information")
}
belongs, err := cidrutil.IPBelongsToCIDRBlocksSlice(req.Connection.RemoteAddr, secretIDCIDRs)
if err != nil {
return nil, fmt.Errorf("failed to check if IP belonged to configured CIDR blocks on the secret ID")
}
if !belongs {
return logical.ErrorResponse(fmt.Sprintf("unauthorized source address")), nil
}
// Ensure that the CIDRs on the secret ID are a subset of that of role's
if err := verifyCIDRRoleSecretIDSubset(secretIDCIDRs, role.BoundCIDRList); err != nil {
return nil, err
}
secretIDStorage := &secretIDStorageEntry{
SecretIDNumUses: role.SecretIDNumUses,
SecretIDTTL: role.SecretIDTTL,
Metadata: make(map[string]string),
CIDRList: secretIDCIDRs,
}
if err = strutil.ParseArbitraryKeyValues(data.Get("metadata").(string), secretIDStorage.Metadata, ","); err != nil {

View File

@ -45,11 +45,11 @@ func TestAppRole_CIDRSubset(t *testing.T) {
}
resp, err = b.HandleRequest(secretIDReq)
if err != nil {
t.Fatal(err)
if resp != nil || resp.IsError() {
t.Fatalf("resp:%#v", resp)
}
if resp == nil || !resp.IsError() {
t.Fatalf("resp:%#v", err, resp)
if err == nil {
t.Fatal("expected an error")
}
roleData["bound_cidr_list"] = "192.168.27.29/16,172.245.30.40/24,10.20.30.40/30"

View File

@ -5,12 +5,13 @@ import (
"crypto/sha256"
"encoding/hex"
"fmt"
"net"
"strings"
"sync"
"time"
"github.com/hashicorp/go-uuid"
"github.com/hashicorp/vault/helper/cidrutil"
"github.com/hashicorp/vault/helper/strutil"
"github.com/hashicorp/vault/logical"
"github.com/hashicorp/vault/logical/framework"
)
@ -26,32 +27,38 @@ type secretIDStorageEntry struct {
// SecretIDs during login.
SecretIDAccessor string `json:"secret_id_accessor" structs:"secret_id_accessor" mapstructure:"secret_id_accessor"`
// Number of times this SecretID can be used to perform the login operation
SecretIDNumUses int `json:"secret_id_num_uses" structs:"secret_id_num_uses" mapstructure:"secret_id_num_uses"`
// Number of times this SecretID can be used to perform the login
// operation
SecretIDNumUses int `json:"secret_id_num_uses"
structs:"secret_id_num_uses" mapstructure:"secret_id_num_uses"`
// Duration after which this SecretID should expire. This is
// croleed by the backend mount's max TTL value.
// Duration after which this SecretID should expire. This is croleed by
// the backend mount's max TTL value.
SecretIDTTL time.Duration `json:"secret_id_ttl" structs:"secret_id_ttl" mapstructure:"secret_id_ttl"`
// The time when the SecretID was created
CreationTime time.Time `json:"creation_time" structs:"creation_time" mapstructure:"creation_time"`
// The time when the SecretID becomes eligible for tidy
// operation. Tidying is performed by the PeriodicFunc of the
// backend which is 1 minute apart.
// The time when the SecretID becomes eligible for tidy operation.
// Tidying is performed by the PeriodicFunc of the backend which is 1
// minute apart.
ExpirationTime time.Time `json:"expiration_time" structs:"expiration_time" mapstructure:"expiration_time"`
// The time representing the last time this storage entry was modified
LastUpdatedTime time.Time `json:"last_updated_time" structs:"last_updated_time" mapstructure:"last_updated_time"`
// Metadata that belongs to the SecretID.
// Metadata that belongs to the SecretID
Metadata map[string]string `json:"metadata" structs:"metadata" mapstructure:"metadata"`
// CIDRList is a set of CIDR blocks that impose source address
// restrictions on the usage of SecretID
CIDRList []string `json:"cidr_list" structs:"cidr_list" mapstructure:"cidr_list"`
}
// Represents the payload of the storage entry of the accessor that maps to a unique
// SecretID. Note that SecretIDs should never be stored in plaintext anywhere in the
// backend. SecretIDHMAC will be used as an index to fetch the properties of the
// SecretID and to delete the SecretID.
// Represents the payload of the storage entry of the accessor that maps to a
// unique SecretID. Note that SecretIDs should never be stored in plaintext
// anywhere in the backend. SecretIDHMAC will be used as an index to fetch the
// properties of the SecretID and to delete the SecretID.
type secretIDAccessorStorageEntry struct {
// Hash of the SecretID which can be used to find the storage index at which
// properties of SecretID is stored.
@ -81,7 +88,7 @@ func (b *backend) validateRoleID(s logical.Storage, roleID string) (*roleStorage
}
// Validates the supplied RoleID and SecretID
func (b *backend) validateCredentials(req *logical.Request, data *framework.FieldData) (*roleStorageEntry, string, map[string]string, error) {
func (b *backend) validateCredentials(req *logical.Request, data *framework.FieldData, sourceIP string) (*roleStorageEntry, string, map[string]string, error) {
var metadata map[string]string
// RoleID must be supplied during every login
roleID := strings.TrimSpace(data.Get("role_id").(string))
@ -114,7 +121,7 @@ func (b *backend) validateCredentials(req *logical.Request, data *framework.Fiel
// Check if the SecretID supplied is valid. If use limit was specified
// on the SecretID, it will be decremented in this call.
var valid bool
valid, metadata, err = b.validateBindSecretID(req.Storage, roleName, secretID, role.HMACKey)
valid, metadata, err = b.validateBindSecretID(req.Storage, roleName, secretID, role.HMACKey, role.BoundCIDRList, sourceIP)
if err != nil {
return nil, "", metadata, err
}
@ -125,20 +132,12 @@ func (b *backend) validateCredentials(req *logical.Request, data *framework.Fiel
if role.BoundCIDRList != "" {
// If 'bound_cidr_list' was set, verify the CIDR restrictions
cidrBlocks := strings.Split(role.BoundCIDRList, ",")
for _, block := range cidrBlocks {
_, cidr, err := net.ParseCIDR(block)
if err != nil {
return nil, "", metadata, fmt.Errorf("invalid cidr: %s", err)
}
var addr string
if req.Connection != nil {
addr = req.Connection.RemoteAddr
}
if addr == "" || !cidr.Contains(net.ParseIP(addr)) {
return nil, "", metadata, fmt.Errorf("unauthorized source address")
}
belongs, err := cidrutil.IPBelongsToCIDRBlocksString(sourceIP, role.BoundCIDRList, ",")
if err != nil {
return nil, "", metadata, fmt.Errorf("failed to verify the CIDR restrictions set on the role: %v", err)
}
if !belongs {
return nil, "", metadata, fmt.Errorf("source address unauthorized through CIDR restrictions on the role")
}
}
@ -146,7 +145,8 @@ func (b *backend) validateCredentials(req *logical.Request, data *framework.Fiel
}
// validateBindSecretID is used to determine if the given SecretID is a valid one.
func (b *backend) validateBindSecretID(s logical.Storage, roleName, secretID, hmacKey string) (bool, map[string]string, error) {
func (b *backend) validateBindSecretID(s logical.Storage, roleName, secretID,
hmacKey, roleBoundCIDRList, sourceIP string) (bool, map[string]string, error) {
secretIDHMAC, err := createHMAC(hmacKey, secretID)
if err != nil {
return false, nil, fmt.Errorf("failed to create HMAC of secret_id: %s", err)
@ -181,6 +181,21 @@ func (b *backend) validateBindSecretID(s logical.Storage, roleName, secretID, hm
// in which case, the SecretID will remain to be valid as long as it is not
// expired.
if result.SecretIDNumUses == 0 {
// Ensure that the CIDRs on the secret ID are still a subset of that of
// role's
if err := verifyCIDRRoleSecretIDSubset(result.CIDRList,
roleBoundCIDRList); err != nil {
return false, nil, err
}
// If CIDR restrictions are present on the secret ID, check if the
// source IP complies to it
if len(result.CIDRList) != 0 {
if belongs, err := cidrutil.IPBelongsToCIDRBlocksSlice(sourceIP, result.CIDRList); !belongs || err != nil {
return false, nil, fmt.Errorf("source address unauthorized through CIDR restrictions on the secret ID: %v", err)
}
}
lock.RUnlock()
return true, result.Metadata, nil
}
@ -225,9 +240,44 @@ func (b *backend) validateBindSecretID(s logical.Storage, roleName, secretID, hm
}
}
// Ensure that the CIDRs on the secret ID are still a subset of that of
// role's
if err := verifyCIDRRoleSecretIDSubset(result.CIDRList,
roleBoundCIDRList); err != nil {
return false, nil, err
}
// If CIDR restrictions are present on the secret ID, check if the
// source IP complies to it
if len(result.CIDRList) != 0 {
if belongs, err := cidrutil.IPBelongsToCIDRBlocksSlice(sourceIP, result.CIDRList); !belongs || err != nil {
return false, nil, fmt.Errorf("source address unauthorized through CIDR restrictions on the secret ID: %v", err)
}
}
return true, result.Metadata, nil
}
// verifyCIDRRoleSecretIDSubset checks if the CIDR blocks set on the secret ID
// are a subset of CIDR blocks set on the role
func verifyCIDRRoleSecretIDSubset(secretIDCIDRs []string, roleBoundCIDRList string) error {
if len(secretIDCIDRs) != 0 {
// Parse the CIDRs on role as a slice
roleCIDRs := strutil.ParseDedupAndSortStrings(roleBoundCIDRList, ",")
// If there are no CIDR blocks on the role, then the subset
// requirement would be satisfied
if len(roleCIDRs) != 0 {
subset, err := cidrutil.SubsetBlocks(roleCIDRs, secretIDCIDRs)
if !subset || err != nil {
return fmt.Errorf("failed to verify subset relationship between CIDR blocks on the role %q and CIDR blocks on the secret ID %q: %v", roleCIDRs, secretIDCIDRs, err)
}
}
}
return nil
}
// Creates a SHA256 HMAC of the given 'value' using the given 'key' and returns
// a hex encoded string.
func createHMAC(key, value string) (string, error) {

View File

@ -124,23 +124,23 @@ func Subset(cidr1, cidr2 string) (bool, error) {
return false, fmt.Errorf("missing CIDR that needs to be checked")
}
_, net1, err := net.ParseCIDR(cidr1)
ip1, net1, err := net.ParseCIDR(cidr1)
if err != nil {
return false, fmt.Errorf("failed to parse the CIDR to be checked against: %q", err)
}
maskLen1, _ := net1.Mask.Size()
if maskLen1 == 0 {
if ip1.To4().String() != "0.0.0.0" && maskLen1 == 0 {
return false, fmt.Errorf("CIDR to be checked against is not in its canonical form")
}
_, net2, err := net.ParseCIDR(cidr2)
ip2, net2, err := net.ParseCIDR(cidr2)
if err != nil {
return false, fmt.Errorf("failed to parse the CIDR that needs to be checked: %q", err)
}
maskLen2, _ := net2.Mask.Size()
if maskLen2 == 0 {
if ip2.To4().String() != "0.0.0.0" && maskLen2 == 0 {
return false, fmt.Errorf("CIDR that needs to be checked is not in its canonical form")
}
@ -179,7 +179,7 @@ func SubsetBlocks(cidrBlocks1, cidrBlocks2 []string) (bool, error) {
isSubset := false
for _, cidrBlock1 := range cidrBlocks1 {
subset, err := Subset(cidrBlock1, cidrBlock2)
if !subset && err != nil {
if err != nil {
return false, err
}
// If CIDR is a subset of any of the CIDR block, its

View File

@ -479,6 +479,16 @@ $ curl -XPOST "http://127.0.0.1:8200/v1/auth/approle/login" -d '{"role_id":"50be
_in plaintext_.
</li>
</ul>
<ul>
<li>
<span class="param">cidr_list</span>
<span class="param-flags">optional</span>
Comma separated list of CIDR blocks 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.
</li>
</ul>
</dd>
<dt>Returns</dt>
@ -721,6 +731,16 @@ $ curl -XPOST "http://127.0.0.1:8200/v1/auth/approle/login" -d '{"role_id":"50be
_in plaintext_.
</li>
</ul>
<ul>
<li>
<span class="param">cidr_list</span>
<span class="param-flags">optional</span>
Comma separated list of CIDR blocks 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.
</li>
</ul>
</dd>
<dt>Returns</dt>