Add two-phase commit support for SVR2.

.idea/codeStyles/Project.xml

@@ -212,5 +212,12 @@
         </rules>
       </arrangement>
     </codeStyleSettings>
+    <codeStyleSettings language="kotlin">
+      <indentOptions>
+        <option name="INDENT_SIZE" value="2" />
+        <option name="CONTINUATION_INDENT_SIZE" value="2" />
+        <option name="TAB_SIZE" value="2" />
+      </indentOptions>
+    </codeStyleSettings>
   </code_scheme>
 </component>

app/build.gradle

@@ -185,7 +185,7 @@ android {
         buildConfigField "String", "SIGNAL_CDSI_URL", "\"https://cdsi.signal.org\""
         buildConfigField "String", "SIGNAL_SERVICE_STATUS_URL", "\"uptime.signal.org\""
         buildConfigField "String", "SIGNAL_KEY_BACKUP_URL", "\"https://api.backup.signal.org\""
-        buildConfigField "String", "SIGNAL_SVR2_URL", "\"https://svr2.staging.signal.org\""
+        buildConfigField "String", "SIGNAL_SVR2_URL", "\"https://svr2.signal.org\""
         buildConfigField "String", "SIGNAL_SFU_URL", "\"https://sfu.voip.signal.org\""
         buildConfigField "String", "SIGNAL_STAGING_SFU_URL", "\"https://sfu.staging.voip.signal.org\""
         buildConfigField "String[]", "SIGNAL_SFU_INTERNAL_NAMES", "new String[]{\"Test\", \"Staging\", \"Development\"}"
@@ -201,7 +201,7 @@ android {
         buildConfigField "String[]", "SIGNAL_CONTENT_PROXY_IPS", content_proxy_ips
         buildConfigField "String", "SIGNAL_AGENT", "\"OWA\""
         buildConfigField "String", "CDSI_MRENCLAVE", "\"0f6fd79cdfdaa5b2e6337f534d3baf999318b0c462a7ac1f41297a3e4b424a57\""
-        buildConfigField "String", "SVR2_MRENCLAVE", "\"dc9fd472a5a9c871a3c7f76f1af60aa9c1f314abf2e8d1e0c4ba25c8aaa2848c\""
+        buildConfigField "String", "SVR2_MRENCLAVE", "\"6ee1042f9e20f880326686dd4ba50c25359f01e9f733eeba4382bca001d45094\""
         buildConfigField "org.thoughtcrime.securesms.KbsEnclave", "KBS_ENCLAVE", "new org.thoughtcrime.securesms.KbsEnclave(\"e18376436159cda3ad7a45d9320e382e4a497f26b0dca34d8eab0bd0139483b5\", " +
                                                                                                                            "\"3a485adb56e2058ef7737764c738c4069dd62bc457637eafb6bbce1ce29ddb89\", " +
                                                                                                                            "\"45627094b2ea4a66f4cf0b182858a8dcf4b8479122c3820fe7fd0551a6d4cf5c\")"
@@ -378,6 +378,8 @@ android {
             buildConfigField "String", "SIGNAL_CDN2_URL", "\"https://cdn2-staging.signal.org\""
             buildConfigField "String", "SIGNAL_CDSI_URL", "\"https://cdsi.staging.signal.org\""
             buildConfigField "String", "SIGNAL_KEY_BACKUP_URL", "\"https://api-staging.backup.signal.org\""
+            buildConfigField "String", "SIGNAL_SVR2_URL", "\"https://svr2.staging.signal.org\""
+            buildConfigField "String", "SVR2_MRENCLAVE", "\"a8a261420a6bb9b61aa25bf8a79e8bd20d7652531feb3381cbffd446d270be95\""
             buildConfigField "org.thoughtcrime.securesms.KbsEnclave", "KBS_ENCLAVE", "new org.thoughtcrime.securesms.KbsEnclave(\"39963b736823d5780be96ab174869a9499d56d66497aa8f9b2244f777ebc366b\", " +
                                                                                                                                "\"9dbc6855c198e04f21b5cc35df839fdcd51b53658454dfa3f817afefaffc95ef\", " +
                                                                                                                                "\"45627094b2ea4a66f4cf0b182858a8dcf4b8479122c3820fe7fd0551a6d4cf5c\")"

libsignal/service/src/main/java/org/whispersystems/signalservice/api/svr/SecureValueRecoveryV2.kt

@@ -3,9 +3,9 @@ package org.whispersystems.signalservice.api.svr
 import io.reactivex.rxjava3.core.Single
 import io.reactivex.rxjava3.schedulers.Schedulers
 import okio.ByteString.Companion.toByteString
-import org.signal.libsignal.svr2.PinHash
 import org.signal.svr2.proto.BackupRequest
 import org.signal.svr2.proto.DeleteRequest
+import org.signal.svr2.proto.ExposeRequest
 import org.signal.svr2.proto.Request
 import org.signal.svr2.proto.RestoreRequest
 import org.whispersystems.signalservice.api.crypto.InvalidCiphertextException
@@ -13,9 +13,11 @@ import org.whispersystems.signalservice.api.kbs.MasterKey
 import org.whispersystems.signalservice.api.kbs.PinHashUtil
 import org.whispersystems.signalservice.api.push.exceptions.NonSuccessfulResponseCodeException
 import org.whispersystems.signalservice.internal.configuration.SignalServiceConfiguration
+import org.whispersystems.signalservice.internal.push.AuthCredentials
 import org.whispersystems.signalservice.internal.push.PushServiceSocket
 import java.io.IOException
 import org.signal.svr2.proto.BackupResponse as ProtoBackupResponse
+import org.signal.svr2.proto.ExposeResponse as ProtoExposeResponse
 import org.signal.svr2.proto.RestoreResponse as ProtoRestoreResponse
 
 /**
@@ -28,45 +30,20 @@ class SecureValueRecoveryV2(
 ) {
 
   /**
-   * Sets the provided data on the SVR service with the provided PIN.
+   * Begins a PIN change.
+   *
+   * Under the hood, setting a PIN is a two-phase process. This is abstracted through the [PinChangeSession].
+   * To use it, simply call [PinChangeSession.execute], which will return the result of the operation.
+   * If the operation is not successful and warrants a retry, it is extremely important to use the same [PinChangeSession].
+   *
+   * Do not have any automated retry system that calls [setPin] unconditionally. Always reuse the same [PinChangeSession]
+   * for as long as it is still valid (i.e. as long as you're still trying to set the same PIN).
    *
    * @param pin The user-specified PIN.
    * @param masterKey The data to set on SVR.
    */
-  fun setPin(pin: PinHash, masterKey: MasterKey): Single<BackupResponse> {
+  fun setPin(userPin: String, masterKey: MasterKey): PinChangeSession {
-    val data = PinHashUtil.createNewKbsData(pin, masterKey)
+    return PinChangeSession(userPin, masterKey)
-
-    val request = Request(
-      backup = BackupRequest(
-        pin = data.kbsAccessKey.toByteString(),
-        data_ = data.cipherText.toByteString(),
-        maxTries = 10
-      )
-    )
-
-    return getAuthorization()
-      .flatMap { auth -> Svr2Socket(serviceConfiguration, mrEnclave).makeRequest(auth, request) }
-      .map { response ->
-        when (response.backup?.status) {
-          ProtoBackupResponse.Status.OK -> {
-            BackupResponse.Success
-          }
-          ProtoBackupResponse.Status.REQUEST_INVALID -> {
-            BackupResponse.ApplicationError(InvalidRequestException("BackupResponse returned status code for REQUEST_INVALID"))
-          }
-          else -> {
-            BackupResponse.ApplicationError(IllegalStateException("Unknown status: ${response.backup?.status}"))
-          }
-        }
-      }
-      .onErrorReturn { throwable ->
-        when (throwable) {
-          is NonSuccessfulResponseCodeException -> BackupResponse.ApplicationError(throwable)
-          is IOException -> BackupResponse.NetworkError(throwable)
-          else -> BackupResponse.ApplicationError(throwable)
-        }
-      }
-      .subscribeOn(Schedulers.io())
   }
 
   /**
@@ -76,22 +53,22 @@ class SecureValueRecoveryV2(
    *
    * If the user is already registered, use [restoreDataPostRegistration]
    */
-  fun restoreDataPreRegistration(authorization: String, pinHash: PinHash): Single<RestoreResponse> {
+  fun restoreDataPreRegistration(authorization: AuthCredentials, userPin: String): Single<RestoreResponse> {
-    return restoreData(Single.just(authorization), pinHash)
+    return restoreData(Single.just(authorization), userPin)
   }
 
   /**
    * Restores data from SVR. Only intended to be called if the user is already registered. If the user is not yet registered, use [restoreDataPreRegistration]
    */
-  fun restoreDataPostRegistration(pinHash: PinHash): Single<RestoreResponse> {
+  fun restoreDataPostRegistration(userPin: String): Single<RestoreResponse> {
-    return restoreData(getAuthorization(), pinHash)
+    return restoreData(getAuthorization(), userPin)
   }
 
   /**
    * Deletes the user's SVR data from the service.
    */
   fun deleteData(): Single<DeleteResponse> {
-    val request = Request(delete = DeleteRequest())
+    val request: (Svr2PinHasher) -> Request = { Request(delete = DeleteRequest()) }
 
     return getAuthorization()
       .flatMap { auth -> Svr2Socket(serviceConfiguration, mrEnclave).makeRequest(auth, request) }
@@ -106,18 +83,27 @@ class SecureValueRecoveryV2(
       .subscribeOn(Schedulers.io())
   }
 
-  private fun restoreData(authorization: Single<String>, pinHash: PinHash): Single<RestoreResponse> {
+  private fun restoreData(authorization: Single<AuthCredentials>, userPin: String): Single<RestoreResponse> {
-    val request = Request(
+    val normalizedPin: ByteArray = PinHashUtil.normalize(userPin)
-      restore = RestoreRequest(pin = pinHash.accessKey().toByteString())
-    )
 
     return authorization
-      .flatMap { auth -> Svr2Socket(serviceConfiguration, mrEnclave).makeRequest(auth, request) }
+      .flatMap { auth ->
-      .map { response ->
+        Svr2Socket(serviceConfiguration, mrEnclave).makeRequest(auth) { pinHasher ->
+          val pinHash = pinHasher.hash(normalizedPin)
+
+          Request(
+            restore = RestoreRequest(
+              pin = pinHash.accessKey().toByteString()
+            )
+          )
+        }
+      }
+      .map { (response, pinHasher) ->
         when (response.restore?.status) {
           ProtoRestoreResponse.Status.OK -> {
             val ciphertext: ByteArray = response.restore.data_.toByteArray()
             try {
+              val pinHash = pinHasher.hash(normalizedPin)
               val masterKey: MasterKey = PinHashUtil.decryptKbsDataIVCipherText(pinHash, ciphertext).masterKey
               RestoreResponse.Success(masterKey)
             } catch (e: InvalidCiphertextException) {
@@ -148,15 +134,126 @@ class SecureValueRecoveryV2(
       .subscribeOn(Schedulers.io())
   }
 
-  private fun getAuthorization(): Single<String> {
+  private fun getAuthorization(): Single<AuthCredentials> {
     return Single.fromCallable { pushServiceSocket.svr2Authorization }
   }
 
+  /**
+   * This class is responsible for doing all the work necessary for changing a PIN.
+   *
+   * It's primary purpose is to serve as an abstraction over the fact that there are actually two separate requests that need to be made:
+   *
+   * (1) Create the backup data (which resets the guess count), and
+   * (2) Expose that data, making it eligible to be restored.
+   *
+   * The first should _never_ be retried after it completes successfully, and this class will help ensure that doesn't happen by doing the
+   * proper bookkeeping.
+   */
+  inner class PinChangeSession(
+    val userPin: String,
+    val masterKey: MasterKey,
+    private var setupComplete: Boolean = false
+  ) {
+
+    /**
+     * Performs the PIN change operation. This is safe to call repeatedly if you get back a retryable error.
+     */
+    fun execute(): Single<BackupResponse> {
+      val normalizedPin: ByteArray = PinHashUtil.normalize(userPin)
+
+      return getAuthorization()
+        .flatMap { auth ->
+          if (setupComplete) {
+            Single.just(auth to ProtoBackupResponse(status = ProtoBackupResponse.Status.OK))
+          } else {
+            getBackupResponse(auth, normalizedPin).map { auth to it }
+          }
+        }
+        .doOnSuccess { (_, response) ->
+          if (response.status == ProtoBackupResponse.Status.OK) {
+            setupComplete = true
+          }
+        }
+        .flatMap { (auth, response) ->
+          when (response.status) {
+            ProtoBackupResponse.Status.OK -> {
+              getExposeResponse(auth, normalizedPin)
+            }
+            ProtoBackupResponse.Status.REQUEST_INVALID -> {
+              Single.just(BackupResponse.ApplicationError(InvalidRequestException("BackupResponse returned status code for REQUEST_INVALID")))
+            }
+            else -> {
+              Single.just(BackupResponse.ApplicationError(IllegalStateException("Unknown status: ${response.status}")))
+            }
+          }
+        }
+        .onErrorReturn { throwable ->
+          when (throwable) {
+            is NonSuccessfulResponseCodeException -> BackupResponse.ApplicationError(throwable)
+            is IOException -> BackupResponse.NetworkError(throwable)
+            else -> BackupResponse.ApplicationError(throwable)
+          }
+        }
+        .subscribeOn(Schedulers.io())
+    }
+
+    private fun getBackupResponse(authorization: AuthCredentials, normalizedPin: ByteArray): Single<ProtoBackupResponse> {
+      val request: (Svr2PinHasher) -> Request = { pinHasher ->
+        val hashedPin = pinHasher.hash(normalizedPin)
+        val data = PinHashUtil.createNewKbsData(hashedPin, masterKey)
+
+        Request(
+          backup = BackupRequest(
+            pin = data.kbsAccessKey.toByteString(),
+            data_ = data.cipherText.toByteString(),
+            maxTries = 10
+          )
+        )
+      }
+
+      return Svr2Socket(serviceConfiguration, mrEnclave)
+        .makeRequest(authorization, request)
+        .map { (response, _) -> response.backup ?: throw IllegalStateException("Backup response not set!") }
+    }
+
+    private fun getExposeResponse(authorization: AuthCredentials, normalizedPin: ByteArray): Single<BackupResponse> {
+      val request: (Svr2PinHasher) -> Request = { pinHasher ->
+        val hashedPin = pinHasher.hash(normalizedPin)
+        val data = PinHashUtil.createNewKbsData(hashedPin, masterKey)
+
+        Request(
+          expose = ExposeRequest(
+            data_ = data.cipherText.toByteString()
+          )
+        )
+      }
+
+      return Svr2Socket(serviceConfiguration, mrEnclave)
+        .makeRequest(authorization, request)
+        .map { (response, _) ->
+          when (response.expose?.status) {
+            ProtoExposeResponse.Status.OK -> {
+              BackupResponse.Success
+            }
+            ProtoExposeResponse.Status.ERROR -> {
+              BackupResponse.ExposeFailure
+            }
+            else -> {
+              BackupResponse.ApplicationError(IllegalStateException("Backup response not set!"))
+            }
+          }
+        }
+    }
+  }
+
   /** Response for setting a PIN. */
   sealed class BackupResponse {
     /** Operation completed successfully. */
     object Success : BackupResponse()
 
+    /** The operation failed because the server was unable to expose the backup data we created. There is no further action that can be taken besides logging the error and treating it as a success. */
+    object ExposeFailure : BackupResponse()
+
     /** There as a network error. Not a bad response, but rather interference or some other inability to make a network request. */
     data class NetworkError(val exception: IOException) : BackupResponse()
 

libsignal/service/src/main/java/org/whispersystems/signalservice/api/svr/Svr2PinHasher.kt

@@ -0,0 +1,22 @@
+/*
+ * Copyright 2023 Signal Messenger, LLC
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+package org.whispersystems.signalservice.api.svr
+
+import org.signal.libsignal.svr2.PinHash
+import org.signal.libsignal.svr2.Svr2Client
+import org.whispersystems.signalservice.internal.push.AuthCredentials
+
+/**
+ * Encapsulates the various dependencies needed to create a [PinHash] in SVR2 without having to expose them.
+ */
+internal class Svr2PinHasher(
+  private val authCredentials: AuthCredentials,
+  private val client: Svr2Client
+) {
+  fun hash(normalizedPin: ByteArray): PinHash {
+    return client.hashPin(normalizedPin, authCredentials.username().toByteArray(Charsets.UTF_8))
+  }
+}

libsignal/service/src/main/java/org/whispersystems/signalservice/api/svr/Svr2Socket.kt

@@ -5,7 +5,6 @@ import io.reactivex.rxjava3.core.SingleEmitter
 import okhttp3.ConnectionSpec
 import okhttp3.OkHttpClient
 import okhttp3.Request
-import okhttp3.Response
 import okhttp3.WebSocket
 import okhttp3.WebSocketListener
 import okio.ByteString
@@ -21,6 +20,7 @@ import org.whispersystems.signalservice.api.util.Tls12SocketFactory
 import org.whispersystems.signalservice.api.util.TlsProxySocketFactory
 import org.whispersystems.signalservice.internal.configuration.SignalServiceConfiguration
 import org.whispersystems.signalservice.internal.configuration.SignalSvr2Url
+import org.whispersystems.signalservice.internal.push.AuthCredentials
 import org.whispersystems.signalservice.internal.util.BlacklistingTrustManager
 import org.whispersystems.signalservice.internal.util.Hex
 import org.whispersystems.signalservice.internal.util.Util
@@ -33,6 +33,7 @@ import java.util.concurrent.atomic.AtomicReference
 import javax.net.ssl.SSLContext
 import javax.net.ssl.SSLSocketFactory
 import javax.net.ssl.X509TrustManager
+import okhttp3.Response as OkHttpResponse
 import org.signal.svr2.proto.Request as Svr2Request
 import org.signal.svr2.proto.Response as Svr2Response
 
@@ -46,11 +47,11 @@ internal class Svr2Socket(
   private val svr2Url: SignalSvr2Url = chooseUrl(configuration.signalSvr2Urls)
   private val okhttp: OkHttpClient = buildOkHttpClient(configuration, svr2Url)
 
-  fun makeRequest(authorization: String, clientRequest: Svr2Request): Single<Svr2Response> {
+  fun makeRequest(authorization: AuthCredentials, clientRequest: (Svr2PinHasher) -> Svr2Request): Single<Response> {
     return Single.create { emitter ->
       val openRequest: Request.Builder = Request.Builder()
         .url("${svr2Url.url}/v1/$mrEnclave")
-        .addHeader("Authorization", authorization)
+        .addHeader("Authorization", authorization.asBasic())
 
       if (svr2Url.hostHeader.isPresent) {
         openRequest.addHeader("Host", svr2Url.hostHeader.get())
@@ -60,6 +61,7 @@ internal class Svr2Socket(
       val webSocket = okhttp.newWebSocket(
         openRequest.build(),
         SvrWebSocketListener(
+          authorization = authorization,
           mrEnclave = mrEnclave,
           clientRequest = clientRequest,
           emitter = emitter
@@ -71,15 +73,17 @@ internal class Svr2Socket(
   }
 
   private class SvrWebSocketListener(
+    private val authorization: AuthCredentials,
     private val mrEnclave: String,
-    private val clientRequest: Svr2Request,
+    private val clientRequest: (Svr2PinHasher) -> Svr2Request,
-    private val emitter: SingleEmitter<Svr2Response>
+    private val emitter: SingleEmitter<Response>
   ) : WebSocketListener() {
 
     private val stage = AtomicReference(Stage.WAITING_TO_INITIALIZE)
     private lateinit var client: Svr2Client
+    private lateinit var pinHasher: Svr2PinHasher
 
-    override fun onOpen(webSocket: WebSocket, response: Response) {
+    override fun onOpen(webSocket: WebSocket, response: OkHttpResponse) {
       Log.d(TAG, "[onOpen]")
       stage.set(Stage.WAITING_FOR_CONNECTION)
     }
@@ -94,6 +98,7 @@ internal class Svr2Socket(
 
           Stage.WAITING_FOR_CONNECTION -> {
             client = Svr2Client.create(Hex.fromStringCondensed(mrEnclave), bytes.toByteArray(), Instant.now())
+            pinHasher = Svr2PinHasher(authorization, client)
 
             Log.d(TAG, "[onMessage] Sending initial handshake...")
             webSocket.send(client.initialRequest().toByteString())
@@ -104,7 +109,7 @@ internal class Svr2Socket(
             client.completeHandshake(bytes.toByteArray())
             Log.d(TAG, "[onMessage] Handshake read success. Sending request...")
 
-            val ciphertextBytes = client.establishedSend(clientRequest.encode())
+            val ciphertextBytes = client.establishedSend(clientRequest(pinHasher).encode())
             webSocket.send(ciphertextBytes.toByteString())
 
             Log.d(TAG, "[onMessage] Request sent.")
@@ -113,7 +118,12 @@ internal class Svr2Socket(
 
           Stage.WAITING_FOR_RESPONSE -> {
             Log.d(TAG, "[onMessage] Received response for our request.")
-            emitter.onSuccess(Svr2Response.ADAPTER.decode(client.establishedRecv(bytes.toByteArray())))
+            emitter.onSuccess(
+              Response(
+                response = Svr2Response.ADAPTER.decode(client.establishedRecv(bytes.toByteArray())),
+                pinHasher = pinHasher
+              )
+            )
           }
 
           Stage.CLOSED -> {
@@ -155,7 +165,7 @@ internal class Svr2Socket(
       }
     }
 
-    override fun onFailure(webSocket: WebSocket, t: Throwable, response: Response?) {
+    override fun onFailure(webSocket: WebSocket, t: Throwable, response: OkHttpResponse?) {
       if (emitter.tryOnError(t)) {
         Log.w(TAG, "[onFailure] response? " + (response != null), t)
         stage.set(Stage.FAILED)
@@ -173,6 +183,11 @@ internal class Svr2Socket(
     FAILED
   }
 
+  data class Response(
+    val response: Svr2Response,
+    val pinHasher: Svr2PinHasher
+  )
+
   companion object {
     private val TAG = Svr2Socket::class.java.simpleName
 

libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java

@@ -429,11 +429,11 @@ public class PushServiceSocket {
     return JsonUtil.fromJsonResponse(body, CdsiAuthResponse.class);
   }
 
-  public String getSvr2Authorization() throws IOException {
+  public AuthCredentials getSvr2Authorization() throws IOException {
     String          body        = makeServiceRequest(SVR2_AUTH, "GET", null);
     AuthCredentials credentials = JsonUtil.fromJsonResponse(body, AuthCredentials.class);
 
-    return credentials.asBasic();
+    return credentials;
   }
 
   public VerifyAccountResponse changeNumber(@Nonnull ChangePhoneNumberRequest changePhoneNumberRequest)

libsignal/service/src/main/protowire/SVR2.proto

@@ -8,6 +8,7 @@ message Request {
 
   oneof inner {
     BackupRequest backup = 2;
+    ExposeRequest expose = 5;
     RestoreRequest restore = 3;
     DeleteRequest delete = 4;
   }
@@ -16,6 +17,7 @@ message Request {
 message Response {
   oneof inner {
     BackupResponse backup = 1;
+    ExposeResponse expose = 4;
     RestoreResponse restore = 2;
     DeleteResponse delete = 3;
   }
@@ -72,3 +74,26 @@ message DeleteRequest {
 
 message DeleteResponse {
 }
+
+message ExposeRequest {
+  bytes data = 1;
+}
+
+//
+// expose
+//
+
+message ExposeResponse {
+  enum Status {
+    UNSET = 0;  // never returned
+    OK = 1;  // successfully restored, [data] will be set
+
+    // If this status comes back after a successful Backup() call,
+    // this should be cause for concern.
+    // It means that someone has either reset, deleted, or tried to brute-force
+    // the backup since it was created.
+    ERROR = 2;
+  }
+
+  Status status = 1;
+}