Fix #448, #594

irc/caps/set.go

@@ -4,8 +4,8 @@
 package caps
 
 import (
+	"bytes"
 	"sort"
-	"strings"
 
 	"github.com/oragono/oragono/irc/utils"
 )
@@ -13,6 +13,9 @@ import (
 // Set holds a set of enabled capabilities.
 type Set [bitsetLen]uint32
 
+// Values holds capability values.
+type Values map[Capability]string
+
 // NewSet returns a new Set, with the given capabilities enabled.
 func NewSet(capabs ...Capability) *Set {
 	var newSet Set
@@ -88,8 +91,10 @@ func (s *Set) Empty() bool {
 	return utils.BitsetEmpty(s[:])
 }
 
+const maxPayloadLength = 440
+
 // String returns all of our enabled capabilities as a string.
-func (s *Set) String(version Version, values *Values) string {
+func (s *Set) String(version Version, values Values) (result []string) {
 	var strs sort.StringSlice
 
 	var capab Capability
@@ -101,7 +106,7 @@ func (s *Set) String(version Version, values *Values) string {
 		}
 		capString := capab.Name()
 		if version == Cap302 {
-			val, exists := values.Get(capab)
+			val, exists := values[capab]
 			if exists {
 				capString += "=" + val
 			}
@@ -109,8 +114,31 @@ func (s *Set) String(version Version, values *Values) string {
 		strs = append(strs, capString)
 	}
 
+	if len(strs) == 0 {
+		return []string{""}
+	}
+
 	// sort the cap string before we send it out
 	sort.Sort(strs)
 
-	return strings.Join(strs, " ")
+	var buf bytes.Buffer
+	for _, str := range strs {
+		tokenLen := len(str)
+		if buf.Len() != 0 {
+			tokenLen += 1
+		}
+		if maxPayloadLength < buf.Len()+tokenLen {
+			result = append(result, buf.String())
+			buf.Reset()
+		}
+		if buf.Len() != 0 {
+			buf.WriteByte(' ')
+		}
+		buf.WriteString(str)
+	}
+	if buf.Len() != 0 {
+		result = append(result, buf.String())
+	}
+
+	return
 }

irc/caps/set_test.go

@@ -44,18 +44,18 @@ func TestSets(t *testing.T) {
 	}
 
 	// test String()
-	values := NewValues()
-	values.Set(InviteNotify, "invitemepls")
+	values := make(Values)
+	values[InviteNotify] = "invitemepls"
 
 	actualCap301ValuesString := s1.String(Cap301, values)
-	expectedCap301ValuesString := "invite-notify userhost-in-names"
-	if actualCap301ValuesString != expectedCap301ValuesString {
-		t.Errorf("Generated Cap301 values string [%s] did not match expected values string [%s]", actualCap301ValuesString, expectedCap301ValuesString)
+	expectedCap301ValuesString := []string{"invite-notify userhost-in-names"}
+	if !reflect.DeepEqual(actualCap301ValuesString, expectedCap301ValuesString) {
+		t.Errorf("Generated Cap301 values string [%v] did not match expected values string [%v]", actualCap301ValuesString, expectedCap301ValuesString)
 	}
 
 	actualCap302ValuesString := s1.String(Cap302, values)
-	expectedCap302ValuesString := "invite-notify=invitemepls userhost-in-names"
-	if actualCap302ValuesString != expectedCap302ValuesString {
+	expectedCap302ValuesString := []string{"invite-notify=invitemepls userhost-in-names"}
+	if !reflect.DeepEqual(actualCap302ValuesString, expectedCap302ValuesString) {
 		t.Errorf("Generated Cap302 values string [%s] did not match expected values string [%s]", actualCap302ValuesString, expectedCap302ValuesString)
 	}
 }

irc/caps/values.go

@@ -1,45 +0,0 @@
-// Copyright (c) 2017 Daniel Oaks <daniel@danieloaks.net>
-// released under the MIT license
-
-package caps
-
-import "sync"
-
-// Values holds capability values.
-type Values struct {
-	sync.RWMutex
-	// values holds our actual capability values.
-	values map[Capability]string
-}
-
-// NewValues returns a new Values.
-func NewValues() *Values {
-	return &Values{
-		values: make(map[Capability]string),
-	}
-}
-
-// Set sets the value for the given capability.
-func (v *Values) Set(capab Capability, value string) {
-	v.Lock()
-	defer v.Unlock()
-
-	v.values[capab] = value
-}
-
-// Unset removes the value for the given capability, if it exists.
-func (v *Values) Unset(capab Capability) {
-	v.Lock()
-	defer v.Unlock()
-
-	delete(v.values, capab)
-}
-
-// Get returns the value of the given capability, and whether one exists.
-func (v *Values) Get(capab Capability) (string, bool) {
-	v.RLock()
-	defer v.RUnlock()
-
-	value, exists := v.values[capab]
-	return value, exists
-}

irc/client.go

@@ -58,6 +58,7 @@ type Client struct {
 	flags              modes.ModeSet
 	hostname           string
 	invitedTo          map[string]bool
+	isSTSOnly          bool
 	isTor              bool
 	languages          []string
 	loginThrottle      connection_limits.GenericThrottle
@@ -220,6 +221,7 @@ func (server *Server) RunClient(conn clientConn) {
 		atime:     now,
 		channels:  make(ChannelSet),
 		ctime:     now,
+		isSTSOnly: conn.Config.IsSTSOnly,
 		isTor:     conn.Config.IsTor,
 		languages: server.Languages().Default(),
 		loginThrottle: connection_limits.GenericThrottle{

irc/config.go

@@ -14,10 +14,12 @@ import (
 	"os"
 	"regexp"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 
 	"code.cloudfoundry.org/bytefmt"
+	"github.com/oragono/oragono/irc/caps"
 	"github.com/oragono/oragono/irc/cloaks"
 	"github.com/oragono/oragono/irc/connection_limits"
 	"github.com/oragono/oragono/irc/custime"
@@ -43,8 +45,9 @@ type TLSListenConfig struct {
 
 // This is the YAML-deserializable type of the value of the `Server.Listeners` map
 type listenerConfigBlock struct {
-	TLS TLSListenConfig
-	Tor bool
+	TLS     TLSListenConfig
+	Tor     bool
+	STSOnly bool `yaml:"sts-only"`
 }
 
 // listenerConfig is the config governing a particular listener (bound address),
@@ -52,6 +55,7 @@ type listenerConfigBlock struct {
 type listenerConfig struct {
 	TLSConfig *tls.Config
 	IsTor     bool
+	IsSTSOnly bool
 }
 
 type AccountConfig struct {
@@ -235,6 +239,8 @@ type STSConfig struct {
 	DurationString string        `yaml:"duration"`
 	Port           int
 	Preload        bool
+	STSOnlyBanner  string `yaml:"sts-only-banner"`
+	bannerLines    []string
 }
 
 // Value returns the STS value to advertise in CAP
@@ -306,6 +312,8 @@ type Config struct {
 		ConnectionLimiter   connection_limits.LimiterConfig   `yaml:"connection-limits"`
 		ConnectionThrottler connection_limits.ThrottlerConfig `yaml:"connection-throttling"`
 		Cloaks              cloaks.CloakConfig                `yaml:"ip-cloaking"`
+		supportedCaps       *caps.Set
+		capValues           caps.Values
 	}
 
 	Languages struct {
@@ -511,6 +519,10 @@ func (conf *Config) prepareListeners() (err error) {
 		for addr, block := range conf.Server.Listeners {
 			var lconf listenerConfig
 			lconf.IsTor = block.Tor
+			lconf.IsSTSOnly = block.STSOnly
+			if lconf.IsSTSOnly && !conf.Server.STS.Enabled {
+				return fmt.Errorf("%s is configured as a STS-only listener, but STS is disabled", addr)
+			}
 			if block.TLS.Cert != "" {
 				tlsConfig, err := loadTlsConfig(block.TLS)
 				if err != nil {
@@ -592,6 +604,15 @@ func LoadConfig(filename string) (config *Config, err error) {
 	if config.Limits.RegistrationMessages == 0 {
 		config.Limits.RegistrationMessages = 1024
 	}
+
+	config.Server.supportedCaps = caps.NewCompleteSet()
+	config.Server.capValues = make(caps.Values)
+
+	err = config.prepareListeners()
+	if err != nil {
+		return nil, fmt.Errorf("failed to prepare listeners: %v", err)
+	}
+
 	if config.Server.STS.Enabled {
 		config.Server.STS.Duration, err = custime.ParseDuration(config.Server.STS.DurationString)
 		if err != nil {
@@ -600,7 +621,18 @@ func LoadConfig(filename string) (config *Config, err error) {
 		if config.Server.STS.Port < 0 || config.Server.STS.Port > 65535 {
 			return nil, fmt.Errorf("STS port is incorrect, should be 0 if disabled: %d", config.Server.STS.Port)
 		}
+		if config.Server.STS.STSOnlyBanner != "" {
+			config.Server.STS.bannerLines = utils.WordWrap(config.Server.STS.STSOnlyBanner, 400)
+		} else {
+			config.Server.STS.bannerLines = []string{fmt.Sprintf("This server is only accessible over TLS. Please reconnect using TLS on port %d.", config.Server.STS.Port)}
+		}
+	} else {
+		config.Server.supportedCaps.Disable(caps.STS)
+		config.Server.STS.Duration = 0
 	}
+	// set this even if STS is disabled
+	config.Server.capValues[caps.STS] = config.Server.STS.Value()
+
 	if config.Server.ConnectionThrottler.Enabled {
 		config.Server.ConnectionThrottler.Duration, err = time.ParseDuration(config.Server.ConnectionThrottler.DurationString)
 		if err != nil {
@@ -626,10 +658,17 @@ func LoadConfig(filename string) (config *Config, err error) {
 		newWebIRC = append(newWebIRC, webirc)
 	}
 	config.Server.WebIRC = newWebIRC
+
 	// process limits
 	if config.Limits.LineLen.Rest < 512 {
 		config.Limits.LineLen.Rest = 512
 	}
+	if config.Limits.LineLen.Rest == 512 {
+		config.Server.supportedCaps.Disable(caps.MaxLine)
+	} else {
+		config.Server.capValues[caps.MaxLine] = strconv.Itoa(config.Limits.LineLen.Rest)
+	}
+
 	var newLogConfigs []logger.LoggingConfig
 	for _, logConfig := range config.Logging {
 		// methods
@@ -713,6 +752,11 @@ func LoadConfig(filename string) (config *Config, err error) {
 		config.Accounts.LoginThrottling.MaxAttempts = 0 // limit of 0 means disabled
 	}
 
+	config.Server.capValues[caps.SASL] = "PLAIN,EXTERNAL"
+	if !config.Accounts.AuthenticationEnabled {
+		config.Server.supportedCaps.Disable(caps.SASL)
+	}
+
 	maxSendQBytes, err := bytefmt.ToBytes(config.Server.MaxSendQString)
 	if err != nil {
 		return nil, fmt.Errorf("Could not parse maximum SendQ size (make sure it only contains whole numbers): %s", err.Error())
@@ -723,6 +767,7 @@ func LoadConfig(filename string) (config *Config, err error) {
 	if err != nil {
 		return nil, fmt.Errorf("Could not load languages: %s", err.Error())
 	}
+	config.Server.capValues[caps.Languages] = config.languageManager.CapValue()
 
 	// RecoverFromErrors defaults to true
 	if config.Debug.RecoverFromErrors != nil {
@@ -798,10 +843,5 @@ func LoadConfig(filename string) (config *Config, err error) {
 		}
 	}
 
-	err = config.prepareListeners()
-	if err != nil {
-		return nil, fmt.Errorf("failed to prepare listeners: %v", err)
-	}
-
 	return config, nil
 }

irc/handlers.go

@@ -301,6 +301,11 @@ func authenticateHandler(server *Server, client *Client, msg ircmsg.IrcMessage,
 	config := server.Config()
 	details := client.Details()
 
+	if client.isSTSOnly {
+		rb.Add(nil, server.name, ERR_SASLFAIL, details.nick, client.t("SASL authentication failed"))
+		return false
+	}
+
 	if details.account != "" {
 		rb.Add(nil, server.name, ERR_SASLALREADY, details.nick, client.t("You're already logged into an account"))
 		return false
@@ -535,6 +540,12 @@ func capHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Respo
 	toRemove := caps.NewSet()
 	var capString string
 
+	config := server.Config()
+	supportedCaps := config.Server.supportedCaps
+	if client.isSTSOnly {
+		supportedCaps = stsOnlyCaps
+	}
+
 	badCaps := false
 	if len(msg.Params) > 1 {
 		capString = msg.Params[1]
@@ -546,7 +557,7 @@ func capHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Respo
 				remove = true
 			}
 			capab, err := caps.NameToCapability(str)
-			if err != nil || (!remove && !SupportedCapabilities.Has(capab)) {
+			if err != nil || (!remove && !supportedCaps.Has(capab)) {
 				badCaps = true
 			} else if !remove {
 				toAdd.Enable(capab)
@@ -556,6 +567,20 @@ func capHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Respo
 		}
 	}
 
+	sendCapLines := func(cset *caps.Set, values caps.Values) {
+		version := rb.session.capVersion
+		capLines := cset.String(version, values)
+		// weechat 1.4 has a bug here where it won't accept the CAP reply unless it contains
+		// the server.name source:
+		for i, capStr := range capLines {
+			if version == caps.Cap302 && i < len(capLines)-1 {
+				rb.Add(nil, server.name, "CAP", details.nick, subCommand, "*", capStr)
+			} else {
+				rb.Add(nil, server.name, "CAP", details.nick, subCommand, capStr)
+			}
+		}
+	}
+
 	switch subCommand {
 	case "LS":
 		if !client.registered {
@@ -568,14 +593,11 @@ func capHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Respo
 				rb.session.capVersion = newVersion
 			}
 		}
-		// weechat 1.4 has a bug here where it won't accept the CAP reply unless it contains
-		// the server.name source... otherwise it doesn't respond to the CAP message with
-		// anything and just hangs on connection.
-		//TODO(dan): limit number of caps and send it multiline in 3.2 style as appropriate.
-		rb.Add(nil, server.name, "CAP", details.nick, subCommand, SupportedCapabilities.String(rb.session.capVersion, CapValues))
+		sendCapLines(supportedCaps, config.Server.capValues)
 
 	case "LIST":
-		rb.Add(nil, server.name, "CAP", details.nick, subCommand, rb.session.capabilities.String(caps.Cap301, CapValues)) // values not sent on LIST so force 3.1
+		// values not sent on LIST
+		sendCapLines(&rb.session.capabilities, nil)
 
 	case "REQ":
 		if !client.registered {

irc/server.go

@@ -39,13 +39,9 @@ var (
 	// supportedChannelModesString acts as a cache for when we introduce users
 	supportedChannelModesString = modes.SupportedChannelModes.String()
 
-	// SupportedCapabilities are the caps we advertise.
-	// MaxLine, SASL and STS may be unset during server startup / rehash.
-	SupportedCapabilities = caps.NewCompleteSet()
-
-	// CapValues are the actual values we advertise to v3.2 clients.
-	// actual values are set during server startup.
-	CapValues = caps.NewValues()
+	// whitelist of caps to serve on the STS-only listener. In particular,
+	// never advertise SASL, to discourage people from sending their passwords:
+	stsOnlyCaps = caps.NewSet(caps.STS, caps.MessageTags, caps.ServerTime, caps.LabeledResponse, caps.Nope)
 )
 
 // ListenerWrapper wraps a listener so it can be safely reconfigured or stopped
@@ -340,6 +336,11 @@ func (server *Server) tryRegister(c *Client, session *Session) (exiting bool) {
 		return
 	}
 
+	if c.isSTSOnly {
+		server.playRegistrationBurst(session)
+		return true
+	}
+
 	// client MUST send PASS if necessary, or authenticate with SASL if necessary,
 	// before completing the other registration commands
 	authOutcome := c.isAuthorized(server.Config())
@@ -407,6 +408,13 @@ func (server *Server) playRegistrationBurst(session *Session) {
 	//TODO(dan): Look at adding last optional [<channel modes with a parameter>] parameter
 	session.Send(nil, server.name, RPL_MYINFO, d.nick, server.name, Ver, supportedUserModesString, supportedChannelModesString)
 
+	if c.isSTSOnly {
+		for _, line := range server.Config().Server.STS.bannerLines {
+			c.Notice(line)
+		}
+		return
+	}
+
 	rb := NewResponseBuffer(session)
 	server.RplISupport(c, rb)
 	server.Lusers(c, rb)
@@ -623,23 +631,17 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) {
 	server.logger.Debug("server", "Regenerating HELP indexes for new languages")
 	server.helpIndexManager.GenerateIndices(config.languageManager)
 
-	currentLanguageValue, _ := CapValues.Get(caps.Languages)
-	newLanguageValue := config.languageManager.CapValue()
-	if currentLanguageValue != newLanguageValue {
+	if oldConfig != nil && config.Server.capValues[caps.Languages] != oldConfig.Server.capValues[caps.Languages] {
 		updatedCaps.Add(caps.Languages)
-		CapValues.Set(caps.Languages, newLanguageValue)
 	}
 
 	// SASL
 	authPreviouslyEnabled := oldConfig != nil && oldConfig.Accounts.AuthenticationEnabled
 	if config.Accounts.AuthenticationEnabled && (oldConfig == nil || !authPreviouslyEnabled) {
 		// enabling SASL
-		SupportedCapabilities.Enable(caps.SASL)
-		CapValues.Set(caps.SASL, "PLAIN,EXTERNAL")
 		addedCaps.Add(caps.SASL)
 	} else if !config.Accounts.AuthenticationEnabled && (oldConfig == nil || authPreviouslyEnabled) {
 		// disabling SASL
-		SupportedCapabilities.Disable(caps.SASL)
 		removedCaps.Add(caps.SASL)
 	}
 
@@ -661,39 +663,17 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) {
 		server.channels.loadRegisteredChannels()
 	}
 
-	// MaxLine
-	if config.Limits.LineLen.Rest != 512 {
-		SupportedCapabilities.Enable(caps.MaxLine)
-		value := fmt.Sprintf("%d", config.Limits.LineLen.Rest)
-		CapValues.Set(caps.MaxLine, value)
-	} else {
-		SupportedCapabilities.Disable(caps.MaxLine)
-	}
-
 	// STS
 	stsPreviouslyEnabled := oldConfig != nil && oldConfig.Server.STS.Enabled
-	stsValue := config.Server.STS.Value()
-	stsDisabledByRehash := false
-	stsCurrentCapValue, _ := CapValues.Get(caps.STS)
+	stsValue := config.Server.capValues[caps.STS]
+	stsCurrentCapValue := ""
+	if oldConfig != nil {
+		stsCurrentCapValue = oldConfig.Server.capValues[caps.STS]
+	}
 	server.logger.Debug("server", "STS Vals", stsCurrentCapValue, stsValue, fmt.Sprintf("server[%v] config[%v]", stsPreviouslyEnabled, config.Server.STS.Enabled))
-	if config.Server.STS.Enabled {
-		// enabling STS
-		SupportedCapabilities.Enable(caps.STS)
-		if !stsPreviouslyEnabled {
-			addedCaps.Add(caps.STS)
-			CapValues.Set(caps.STS, stsValue)
-		} else if stsValue != stsCurrentCapValue {
-			// STS policy updated
-			CapValues.Set(caps.STS, stsValue)
-			updatedCaps.Add(caps.STS)
-		}
-	} else {
-		// disabling STS
-		SupportedCapabilities.Disable(caps.STS)
-		if stsPreviouslyEnabled {
-			removedCaps.Add(caps.STS)
-			stsDisabledByRehash = true
-		}
+	if (config.Server.STS.Enabled != stsPreviouslyEnabled) || (stsValue != stsCurrentCapValue) {
+		// XXX: STS is always removed by CAP NEW sts=duration=0, not CAP DEL
+		addedCaps.Add(caps.STS)
 	}
 
 	// resize history buffers as needed
@@ -708,42 +688,35 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) {
 
 	// burst new and removed caps
 	var capBurstSessions []*Session
-	added := make(map[caps.Version]string)
-	var removed string
+	added := make(map[caps.Version][]string)
+	var removed []string
 
 	// updated caps get DEL'd and then NEW'd
 	// so, we can just add updated ones to both removed and added lists here and they'll be correctly handled
-	server.logger.Debug("server", "Updated Caps", updatedCaps.String(caps.Cap301, CapValues))
+	server.logger.Debug("server", "Updated Caps", strings.Join(updatedCaps.String(caps.Cap301, config.Server.capValues), " "))
 	addedCaps.Union(updatedCaps)
 	removedCaps.Union(updatedCaps)
 
 	if !addedCaps.Empty() || !removedCaps.Empty() {
 		capBurstSessions = server.clients.AllWithCapsNotify()
 
-		added[caps.Cap301] = addedCaps.String(caps.Cap301, CapValues)
-		added[caps.Cap302] = addedCaps.String(caps.Cap302, CapValues)
+		added[caps.Cap301] = addedCaps.String(caps.Cap301, config.Server.capValues)
+		added[caps.Cap302] = addedCaps.String(caps.Cap302, config.Server.capValues)
 		// removed never has values, so we leave it as Cap301
-		removed = removedCaps.String(caps.Cap301, CapValues)
+		removed = removedCaps.String(caps.Cap301, config.Server.capValues)
 	}
 
 	for _, sSession := range capBurstSessions {
-		if stsDisabledByRehash {
-			// remove STS policy
-			//TODO(dan): this is an ugly hack. we can write this better.
-			stsPolicy := "sts=duration=0"
-			if !addedCaps.Empty() {
-				added[caps.Cap302] = added[caps.Cap302] + " " + stsPolicy
-			} else {
-				addedCaps.Enable(caps.STS)
-				added[caps.Cap302] = stsPolicy
-			}
-		}
 		// DEL caps and then send NEW ones so that updated caps get removed/added correctly
 		if !removedCaps.Empty() {
-			sSession.Send(nil, server.name, "CAP", sSession.client.Nick(), "DEL", removed)
+			for _, capStr := range removed {
+				sSession.Send(nil, server.name, "CAP", sSession.client.Nick(), "DEL", capStr)
+			}
 		}
 		if !addedCaps.Empty() {
-			sSession.Send(nil, server.name, "CAP", sSession.client.Nick(), "NEW", added[sSession.capVersion])
+			for _, capStr := range added[sSession.capVersion] {
+				sSession.Send(nil, server.name, "CAP", sSession.client.Nick(), "NEW", capStr)
+			}
 		}
 	}
 
@@ -905,15 +878,11 @@ func (server *Server) setupListeners(config *Config) (err error) {
 		}
 	}
 
+	publicPlaintextListener := ""
 	// create new listeners that were not previously configured
-	numTlsListeners := 0
-	hasStandardTlsListener := false
 	for newAddr, newConfig := range config.Server.trueListeners {
-		if newConfig.TLSConfig != nil {
-			numTlsListeners += 1
-			if strings.HasSuffix(newAddr, ":6697") {
-				hasStandardTlsListener = true
-			}
+		if strings.HasPrefix(newAddr, ":") && !newConfig.IsTor && !newConfig.IsSTSOnly && newConfig.TLSConfig == nil {
+			publicPlaintextListener = newAddr
 		}
 		_, exists := server.listeners[newAddr]
 		if !exists {
@@ -929,12 +898,8 @@ func (server *Server) setupListeners(config *Config) (err error) {
 		}
 	}
 
-	if numTlsListeners == 0 {
-		server.logger.Warning("server", "You are not exposing an SSL/TLS listening port. You should expose at least one port (typically 6697) to accept TLS connections")
-	}
-
-	if !hasStandardTlsListener {
-		server.logger.Warning("server", "Port 6697 is the standard TLS port for IRC. You should (also) expose port 6697 as a TLS port to ensure clients can connect securely")
+	if publicPlaintextListener != "" {
+		server.logger.Warning("listeners", fmt.Sprintf("Your server is configured with public plaintext listener %s. Consider disabling it for improved security and privacy.", publicPlaintextListener))
 	}
 
 	return