gharris1727 commented on PR #16788: URL: https://github.com/apache/kafka/pull/16788#issuecomment-2272264286
> We could possibly add retry logic for followers to wait for a session key to be published... this would probably require unrelated changes to this PR and can probably be considered out of scope. I agree, lets not do that. > Returning a 403 response instead of a 503 won't change the retry logic of the worker issuing the request. Ah, you're right. In that case, does the current 503 affect correctness in our tests? I would expect a failure to be followed by a successful retry. > I'd prefer not to revert the 503 logic, since it is a useful signal for people reading worker logs (including me... it really helped when debugging these test failures!), and we might even leverage it for retry logic in the future. This PR changes most of those 503s to 403s when validation is performed against stale keys, so I would argue this point works against this PR. Related to that, I did think of a security risk that the current PR has: If we're reading expired session keys during startup, there are brief periods of time when expired keys are used to validate requests. If at some point in the cluster's past a session key was leaked and had since undergone rotation, a malicious request using an expired key could pass validation. It's a very time-sensitive attack, but worth considering. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
