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.

This requirement for addObserver to be called on the main thread is part of the Lifecycle 2.3.0-alpha06 release notes, which Fragment/Activity depend on:

LifecycleRegistry now verifies that its methods are called on main thread. It was always a requirement for lifecycles of activities, fragments etc. An addition of observers from non-main threads resulted in hard to catch crashes in runtime.

It affects all APIs that use Lifecycle, including Navigation.

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

Thanks! Somehow I've missed that.

src-files.txt

2.8 KB

View

Download

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

Project: platform/frameworks/support
Branch: androidx-main

commit 76230d4cbcf193ad3c01e2fb317a1bb3c571b1de
Author: Ian Lake <ilake@google.com>
Date: Tue Apr 06 12:53:25 2021

Mark navigate/setGraph as @MainThread

With the upgrade to Lifecycle 2.3.1,
Lifecycle now enforces that all creation and
changes to the Lifecycle state (including adding
and removing observers) should be done on the
main thread.

By marking navigate() and setGraph() as
`@MainThread`, we can give developeres better
visibility that these operations should only
be done on the main thread.

Relnote: "Navigation now depends on
[Lifecycle `2.3.1`](/jetpack/androidx/releases/lifecycle#2.3.1)
and now marks `setGraph()` and `navigate()`, the
methods that update the `NavBackStackEntry` `Lifecycle`,
as `@MainThread`, aligning Navigation with the main thread
enforcement introduced in Lifecycle `2.3.0`."
Test: existing and updated tests pass
BUG: 171125856

Change-Id: Ifcbe407d9cb186b50f05cbbd8bc5e11e19115a82

M navigation/navigation-dynamic-features-fragment/src/androidTest/java/androidx/navigation/dynamicfeatures/fragment/DynamicNavHostFragmentTest.kt
M navigation/navigation-runtime/api/current.txt
M navigation/navigation-runtime/api/public_plus_experimental_current.txt
M navigation/navigation-runtime/api/restricted_current.txt
M navigation/navigation-runtime/build.gradle
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt

https://android-review.googlesource.com/1665821

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

For the upcoming Navigation 2.4.0-alpha01 release, we've made it explicit that setGraph() and navigate() should only be called on the main thread (by adding @MainThread annotations to the methods), thus aligning with the main thread enforcement of Lifecycle 2.3.

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

Project: platform/frameworks/support
Branch: androidx-main

commit 00c608a86c44b066198bd8c1dc7e2aef6088d4e1
Author: Ian Lake <ilake@google.com>
Date: Mon Apr 12 14:04:54 2021

Mark popBackStack and navigateUp as @MainThread

In addition to the navigate() and setGraph()
methods updated in

https://android-review.googlesource.com/1665821
the methods for popping the back stack
should also be marked as @MainThread.

Relnote: "Navigation now depends on
[Lifecycle `2.3.1`](/jetpack/androidx/releases/lifecycle#2.3.1)
and now marks all of the methods that update the
`NavBackStackEntry` `Lifecycle`, including `setGraph()`,
`navigate()`, `popBackStack()`, and `navigateUp()`,
as `@MainThread`, aligning Navigation with the main thread
enforcement introduced in Lifecycle `2.3.0`."
Test: existing and updated tests pass
BUG: 171125856

Change-Id: Ib8bedbb3fe384b28c7441cbd57a0751eba307a2b

M navigation/navigation-runtime/api/current.txt
M navigation/navigation-runtime/api/public_plus_experimental_current.txt
M navigation/navigation-runtime/api/restricted_current.txt
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt

https://android-review.googlesource.com/1673448

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

Thank you very much. It looks like the issue in #6 is that a stock Sun/Oracle implementation of SHA1withRSA Signature is used instead of a PKCS11-specific one. The stock Sun/Oracle implementation doesn't know (as expected) how to handle hardware-backed PrivateKey instances, which is the type of keys loaded from PKCS#11 hardware-backed keystore. I wonder whether jarsigner contains additional code, specifically for PKCS11 keystores. Or, perhaps, jarsigner is run with additional code/JARs in its CLASSPATH...

I'll dig around to investigate. For now, it does indeed look like you'll need to continue using jarsigner to sign your APKs.

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

[Comment deleted]

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

[Comment deleted]

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

Fixes up for review:

https://android-review.googlesource.com/#/c/362613/ (depends on

https://android-review.googlesource.com/#/c/362029/).

There are two issues here:
1. --ks NONE means KeyStore.load needs to be invoked with a null InputStream rather than a null LoadStoreParameter.
2. before signing, sun.security.pkcs11.SunPKCS11 Provider needs to be added to the list of registered JCA providers. Otherwise, JCA cannot find a Provider which can offer Signature.SHA1withRSA and/or Signature.SHA256withRSA for the hardware-backed PrivateKey created by the PKCS11 KeyStore.

With the above fixes in place, the following should work:

apksigner sign \
--provider-class sun.security.pkcs11.SunPKCS11 \
--provider-arg "$JDK_PATH\bin\eToken.cfg" \
--ks NONE \
--ks-pass "pass:$STOREPASS" \
--ks-type PKCS11 \
--ks-key-alias "my alias" \
some.apk

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

The fixes have landed. Would you please confirm that, if you build apksigner from commit b3049643c3eba5fdbecc7550df8e15da2ba35934 or newer, it works with your eToken (see command example in

comment #10

)? Thank you very much for helping identify and fix this issue.

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

Éric, would you mind confirming that the fix mentioned in

comment #10

/#11 makes apksigner work with your eToken?

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

[Comment deleted]

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

Hi thanks for your patch and sorry for the delay (I was working on another project last week).
Yes it do the jobs and I can now sign the app file using the Usb-Dongle.
The "apksigner -verify" will also return positive result.
Regards, Éric.

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

When I use my private KeyStore, an Exception happens:
--ks "easySoft-App2.p12"
--ks-type PKCS12
--ks-pass pass:xxxxx
--ks-key-alias easysoft.test
my.apk
Failed to load signer "signer #1"
java.io.IOException: parseAlgParameters failed: PBE AlgorithmParameters not available
at sun.security.pkcs12.PKCS12KeyStore.parseAlgParameters(PKCS12KeyStore.java:792)
at sun.security.pkcs12.PKCS12KeyStore.engineLoad(PKCS12KeyStore.java:1998)
at java.security.KeyStore.load(KeyStore.java:1445)
at com.android.apksigner.ApkSignerTool$SignerParams.loadKeyStoreFromFile(ApkSignerTool.java:808)
at com.android.apksigner.ApkSignerTool$SignerParams.loadPrivateKeyAndCertsFromKeyStore(ApkSignerTool.java:700)
at com.android.apksigner.ApkSignerTool$SignerParams.loadPrivateKeyAndCerts(ApkSignerTool.java:646)
at com.android.apksigner.ApkSignerTool$SignerParams.access$500(ApkSignerTool.java:600)
at com.android.apksigner.ApkSignerTool.sign(ApkSignerTool.java:255)
at com.android.apksigner.ApkSignerTool.main(ApkSignerTool.java:88)
Caused by: java.security.NoSuchAlgorithmException: PBE AlgorithmParameters not available
at sun.security.jca.GetInstance.getInstance(GetInstance.java:159)
at java.security.Security.getImpl(Security.java:695)
at java.security.AlgorithmParameters.getInstance(AlgorithmParameters.java:146)
at sun.security.pkcs12.PKCS12KeyStore.parseAlgParameters(PKCS12KeyStore.java:786)
... 8 more

I hope it's not due to the patch.
Regards.

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

Thanks. I'm really glad we've sorted out the PKCS #11 issue. Please file a separate ticket for the PKCS #12 issue, and post a link here for continuity. In that new ticket, please also mention whether this works with jarsigner and what parameters you pass into jarsigner. This would be similar to the original report here which was quite detailed and informative.

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

Marked as fixed.

The fix has been released in apksigner 0.7, released as part of Android SDK Build Tools 26.0.1.

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

Thanks for this

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

You're welcome!

Issue 37140484

Description

Issue summary

Comments

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Add comment

Issue metadata