MirrorMirror
/
Signal-Server
mirror of https://github.com/signalapp/Signal-Server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Return 401 instead of 404 on unknown backup-ids

This commit is contained in:
Ravi Khadiwala 2024-04-02 15:03:24 -05:00 committed by ravi-signal
parent 1ebc17352f
commit 63c8b275d1
3 changed files with 17 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
service/src/main/java/org/whispersystems/textsecuregcm/backup/BackupManager.java
View File

@ -23,6 +23,7 @@ import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage; import java.util.concurrent.CompletionStage;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.StringUtils;
import org.signal.libsignal.protocol.ecc.Curve;
import org.signal.libsignal.protocol.ecc.ECPublicKey; import org.signal.libsignal.protocol.ecc.ECPublicKey;
import org.signal.libsignal.zkgroup.GenericServerSecretParams; import org.signal.libsignal.zkgroup.GenericServerSecretParams;
import org.signal.libsignal.zkgroup.VerificationFailedException; import org.signal.libsignal.zkgroup.VerificationFailedException;
@ -428,6 +429,7 @@ public class BackupManager {
.toFuture(); .toFuture();
} }
private static final ECPublicKey INVALID_PUBLIC_KEY = Curve.generateKeyPair().getPublicKey();
/** /**
* Authenticate the ZK anonymous backup credential's presentation * Authenticate the ZK anonymous backup credential's presentation
* <p> * <p>
@ -449,12 +451,13 @@ public class BackupManager {
.retrieveAuthenticationData(presentation.getBackupId()) .retrieveAuthenticationData(presentation.getBackupId())
.thenApply(optionalAuthenticationData -> { .thenApply(optionalAuthenticationData -> {
final BackupsDb.AuthenticationData authenticationData = optionalAuthenticationData final BackupsDb.AuthenticationData authenticationData = optionalAuthenticationData
.orElseThrow(() -> { .orElseGet(() -> {
Metrics.counter(ZK_AUTHN_COUNTER_NAME, Metrics.counter(ZK_AUTHN_COUNTER_NAME,
SUCCESS_TAG_NAME, String.valueOf(false), SUCCESS_TAG_NAME, String.valueOf(false),
FAILURE_REASON_TAG_NAME, "missing_public_key") FAILURE_REASON_TAG_NAME, "missing_public_key")
.increment(); .increment();
return Status.NOT_FOUND.withDescription("Backup not found").asRuntimeException(); // There was no stored public key, use a bunk public key so that validation will fail
return new BackupsDb.AuthenticationData(INVALID_PUBLIC_KEY, null, null);
}); });
return new AuthenticatedBackupUser( return new AuthenticatedBackupUser(
presentation.getBackupId(), presentation.getBackupId(),

5
service/src/main/java/org/whispersystems/textsecuregcm/controllers/ArchiveController.java
View File

@ -170,7 +170,10 @@ public class ArchiveController {
@ApiResponse( @ApiResponse(
responseCode = "403", responseCode = "403",
description = "Forbidden. The request had insufficient permissions to perform the requested action") description = "Forbidden. The request had insufficient permissions to perform the requested action")
@ApiResponse(responseCode = "401", description = "The provided backup auth credential presentation could not be verified") @ApiResponse(responseCode = "401", description = """
The provided backup auth credential presentation could not be verified or
The public key signature was invalid or
There is no backup associated with the backup-id in the presentation""")
@ApiResponse(responseCode = "400", description = "Bad arguments. The request may have been made on an authenticated channel") @ApiResponse(responseCode = "400", description = "Bad arguments. The request may have been made on an authenticated channel")
@interface ApiResponseZkAuth {} @interface ApiResponseZkAuth {}

13
service/src/test/java/org/whispersystems/textsecuregcm/backup/BackupManagerTest.java
View File

@ -226,7 +226,7 @@ public class BackupManagerTest {
StatusRuntimeException.class, StatusRuntimeException.class,
backupManager.authenticateBackupUser(presentation, signature)) backupManager.authenticateBackupUser(presentation, signature))
.getStatus().getCode()) .getStatus().getCode())
.isEqualTo(Status.NOT_FOUND.getCode()); .isEqualTo(Status.UNAUTHENTICATED.getCode());
} }
@Test @Test
@ -306,10 +306,10 @@ public class BackupManagerTest {
// should be rejected the day after that // should be rejected the day after that
testClock.pin(Instant.ofEpochSecond(1).plus(Duration.ofDays(3))); testClock.pin(Instant.ofEpochSecond(1).plus(Duration.ofDays(3)));
assertThat(CompletableFutureTestUtil.assertFailsWithCause( assertThatExceptionOfType(StatusRuntimeException.class)
StatusRuntimeException.class, .isThrownBy(() -> backupManager.authenticateBackupUser(oldCredential, signature))
backupManager.authenticateBackupUser(oldCredential, signature)) .extracting(StatusRuntimeException::getStatus)
.getStatus().getCode()) .extracting(Status::getCode)
.isEqualTo(Status.UNAUTHENTICATED.getCode()); .isEqualTo(Status.UNAUTHENTICATED.getCode());
} }
@ -752,6 +752,7 @@ public class BackupManagerTest {
} catch (InvalidKeyException e) { } catch (InvalidKeyException e) {
throw new RuntimeException(e); throw new RuntimeException(e);
} }
return new AuthenticatedBackupUser(backupId, backupTier, BackupsDb.generateDirName(secureRandom), BackupsDb.generateDirName(secureRandom)); return new AuthenticatedBackupUser(backupId, backupTier, BackupsDb.generateDirName(secureRandom),
BackupsDb.generateDirName(secureRandom));
} }
} }