Revert authorization/authentication split

Authentication provider module is responsible only for authentication.
Nothing more. Access control (authorization) should be kept separate.
This commit is contained in:
fox.cpp 2020-02-28 01:38:40 +03:00
parent 3092ca0ca5
commit 55a91a37b7
No known key found for this signature in database
GPG key ID: E76D97CCEDE90B6C
14 changed files with 130 additions and 288 deletions

View file

@ -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)
@ -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
@ -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
}
```
@ -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
@ -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
@ -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

View file

@ -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() {

View file

@ -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() {

View file

@ -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 {
@ -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
}
}
if len(identities) == 0 {
return nil, errors.New("plain_separate: unknown credentials")
}
}
var (
lastErr error
ok bool
)
for _, pass := range a.passwd {
passIDs, err := pass.AuthPlain(username, password)
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 tblOk {
ok = true
break
}
}
if !ok {
return errors.New("user not found in tables")
}
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() {

View file

@ -2,7 +2,6 @@ package plain_separate
import (
"errors"
"reflect"
"testing"
"github.com/emersion/go-sasl"
@ -10,19 +9,19 @@ import (
)
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 {
@ -38,45 +37,39 @@ 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) {
@ -84,31 +77,28 @@ func TestPlainSplit_UserPass(t *testing.T) {
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) {
@ -116,34 +106,31 @@ func TestPlainSplit_MultiUser_Pass(t *testing.T) {
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)
}
}

View file

@ -10,7 +10,6 @@ import (
modconfig "github.com/foxcpp/maddy/internal/config/module"
"github.com/foxcpp/maddy/internal/log"
"github.com/foxcpp/maddy/internal/module"
"golang.org/x/text/secure/precis"
)
var (
@ -39,85 +38,52 @@ func (s *SASLAuth) SASLMechanisms() []string {
return mechs
}
func (s *SASLAuth) AuthPlain(username, password string) ([]string, error) {
func (s *SASLAuth) AuthPlain(username, password string) error {
if len(s.Plain) == 0 {
return nil, ErrUnsupportedMech
return ErrUnsupportedMech
}
var lastErr error
accounts := make([]string, 0, 1)
for _, p := range s.Plain {
pAccs, err := p.AuthPlain(username, password)
err := p.AuthPlain(username, password)
if err == nil {
return nil
}
if err != nil {
lastErr = err
continue
}
if s.OnlyFirstID {
return pAccs, nil
}
accounts = append(accounts, pAccs...)
}
if len(accounts) == 0 {
return nil, fmt.Errorf("no auth. provider accepted creds, last err: %w", lastErr)
}
return accounts, nil
}
func filterIdentity(accounts []string, identity string) ([]string, error) {
if identity == "" {
return accounts, nil
}
matchFound := false
for _, acc := range accounts {
if precis.UsernameCaseMapped.Compare(acc, identity) {
accounts = []string{identity}
matchFound = true
break
}
}
if !matchFound {
return nil, errors.New("auth: invalid credentials")
}
return accounts, nil
return fmt.Errorf("no auth. provider accepted creds, last err: %w", lastErr)
}
// CreateSASL creates the sasl.Server instance for the corresponding mechanism.
//
// successCb will be called with the slice of authorization identities
// associated with credentials used.
// If it fails - authentication will fail too.
func (s *SASLAuth) CreateSASL(mech string, remoteAddr net.Addr, successCb func([]string) error) sasl.Server {
func (s *SASLAuth) CreateSASL(mech string, remoteAddr net.Addr, successCb func(identity string) error) sasl.Server {
switch mech {
case sasl.Plain:
return sasl.NewPlainServer(func(identity, username, password string) error {
accounts, err := s.AuthPlain(username, password)
if err != nil {
s.Log.Error("authentication failed", err, "username", username, "identity", identity, "src_ip", remoteAddr)
return errors.New("auth: invalid credentials")
}
if len(accounts) == 0 {
accounts = []string{username}
}
accounts, err = filterIdentity(accounts, identity)
if err != nil {
s.Log.Error("not authorized", err, "username", username, "identity", identity, "src_ip", remoteAddr)
return errors.New("auth: invalid credentials")
if identity == "" {
identity = username
}
return successCb(accounts)
})
case sasl.Login:
return sasl.NewLoginServer(func(username, password string) error {
accounts, err := s.AuthPlain(username, password)
err := s.AuthPlain(username, password)
if err != nil {
s.Log.Error("authentication failed", err, "username", username, "src_ip", remoteAddr)
return errors.New("auth: invalid credentials")
}
return successCb(accounts)
return successCb(identity)
})
case sasl.Login:
return sasl.NewLoginServer(func(username, password string) error {
err := s.AuthPlain(username, password)
if err != nil {
s.Log.Error("authentication failed", err, "username", username, "src_ip", remoteAddr)
return errors.New("auth: invalid credentials")
}
return successCb(username)
})
}
return FailingSASLServ{Err: ErrUnsupportedMech}
@ -126,24 +92,20 @@ func (s *SASLAuth) CreateSASL(mech string, remoteAddr net.Addr, successCb func([
// AddProvider adds the SASL authentication provider to its mapping by parsing
// the 'auth' configuration directive.
func (s *SASLAuth) AddProvider(m *config.Map, node *config.Node) error {
mod, err := modconfig.SASLAuthDirective(m, node)
if err != nil {
var any interface{}
if err := modconfig.ModuleFromNode(node.Args, node, m.Globals, &any); err != nil {
return err
}
saslAuth := mod.(module.SASLProvider)
for _, mech := range saslAuth.SASLMechanisms() {
switch mech {
case sasl.Login, sasl.Plain:
plainAuth, ok := saslAuth.(module.PlainAuth)
if !ok {
return m.MatchErr("auth: provider does not implement PlainAuth even though it reports PLAIN/LOGIN mechanism")
}
s.Plain = append(s.Plain, plainAuth)
default:
return m.MatchErr("auth: unknown SASL mechanism")
}
hasAny := false
if plainAuth, ok := any.(module.PlainAuth); ok {
s.Plain = append(s.Plain, plainAuth)
hasAny = true
}
if !hasAny {
return m.MatchErr("auth: specified module does not provide any SASL mechanism")
}
return nil
}

View file

@ -3,28 +3,22 @@ package auth
import (
"errors"
"net"
"reflect"
"testing"
"github.com/emersion/go-sasl"
"github.com/foxcpp/maddy/internal/module"
"github.com/foxcpp/maddy/internal/testutils"
)
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
}
func TestCreateSASL(t *testing.T) {
@ -32,15 +26,15 @@ func TestCreateSASL(t *testing.T) {
Log: testutils.Logger(t, "saslauth"),
Plain: []module.PlainAuth{
&mockAuth{
db: map[string][]string{
"user1": []string{"user1a", "user1b"},
db: map[string]bool{
"user1": true,
},
},
},
}
t.Run("XWHATEVER", func(t *testing.T) {
srv := a.CreateSASL("XWHATEVER", &net.TCPAddr{}, func([]string) error { return nil })
srv := a.CreateSASL("XWHATEVER", &net.TCPAddr{}, func(string) error { return nil })
_, _, err := srv.Next([]byte(""))
if err == nil {
t.Error("No error for XWHATEVER use")
@ -48,9 +42,10 @@ func TestCreateSASL(t *testing.T) {
})
t.Run("PLAIN", func(t *testing.T) {
var ids []string
srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(passed []string) error {
ids = passed
srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(id string) error {
if id != "user1" {
t.Fatal("Wrong auth. identities passed to callback:", id)
}
return nil
})
@ -58,15 +53,13 @@ func TestCreateSASL(t *testing.T) {
if err != nil {
t.Error("Unexpected error:", err)
}
if !reflect.DeepEqual(ids, []string{"user1a", "user1b"}) {
t.Error("Wrong auth. identities passed to callback:", ids)
}
})
t.Run("PLAIN with autorization identity", func(t *testing.T) {
var ids []string
srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(passed []string) error {
ids = passed
t.Run("PLAIN with authorization identity", func(t *testing.T) {
srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(id string) error {
if id != "user1a" {
t.Fatal("Wrong authorization identity passed:", id)
}
return nil
})
@ -74,30 +67,5 @@ func TestCreateSASL(t *testing.T) {
if err != nil {
t.Error("Unexpected error:", err)
}
if !reflect.DeepEqual(ids, []string{"user1a"}) {
t.Error("Wrong auth. identities passed to callback:", ids)
}
})
t.Run("PLAIN with wrong authorization identity", func(t *testing.T) {
srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(passed []string) error {
return nil
})
_, _, err := srv.Next([]byte("user1c\x00user1\x00aa"))
if err == nil {
t.Error("Next should fail")
}
})
t.Run("PLAIN with wrong authentication identity", func(t *testing.T) {
srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(passed []string) error {
return nil
})
_, _, err := srv.Next([]byte("\x00user2\x00aa"))
if err == nil {
t.Error("Next should fail")
}
})
}

View file

@ -66,32 +66,32 @@ 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 {
return []string{username}, external.AuthUsingHelper(a.helperPath, username, password)
return external.AuthUsingHelper(a.helperPath, username, password)
}
ent, err := Lookup(username)
if err != nil {
return nil, err
return err
}
if !ent.IsAccountValid() {
return nil, fmt.Errorf("shadow: account is expired")
return fmt.Errorf("shadow: account is expired")
}
if !ent.IsPasswordValid() {
return nil, fmt.Errorf("shadow: password is expired")
return fmt.Errorf("shadow: password is expired")
}
if err := ent.VerifyPassword(password); err != nil {
if err == ErrWrongPassword {
return nil, module.ErrUnknownCredentials
return module.ErrUnknownCredentials
}
return nil, err
return err
}
return []string{username}, nil
return nil
}
func init() {

View file

@ -51,14 +51,6 @@ func StorageDirective(m *config.Map, node *config.Node) (interface{}, error) {
return backend, nil
}
func SASLAuthDirective(m *config.Map, node *config.Node) (interface{}, error) {
var provider module.SASLProvider
if err := ModuleFromNode(node.Args, node, m.Globals, &provider); err != nil {
return nil, err
}
return provider, nil
}
func TableDirective(m *config.Map, node *config.Node) (interface{}, error) {
var tbl module.Table
if err := ModuleFromNode(node.Args, node, m.Globals, &tbl); err != nil {

View file

@ -123,8 +123,8 @@ func (endp *Endpoint) Init(cfg *config.Map) error {
for _, mech := range endp.saslAuth.SASLMechanisms() {
endp.serv.EnableAuth(mech, func(c imapserver.Conn) sasl.Server {
return endp.saslAuth.CreateSASL(mech, c.Info().RemoteAddr, func(ids []string) error {
return endp.openAccount(c, ids)
return endp.saslAuth.CreateSASL(mech, c.Info().RemoteAddr, func(identity string) error {
return endp.openAccount(c, identity)
})
})
}
@ -199,8 +199,8 @@ func (endp *Endpoint) Close() error {
return nil
}
func (endp *Endpoint) openAccount(c imapserver.Conn, identities []string) error {
u, err := endp.Store.GetOrCreateUser(identities[0])
func (endp *Endpoint) openAccount(c imapserver.Conn, identity string) error {
u, err := endp.Store.GetOrCreateUser(identity)
if err != nil {
return err
}
@ -211,14 +211,12 @@ func (endp *Endpoint) openAccount(c imapserver.Conn, identities []string) error
}
func (endp *Endpoint) Login(connInfo *imap.ConnInfo, username, password string) (imapbackend.User, error) {
_, err := endp.saslAuth.AuthPlain(username, password)
err := endp.saslAuth.AuthPlain(username, password)
if err != nil {
endp.Log.Error("authentication failed", err, "username", username, "src_ip", connInfo.RemoteAddr)
return nil, imapbackend.ErrInvalidCredentials
}
// TODO: Wrap GetOrCreateUser and possibly implement INBOXES extension
// (though it is draft 00 for quite some time so it likely has no future).
return endp.Store.GetOrCreateUser(username)
}

View file

@ -269,8 +269,8 @@ func (endp *Endpoint) setConfig(cfg *config.Map) error {
return auth.FailingSASLServ{Err: endp.wrapErr("", true, err)}
}
return endp.saslAuth.CreateSASL(mech, state.RemoteAddr, func(ids []string) error {
c.SetSession(endp.newSession(false, ids[0], "", &state))
return endp.saslAuth.CreateSASL(mech, state.RemoteAddr, func(id string) error {
c.SetSession(endp.newSession(false, id, "", &state))
return nil
})
})
@ -332,14 +332,13 @@ func (endp *Endpoint) Login(state *smtp.ConnectionState, username, password stri
return nil, endp.wrapErr("", true, err)
}
_, err := endp.saslAuth.AuthPlain(username, password)
err := endp.saslAuth.AuthPlain(username, password)
if err != nil {
// TODO: Update fail2ban filters.
endp.Log.Error("authentication failed", err, "username", username, "src_ip", state.RemoteAddr)
return nil, errors.New("Invalid credentials")
}
// TODO: Pass valid identifies to SMTP pipeline.
return endp.newSession(false, username, password, state), nil
}

View file

@ -12,23 +12,5 @@ var (
// PlainAuth is the interface implemented by modules providing authentication using
// username:password pairs.
type PlainAuth interface {
AuthPlain(username, password string) ([]string, error)
}
// SASLProvider is the interface implemented by modules and used by protocol
// endpoints that rely on SASL framework for user authentication.
//
// This actual interface is only used to indicate that the module is a
// SASL-compatible auth. provider. For each unique value returned by
// SASLMechanisms, the module object should also implement the coresponding
// mechanism-specific interface.
//
// *Rationale*: There is no single generic interface that would handle any SASL
// mechanism while permiting the use of a credentials set estabilished once with
// multiple auth. providers at once.
//
// Per-mechanism interfaces:
// - PLAIN => PlainAuth
type SASLProvider interface {
SASLMechanisms() []string
AuthPlain(username, password string) error
}

View file

@ -4,7 +4,6 @@ import (
"context"
"github.com/emersion/go-message/textproto"
"github.com/emersion/go-sasl"
"github.com/foxcpp/maddy/internal/buffer"
"github.com/foxcpp/maddy/internal/config"
)
@ -16,12 +15,8 @@ import (
// and the actual server code (but the latter is kinda pointless).
type Dummy struct{ instName string }
func (d *Dummy) SASLMechanisms() []string {
return []string{sasl.Plain, sasl.Login}
}
func (d *Dummy) AuthPlain(username, _ string) ([]string, error) {
return []string{username}, nil
func (d *Dummy) AuthPlain(username, _ string) error {
return nil
}
func (d *Dummy) Name() string {

View file

@ -420,25 +420,25 @@ func prepareUsername(username string) (string, error) {
return mbox + "@" + domain, nil
}
func (store *Storage) AuthPlain(username, password string) ([]string, error) {
func (store *Storage) AuthPlain(username, password string) error {
// TODO: Pass session context there.
defer trace.StartRegion(context.Background(), "sql/AuthPlain").End()
accountName, err := prepareUsername(username)
if err != nil {
return nil, err
return err
}
password, err = precis.OpaqueString.CompareKey(password)
if err != nil {
return nil, err
return err
}
// TODO: Make go-imap-sql CheckPlain return an actual error.
if !store.Back.CheckPlain(accountName, password) {
return nil, module.ErrUnknownCredentials
return module.ErrUnknownCredentials
}
return []string{username}, nil
return nil
}
func (store *Storage) GetOrCreateUser(username string) (backend.User, error) {