Safer ExternalPermutationWorkerFactory connections#10257
Safer ExternalPermutationWorkerFactory connections#10257niloc132 wants to merge 2 commits intogwtproject:mainfrom
Conversation
Cookies are now sent/received as 32 ascii chars before any ObjectOutputStream data is sent, and the server validates before returning the socket. An invalid socket is closed and ignored, and the next valid socket will be used instead. Also improved cookie randomness source. Fixes gwtproject#10256
vegegoku
left a comment
vegegoku
left a comment
There was a problem hiding this comment.
LGTM, might need to fix the checkstyle warnings.
| if (--accepts == 0) { | ||
| sock.close(); | ||
| sock = null; | ||
| while (!validCookies.isEmpty()) { |
There was a problem hiding this comment.
The old code did throw an error on invalid "cookie", this seems to retry until a valid "cookie" is sent, is that an intentional change?
There was a problem hiding this comment.
Yes - rather than permit a "denial of service" where an attacking client can guess the port but get the cookie wrong and thus kill the compile job, this implementation just ignores the client since they aren't connecting correctly anyway.
If I've done this right, we still warn about the bad connection, but we don't let it kill the compiler. The only way kill the job is if it was the right client, but somehow had the wrong cookie, which would have failed in the old code too.
There was a problem hiding this comment.
Restating this slightly:
- "good" clients will effectively never have this issue, but if they do, we will get a warning, and the client itself will crash out.
- "bad" clients will no longer be able to just guess one of the outstanding cookies and take down the server. Technically a malicious client could guess and guess and never be rate limited... but they have 128 bits of search space to work through, which is infeasible.
3 participants
Cookies are now sent/received as 32 ascii chars before any ObjectOutputStream data is sent, and the server validates before returning the socket. An invalid socket is closed and ignored, and the next valid socket will be used instead.
Also improved cookie randomness source.
Fixes #10256