Skip to content

Commit

Permalink
Use constant-time comparison for checking request cookie
Browse files Browse the repository at this point in the history
Guards against timing attacks to guess the cookie.

PiperOrigin-RevId: 363210285
  • Loading branch information
michajlo authored and copybara-github committed Mar 16, 2021
1 parent 8e9d300 commit 8d4663a
Showing 1 changed file with 15 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import java.io.OutputStream;
import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.SecureRandom;
import java.util.Optional;
import java.util.concurrent.Executor;
Expand Down Expand Up @@ -473,7 +474,7 @@ private void writeServerFile(String name, String contents) throws AbruptExitExce
}

private void executeCommand(RunRequest request, BlockingStreamObserver<RunResponse> observer) {
boolean badCookie = !request.getCookie().equals(requestCookie);
boolean badCookie = !isValidRequestCookie(request.getCookie());
if (badCookie || request.getClientDescription().isEmpty()) {
try {
FailureDetail failureDetail =
Expand Down Expand Up @@ -616,7 +617,7 @@ public void run(final RunRequest request, final StreamObserver<RunResponse> obse
public void ping(PingRequest pingRequest, StreamObserver<PingResponse> streamObserver) {
try (RunningCommand command = commandManager.createCommand()) {
PingResponse.Builder response = PingResponse.newBuilder();
if (pingRequest.getCookie().equals(requestCookie)) {
if (isValidRequestCookie(pingRequest.getCookie())) {
response.setCookie(responseCookie);
}

Expand All @@ -629,7 +630,7 @@ public void ping(PingRequest pingRequest, StreamObserver<PingResponse> streamObs
public void cancel(
final CancelRequest request, final StreamObserver<CancelResponse> streamObserver) {
logger.atInfo().log("Got CancelRequest for command id %s", request.getCommandId());
if (!request.getCookie().equals(requestCookie)) {
if (!isValidRequestCookie(request.getCookie())) {
streamObserver.onCompleted();
return;
}
Expand All @@ -651,6 +652,17 @@ private void doCancel(CancelRequest request, StreamObserver<CancelResponse> stre
}
}

/**
* Returns whether or not the provided cookie is valid for this server using a constant-time
* comparison in order to guard against timing attacks.
*/
private boolean isValidRequestCookie(String incomingRequestCookie) {
// Note that cookie file was written as latin-1, so use that here.
return MessageDigest.isEqual(
incomingRequestCookie.getBytes(StandardCharsets.ISO_8859_1),
requestCookie.getBytes(StandardCharsets.ISO_8859_1));
}

private static AbruptExitException createFilesystemFailureException(
String message, IOException e) {
return new AbruptExitException(
Expand Down

0 comments on commit 8d4663a

Please sign in to comment.