Page MenuHomePhabricator

[native] Implement Android encryption for Rust
ClosedPublic

Authored by michal on Nov 21 2023, 5:33 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 27, 9:45 AM
Unknown Object (File)
Fri, Dec 27, 9:45 AM
Unknown Object (File)
Fri, Dec 27, 9:45 AM
Unknown Object (File)
Fri, Dec 27, 9:43 AM
Unknown Object (File)
Fri, Dec 27, 9:33 AM
Unknown Object (File)
Wed, Dec 25, 6:09 AM
Unknown Object (File)
Fri, Dec 20, 10:45 PM
Unknown Object (File)
Thu, Dec 19, 6:54 PM
Subscribers

Details

Summary

ENG-5343

Android native implementation of Rust encryption functions. Uses the previosly refactored ByteBuffers.

Depends on D9932, D9938

Test Plan

Run this code on a physical Android device:

fn test(promise_id: u32) {
  RUNTIME.spawn(async move {
    let f = move || -> Result<String, cxx::Exception> {
      let key = &mut [0; constants::aes::KEY_SIZE];
      ffi::generate_key(key)?;
      let plaintext = &mut [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
      let sealed_data =
        &mut [0; 10 + constants::aes::IV_LENGTH + constants::aes::TAG_LENGTH];
      ffi::encrypt(key, plaintext, sealed_data)?;
      let plaintext2 = &mut [0; 10];
      ffi::decrypt(key, sealed_data, plaintext2)?;

      Ok(format!(
        "
    Key: {key:?}
    Plaintext: {plaintext:?}
    Sealed Data: {sealed_data:?}
    Plaintext2: {plaintext2:?}"
      ))
    };
    handle_string_result_as_callback(f(), promise_id);
  });
}

Example output:

Key: [170, 59, 114, 153, 221, 12, 179, 127, 37, 56, 196, 135, 93, 180, 20, 162, 69, 11, 123, 65, 31, 19, 243, 182, 150, 75, 217, 232, 232, 126, 235, 77]
Plaintext: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
Sealed Data: [218, 85, 237, 92, 79, 91, 84, 15, 186, 180, 54, 92, 96, 95, 159, 4, 78, 248, 189, 252, 105, 64, 179, 135, 129, 155, 34, 250, 13, 151, 116, 157, 98, 122, 98, 234, 77, 16]
Plaintext2: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Looks good, I'm wondering if this can be done a little bit better.
We have

class AESCryptoModuleCompat {
  // non-static logic here
  
   companion object {
      // the same logic duplicated, but static
   }
}

Maybe we can make the non-static methods internally call the companion a.k.a static methods? Doesn't matter much though, they're mostly one-liners anyway

This revision is now accepted and ready to land.Nov 22 2023, 7:48 AM
marcin added inline comments.
native/android/app/src/cpp/AESCrypto.cpp
24 ↗(On Diff #33455)

Are there any risk associated with the fact that those methods are:

  • not safe to call from any thread
  • public

at the same time?

Rebase.

An issue with with non-static methods calling the static methods is that the ByteBuffer -> SecretKeySpec conversion must go through ByteArray (with a copy). So that means that we would have ByteArray -no-copy-> ByteBuffer -with-copy-> ByteArray -> SecretKeySpec. As the static methods are very minimal and most of the code is in encryptAES/ decryptAES functions I don't it's worth it.

native/android/app/src/cpp/AESCrypto.cpp
24 ↗(On Diff #33455)

By "not safe to call from any thread" do you mean that they aren't wrapped in NativeAndroidAccessProvider::runTask? This is already a pattern we use for CommSecureStore and PlatformSpecificTools