Remove Android support for out-of-process KeyStores

Remove Android support for out-of-process KeyStores

Support for out-of-process KeyStores was added to Chrome for Android
builds to support certain OEM-specific functionality. It worked by binding
to a service that implemented the RemoteAndroidKeyStoreInterface, and
using that for private key operations, rather than the default in-process
KeyStore.

However, support for this functionality in Chrome is being retired;
device manufacturers wishing to offer this functionality to Chrome
users can simply fall back to the default KeyStore implementation.

As such, remove the code and service descriptors; only local,
in-process KeyStores will be supported.

BUG=560614
Committed: https://crrev.com/e04b8f5353ab621c48b90bb46fee24fd0c0f4a29
Cr-Commit-Position: refs/heads/master@{#362649}

[Changwan Ryu, 2015-11-25 07:46:17]

Description was changed from

==========
Clean up chrome support library support

BUG=560614
==========

to

==========
Clean up chrome support library support

We decided not to support smart card authentication any more.

BUG=560614
==========

[Changwan Ryu, 2015-11-25 07:54:21]

changwan@chromium.org changed reviewers:
+ yfriedman@chromium.org

[Yaron, 2015-11-25 16:24:31]

https://codereview.chromium.org/1474603004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java
(left):

https://codereview.chromium.org/1474603004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java:361:
KeyStoreSelectionDialog selectionDialog = new KeyStoreSelectionDialog(
You can remove this class too and the strings added in
https://codereview.chromium.org/133373009

https://codereview.chromium.org/1474603004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java
(right):

https://codereview.chromium.org/1474603004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java:48:
new DefaultAndroidKeyStore();
Any reason not to remove the AndroidKeyStore interface? Nothing takes advantage
of it (e.g. for testing)

[davidben, 2015-11-25 17:48:14]

https://codereview.chromium.org/1474603004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java
(right):

https://codereview.chromium.org/1474603004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java:48:
new DefaultAndroidKeyStore();
On 2015/11/25 16:24:31, Yaron wrote:
> Any reason not to remove the AndroidKeyStore interface? Nothing takes
advantage
> of it (e.g. for testing)

+1

I believe this is the CL that introduced it:
https://codereview.chromium.org/166143002 The code's changed in the meantime, so
there'll probably be some unraveling to do, but one less layer of indirection.

[Changwan Ryu, 2015-11-26 02:52:11]

https://codereview.chromium.org/1474603004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java
(left):

https://codereview.chromium.org/1474603004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java:361:
KeyStoreSelectionDialog selectionDialog = new KeyStoreSelectionDialog(
On 2015/11/25 16:24:31, Yaron wrote:
> You can remove this class too and the strings added in
> https://codereview.chromium.org/133373009

Done.

https://codereview.chromium.org/1474603004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java
(right):

https://codereview.chromium.org/1474603004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java:48:
new DefaultAndroidKeyStore();
On 2015/11/25 17:48:14, davidben (slow) wrote:
> On 2015/11/25 16:24:31, Yaron wrote:
> > Any reason not to remove the AndroidKeyStore interface? Nothing takes
> advantage
> > of it (e.g. for testing)
> 
> +1
> 
> I believe this is the CL that introduced it:
> https://codereview.chromium.org/166143002 The code's changed in the meantime,
so
> there'll probably be some unraveling to do, but one less layer of indirection.

No need to keep this. Thanks for the pointer. Done as suggested.

[Yaron, 2015-11-27 15:16:59]

yfriedman@chromium.org changed reviewers:
+ davidben@chromium.org

[Yaron, 2015-11-27 15:16:59]

lgtm - thanks!
You'll need someone from aw/ to look too

[Changwan Ryu, 2015-11-30 00:16:35]

changwan@chromium.org changed reviewers:
+ sgurun@chromium.org

[Changwan Ryu, 2015-11-30 00:16:35]

sgurun@chromium.org: Please review changes in 

android_webview/

[sgurun-gerrit only, 2015-11-30 19:05:20]

lgtm with a minor comment.

https://codereview.chromium.org/1474603004/diff/40001/android_webview/unittes...
File
android_webview/unittestjava/src/org/chromium/android_webview/unittest/MockAwContentsClientBridge.java
(right):

https://codereview.chromium.org/1474603004/diff/40001/android_webview/unittes...
android_webview/unittestjava/src/org/chromium/android_webview/unittest/MockAwContentsClientBridge.java:45:
private PrivateKey createTestPrivateKey() {
you can drop this method then, and use null at the call site.

[Changwan Ryu, 2015-12-01 08:17:23]

https://codereview.chromium.org/1474603004/diff/40001/android_webview/unittes...
File
android_webview/unittestjava/src/org/chromium/android_webview/unittest/MockAwContentsClientBridge.java
(right):

https://codereview.chromium.org/1474603004/diff/40001/android_webview/unittes...
android_webview/unittestjava/src/org/chromium/android_webview/unittest/MockAwContentsClientBridge.java:45:
private PrivateKey createTestPrivateKey() {
On 2015/11/30 19:05:20, sgurun wrote:
> you can drop this method then, and use null at the call site.

Done.

[Yaron, 2015-12-01 15:21:49]

On 2015/12/01 08:17:23, Changwan Ryu wrote:
>
https://codereview.chromium.org/1474603004/diff/40001/android_webview/unittes...
> File
>
android_webview/unittestjava/src/org/chromium/android_webview/unittest/MockAwContentsClientBridge.java
> (right):
> 
>
https://codereview.chromium.org/1474603004/diff/40001/android_webview/unittes...
>
android_webview/unittestjava/src/org/chromium/android_webview/unittest/MockAwContentsClientBridge.java:45:
> private PrivateKey createTestPrivateKey() {
> On 2015/11/30 19:05:20, sgurun wrote:
> > you can drop this method then, and use null at the call site.
> 
> Done.

lgtm

[Changwan Ryu, 2015-12-02 00:41:30]

changwan@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Changwan Ryu, 2015-12-02 00:41:30]

adding rsleevi for net/ changes since davidben is behind on reviews.

[Ryan Sleevi, 2015-12-02 00:54:21]

On 2015/12/02 00:41:30, Changwan Ryu wrote:
> adding rsleevi for net/ changes since davidben is behind on reviews.

Initial remark: Please reword the description to be clear about what's changing,
why, and the impact.

"Remove Android support for out-of-process KeyStores

Support for out-of-process KeyStores was added to Chrome for Android
builds for [reason X, which is suitable for public discussion]. It
worked by [confirm this description is accurate] binding to a service
that implemented the RemoteAndroidKeyStoreInterface, and using that
for private key operations, rather than the default in-process
KeyStore.

However, support for this functionality in Chrome is being retired;
device manufacturers wishing to offer this functionality to Chrome
users can do [describe alternative here - presumably, it's replace
the default KeyStore implementation in Android? Or is there no
alternative now?]

As such, remove the code and service descriptors; only local,
in-process KeyStores will be supported.

BUG=blah"

[Ryan Sleevi, 2015-12-02 01:06:05]

https://codereview.chromium.org/1474603004/diff/60001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java
(right):

https://codereview.chromium.org/1474603004/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java:40:
private final ClientCertLookupTable mLookupTable;
This change is unrelated, isn't it?

https://codereview.chromium.org/1474603004/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java:65:
private final int mPort;
This is all unrelated, isn't it?

https://codereview.chromium.org/1474603004/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java
(right):

https://codereview.chromium.org/1474603004/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java:48:
* AsyncTask retrieve the authentication material from the system key store.
grammar: s/retrieve/retrieves/

https://codereview.chromium.org/1474603004/diff/60001/net/android/java/src/or...
File net/android/java/src/org/chromium/net/AndroidKeyStore.java (right):

https://codereview.chromium.org/1474603004/diff/60001/net/android/java/src/or...
net/android/java/src/org/chromium/net/AndroidKeyStore.java:102: // on Android
4.0.x and 4.1.x. Fixed in 4.2 and higher.
We dropped support for 4.0.x and 4.1.x, right? Does this comment still need to
be carried over?

(If not, we should delete this code in a follow-up CL)

https://codereview.chromium.org/1474603004/diff/60001/net/android/java/src/or...
net/android/java/src/org/chromium/net/AndroidKeyStore.java:211: * never use this
on Android 4.2 or higher.
Same comments re: Android version support

[Changwan Ryu, 2015-12-02 05:33:36]

Description was changed from

==========
Clean up chrome support library support

We decided not to support smart card authentication any more.

BUG=560614
==========

to

==========
Remove Android support for out-of-process KeyStores

Support for out-of-process KeyStores was added to Chrome for Android
builds to support certain OEM-specific functionality. It worked by binding
to a service that implemented the RemoteAndroidKeyStoreInterface, and
using that for private key operations, rather than the default in-process
KeyStore.

However, support for this functionality in Chrome is being retired;
device manufacturers wishing to offer this functionality to Chrome
users can simply fall back to the default KeyStore implementation.

As such, remove the code and service descriptors; only local,
in-process KeyStores will be supported.

BUG=560614
==========

[Changwan Ryu, 2015-12-02 05:34:13]

Changed CL description as suggested. Thanks for pointing it out and drafting it!
PTAL.

https://codereview.chromium.org/1474603004/diff/60001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java
(right):

https://codereview.chromium.org/1474603004/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java:40:
private final ClientCertLookupTable mLookupTable;
On 2015/12/02 01:06:04, Ryan Sleevi wrote:
> This change is unrelated, isn't it?

This is automatic change from Eclipse, and it is customary to accept this type
of changes even though it's unrelated.

https://codereview.chromium.org/1474603004/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java:65:
private final int mPort;
On 2015/12/02 01:06:04, Ryan Sleevi wrote:
> This is all unrelated, isn't it?

Same as above. This is automatic change coming from Eclipse when you save a
file.

https://codereview.chromium.org/1474603004/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java
(right):

https://codereview.chromium.org/1474603004/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java:48:
* AsyncTask retrieve the authentication material from the system key store.
On 2015/12/02 01:06:04, Ryan Sleevi wrote:
> grammar: s/retrieve/retrieves/

Done.

https://codereview.chromium.org/1474603004/diff/60001/net/android/java/src/or...
File net/android/java/src/org/chromium/net/AndroidKeyStore.java (right):

https://codereview.chromium.org/1474603004/diff/60001/net/android/java/src/or...
net/android/java/src/org/chromium/net/AndroidKeyStore.java:102: // on Android
4.0.x and 4.1.x. Fixed in 4.2 and higher.
On 2015/12/02 01:06:04, Ryan Sleevi wrote:
> We dropped support for 4.0.x and 4.1.x, right? Does this comment still need to
> be carried over?
> 
> (If not, we should delete this code in a follow-up CL)

We still support 4.1 (JB). Updated the comment.

https://codereview.chromium.org/1474603004/diff/60001/net/android/java/src/or...
net/android/java/src/org/chromium/net/AndroidKeyStore.java:211: * never use this
on Android 4.2 or higher.
On 2015/12/02 01:06:04, Ryan Sleevi wrote:
> Same comments re: Android version support

We still support 4.1, so this comment is still valid.

[Ryan Sleevi, 2015-12-02 06:16:54]

lgtm

[Changwan Ryu, 2015-12-02 06:26:31]

The CQ bit was checked by changwan@chromium.org

[Changwan Ryu, 2015-12-02 06:26:32]

The patchset sent to the CQ was uploaded after l-g-t-m from sgurun@chromium.org,
yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/1474603004/#ps80001
(title: "updated comments")

[commit-bot: I haz the power, 2015-12-02 06:26:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1474603004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1474603004/80001

[commit-bot: I haz the power, 2015-12-02 06:44:07]

Description was changed from

==========
Remove Android support for out-of-process KeyStores

Support for out-of-process KeyStores was added to Chrome for Android
builds to support certain OEM-specific functionality. It worked by binding
to a service that implemented the RemoteAndroidKeyStoreInterface, and
using that for private key operations, rather than the default in-process
KeyStore.

However, support for this functionality in Chrome is being retired;
device manufacturers wishing to offer this functionality to Chrome
users can simply fall back to the default KeyStore implementation.

As such, remove the code and service descriptors; only local,
in-process KeyStores will be supported.

BUG=560614
==========

to

==========
Remove Android support for out-of-process KeyStores

Support for out-of-process KeyStores was added to Chrome for Android
builds to support certain OEM-specific functionality. It worked by binding
to a service that implemented the RemoteAndroidKeyStoreInterface, and
using that for private key operations, rather than the default in-process
KeyStore.

However, support for this functionality in Chrome is being retired;
device manufacturers wishing to offer this functionality to Chrome
users can simply fall back to the default KeyStore implementation.

As such, remove the code and service descriptors; only local,
in-process KeyStores will be supported.

BUG=560614
==========

[commit-bot: I haz the power, 2015-12-02 06:44:07]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-12-02 06:45:10]

Description was changed from

==========
Remove Android support for out-of-process KeyStores

Support for out-of-process KeyStores was added to Chrome for Android
builds to support certain OEM-specific functionality. It worked by binding
to a service that implemented the RemoteAndroidKeyStoreInterface, and
using that for private key operations, rather than the default in-process
KeyStore.

However, support for this functionality in Chrome is being retired;
device manufacturers wishing to offer this functionality to Chrome
users can simply fall back to the default KeyStore implementation.

As such, remove the code and service descriptors; only local,
in-process KeyStores will be supported.

BUG=560614
==========

to

==========
Remove Android support for out-of-process KeyStores

Support for out-of-process KeyStores was added to Chrome for Android
builds to support certain OEM-specific functionality. It worked by binding
to a service that implemented the RemoteAndroidKeyStoreInterface, and
using that for private key operations, rather than the default in-process
KeyStore.

However, support for this functionality in Chrome is being retired;
device manufacturers wishing to offer this functionality to Chrome
users can simply fall back to the default KeyStore implementation.

As such, remove the code and service descriptors; only local,
in-process KeyStores will be supported.

BUG=560614

Committed: https://crrev.com/e04b8f5353ab621c48b90bb46fee24fd0c0f4a29
Cr-Commit-Position: refs/heads/master@{#362649}
==========

[commit-bot: I haz the power, 2015-12-02 06:45:11]

Patchset 5 (id:??) landed as
https://crrev.com/e04b8f5353ab621c48b90bb46fee24fd0c0f4a29
Cr-Commit-Position: refs/heads/master@{#362649}