Skip to content

Commit

Permalink
Revert authorization/authentication split
Browse files Browse the repository at this point in the history
Authentication provider module is responsible only for authentication.
Nothing more. Access control (authorization) should be kept separate.
  • Loading branch information
foxcpp committed Feb 27, 2020
1 parent 3092ca0 commit 55a91a3
Show file tree
Hide file tree
Showing 14 changed files with 121 additions and 279 deletions.
21 changes: 3 additions & 18 deletions docs/man/maddy-auth.5.scd
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ Most likely, you are going to use these modules with 'auth' directive of IMAP
sql module described in *maddy-storage*(5) can also be used as a authentication
backend.

The authorization identity is required to be a valid RFC 5321 e-mail address.
It is returned as the authorization identity.
The username is required to be a valid RFC 5321 e-mail address.

# External authentication module (extauth)

Expand All @@ -32,8 +31,6 @@ authentication is considered successful. If the status code is 1 -
authentication is failed. If the status code is 2 - another unrelated error has
happened. Additional information should be written to stderr.

The authorization identtity is the same as authorization identity.

```
extauth {
helper /usr/bin/ldap-helper
Expand Down Expand Up @@ -79,13 +76,10 @@ maddy should be built with libpam build tag to use this module without
go get -tags 'libpam' ...
```

The authorization identtity is the same as authorization identity.

```
pam {
debug no
use_helper no
expect_address yes
}
```

Expand All @@ -109,7 +103,7 @@ privileges (e.g. when using system accounts).
For 2, you need to make maddy-pam-helper binary setuid, see
README.md in source tree for details.

TL;DR (assuming you have maddy group):
TL;DR (assuming you have the maddy group):
```
chown root:maddy /usr/lib/maddy/maddy-pam-helper
chmod u+xs,g+x,o-x /usr/lib/maddy/maddy-pam-helper
Expand All @@ -120,8 +114,6 @@ chmod u+xs,g+x,o-x /usr/lib/maddy/maddy-pam-helper
Implements authentication by reading /etc/shadow. Alternatively it can be
configured to use helper binary like extauth does.

The authorization identtity is the same as authorization identity.

```
shadow {
debug no
Expand Down Expand Up @@ -175,14 +167,7 @@ How it works:
2. Each table specified with the 'user' directive looked up using normalized
username. If match is not found in any table, authentication fails.
3. Each authentication provider specified with the 'pass' directive is tried.
First match wins same as in step 2.

The set of authorization identities estabilished by plain_separate is the list of
strings found in 'user' tables. If there are no 'user' tables (only 'pass'
directives are specified), the list of authorization identities returned by
auth. providers is used.
*Note*: If a username table returns empty string, the authorization identity is
assumed to be the same as authentication identity.
If authentication with all providers fails - an error is returned.

## Configuration directives

Expand Down
7 changes: 3 additions & 4 deletions internal/auth/external/externalauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,13 @@ func (ea *ExternalAuth) Init(cfg *config.Map) error {
return nil
}

func (ea *ExternalAuth) AuthPlain(username, password string) ([]string, error) {
func (ea *ExternalAuth) AuthPlain(username, password string) error {
accountName, ok := auth.CheckDomainAuth(username, ea.perDomain, ea.domains)
if !ok {
return nil, module.ErrUnknownCredentials
return module.ErrUnknownCredentials
}

// TODO: Extend process protocol to support multiple authorization identities.
return []string{username}, AuthUsingHelper(ea.helperPath, accountName, password)
return AuthUsingHelper(ea.helperPath, accountName, password)
}

func init() {
Expand Down
8 changes: 4 additions & 4 deletions internal/auth/pam/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,17 @@ func (a *Auth) Init(cfg *config.Map) error {
return nil
}

func (a *Auth) AuthPlain(username, password string) ([]string, error) {
func (a *Auth) AuthPlain(username, password string) error {
if a.useHelper {
if err := external.AuthUsingHelper(a.helperPath, username, password); err != nil {
return nil, err
return err
}
}
err := runPAMAuth(username, password)
if err != nil {
return nil, err
return err
}
return []string{username}, nil
return nil
}

func init() {
Expand Down
61 changes: 18 additions & 43 deletions internal/auth/plain_separate/plain_separate.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/foxcpp/maddy/internal/config"
"github.com/foxcpp/maddy/internal/log"
"github.com/foxcpp/maddy/internal/module"
"golang.org/x/text/secure/precis"
)

type Auth struct {
Expand Down Expand Up @@ -54,56 +53,32 @@ func (a *Auth) Init(cfg *config.Map) error {
return nil
}

func (a *Auth) AuthPlain(username, password string) ([]string, error) {
key, err := precis.UsernameCaseMapped.CompareKey(username)
if err != nil {
return nil, err
}

identities := make([]string, 0, 1)
if len(a.userTbls) != 0 {
for _, tbl := range a.userTbls {
repl, ok, err := tbl.Lookup(key)
if err != nil {
return nil, err
}
if !ok {
continue
}
if repl != "" {
identities = append(identities, repl)
} else {
identities = append(identities, key)
}
if a.onlyFirstID && len(identities) != 0 {
break
}
func (a *Auth) AuthPlain(username, password string) error {
ok := len(a.userTbls) == 0
for _, tbl := range a.userTbls {
_, tblOk, err := tbl.Lookup(username)
if err != nil {
return err
}
if len(identities) == 0 {
return nil, errors.New("plain_separate: unknown credentials")
if tblOk {
ok = true
break
}
}
if !ok {
return errors.New("user not found in tables")
}

var (
lastErr error
ok bool
)
for _, pass := range a.passwd {
passIDs, err := pass.AuthPlain(username, password)
if err != nil {
var lastErr error
for _, p := range a.passwd {
if err := p.AuthPlain(username, password); err != nil {
lastErr = err
continue
}
if len(a.userTbls) == 0 {
identities = append(identities, passIDs...)
}
ok = true
}
if !ok {
return nil, lastErr
}

return identities, nil
return nil
}
return lastErr
}

func init() {
Expand Down
65 changes: 26 additions & 39 deletions internal/auth/plain_separate/plain_separate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,26 @@ package plain_separate

import (
"errors"
"reflect"
"testing"

"github.com/emersion/go-sasl"
"github.com/foxcpp/maddy/internal/module"
)

type mockAuth struct {
db map[string][]string
db map[string]bool
}

func (mockAuth) SASLMechanisms() []string {
return []string{sasl.Plain, sasl.Login}
}

func (m mockAuth) AuthPlain(username, _ string) ([]string, error) {
ids, ok := m.db[username]
func (m mockAuth) AuthPlain(username, _ string) error {
ok := m.db[username]
if !ok {
return nil, errors.New("invalid creds")
return errors.New("invalid creds")
}
return ids, nil
return nil
}

type mockTable struct {
Expand All @@ -38,112 +37,100 @@ func TestPlainSplit_NoUser(t *testing.T) {
a := Auth{
passwd: []module.PlainAuth{
mockAuth{
db: map[string][]string{
"user1": []string{"user1a", "user1b"},
db: map[string]bool{
"user1": true,
},
},
},
}

ids, err := a.AuthPlain("user1", "aaa")
err := a.AuthPlain("user1", "aaa")
if err != nil {
t.Fatal("Unexpected error:", err)
}
if !reflect.DeepEqual(ids, []string{"user1a", "user1b"}) {
t.Fatal("Wrong ids returned:", ids)
}
}

func TestPlainSplit_NoUser_MultiPass(t *testing.T) {
a := Auth{
passwd: []module.PlainAuth{
mockAuth{
db: map[string][]string{
"user2": []string{"user2a", "user2b"},
db: map[string]bool{
"user2": true,
},
},
mockAuth{
db: map[string][]string{
"user1": []string{"user1a", "user1b"},
db: map[string]bool{
"user1": true,
},
},
},
}

ids, err := a.AuthPlain("user1", "aaa")
err := a.AuthPlain("user1", "aaa")
if err != nil {
t.Fatal("Unexpected error:", err)
}
if !reflect.DeepEqual(ids, []string{"user1a", "user1b"}) {
t.Fatal("Wrong ids returned:", ids)
}
}

func TestPlainSplit_UserPass(t *testing.T) {
a := Auth{
userTbls: []module.Table{
mockTable{
db: map[string]string{
"user1": "user2",
"user1": "",
},
},
},
passwd: []module.PlainAuth{
mockAuth{
db: map[string][]string{
"user2": []string{"user2a", "user2b"},
db: map[string]bool{
"user2": true,
},
},
mockAuth{
db: map[string][]string{
"user1": []string{"user1a", "user1b"},
db: map[string]bool{
"user1": true,
},
},
},
}

ids, err := a.AuthPlain("user1", "aaa")
err := a.AuthPlain("user1", "aaa")
if err != nil {
t.Fatal("Unexpected error:", err)
}
if !reflect.DeepEqual(ids, []string{"user2"}) {
t.Fatal("Wrong ids returned:", ids)
}
}

func TestPlainSplit_MultiUser_Pass(t *testing.T) {
a := Auth{
userTbls: []module.Table{
mockTable{
db: map[string]string{
"userWH": "user1",
"userWH": "",
},
},
mockTable{
db: map[string]string{
"user1": "user2",
"user1": "",
},
},
},
passwd: []module.PlainAuth{
mockAuth{
db: map[string][]string{
"user2": []string{"user2a", "user2b"},
db: map[string]bool{
"user2": true,
},
},
mockAuth{
db: map[string][]string{
"user1": []string{"user1a", "user1b"},
db: map[string]bool{
"user1": true,
},
},
},
}

ids, err := a.AuthPlain("user1", "aaa")
err := a.AuthPlain("user1", "aaa")
if err != nil {
t.Fatal("Unexpected error:", err)
}
if !reflect.DeepEqual(ids, []string{"user2"}) {
t.Fatal("Wrong ids returned:", ids)
}
}
Loading

0 comments on commit 55a91a3

Please # to comment.