fix #952

conventional.yaml

@@ -233,17 +233,6 @@ server:
         # fake TLD at the end of the hostname, e.g., pwbs2ui4377257x8.oragono
         netname: "oragono"
 
-        # secret key to prevent dictionary attacks against cloaked IPs
-        # any high-entropy secret is valid for this purpose:
-        # you MUST generate a new one for your installation.
-        # suggestion: use the output of `oragono mksecret`
-        # note that rotating this key will invalidate all existing ban masks.
-        secret: "siaELnk6Kaeo65K3RCrwJjlWaZ-Bt3WuZ2L8MXLbNb4"
-
-        # name of an environment variable to pull the secret from, for use with
-        # k8s secret distribution:
-        # secret-environment-variable: "ORAGONO_CLOAKING_SECRET"
-
         # the cloaked hostname is derived only from the CIDR (most significant bits
         # of the IP address), up to a configurable number of bits. this is the
         # granularity at which bans will take effect for IPv4. Note that changing

irc/cloaks/cloak_test.go

@@ -27,7 +27,7 @@ func cloakConfForTesting() CloakConfig {
 	config := CloakConfig{
 		Enabled:     true,
 		Netname:     "oragono",
-		Secret:      "_BdVPWB5sray7McbFmeuJL996yaLgG4l9tEyficGXKg",
+		secret:      "_BdVPWB5sray7McbFmeuJL996yaLgG4l9tEyficGXKg",
 		CidrLenIPv4: 32,
 		CidrLenIPv6: 64,
 		NumBits:     80,
@@ -55,7 +55,7 @@ func TestCloakDeterminism(t *testing.T) {
 	assertEqual(config.ComputeCloak(v6ipdifferentcidr), "ccmptyrjwsxv4f4d.oragono", t)
 
 	// cloak values must be sensitive to changes in the secret key
-	config.Secret = "HJcXK4lLawxBE4-9SIdPji_21YiL3N5r5f5-SPNrGVY"
+	config.SetSecret("HJcXK4lLawxBE4-9SIdPji_21YiL3N5r5f5-SPNrGVY")
 	assertEqual(config.ComputeCloak(v4ip), "4khy3usk8mfu42pe.oragono", t)
 	assertEqual(config.ComputeCloak(v6mappedIP), "4khy3usk8mfu42pe.oragono", t)
 	assertEqual(config.ComputeCloak(v6ip), "mxpk3c83vdxkek9j.oragono", t)
@@ -66,7 +66,7 @@ func TestCloakShortv4Cidr(t *testing.T) {
 	config := CloakConfig{
 		Enabled:     true,
 		Netname:     "oragono",
-		Secret:      "_BdVPWB5sray7McbFmeuJL996yaLgG4l9tEyficGXKg",
+		secret:      "_BdVPWB5sray7McbFmeuJL996yaLgG4l9tEyficGXKg",
 		CidrLenIPv4: 24,
 		CidrLenIPv6: 64,
 		NumBits:     60,

irc/cloaks/cloaks.go

@@ -5,7 +5,6 @@ package cloaks
 import (
 	"fmt"
 	"net"
-	"os"
 
 	"golang.org/x/crypto/sha3"
 
@@ -13,25 +12,22 @@ import (
 )
 
 type CloakConfig struct {
-	Enabled      bool
+	Enabled           bool
-	Netname      string
+	Netname           string
-	Secret       string
+	CidrLenIPv4       int    `yaml:"cidr-len-ipv4"`
-	SecretEnvVar string `yaml:"secret-environment-variable"`
+	CidrLenIPv6       int    `yaml:"cidr-len-ipv6"`
-	CidrLenIPv4  int    `yaml:"cidr-len-ipv4"`
+	NumBits           int    `yaml:"num-bits"`
-	CidrLenIPv6  int    `yaml:"cidr-len-ipv6"`
+	LegacySecretValue string `yaml:"secret"`
-	NumBits      int    `yaml:"num-bits"`
 
+	secret   string
 	numBytes int
 	ipv4Mask net.IPMask
 	ipv6Mask net.IPMask
 }
 
 func (cloakConfig *CloakConfig) Initialize() {
-	if cloakConfig.SecretEnvVar != "" {
+	if !cloakConfig.Enabled {
-		envSecret := os.Getenv(cloakConfig.SecretEnvVar)
+		return
-		if envSecret != "" {
-			cloakConfig.Secret = envSecret
-		}
 	}
 
 	// sanity checks:
@@ -52,15 +48,20 @@ func (cloakConfig *CloakConfig) Initialize() {
 	cloakConfig.ipv6Mask = net.CIDRMask(cloakConfig.CidrLenIPv6, 128)
 }
 
+func (cloakConfig *CloakConfig) SetSecret(secret string) {
+	cloakConfig.secret = secret
+}
+
 // simple cloaking algorithm: normalize the IP to its CIDR,
 // then hash the resulting bytes with a secret key,
 // then truncate to the desired length, b32encode, and append the fake TLD.
 func (config *CloakConfig) ComputeCloak(ip net.IP) string {
 	if !config.Enabled {
 		return ""
-	} else if config.NumBits == 0 {
+	} else if config.NumBits == 0 || config.secret == "" {
 		return config.Netname
 	}
+
 	var masked net.IP
 	v4ip := ip.To4()
 	if v4ip != nil {
@@ -70,9 +71,9 @@ func (config *CloakConfig) ComputeCloak(ip net.IP) string {
 	}
 	// SHA3(K || M):
 	// https://crypto.stackexchange.com/questions/17735/is-hmac-needed-for-a-sha-3-based-mac
-	input := make([]byte, len(config.Secret)+len(masked))
+	input := make([]byte, len(config.secret)+len(masked))
-	copy(input, config.Secret[:])
+	copy(input, config.secret[:])
-	copy(input[len(config.Secret):], masked)
+	copy(input[len(config.secret):], masked)
 	digest := sha3.Sum512(input)
 	b32digest := utils.B32Encoder.EncodeToString(digest[:config.numBytes])
 	return fmt.Sprintf("%s.%s", b32digest, config.Netname)

irc/config.go

@@ -1114,9 +1114,6 @@ func LoadConfig(filename string) (config *Config, err error) {
 
 	config.Server.Cloaks.Initialize()
 	if config.Server.Cloaks.Enabled {
-		if config.Server.Cloaks.Secret == "" || config.Server.Cloaks.Secret == "siaELnk6Kaeo65K3RCrwJjlWaZ-Bt3WuZ2L8MXLbNb4" {
-			return nil, fmt.Errorf("You must generate a new value of server.ip-cloaking.secret to enable cloaking")
-		}
 		if !utils.IsHostname(config.Server.Cloaks.Netname) {
 			return nil, fmt.Errorf("Invalid netname for cloaked hostnames: %s", config.Server.Cloaks.Netname)
 		}

irc/database.go

@@ -23,7 +23,9 @@ const (
 	// 'version' of the database schema
 	keySchemaVersion = "db.version"
 	// latest schema of the db
-	latestDbSchema = "10"
+	latestDbSchema = "11"
+
+	keyCloakSecret = "crypto.cloak_secret"
 )
 
 type SchemaChanger func(*Config, *buntdb.Tx) error
@@ -63,6 +65,7 @@ func initializeDB(path string) error {
 	err = store.Update(func(tx *buntdb.Tx) error {
 		// set schema version
 		tx.Set(keySchemaVersion, latestDbSchema, nil)
+		tx.Set(keyCloakSecret, utils.GenerateSecretKey(), nil)
 		return nil
 	})
 
@@ -186,6 +189,21 @@ func UpgradeDB(config *Config) (err error) {
 	return err
 }
 
+func LoadCloakSecret(db *buntdb.DB) (result string) {
+	db.View(func(tx *buntdb.Tx) error {
+		result, _ = tx.Get(keyCloakSecret)
+		return nil
+	})
+	return
+}
+
+func StoreCloakSecret(db *buntdb.DB, secret string) {
+	db.Update(func(tx *buntdb.Tx) error {
+		tx.Set(keyCloakSecret, secret, nil)
+		return nil
+	})
+}
+
 func schemaChangeV1toV2(config *Config, tx *buntdb.Tx) error {
 	// == version 1 -> 2 ==
 	// account key changes and account.verified key bugfix.
@@ -621,6 +639,17 @@ func schemaChangeV9ToV10(config *Config, tx *buntdb.Tx) error {
 	return nil
 }
 
+// #952: move the cloak secret into the database,
+// generate a new one if necessary
+func schemaChangeV10ToV11(config *Config, tx *buntdb.Tx) error {
+	cloakSecret := config.Server.Cloaks.LegacySecretValue
+	if cloakSecret == "" || cloakSecret == "siaELnk6Kaeo65K3RCrwJjlWaZ-Bt3WuZ2L8MXLbNb4" {
+		cloakSecret = utils.GenerateSecretKey()
+	}
+	_, _, err := tx.Set(keyCloakSecret, cloakSecret, nil)
+	return err
+}
+
 func init() {
 	allChanges := []SchemaChange{
 		{
@@ -668,6 +697,11 @@ func init() {
 			TargetVersion:  "10",
 			Changer:        schemaChangeV9ToV10,
 		},
+		{
+			InitialVersion: "10",
+			TargetVersion:  "11",
+			Changer:        schemaChangeV10ToV11,
+		},
 	}
 
 	// build the index

irc/hostserv.go

@@ -9,7 +9,10 @@ import (
 	"regexp"
 	"time"
 
+	"github.com/goshuirc/irc-go/ircfmt"
+
 	"github.com/oragono/oragono/irc/sno"
+	"github.com/oragono/oragono/irc/utils"
 )
 
 const (
@@ -171,6 +174,19 @@ the offered vhosts, use /HOSTSERV OFFERLIST.`,
 			minParams:    1,
 			maxParams:    1,
 		},
+		"setcloaksecret": {
+			handler: hsSetCloakSecretHandler,
+			help: `Syntax: $bSETCLOAKSECRET$b <secret> [code]
+
+SETCLOAKSECRET can be used to set or rotate the cloak secret. You should use
+a cryptographically strong secret. To prevent accidental modification, a
+verification code is required; invoking the command without a code will
+display the necessary code.`,
+			helpShort: `$bSETCLOAKSECRET$b modifies the IP cloaking secret.`,
+			capabs:    []string{"vhosts", "rehash"},
+			minParams: 1,
+			maxParams: 2,
+		},
 	}
 )
 
@@ -429,3 +445,15 @@ func hsTakeHandler(server *Server, client *Client, command string, params []stri
 		server.snomasks.Send(sno.LocalVhosts, fmt.Sprintf("Client %s (account %s) took vhost %s", client.Nick(), account, vhost))
 	}
 }
+
+func hsSetCloakSecretHandler(server *Server, client *Client, command string, params []string, rb *ResponseBuffer) {
+	secret := params[0]
+	expectedCode := utils.ConfirmationCode(secret, server.ctime)
+	if len(params) == 1 || params[1] != expectedCode {
+		hsNotice(rb, ircfmt.Unescape(client.t("$bWarning: changing the cloak secret will invalidate stored ban/invite/exception lists.$b")))
+		hsNotice(rb, fmt.Sprintf(client.t("To confirm, type: /HS SETCLOAKSECRET %[1]s %[2]s"), secret, expectedCode))
+		return
+	}
+	StoreCloakSecret(server.store, secret)
+	hsNotice(rb, client.t("Rotated the cloak secret; you must rehash or restart the server for it to take effect"))
+}

irc/server.go

@@ -550,9 +550,34 @@ func (server *Server) applyConfig(config *Config) (err error) {
 		}
 	}
 
+	server.logger.Info("server", "Using datastore", config.Datastore.Path)
+	if initial {
+		if err := server.loadDatastore(config); err != nil {
+			return err
+		}
+	} else {
+		if config.Datastore.MySQL.Enabled && config.Datastore.MySQL != oldConfig.Datastore.MySQL {
+			server.historyDB.SetConfig(config.Datastore.MySQL)
+		}
+	}
+
+	// now that the datastore is initialized, we can load the cloak secret from it
+	// XXX this modifies config after the initial load, which is naughty,
+	// but there's no data race because we haven't done SetConfig yet
+	if config.Server.Cloaks.Enabled {
+		config.Server.Cloaks.SetSecret(LoadCloakSecret(server.store))
+	}
+
 	// activate the new config
 	server.SetConfig(config)
 
+	// load [dk]-lines, registered users and channels, etc.
+	if initial {
+		if err := server.loadFromDatastore(config); err != nil {
+			return err
+		}
+	}
+
 	// burst new and removed caps
 	addedCaps, removedCaps := config.Diff(oldConfig)
 	var capBurstSessions []*Session
@@ -582,17 +607,6 @@ func (server *Server) applyConfig(config *Config) (err error) {
 		}
 	}
 
-	server.logger.Info("server", "Using datastore", config.Datastore.Path)
-	if initial {
-		if err := server.loadDatastore(config); err != nil {
-			return err
-		}
-	} else {
-		if config.Datastore.MySQL.Enabled && config.Datastore.MySQL != oldConfig.Datastore.MySQL {
-			server.historyDB.SetConfig(config.Datastore.MySQL)
-		}
-	}
-
 	server.setupPprofListener(config)
 
 	// set RPL_ISUPPORT
@@ -702,10 +716,13 @@ func (server *Server) loadDatastore(config *Config) error {
 	db, err := OpenDatabase(config)
 	if err == nil {
 		server.store = db
+		return nil
 	} else {
 		return fmt.Errorf("Failed to open datastore: %s", err.Error())
 	}
+}
 
+func (server *Server) loadFromDatastore(config *Config) (err error) {
 	// load *lines (from the datastores)
 	server.logger.Debug("server", "Loading D/Klines")
 	server.loadDLines()

oragono.go

@@ -17,7 +17,6 @@ import (
 	"github.com/oragono/oragono/irc"
 	"github.com/oragono/oragono/irc/logger"
 	"github.com/oragono/oragono/irc/mkcerts"
-	"github.com/oragono/oragono/irc/utils"
 	"golang.org/x/crypto/bcrypt"
 	"golang.org/x/crypto/ssh/terminal"
 )
@@ -97,7 +96,6 @@ Usage:
 	oragono upgradedb [--conf <filename>] [--quiet]
 	oragono genpasswd [--conf <filename>] [--quiet]
 	oragono mkcerts [--conf <filename>] [--quiet]
-	oragono mksecret [--conf <filename>] [--quiet]
 	oragono run [--conf <filename>] [--quiet] [--smoke]
 	oragono -h | --help
 	oragono --version
@@ -109,7 +107,7 @@ Options:
 
 	arguments, _ := docopt.ParseArgs(usage, nil, version)
 
-	// don't require a config file for genpasswd or mksecret
+	// don't require a config file for genpasswd
 	if arguments["genpasswd"].(bool) {
 		var password string
 		fd := int(os.Stdin.Fd())
@@ -135,9 +133,6 @@ Options:
 			fmt.Println()
 		}
 		return
-	} else if arguments["mksecret"].(bool) {
-		fmt.Println(utils.GenerateSecretKey())
-		return
 	} else if arguments["mkcerts"].(bool) {
 		doMkcerts(arguments["--conf"].(string), arguments["--quiet"].(bool))
 		return

oragono.yaml

@@ -254,17 +254,6 @@ server:
         # fake TLD at the end of the hostname, e.g., pwbs2ui4377257x8.oragono
         netname: "oragono"
 
-        # secret key to prevent dictionary attacks against cloaked IPs
-        # any high-entropy secret is valid for this purpose:
-        # you MUST generate a new one for your installation.
-        # suggestion: use the output of `oragono mksecret`
-        # note that rotating this key will invalidate all existing ban masks.
-        secret: "siaELnk6Kaeo65K3RCrwJjlWaZ-Bt3WuZ2L8MXLbNb4"
-
-        # name of an environment variable to pull the secret from, for use with
-        # k8s secret distribution:
-        # secret-environment-variable: "ORAGONO_CLOAKING_SECRET"
-
         # the cloaked hostname is derived only from the CIDR (most significant bits
         # of the IP address), up to a configurable number of bits. this is the
         # granularity at which bans will take effect for IPv4. Note that changing