Skip to content

WebCrypto's CryptoKey type is incompatible on Dart 2.12.4 #45745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
matehat opened this issue Apr 18, 2021 · 11 comments
Closed

WebCrypto's CryptoKey type is incompatible on Dart 2.12.4 #45745

matehat opened this issue Apr 18, 2021 · 11 comments
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. web-libraries Issues impacting dart:html, etc., libraries

Comments

@matehat
Copy link

matehat commented Apr 18, 2021

Given the simple test:

@JS()
library web_test;

import 'dart:html';
import 'dart:math';
import 'dart:typed_data';

import 'package:js/js.dart';
import 'package:js/js_util.dart';
import 'package:test/test.dart';

@JS('Promise')
class Promise<T> {
  external factory Promise._();
}

@JS('crypto.subtle.importKey')
external Promise<CryptoKey> importKey(
  String format,
  dynamic keyData,
  dynamic algorithm,
  bool extractable,
  List<String> keyUsages,
);

void main() {
  group('A group of tests', () {
    final random = Random.secure();

    test('First Test', () async {
      final rawData = Uint8List.fromList(
          List<int>.generate(32, (index) => random.nextInt(256),),);

      final promise = importKey(
        'raw',
        rawData.buffer,
        'AES-CTR',
        true,
        ['encrypt', 'decrypt'],
      );
      final result = await promiseToFuture<CryptoKey>(promise);
      expect(result, isA<CryptoKey>());
    });
  });
}

The test hangs and fails after 30 seconds. Running the tests with the --pause-after-load flag to obtain a javascript console, we can see the following exception being printed out:

js_helper.dart:1130 Uncaught (in promise) TypeError: Instance of 'CryptoKey': type 'UnknownJavaScriptObject' is not a subtype of type 'FutureOr<CryptoKey>?'
    at Object.wrapException (js_helper.dart:1130)
    at Object._failedAsCheck (rti.dart:1018)
    at Rti._generalNullableAsCheckImplementation [as _as] (rti.dart:1011)
    at Rti._installSpecializedAsCheck (rti.dart:933)
    at promiseToFuture_closure.call$1 (js_util.dart:165)
    at invokeClosure (js_helper.dart:1826)
    at js_helper.dart:1858
  • This happens when the code is compiled with dartdevc and dart2js (unless with -O3 or -O4).

  • With -O3 or -O4, the test runs to completion, but fails the type assertion.

  • Still with any of these flags, if I run the compiled code in in Chrome and set a breakpoint in the Chrome dev console, I can see that the variable is indeed a CryptoKey in Javascript land.

    CleanShot 2021-04-18 at 11 30 48

Environment

  • Dart SDK Version (dart --version): 2.12.4
  • Whether you are using Windows, MacOSX, or Linux (if applicable): MacOSX
  • Whether you are using Chrome, Safari, Firefox, Edge (if applicable): Chrome
@matehat matehat changed the title WebCrypto's CryptoKey type not accessible on Dart 2.12.4 WebCrypto's CryptoKey type is incompatible on Dart 2.12.4 Apr 18, 2021
@athomas athomas added the area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. label Apr 19, 2021
@matehat
Copy link
Author

matehat commented Apr 19, 2021

To add a little bit of weight to that: we're a startup in the healthcare industry doing mobile/web apps for healthcare professionals to securely collaborate about patients. Communications are encrypted end-to-end using different Dart libraries.

Since the null safety migration, we're stuck on the desktop/web front because it's now impossible to do encryption/decryption in a performant matter. On mobile/native land, it's all good though.

@sigmundch sigmundch added the web-libraries Issues impacting dart:html, etc., libraries label Apr 19, 2021
@sigmundch
Copy link
Member

/cc @srujzs

@srujzs
Copy link
Contributor

srujzs commented Apr 22, 2021

I can repro this with the latest 2.14 with dart2js (although not with ddc for some reason on either 2.14 or 2.12.4 as far as I can tell). There's definitely an issue here with the native binding since as you say the result is a CryptoKey and I can still interact with it with js_util.

The symptoms and the error very closely resemble a similar issue: #44324, but the solution of that (adding @Creates annotations so that the type is live) doesn't apply here since we're using JS interop to access an API that returns a CryptoKey.

@sigmundch
Copy link
Member

/cc @rakudrama

Oh - I had thought dart2js conservatively assumed that any JS-interop call could return any native type, including browser native types. However, it appears that by design we decided not to:

/// We assume that JS-interop APIs cannot instantiate Dart types or

That would explain why we have a similar tree-shaking issue here. What's worse here is that it appears that there is no API in dart:html that can return a CryptoKey.

When we switch to our static js-interop approach and CryptoKey is modeled using that, then this wont be an issue anymore. Until then, I wonder what's the best approach to avoid this problem. Some possibilities:

  • treat all native types live as a result of JS-interop, this has a potential to generate a lot of unnecessary bloat
  • fix and expose _SubtleCripto with the importKey getter
  • delete _SubtleCrypto and CryptoKey so that users can declare them via JS-interop (relates to Using a browser API that returns Promise #44664)
  • warn about these cases and show workarounds (e.g. remove types and use js_util to invoke its members).

On a separate note, once we fix the liveness issue, it may be worth noting that the example above is typing the result of the API as Promise<CryptoKey>, but should probably be just Promise or Promise<dynamic>. The general problem (which eventually we'd like to disallow with a compiler error) is that JS-interop types shouldn't be considered generic. That's because those types are created outside the domain of Dart, so they don't contain the type information we need to properly track type parameters.

@srujzs
Copy link
Contributor

srujzs commented Apr 22, 2021

I tried this with a validation check as well where I made SubtleCrypto public, added a different member of SubtleCrypto e.g. generateKey which then generates a CryptoKey and lets dart2js know with @Creates. I make a call to it before any of the above which then resolves the liveness issue.

For example:

@Native("SubtleCrypto")
abstract class SubtleCrypto extends Interceptor {
  // To suppress missing implicit constructor warnings.
  factory SubtleCrypto._() {
    throw new UnsupportedError("Not supported");
  }

  @JSName("generateKey")
  @Creates("CryptoKey")
  dynamic _generateKey(Object algorithm, bool extractable, List<String> keyUsages) native;

  Future<CryptoKey> generateKey(Map algorithm, bool extractable, List<String> keyUsages) {
    var jsAlgorithm = convertDartToNative_Dictionary(algorithm);
    return promiseToFuture<CryptoKey>(_generateKey(jsAlgorithm, extractable, keyUsages));
  }
}

The following gets placed in main above the provided repro:

  var tempKey = await window.crypto!.subtle!.generateKey({
      'name': "ECDSA",
      'namedCurve': "P-384"
    }, true, ["sign"]);

I think really we can do maybe one thing simpler in that if dart2js sees a Native type being used at all in the program, it's considered potentially live. For example, if there exists a cast to CryptoKey e.g. as CryptoKey, I would expect dart2js to assume that a CryptoKey can be returned and consider it live. I don't know how this maps into dart2js (and my take on this might be naive) but this makes JS interop workarounds much easier since it doesn't require users to continuously stick with js_util even with objects that are clearly Native.

@srujzs
Copy link
Contributor

srujzs commented Apr 22, 2021

We chatted a bit about what we can do here and trying my best to summarize:

  • In the case where we want to make every Native live when JS interop is involved, this might end up greatly bloating the code size and reduce optimization possibilities.
  • In the case where we want to say a type is live based on usage (cast or otherwise), this might be more complicated and there may be ramifications due to the way interceptors are implemented. More investigation would be needed.

This does present a small case study though in which it might make sense to remove this from dart:html and say we don't plan on adding it back (avoiding a breaking change), but this will still require users to use JS interop to declare the type. Similarly, it provides an example of an API that might make sense to move to a separate library when we have a JS interop using static extension types ready.

As a workaround for now and for your purposes, Mathieu, I would suggest using an @anonymous JS interop type to interface CryptoKey instead of the Native type e.g.

@JS()
@anonymous
class JSCryptoKey {
  external String? get type;
}

and on usage:

  final promise = importKey(
    'raw',
    rawData.buffer,
    'AES-CTR',
    true,
    ['encrypt', 'decrypt'],
  );
  var future = promiseToFuture(promise);
  JSCryptoKey key = await future;
  print(key.type!);

Note that this will not work without @anonymous, since there will be a name collision. It's not ideal since it requires a separate type, but probably better than dealing with js_util every time you want to use the key. Also note that the generics are omitted as the type parameters do not get reified, as Siggi has mentioned above.

Edit: On further inspection, the workaround only works because the type is not live. It might not be an issue in dart2js because CryptoKey can never be live from the way our API is written. However, on ddc, the above workaround fails because native types are always live by default.

Which is frustrating because that means the only real workaround here is js_util:

  var key = await future;
  print(js_util.getProperty(key, 'type'));

@matehat
Copy link
Author

matehat commented Apr 23, 2021

Thanks a lot for your investigations and suggestions @srujzs and @sigmundch !

@Pacane
Copy link

Pacane commented Nov 4, 2022

cc: @kevmoo

This issue was opened by our team. It blocked us from using JS interop to get decent web crypto. We ended up using this which is not ideal.

Turns out it's fork of a fork of an unofficial google package. (Forked because the conditional imports/SDK dependencies aren't done properly to use it in a pure web environment. Forcing us into using Flutter for web, even for a very simple page that shouldn't need flutter).

@kevmoo
Copy link
Member

kevmoo commented Nov 4, 2022

I just ran this locally (with 2.19.0-361.0.dev (dev) (Tue Nov 1 15:30:40 2022 -0700) on "macos_arm64") and it worked fine.

I'd look at https://github.com/google/webcrypto.dart if you want this functionality!

@Pacane
Copy link

Pacane commented Nov 4, 2022

I'd look at https://github.com/google/webcrypto.dart if you want this functionality!

Yes! we've looked for that, but as I mentioned, I had to fork it to remove all the flutter dependencies for one of our non-flutter web projects. Not ideal, but it works.

@kevmoo
Copy link
Member

kevmoo commented Nov 4, 2022

See also: #50290

I ran this on latest dev and it works. Closing out...

@kevmoo kevmoo closed this as completed Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. web-libraries Issues impacting dart:html, etc., libraries
Projects
None yet
Development

No branches or pull requests

6 participants