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.

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

k/server: Better handling of SCRAM parsing errors #12042

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

michael-redpanda
Copy link
Contributor

@michael-redpanda michael-redpanda commented Jul 12, 2023

Before this change, if Redpanda received a badly formatted SCRAM message, it would not respond with an error and the offending client would timeout with no error code. This change will now return an error code and print a more helpful message to the logs.

Fixes: #11349

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.1.x
  • v22.3.x
  • v22.2.x

Release Notes

Improvements

  • Redpanda will now gracefully handle badly formatted SCRAM authentication messages

Before this change, if Redpanda received a badly formatted SCRAM
message, it would not respond with an error and the offending client
would timeout with no error code.  This change will now return an error
code and print a more helpful message to the logs.

Fixes: redpanda-data#11349

Signed-off-by: Michael Boquard <[email protected]>
@michael-redpanda
Copy link
Contributor Author

Example of new error message:

WARN  2023-07-11 21:46:52,044 [shard 0] kafka - server.cc:392 - [127.0.0.1:45364]  Error processing SASL authentication request for rpk: security::scram_exception (scram_algorithm.cc:201 - Invalid SCRAM client first message: n,,n=,r=xaMAKQUpxVDWrxkHYx88H/cuqCU)

Copy link
Member

@BenPope BenPope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tmgstevens
Copy link

Can this include the client_id? Do we have that?

@michael-redpanda
Copy link
Contributor Author

michael-redpanda commented Jul 12, 2023

Can this include the client_id? Do we have that?

It does:

[{IP}:{PORT}] Error processing SASL authentication request for {client.id}

https://github.com/michael-redpanda/redpanda/blob/5ec43ff19c1b5cf7f847e581c224db940611a3b0/src/v/kafka/server/server.cc#L392-L398

@tmgstevens
Copy link

It does:

[{IP}:{PORT}] Error processing SASL authentication request for {client.id}

Ah yes - got it!

LGTM, thanks for getting this in.

@michael-redpanda michael-redpanda merged commit f5a0b0d into redpanda-data:dev Jul 12, 2023
34 of 35 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v23.1.x

@vbotbuildovich
Copy link
Collaborator

/backport v22.3.x

@vbotbuildovich
Copy link
Collaborator

/backport v22.2.x

@vbotbuildovich
Copy link
Collaborator

Failed to run cherry-pick command. I executed the commands below:

git checkout -b backport-pr-12042-v22.3.x-660 remotes/upstream/v22.3.x
git cherry-pick -x 5ec43ff19c1b5cf7f847e581c224db940611a3b0

Workflow run logs.

@vbotbuildovich
Copy link
Collaborator

Failed to run cherry-pick command. I executed the commands below:

git checkout -b backport-pr-12042-v22.2.x-839 remotes/upstream/v22.2.x
git cherry-pick -x 5ec43ff19c1b5cf7f847e581c224db940611a3b0

Workflow run logs.

@Neustradamus
Copy link

Good job!

Linked to:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve handling of invalid SASL SCRAM messages
7 participants