apksigner produce an Exception with PKCS#11 KeyStore on Windows [37140484]

Fixed

Bug

[AOSP] win

[AOSP] assigned

Status Update

No update yet.

Description

lo...@gmail.com

created issue #1

Mar 24, 2017 08:10AM

*****************************************************************
***** *****
***** !!!! THIS BUG TRACKER IS FOR GERRIT CODE REVIEW !!!! *****
***** *****
***** DO NOT SUBMIT BUGS FOR CHROME, ANDROID, CYANOGENMOD, *****
***** INTERNAL ISSUES WITH YOUR COMPANY'S GERRIT SETUP, ETC.*****
***** *****
***** THOSE ISSUES BELONG IN DIFFERENT ISSUE TRACKERS *****
***** *****
*****************************************************************

Affected Version:

What steps will reproduce the problem?
1.Use the native KeyStore from Windows
2.User a certificate witch need an USE-eToken
3.callapksigner with those parameters:
@("sign",
"--ks", "$easyTokenSignCrtPath",
"--ks-type", "PKCS11",
"--ks-pass", "file:$apkSignerPwPath",
"--ks-provider-class", "sun.security.pkcs11.SunPKCS11",
"--ks-provider-arg", "C:\PROGRA~1\Java\jdk1.8.0_121\bin\eToken.cfg",
"--ks-key-alias", "my alias",
"$scrDir\$toSign")

What is the expected output?
apk file should be sign, all those parameters are working with jarsigner, on the same computer

What do you see instead?

Please provide any additional information below.
- Windows Server 2016
- JDK 1.8.0_121
- eToken works with jarsigner on the same Computer, parameters are:
$jarSignerParams=@(
"-keystore","NONE",
"-storepass", "$STOREPASS",
"-storetype", "PKCS11",
"-tsa", "

http://timestamp.globalsign.com/scripts/timestamp.dll",
"-providerclass", "sun.security.pkcs11.SunPKCS11",
"-providerArg", "$JDK_PATH\bin\eToken.cfg"
"$scrDir\$toSignJarsigner",
"my alias")

As far as I could analyse, {KeyStore}.engineLoad() should not be called will 'null' as parameter, which is done in apksigner

https://android.googlesource.com/platform/tools/apksig/+/master/src/apksigner/java/com/android/apksigner/ApkSignerTool.java#628

https://github.com/frohoff/jdk8u-jdk/blob/master/src/share/classes/sun/security/pkcs11/P11KeyStore.java#L790

Regards, Éric

Comments

kl...@google.com <kl...@google.com> #2Mar 25, 2017 07:28PM

Assigned to kl...@google.com.

Thank you for this feedback. The team may reach out for more information on triaging or reproducing this issue.

kl...@google.com <kl...@google.com> #3Mar 25, 2017 07:56PM

I have tried the example project you've provided, it does not reproduce.

Could you please try running with -Pandroid.useDexArchive=false? This disables the new dexing pipeline, and fallbacks to the old one.

src-files.txt

2.8 KB

View

Download

e....@gmail.com <e....@gmail.com> #4Mar 27, 2017 09:28AM

@Mario Did you get a chance to try the build with -Pandroid.useDexArchive=false flag?

kl...@google.com <kl...@google.com> #5Mar 28, 2017 03:27AM

I'm the college of @Mario and I make it work without issues and without using -Pandroid.useDexArchive=false by using alpha6 and removing retrolambda

lo...@gmail.com <lo...@gmail.com> #6Mar 30, 2017 08:53AM

Sorry my comment from Apr 21, 2017 12:41PM was wrong.
With -Pandroid.useDexArchive=false it looks like this

:app:transformClassesWithDesugarForNonpayItalyCompatDebugAndroidTest
:app:transformClassesWithPreDexForNonpayItalyCompatDebugAndroidTest
Dex: warning: Ignoring InnerClasses attribute for an anonymous inner class
(org.ccil.cowan.tagsoup.Parser$1) that doesn't come with an
associated EnclosingMethod attribute. This class was probably produced by a
compiler that did not target the modern .class file format. The recommended
solution is to recompile the class from source, using an up-to-date compiler
and without specifying any "-target" type options. The consequence of ignoring
this warning is that reflective operations on this class will incorrectly
indicate that it is *not* an inner class.

:app:transformDexWithDexForNonpayItalyCompatDebugAndroidTest FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:transformDexWithDexForNonpayItalyCompatDebugAndroidTest'.
> com.android.build.api.transform.TransformException: com.android.ide.common.process.ProcessException: java.util.concurrent.ExecutionException: com.android.dex.DexIndexOverflowException: method ID not in [0, 0xffff]: 65536

kl...@google.com <kl...@google.com> #7Mar 30, 2017 03:09PM

We updated our example repo

https://github.com/originx/Gradle_troubles_android_enteprise where you can see the issue.
Just run

./gradlew spoonGermanyDebugAndroidTest
or
./gradlew spoonGermanyDebugAndroidTest -Pandroid.useDexArchive=false

and you will see the issue

kl...@google.com <kl...@google.com> #8Apr 1, 2017 10:57PM

Your tests do not fit into a single dex file. I've tried plugin 2.3.0 (I've also had to enable retrolambda for app, core, and pay modules), and the number of references that are generated is higher than 64k.
Please note that if you are using plugin's internal or Retrolamba support for Java 8 language features, for each lambda expression you will generate 3 additional references, so it might affect your reference count quite significantly.

kl...@google.com <kl...@google.com> #9Apr 1, 2017 10:58PM

Hello and thank you for the reply. What is the expected fix for this the ticket status was moved to "verified (fixed)" it is rather confusing for me. Do you suggest we have too many tests and that's it?
Could you please clarify?
Thank you for your time

kl...@google.com <kl...@google.com> #10Apr 1, 2017 10:59PM

Your test sources and test-only dependencies are generating too many references. Java 8 language features are not related to this, as even when I remove compileOptions from all modules, the test dependencies cannot fit into a single dex file.

I suggest you take a look at Proguard rules that you can specify when running the tests -

https://google.github.io/android-gradle-dsl/current/com.android.build.gradle.internal.dsl.ProductFlavor.html#com.android.build.gradle.internal.dsl.ProductFlavor:testProguardFile(java.lang.Object). I've also updated the bug status, not to be confusing.

kl...@google.com <kl...@google.com> #11Apr 2, 2017 01:52AM

Hello,

Thank you for the link and clarification.
Pardon my many questions I just want to understand the problem better.

We use proguard for the app and define proguard rules which classes to keep for the test runner that we need early on in the start of the test run.
Why did it work with older build tools?
Also why doesnt multidex work for the test app or proguard rules from the debug builds?

Thank you for the quick response

kl...@google.com <kl...@google.com> #12Apr 7, 2017 11:06PM

No worries Mario, I would also like to understand where the issue is coming from :)

The fact that it worked with 2.3.0 and not with 2.4.0-alpha4 is worrying. Because I am unable to reproduce with the test project you've provided, can you please compare the contents of build/intermediates/transforms/preDex/androidTest/ when building tests:
- from clean, with 2.3.0
- from clean, with 2.4.0-alpha4, with -Pandroid.useDexArchive=false
The content of these 2 directories should be the same. Please report any differences.

The support in the test runner is missing to be able to run test apk with multiple DEX files. I don't have much more info on it, sorry.

e....@gmail.com <e....@gmail.com> #13Apr 10, 2017 10:02AM

Hello,

sorry I am a bit restricted in the work environment so I cannot build the demo project from this location but I executed a diff on a real project where the issue happens for dex folders (predex is not available in 2.3.0) its disabled for our project.

this is for 2.3.0 and 2.4.0-alpha4 in location build/intermediates/transforms/dex/androidTest/nonPayMexicoBleeding for following command:
./gradlew clean connectedNonpayMexicoBleedingDebugAndroidTest
I used flag -Pandroid.useDexArchive=false for 2.4.0 only.

diff -rq nonpayMexicoBleeding\ 2.3.0/ nonpayMexicoBleeding\ 2.4.0-alpha4/
Result:
Only in nonpayMexicoBleeding 2.3.0/debug/folders/1000: 1
Only in nonpayMexicoBleeding 2.3.0/debug/folders/1000: 10
Only in nonpayMexicoBleeding 2.4.0-alpha4/debug/folders/1000: 1f

Does this help?

e....@gmail.com <e....@gmail.com> #14Apr 10, 2017 10:03AM

I updated the demo project so you have reproducable example:

https://github.com/originx/Gradle_troubles_android_enteprise

./gradlew clean connectedNonpayMexicoBleedingDebugAndroidTest

Gradle 3.4.0-alpha7 breaks
Commit: ca9e7f7134e5418f9e27e3164efe25521a324a46

Gradle 3.1.2 works
Commit: cfa48cf0b25427d48b328ef6cf80d51fc4e5933b

Cheers

e....@gmail.com <e....@gmail.com> #15Apr 10, 2017 12:26PM

Thanks for creating a repro project. I'll try it asap.

kl...@google.com <kl...@google.com> #16Apr 11, 2017 12:41AM

It works with 2.3.1 because we do not merge the individual dex files, so every test dependency gets dexed individually, and packaged in the test APK. In 2.4.0-alpha7, we are merging all test dependencies to a DEX file, and in this case, they do not fit.

kl...@google.com <kl...@google.com> #17Jul 25, 2017 07:34PM

Marked as fixed.

so we would need to turn on multidex on the test APK to make this work ?

ge...@gmail.com <ge...@gmail.com> #18Dec 9, 2017 10:51PM

I think we can do the following:
- If the tested app is mono-dex, test will be mono dex for < 21, and multidex for 21+.
- If the tested app is native multidex, test is as well.
- And if the tested app is legacy multidex, test is mono dex (because currently the test runner does not support pre 21 test multidex apks).
WDYT?

kl...@google.com <kl...@google.com> #19Dec 9, 2017 11:19PM

Hello,

Sounds like a solution that would make it work on gradle 3.4.0>= ?

What are the side effects from the architecture point of view, performance etc, depending on the effort there would it pay off to bring full multidex support to test runner and test apk both?

I always find it confusing what is the difference between:
-multidex test apk
-multidex test runner

I thought that multidex apk is already supported by MultidexInstrumentationTestRunner

https://developer.android.com/reference/com/android/test/runner/MultiDexTestRunner.html

so that should work, or are you suggesting to introduce the multidex support to the test runner also (what is less effort and more benefit from your side? )

Best regards & thanks for your time