From 4036213dd5d39a10fbce8989ff307a12fbc40c31 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sat, 13 Mar 2021 02:38:13 -0500 Subject: [PATCH 1/4] Simplify peerIP determination in connectionChecker() peerIP = net.ParseIP(addr.IP.String()) can be simplified to just: peerIP = addr.IP but we can also skip the safe cast since we know the net.Addr will always be net.TCPAddr because we only have TCP listeners. --- main.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/main.go b/main.go index de436c8..7837529 100644 --- a/main.go +++ b/main.go @@ -17,14 +17,8 @@ import ( ) func connectionChecker(peer smtpd.Peer) error { - var peerIP net.IP - if addr, ok := peer.Addr.(*net.TCPAddr); ok { - peerIP = net.ParseIP(addr.IP.String()) - } else { - log.WithField("ip", addr.IP). - Warn("failed to parse IP") - return smtpd.Error{Code: 421, Message: "Denied"} - } + // This can't panic because we only have TCP listeners + peerIP := peer.Addr.(*net.TCPAddr).IP nets := strings.Split(*allowedNets, " ") From ef3f9c8ea0eea57c5e36d91c482cf1da3bbab288 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sat, 13 Mar 2021 02:42:30 -0500 Subject: [PATCH 2/4] Move parsing of "allowed_nets" out to ConfigLoad() This has several benefits: - Configuration errors are caught at startup rather than upon a connection - connectionChecker() has less work to do for each connection --- config.go | 20 +++++++++++++++++++- main.go | 5 +---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/config.go b/config.go index d7335bd..eabaf37 100644 --- a/config.go +++ b/config.go @@ -2,6 +2,7 @@ package main import ( "flag" + "net" "github.com/vharitonsky/iniflags" ) @@ -21,7 +22,8 @@ var ( localCert = flag.String("local_cert", "", "SSL certificate for STARTTLS/TLS") localKey = flag.String("local_key", "", "SSL private key for STARTTLS/TLS") localForceTLS = flag.Bool("local_forcetls", false, "Force STARTTLS (needs local_cert and local_key)") - allowedNets = flag.String("allowed_nets", "127.0.0.1/8 ::1/128", "Networks allowed to send mails") + allowedNetsStr = flag.String("allowed_nets", "127.0.0.1/8 ::1/128", "Networks allowed to send mails") + allowedNets = []*net.IPNet{} allowedSender = flag.String("allowed_sender", "", "Regular expression for valid FROM EMail addresses") allowedRecipients = flag.String("allowed_recipients", "", "Regular expression for valid TO EMail addresses") allowedUsers = flag.String("allowed_users", "", "Path to file with valid users/passwords") @@ -33,6 +35,20 @@ var ( versionInfo = flag.Bool("version", false, "Show version information") ) + +func setupAllowedNetworks() { + for _, netstr := range splitstr(*allowedNetsStr, ' ') { + _, allowedNet, err := net.ParseCIDR(netstr) + if err != nil { + log.WithField("netstr", netstr). + WithError(err). + Fatal("Invalid CIDR notation in allowed_nets") + } + + allowedNets = append(allowedNets, allowedNet) + } +} + func ConfigLoad() { iniflags.Parse() @@ -42,4 +58,6 @@ func ConfigLoad() { if (*remoteHost == "") { log.Warn("remote_host not set; mail will not be forwarded!") } + + setupAllowedNetworks() } diff --git a/main.go b/main.go index 7837529..6289bbc 100644 --- a/main.go +++ b/main.go @@ -20,11 +20,8 @@ func connectionChecker(peer smtpd.Peer) error { // This can't panic because we only have TCP listeners peerIP := peer.Addr.(*net.TCPAddr).IP - nets := strings.Split(*allowedNets, " ") - - for i := range nets { - _, allowedNet, _ := net.ParseCIDR(nets[i]) + for _, allowedNet := range allowedNets { if allowedNet.Contains(peerIP) { return nil } From 0503c12ccd914fe6a518bdc235b06cbf5a434fcc Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sat, 13 Mar 2021 02:43:08 -0500 Subject: [PATCH 3/4] Allow "allowed_nets" to be empty, meaning any network is allowed --- main.go | 4 ++++ smtprelay.ini | 1 + 2 files changed, 5 insertions(+) diff --git a/main.go b/main.go index 6289bbc..cbd5993 100644 --- a/main.go +++ b/main.go @@ -20,6 +20,10 @@ func connectionChecker(peer smtpd.Peer) error { // This can't panic because we only have TCP listeners peerIP := peer.Addr.(*net.TCPAddr).IP + if len(allowedNets) == 0 { + // Special case: empty string means allow everything + return nil + } for _, allowedNet := range allowedNets { if allowedNet.Contains(peerIP) { diff --git a/smtprelay.ini b/smtprelay.ini index c1e90e8..91e1cc6 100644 --- a/smtprelay.ini +++ b/smtprelay.ini @@ -31,6 +31,7 @@ ;local_forcetls = false ; Networks that are allowed to send mails to us +; Defaults to localhost. If set to "", then any address is allowed. ;allowed_nets = 127.0.0.1/8 ::1/128 ; Regular expression for valid FROM EMail addresses From 918df65a3ad06ad7c50b95b69c7ecfe56c1b14fe Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sat, 13 Mar 2021 02:45:44 -0500 Subject: [PATCH 4/4] Require that networks in allowed_nets are networks and not hosts --- config.go | 14 ++++++++++++-- smtprelay.ini | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/config.go b/config.go index eabaf37..36f32e5 100644 --- a/config.go +++ b/config.go @@ -5,6 +5,7 @@ import ( "net" "github.com/vharitonsky/iniflags" + "github.com/sirupsen/logrus" ) var ( @@ -22,7 +23,7 @@ var ( localCert = flag.String("local_cert", "", "SSL certificate for STARTTLS/TLS") localKey = flag.String("local_key", "", "SSL private key for STARTTLS/TLS") localForceTLS = flag.Bool("local_forcetls", false, "Force STARTTLS (needs local_cert and local_key)") - allowedNetsStr = flag.String("allowed_nets", "127.0.0.1/8 ::1/128", "Networks allowed to send mails") + allowedNetsStr = flag.String("allowed_nets", "127.0.0.0/8 ::1/128", "Networks allowed to send mails") allowedNets = []*net.IPNet{} allowedSender = flag.String("allowed_sender", "", "Regular expression for valid FROM EMail addresses") allowedRecipients = flag.String("allowed_recipients", "", "Regular expression for valid TO EMail addresses") @@ -38,13 +39,22 @@ var ( func setupAllowedNetworks() { for _, netstr := range splitstr(*allowedNetsStr, ' ') { - _, allowedNet, err := net.ParseCIDR(netstr) + baseIP, allowedNet, err := net.ParseCIDR(netstr) if err != nil { log.WithField("netstr", netstr). WithError(err). Fatal("Invalid CIDR notation in allowed_nets") } + // Reject any network specification where any host bits are set, + // meaning the address refers to a host and not a network. + if !allowedNet.IP.Equal(baseIP) { + log.WithFields(logrus.Fields{ + "given_net": netstr, + "proper_net": allowedNet, + }).Fatal("Invalid network in allowed_nets (host bits set)") + } + allowedNets = append(allowedNets, allowedNet) } } diff --git a/smtprelay.ini b/smtprelay.ini index 91e1cc6..0b97399 100644 --- a/smtprelay.ini +++ b/smtprelay.ini @@ -32,7 +32,7 @@ ; Networks that are allowed to send mails to us ; Defaults to localhost. If set to "", then any address is allowed. -;allowed_nets = 127.0.0.1/8 ::1/128 +;allowed_nets = 127.0.0.0/8 ::1/128 ; Regular expression for valid FROM EMail addresses ; Example: ^(.*)@localhost.localdomain$