Skip to content

Commit

Permalink
fix: prevent access from anonymous user (#21242)
Browse files Browse the repository at this point in the history
Co-authored-by: Qiu Jian <[email protected]>
  • Loading branch information
swordqiu and Qiu Jian authored Sep 14, 2024
1 parent 6acbf34 commit 09e9ffb
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 13 deletions.
14 changes: 7 additions & 7 deletions pkg/cloudcommon/policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func getMaskedLoginIp(userCred mcclient.TokenCredential) string {
}

func policyKey(userCred mcclient.TokenCredential) string {
if userCred == nil || auth.IsGuestToken(userCred) {
if userCred == nil || len(userCred.GetTokenString()) == 0 || auth.IsGuestToken(userCred) {
return auth.GUEST_TOKEN
}
keys := []string{userCred.GetProjectId()}
Expand Down Expand Up @@ -305,12 +305,6 @@ func (manager *SPolicyManager) fetchMatchedPolicies(userCred mcclient.TokenCrede
}

func (manager *SPolicyManager) allow(scope rbacscope.TRbacScope, userCred mcclient.TokenCredential, service string, resource string, action string, extra ...string) rbacutils.SPolicyResult {
// first download userCred policy
policies, err := manager.fetchMatchedPolicies(userCred)
if err != nil {
log.Errorf("fetchMatchedPolicyGroup fail %s", err)
return rbacutils.PolicyDeny
}
// check permission
key := permissionKey(scope, userCred, service, resource, action, extra...)
val := manager.permissionCache.AtomicGet(key)
Expand All @@ -321,6 +315,12 @@ func (manager *SPolicyManager) allow(scope rbacscope.TRbacScope, userCred mcclie
return val.(rbacutils.SPolicyResult)
}

// first download userCred policy
policies, err := manager.fetchMatchedPolicies(userCred)
if err != nil {
log.Errorf("fetchMatchedPolicyGroup fail %s", err)
return rbacutils.PolicyDeny
}
policySet, ok := policies.Policies[scope]
if !ok {
policySet = rbacutils.TPolicySet{}
Expand Down
11 changes: 9 additions & 2 deletions pkg/keystone/models/rolepolicies.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ func (manager *SRolePolicyManager) FetchCustomizeColumns(

func (manager *SRolePolicyManager) getMatchPolicyIds(userCred rbacutils.IRbacIdentity, tm time.Time) ([]string, error) {
isGuest := true
if userCred != nil && !auth.IsGuestToken(userCred) {
if userCred != nil && len(userCred.GetProjectId()) > 0 && len(userCred.GetRoleIds()) > 0 && !auth.IsGuestToken(userCred) {
isGuest = false
}
return manager.getMatchPolicyIds2(isGuest, userCred.GetRoleIds(), userCred.GetProjectId(), userCred.GetLoginIp(), tm)
Expand Down Expand Up @@ -437,6 +437,9 @@ func (p sUserProjectPair) GetProjectId() string {
}

func (p sUserProjectPair) GetRoleIds() []string {
if len(p.userId) == 0 || len(p.projectId) == 0 {
return nil
}
roles, _ := AssignmentManager.FetchUserProjectRoles(p.userId, p.projectId)
ret := make([]string, len(roles))
for i := range roles {
Expand All @@ -446,14 +449,17 @@ func (p sUserProjectPair) GetRoleIds() []string {
}

func (p sUserProjectPair) GetUserId() string {
return ""
return p.userId
}

func (p sUserProjectPair) GetLoginIp() string {
return ""
}

func (p sUserProjectPair) GetTokenString() string {
if len(p.userId) == 0 {
return auth.GUEST_TOKEN
}
return p.userId
}

Expand All @@ -479,6 +485,7 @@ func (manager *SRolePolicyManager) GetMatchPolicyGroupByCred(ctx context.Context
userId := userCred.GetUserId()
if len(userId) == 0 {
// anonymous access
log.Debugf("anomymouse accessed policies: %s", jsonutils.Marshal(names))
return names, policies, nil
}
usr, err := UserManager.fetchUserById(userId)
Expand Down
2 changes: 2 additions & 0 deletions pkg/keystone/tokens/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"net/http"
"time"

"yunion.io/x/log"
"yunion.io/x/pkg/errors"
"yunion.io/x/pkg/util/rbacscope"

Expand Down Expand Up @@ -71,6 +72,7 @@ func doCheckPolicies(ctx context.Context, input mcclient.SCheckPoliciesInput) (*
if policy.PolicyManager.Allow(rbacscope.ScopeSystem, adminToken, api.SERVICE_TYPE, "tokens", "perform", "check_policies").Result.IsDeny() {
return nil, httperrors.NewForbiddenError("%s not allow to check policies", adminToken.GetUserName())
}
log.Debugf("doCheckPolicies userId: %s projectId: %s", input.UserId, input.ProjectId)
names, group, err := models.RolePolicyManager.GetMatchPolicyGroupByInput(ctx, input.UserId, input.ProjectId, time.Now(), false)
if err != nil {
return nil, errors.Wrap(err, "GetMatchPolicyGroupByInput")
Expand Down
2 changes: 1 addition & 1 deletion pkg/mcclient/auth/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (

var (
GUEST_USER = "guest"
GUEST_TOKEN = "guest_token"
GUEST_TOKEN = rbacutils.GUEST_TOKEN // "guest_token"
GuestToken = mcclient.SSimpleToken{
User: GUEST_USER,
Token: GUEST_TOKEN,
Expand Down
33 changes: 30 additions & 3 deletions pkg/util/rbacutils/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,28 @@ type SRbacPolicy struct {
Rules TPolicy
}

func (p SRbacPolicy) needAuth() bool {
if len(p.Condition) > 0 {
return true
}
if len(p.DomainId) > 0 {
return true
}
if len(p.SharedDomainIds) > 0 {
return true
}
if len(p.Projects) > 0 {
return true
}
if len(p.Roles) > 0 {
return true
}
if p.Auth {
return true
}
return false
}

var (
tenantEqualsPattern = regexp.MustCompile(`tenant\s*==\s*['"]?(\w+)['"]?`)
roleContainsPattern = regexp.MustCompile(`roles.contains\(['"]?(\w+)['"]?\)`)
Expand Down Expand Up @@ -221,10 +243,15 @@ func (policy *SRbacPolicy) MatchRoles(roleNames []string) bool {
// int match weight, the higher the value, the more exact the match
// the more exact match wins
func (policy *SRbacPolicy) Match(userCred IRbacIdentity2) (bool, int) {
if !policy.Auth && len(policy.Roles) == 0 && len(policy.Projects) == 0 && len(policy.Ips) == 0 {
return true, 1
if !policy.needAuth() {
if len(policy.Ips) == 0 {
return true, 1
}
if containsIp(policy.Ips, userCred.GetLoginIp()) {
return true, 1
}
}
if userCred == nil || len(userCred.GetTokenString()) == 0 {
if userCred == nil || len(userCred.GetTokenString()) == 0 || userCred.GetTokenString() == GUEST_TOKEN {
return false, 0
}
weight := 0
Expand Down
2 changes: 2 additions & 0 deletions pkg/util/rbacutils/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ const (
// ScopeObject = "object"
ScopeUser = rbacscope.ScopeUser
ScopeNone = rbacscope.ScopeNone*/

GUEST_TOKEN = "guest_token"
)

func (r TRbacResult) IsAllow() bool {
Expand Down

0 comments on commit 09e9ffb

Please sign in to comment.