Skip to content

intent to implement: dart:crypto #50290

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
devoncarew opened this issue Oct 24, 2022 · 13 comments
Closed

intent to implement: dart:crypto #50290

devoncarew opened this issue Oct 24, 2022 · 13 comments
Assignees
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). library-core

Comments

@devoncarew
Copy link
Member

devoncarew commented Oct 24, 2022

This is an issue to raise awareness that we're considering adding a new library to the Dart SDK - dart:crypto. This would be to provide basic cryptographic APIs to Dart. Some details:

  • we would not implement the cryptographic algorithms ourselves but would instead wrap existing implementations
  • on the web we'd wrap the Web Crypto API; https://developer.mozilla.org/en-US/docs/Web/API/Crypto
  • on other platforms we'd wrap BoringSSL (currently shipped as part of the Dart VM in order to implement SSL)

For a preview of what the Dart API might look like, see https://github.com/google/webcrypto.dart.

Are you certain you'll ship this?

No, but we're actively considering it. We'll keep this issue updated as plans firm up.

What's the timeframe for shipping it?

We don't have a concrete timeframe; it likely wouldn't happen sooner then ~6 months from now.

Have you considered shipping it as a package instead of as a dart: library?

Yes. Shipping it as a package is currently challenging as we'd need to ship native code with the package, and, we'd need to coordinate the version of BoringSSL used by the package with the version already shipped in the VM. Additionally, crypto is likely a fundamental enough thing to a platform that it's worth having it as a dart: library.

@devoncarew devoncarew added library-core area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). labels Oct 24, 2022
@a-siva
Copy link
Contributor

a-siva commented Oct 24, 2022

As discussed in the presentation done on dart:crypto we need to also account for the case that boringssl might not be linked in with the VM or Flutter engine when cupertino_http or cronet_http packages are used in an application. We would need an architecture that enables plugging in a different implementation similar to how we now have pluggablity for the http stack.

@bivens-dev
Copy link

bivens-dev commented Oct 24, 2022

Just out of curiosity here but would there be any consideration towards wrapping something like Tink as a way to surface

  1. Known good crypto primitives that are vetted by experts
  2. Exposing them in such a way that they are designed to be difficult to misuse

AFAIK Tink is actually built on top of BoringSSL itself but brings that added safety and development experience to the problem working with crypto primitives which as they mention in their readme can sometimes leave developers feel like they are juggling chainsaws 😂

@devoncarew
Copy link
Member Author

As discussed in the presentation done on dart:crypto we need to also account for the case that boringssl might not be linked in with the VM or Flutter engine when cupertino_http or cronet_http packages are used in an application. We would need an architecture that enables plugging in a different implementation similar to how we now have pluggablity for the http stack.

I don't think we'll end up with a pluggable implementation (though I do remember that being discussed). I think that we'll rely on eventual tree-shaking for libraries using native code via FFI; so, if user code did not end up using the dart:io implementation of SSL (they either weren't using it or had plugged in their own networking stack), and were not using dart:crypto, then BoringSSL could be tree-shaken away. cc @dcharkes

@jonasfj
Copy link
Member

jonasfj commented Oct 24, 2022

I think high-level abstractions like tink might be better provided by a package on pub.dev; if we only offered such APIs I think it would limit the compatibility with external/older systems. And such libraries can hopefully be written on top of dart:crypto.

The proposal here is to offer the primitives available to JS in browsers through window.crypto.
It's still unclear if we should ship the keyUsages/exportable capability logic and AES-KW from web cryptography. That's not part of the current prototype. Feel free to provide feedback on the repository: https://github.com/google/webcrypto.dart

The benefit is that we get a single library that works across all platforms (including web and native).

@a-siva
Copy link
Contributor

a-siva commented Oct 24, 2022

I don't think we'll end up with a pluggable implementation (though I do remember that being discussed). I think that we'll rely on eventual tree-shaking for libraries using native code via FFI; so, if user code did not end up using the dart:io implementation of SSL (they either weren't using it or had plugged in their own networking stack), and were not using dart:crypto, then BoringSSL could be tree-shaken away. cc @dcharkes

This brings in a new dependency of boringssl in the VM through dart:crypto and I wanted to avoid that. Application size is a very big factor for flutter apps and we are constantly looking for ways to reduce this size and by adding this dependency we would be increasing the size (especially in cases where the native implementation is being used).

@dcharkes
Copy link
Contributor

I think that we'll rely on eventual tree-shaking for libraries using native code via FFI; so, if user code did not end up using the dart:io implementation of SSL (they either weren't using it or had plugged in their own networking stack), and were not using dart:crypto, then BoringSSL could be tree-shaken away. cc @dcharkes

That would be ideal indeed. Please note that tree-shaking away native code comes in various flavours with various engineering challenges:

  1. A dynamic library bound to in a package: library is not referred at all, we don't include it. Should be relatively easy to implement with the work that would build on top [vm/ffi] Native asset resolution by the Dart VM for FfiNatives #49803 for supporting native libaries in dart run/dart compile/flutter build.
  2. A static library bound to in a package: library and only a subset symbols are used. We can use the native linker for the linking step which would remove unreachable native code. This will only work with assembly snapshots, because our elf snapshots are already a dynamic library which is too late in the native compilation pipeline to link static libraries. This means both a native linker and native compiler have to be available on the users' system (we could ship it with the SDK). Currently, we rely heavily on the ELF snapshots for various OSes. So we would either need to figure out how to generate ELF static libraries instead of ELF dynamic libraries (af93ebc) in the VMs ELF-writer, or make assembly snapshots work on all our backends.
  3. A static library bound to in a dart: library. This requires all logic of (2), but in addition that we change our dartaotruntime to a static library rather than an executable and run the native compiler to link an assembly-aot-snapshot with the dartaotruntime. Then the native linker would remove all native code in the dartaotruntime that is not referred.

cc @mraleph @mkustermann

If we would be able to get rid of boringssl in the VM and only have it through dart: packages, tree-shaking would be easier with approach (1). However, if boringssl is in the dartaotruntime, we include it twice in an app.

(2) and (3) are a lot more engineering effort, and require getting alignment on shipping/requiring a native compiler and linker. @mit-mit

At least (2) is where I would want to end up with the Dart eco system and native code. I'm not sure yet about (3).

I do not know if (3) would make signing harder or easier. cc @sstrickl

@mkustermann
Copy link
Member

We are generally moving into a world where packages will come with native code and we have to make that use case as easy as possible. If there are hurdles for package:webcrypto we should try to identify them and come up with solutions - because users may run into the same issues (and they cannot add dart:* libraries).

The one thing that is currently not possible - as mentioned above - is to re-use the boringSSL that may be available in the Dart VM / Flutter engine - so there may be an opportunity to save some size, which bears the question: How much code size are we talking about here?

Looking at

  • flutter engine (which includes dart and boringssl): it contains around 350 KB of code for boringssl (see binary size analysis dump in third_party/boringssl)
  • example app in package:webcrypto: the resulting APK has 1.2 MB in it (1204272 ... lib/arm64-v8a/libwebcrypto.so)

=> Blindly adding all of what is in libwebcrypto.so to the flutter engine / dart vm would significantly regress size.
=> If one would tree-shake libwebcrypto.so to contain only the parts that are needed by dart code using package:webcrypto - it may be quite small - making the need to share with the flutter/dart-vm's version not as important.

@jonasfj
Copy link
Member

jonasfj commented Oct 25, 2022

@mkustermann, I'm actually not sure why libwebcrypto.so probably because just getting it to build was hard enough 🙈

Logic in package:webcrypto is implemented using dart:ffi which access a table of symbols from BoringSSL.
I did an ugly hack where I ensured those symbols retained in the Dart SDK and build the counter app, wich increased app-release.apk size by 2.3kb (see dart-crypto-size.patch, it only aims to retain symbols necessary).

Of course, it's possible I've measured it wrong, or that we'll want other features / tweaks that increase size; in which case we'll have to weigh the pros / cons.

@mkustermann
Copy link
Member

As a side note: The following change allows shrinking the 1 MB libwebcrypto.so:

diff --git a/android/CMakeLists.txt b/android/CMakeLists.txt
index 7c8e79b..9768805 100644
--- a/android/CMakeLists.txt
+++ b/android/CMakeLists.txt
@@ -14,6 +14,10 @@
 
 cmake_minimum_required(VERSION 3.6.0)
 
+SET(CMAKE_C_FLAGS  "${CMAKE_CXX_FLAGS} -flto")
+SET(CMAKE_CXX_FLAGS  "${CMAKE_CXX_FLAGS} -flto")
+SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -flto")
+
 enable_language(ASM)
 
 # Set as required by android-sources.cmake included below
@@ -40,7 +44,8 @@ add_definitions(-DOPENSSL_SMALL)
 if(ANDROID_ABI STREQUAL "armeabi-v7a")
   set(crypto_sources_ARCH ${crypto_sources_linux_arm})
 elseif ( ANDROID_ABI STREQUAL "arm64-v8a")
-  set(crypto_sources_ARCH ${crypto_sources_linux_aarch64})
+  set(crypto_sources_ARCH)
+  add_definitions(-DOPENSSL_NO_ASM)
 elseif ( ANDROID_ABI STREQUAL "x86")
   set(crypto_sources_ARCH ${crypto_sources_linux_x86})
 elseif ( ANDROID_ABI STREQUAL "x86_64")

it will produce

% unzip -l build/app/outputs/flutter-apk/app-release.apk | grep '[-]v8a/libwebcrypto.so' 
   399224  1981-01-01 01:01   lib/arm64-v8a/libwebcrypto.so

=> ~ 400 kb

When clearing out all symbols referenced (in src/symbols.generated.c), it will produce

% unzip -l build/app/outputs/flutter-apk/app-release.apk | grep '[-]v8a/libwebcrypto.so'
     9920  1981-01-01 01:01   lib/arm64-v8a/libwebcrypto.so

=> ~ 10 kb

So with this simple experiment it seems the base overhead is 10 kb plus any C routines that are actually needed for the Dart code (pay as you go).

Another orthogonal question: I can imagine there's useful crypto algorithms one would expect in a package:crypto that are not needed for TLS connections and may therefore not even be part of BoringSSL. Does the package:webcrypto have additional C libraries it uses for such?

@dcharkes
Copy link
Contributor

When clearing out all symbols referenced (in src/symbols.generated.c)

As discussed with @mkustermann offline, @jonasfj we will need to make the symbols visible and bind to them with @FfiNatives in order for tree-shaking (2) to work. The current indirection in package:webcrypto to avoid name collisions will not support tree-shaking because of the dynamic binding.

@jonasfj
Copy link
Member

jonasfj commented Oct 25, 2022

At least (2) is where I would want to end up with the Dart eco system and native code.

Agreed, this would be lovely.

And shipping crypto as a package would work reasonably, it's a bit unfortunate that apps would include parts of BoringSSL twice, but if we treeshake the parts not used it might be fine.

@mkustermann => ~ 400 kb

Nice, I didn't get -flto to eat much, but digging through logs it was probably because my ndk is too broken to strip symbols 🙈

@arolan
Copy link

arolan commented Oct 25, 2024

hello, we are wondering if you have updates on crypto library for dart? do you plan to add it soon? thank you in advance!

@jonasfj
Copy link
Member

jonasfj commented Oct 29, 2024

I think we were unable to reach consensus for a dart:crypto library. There is a lot of reasons to prefer that libraries are built as packages instead. It makes it a lot easier to evolve the API.

And while shipping packages with native binaries is a bit hard today, work is progressing on support for native assets.

In the mean time the API I wanted to propose for a crypto library is available as a Flutter plugin called package:webcrypto on pub.dev.
It's still under active development, the API is fairly stable and it works across most platforms.


I'm closing this issue, as I don't think we're going to implement a dart:crypto library.

@jonasfj jonasfj closed this as completed Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). library-core
Projects
None yet
Development

No branches or pull requests

7 participants