Skip to content

Commit

Permalink
Merge pull request #1246 from novasamatech/fix/detect-wallet-with-no-…
Browse files Browse the repository at this point in the history
…secrets

Improve validation before Cloud backup application
  • Loading branch information
ERussel authored Oct 16, 2024
2 parents ef534f0 + d7c0953 commit 5e5a3b5
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ extension ICloudBackupServiceFactory: CloudBackupServiceFactoryProtocol {
CloudBackupSecretsExporter(
walletConverter: CloudBackupFileModelConverter(),
cryptoManager: createCryptoManager(),
validator: ICloudBackupValidator(),
keychain: keychain
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,24 @@ enum CloudBackupSecretsExporterError: Error {
case unsupportedWallet(MetaAccountModelType)
case invalidSecret(UInt8)
case brokenSecrets(MetaAccountModel.Id)
case validationFailed
}

final class CloudBackupSecretsExporter {
let walletConverter: CloudBackupFileModelConverting
let cryptoManager: CloudBackupCryptoManagerProtocol
let keychain: KeystoreProtocol
let validator: CloudBackupValidating

init(
walletConverter: CloudBackupFileModelConverting,
cryptoManager: CloudBackupCryptoManagerProtocol,
validator: CloudBackupValidating,
keychain: KeystoreProtocol
) {
self.walletConverter = walletConverter
self.cryptoManager = cryptoManager
self.validator = validator
self.keychain = keychain
}

Expand Down Expand Up @@ -333,12 +337,16 @@ extension CloudBackupSecretsExporter: CloudBackupSecretsExporting {
try createPrivateInfo(from: wallet)
}

let publicData = CloudBackup.PublicData(modifiedAt: modifiedAt, wallets: publicWalletsData)
let privateInfo = CloudBackup.DecryptedFileModel.PrivateData(wallets: Set(privateInfoList))

guard validator.validate(publicData: publicData, matches: privateInfo) else {
throw CloudBackupSecretsExporterError.validationFailed
}

let encodedPrivateInfo = try JSONEncoder().encode(privateInfo)
let encryptedInfo = try cryptoManager.encrypt(data: encodedPrivateInfo, password: password)

let publicData = CloudBackup.PublicData(modifiedAt: modifiedAt, wallets: publicWalletsData)

return .init(publicData: publicData, privateData: encryptedInfo.toHex())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,58 @@ protocol CloudBackupValidating {
) -> Bool
}

final class ICloudBackupValidator {}
final class ICloudBackupValidator {
private func validateSecrets(
privateInfo: CloudBackup.DecryptedFileModel.WalletPrivateInfo
) -> Bool {
privateInfo.substrate?.keypair != nil ||
privateInfo.ethereum?.keypair != nil ||
privateInfo.chainAccounts.contains { $0.keypair != nil }
}

private func validateLedger(privateInfo: CloudBackup.DecryptedFileModel.WalletPrivateInfo) -> Bool {
privateInfo.chainAccounts.contains { $0.derivationPath != nil }
}

private func validateGenericLedger(privateInfo: CloudBackup.DecryptedFileModel.WalletPrivateInfo) -> Bool {
privateInfo.substrate?.derivationPath != nil
}
}

extension ICloudBackupValidator: CloudBackupValidating {
func validate(
publicData _: CloudBackup.PublicData,
matches _: CloudBackup.DecryptedFileModel.PrivateData
publicData: CloudBackup.PublicData,
matches: CloudBackup.DecryptedFileModel.PrivateData
) -> Bool {
// TODO: Implement validation
true
let privateDataDict = matches.wallets.reduce(
into: [MetaAccountModel.Id: CloudBackup.DecryptedFileModel.WalletPrivateInfo]()
) {
$0[$1.walletId] = $1
}

return publicData.wallets.allSatisfy { wallet in
switch wallet.type {
case .secrets:
guard let privateInfo = privateDataDict[wallet.walletId] else {
return false
}

return validateSecrets(privateInfo: privateInfo)
case .ledger:
guard let privateInfo = privateDataDict[wallet.walletId] else {
return false
}

return validateLedger(privateInfo: privateInfo)
case .genericLedger:
guard let privateInfo = privateDataDict[wallet.walletId] else {
return false
}

return validateGenericLedger(privateInfo: privateInfo)
case .watchOnly, .paritySigner, .polkadotVault:
return true
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,64 @@ final class CloudBackupSyncTests: XCTestCase {
XCTAssertTrue(try setupResult.syncMetadataManager.hasPassword())
}

func testPreventApplyChangesIfSecretWalletHasNoSecrets() throws {
let optIssue = performDetectIssueTest { params in
// create a wallet with secrets
try? AccountCreationHelper.createMetaAccountFromMnemonic(
cryptoType: .sr25519,
keychain: params.keystore,
settings: params.walletSettings
)

// and then remove secrets before syncing
try? KeystoreValidationHelper.clearKeystore(
for: params.walletSettings.value,
keystore: params.keystore
)
}

XCTAssertEqual(optIssue, .internalFailure)
}

func testPreventApplyChangesIfLegacyLedgerWalletHasNoDPath() throws {
let optIssue = performDetectIssueTest { params in
guard let ledgerApp = SupportedLedgerApp.substrate().first else {
return
}

try? AccountCreationHelper.createSubstrateLedgerAccount(
from: ledgerApp,
keychain: params.keystore,
settings: params.walletSettings
)

// and then remove secrets before syncing
try? KeystoreValidationHelper.clearKeystore(
for: params.walletSettings.value,
keystore: params.keystore
)
}

XCTAssertEqual(optIssue, .internalFailure)
}

func testPreventApplyChangesIfGenericLedgerWalletHasNoDPath() throws {
let optIssue = performDetectIssueTest { params in
try? AccountCreationHelper.createSubstrateGenericLedgerWallet(
keychain: params.keystore,
settings: params.walletSettings
)

// and then remove secrets before syncing
try? KeystoreValidationHelper.clearKeystore(
for: params.walletSettings.value,
keystore: params.keystore
)
}

XCTAssertEqual(optIssue, .internalFailure)
}

func testDetectLocalChanges() throws {
try performSyncTest(
configuringLocal: { params in
Expand Down Expand Up @@ -627,6 +685,32 @@ final class CloudBackupSyncTests: XCTestCase {
)
}

private func performDetectIssueTest(
configuringLocal: LocalWalletsSetupClosure
) -> CloudBackupSyncResult.Issue? {
let setupResult = setupSyncSevice(
configuringLocal: configuringLocal,
configuringBackup: { params in
params.syncMetadataManager.isBackupEnabled = true
try? params.syncMetadataManager.savePassword(Self.defaultPassword)
params.syncMetadataManager.saveLastSyncTimestamp(nil)
}
)

let syncChanges: CloudBackupSyncResult.Changes? = syncAndWait(service: setupResult.syncService) { result in
switch result {
case let .changes(changes):
return changes
default:
return nil
}
}

XCTAssertNotNil(syncChanges)

return applyChangesAndDetectIssue(for: setupResult.syncService)
}

private func performSyncTest(
configuringLocal: LocalWalletsSetupClosure,
changingAfterBackup: LocalWalletsChangeClosure,
Expand Down Expand Up @@ -723,6 +807,21 @@ final class CloudBackupSyncTests: XCTestCase {
}
}

private func applyChangesAndDetectIssue(
for syncService: CloudBackupSyncServiceProtocol
) -> CloudBackupSyncResult.Issue? {
syncService.applyChanges(notifyingIn: .global(), closure: {_ in })

return syncAndWait(service: syncService) { result in
switch result {
case let .issue(issue):
return issue
default:
return nil
}
}
}

private func setupSyncSevice(
configuringLocal: LocalWalletsSetupClosure,
configuringBackup: BackupSetupClosure
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ extension MockCloudBackupOperationFactory: CloudBackupOperationFactoryProtocol {
dataClosure: @escaping () throws -> Data
) -> BaseOperation<Void> {
ClosureOperation {
self.data = try? dataClosure()
self.data = try dataClosure()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ extension MockCloudBackupServiceFactory: CloudBackupServiceFactoryProtocol {
CloudBackupSecretsExporter(
walletConverter: CloudBackupFileModelConverter(),
cryptoManager: createCryptoManager(),
validator: ICloudBackupValidator(),
keychain: keychain
)
}
Expand Down

0 comments on commit 5e5a3b5

Please sign in to comment.