sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
951 stars 405 forks source link

SASL EXTERNAL incorrectly requires a password to be set #2560

Closed dgw closed 7 months ago

dgw commented 7 months ago

Description

SASL EXTERNAL doesn't work correctly unless e.g. auth_password is set.

Big capability refactor (#2341, specifically 0401ae9f9e6745ee7b899f8620fcea0d0a61c056) likely caused this, but I didn't go back and test an older revision. Just building on @half-duplex's sleuthing from a session working on #2362 earlier tonight.

Reproduction steps

  1. Provide a client_cert_file path (.pem file), auth_method = sasl and auth_target = EXTERNAL in Sopel's config file. a. I registered the certificate fingerprint with a NickServ account for completeness, but this shouldn't be required to repro.
  2. Start Sopel and wait for it to connect.
  3. Sopel will quit with an error:
[2023-11-12 01:11:12,806] sopel.bot            INFO     - Client capability negotiation list: away-notify, chghost, invite-notify, multi-prefix, sasl, userhost-in-names
[2023-11-12 01:11:12,839] sopel.coretasks      ERROR    - Configuration error on ACK capability "sasl": ConfigurationError: SASL authentication required but no password available; please check your configuration file.
[2023-11-12 01:11:12,871] sopel.irc            ERROR    - ERROR received from server: Closing Link: your.host.name ()

Expected behavior

Successful SASL EXTERNAL authentication:

[2023-11-12 01:11:42,497] sopel.irc            INFO     - Connected, initiating setup sequence
[2023-11-12 01:11:43,763] sopel.bot            INFO     - Client capability negotiation list: away-notify, chghost, invite-notify, multi-prefix, sasl, userhost-in-names
[2023-11-12 01:11:43,920] sopel.coretasks      INFO     - Successful SASL Auth.
[2023-11-12 01:11:43,920] sopel.bot            INFO     - End of client capability negotiation requests.
[2023-11-12 01:11:44,001] sopel.coretasks      INFO     - Enabled client capabilities: chghost, multi-prefix, userhost-in-names, away-notify, sasl

Relevant logs

No response

Notes

@half-duplex added a very simple patch for this already in #2362 (see below). I'm just opening this issue so we don't forget about this bug, regardless of whether SCRAM-SHA-256 support makes it into 8.0.0 or not.

diff --git a/sopel/coretasks.py b/sopel/coretasks.py
index 22f7fe1f..97c44849 100644
--- a/sopel/coretasks.py
+++ b/sopel/coretasks.py
@@ -112,7 +112,7 @@ def _handle_sasl_capability(
         return plugin.CapabilityNegotiation.ERROR
     # Check SASL configuration (password is required)
     password, mech = _get_sasl_pass_and_mech(bot)
-    if not password:
+    if mech != "EXTERNAL" and not password:
         raise config.ConfigurationError(
             'SASL authentication required but no password available; '
             'please check your configuration file.',

Sopel version

51300a1ab854d6ec82d90df1bc876188c03335ff

Installation method

pip install

Python version

No response

Operating system

No response

IRCd

No response

Relevant plugins

No response

Neustradamus commented 7 months ago

@dgw: To follow this ticket :)

dgw commented 7 months ago

@Neustradamus Please just use the button to subscribe to an issue instead of mentioning people for no reason

(This also goes for https://github.com/sopel-irc/sopel/issues/2501#issuecomment-1807072947 but I won't comment the same twice)