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

Fix ACL #71

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
9 changes: 5 additions & 4 deletions cmd/grumble/freeze.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"os"
"path/filepath"
"strconv"
"strings"
"time"

"github.com/golang/protobuf/proto"
Expand Down Expand Up @@ -262,7 +263,7 @@ func (c *Channel) Unfreeze(fc *freezer.Channel) {
if fgrp.Name == nil {
continue
}
g := acl.Group{}
g := acl.EmptyGroupWithName(fgrp.GetName())
if fgrp.Inherit != nil {
g.Inherit = *fgrp.Inherit
}
Expand Down Expand Up @@ -483,7 +484,7 @@ func NewServerFromFrozen(name string) (s *Server, err error) {
// Update the server's user maps to point correctly
// to the new user.
s.Users[u.Id] = u
s.UserNameMap[u.Name] = u
s.UserNameMap[strings.ToLower(u.Name)] = u
if len(u.CertHash) > 0 {
s.UserCertMap[u.CertHash] = u
}
Expand Down Expand Up @@ -553,7 +554,7 @@ func NewServerFromFrozen(name string) (s *Server, err error) {
// Update the various user maps in the server to
// be able to correctly look up the user.
s.Users[user.Id] = user
s.UserNameMap[user.Name] = user
s.UserNameMap[strings.ToLower(user.Name)] = user
if len(user.CertHash) > 0 {
s.UserCertMap[user.CertHash] = user
}
Expand All @@ -574,7 +575,7 @@ func NewServerFromFrozen(name string) (s *Server, err error) {
if ok {
// Clear the server maps. That should do it.
delete(s.Users, userId)
delete(s.UserNameMap, user.Name)
delete(s.UserNameMap, strings.ToLower(user.Name))
if len(user.CertHash) > 0 {
delete(s.UserCertMap, user.CertHash)
}
Expand Down
7 changes: 5 additions & 2 deletions cmd/grumble/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"crypto/tls"
"fmt"
"net"
"strings"
"time"

"github.com/golang/protobuf/proto"
Expand Down Expand Up @@ -1211,6 +1212,7 @@ func (server *Server) handleAclMessage(client *Client, msg *Message) {
if pbacl.UserId != nil {
chanacl.UserId = int(*pbacl.UserId)
} else {
chanacl.UserId = -1
chanacl.Group = *pbacl.Group
}
chanacl.Deny = acl.Permission(*pbacl.Deny & acl.AllPermissions)
Expand All @@ -1223,13 +1225,14 @@ func (server *Server) handleAclMessage(client *Client, msg *Message) {
server.ClearCaches()

// Regular user?
if !acl.HasPermission(&channel.ACL, client, acl.WritePermission) && client.IsRegistered() || client.HasCertificate() {
if !acl.HasPermission(&channel.ACL, client, acl.WritePermission) && (client.IsRegistered() || client.HasCertificate()) {
chanacl := acl.ACL{}
chanacl.ApplyHere = true
chanacl.ApplySubs = false
if client.IsRegistered() {
chanacl.UserId = client.UserId()
} else if client.HasCertificate() {
chanacl.UserId = -1
chanacl.Group = "$" + client.CertHash()
}
chanacl.Deny = acl.Permission(acl.NonePermission)
Expand Down Expand Up @@ -1267,7 +1270,7 @@ func (server *Server) handleQueryUsers(client *Client, msg *Message) {
}

for _, name := range query.Names {
user, exists := server.UserNameMap[name]
user, exists := server.UserNameMap[strings.ToLower(name)]
if exists {
reply.Ids = append(reply.Ids, user.Id)
reply.Names = append(reply.Names, name)
Expand Down
26 changes: 11 additions & 15 deletions cmd/grumble/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func NewServer(id int64) (s *Server, err error) {
s.UserCertMap = make(map[string]*User)
s.UserNameMap = make(map[string]*User)
s.Users[0], err = NewUser(0, "SuperUser")
s.UserNameMap["SuperUser"] = s.Users[0]
s.UserNameMap["superuser"] = s.Users[0]
s.nextUserId = 1

s.Channels = make(map[int]*Channel)
Expand Down Expand Up @@ -508,7 +508,7 @@ func (server *Server) handleAuthenticate(client *Client, msg *Message) {
} else {
if server.CheckSuperUserPassword(*auth.Password) {
ok := false
client.user, ok = server.UserNameMap[client.Username]
client.user, ok = server.UserNameMap[strings.ToLower(client.Username)]
if !ok {
client.RejectAuth(mumbleproto.Reject_InvalidUsername, "")
return
Expand All @@ -520,7 +520,7 @@ func (server *Server) handleAuthenticate(client *Client, msg *Message) {
}
} else {
// First look up registration by name.
user, exists := server.UserNameMap[client.Username]
user, exists := server.UserNameMap[strings.ToLower(client.Username)]
if exists {
if client.HasCertificate() && user.CertHash == client.CertHash() {
client.user = user
Expand Down Expand Up @@ -689,12 +689,10 @@ func (server *Server) finishAuthenticate(client *Client) {
if client.IsSuperUser() {
sync.Permissions = proto.Uint64(uint64(acl.AllPermissions))
} else {
// fixme(mkrautz): previously we calculated the user's
// permissions and sent them to the client in here. This
// code relied on our ACL cache, but that has been temporarily
// thrown out because of our ACL handling code moving to its
// own package.
sync.Permissions = nil
// todo: acl caching?
// It might seem like we should send the permissions relative to the channel joined, but
// Murmur sends the root channel permissions, so we do the same
sync.Permissions = proto.Uint64(uint64(acl.EffectivePermission(&server.RootChannel().ACL, client)))
}
if err := client.sendMessage(sync); err != nil {
client.Panicf("%v", err)
Expand Down Expand Up @@ -899,10 +897,8 @@ func (server *Server) sendClientPermissions(client *Client, channel *Channel) {
return
}

// fixme(mkrautz): re-add when we have ACL caching
return

perm := acl.Permission(acl.NonePermission)
// todo: acl caching?
perm := acl.EffectivePermission(&channel.ACL, client)
client.sendMessage(&mumbleproto.PermissionQuery{
ChannelId: proto.Uint32(uint32(channel.Id)),
Permissions: proto.Uint32(uint32(perm)),
Expand Down Expand Up @@ -1142,7 +1138,7 @@ func (s *Server) RegisterClient(client *Client) (uid uint32, err error) {
uid = s.nextUserId
s.Users[uid] = user
s.UserCertMap[client.CertHash()] = user
s.UserNameMap[client.Username] = user
s.UserNameMap[strings.ToLower(client.Username)] = user

return uid, nil
}
Expand All @@ -1157,7 +1153,7 @@ func (s *Server) RemoveRegistration(uid uint32) (err error) {
// Remove from user maps
delete(s.Users, uid)
delete(s.UserCertMap, user.CertHash)
delete(s.UserNameMap, user.Name)
delete(s.UserNameMap, strings.ToLower(user.Name))

// Remove from groups and ACLs.
s.removeRegisteredUserFromChannel(uid, s.RootChannel())
Expand Down
30 changes: 16 additions & 14 deletions pkg/acl/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ type ACL struct {
// The ApplyHere flag determines whether the ACL
// should apply to the current channel.
ApplyHere bool
// The ApplySubs flag determines whethr the ACL
// The ApplySubs flag determines whether the ACL
// should apply to subchannels.
ApplySubs bool

Expand All @@ -87,17 +87,21 @@ func (acl *ACL) IsChannelACL() bool {
// HasPermission checks whether the given user has permission perm in the given context.
// The permission perm must be a single permission and not a combination of permissions.
func HasPermission(ctx *Context, user User, perm Permission) bool {
// This test mirrors the Murmur behaviour. Unfortunately, this means that a combination of permissions
// results in an inclusive OR check i.e. if any one of the bits are set return true.
return (EffectivePermission(ctx, user) & perm) != NonePermission
}

// EffectivePermission returns the effective permission bits for a user in the given context.
func EffectivePermission(ctx *Context, user User) Permission {
// We can't check permissions on a nil ctx.
if ctx == nil {
panic("acl: HasPermission got nil context")
panic("acl: EffectivePermission got nil context")
}

// SuperUser can't speak or whisper, but everything else is OK
if user.UserId() == 0 {
if perm == SpeakPermission || perm == WhisperPermission {
return false
}
return true
return Permission(AllPermissions &^ (SpeakPermission | WhisperPermission))
}

// Default permissions
Expand All @@ -116,13 +120,13 @@ func HasPermission(ctx *Context, user User, perm Permission) bool {
}
// Iterate through ACLs that are defined on ctx. Note: this does not include
// ACLs that iter has inherited from a parent (unless there is also a group on
// iter with the same name, that changes the permissions a bit!)
// iter with the same name, that modifies the permissions a bit!)
for _, acl := range ctx.ACLs {
// Determine whether the ACL applies to user.
// If it is a user ACL and the user id of the ACL
// matches user's id, we're good to go.
//
// If it's a group ACL, we have to parse and interpret
// If it is a group ACL, we have to parse and interpret
// the group string in the current context to determine
// membership. For that we use GroupMemberCheck.
matchUser := acl.IsUserACL() && acl.UserId == user.UserId()
Expand Down Expand Up @@ -158,12 +162,10 @@ func HasPermission(ctx *Context, user User, perm Permission) bool {

// The +write permission implies all permissions except for +speak and +whisper.
// This means that if the user has WritePermission, we should return true for all
// permissions exccept SpeakPermission and WhisperPermission.
if perm != SpeakPermission && perm != WhisperPermission {
return (granted & (perm | WritePermission)) != NonePermission
} else {
return (granted & perm) != NonePermission
// permissions except SpeakPermission and WhisperPermission.
if (granted & WritePermission) != NonePermission {
return granted | (AllPermissions &^ (SpeakPermission | WhisperPermission))
}

return false
return granted
}
2 changes: 1 addition & 1 deletion pkg/acl/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ func GroupMemberCheck(current *Context, acl *Context, name string, user User) (o
func (ctx *Context) GroupNames() []string {
names := map[string]bool{}
origCtx := ctx
contexts := []*Context{}
contexts := buildChain(ctx)

// Walk through the whole context chain and all groups in it.
for _, ctx := range contexts {
Expand Down