From a45c7090c4a7a8c5fd56576ff9c2a3d02cb15dec Mon Sep 17 00:00:00 2001 From: "fox.cpp" Date: Thu, 27 Feb 2020 01:22:47 +0300 Subject: [PATCH] Improve auth. provider interface The authentication provider can now provide multiple authorization identities associated with credentials. Protocols that support that (e.g. JMAP, SASL) can let the client select the wanted identity. --- internal/auth/external/externalauth.go | 7 +++--- internal/auth/external/helperauth.go | 30 +++++++++++--------------- internal/auth/pam/module.go | 15 ++++++------- internal/auth/shadow/module.go | 26 +++++++++------------- internal/config/module/auth.go | 2 +- internal/endpoint/imap/imap.go | 9 +++++--- internal/endpoint/smtp/smtp.go | 9 +++++--- internal/endpoint/smtp/smtp_test.go | 2 +- internal/module/auth.go | 15 ++++++++++--- internal/module/dummy.go | 6 +++--- internal/storage/sql/sql.go | 16 ++++++++------ 11 files changed, 72 insertions(+), 65 deletions(-) diff --git a/internal/auth/external/externalauth.go b/internal/auth/external/externalauth.go index ae7e645..4a34e33 100644 --- a/internal/auth/external/externalauth.go +++ b/internal/auth/external/externalauth.go @@ -71,13 +71,14 @@ func (ea *ExternalAuth) Init(cfg *config.Map) error { return nil } -func (ea *ExternalAuth) CheckPlain(username, password string) bool { +func (ea *ExternalAuth) AuthPlain(username, password string) ([]string, error) { accountName, ok := auth.CheckDomainAuth(username, ea.perDomain, ea.domains) if !ok { - return false + return nil, module.ErrUnknownCredentials } - return AuthUsingHelper(ea.Log, ea.helperPath, accountName, password) + // TODO: Extend process protocol to support multiple authorization identities. + return []string{username}, AuthUsingHelper(ea.helperPath, accountName, password) } func init() { diff --git a/internal/auth/external/helperauth.go b/internal/auth/external/helperauth.go index a558e8d..edc55c7 100644 --- a/internal/auth/external/helperauth.go +++ b/internal/auth/external/helperauth.go @@ -1,44 +1,38 @@ package external import ( + "fmt" "io" "os/exec" - "strings" - "github.com/foxcpp/maddy/internal/log" + "github.com/foxcpp/maddy/internal/module" ) -func AuthUsingHelper(l log.Logger, binaryPath, accountName, password string) bool { +func AuthUsingHelper(binaryPath, accountName, password string) error { cmd := exec.Command(binaryPath) stdin, err := cmd.StdinPipe() if err != nil { - l.Println("failed to obtain stdin pipe for helper process:", err) - return false + return fmt.Errorf("helperauth: stdin init: %w", err) } if err := cmd.Start(); err != nil { - l.Println("failed to start helper process:", err) - return false + return fmt.Errorf("helperauth: process start: %w", err) } if _, err := io.WriteString(stdin, accountName+"\n"); err != nil { - l.Println("failed to write stdin of helper process:", err) - return false + return fmt.Errorf("helperauth: stdin write: %w", err) } if _, err := io.WriteString(stdin, password+"\n"); err != nil { - l.Println("failed to write stdin of helper process:", err) - return false + return fmt.Errorf("helperauth: stdin write: %w", err) } if err := cmd.Wait(); err != nil { - l.Debugln(err) if exitErr, ok := err.(*exec.ExitError); ok { // Exit code 1 is for authentication failure. - // Exit code 2 is for other errors. - if exitErr.ExitCode() == 2 { - l.Println(strings.TrimSpace(string(exitErr.Stderr))) + if exitErr.ExitCode() != 1 { + return fmt.Errorf("helperauth: %w: %v", err, string(exitErr.Stderr)) } + return module.ErrUnknownCredentials } else { - l.Println("failed to wait for helper process:", err) + return fmt.Errorf("helperauth: process wait: %w", err) } - return false } - return true + return nil } diff --git a/internal/auth/pam/module.go b/internal/auth/pam/module.go index 167ad15..84d8e1e 100644 --- a/internal/auth/pam/module.go +++ b/internal/auth/pam/module.go @@ -61,29 +61,28 @@ func (a *Auth) Init(cfg *config.Map) error { return nil } -func (a *Auth) CheckPlain(username, password string) bool { +func (a *Auth) AuthPlain(username, password string) ([]string, error) { var accountName string if a.expectAddress { var err error accountName, _, err = address.Split(username) if err != nil { - return false + return nil, err } } else { accountName = username } if a.useHelper { - return external.AuthUsingHelper(a.Log, a.helperPath, accountName, password) + if err := external.AuthUsingHelper(a.helperPath, accountName, password); err != nil { + return nil, err + } } err := runPAMAuth(accountName, password) if err != nil { - if err == ErrInvalidCredentials { - a.Log.Println(err) - } - return false + return nil, err } - return true + return []string{username}, nil } func init() { diff --git a/internal/auth/shadow/module.go b/internal/auth/shadow/module.go index ddac037..78dce31 100644 --- a/internal/auth/shadow/module.go +++ b/internal/auth/shadow/module.go @@ -67,43 +67,37 @@ func (a *Auth) Init(cfg *config.Map) error { return nil } -func (a *Auth) CheckPlain(username, password string) bool { +func (a *Auth) AuthPlain(username, password string) ([]string, error) { accountName, _, err := address.Split(username) if err != nil { - return false + return nil, err } if a.useHelper { - return external.AuthUsingHelper(a.Log, a.helperPath, accountName, password) + return []string{username}, external.AuthUsingHelper(a.helperPath, accountName, password) } ent, err := Lookup(accountName) if err != nil { - if err != ErrNoSuchUser { - a.Log.Error("lookup error", err, "username", username) - } - return false + return nil, err } if !ent.IsAccountValid() { - a.Log.Msg("account is expired", "username", username) - return false + return nil, fmt.Errorf("shadow: account is expired") } if !ent.IsPasswordValid() { - a.Log.Msg("password is expired", "username", username) - return false + return nil, fmt.Errorf("shadow: password is expired") } if err := ent.VerifyPassword(password); err != nil { - if err != ErrWrongPassword { - a.Log.Printf("%v", err) + if err == ErrWrongPassword { + return nil, module.ErrUnknownCredentials } - a.Log.Msg("password verification failed", "username", username) - return false + return nil, err } - return true + return []string{username}, nil } func init() { diff --git a/internal/config/module/auth.go b/internal/config/module/auth.go index 81b871e..2812d50 100644 --- a/internal/config/module/auth.go +++ b/internal/config/module/auth.go @@ -6,7 +6,7 @@ import ( ) func AuthDirective(m *config.Map, node *config.Node) (interface{}, error) { - var provider module.AuthProvider + var provider module.PlainAuth if err := ModuleFromNode(node.Args, node, m.Globals, &provider); err != nil { return nil, err } diff --git a/internal/endpoint/imap/imap.go b/internal/endpoint/imap/imap.go index 82039d2..091b9e9 100644 --- a/internal/endpoint/imap/imap.go +++ b/internal/endpoint/imap/imap.go @@ -32,7 +32,7 @@ type Endpoint struct { addrs []string serv *imapserver.Server listeners []net.Listener - Auth module.AuthProvider + Auth module.PlainAuth Store module.Storage updater imapbackend.BackendUpdater @@ -184,11 +184,14 @@ func (endp *Endpoint) Close() error { } func (endp *Endpoint) Login(connInfo *imap.ConnInfo, username, password string) (imapbackend.User, error) { - if !endp.Auth.CheckPlain(username, password) { - endp.Log.Msg("authentication failed", "username", username, "src_ip", connInfo.RemoteAddr) + _, err := endp.Auth.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) } diff --git a/internal/endpoint/smtp/smtp.go b/internal/endpoint/smtp/smtp.go index 412fd98..afb2d01 100644 --- a/internal/endpoint/smtp/smtp.go +++ b/internal/endpoint/smtp/smtp.go @@ -519,7 +519,7 @@ func (endp *Endpoint) wrapErr(msgId string, mangleUTF8 bool, err error) error { type Endpoint struct { hostname string - Auth module.AuthProvider + Auth module.PlainAuth serv *smtp.Server name string addrs []string @@ -803,11 +803,14 @@ func (endp *Endpoint) Login(state *smtp.ConnectionState, username, password stri return nil, endp.wrapErr("", true, err) } - if !endp.Auth.CheckPlain(username, password) { - endp.Log.Msg("authentication failed", "username", username, "src_ip", state.RemoteAddr) + _, err := endp.Auth.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 } diff --git a/internal/endpoint/smtp/smtp_test.go b/internal/endpoint/smtp/smtp_test.go index c72d65d..a9995cb 100644 --- a/internal/endpoint/smtp/smtp_test.go +++ b/internal/endpoint/smtp/smtp_test.go @@ -27,7 +27,7 @@ const testMsg = "From: \r\n" + "\r\n" + "foobar\r\n" -func testEndpoint(t *testing.T, modName string, auth module.AuthProvider, tgt module.DeliveryTarget, checks []module.Check, cfg []config.Node) *Endpoint { +func testEndpoint(t *testing.T, modName string, auth module.PlainAuth, tgt module.DeliveryTarget, checks []module.Check, cfg []config.Node) *Endpoint { t.Helper() mod, err := New(modName, []string{"tcp://127.0.0.1:" + testPort}) diff --git a/internal/module/auth.go b/internal/module/auth.go index c0bf408..0e22ecb 100644 --- a/internal/module/auth.go +++ b/internal/module/auth.go @@ -1,7 +1,16 @@ package module -// AuthProvider is the interface implemented by modules providing authentication using +import "errors" + +var ( + // ErrUnknownCredentials should be returned by auth. provider if supplied + // credentials are valid for it but are not recognized (e.g. not found in + // used DB). + ErrUnknownCredentials = errors.New("unknown credentials") +) + +// PlainAuth is the interface implemented by modules providing authentication using // username:password pairs. -type AuthProvider interface { - CheckPlain(username, password string) bool +type PlainAuth interface { + AuthPlain(username, password string) ([]string, error) } diff --git a/internal/module/dummy.go b/internal/module/dummy.go index d7e4034..dc31668 100644 --- a/internal/module/dummy.go +++ b/internal/module/dummy.go @@ -8,15 +8,15 @@ import ( "github.com/foxcpp/maddy/internal/config" ) -// Dummy is a struct that implements AuthProvider and DeliveryTarget +// Dummy is a struct that implements PlainAuth and DeliveryTarget // interfaces but does nothing. Useful for testing. // // It is always registered under the 'dummy' name and can be used in both tests // and the actual server code (but the latter is kinda pointless). type Dummy struct{ instName string } -func (d *Dummy) CheckPlain(_, _ string) bool { - return true +func (d *Dummy) AuthPlain(username, _ string) ([]string, error) { + return []string{username}, nil } func (d *Dummy) Name() string { diff --git a/internal/storage/sql/sql.go b/internal/storage/sql/sql.go index a917080..435ff3b 100644 --- a/internal/storage/sql/sql.go +++ b/internal/storage/sql/sql.go @@ -3,7 +3,7 @@ // // Interfaces implemented: // - module.StorageBackend -// - module.AuthProvider +// - module.PlainAuth // - module.DeliveryTarget package sql @@ -420,21 +420,25 @@ func prepareUsername(username string) (string, error) { return mbox + "@" + domain, nil } -func (store *Storage) CheckPlain(username, password string) bool { +func (store *Storage) AuthPlain(username, password string) ([]string, error) { // TODO: Pass session context there. - defer trace.StartRegion(context.Background(), "sql/CheckPlain").End() + defer trace.StartRegion(context.Background(), "sql/AuthPlain").End() accountName, err := prepareUsername(username) if err != nil { - return false + return nil, err } password, err = precis.OpaqueString.CompareKey(password) if err != nil { - return false + return nil, err } - return store.Back.CheckPlain(accountName, password) + // TODO: Make go-imap-sql CheckPlain return an actual error. + if !store.Back.CheckPlain(accountName, password) { + return nil, module.ErrUnknownCredentials + } + return []string{username}, nil } func (store *Storage) GetOrCreateUser(username string) (backend.User, error) {