From b202a2209e38ee1d36f16756716ca8cc7fec0823 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 14 Feb 2021 23:21:42 -0500 Subject: [PATCH 1/7] Refactor out getTLSConfig() --- main.go | 54 ++++++++++++++++++++++-------------------------------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/main.go b/main.go index e045325..85d8e30 100644 --- a/main.go +++ b/main.go @@ -188,7 +188,7 @@ func mailHandler(peer smtpd.Peer, env smtpd.Envelope) error { return nil } -func main() { +func getTLSConfig() *tls.Config { // Ciphersuites as defined in stock Go but without 3DES and RC4 // https://golang.org/src/crypto/tls/cipher_suites.go var tlsCipherSuites = []uint16{ @@ -214,6 +214,24 @@ func main() { tls.TLS_RSA_WITH_AES_256_CBC_SHA, } + if *localCert == "" || *localKey == "" { + log.Fatal("TLS certificate/key not defined in config") + } + + cert, err := tls.LoadX509KeyPair(*localCert, *localKey) + if err != nil { + log.Fatal(err) + } + + return &tls.Config{ + PreferServerCipherSuites: true, + MinVersion: tls.VersionTLS11, + CipherSuites: tlsCipherSuites, + Certificates: []tls.Certificate{cert}, + } +} + +func main() { ConfigLoad() if *versionInfo { @@ -260,24 +278,10 @@ func main() { } else if strings.HasPrefix(listeners[i], "starttls://") { listener = strings.TrimPrefix(listener, "starttls://") - if *localCert == "" || *localKey == "" { - log.Fatal("TLS certificate/key not defined in config") - } - - cert, err := tls.LoadX509KeyPair(*localCert, *localKey) - if err != nil { - log.Fatal(err) - } - - server.TLSConfig = &tls.Config{ - PreferServerCipherSuites: true, - MinVersion: tls.VersionTLS11, - CipherSuites: tlsCipherSuites, - Certificates: []tls.Certificate{cert}, - } + server.TLSConfig = getTLSConfig() server.ForceTLS = *localForceTLS - log.Printf("Listen on %s (STARTSSL) ...\n", listener) + log.Printf("Listen on %s (STARTTLS) ...\n", listener) lsnr, err := net.Listen("tcp", listener) if err != nil { log.Fatal(err) @@ -289,21 +293,7 @@ func main() { listener = strings.TrimPrefix(listener, "tls://") - if *localCert == "" || *localKey == "" { - log.Fatal("TLS certificate/key not defined in config") - } - - cert, err := tls.LoadX509KeyPair(*localCert, *localKey) - if err != nil { - log.Fatal(err) - } - - server.TLSConfig = &tls.Config{ - PreferServerCipherSuites: true, - MinVersion: tls.VersionTLS11, - CipherSuites: tlsCipherSuites, - Certificates: []tls.Certificate{cert}, - } + server.TLSConfig = getTLSConfig() log.Printf("Listen on %s (TLS) ...\n", listener) lsnr, err := tls.Listen("tcp", listener, server.TLSConfig) From fd3f513b18cb4df70ccb4b43baebfdf6b1e66ae3 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 14 Feb 2021 23:24:25 -0500 Subject: [PATCH 2/7] Don't run ListenAndServe in a goroutine Any errors returned in ListenAndServe() (e.g. port already in use) will be swallowed and not evident to the user. --- main.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index 85d8e30..afa258d 100644 --- a/main.go +++ b/main.go @@ -274,7 +274,14 @@ func main() { if strings.Index(listeners[i], "://") == -1 { log.Printf("Listen on %s ...\n", listener) - go server.ListenAndServe(listener) + + lsnr, err := net.Listen("tcp", listener) + if err != nil { + log.Fatal(err) + } + defer lsnr.Close() + + go server.Serve(lsnr) } else if strings.HasPrefix(listeners[i], "starttls://") { listener = strings.TrimPrefix(listener, "starttls://") From 4fd6bb1004eaa4b043fc138b42f29ea5d93a8408 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 14 Feb 2021 23:30:31 -0500 Subject: [PATCH 3/7] Refactor common code in listener setup --- main.go | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/main.go b/main.go index afa258d..20ffd56 100644 --- a/main.go +++ b/main.go @@ -272,16 +272,13 @@ func main() { server.Authenticator = authChecker } + var lsnr net.Listener + var err error + if strings.Index(listeners[i], "://") == -1 { log.Printf("Listen on %s ...\n", listener) - lsnr, err := net.Listen("tcp", listener) - if err != nil { - log.Fatal(err) - } - defer lsnr.Close() - - go server.Serve(lsnr) + lsnr, err = net.Listen("tcp", listener) } else if strings.HasPrefix(listeners[i], "starttls://") { listener = strings.TrimPrefix(listener, "starttls://") @@ -289,30 +286,24 @@ func main() { server.ForceTLS = *localForceTLS log.Printf("Listen on %s (STARTTLS) ...\n", listener) - lsnr, err := net.Listen("tcp", listener) - if err != nil { - log.Fatal(err) - } - defer lsnr.Close() - - go server.Serve(lsnr) + lsnr, err = net.Listen("tcp", listener) } else if strings.HasPrefix(listeners[i], "tls://") { - listener = strings.TrimPrefix(listener, "tls://") server.TLSConfig = getTLSConfig() log.Printf("Listen on %s (TLS) ...\n", listener) - lsnr, err := tls.Listen("tcp", listener, server.TLSConfig) - if err != nil { - log.Fatal(err) - } - defer lsnr.Close() - - go server.Serve(lsnr) + lsnr, err = tls.Listen("tcp", listener, server.TLSConfig) } else { log.Fatal("Unknown protocol in listener ", listener) } + + if err != nil { + log.Fatal(err) + } + defer lsnr.Close() + + go server.Serve(lsnr) } for true { From ecf830865cf82f4ce46c6fd37584dbe8074c94eb Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 14 Feb 2021 23:49:17 -0500 Subject: [PATCH 4/7] Add helpful log messages for various error cases --- main.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index 20ffd56..bced50a 100644 --- a/main.go +++ b/main.go @@ -33,6 +33,7 @@ func connectionChecker(peer smtpd.Peer) error { } } + log.Printf("Connection from peer=[%s] denied: Not in allowed_nets\n", peerIP) return smtpd.Error{Code: 421, Message: "Denied"} } @@ -84,10 +85,13 @@ func senderChecker(peer smtpd.Peer, addr string) error { if *allowedUsers != "" && peer.Username != "" { user, err := AuthFetch(peer.Username) if err != nil { + // Shouldn't happen: authChecker already validated username+password return smtpd.Error{Code: 451, Message: "Bad sender address"} } if !addrAllowed(addr, user.allowedAddresses) { + log.Printf("Mail from=<%s> not allowed for authenticated user %s (%v)\n", + addr, peer.Username, peer.Addr) return smtpd.Error{Code: 451, Message: "Bad sender address"} } } @@ -106,6 +110,8 @@ func senderChecker(peer smtpd.Peer, addr string) error { return nil } + log.Printf("Mail from=<%s> not allowed by allowed_sender pattern for peer %v\n", + addr, peer.Addr) return smtpd.Error{Code: 451, Message: "Bad sender address"} } @@ -124,13 +130,15 @@ func recipientChecker(peer smtpd.Peer, addr string) error { return nil } + log.Printf("Mail to=<%s> not allowed by allowed_recipients pattern for peer %v\n", + addr, peer.Addr) return smtpd.Error{Code: 451, Message: "Bad recipient address"} } func authChecker(peer smtpd.Peer, username string, password string) error { err := AuthCheckPassword(username, password) if err != nil { - log.Printf("Auth error: %v\n", err) + log.Printf("Auth error for peer %v: %v\n", peer.Addr, err) return smtpd.Error{Code: 535, Message: "Authentication credentials invalid"} } return nil From 7fa0eebf95a75c443398b51dc4fb66a2875d9c8a Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Mon, 15 Feb 2021 00:00:38 -0500 Subject: [PATCH 5/7] Simplify range code for setting up listeners --- main.go | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/main.go b/main.go index bced50a..a29b234 100644 --- a/main.go +++ b/main.go @@ -257,11 +257,9 @@ func main() { log.SetOutput(io.MultiWriter(os.Stdout, f)) } - listeners := strings.Split(*listen, " ") - - for i := range listeners { - listener := listeners[i] + // Create a server for each desired listen address + for _, listenAddr := range strings.Split(*listen, " ") { server := &smtpd.Server{ Hostname: *hostName, WelcomeMessage: *welcomeMsg, @@ -283,27 +281,27 @@ func main() { var lsnr net.Listener var err error - if strings.Index(listeners[i], "://") == -1 { - log.Printf("Listen on %s ...\n", listener) + if strings.Index(listenAddr, "://") == -1 { + log.Printf("Listen on %s ...\n", listenAddr) - lsnr, err = net.Listen("tcp", listener) - } else if strings.HasPrefix(listeners[i], "starttls://") { - listener = strings.TrimPrefix(listener, "starttls://") + lsnr, err = net.Listen("tcp", listenAddr) + } else if strings.HasPrefix(listenAddr, "starttls://") { + listenAddr = strings.TrimPrefix(listenAddr, "starttls://") server.TLSConfig = getTLSConfig() server.ForceTLS = *localForceTLS - log.Printf("Listen on %s (STARTTLS) ...\n", listener) - lsnr, err = net.Listen("tcp", listener) - } else if strings.HasPrefix(listeners[i], "tls://") { - listener = strings.TrimPrefix(listener, "tls://") + log.Printf("Listen on %s (STARTTLS) ...\n", listenAddr) + lsnr, err = net.Listen("tcp", listenAddr) + } else if strings.HasPrefix(listenAddr, "tls://") { + listenAddr = strings.TrimPrefix(listenAddr, "tls://") server.TLSConfig = getTLSConfig() - log.Printf("Listen on %s (TLS) ...\n", listener) - lsnr, err = tls.Listen("tcp", listener, server.TLSConfig) + log.Printf("Listen on %s (TLS) ...\n", listenAddr) + lsnr, err = tls.Listen("tcp", listenAddr, server.TLSConfig) } else { - log.Fatal("Unknown protocol in listener ", listener) + log.Fatal("Unknown protocol in listen address ", listenAddr) } if err != nil { From 70dfe6b128b8152464cfe03c20a3dd96f6690bca Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Mon, 15 Feb 2021 00:01:02 -0500 Subject: [PATCH 6/7] Only call AuthLoadFile() once at startup --- main.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/main.go b/main.go index a29b234..f543efb 100644 --- a/main.go +++ b/main.go @@ -257,6 +257,13 @@ func main() { log.SetOutput(io.MultiWriter(os.Stdout, f)) } + // Load allowed users file + if *allowedUsers != "" { + err := AuthLoadFile(*allowedUsers) + if err != nil { + log.Fatalf("Authentication file: %s\n", err) + } + } // Create a server for each desired listen address for _, listenAddr := range strings.Split(*listen, " ") { @@ -270,11 +277,6 @@ func main() { } if *allowedUsers != "" { - err := AuthLoadFile(*allowedUsers) - if err != nil { - log.Fatalf("Authentication file: %s\n", err) - } - server.Authenticator = authChecker } From 009ae8f73a0fb392799452f0181f9befbef55986 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Mon, 15 Feb 2021 00:18:15 -0500 Subject: [PATCH 7/7] hasher: Check number of arguments This makes for a better user experience than a go segfault. --- cmd/hasher.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/hasher.go b/cmd/hasher.go index d3c3459..ee8d6bb 100644 --- a/cmd/hasher.go +++ b/cmd/hasher.go @@ -8,11 +8,15 @@ import ( ) func main() { + if len(os.Args) != 2 { + fmt.Fprintln(os.Stderr, "Usage: hasher PASSWORD") + os.Exit(1) + } password := os.Args[1] hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) if err != nil { - fmt.Println("Error generating hash: %s", err) + fmt.Fprintln(os.Stderr, "Error generating hash: %s", err) } fmt.Println(string(hash)) }