Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SASL EXTERNAL incorrectly requires a password to be set #2560

Closed
dgw opened this issue Nov 12, 2023 · 2 comments · Fixed by #2561 · May be fixed by #2362
Closed

SASL EXTERNAL incorrectly requires a password to be set #2560

dgw opened this issue Nov 12, 2023 · 2 comments · Fixed by #2561 · May be fixed by #2362
Labels
Bug Things to squish; generally used for issues
Milestone
8.0.0

Comments

@dgw
Copy link
Member

dgw commented Nov 12, 2023

Description

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

Big capability refactor (#2341, specifically 0401ae9) 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

51300a1

Installation method

pip install

Python version

No response

Operating system

No response

IRCd

No response

Relevant plugins

No response
@dgw dgw added the Bug Things to squish; generally used for issues label Nov 12, 2023
@dgw dgw added this to the 8.0.0 milestone Nov 12, 2023
@dgw dgw linked a pull request Nov 12, 2023 that will close this issue
coretasks: implement scram-sha-256 #2362
Open
4 tasks
@Neustradamus

This comment has been minimized.

Sign in to view
@half-duplex half-duplex mentioned this issue Nov 12, 2023
Merged
4 tasks
@dgw

This comment has been minimized.

Sign in to view
@dgw dgw closed this as completed in #2561 Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Things to squish; generally used for issues
Projects
None yet
Milestone
8.0.0
2 participants
@Neustradamus @dgw