Bug 1552539 - Refactor Android WebAuthn methods to use more GeckoBundles r=keeler
authorJ.C. Jones <jjones@mozilla.com>
Fri, 17 May 2019 18:36:01 +0000
changeset 474384 02f6bb0dec8a9c95ed4a01235ee684b24fd95251
parent 474383 f12fb7f69d5180c2c133b441e23b0b8b8d4fe123
child 474385 f5f6979aa22ac36f8caefbed781a6d97788d5f89
push id113152
push userdluca@mozilla.com
push dateSat, 18 May 2019 10:33:03 +0000
treeherdermozilla-inbound@9b2f851979cb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1552539
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1552539 - Refactor Android WebAuthn methods to use more GeckoBundles r=keeler Differential Revision: https://phabricator.services.mozilla.com/D31636
dom/webauthn/AndroidWebAuthnTokenManager.cpp
mobile/android/base/java/org/mozilla/gecko/util/WebAuthnUtils.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/WebAuthnTokenManager.java
--- a/dom/webauthn/AndroidWebAuthnTokenManager.cpp
+++ b/dom/webauthn/AndroidWebAuthnTokenManager.cpp
@@ -56,23 +56,16 @@ void AndroidWebAuthnTokenManager::Drop()
   gAndroidWebAuthnManager = nullptr;
   gAndroidPBackgroundThread = nullptr;
 }
 
 RefPtr<U2FRegisterPromise> AndroidWebAuthnTokenManager::Register(
     const WebAuthnMakeCredentialInfo& aInfo, bool aForceNoneAttestation) {
   AssertIsOnOwningThread();
 
-  if (aInfo.Extra().isNothing()) {
-    // Mostly ready for U2F, but requires using a different provider
-    // at the Java-side. Finish out in Bug 1550625
-    return U2FRegisterPromise::CreateAndReject(NS_ERROR_DOM_NOT_SUPPORTED_ERR,
-                                               __func__);
-  }
-
   ClearPromises();
 
   GetMainThreadEventTarget()->Dispatch(NS_NewRunnableFunction(
       "java::WebAuthnTokenManager::WebAuthnMakeCredential",
       [aInfo, aForceNoneAttestation]() {
         AssertIsOnMainThread();
 
         // Produce the credential exclusion list
@@ -103,25 +96,25 @@ RefPtr<U2FRegisterPromise> AndroidWebAut
             const_cast<void*>(static_cast<const void*>(challBuf.Elements())),
             challBuf.Length());
 
         nsTArray<uint8_t> uidBuf;
 
         // Get authenticator selection criteria
         GECKOBUNDLE_START(authSelBundle);
         GECKOBUNDLE_START(extensionsBundle);
-        GECKOBUNDLE_START(identifierBundle);
+        GECKOBUNDLE_START(credentialBundle);
 
         if (aInfo.Extra().isSome()) {
           const auto& extra = aInfo.Extra().ref();
           const auto& rp = extra.Rp();
           const auto& user = extra.User();
 
           // If we have extra data, then this is WebAuthn, not U2F
-          GECKOBUNDLE_PUT(identifierBundle, "isWebAuthn",
+          GECKOBUNDLE_PUT(credentialBundle, "isWebAuthn",
                           java::sdk::Integer::ValueOf(1));
 
           // Get the attestation preference and override if the user asked
           AttestationConveyancePreference attestation =
               extra.attestationConveyancePreference();
 
           if (aForceNoneAttestation) {
             // Add UI support to trigger this, bug 1550164
@@ -171,41 +164,47 @@ RefPtr<U2FRegisterPromise> AndroidWebAut
                   extensionsBundle, "fidoAppId",
                   jni::StringParam(
                       ext.get_WebAuthnExtensionAppId().appIdentifier()));
             }
           }
 
           uidBuf.Assign(user.Id());
 
-          GECKOBUNDLE_PUT(identifierBundle, "rpName",
+          GECKOBUNDLE_PUT(credentialBundle, "rpName",
                           jni::StringParam(rp.Name()));
-          GECKOBUNDLE_PUT(identifierBundle, "rpIcon",
+          GECKOBUNDLE_PUT(credentialBundle, "rpIcon",
                           jni::StringParam(rp.Icon()));
-          GECKOBUNDLE_PUT(identifierBundle, "userName",
+          GECKOBUNDLE_PUT(credentialBundle, "userName",
                           jni::StringParam(user.Name()));
-          GECKOBUNDLE_PUT(identifierBundle, "userIcon",
+          GECKOBUNDLE_PUT(credentialBundle, "userIcon",
                           jni::StringParam(user.Icon()));
-          GECKOBUNDLE_PUT(identifierBundle, "userDisplayName",
+          GECKOBUNDLE_PUT(credentialBundle, "userDisplayName",
                           jni::StringParam(user.DisplayName()));
         }
 
+        GECKOBUNDLE_PUT(credentialBundle, "rpId",
+                        jni::StringParam(aInfo.RpId()));
+        GECKOBUNDLE_PUT(credentialBundle, "origin",
+                        jni::StringParam(aInfo.Origin()));
+        GECKOBUNDLE_PUT(credentialBundle, "timeoutMS",
+                        java::sdk::Double::New(aInfo.TimeoutMS()));
+
         GECKOBUNDLE_FINISH(authSelBundle);
         GECKOBUNDLE_FINISH(extensionsBundle);
-        GECKOBUNDLE_FINISH(identifierBundle);
+        GECKOBUNDLE_FINISH(credentialBundle);
 
         // For non-WebAuthn cases, uidBuf is empty (and unused)
         jni::ByteBuffer::LocalRef uid = jni::ByteBuffer::New(
             const_cast<void*>(static_cast<const void*>(uidBuf.Elements())),
             uidBuf.Length());
 
         java::WebAuthnTokenManager::WebAuthnMakeCredential(
-            aInfo.RpId(), identifierBundle, uid, challenge, aInfo.TimeoutMS(),
-            aInfo.Origin(), idList, transportList, authSelBundle,
-            extensionsBundle);
+            credentialBundle, uid, challenge, idList, transportList,
+            authSelBundle, extensionsBundle);
       }));
 
   return mRegisterPromise.Ensure(__func__);
 }
 
 void AndroidWebAuthnTokenManager::HandleRegisterResult(
     const AndroidWebAuthnResult& aResult) {
   // This is likely running on the main thread, so we'll always dispatch to the
@@ -264,34 +263,52 @@ RefPtr<U2FSignPromise> AndroidWebAuthnTo
             transportBuf.Length());
 
         const nsTArray<uint8_t>& challBuf = aInfo.Challenge();
         jni::ByteBuffer::LocalRef challenge = jni::ByteBuffer::New(
             const_cast<void*>(static_cast<const void*>(challBuf.Elements())),
             challBuf.Length());
 
         // Get extensions
+        GECKOBUNDLE_START(assertionBundle);
         GECKOBUNDLE_START(extensionsBundle);
         if (aInfo.Extra().isSome()) {
           const auto& extra = aInfo.Extra().ref();
 
+          // If we have extra data, then this is WebAuthn, not U2F
+          GECKOBUNDLE_PUT(assertionBundle, "isWebAuthn",
+                          java::sdk::Integer::ValueOf(1));
+
+          // User Verification Requirement is not currently used in the
+          // Android FIDO API. Adding it should look like
+          // AttestationConveyancePreference
+
           for (const WebAuthnExtension& ext : extra.Extensions()) {
             if (ext.type() == WebAuthnExtension::TWebAuthnExtensionAppId) {
               GECKOBUNDLE_PUT(
                   extensionsBundle, "fidoAppId",
                   jni::StringParam(
                       ext.get_WebAuthnExtensionAppId().appIdentifier()));
             }
           }
         }
+
+        GECKOBUNDLE_PUT(assertionBundle, "rpId",
+                        jni::StringParam(aInfo.RpId()));
+        GECKOBUNDLE_PUT(assertionBundle, "origin",
+                        jni::StringParam(aInfo.Origin()));
+        GECKOBUNDLE_PUT(assertionBundle, "timeoutMS",
+                        java::sdk::Double::New(aInfo.TimeoutMS()));
+
+        GECKOBUNDLE_FINISH(assertionBundle);
         GECKOBUNDLE_FINISH(extensionsBundle);
 
         java::WebAuthnTokenManager::WebAuthnGetAssertion(
-            aInfo.RpId(), challenge, aInfo.TimeoutMS(), aInfo.Origin(), idList,
-            transportList, extensionsBundle);
+            challenge, idList, transportList, assertionBundle,
+            extensionsBundle);
       }));
 
   return mSignPromise.Ensure(__func__);
 }
 
 void AndroidWebAuthnTokenManager::HandleSignResult(
     const AndroidWebAuthnResult& aResult) {
   // This is likely running on the main thread, so we'll always dispatch to the
--- a/mobile/android/base/java/org/mozilla/gecko/util/WebAuthnUtils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/util/WebAuthnUtils.java
@@ -68,31 +68,36 @@ public class WebAuthnUtils
         }
         if ((transports & AUTHENTICATOR_TRANSPORT_BLE) == AUTHENTICATOR_TRANSPORT_BLE) {
             result.add(Transport.BLUETOOTH_LOW_ENERGY);
         }
 
         return result;
     }
 
-    public static void makeCredential(final String rpId, final GeckoBundle identifiers,
+    public static void makeCredential(final GeckoBundle credentialBundle,
                                       final byte[] userId, final byte[] challenge,
-                                      final long timeoutMs, final String originStr,
                                       final WebAuthnTokenManager.WebAuthnPublicCredential[] excludeList,
                                       final GeckoBundle authenticatorSelection,
                                       final GeckoBundle extensions,
                                       WebAuthnTokenManager.WebAuthnMakeCredentialResponse handler) {
         final Activity currentActivity =
             GeckoActivityMonitor.getInstance().getCurrentActivity();
 
         if (currentActivity == null) {
             handler.onFailure("UNKNOWN_ERR");
             return;
         }
 
+        if (!credentialBundle.containsKey("isWebAuthn")) {
+            // FIDO U2F not supported by Android (for us anyway) at this time
+            handler.onFailure("NOT_SUPPORTED_ERR");
+            return;
+        }
+
         PublicKeyCredentialCreationOptions.Builder requestBuilder =
             new PublicKeyCredentialCreationOptions.Builder();
 
         Fido2PrivilegedApiClient fidoClient = // Only works in released builds
             Fido.getFido2PrivilegedApiClient(currentActivity.getApplicationContext());
 
         List<PublicKeyCredentialParameters> params =
             new ArrayList<PublicKeyCredentialParameters>();
@@ -106,19 +111,19 @@ public class WebAuthnUtils
             }) {
             params.add(new PublicKeyCredentialParameters(
                 PublicKeyCredentialType.PUBLIC_KEY.toString(),
                 algo.getAlgoValue()));
         }
 
         PublicKeyCredentialUserEntity user =
             new PublicKeyCredentialUserEntity(userId,
-                    identifiers.getString("userName", ""),
-                    identifiers.getString("userIcon", ""),
-                    identifiers.getString("userDisplayName", ""));
+                    credentialBundle.getString("userName", ""),
+                    credentialBundle.getString("userIcon", ""),
+                    credentialBundle.getString("userDisplayName", ""));
 
         AttestationConveyancePreference pref =
             AttestationConveyancePreference.NONE;
         String attestationPreference =
             authenticatorSelection.getString("attestationPreference", "NONE");
         if (attestationPreference.equalsIgnoreCase(
                               AttestationConveyancePreference.DIRECT.name())) {
             pref = AttestationConveyancePreference.DIRECT;
@@ -153,34 +158,35 @@ public class WebAuthnUtils
             excludedList.add(
                 new PublicKeyCredentialDescriptor(
                                     PublicKeyCredentialType.PUBLIC_KEY.toString(),
                                     cred.mId,
                                     getTransportsForByte(cred.mTransports)));
         }
 
         PublicKeyCredentialRpEntity rp =
-            new PublicKeyCredentialRpEntity(rpId,
-                    identifiers.getString("rpName", ""),
-                    identifiers.getString("rpIcon", ""));
+            new PublicKeyCredentialRpEntity(
+                    credentialBundle.getString("rpId"),
+                    credentialBundle.getString("rpName", ""),
+                    credentialBundle.getString("rpIcon", ""));
 
         PublicKeyCredentialCreationOptions requestOptions =
             requestBuilder
                 .setUser(user)
                 .setAttestationConveyancePreference(pref)
                 .setAuthenticatorSelection(sel)
                 .setAuthenticationExtensions(ext)
                 .setChallenge(challenge)
                 .setRp(rp)
                 .setParameters(params)
-                .setTimeoutSeconds(timeoutMs / 1000.0)
+                .setTimeoutSeconds(credentialBundle.getLong("timeoutMS") / 1000.0)
                 .setExcludeList(excludedList)
                 .build();
 
-        Uri origin = Uri.parse(originStr);
+        Uri origin = Uri.parse(credentialBundle.getString("origin"));
 
         BrowserPublicKeyCredentialCreationOptions browserOptions =
             new BrowserPublicKeyCredentialCreationOptions.Builder()
                 .setPublicKeyCredentialCreationOptions(requestOptions)
                 .setOrigin(origin)
                 .build();
 
         Task<Fido2PendingIntent> result = fidoClient.getRegisterIntent(browserOptions);
@@ -255,29 +261,35 @@ public class WebAuthnUtils
                 mHandler.onFailure("ABORT_ERR");
                 return;
             }
 
             mHandler.onFailure("UNKNOWN_ERR");
         }
     }
 
-    public static void getAssertion(final String rpId, final byte[] challenge,
-                                    final long timeoutMs, final String originStr,
+    public static void getAssertion(final byte[] challenge,
                                     final WebAuthnTokenManager.WebAuthnPublicCredential[] allowList,
+                                    final GeckoBundle assertionBundle,
                                     final GeckoBundle extensions,
                                     WebAuthnTokenManager.WebAuthnGetAssertionResponse handler) {
         final Activity currentActivity =
             GeckoActivityMonitor.getInstance().getCurrentActivity();
 
         if (currentActivity == null) {
             handler.onFailure("UNKNOWN_ERR");
             return;
         }
 
+        if (!assertionBundle.containsKey("isWebAuthn")) {
+            // FIDO U2F not supported by Android (for us anyway) at this time
+            handler.onFailure("NOT_SUPPORTED_ERR");
+            return;
+        }
+
         List<PublicKeyCredentialDescriptor> allowedList =
             new ArrayList<PublicKeyCredentialDescriptor>();
         for (WebAuthnTokenManager.WebAuthnPublicCredential cred : allowList) {
             allowedList.add(
                 new PublicKeyCredentialDescriptor(
                                     PublicKeyCredentialType.PUBLIC_KEY.toString(),
                                     cred.mId,
                                     getTransportsForByte(cred.mTransports)));
@@ -293,22 +305,22 @@ public class WebAuthnUtils
                 new FidoAppIdExtension(extensions.getString("fidoAppId")));
         }
         AuthenticationExtensions ext = extBuilder.build();
 
         PublicKeyCredentialRequestOptions requestOptions =
             new PublicKeyCredentialRequestOptions.Builder()
                 .setChallenge(challenge)
                 .setAllowList(allowedList)
-                .setTimeoutSeconds(timeoutMs / 1000.0)
-                .setRpId(rpId)
+                .setTimeoutSeconds(assertionBundle.getLong("timeoutMS") / 1000.0)
+                .setRpId(assertionBundle.getString("rpId"))
                 .setAuthenticationExtensions(ext)
                 .build();
 
-        Uri origin = Uri.parse(originStr);
+        Uri origin = Uri.parse(assertionBundle.getString("origin"));
         BrowserPublicKeyCredentialRequestOptions browserOptions =
             new BrowserPublicKeyCredentialRequestOptions.Builder()
                 .setPublicKeyCredentialRequestOptions(requestOptions)
                 .setOrigin(origin)
                 .build();
 
         Task<Fido2PendingIntent> result = fidoClient.getSignIntent(browserOptions);
 
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/WebAuthnTokenManager.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/WebAuthnTokenManager.java
@@ -58,20 +58,19 @@ public class WebAuthnTokenManager {
 
     public interface WebAuthnMakeCredentialResponse {
         void onSuccess(final byte[] clientDataJson, final byte[] keyHandle,
                        final byte[] attestationObject);
         void onFailure(String errorCode);
     }
 
     @WrapForJNI(calledFrom = "gecko")
-    private static void webAuthnMakeCredential(final String rpId, final GeckoBundle identifiers,
+    private static void webAuthnMakeCredential(final GeckoBundle credentialBundle,
                                                final ByteBuffer userId,
                                                final ByteBuffer challenge,
-                                               final long timeoutMs, final String origin,
                                                final Object[] idList,
                                                final ByteBuffer transportList,
                                                final GeckoBundle authenticatorSelection,
                                                final GeckoBundle extensions) {
         ArrayList<WebAuthnPublicCredential> excludeList;
 
         // TODO: Return a GeckoResult instead, Bug 1550116
 
@@ -105,25 +104,23 @@ public class WebAuthnTokenManager {
             @Override
             public void onFailure(final String errorCode) {
                 webAuthnMakeCredentialReturnError(errorCode);
             }
         };
 
         try {
             final Class<?> cls = Class.forName("org.mozilla.gecko.util.WebAuthnUtils");
-            Class<?>[] argTypes = new Class<?>[] { String.class, GeckoBundle.class,
-                                                   byte[].class, byte[].class, long.class,
-                                                   String.class, WebAuthnPublicCredential[].class,
+            Class<?>[] argTypes = new Class<?>[] { GeckoBundle.class, byte[].class, byte[].class,
+                                                   WebAuthnPublicCredential[].class,
                                                    GeckoBundle.class, GeckoBundle.class,
                                                    WebAuthnMakeCredentialResponse.class };
             Method make = cls.getDeclaredMethod("makeCredential", argTypes);
 
-            make.invoke(null, rpId, identifiers, userBytes, challBytes,
-                        timeoutMs, origin,
+            make.invoke(null, credentialBundle, userBytes, challBytes,
                         excludeList.toArray(new WebAuthnPublicCredential[0]),
                         authenticatorSelection, extensions, handler);
         } catch (Exception e) {
             Log.w(LOGTAG, "Couldn't run WebAuthnUtils", e);
             webAuthnMakeCredentialReturnError("UNKNOWN_ERR");
             return;
         }
     }
@@ -138,20 +135,20 @@ public class WebAuthnTokenManager {
     public interface WebAuthnGetAssertionResponse {
         void onSuccess(final byte[] clientDataJson, final byte[] keyHandle,
                        final byte[] authData, final byte[] signature,
                        final byte[] userHandle);
         void onFailure(String errorCode);
     }
 
     @WrapForJNI(calledFrom = "gecko")
-    private static void webAuthnGetAssertion(final String rpId, final ByteBuffer challenge,
-                                             final long timeoutMs, final String origin,
+    private static void webAuthnGetAssertion(final ByteBuffer challenge,
                                              final Object[] idList,
                                              final ByteBuffer transportList,
+                                             final GeckoBundle assertionBundle,
                                              final GeckoBundle extensions) {
         ArrayList<WebAuthnPublicCredential> allowList;
 
         // TODO: Return a GeckoResult instead, Bug 1550116
 
         if (!GeckoAppShell.isFennec()) {
             Log.w(LOGTAG, "Currently only supported on Fennec");
             webAuthnGetAssertionReturnError("NOT_SUPPORTED_ERR");
@@ -180,25 +177,24 @@ public class WebAuthnTokenManager {
             @Override
             public void onFailure(final String errorCode) {
                 webAuthnGetAssertionReturnError(errorCode);
             }
         };
 
         try {
             final Class<?> cls = Class.forName("org.mozilla.gecko.util.WebAuthnUtils");
-            Class<?>[] argTypes = new Class<?>[] { String.class, byte[].class, long.class,
-                                                   String.class, WebAuthnPublicCredential[].class,
-                                                   GeckoBundle.class,
+            Class<?>[] argTypes = new Class<?>[] { byte[].class, WebAuthnPublicCredential[].class,
+                                                   GeckoBundle.class, GeckoBundle.class,
                                                    WebAuthnGetAssertionResponse.class };
             Method make = cls.getDeclaredMethod("getAssertion", argTypes);
 
-            make.invoke(null, rpId, challBytes, timeoutMs, origin,
+            make.invoke(null, challBytes,
                         allowList.toArray(new WebAuthnPublicCredential[0]),
-                        extensions, handler);
+                        assertionBundle, extensions, handler);
         } catch (Exception e) {
             Log.w(LOGTAG, "Couldn't run WebAuthnUtils", e);
             webAuthnGetAssertionReturnError("UNKNOWN_ERR");
             return;
         }
     }
 
     @WrapForJNI(dispatchTo = "gecko")