From 97ba1c3d63052d4ccaad602c7df84acd116e27b2 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 25 Apr 2021 19:22:08 -0400 Subject: [PATCH] fix #1634: 1. Fix auth bypass in the default configuration with the addition of server.password (the REGISTER command was allowed before connection registration, allowing unauthenticated users to REGISTER and then take advantage of skip-server-password) 2. Caution operators against the use of require-sasl without disabling user-initiated account registration. (Such a configuration is still valid in the case of a public server that requires everyone to register.) --- CHANGELOG.md | 16 ++++++++++++++++ default.yaml | 5 ++++- docs/MANUAL.md | 2 +- irc/config.go | 23 +++++++++++++---------- traditional.yaml | 5 ++++- 5 files changed, 38 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 928c154d..e7ab3c74 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/default.yaml b/default.yaml index 87a91a57..22f9c698 100644 --- a/default.yaml +++ b/default.yaml @@ -433,7 +433,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 diff --git a/docs/MANUAL.md b/docs/MANUAL.md index 8913c66d..3b8d12cd 100644 --- a/docs/MANUAL.md +++ b/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: diff --git a/irc/config.go b/irc/config.go index d421a073..81455fae 100644 --- a/irc/config.go +++ b/irc/config.go @@ -1299,6 +1299,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()) @@ -1389,16 +1402,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 } diff --git a/traditional.yaml b/traditional.yaml index 47601ac4..9889c2ac 100644 --- a/traditional.yaml +++ b/traditional.yaml @@ -405,7 +405,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