code review

This commit is contained in:
Valere 2020-02-26 09:49:16 +01:00 committed by Benoit Marty
parent 0cfc9451ca
commit b4a783198b
11 changed files with 136 additions and 166 deletions

View file

@ -92,7 +92,7 @@ interface SharedSecretStorageService {
* @param secret The secret contents.
* @param keys The list of (ID,privateKey) of the keys to use to encrypt the secret.
*/
fun storeSecret(name: String, secretBase64: String, keys: List<Pair<String?, SsssKeySpec?>>, callback: MatrixCallback<Unit>)
fun storeSecret(name: String, secretBase64: String, keys: List<KeyRef>, callback: MatrixCallback<Unit>)
/**
* Use this call to determine which SSSSKeySpec to use for requesting secret
@ -110,4 +110,9 @@ interface SharedSecretStorageService {
fun getSecret(name: String, keyId: String?, secretKey: SsssKeySpec, callback: MatrixCallback<String>)
fun checkShouldBeAbleToAccessSecrets(secretNames: List<String>, keyId: String?) : IntegrityResult
data class KeyRef(
val keyId: String?,
val keySpec: SsssKeySpec?
)
}

View file

@ -180,17 +180,17 @@ internal class DefaultSharedSecretStorageService @Inject constructor(
return getKey(keyId)
}
override fun storeSecret(name: String, secretBase64: String, keys: List<Pair<String?, SsssKeySpec?>>, callback: MatrixCallback<Unit>) {
override fun storeSecret(name: String, secretBase64: String, keys: List<SharedSecretStorageService.KeyRef>, callback: MatrixCallback<Unit>) {
cryptoCoroutineScope.launch(coroutineDispatchers.main) {
val encryptedContents = HashMap<String, EncryptedSecretContent>()
try {
keys.forEach {
val keyId = it.first
val keyId = it.keyId
// encrypt the content
when (val key = keyId?.let { getKey(keyId) } ?: getDefaultKey()) {
is KeyInfoResult.Success -> {
if (key.keyInfo.content.algorithm == SSSS_ALGORITHM_AES_HMAC_SHA2) {
encryptAesHmacSha2(it.second!!, name, secretBase64).let {
encryptAesHmacSha2(it.keySpec!!, name, secretBase64).let {
encryptedContents[key.keyInfo.id] = it
}
} else {

View file

@ -84,7 +84,7 @@ internal class DefaultRoomVerificationUpdateTask @Inject constructor(
Timber.v("## SAS Verification live observer: received msgId: ${event.eventId} type: ${event.getClearType()}")
// Relates to is not encrypted
val relatesTo = event.content.toModel<MessageRelationContent>()?.relatesTo?.eventId
val relatesToEventId = event.content.toModel<MessageRelationContent>()?.relatesTo?.eventId
if (event.senderId == userId) {
// If it's send from me, we need to keep track of Requests or Start
@ -105,8 +105,8 @@ internal class DefaultRoomVerificationUpdateTask @Inject constructor(
event.getClearContent().toModel<MessageVerificationStartContent>()?.let {
if (it.fromDevice != deviceId) {
// The verification is started from another device
Timber.v("## SAS Verification live observer: Transaction started by other device tid:$relatesTo ")
relatesTo?.let { txId -> transactionsHandledByOtherDevice.add(txId) }
Timber.v("## SAS Verification live observer: Transaction started by other device tid:$relatesToEventId ")
relatesToEventId?.let { txId -> transactionsHandledByOtherDevice.add(txId) }
params.verificationService.onRoomRequestHandledByOtherDevice(event)
}
}
@ -114,13 +114,13 @@ internal class DefaultRoomVerificationUpdateTask @Inject constructor(
event.getClearContent().toModel<MessageVerificationReadyContent>()?.let {
if (it.fromDevice != deviceId) {
// The verification is started from another device
Timber.v("## SAS Verification live observer: Transaction started by other device tid:$relatesTo ")
relatesTo?.let { txId -> transactionsHandledByOtherDevice.add(txId) }
Timber.v("## SAS Verification live observer: Transaction started by other device tid:$relatesToEventId ")
relatesToEventId?.let { txId -> transactionsHandledByOtherDevice.add(txId) }
params.verificationService.onRoomRequestHandledByOtherDevice(event)
}
}
} else if (EventType.KEY_VERIFICATION_CANCEL == event.getClearType() || EventType.KEY_VERIFICATION_DONE == event.getClearType()) {
relatesTo?.let {
relatesToEventId?.let {
transactionsHandledByOtherDevice.remove(it)
params.verificationService.onRoomRequestHandledByOtherDevice(event)
}
@ -130,9 +130,9 @@ internal class DefaultRoomVerificationUpdateTask @Inject constructor(
return@forEach
}
if (relatesTo != null && transactionsHandledByOtherDevice.contains(relatesTo)) {
if (relatesToEventId != null && transactionsHandledByOtherDevice.contains(relatesToEventId)) {
// Ignore this event, it is directed to another of my devices
Timber.v("## SAS Verification live observer: Ignore Transaction handled by other device tid:$relatesTo ")
Timber.v("## SAS Verification live observer: Ignore Transaction handled by other device tid:$relatesToEventId ")
return@forEach
}
when (event.getClearType()) {

View file

@ -1,27 +1,12 @@
/*
* Copyright (c) 2020 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/*
* Copyright (C) 2015 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,

View file

@ -19,6 +19,7 @@
android:supportsRtl="true"
android:theme="@style/AppTheme.Light"
tools:replace="android:allowBackup">
<activity
android:name=".features.MainActivity"
android:theme="@style/AppTheme.Launcher" />
@ -56,6 +57,7 @@
<activity
android:name=".features.crypto.keysbackup.settings.KeysBackupManageActivity"
android:label="@string/encryption_message_recovery" />
<activity
android:name=".features.reactions.EmojiReactionPickerActivity"
android:label="@string/title_activity_emoji_reaction_picker" />
@ -71,7 +73,7 @@
android:value=".features.home.HomeActivity" />
</activity>
<activity android:name=".features.debug.DebugMenuActivity" />
<activity android:name=".features.createdirect.CreateDirectRoomActivity" />
<activity android:name="im.vector.riotx.features.createdirect.CreateDirectRoomActivity" />
<activity android:name=".features.webview.VectorWebViewActivity" />
<activity android:name=".features.link.LinkHandlerActivity">
<intent-filter>
@ -95,7 +97,6 @@
<intent-filter>
<action android:name="android.intent.action.SEND" />
<data android:mimeType="*/*" />
<category android:name="android.intent.category.DEFAULT" />
@ -103,14 +104,15 @@
</intent-filter>
<intent-filter>
<action android:name="android.intent.action.SEND_MULTIPLE" />
<data android:mimeType="*/*" />
<category android:name="android.intent.category.DEFAULT" />
<category android:name="android.intent.category.OPENABLE" />
</intent-filter>
</activity>
<activity android:name=".features.roomprofile.RoomProfileActivity" />
<activity android:name=".features.signout.hard.SignedOutActivity" />
<activity
android:name=".features.signout.soft.SoftLogoutActivity"
@ -127,6 +129,7 @@
<data android:host="matrix.to" />
</intent-filter>
</activity>
<activity
android:name=".features.roommemberprofile.RoomMemberProfileActivity"
android:parentActivityName=".features.home.HomeActivity">
@ -134,6 +137,7 @@
android:name="android.support.PARENT_ACTIVITY"
android:value=".features.home.HomeActivity" />
</activity>
<activity android:name=".features.qrcode.QrCodeScannerActivity" />
<activity android:name=".features.crypto.quads.SharedSecureStorageActivity" />
@ -146,17 +150,25 @@
android:theme="@style/AppTheme.AttachmentsPreview" />
<!-- Services -->
<service
android:name=".core.services.CallService"
android:exported="false" />
<service
android:name=".core.services.VectorSyncService"
android:exported="false" /> <!-- Receivers -->
android:exported="false" />
<!-- Receivers -->
<!-- Exported false, should only be accessible from this app!! -->
<receiver
android:name=".features.notifications.NotificationBroadcastReceiver"
android:enabled="true"
android:exported="false" /> <!-- Providers -->
android:exported="false" />
<!-- Providers -->
<provider
android:name="androidx.core.content.FileProvider"
android:authorities="${applicationId}.fileProvider"

View file

@ -31,8 +31,6 @@ import im.vector.riotx.core.error.ErrorFormatter
import im.vector.riotx.core.extensions.addFragment
import im.vector.riotx.core.platform.SimpleFragmentActivity
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.CompositeDisposable
import io.reactivex.disposables.Disposable
import kotlinx.android.parcel.Parcelize
import kotlinx.android.synthetic.main.activity.*
import javax.inject.Inject
@ -46,13 +44,6 @@ class SharedSecureStorageActivity : SimpleFragmentActivity() {
val resultKeyStoreAlias: String
) : Parcelable
private val uiDisposables = CompositeDisposable()
private fun Disposable.disposeOnDestroyView(): Disposable {
uiDisposables.add(this)
return this
}
private val viewModel: SharedSecureStorageViewModel by viewModel()
@Inject lateinit var viewModelFactory: SharedSecureStorageViewModel.Factory
@Inject lateinit var errorFormatter: ErrorFormatter
@ -75,7 +66,7 @@ class SharedSecureStorageActivity : SimpleFragmentActivity() {
.subscribe {
observeViewEvents(it)
}
.disposeOnDestroyView()
.disposeOnDestroy()
viewModel.subscribe(this) {
// renderState(it)
@ -85,7 +76,6 @@ class SharedSecureStorageActivity : SimpleFragmentActivity() {
private fun observeViewEvents(it: SharedSecureStorageViewEvent?) {
when (it) {
is SharedSecureStorageViewEvent.Dismiss -> {
setResult(Activity.RESULT_CANCELED)
finish()
}
is SharedSecureStorageViewEvent.Error -> {
@ -95,7 +85,6 @@ class SharedSecureStorageActivity : SimpleFragmentActivity() {
.setCancelable(false)
.setPositiveButton(R.string.ok) { _, _ ->
if (it.dismiss) {
setResult(Activity.RESULT_CANCELED)
finish()
}
}

View file

@ -24,9 +24,9 @@ import com.squareup.inject.assisted.Assisted
import com.squareup.inject.assisted.AssistedInject
import im.vector.matrix.android.api.listeners.ProgressListener
import im.vector.matrix.android.api.session.Session
import im.vector.matrix.android.api.session.securestorage.RawBytesKeySpec
import im.vector.matrix.android.api.session.securestorage.IntegrityResult
import im.vector.matrix.android.api.session.securestorage.KeyInfoResult
import im.vector.matrix.android.api.session.securestorage.RawBytesKeySpec
import im.vector.matrix.android.internal.crypto.crosssigning.toBase64NoPadding
import im.vector.matrix.android.internal.util.awaitCallback
import im.vector.riotx.R
@ -69,81 +69,87 @@ class SharedSecureStorageViewModel @AssistedInject constructor(
override fun handle(action: SharedSecureStorageAction) = withState {
when (action) {
is SharedSecureStorageAction.TogglePasswordVisibility -> {
setState {
copy(
passphraseVisible = !passphraseVisible
)
}
}
is SharedSecureStorageAction.Cancel -> {
_viewEvents.post(SharedSecureStorageViewEvent.Dismiss)
}
is SharedSecureStorageAction.SubmitPassphrase -> {
val decryptedSecretMap = HashMap<String, String>()
GlobalScope.launch(Dispatchers.IO) {
runCatching {
_viewEvents.post(SharedSecureStorageViewEvent.ShowModalLoading)
val passphrase = action.passphrase
val keyInfoResult = session.sharedSecretStorageService.getDefaultKey()
if (!keyInfoResult.isSuccess()) {
_viewEvents.post(SharedSecureStorageViewEvent.HideModalLoading)
_viewEvents.post(SharedSecureStorageViewEvent.Error("Cannot find ssss key"))
return@launch
}
val keyInfo = (keyInfoResult as KeyInfoResult.Success).keyInfo
is SharedSecureStorageAction.TogglePasswordVisibility -> handleTogglePasswordVisibility()
is SharedSecureStorageAction.Cancel -> handleCancel()
is SharedSecureStorageAction.SubmitPassphrase -> handleSubmitPassphrase(action)
}
}
_viewEvents.post(SharedSecureStorageViewEvent.UpdateLoadingState(
WaitingViewData(
message = stringProvider.getString(R.string.keys_backup_restoring_computing_key_waiting_message),
isIndeterminate = true
)
))
val keySpec = RawBytesKeySpec.fromPassphrase(
passphrase,
keyInfo.content.passphrase?.salt ?: "",
keyInfo.content.passphrase?.iterations ?: 0,
// TODO
object : ProgressListener {
override fun onProgress(progress: Int, total: Int) {
_viewEvents.post(SharedSecureStorageViewEvent.UpdateLoadingState(
WaitingViewData(
message = stringProvider.getString(R.string.keys_backup_restoring_computing_key_waiting_message),
isIndeterminate = false,
progress = progress,
progressTotal = total
)
))
}
}
private fun handleSubmitPassphrase(action: SharedSecureStorageAction.SubmitPassphrase) {
val decryptedSecretMap = HashMap<String, String>()
GlobalScope.launch(Dispatchers.IO) {
runCatching {
_viewEvents.post(SharedSecureStorageViewEvent.ShowModalLoading)
val passphrase = action.passphrase
val keyInfoResult = session.sharedSecretStorageService.getDefaultKey()
if (!keyInfoResult.isSuccess()) {
_viewEvents.post(SharedSecureStorageViewEvent.HideModalLoading)
_viewEvents.post(SharedSecureStorageViewEvent.Error("Cannot find ssss key"))
return@launch
}
val keyInfo = (keyInfoResult as KeyInfoResult.Success).keyInfo
_viewEvents.post(SharedSecureStorageViewEvent.UpdateLoadingState(
WaitingViewData(
message = stringProvider.getString(R.string.keys_backup_restoring_computing_key_waiting_message),
isIndeterminate = true
)
withContext(Dispatchers.IO) {
args.requestedSecrets.forEach {
val res = awaitCallback<String> { callback ->
session.sharedSecretStorageService.getSecret(
name = it,
keyId = keyInfo.id,
secretKey = keySpec,
callback = callback)
}
decryptedSecretMap[it] = res
))
val keySpec = RawBytesKeySpec.fromPassphrase(
passphrase,
keyInfo.content.passphrase?.salt ?: "",
keyInfo.content.passphrase?.iterations ?: 0,
// TODO
object : ProgressListener {
override fun onProgress(progress: Int, total: Int) {
_viewEvents.post(SharedSecureStorageViewEvent.UpdateLoadingState(
WaitingViewData(
message = stringProvider.getString(R.string.keys_backup_restoring_computing_key_waiting_message),
isIndeterminate = false,
progress = progress,
progressTotal = total
)
))
}
}
}.fold({
_viewEvents.post(SharedSecureStorageViewEvent.HideModalLoading)
val safeForIntentCypher = ByteArrayOutputStream().also {
it.use {
session.securelyStoreObject(decryptedSecretMap as Map<String, String>, args.resultKeyStoreAlias, it)
}
}.toByteArray().toBase64NoPadding()
_viewEvents.post(SharedSecureStorageViewEvent.FinishSuccess(safeForIntentCypher))
}, {
_viewEvents.post(SharedSecureStorageViewEvent.HideModalLoading)
_viewEvents.post(SharedSecureStorageViewEvent.InlineError(it.localizedMessage))
})
)
withContext(Dispatchers.IO) {
args.requestedSecrets.forEach {
val res = awaitCallback<String> { callback ->
session.sharedSecretStorageService.getSecret(
name = it,
keyId = keyInfo.id,
secretKey = keySpec,
callback = callback)
}
decryptedSecretMap[it] = res
}
}
}
}.fold({
_viewEvents.post(SharedSecureStorageViewEvent.HideModalLoading)
val safeForIntentCypher = ByteArrayOutputStream().also {
it.use {
session.securelyStoreObject(decryptedSecretMap as Map<String, String>, args.resultKeyStoreAlias, it)
}
}.toByteArray().toBase64NoPadding()
_viewEvents.post(SharedSecureStorageViewEvent.FinishSuccess(safeForIntentCypher))
}, {
_viewEvents.post(SharedSecureStorageViewEvent.HideModalLoading)
_viewEvents.post(SharedSecureStorageViewEvent.InlineError(it.localizedMessage))
})
}
}
private fun handleCancel() {
_viewEvents.post(SharedSecureStorageViewEvent.Dismiss)
}
private fun handleTogglePasswordVisibility() {
setState {
copy(
passphraseVisible = !passphraseVisible
)
}
}

View file

@ -19,15 +19,8 @@ package im.vector.riotx.features.crypto.quads
import android.os.Bundle
import android.view.View
import android.view.inputmethod.EditorInfo
import android.widget.Button
import android.widget.EditText
import android.widget.ImageView
import android.widget.TextView
import butterknife.BindView
import butterknife.OnClick
import com.airbnb.mvrx.activityViewModel
import com.airbnb.mvrx.withState
import com.google.android.material.textfield.TextInputLayout
import com.jakewharton.rxbinding3.view.clicks
import com.jakewharton.rxbinding3.widget.editorActionEvents
import com.jakewharton.rxbinding3.widget.textChanges
@ -35,6 +28,7 @@ import im.vector.riotx.R
import im.vector.riotx.core.extensions.showPassword
import im.vector.riotx.core.platform.VectorBaseFragment
import io.reactivex.android.schedulers.AndroidSchedulers
import kotlinx.android.synthetic.main.fragment_ssss_access_from_passphrase.*
import me.gujun.android.span.span
import java.util.concurrent.TimeUnit
@ -44,36 +38,10 @@ class SharedSecuredStoragePassphraseFragment : VectorBaseFragment() {
val sharedViewModel: SharedSecureStorageViewModel by activityViewModel()
@BindView(R.id.ssss_restore_with_passphrase_warning_text)
lateinit var warningText: TextView
@BindView(R.id.ssss_restore_with_passphrase_warning_reason)
lateinit var reasonText: TextView
@BindView(R.id.ssss_passphrase_enter_til)
lateinit var mPassphraseInputLayout: TextInputLayout
@BindView(R.id.ssss_passphrase_enter_edittext)
lateinit var mPassphraseTextEdit: EditText
@BindView(R.id.ssss_view_show_password)
lateinit var mPassphraseReveal: ImageView
@BindView(R.id.ssss_passphrase_submit)
lateinit var submitButton: Button
@BindView(R.id.ssss_passphrase_cancel)
lateinit var cancelButton: Button
@OnClick(R.id.ssss_view_show_password)
fun toggleVisibilityMode() {
sharedViewModel.handle(SharedSecureStorageAction.TogglePasswordVisibility)
}
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
warningText.text = span {
ssss_restore_with_passphrase_warning_text.text = span {
span(getString(R.string.enter_secret_storage_passphrase_warning)) {
textStyle = "bold"
}
@ -81,9 +49,9 @@ class SharedSecuredStoragePassphraseFragment : VectorBaseFragment() {
+getString(R.string.enter_secret_storage_passphrase_warning_text)
}
reasonText.text = getString(R.string.enter_secret_storage_passphrase_reason_verify)
ssss_restore_with_passphrase_warning_reason.text = getString(R.string.enter_secret_storage_passphrase_reason_verify)
mPassphraseTextEdit.editorActionEvents()
ssss_passphrase_enter_edittext.editorActionEvents()
.debounce(300, TimeUnit.MILLISECONDS)
.observeOn(AndroidSchedulers.mainThread())
.subscribe {
@ -93,22 +61,22 @@ class SharedSecuredStoragePassphraseFragment : VectorBaseFragment() {
}
.disposeOnDestroyView()
mPassphraseTextEdit.textChanges()
ssss_passphrase_enter_edittext.textChanges()
.subscribe {
mPassphraseInputLayout.error = null
submitButton.isEnabled = it.isNotBlank()
ssss_passphrase_enter_til.error = null
ssss_passphrase_submit.isEnabled = it.isNotBlank()
}
.disposeOnDestroyView()
sharedViewModel.observeViewEvents {
when (it) {
is SharedSecureStorageViewEvent.InlineError -> {
mPassphraseInputLayout.error = it.message
ssss_passphrase_enter_til.error = it.message
}
}
}
submitButton.clicks()
ssss_passphrase_submit.clicks()
.debounce(300, TimeUnit.MILLISECONDS)
.observeOn(AndroidSchedulers.mainThread())
.subscribe {
@ -116,25 +84,33 @@ class SharedSecuredStoragePassphraseFragment : VectorBaseFragment() {
}
.disposeOnDestroyView()
cancelButton.clicks()
ssss_passphrase_cancel.clicks()
.debounce(300, TimeUnit.MILLISECONDS)
.observeOn(AndroidSchedulers.mainThread())
.subscribe {
sharedViewModel.handle(SharedSecureStorageAction.Cancel)
}
.disposeOnDestroyView()
ssss_view_show_password.clicks()
.debounce(300, TimeUnit.MILLISECONDS)
.observeOn(AndroidSchedulers.mainThread())
.subscribe {
sharedViewModel.handle(SharedSecureStorageAction.TogglePasswordVisibility)
}
.disposeOnDestroyView()
}
fun submit() {
val text = mPassphraseTextEdit.text.toString()
val text = ssss_passphrase_enter_edittext.text.toString()
if (text.isBlank()) return // Should not reach this point as button disabled
submitButton.isEnabled = false
ssss_passphrase_submit.isEnabled = false
sharedViewModel.handle(SharedSecureStorageAction.SubmitPassphrase(text))
}
override fun invalidate() = withState(sharedViewModel) { state ->
val shouldBeVisible = state.passphraseVisible
mPassphraseTextEdit.showPassword(shouldBeVisible)
mPassphraseReveal.setImageResource(if (shouldBeVisible) R.drawable.ic_eye_closed_black else R.drawable.ic_eye_black)
ssss_passphrase_enter_edittext.showPassword(shouldBeVisible)
ssss_view_show_password.setImageResource(if (shouldBeVisible) R.drawable.ic_eye_closed_black else R.drawable.ic_eye_black)
}
}

View file

@ -106,8 +106,7 @@ class VerificationBottomSheet : VectorBaseBottomSheetDialogFragment() {
.setTitle(getString(R.string.dialog_title_error))
.setMessage(it.errorMessage)
.setCancelable(false)
.setPositiveButton(R.string.ok) { _, _ ->
}
.setPositiveButton(R.string.ok, null)
.show()
Unit
}

View file

@ -32,6 +32,5 @@
<!-- Max width for some buttons -->
<dimen name="button_max_width">280dp</dimen>
<dimen name="fab_margin">16dp</dimen>
</resources>

View file

@ -2151,5 +2151,4 @@ Not all features in Riot are implemented in RiotX yet. Main missing (and coming
<string name="qr_code_scanned_by_other_no">No</string>
<string name="no_connectivity_to_the_server_indicator">Connectivity to the server has been lost</string>
<string name="title_activity_share_secure_storage">ShareSecureStorageActivity</string>
</resources>