Add tests for files fixes... not yet ready, patterns are failing.

This commit is contained in:
Ignacio Gómez 2021-02-26 22:13:24 -03:00
parent d2fc01e2f7
commit e7c2aec65f
No known key found for this signature in database
GPG Key ID: 15A77C6BEC604B06
4 changed files with 217 additions and 81 deletions

View File

@ -185,12 +185,11 @@ func (o *Files) readPasswords() (int, error) {
}
//ReadAcls reads the Acl file and associates them to existing users. It omits any non existing users.
// ReadAcls reads the Acl file and associates them to existing users. It omits any non existing users.
func (o *Files) readAcls() (int, error) {
linesCount := 0
//Set currentUser as empty string
currentUser := ""
userExists := false
file, err := os.Open(o.AclPath)
if err != nil {
@ -204,94 +203,101 @@ func (o *Files) readAcls() (int, error) {
for scanner.Scan() {
index++
line := scanner.Text()
//Check comment or empty line to skip them.
if checkCommentOrEmpty(scanner.Text()) {
continue
}
//If we see a user line, change the current user.
if strings.Contains(line, "user") {
//Try to get username
lineArr := strings.Fields(line)
line := strings.TrimSpace(scanner.Text())
//Check format
if lineArr[0] == "user" {
username := strings.Join(lineArr[1:], " ")
lineArr := strings.Fields(line)
prefix := lineArr[0]
_, ok := o.Users[username]
//Check that user exists
if !ok {
log.Warnf("user %s doesn't exist, skipping acl", username)
continue
}
currentUser = username
} else {
return 0, errors.Errorf("Files backend error: wrong acl format at line %d", index)
if prefix == "user" {
// Since there may be more than one consecutive space in the username, we have to remove the prefix and trim to get the username.
username, err := removeAndTrim(prefix, line, index)
if err != nil {
return 0, err
}
} else if strings.Contains(line, "topic") || strings.Contains(line, "pattern") {
//Split and check for privileges.
lineArr := strings.Fields(line)
correctFormat := lineArr[0] == "topic" || lineArr[0] == "pattern"
_, ok := o.Users[username]
if correctFormat {
if !ok {
log.Warnf("user %s doesn't exist, skipping acls", username)
// Flag username to skip topics later.
userExists = false
continue
}
var aclRecord = AclRecord{
Topic: "",
Acc: MOSQ_ACL_NONE,
userExists = true
currentUser = username
} else if prefix == "topic" || prefix == "pattern" {
var aclRecord = AclRecord{
Topic: "",
Acc: MOSQ_ACL_NONE,
}
/* If len is 2, then we assume ReadWrite privileges.
Notice that Mosquitto docs prevent whitespaces in the topic when there's no explicit access given:
"The access type is controlled using "read", "write", "readwrite" or "deny". This parameter is optional (unless <topic> includes a space character)"
https://mosquitto.org/man/mosquitto-conf-5.html
When access is given, then the topic may contain whitespaces.
Nevertheless, there may be white spaces between topic/pattern and the permission or the topic itself.
Fields captures the case in which there's only topic/pattern and the given topic because it trims extra spaces between them.
*/
if len(lineArr) == 2 {
aclRecord.Topic = lineArr[1]
aclRecord.Acc = MOSQ_ACL_READWRITE
} else {
// There may be more than one space between topic/pattern and the permission, as well as between the latter and the topic itself.
// Hence, we remove the prefix, trim the line and split on white space to get the permission.
line, err = removeAndTrim(prefix, line, index)
if err != nil {
return 0, err
}
/* If len is 2, then we assume ReadWrite privileges.
lineArr = strings.Split(line, " ")
permission := lineArr[0]
Notice that Mosquitto docs prevent whitespaces when there's no explicit access given:
"The access type is controlled using "read", "write", "readwrite" or "deny". This parameter is optional (unless <topic> includes a space character)"
https://mosquitto.org/man/mosquitto-conf-5.html
When access is given, then the topic may contain whitespaces.
*/
if len(lineArr) == 2 {
aclRecord.Topic = lineArr[1]
aclRecord.Acc = MOSQ_ACL_READWRITE
} else {
permission := lineArr[1]
topic := strings.Join(lineArr[2:], " ")
switch permission {
case read, write, readwrite, subscribe, deny:
aclRecord.Acc = permissions[permission]
default:
return 0, errors.Errorf("Files backend error: wrong acl format at line %d", index)
}
aclRecord.Topic = topic
// Again, there may be more than one space between the permission and the topic, so we'll trim what's left after removing it and that'll be the topic.
topic, err := removeAndTrim(permission, line, index)
if err != nil {
return 0, err
}
if strings.Contains(line, "topic") {
//Append to user or general depending on currentUser.
if currentUser != "" {
fUser, ok := o.Users[currentUser]
if !ok {
return 0, errors.Errorf("Files backend error: user %s does not exist for acl at line %d", lineArr[1], index)
}
fUser.AclRecords = append(fUser.AclRecords, aclRecord)
} else {
o.AclRecords = append(o.AclRecords, aclRecord)
switch permission {
case read, write, readwrite, subscribe, deny:
aclRecord.Acc = permissions[permission]
default:
return 0, errors.Errorf("Files backend error: wrong acl format at line %d", index)
}
aclRecord.Topic = topic
}
if prefix == "topic" {
if currentUser != "" {
// Skip topic when user was not found.
if !userExists {
continue
}
fUser, ok := o.Users[currentUser]
if !ok {
return 0, errors.Errorf("Files backend error: user does not exist for acl at line %d", index)
}
fUser.AclRecords = append(fUser.AclRecords, aclRecord)
} else {
//Append to general acls.
o.AclRecords = append(o.AclRecords, aclRecord)
}
linesCount++
} else {
return 0, errors.Errorf("Files backend error: wrong acl format at line %d", index)
o.AclRecords = append(o.AclRecords, aclRecord)
}
linesCount++
} else {
return 0, errors.Errorf("Files backend error: wrong acl format at line %d", index)
}
@ -300,6 +306,15 @@ func (o *Files) readAcls() (int, error) {
return linesCount, nil
}
func removeAndTrim(prefix, line string, index int) (string, error) {
if len(line)-len(prefix) < 1 {
return "", errors.Errorf("Files backend error: wrong acl format at line %d", index)
}
newLine := strings.TrimSpace(line[len(prefix):])
return newLine, nil
}
func checkCommentOrEmpty(line string) bool {
if len(strings.Replace(line, " ", "", -1)) == 0 || line[0:1] == "#" {
return true
@ -340,16 +355,38 @@ func (o *Files) CheckAcl(username, topic, clientid string, acc int32) (bool, err
fileUser, ok := o.Users[username]
// If user exists, check against his acls and common ones. If not, check against common acls only.
// But first check if the topic was explicitly denied and refuse to authorize if so.
// Check if the topic was explicitly denied and refuse to authorize if so.
if ok {
for _, aclRecord := range fileUser.AclRecords {
match := TopicsMatch(aclRecord.Topic, topic)
if match {
if aclRecord.Acc == MOSQ_ACL_DENY {
return false, nil
}
}
}
}
for _, aclRecord := range o.AclRecords {
aclTopic := strings.Replace(aclRecord.Topic, "%c", clientid, -1)
aclTopic = strings.Replace(aclTopic, "%u", username, -1)
match := TopicsMatch(aclTopic, topic)
if match {
if aclRecord.Acc == MOSQ_ACL_DENY {
return false, nil
}
}
}
// No denials, check against user's acls and common ones. If not authorized, check against pattern acls.
if ok {
for _, aclRecord := range fileUser.AclRecords {
match := TopicsMatch(aclRecord.Topic, topic)
if match {
if acc == int32(aclRecord.Acc) || int32(aclRecord.Acc) == MOSQ_ACL_READWRITE || (acc == MOSQ_ACL_SUBSCRIBE && topic != "#" && (int32(aclRecord.Acc) == MOSQ_ACL_READ || int32(aclRecord.Acc) == MOSQ_ACL_SUBSCRIBE)) {
return true, nil
}
@ -364,10 +401,6 @@ func (o *Files) CheckAcl(username, topic, clientid string, acc int32) (bool, err
match := TopicsMatch(aclTopic, topic)
if match {
if aclRecord.Acc == MOSQ_ACL_DENY {
return false, nil
}
if acc == int32(aclRecord.Acc) || int32(aclRecord.Acc) == MOSQ_ACL_READWRITE || (acc == MOSQ_ACL_SUBSCRIBE && topic != "#" && (int32(aclRecord.Acc) == MOSQ_ACL_READ || int32(aclRecord.Acc) == MOSQ_ACL_SUBSCRIBE)) {
return true, nil
}

View File

@ -37,6 +37,9 @@ func TestFiles(t *testing.T) {
/*
ACL file looks like this:
topic test/general
topic deny test/general_denied
user test1
topic write test/topic/1
topic read test/topic/2
@ -46,21 +49,30 @@ func TestFiles(t *testing.T) {
user test3
topic read test/#
topic deny test/denied
user test with space
topic test/space
topic read test/multiple spaces in/topic
topic read test/lots of spaces in/topic and borders
user not_present
topic read test/#
topic read test/not_present
pattern read test/%u
pattern read test/%c
*/
// passwords are the same as users,
// except for user4 that's not present in psswords and should be skipped when reading acls
// except for user4 that's not present in passwords and should be skipped when reading acls
user1 := "test1"
user2 := "test2"
user3 := "test3"
user4 := "not_present"
elton := "test with space" // You know, because he's a rocket man. Ok, I'll let myself out.
generalTopic := "test/general"
generalDeniedTopic := "test/general_denied"
Convey("All users but not present ones should have a record", func() {
_, ok := files.Users[user1]
@ -74,6 +86,45 @@ func TestFiles(t *testing.T) {
_, ok = files.Users[user4]
So(ok, ShouldBeFalse)
_, ok = files.Users[elton]
So(ok, ShouldBeTrue)
})
Convey("All users should be able to read the general topic", func() {
authenticated, err := files.CheckAcl(user1, generalTopic, clientID, 1)
So(err, ShouldBeNil)
So(authenticated, ShouldBeTrue)
authenticated, err = files.CheckAcl(user2, generalTopic, clientID, 1)
So(err, ShouldBeNil)
So(authenticated, ShouldBeTrue)
authenticated, err = files.CheckAcl(user3, generalTopic, clientID, 1)
So(err, ShouldBeNil)
So(authenticated, ShouldBeTrue)
authenticated, err = files.CheckAcl(elton, generalTopic, clientID, 1)
So(err, ShouldBeNil)
So(authenticated, ShouldBeTrue)
})
Convey("No user should be able to read the general denied topic", func() {
authenticated, err := files.CheckAcl(user1, generalDeniedTopic, clientID, 1)
So(err, ShouldBeNil)
So(authenticated, ShouldBeFalse)
authenticated, err = files.CheckAcl(user2, generalDeniedTopic, clientID, 1)
So(err, ShouldBeNil)
So(authenticated, ShouldBeFalse)
authenticated, err = files.CheckAcl(user3, generalDeniedTopic, clientID, 1)
So(err, ShouldBeNil)
So(authenticated, ShouldBeFalse)
authenticated, err = files.CheckAcl(elton, generalDeniedTopic, clientID, 1)
So(err, ShouldBeNil)
So(authenticated, ShouldBeFalse)
})
Convey("Given a username and a correct password, it should correctly authenticate it", func() {
@ -112,6 +163,22 @@ func TestFiles(t *testing.T) {
testTopic3 := `test/other/1`
testTopic4 := `other/1`
readWriteTopic := "readwrite/topic"
spaceTopic := "test/space"
multiSpaceTopic := "test/multiple spaces in/topic"
lotsOfSpacesTopic := "test/lots of spaces in/topic and borders"
deniedTopic := "test/denied"
Convey("Topics for non existing users should be ignored", func() {
for record := range files.AclRecords {
So(record, ShouldNotEqual, "test/not_present")
}
for _, user := range files.Users {
for record := range user.AclRecords {
So(record, ShouldNotEqual, "test/not_present")
}
}
})
Convey("User 1 should be able to publish and not subscribe to test topic 1, and only subscribe but not publish to topic 2", func() {
tt1, err1 := files.CheckAcl(user1, testTopic1, clientID, 2)
@ -151,20 +218,24 @@ func TestFiles(t *testing.T) {
So(tt3, ShouldBeFalse)
})
Convey("User 3 should be able to read any test/X but not other/...", func() {
Convey("User 3 should be able to read any test/X but not other/... nor test/denied\n\n", func() {
fmt.Printf("\n\nUser 3 acls: %#v", files.Users[user3].AclRecords)
tt1, err1 := files.CheckAcl(user3, testTopic1, clientID, 1)
tt2, err2 := files.CheckAcl(user3, testTopic2, clientID, 1)
tt3, err3 := files.CheckAcl(user3, testTopic3, clientID, 1)
tt4, err4 := files.CheckAcl(user3, testTopic4, clientID, 1)
tt5, err5 := files.CheckAcl(user3, deniedTopic, clientID, 1)
So(err1, ShouldBeNil)
So(err2, ShouldBeNil)
So(err3, ShouldBeNil)
So(err4, ShouldBeNil)
So(err5, ShouldBeNil)
So(tt1, ShouldBeTrue)
So(tt2, ShouldBeTrue)
So(tt3, ShouldBeTrue)
So(tt4, ShouldBeFalse)
So(tt5, ShouldBeFalse)
})
Convey("User 4 should not be able to read since it's not in the passwords file", func() {
@ -174,12 +245,34 @@ func TestFiles(t *testing.T) {
So(tt1, ShouldBeFalse)
})
//Now check against patterns.
Convey("Elton Bowie should be able to read and write to `test/space`, and only read from other topics", func() {
tt1, err1 := files.CheckAcl(elton, spaceTopic, clientID, 2)
tt2, err2 := files.CheckAcl(elton, multiSpaceTopic, clientID, 1)
tt3, err3 := files.CheckAcl(elton, multiSpaceTopic, clientID, 2)
tt4, err4 := files.CheckAcl(elton, lotsOfSpacesTopic, clientID, 1)
tt5, err5 := files.CheckAcl(elton, lotsOfSpacesTopic, clientID, 2)
So(err1, ShouldBeNil)
So(err2, ShouldBeNil)
So(err3, ShouldBeNil)
So(err4, ShouldBeNil)
So(err5, ShouldBeNil)
So(tt1, ShouldBeTrue)
So(tt2, ShouldBeTrue)
So(tt3, ShouldBeFalse)
So(tt4, ShouldBeTrue)
So(tt5, ShouldBeFalse)
})
//Now check against patterns.
Convey("Given a topic that mentions username, acl check should pass", func() {
tt1, err1 := files.CheckAcl(user1, "test/test1", clientID, 1)
So(err1, ShouldBeNil)
So(tt1, ShouldBeTrue)
tt2, err2 := files.CheckAcl(elton, "test/test with space", clientID, 1)
So(err2, ShouldBeNil)
So(tt2, ShouldBeTrue)
})
Convey("Given a topic that mentions clientid, acl check should pass", func() {

View File

@ -1,3 +1,6 @@
topic read test/general
topic deny test/general_denied
user test1
topic write test/topic/1
topic read test/topic/2
@ -8,9 +11,15 @@ topic read test/topic/+
user test3
topic read test/#
topic deny test/denied
user test with space
topic test/space
topic read test/multiple spaces in/topic
topic read test/lots of spaces in/topic and borders
user not_present
topic read test/#
topic read test/not_present
pattern read test/%u
pattern read test/%c

View File

@ -1,3 +1,4 @@
test1:PBKDF2$sha512$100000$2WQHK5rjNN+oOT+TZAsWAw==$TDf4Y6J+9BdnjucFQ0ZUWlTwzncTjOOeE00W4Qm8lfPQyPCZACCjgfdK353jdGFwJjAf6vPAYaba9+z4GWK7Gg==
test2:PBKDF2$sha512$100000$o513B9FfaKTL6xalU+UUwA==$mAUtjVg1aHkDpudOnLKUQs8ddGtKKyu+xi07tftd5umPKQKnJeXf1X7RpoL/Gj/ZRdpuBu5GWZ+NZ2rYyAsi1g==
test3:PBKDF2$sha512$100000$gDJp1GiuxauYi6jM+aI+vw==$9Rn4GrsfUkpyXdqfN3COU4oKpy7NRiLkcyutQ7I3ki1I2oY8/fuBnu+3oPKOm8WkAlpOnuwvTMGvii5QIIKmWA==
test3:PBKDF2$sha512$100000$gDJp1GiuxauYi6jM+aI+vw==$9Rn4GrsfUkpyXdqfN3COU4oKpy7NRiLkcyutQ7I3ki1I2oY8/fuBnu+3oPKOm8WkAlpOnuwvTMGvii5QIIKmWA==
test with space:PBKDF2$sha512$100000$uB2YB/cgHc+FOOzzfyy8TQ==$+m2jZlNjJ9w7GEDvcThfJ2fJGvClupdh/ygamPDrxks+CKv5SlcFMwIjElDrosmpMYMAhtGcE0CEhAFMQ2EqQQ==