Merge pull request #694 from slingamn/issue688_forward_confirmed_dns.4

#688: forward-confirmed reverse dns

irc/client.go

@@ -259,11 +259,10 @@ func (server *Server) RunClient(conn clientConn, proxyLine string) {
 		// cover up details of the tor proxying infrastructure (not a user privacy concern,
 		// but a hardening measure):
 		session.proxiedIP = utils.IPv4LoopbackAddress
+		client.proxiedIP = session.proxiedIP
 		session.rawHostname = config.Server.TorListeners.Vhost
+		client.rawHostname = session.rawHostname
 	} else {
-		// set the hostname for this client (may be overridden later by PROXY or WEBIRC)
-		session.rawHostname = utils.LookupHostname(session.realIP.String())
-		client.cloakedHostname = config.Server.Cloaks.ComputeCloak(session.realIP)
 		remoteAddr := conn.Conn.RemoteAddr()
 		if utils.AddrIsLocal(remoteAddr) {
 			// treat local connections as secure (may be overridden later by WEBIRC)
@@ -274,13 +273,71 @@ func (server *Server) RunClient(conn clientConn, proxyLine string) {
 		}
 	}
 	client.realIP = session.realIP
-	client.rawHostname = session.rawHostname
-	client.proxiedIP = session.proxiedIP
 
 	server.stats.Add()
 	client.run(session, proxyLine)
 }
 
+// resolve an IP to an IRC-ready hostname, using reverse DNS, forward-confirming if necessary,
+// and sending appropriate notices to the client
+func (client *Client) lookupHostname(session *Session, overwrite bool) {
+	if client.isTor {
+		return
+	} // else: even if cloaking is enabled, look up the real hostname to show to operators
+
+	config := client.server.Config()
+	ip := session.realIP
+	if session.proxiedIP != nil {
+		ip = session.proxiedIP
+	}
+	ipString := ip.String()
+
+	var hostname, candidate string
+	if config.Server.lookupHostnames {
+		session.Notice("*** Looking up your hostname...")
+
+		names, err := net.LookupAddr(ipString)
+		if err == nil && 0 < len(names) {
+			candidate = strings.TrimSuffix(names[0], ".")
+		}
+		if utils.IsHostname(candidate) {
+			if config.Server.ForwardConfirmHostnames {
+				addrs, err := net.LookupHost(candidate)
+				if err == nil {
+					for _, addr := range addrs {
+						if addr == ipString {
+							hostname = candidate // successful forward confirmation
+							break
+						}
+					}
+				}
+			} else {
+				hostname = candidate
+			}
+		}
+	}
+
+	if hostname != "" {
+		session.Notice("*** Found your hostname")
+	} else {
+		if config.Server.lookupHostnames {
+			session.Notice("*** Couldn't look up your hostname")
+		}
+		hostname = utils.IPStringToHostname(ipString)
+	}
+
+	session.rawHostname = hostname
+	cloakedHostname := config.Server.Cloaks.ComputeCloak(ip)
+	client.stateMutex.Lock()
+	defer client.stateMutex.Unlock()
+	// update the hostname if this is a new connection or a resume, but not if it's a reattach
+	if overwrite || client.rawHostname == "" {
+		client.rawHostname = hostname
+		client.cloakedHostname = cloakedHostname
+		client.updateNickMaskNoMutex()
+	}
+}
+
 func (client *Client) doIdentLookup(conn net.Conn) {
 	_, serverPortString, err := net.SplitHostPort(conn.LocalAddr().String())
 	if err != nil {
@@ -617,7 +674,7 @@ func (session *Session) playResume() {
 
 	details := client.Details()
 	oldNickmask := details.nickMask
-	client.SetRawHostname(session.rawHostname)
+	client.lookupHostname(session, true)
 	hostname := client.Hostname() // may be a vhost
 	timestampString := timestamp.Format(IRCv3TimestampFormat)
 
@@ -887,6 +944,10 @@ func (client *Client) updateNickMaskNoMutex() {
 		}
 	}
 
+	if client.hostname == "" {
+		return // pre-registration, don't bother generating the hostname
+	}
+
 	cfhostname, err := Casefold(client.hostname)
 	if err != nil {
 		client.server.logger.Error("internal", "hostname couldn't be casefolded", client.hostname, err.Error())
@@ -1255,6 +1316,10 @@ func (client *Client) Notice(text string) {
 	client.Send(nil, client.server.name, "NOTICE", client.Nick(), text)
 }
 
+func (session *Session) Notice(text string) {
+	session.Send(nil, session.client.server.name, "NOTICE", session.client.Nick(), text)
+}
+
 func (client *Client) addChannel(channel *Channel) {
 	client.stateMutex.Lock()
 	client.channels[channel] = true

irc/config.go

@@ -293,19 +293,22 @@ type Config struct {
 		Listen       []string
 		TLSListeners map[string]TLSListenConfig `yaml:"tls-listeners"`
 		// either way, the result is this:
-		trueListeners        map[string]listenerConfig
+		trueListeners           map[string]listenerConfig
-		STS                  STSConfig
+		STS                     STSConfig
-		CheckIdent           bool `yaml:"check-ident"`
+		LookupHostnames         *bool `yaml:"lookup-hostnames"`
-		MOTD                 string
+		lookupHostnames         bool
-		motdLines            []string
+		ForwardConfirmHostnames bool `yaml:"forward-confirm-hostnames"`
-		MOTDFormatting       bool     `yaml:"motd-formatting"`
+		CheckIdent              bool `yaml:"check-ident"`
-		ProxyAllowedFrom     []string `yaml:"proxy-allowed-from"`
+		MOTD                    string
-		proxyAllowedFromNets []net.IPNet
+		motdLines               []string
-		WebIRC               []webircConfig `yaml:"webirc"`
+		MOTDFormatting          bool     `yaml:"motd-formatting"`
-		MaxSendQString       string         `yaml:"max-sendq"`
+		ProxyAllowedFrom        []string `yaml:"proxy-allowed-from"`
-		MaxSendQBytes        int
+		proxyAllowedFromNets    []net.IPNet
-		AllowPlaintextResume bool `yaml:"allow-plaintext-resume"`
+		WebIRC                  []webircConfig `yaml:"webirc"`
-		Compatibility        struct {
+		MaxSendQString          string         `yaml:"max-sendq"`
+		MaxSendQBytes           int
+		AllowPlaintextResume    bool `yaml:"allow-plaintext-resume"`
+		Compatibility           struct {
 			ForceTrailing      *bool `yaml:"force-trailing"`
 			forceTrailing      bool
 			SendUnprefixedSasl bool `yaml:"send-unprefixed-sasl"`
@@ -590,7 +593,7 @@ func LoadConfig(filename string) (config *Config, err error) {
 	if config.Server.Name == "" {
 		return nil, ErrServerNameMissing
 	}
-	if !utils.IsHostname(config.Server.Name) {
+	if !utils.IsServerName(config.Server.Name) {
 		return nil, ErrServerNameNotHostname
 	}
 	if config.Datastore.Path == "" {
@@ -635,6 +638,13 @@ func LoadConfig(filename string) (config *Config, err error) {
 	// set this even if STS is disabled
 	config.Server.capValues[caps.STS] = config.Server.STS.Value()
 
+	// lookup-hostnames defaults to true if unset
+	if config.Server.LookupHostnames != nil {
+		config.Server.lookupHostnames = *config.Server.LookupHostnames
+	} else {
+		config.Server.lookupHostnames = true
+	}
+
 	// process webirc blocks
 	var newWebIRC []webircConfig
 	for _, webirc := range config.Server.WebIRC {

irc/gateways.go

@@ -76,18 +76,12 @@ func (client *Client) ApplyProxiedIP(session *Session, proxiedIP string, tls boo
 	client.server.connectionLimiter.RemoveClient(session.realIP)
 
 	// given IP is sane! override the client's current IP
-	ipstring := parsedProxiedIP.String()
+	client.server.logger.Info("localconnect-ip", "Accepted proxy IP for client", parsedProxiedIP.String())
-	client.server.logger.Info("localconnect-ip", "Accepted proxy IP for client", ipstring)
-	rawHostname := utils.LookupHostname(ipstring)
-	cloakedHostname := client.server.Config().Server.Cloaks.ComputeCloak(parsedProxiedIP)
 
 	client.stateMutex.Lock()
 	defer client.stateMutex.Unlock()
 	client.proxiedIP = parsedProxiedIP
-	client.rawHostname = rawHostname
 	session.proxiedIP = parsedProxiedIP
-	session.rawHostname = rawHostname
-	client.cloakedHostname = cloakedHostname
 	// nickmask will be updated when the client completes registration
 	// set tls info
 	client.certfp = ""

irc/getters.go

@@ -239,14 +239,6 @@ func (client *Client) RawHostname() (result string) {
 	return
 }
 
-func (client *Client) SetRawHostname(rawHostname string) {
-	client.stateMutex.Lock()
-	defer client.stateMutex.Unlock()
-
-	client.rawHostname = rawHostname
-	client.updateNickMaskNoMutex()
-}
-
 func (client *Client) AwayMessage() (result string) {
 	client.stateMutex.RLock()
 	result = client.awayMessage

irc/nickserv.go

@@ -802,16 +802,16 @@ func nsSessionsHandler(server *Server, client *Client, command string, params []
 	target := client
 
 	if 0 < len(params) {
-		// same permissions check as RPL_WHOISACTUALLY for now:
-		if !client.HasMode(modes.Operator) {
-			nsNotice(rb, client.t("Command restricted"))
-			return
-		}
 		target = server.clients.Get(params[0])
 		if target == nil {
 			nsNotice(rb, client.t("No such nick"))
 			return
 		}
+		// same permissions check as RPL_WHOISACTUALLY for now:
+		if target != client && !client.HasMode(modes.Operator) {
+			nsNotice(rb, client.t("Command restricted"))
+			return
+		}
 	}
 
 	sessionData, currentIndex := target.AllSessionData(rb.session)

irc/server.go

@@ -377,12 +377,9 @@ func (server *Server) tryRegister(c *Client, session *Session) (exiting bool) {
 		return
 	}
 
-	// check KLINEs
+	// we have nickname, username, and the final value of the IP address:
-	isBanned, info := server.klines.CheckMasks(c.AllNickmasks()...)
+	// do the hostname lookup and set the nickmask
-	if isBanned {
+	session.client.lookupHostname(session, false)
-		c.Quit(info.BanMessage(c.t("You are banned from this server (%s)")), nil)
-		return true
-	}
 
 	if session.client != c {
 		// reattached, bail out.
@@ -392,6 +389,13 @@ func (server *Server) tryRegister(c *Client, session *Session) (exiting bool) {
 		return
 	}
 
+	// check KLINEs
+	isBanned, info := server.klines.CheckMasks(c.AllNickmasks()...)
+	if isBanned {
+		c.Quit(info.BanMessage(c.t("You are banned from this server (%s)")), nil)
+		return true
+	}
+
 	// registration has succeeded:
 	c.SetRegistered()
 

irc/utils/net.go

@@ -6,6 +6,7 @@ package utils
 
 import (
 	"net"
+	"regexp"
 	"strings"
 )
 
@@ -13,6 +14,8 @@ var (
 	// subnet mask for an ipv6 /128:
 	mask128             = net.CIDRMask(128, 128)
 	IPv4LoopbackAddress = net.ParseIP("127.0.0.1").To16()
+
+	validHostnameLabelRegexp = regexp.MustCompile(`^[0-9A-Za-z.\-]+$`)
 )
 
 // AddrIsLocal returns whether the address is from a trusted local connection (loopback or unix).
@@ -40,30 +43,19 @@ func AddrIsUnix(addr net.Addr) bool {
 	return ok
 }
 
-// LookupHostname returns the hostname for `addr` if it has one. Otherwise, just returns `addr`.
+// IPStringToHostname converts a string representation of an IP address to an IRC-ready hostname
-func LookupHostname(addr string) string {
+func IPStringToHostname(ipStr string) string {
-	names, err := net.LookupAddr(addr)
+	if 0 < len(ipStr) && ipStr[0] == ':' {
-	if err == nil && len(names) > 0 {
-		candidate := strings.TrimSuffix(names[0], ".")
-		if IsHostname(candidate) {
-			return candidate
-		}
-	}
-
-	// return original address if no hostname found
-	if len(addr) > 0 && addr[0] == ':' {
 		// fix for IPv6 hostnames (so they don't start with a colon), same as all other IRCds
-		addr = "0" + addr
+		ipStr = "0" + ipStr
 	}
-	return addr
+	return ipStr
 }
 
-var allowedHostnameChars = "abcdefghijklmnopqrstuvwxyz1234567890-."
-
 // IsHostname returns whether we consider `name` a valid hostname.
 func IsHostname(name string) bool {
-	// IRC hostnames specifically require a period
+	name = strings.TrimSuffix(name, ".")
-	if !strings.Contains(name, ".") || len(name) < 1 || len(name) > 253 {
+	if len(name) < 1 || len(name) > 253 {
 		return false
 	}
 
@@ -72,11 +64,7 @@ func IsHostname(name string) bool {
 		if len(part) < 1 || len(part) > 63 || strings.HasPrefix(part, "-") || strings.HasSuffix(part, "-") {
 			return false
 		}
-	}
+		if !validHostnameLabelRegexp.MatchString(part) {
-
-	// ensure all chars of hostname are valid
-	for _, char := range strings.Split(strings.ToLower(name), "") {
-		if !strings.Contains(allowedHostnameChars, char) {
 			return false
 		}
 	}
@@ -84,6 +72,12 @@ func IsHostname(name string) bool {
 	return true
 }
 
+// IsServerName returns whether we consider `name` a valid IRC server name.
+func IsServerName(name string) bool {
+	// IRC server names specifically require a period
+	return IsHostname(name) && strings.IndexByte(name, '.') != -1
+}
+
 // Convenience to test whether `ip` is contained in any of `nets`.
 func IPInNets(ip net.IP, nets []net.IPNet) bool {
 	for _, network := range nets {

irc/utils/net_test.go

@@ -24,14 +24,18 @@ var (
 		"gsf.ds342.co.uk",
 		"324.net.uk",
 		"xn--bcher-kva.ch",
+		"pentos",
+		"pentos.",
+		"www.google.com.",
 	}
 
 	badHostnames = []string{
 		"-lol-.net.uk",
 		"-lol.net.uk",
 		"_irc._sctp.lol.net.uk",
-		"irc",
+		"irc.l%l.net.uk",
-		"com",
+		"irc..net.uk",
+		".",
 		"",
 	}
 )
@@ -56,6 +60,15 @@ func TestIsHostname(t *testing.T) {
 	}
 }
 
+func TestIsServerName(t *testing.T) {
+	if IsServerName("pentos") {
+		t.Error("irc server names must contain a period")
+	}
+	if !IsServerName("darwin.network") {
+		t.Error("failed to validate a perfectly good server name")
+	}
+}
+
 func TestNormalizeToNet(t *testing.T) {
 	a := net.ParseIP("8.8.8.8")
 	b := net.ParseIP("8.8.4.4")

oragono.yaml

@@ -85,6 +85,14 @@ server:
         # should clients include this STS policy when they ship their inbuilt preload lists?
         preload: false
 
+    # whether to look up user hostnames with reverse DNS
+    # (to suppress this for privacy purposes, use the ip-cloaking options below)
+    lookup-hostnames: true
+    # whether to confirm hostname lookups using "forward-confirmed reverse DNS", i.e., for
+    # any hostname returned from reverse DNS, resolve it back to an IP address and reject it
+    # unless it matches the connecting IP
+    forward-confirm-hostnames: true
+
     # use ident protocol to get usernames
     check-ident: false