Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support bundle verification with multiple public keys #826

Open
baka3k opened this issue Dec 13, 2024 · 2 comments
Open

Support bundle verification with multiple public keys #826

baka3k opened this issue Dec 13, 2024 · 2 comments
Labels
status:investigating The issue needs more research and information. type:feature Proposal or idea for a new feature or enhancement.

Comments

@baka3k
Copy link

baka3k commented Dec 13, 2024

Description

The situation we're facing is as follows:
We have multiple independent MiniApp development teams, and we want each team to use their own unique private key.

This means that when verifying a bundle, we need to pass in the public key of the respective team (which is fetched from the server).

However, when examining the repack source code, we found that the 'verify' function is currently ONLY allowing for a single public key that is hardcoded into the source code, as shown below."

fun verifyBundle(context: Context, token: String?, fileContent: ByteArray?) {
            if (token == null) {
                throw Exception("The bundle verification failed because no token for the bundle was found.")
            }

            val stringPublicKey = getPublicKeyFromStringsIfExist(context)
                    ?: throw Exception("The bundle verification failed because PublicKey was not found in the bundle. Make sure you've added the PublicKey to the res/values/strings.xml under RepackPublicKey key.")

            val publicKey = parsePublicKey(stringPublicKey)
                    ?: throw Exception("The bundle verification failed because the PublicKey is invalid.")

            val claims: Map<String, Any?> = verifyAndDecodeToken(token, publicKey)

            val contentHash = claims["hash"] as String?
                    ?: throw Exception("The bundle verification failed because the token is invalid.")

            val fileHash = computeHash(fileContent)

            if (contentHash != fileHash) {
                throw Exception("The bundle verification failed because the bundle hash is invalid.")
            }
        }

 private fun getPublicKeyFromStringsIfExist(
                context: Context
        ): String? {
            val packageName: String = context.packageName
            val resId: Int =
                    context.resources.getIdentifier("RepackPublicKey", "string", packageName)
            if (resId != 0) {
                return context.getString(resId).ifEmpty {
                    null
                }
            }
            return null
        }

Suggested solution

It would be much better if the verify bundle function could accept an additional public key as a parameter, instead of hardcoding it as it is now, such as:

fun verifyBundle(context: Context, 
                         token: String?, 
                         fileContent: ByteArray?,
                         publicKeyAsByteArray: ByteArray? // add this parameter
) {
  val publicKey = publicKeyFromByteArray(publicKeyAsByteArray)
// 1. verify bundle by public key
// 2. check sum bundle

With this approach, we can have multiple mini-app development teams, each with their own private key for signing their mini-apps. We can authenticate development teams using their keys, allowing them to join the ecosystem or rejecting teams without relying on the main app's private key.
Furthermore, if the main app's private key is lost or compromised, mini-apps created by various development teams can still operate. We have the flexibility to re-sign them whenever needed,(similar Google's Play App Signing approach)

Would you please consider this proposal

Thanks & BestRegard,

Additional context

No response

@baka3k baka3k added status:new New issue, not reviewed by the team yet. type:feature Proposal or idea for a new feature or enhancement. labels Dec 13, 2024
@baka3k
Copy link
Author

baka3k commented Dec 30, 2024

I have forked and and made some modifications to allow for public key injection at repository

https://github.com/baka3k/repackcodesigning

https://github.com/baka3k/repackcodesigning/blob/main/packages/repack/android/src/main/java/com/callstack/repack/ScriptManagerModule.kt

class ScriptManagerModule(
    reactContext: ReactApplicationContext,
    publicKey: PublicKey?,
    coroutineDispatcher: CoroutineDispatcher = Dispatchers.IO
) : ScriptManagerSpec(reactContext) {
    private val nativeLoader = NativeScriptLoader(reactApplicationContext)
    private val remoteLoader =
        RemoteScriptLoader(reactApplicationContext, nativeLoader, publicKey = publicKey)

https://github.com/baka3k/repackcodesigning/blob/main/packages/repack/android/src/main/java/com/callstack/repack/RemoteScriptLoader.kt

   if (config.verifyScriptSignature == "strict" || (config.verifyScriptSignature == "lax" && token != null)) {
                            CodeSigningUtils.verifyBundle(reactContext, token, bundle, publicKey)
                        }

https://github.com/baka3k/repackcodesigning/blob/main/packages/repack/android/src/main/java/com/callstack/repack/CodeSigningUtils.kt

fun verifyBundle(
            context: Context,
            token: String?,
            fileContent: ByteArray?,
            publickey: PublicKey?
        ) {
            if (token == null) {
                throw Exception("The bundle verification failed because no token for the bundle was found.")
            }
            var publicKey = publickey 
            if (publicKey == null) {
                // find default
                val stringPublicKey = getPublicKeyFromStringsIfExist(context)
                    ?: throw Exception("The bundle verification failed because PublicKey was not found in the bundle. Make sure you've added the PublicKey to the res/values/strings.xml under RepackPublicKey key.")
                publicKey = parsePublicKey(stringPublicKey)
                    ?: throw Exception("The bundle verification failed because the PublicKey is invalid.")
            }

If no public key is passed, the system will retrieve the old public key from the repack stored in res/values/strings.xml.
However, if a public key is supplied, that specific public key will be utilized.

Beside that, There are a few points I want to notice about the existing issues related multithreading in the source code Repack
I add log to get thread which use to run in background function
Image
And get results
Image

You can see that all the processes inside "runInBackground" are executed with the thread outside "runInBackground" - mqt_v_native
That means they are executed on the SAME thread.
The reason is

  val handler = Handler()

If you don't pass a Looper to a Handler, it will default to using the Looper of the thread that created it. This means that all tasks, both inside and outside the 'runInBackground' function, will be executed on the same thread.
therefore, the runInBackground function might not work as expected
I've made a few changes:

class ScriptManagerModule(
    reactContext: ReactApplicationContext,
    publicKey: PublicKey?,
    coroutineDispatcher: CoroutineDispatcher = Dispatchers.IO // add dispatcher or default
) : ScriptManagerSpec(reactContext) {
    private val nativeLoader = NativeScriptLoader(reactApplicationContext)
    private val remoteLoader = RemoteScriptLoader(reactApplicationContext, nativeLoader, publicKey = publicKey)
    private val fileSystemLoader = FileSystemScriptLoader(reactApplicationContext, nativeLoader)
    private val coroutineExceptionHandler = CoroutineExceptionHandler { _, throwable ->
        Log.e("ScriptManagerModule", "CoroutineExceptionHandler ${throwable.message}", throwable)
    }
    private val job = SupervisorJob()
    private val coroutineScope = CoroutineScope(job + coroutineExceptionHandler + coroutineDispatcher)

    private fun runInBackground(fn: () -> Unit) {
        coroutineScope.launch { // run in worker thread
            fn()
        }
    }

Could we discuss this item further?

Thanks & BestRegards

@jbroma jbroma added status:investigating The issue needs more research and information. and removed status:new New issue, not reviewed by the team yet. labels Dec 31, 2024
@jbroma
Copy link
Member

jbroma commented Dec 31, 2024

hi @baka3k, I've taken a look at the whole proposal and your implementation and it looks good to me - there are few nitpicks that we would have to address but let's address that within the actual PR (like the JS API which is missing currently). Sorry for the lack of attention for this feature - the load is very high currently and it's simply hard for me to find the time.

Regarding the threading issue - super glad you found it and investigated it 🎉
If you could open a separate PR for that I'll be happy to assist and get this merged!

cc @okwasniewski would love to know your take on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:investigating The issue needs more research and information. type:feature Proposal or idea for a new feature or enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants