airgap-it / airgap-vault

The AirGap Vault is installed on a spare smartphone that has no connection to any network, thus it is air gapped. This app handles the private key.
MIT License
386 stars 109 forks source link

Cannot reproduce 3.11.1 for Android #103

Closed Giszmo closed 2 years ago

Giszmo commented 2 years ago

Sadly the issue with the minified main.js keeps cropping up and I wonder why and how to avoid it.

Objectionable diff is:

Files /tmp/fromPlay_it.airgap.vault_35477/assets/public/index.html and /tmp/fromBuild_it.airgap.vault_35477/assets/public/index.html differ
Only in /tmp/fromPlay_it.airgap.vault_35477/assets/public: main.6c0c07ffb31b2f9117c8.js
Only in /tmp/fromBuild_it.airgap.vault_35477/assets/public: main.6fac43b747228db945b3.js

In index.html only the main.js file names differ but the two main.*.js files differ in dozens of chunks (after preffifying) and with them being minified, it's hard to just trust them. Would not minifying be an option? Although that wouldn't fix reproducibility issues, it would help understand what's happening and why.

I noticed that dependencies are not well pinned. Not only aren't they pinned to a cryptographic hash of the dependency, they are not even pinned to a version number. From https://github.com/airgap-it/airgap-vault/blob/abbed9486d42fc10279018ec789566b71cf9cce2/package.json:

  "dependencies": {
    ...
    "@angular/common": "^11.2.9",
    "@angular/core": "^11.2.9",
    "@angular/forms": "^11.2.9",
Giszmo commented 2 years ago

Same issue for 3.11.2

AndreasGassmann commented 2 years ago

Thanks for the report. I would love to track down the root cause of this as well.

We will discuss the removal of the minification step, but as you said, this will not resolve the original problem.

The versions in the package.json file are not locked, but the ones in the yarn.lock file are. During our build, we use the --frozen-lockfile flag, which should make sure that the exact dependencies are installed. https://classic.yarnpkg.com/en/docs/cli/install#yarn-install---frozen-lockfile-. This has to be done, because locking the versions in the package.json file means that only the direct dependencies are locked. All sub-dependencies would still always use the latest versions, so that's what the lock file is for.

mohammadrafigh commented 2 years ago

@AndreasGassmann I tried 3.11.2 with recent changes on server and the npm error has gone, yarn did the trick. But unfortunately the steps (layers) of building Airgap docker image are getting huge, so applying simple changes on previous layer (like a simple cp command) takes a lot of time to be completed by server. So 60 mins of timeout was not enough to reproduce the apk. I increased the timeout to 90 mins but still we get timeout and a build failure from gradle at STEP 31 RUN /app/android/gradlew --project-dir /app/android build.

Here are the logs:

> Task :app:stripAppiumDebugSymbols
Unable to strip the following libraries, packaging them as they are: libsapling.so, libsqlc-ndk-native-driver.so, libtool-checker.so.

> Task :app:extractAppiumNativeSymbolTables
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86/libsapling.so because unable to locate the objcopy executable for the x86 ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86/libsqlc-ndk-native-driver.so because unable to locate the objcopy executable for the x86 ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86/libtool-checker.so because unable to locate the objcopy executable for the x86 ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/arm64-v8a/libsapling.so because unable to locate the objcopy executable for the arm64-v8a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/arm64-v8a/libsqlc-ndk-native-driver.so because unable to locate the objcopy executable for the arm64-v8a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/arm64-v8a/libtool-checker.so because unable to locate the objcopy executable for the arm64-v8a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/armeabi-v7a/libsapling.so because unable to locate the objcopy executable for the armeabi-v7a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/armeabi-v7a/libsqlc-ndk-native-driver.so because unable to locate the objcopy executable for the armeabi-v7a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/armeabi-v7a/libtool-checker.so because unable to locate the objcopy executable for the armeabi-v7a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86_64/libsapling.so because unable to locate the objcopy executable for the x86_64 ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86_64/libsqlc-ndk-native-driver.so because unable to locate the objcopy executable for the x86_64 ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86_64/libtool-checker.so because unable to locate the objcopy executable for the x86_64 ABI.

> Task :app:mergeAppiumNativeDebugMetadata NO-SOURCE
> Task :capacitor-app-launcher:bundleLibRuntimeToJarRelease
> Task :capacitor-clipboard:bundleLibRuntimeToJarRelease
> Task :capacitor-splash-screen:bundleLibRuntimeToJarRelease
> Task :capacitor-status-bar:bundleLibRuntimeToJarRelease
> Task :app:checkAppiumDuplicateClasses
> Task :app:compressAppiumAssets
> Task :app:dexBuilderAppium
> Task :app:desugarAppiumFileDependencies
Reproducibility check timed out!

Please let me know if we should increase the build timeout to 120 mins if it helps, Anyway I think some Dockerfile steps could be collapsed.

AndreasGassmann commented 2 years ago

I didn't create the docker files and I'm not a docker expert, but we use variations of this setup for all our projects. I just checked, and in our internal gitlab runner, the android build takes less than 8 minutes: https://github.com/airgap-it/airgap-vault/blob/master/.gitlab-ci.yml#L42-L62

I can see in the logs that a few layers were cached, so I'm not sure how long the build would take if nothing was cached. But we have our timeout set to 1 hour, and we never hit that limit. If I remember correctly, when I ran the walletscrutiny script last time in my VM, it took around 20 minutes?

Could you increase the limit to 120m or even higher once, so we can see if it EVER finishes? Maybe it just hangs at a specific point and will never finish.

Do you have any suggestions what we could change in our build to make it more efficient?

mohammadrafigh commented 2 years ago

I will take a look and update you.

Giszmo commented 2 years ago

For what it's worth, build now worked with the server. It had run out of RAM and we upgraded to 16GB.

mohammadrafigh commented 2 years ago

Sent a PR with minor changes in Dockerfile to remove unnecessary layers. As @Giszmo the build is working on server but 3.11.2 is not reproducible due to some diffs in index.html, please check the logs with your API key in Walletscrutiny developers panel.

AndreasGassmann commented 2 years ago

Thanks for looking into this and thanks @mohammadrafigh for the PR. I will look into it ASAP.

AndreasGassmann commented 2 years ago

So I did some investigation yesterday and what I found is that it seems that the order of code is random in some cases. The diff was 3000 lines long, but there was only 1 huge chunk that was in a different place (but same code), and then a few individual lines that were different because the variable name of that chunk was different.

In my case, it was one specific package that was "in the wrong place". I will continue to investigate why that is.

I also disabled the optimisation step, but the file size of main.x.js doubled, so this isn't a feasible solution either.

mohammadrafigh commented 2 years ago

Ok since 3.12.0 is reproducible then if more investigation is not needed, you can close this issue.

AndreasGassmann commented 2 years ago

Thanks for all the help.

mohammadrafigh commented 2 years ago

@AndreasGassmann Something random is happening, in your private test 3.12.0 was reproducible but the play store version seems to be not reproducible again please take a look at these logs:

===== Begin Results =====
appId:          it.airgap.vault
signer:         486381324d8669c80ca9b8c79d383dc972ec284227d65ebfe9e31cad5fd3f342
apkVersionName: 3.12.0
apkVersionCode: 36191
verdict:        
appHash:        2b5775c3d7d569d88b0c023d9f45dad1eca7902271dbba5b0c93bee0710851b2
commit:         cb039b59d211077c9568701afef7339ac34ef1f5

Diff:
Only in /tmp/fromPlay_it.airgap.vault_36191/assets/public: 3.8c9df035d827f0551568.js
Only in /tmp/fromBuild_it.airgap.vault_36191/assets/public: 3.ed396a67eea487509dc8.js
Files /tmp/fromPlay_it.airgap.vault_36191/assets/public/index.html and /tmp/fromBuild_it.airgap.vault_36191/assets/public/index.html differ
Only in /tmp/fromPlay_it.airgap.vault_36191/assets/public: main.58e5cb7e57243ca85507.js
Only in /tmp/fromBuild_it.airgap.vault_36191/assets/public: main.cf1142ad441b50c70620.js
Only in /tmp/fromBuild_it.airgap.vault_36191/assets/public: runtime.5cbd2fc5de80d469883b.js
Only in /tmp/fromPlay_it.airgap.vault_36191/assets/public: runtime.8d962b508f1357b3af89.js
Only in /tmp/fromPlay_it.airgap.vault_36191/META-INF: MANIFEST.MF
Only in /tmp/fromPlay_it.airgap.vault_36191/META-INF: PAPERS.RSA
Only in /tmp/fromPlay_it.airgap.vault_36191/META-INF: PAPERS.SF

Revision, tag (and its signature):
object cb039b59d211077c9568701afef7339ac34ef1f5
type commit
tag v3.12.0
tagger Andreas Gassmann  1637446858 +0100

v3.12.0
===== End Results =====

Do you use a specific API key or secret which is different between your production and development environment? maybe this is the case?

AndreasGassmann commented 2 years ago

Did you compare the hash of the APK that I uploaded and the one from the play store? Unless google changed something during the release process, they must be the same. I actually downloaded the APK from the google play store developer panel while it was in review and uploaded that exact version to walletscrutiny to check if it was reproducible. I had to do that because I wasn't the one who signed the APK, so this was the easiest way for me to get the signed APK.

mohammadrafigh commented 2 years ago

here is the hash of the APK in public state: 2b5775c3d7d569d88b0c023d9f45dad1eca7902271dbba5b0c93bee0710851b2 and it's the hash of the apk you uploaded using your api key: bd5fed5058e007082ded70d520abbc3112022c33b77c43ed7ba09459180ddc20

The hashes are different which is weird. for more info please check developers panel, you can see both test results for public (without api key) and your private test.

Giszmo commented 2 years ago

It would be good to compare the two APKs with diffoscope. I have seen the hash change after upload to Google Play but it was because of different compression or something as the content was identical.

AbakumovAlexandr commented 2 years ago

@AndreasGassmann Is there any updates\progress on this?

The walletscrutiny status is still not good.

AndreasGassmann commented 2 years ago

This is an old issue, let's continue this here: https://github.com/airgap-it/airgap-vault/issues/127

AbakumovAlexandr commented 2 years ago

Great, let's track it there! Thank you!