Repository navigation
Conversation
ABI support is now limited by included native library.
| makeTemporaryFileDescriptor().use { uncompressedFd -> | ||
| GZIPInputStream(FileInputStream(compressedFd), DEFAULT_BUFFER_SIZE).use { inputStream -> | ||
| DigestOutputStream(FileOutputStream(uncompressedFd.v), sha256).use { outputStream -> | ||
| val bytesCopied = inputStream.copyTo2(outputStream, job) | ||
| if (bytesCopied != apk.size) { | ||
| throw GeneralSecurityException("size mismatch for file ${apk.name}") | ||
| if (deltaBaseFd != null) { | ||
| try { | ||
| val res = inaPatch(deltaBaseFd, compressedFd, uncompressedFd.v) | ||
| if (res != apk.size) { | ||
| throw GeneralSecurityException("patched size mismatch for ${apk.name}: expected ${apk.size}, got $res") | ||
| } | ||
| lseekToStart(uncompressedFd.v) | ||
| if (!checkSha256(uncompressedFd.v, apk.size, sha256, apk.sha256)) { | ||
| throw GeneralSecurityException("sha256 mismatch for patched ${apk.name}") | ||
| } |
There was a problem hiding this comment.
Previous gzip-based code used inputStream.copyTo2(outputStream, job) where copyTo2 checked for coroutine cancellation between chunks. However, the new calls such as Ina.unbrotli() appear to lack cancellation signal support
There was a problem hiding this comment.
That's correct. Adding cancellation support would substantially complicate the implemenation. In practice, decompression takes few hundred milliseconds. I think that the complexity increase is not worth it in this case.
| osPackageInfo = pkgManager.getPackageInfoOrNull(pkgName) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The setter for PackageState.osPackageInfo now calls onDeltaStatusChanged. In updateRepo at this osPackageInfo = pkgManager.getPackageInfoOrNull(pkgName), the setter is invoked which calls onDeltaStatusChanged. As this is done for every (non shared library) package, this leads to repo.packages.size main thread handlers being posted to dispatch onPackageStateChanged
UpdatesScreen is one listener that implements onPackageStateChanged:
override fun onPackageStateChanged(state: PackageState) {
super.onPackageStateChanged(state)
val status = state.status()
if (status != PackageState.Status.OUT_OF_DATE && status != PackageState.Status.INSTALLING) {
updateList()
} else {
updateAllViewsExceptList()
}
}updateList can be called here which would scan and call listAdapter.updateList on N packages, where N is the number of packages that are up-to-date, not installed, or disabled. This potentially leads to O(N^2) work from an updateRepo call
There was a problem hiding this comment.
Rearranged onDeltaStatusChanged() calls to avoid this issue in the new revision.
| fun cancelPendingAction(notificationId: Int) { | ||
| checkMainThread() | ||
| pendingActions.removeAll { it.notificationId == notificationId } | ||
| Notifications.cancel(notificationId) | ||
| } |
There was a problem hiding this comment.
Note that addPendingAction waits 300 ms via mainHandler.postDelayed(300L) before inserting an action into pendingActions and showing its notification, so it's possible to race against that
There was a problem hiding this comment.
Added handling for this in the new revision.
| } else { | ||
| val res = Ina.unbrotli(compressedFd, uncompressedFd.v) | ||
| if (res != apk.size) { |
There was a problem hiding this comment.
Would it also make sense to cover this with inaPatcherSemaphore?
There was a problem hiding this comment.
Memory usage for brotli decompression is low.
| inputStream.copyTo2(size, outputStream, job, path) | ||
| }} | ||
| } | ||
| return |
There was a problem hiding this comment.
What happens if the server has a signature that passes the download’s length check but are subsequently rejected by the OS? It seems those bad signatures could be continued to be cached
There was a problem hiding this comment.
Yes, but what would be the point in re-downloading the signatures?
There was a problem hiding this comment.
Though APKs and deltas are re-downloaded on SHA-256 check failure, so having similar behavior for signatures would make sense in case of on-server or on-device data corruption.
There was a problem hiding this comment.
Added SHA-256 checks for APK signatures in the new revision. Signatures are re-downloaded after SHA-256 mismatch.
This commit also adds size and hash checks for APK signatures.
e942985 to
4111973
Compare
2 participants
Depends on:
https://github.com/GrapheneOS/ina
GrapheneOS/apps.grapheneos.org#186
GrapheneOS/releases.grapheneos.org#50
Closes #26
Closes #409
Closes #428
Closes #479