Merge pull request #1635 from slingamn/pass

fix #1634 (forward-porting to master)

CHANGELOG.md

@@ -1,6 +1,22 @@
 # Changelog
 All notable changes to Oragono will be documented in this file.
 
+## [2.6.1] - 2021-04-26
+
+Oragono 2.6.1 is a bugfix release, fixing a security issue that is critical for some private server configurations. We regret the oversight.
+
+The issue affects two classes of server configuration:
+
+1. Private servers that use `server.password` (i.e., the `PASS` command) for protection. If `accounts.registration.allow-before-connect` is enabled, the `REGISTER` command can be used to bypass authentication. Affected operators should set this field to `false`, or upgrade to 2.6.1, which disallows the insecure configuration. (If the field does not appear in the configuration file, the configuration is secure since the value defaults to false when unset.)
+2. Private servers that use `accounts.require-sasl` for protection. If these servers do not additionally set `accounts.registration.enabled` to `false`, the `REGISTER` command can potentially be used to bypass authentication. Affected operators should set `accounts.registration.enabled` to false; this recommendation appeared in the operator manual but was not emphasized sufficiently. (Configurations that require SASL but allow open registration are potentially valid, e.g., in the case of public servers that require everyone to use a registered account; accordingly, Oragono 2.6.1 continues to permit such configurations.)
+
+This release includes no changes to the config file format or the database.
+
+Many thanks to [@ajaspers](https://github.com/ajaspers) for reporting the issue.
+
+### Security
+* Fixed and documented potential authentication bypasses via the `REGISTER` command (#1634, thanks [@ajaspers](https://github.com/ajaspers)!)
+
 ## [2.6.0] - 2021-04-18
 
 We're pleased to announce Oragono 2.6.0, a new stable release.

default.yaml

@@ -435,7 +435,10 @@ accounts:
     # require-sasl controls whether clients are required to have accounts
     # (and sign into them using SASL) to connect to the server
     require-sasl:
-        # if this is enabled, all clients must authenticate with SASL while connecting
+        # if this is enabled, all clients must authenticate with SASL while connecting.
+        # WARNING: for a private server, you MUST set accounts.registration.enabled
+        # to false as well, in order to prevent non-administrators from registering
+        # accounts.
         enabled: false
 
         # IPs/CIDRs which are exempted from the account requirement

docs/MANUAL.md

@@ -314,7 +314,7 @@ To enable this mode, set the following configs:
 
 This mode is comparable to Slack, Mattermost, or similar products intended as internal chat servers for an organization or team. In this mode, clients cannot connect to the server unless they log in with SASL as part of the initial handshake. This allows Oragono to be deployed facing the public Internet, with fine-grained control over who can log in.
 
-In this mode, clients must have a valid account to connect, so they cannot register their own accounts. Accordingly, an operator must do the initial account creation, using the `SAREGISTER` command of NickServ. (For more details, `/msg NickServ help saregister`.) To bootstrap this process, you can make an initial connection from localhost, which is exempt (by default) from the requirement, or temporarily add your own IP to the exemption list. You can also use a more permissive configuration for bootstrapping, then switch to this one once you have your account. Another possibility is permanently exempting an internal network, e.g., `10.0.0.0/8`, that only trusted people can access.
+In this mode, clients must not be allowed to register their own accounts, so user-initiated account registration must be disabled. Accordingly, an operator must do the initial account creation, using the `SAREGISTER` command of NickServ. (For more details, `/msg NickServ help saregister`.) To bootstrap this process, you can make an initial connection from localhost, which is exempt (by default) from the requirement, or temporarily add your own IP to the exemption list. You can also use a more permissive configuration for bootstrapping, then switch to this one once you have your account. Another possibility is permanently exempting an internal network, e.g., `10.0.0.0/8`, that only trusted people can access.
 
 To enable this mode, use the configs from the "nick equals account" section (i.e., start from `default.yaml`) and make these modifications:
 

irc/config.go

@@ -1319,6 +1319,19 @@ func LoadConfig(filename string) (config *Config, err error) {
 
 	config.Accounts.defaultUserModes = ParseDefaultUserModes(config.Accounts.DefaultUserModes)
 
+	if config.Server.Password != "" {
+		config.Server.passwordBytes, err = decodeLegacyPasswordHash(config.Server.Password)
+		if err != nil {
+			return nil, err
+		}
+		if config.Accounts.LoginViaPassCommand && !config.Accounts.SkipServerPassword {
+			return nil, errors.New("Using a server password and login-via-pass-command requires skip-server-password as well")
+		}
+		// #1634: accounts.registration.allow-before-connect is an auth bypass
+		// for configurations that start from default and then enable server.password
+		config.Accounts.Registration.AllowBeforeConnect = false
+	}
+
 	config.Accounts.RequireSasl.exemptedNets, err = utils.ParseNetList(config.Accounts.RequireSasl.Exempted)
 	if err != nil {
 		return nil, fmt.Errorf("Could not parse require-sasl exempted nets: %v", err.Error())
@@ -1409,16 +1422,6 @@ func LoadConfig(filename string) (config *Config, err error) {
 	// parse default channel modes
 	config.Channels.defaultModes = ParseDefaultChannelModes(config.Channels.DefaultModes)
 
-	if config.Server.Password != "" {
-		config.Server.passwordBytes, err = decodeLegacyPasswordHash(config.Server.Password)
-		if err != nil {
-			return nil, err
-		}
-		if config.Accounts.LoginViaPassCommand && !config.Accounts.SkipServerPassword {
-			return nil, errors.New("Using a server password and login-via-pass-command requires skip-server-password as well")
-		}
-	}
-
 	if config.Accounts.Registration.BcryptCost == 0 {
 		config.Accounts.Registration.BcryptCost = passwd.DefaultCost
 	}

traditional.yaml

@@ -407,7 +407,10 @@ accounts:
     # require-sasl controls whether clients are required to have accounts
     # (and sign into them using SASL) to connect to the server
     require-sasl:
-        # if this is enabled, all clients must authenticate with SASL while connecting
+        # if this is enabled, all clients must authenticate with SASL while connecting.
+        # WARNING: for a private server, you MUST set accounts.registration.enabled
+        # to false as well, in order to prevent non-administrators from registering
+        # accounts.
         enabled: false
 
         # IPs/CIDRs which are exempted from the account requirement