diff --git a/backends/constants.go b/backends/constants.go index 79389c0..040db65 100644 --- a/backends/constants.go +++ b/backends/constants.go @@ -8,4 +8,5 @@ const ( MOSQ_ACL_WRITE = 0x02 MOSQ_ACL_READWRITE = 0x03 MOSQ_ACL_SUBSCRIBE = 0x04 + MOSQ_ACL_DENY = 0x11 ) diff --git a/backends/files.go b/backends/files.go index ac3ab13..34f2855 100644 --- a/backends/files.go +++ b/backends/files.go @@ -14,6 +14,22 @@ import ( log "github.com/sirupsen/logrus" ) +const ( + read = "read" + write = "write" + readwrite = "readwrite" + subscribe = "subscribe" + deny = "deny" +) + +var permissions = map[string]byte{ + read: MOSQ_ACL_READ, + write: MOSQ_ACL_WRITE, + readwrite: MOSQ_ACL_READWRITE, + subscribe: MOSQ_ACL_SUBSCRIBE, + deny: MOSQ_ACL_DENY, +} + //FileUer keeps a user password and acl records. type FileUser struct { Password string @@ -23,7 +39,7 @@ type FileUser struct { //AclRecord holds a topic and access privileges. type AclRecord struct { Topic string - Acc byte //None 0x00, Read 0x01, Write 0x02, ReadWrite: Read | Write : 0x03 + Acc byte //None 0x00, Read 0x01, Write 0x02, ReadWrite: Read | Write : 0x03, Subscribe 0x04, Deny 0x11 } //FileBE holds paths to files, list of file users and general (no user or pattern) acl records. @@ -169,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 { @@ -188,127 +203,118 @@ 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 len(lineArr) == 2 && lineArr[0] == "user" { - _, ok := o.Users[lineArr[1]] + lineArr := strings.Fields(line) + prefix := lineArr[0] - //Check that user exists - if !ok { - log.Warnf("user %s doesn't exist, skipping acl", lineArr[1]) - continue - } + 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 + } - currentUser = lineArr[1] + _, ok := o.Users[username] - } else { - return 0, errors.Errorf("Files backend error: wrong acl format at line %d", index) + if !ok { + log.Warnf("user %s doesn't exist, skipping acls", username) + // Flag username to skip topics later. + userExists = false + continue } - } else if strings.Contains(line, "topic") { - //Split and check for read, write or empty (readwwrite) privileges. - lineArr := strings.Fields(line) + 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. - if (len(lineArr) == 2 || len(lineArr) == 3) && lineArr[0] == "topic" { + 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 includes a space character)" + https://mosquitto.org/man/mosquitto-conf-5.html + When access is given, then the topic may contain whitespaces. - var aclRecord = AclRecord{ - Topic: "", - Acc: MOSQ_ACL_NONE, + 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. - if len(lineArr) == 2 { - aclRecord.Topic = lineArr[1] - aclRecord.Acc = MOSQ_ACL_READWRITE - } else { - aclRecord.Topic = lineArr[2] - if lineArr[1] == "read" { - aclRecord.Acc = MOSQ_ACL_READ - } else if lineArr[1] == "write" { - aclRecord.Acc = MOSQ_ACL_WRITE - } else if lineArr[1] == "readwrite" { - aclRecord.Acc = MOSQ_ACL_READWRITE - } else if lineArr[1] == "subscribe" { - aclRecord.Acc = MOSQ_ACL_SUBSCRIBE - } else { - return 0, errors.Errorf("Files backend error: wrong acl format at line %d", index) - } + lineArr = strings.Split(line, " ") + permission := lineArr[0] + + // 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 + } + + 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) } - //Append to user or general depending on currentUser. + 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 %s does not exist for acl at line %d", lineArr[1], index) + return 0, errors.Errorf("Files backend error: user does not exist for acl at line %d", index) } fUser.AclRecords = append(fUser.AclRecords, aclRecord) } else { o.AclRecords = append(o.AclRecords, aclRecord) } - - linesCount++ - } else { - return 0, errors.Errorf("Files backend error: wrong acl format at line %d", index) - } - - } else if strings.Contains(line, "pattern") { - - //Split and check for read, write or empty (readwwrite) privileges. - lineArr := strings.Fields(line) - - if (len(lineArr) == 2 || len(lineArr) == 3) && lineArr[0] == "pattern" { - - var aclRecord = AclRecord{ - Topic: "", - Acc: MOSQ_ACL_NONE, - } - - //If len is 2, then we assume ReadWrite privileges. - if len(lineArr) == 2 { - aclRecord.Topic = lineArr[1] - aclRecord.Acc = MOSQ_ACL_READWRITE - } else { - aclRecord.Topic = lineArr[2] - if lineArr[1] == "read" { - aclRecord.Acc = MOSQ_ACL_READ - } else if lineArr[1] == "write" { - aclRecord.Acc = MOSQ_ACL_WRITE - } else if lineArr[1] == "readwrite" { - aclRecord.Acc = MOSQ_ACL_READWRITE - } else if lineArr[1] == "subscribe" { - aclRecord.Acc = MOSQ_ACL_SUBSCRIBE - } else { - return 0, errors.Errorf("Files backend error: wrong acl format at line %d", index) - } - } - - //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) } + linesCount++ + + } else { + return 0, errors.Errorf("Files backend error: wrong acl format at line %d", index) } } 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 @@ -349,11 +355,41 @@ 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. + // Check if the topic was explicitly denied and refuse to authorize if so. if ok { for _, aclRecord := range fileUser.AclRecords { - if TopicsMatch(aclRecord.Topic, topic) && (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 + 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 + } } } } @@ -361,8 +397,13 @@ func (o *Files) CheckAcl(username, topic, clientid string, acc int32) (bool, err //Replace all occurrences of %c for clientid and %u for username aclTopic := strings.Replace(aclRecord.Topic, "%c", clientid, -1) aclTopic = strings.Replace(aclTopic, "%u", username, -1) - if TopicsMatch(aclTopic, topic) && (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 + + match := TopicsMatch(aclTopic, 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 + } } } diff --git a/backends/files_test.go b/backends/files_test.go index 18e29d3..34802ed 100644 --- a/backends/files_test.go +++ b/backends/files_test.go @@ -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() { diff --git a/test-files/acls b/test-files/acls index 26fb4ed..366753b 100644 --- a/test-files/acls +++ b/test-files/acls @@ -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 \ No newline at end of file diff --git a/test-files/passwords b/test-files/passwords index 04a0ac1..0baee73 100644 --- a/test-files/passwords +++ b/test-files/passwords @@ -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== \ No newline at end of file +test3:PBKDF2$sha512$100000$gDJp1GiuxauYi6jM+aI+vw==$9Rn4GrsfUkpyXdqfN3COU4oKpy7NRiLkcyutQ7I3ki1I2oY8/fuBnu+3oPKOm8WkAlpOnuwvTMGvii5QIIKmWA== +test with space:PBKDF2$sha512$100000$uB2YB/cgHc+FOOzzfyy8TQ==$+m2jZlNjJ9w7GEDvcThfJ2fJGvClupdh/ygamPDrxks+CKv5SlcFMwIjElDrosmpMYMAhtGcE0CEhAFMQ2EqQQ== \ No newline at end of file