Remove trailing NUL byte from AUTH LOGIN challenges (Fixes #566)#620
Open
apoorvdarshan wants to merge 1 commit into
Open
Remove trailing NUL byte from AUTH LOGIN challenges (Fixes #566)#620apoorvdarshan wants to merge 1 commit into
apoorvdarshan wants to merge 1 commit into
Conversation
The AUTH LOGIN username/password challenge constants carried a trailing
NUL byte ("User Name\x00" / "Password\x00"). Base64-encoded and sent as
the 334 challenge, these produced "VXNlciBOYW1lAA==" / "UGFzc3dvcmQA",
whose decoded value contains an embedded '\x00'. Strict clients such as
Go's net/smtp (used by Prometheus Alertmanager) and curl do an exact
compare on the decoded challenge and abort AUTH, yielding errors like
"unexpected server challenge".
Drop the trailing NUL from both constants so the challenges decode to
"User Name" / "Password". Update the matching test-support status codes
and add a regression test asserting the base64 challenges contain no NUL.
Fixes aio-libs#566.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What do these changes do?
The
AUTH LOGINusername and password challenge constants inSMTPcarried a trailing NUL byte:Base64-encoded and sent as the
334challenge, these produceVXNlciBOYW1lAA==/UGFzc3dvcmQA, whose decoded value contains an embedded\x00(User Name\x00/Password\x00).Strict SMTP clients do an exact string compare on the decoded challenge and abort AUTH when it contains the NUL byte. This breaks:
net/smtp(used by Prometheus Alertmanager v0.27+), which errors withunexpected server challengeObserved on the wire:
Fix
Remove the trailing
\x00from both class constants so the challenges decode toUser Name/Password:The
challenge_auth()call sites need no change. The base64 challenge values in theSMTP_STATUS_CODEStest-support module (S334_AUTH_USERNAME,S334_AUTH_PASSWORD) are updated to match the new, NUL-free encoding.This is the minimal correctness fix for the null-byte problem reported in the issue. Changing the challenge text to
Username:/Password:(the issue's second, optional suggestion) is intentionally left out, as that is a separate behavioral/design decision.Are there changes in behavior for the user?
The
334 AUTH LOGINchallenges no longer contain a trailing NUL byte, so strict clients (Gonet/smtp, curl) can completeAUTH LOGIN. The plaintext prompts remainUser Name/Password.Related issue number
Fixes #566
Tests
test_login_challenges_have_no_nul_byte, which performs anAUTH LOGINexchange and asserts the base64-decoded username and password challenges contain no\x00. It fails on the pristine code (assert b'\x00' not in b'User Name\x00') and passes with the fix.NEWS.rstentry was added under1.4.7 (aiosmtpd-next).Disclosure: prepared with AI assistance; reviewed and verified locally.