Wire up SSL client authentication for OpenSSL/Android through the net/ stack

chrome/browser/ui/android/ssl_client_certificate_selector_android.h

Index: chrome/browser/ui/android/ssl_client_certificate_selector_android.h
diff --git a/chrome/browser/ui/android/ssl_client_certificate_selector_android.h b/chrome/browser/ui/android/ssl_client_certificate_selector_android.h
new file mode 100644
index 0000000000000000000000000000000000000000..e33ff0d8d2c498746e0c3c17d558e0b44278e4be
--- /dev/null
+++ b/chrome/browser/ui/android/ssl_client_certificate_selector_android.h
@@ -0,0 +1,65 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_ANDROID_SSL_CLIENT_CERTIFICATE_SELECTOR_ANDROID_H_
+#define CHROME_BROWSER_UI_ANDROID_SSL_CLIENT_CERTIFICATE_SELECTOR_ANDROID_H_

[Ryan Sleevi, 2013/02/12 00:25:17]

Why is the _Android suffix necessary? You're in chrome/browser/ui/android - it
seems unnecessary.

[digit1, 2013/02/12 15:05:25]

It's not necessary, but some developers prefer to add it nonetheless because it
makes it easier to distinguish between different implementations of the same
files in their favorite editor/IDE.

Given that there are already several files using this convention in this
directory, I just followed the trend.

I really don't have any preference here though, so if you insist on changing
this, please let me know.

+
+#include <jni.h>
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "chrome/browser/ssl/ssl_client_certificate_selector.h"
+
+namespace net {
+class SSLCertRequestInfo;
+}  // namespace net
+
+namespace browser {

[Ryan Sleevi, 2013/02/12 20:12:58]

namespace chrome {
namespace browser {

[digit1, 2013/02/13 18:24:34]

Actually, this can't be under the chrome namespace due to weird interactions
with the JNI generated headers. To be more exact, this makes
chrome/browser/android/chrome_jni_registrar.cc fail to compile.

Also, the other ports like chrome/browser/ui/views/ don't even use any namespace
to declare their helper classes. I didn't want to do this.

+
+class SSLClientCertRequest {

[Ryan Sleevi, 2013/02/12 00:25:17]

??? This doesn't match the naming of the file - totally surprised to see this
class in here.

Did you mean to call it SSLClientCertificateSelector?

[digit1, 2013/02/12 15:05:25]

I really meant SSLClientCertRequest, it's a C++ class that matches its Java
counterpart, as expected by the JNI generator. The main reason why this is here
is because the auto-generated header "jni/SSLClientCertificateSelector_jni.h"
needs the full declaration of the class before the include (that happens in the
.cc file).

I'll move this to its own header/source file to clean this up.

[Ryan Sleevi, 2013/02/12 20:12:58]

But then there is no SSLClientCertificateSelectorAndroid, is there?

Or are you seeing this header containing only the one function from line 59/60?

[digit1, 2013/02/13 18:24:34]

I've removed the chrome/ changes from this review, but if you look at
intermediate ones (e.g. Patch set 4), you'll see the following:

- ssl_client_certificate_selector_android.h is removed.
- ssl_client_certificate_request.h / .cc are added.
- ssl_client_certificate_selector_android.cc implements
chrome::ShowSSLClientCertificateSelector(), by using
SSLClientCertificateRequest.

+ public:
+  SSLClientCertRequest(
+    net::SSLCertRequestInfo* cert_request_info,
+    const chrome::SelectCertificateCallback& callback)
+    : cert_request_info_(cert_request_info),
+      callback_(callback) { }

[Ryan Sleevi, 2013/02/12 20:12:58]

style: indents are wrong (need two additional spaces on line 23 & 24), and
braces should be should be on separate lines for multiline initializer lists.

[digit1, 2013/02/13 18:24:34]

thanks, I'll fix this in the next patch that revives the chrome changes.

+
+  ~SSLClientCertRequest() {}
+
+  // Start the request
+  bool Start();
+
+  // Called from Java through JNI when the request completes or was
+  // cancelled by the user.
+  // |env| is the current threads' JNIEnv handle.
+  // |obj| is a JNI reference to the Java object instance associated
+  // with this request.
+  // |private_key_alias| is a JNI string reference to the private key
+  // unique name.
+  // |encoded_chain_ref| is a JNI reference to an array of byte arrays
+  // modelling the encoded client certificate chain. Will be null if
+  // the request was cancelled or an error occured.
+  // |private_key_ref| is a JNI reference to the PrivateKey object for
+  // the client certificate. Will be null if the request was cancelled
+  // or an error occured.
+  void OnRequestCompletion(JNIEnv* env,
+                           jobject obj,
+                           jstring private_key_alias_ref,
+                           jobjectArray encoded_chain_ref,
+                           jobject private_key_ref);
+
+ private:
+  net::SSLCertRequestInfo* cert_request_info_;
+  chrome::SelectCertificateCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(SSLClientCertRequest);
+};
+
+// Register JNI methods.
+bool RegisterSSLClientCertificateSelectorAndroid(JNIEnv* env);
+
+}  // namespace browser
+
+#endif  // CHROME_BROWSER_UI_ANDROID_SSL_CLIENT_CERTIFICATE_SELECTOR_ANDROID_H_
+