atsign-foundation / at_client_sdk

The Dart implementation of atSDK used for implementing Atsign's technology into other software
https://pub.dev/publishers/atsign.org/packages
BSD 3-Clause "New" or "Revised" License
1.46k stars 31 forks source link

Invalid Or Corrupted Pad Block #474

Closed kmahon99 closed 2 years ago

kmahon99 commented 2 years ago

Describe the bug Adding an base 64 encoded image entry to the local secondary seems to work, but the data that is being returned when I get the image entry is throwing an AT0023: "Invalid argument(s): Invalid or corrupted pad block" error.

To Reproduce Steps to reproduce the behavior:

  1. Create an AtKey: var key = AtKey()
  2. Assign a Uuid to the key name: atKey.key = const Uuid().v1();
  3. Set the shared properties to only be shared with my current atSign: atKey.sharedBy = await clientSDKInstance.getAtSign(); atKey.sharedWith = await clientSDKInstance.getAtSign();
  4. Upload the image: await ClientSdkService.getInstance().put(atKey, base64Encode(media));
  5. Fetch all keys in the client, the client will list a set of keys that do indeed include the image's Uuid: var imageKey = await AtClientManager.getInstance().atClient.getAtKeys();
  6. Attempting to get the associated data throws the above error: var data = await AtClientManager.getInstance().atClient.get(imageKey);

Expected behavior The image data is fetched and returned as base 64, which can be decoded and shown in the app.

Screenshots

Smartphone (please complete the following information):

Were you using an @‎application when the bug was found? Custom app created by me.

Additional context I used to be able to fetch the image, but there was an issue where the secondary sync was not working. Now the sync seems to work, but the client can't get the data.

Logs: https://cdn.sourceb.in/bins/qgmGMq0LT7/0 https://cdn.sourceb.in/bins/P1HCSyXtHd/0

(Note that the pin exception is part of a separate package and has nothing to do with this issue)

gkc commented 2 years ago

Thanks @kmahon99 we will tackle this issue in our next sprint which starts today.

yahu1031 commented 2 years ago

@kmahon99 I see server timed out and socket exceptions in mine(custom app) as well. But not the corrupted pad block issue. May be specific to atsign?

Here are logs for future investigation.

Issues found following these steps.

Required info

# pubspec.yaml

dependencies:
  at_client_mobile: ^3.1.13
  at_onboarding_flutter: ^4.0.1
  cupertino_icons: ^1.0.4
  dotted_border: ^2.0.0+2
  file_picker: ^4.5.1
  flutter:
    sdk: flutter
  flutter_launcher_icons: ^0.9.2
  flutter_local_notifications: ^9.4.0
  flutter_qr_reader: ^1.0.5
  freezed_annotation: ^1.1.0
  intl: ^0.17.0
  json_annotation: ^4.4.0
  local_auth: ^1.1.11
  lottie: ^1.2.2
  package_info_plus: ^1.4.2
  page_transition: ^2.0.5
  path_provider: ^2.0.9
  permission_handler: ^9.2.0
  provider: ^6.0.2
  secure_application: ^3.8.0
  share_plus: ^4.0.4
  shared_preferences: ^2.0.13
  tabler_icons: ^0.0.3
  uuid: ^3.0.6

dependency_overrides:
  at_lookup: ^3.0.14
  at_commons: ^3.0.13
  at_client:
    git:
      url: https://github.com/atsign-foundation/at_client_sdk.git
      ref: trunk
      path: at_client

dev_dependencies:
  build_runner: ^2.1.8
  flutter_lints: ^1.0.4
  freezed: ^1.1.1
  import_sorter: ^4.6.0
  json_serializable: ^6.1.5
yahu1031 commented 2 years ago

@kmahon99 I would like to confirm a few things.

  1. From point 3(reproducible steps), It looks like both sharedWith and sharedBy values are the same which means it must be a self key.
  2. Just FYI there is a property called isBinary in metadata, If you pass true values, it will handle the Base64.encode() internally.
  3. From point 5(reproducible steps), getAtKeys() will return List<Atkeys>. Looks like you are passing the whole List<AtKeys> to get() method where it returns AtValue.

I would like you to make sure you are doing it correctly and Would like to request you to share a sample code block if you are doing it correctly.

gkc commented 2 years ago

Hi Kevin

I've spent some time looking into this as well, but haven't been able to reproduce it thus far.

Here's the logic trail I'm using to try to narrow down where the problem might be

I've tried various permutations on your code snippets, but I can't make either of those conditions happen

A couple of more questions / requests for you

  1. Are you able to reproduce this easily? If so, the fastest way forward will likely be to get on a video conference and talk us through it; if that's not possible for you right now then please can you share a little bit more code, because I am failing to cause the problem to happen
  2. What version of at_client package were you using?
  3. Are you doing anything in particular with Metadata?
  4. Were there any 'interesting' lifecycle events leading up to this? e.g. on-boardings, use of multiple at-signs, resetting of at-signs, etc ? Or anything else unusual you can think of?

One wild idea worth mentioning: There have been a lot of bug fixes and improvements in the client SDK over the past few months. Given the code snippet you provided, it might be that, when you originally 'put' the data, the data was stored unencrypted (wrongly), because you hadn't explicitly set a Metadata with isEncrypted=true ... let me know if you think that might possibly be what's happened and I will try to reproduce it (have the key/value pair created using an at_client 3.0.8 or lower; then try to read it back using a more recent at_client)

Cheers

Gary

PS Apologies for not starting work on this issue sooner; somehow none of us noticed it until it came to sprint planning!

kmahon99 commented 2 years ago

Sorry guys, didn't notice that this had been picked up. To answer your questions:

@yahu1031

  1. Yes this is a self key, I'm only sharing it with myself to perform a simple test of encrypting an image and then retrieving, decrypting and displaying the image in another widget.
  2. I'm not sure what you mean? I do a base64 encode because the clientSDKinstance.put() method will only accept strings as values, I can't pass the binary image itself into that method, or is there a way to do that?
  3. For clarification: I get all keys and then iterate through the list of keys where each one is passed in individually to the get() method, which I expect to retrieve the data.

@gkc

  1. This happens every time I try to upload an image, I can't get this to work at all.
  2. I'm using 3.0.15
  3. No I'm just specifying that the data should be encrypted and non-public.
  4. We had issues with onboarding before, I had to remove an old at key and reinstall the app due to bad key formatting that was causing lookup issues, but it's a clean install now which isn't giving any issues that I can see. The only thing is that onboarding seems to be retrieving the key twice for some reason.

I didn't realize that the data wouldn't be encrypted if I didn't explicitly specify it, I didn't have it initially but now I do and I'm still getting the same result unfortunately.

yahu1031 commented 2 years ago

@kmahon99 What I mean by encoding to base64 is, If you set metadata of an AtKey to isBinary: true, You don't actually need to do encoding from your code block. Where atClientInstance.put(AtKey key, dynamic value), you can just pass the bytes there in the place of value. You can refer to the internal code of put request transformation here.

kmahon99 commented 2 years ago

Ah ok, I was using the wrong put method, but I'm still getting the same issue with the pad block.

yahu1031 commented 2 years ago

Looks like you wrote your own class and created a put method there. BTW can you please check with the latest versions of at_client and at_client_mobile and update us.

kmahon99 commented 2 years ago

I've updated at_client and at_client_mobile, but now I've got the old kotlin issue with biometric storage: `Execution failed for task ':biometric_storage:kaptDebugKotlin'.

A failure occurred while executing org.jetbrains.kotlin.gradle.internal.KaptWithoutKotlincTask$KaptExecutionWorkAction java.lang.reflect.InvocationTargetException (no error message)`

yahu1031 commented 2 years ago

Use 1.5.21 kotlin version. That may resolve it. @kmahon99

kmahon99 commented 2 years ago

Ok, I've fixed the Kotlin issue but I'm still getting the original pad block error when uploading.

yahu1031 commented 2 years ago

While uploading? We can get on a call if you are sure it is while uploading data.

gkc commented 2 years ago

@kmahon99 two quick questions:

  1. What size is the image you are uploading?
  2. What is the atSign you are using?

Thanks

Gary

gkc commented 2 years ago

PS Have you tried deleting it, waiting for sync, and then re-creating it?

gkc commented 2 years ago

FYI Here's the code I'm using to try to recreate your issue, @kmahon99

  List<int> media = utf8.encode("The quick brown fox jumps over the lazy dog");
  var mediaDigest = sha1.convert(media);

  var metadata = Metadata()..isPublic = false..isEncrypted = true;
  var atKey = AtKey()..key = 'media'..namespace='issue474'..sharedBy = atSign..sharedWith = atSign;
  // Upload the image:
  print('media digest: $mediaDigest');
  var mediaBase64 = base64Encode(media);
  print('put encoded as base64: $mediaBase64');
  await atClient.put(atKey, mediaBase64);

  var valueReadBack, mediaReadBack, mediaReadBackDigest;
  // Read the data back
  valueReadBack = await atClient.get(AtKey()..key = 'media'..namespace='issue474'..sharedBy = atSign..sharedWith = atSign);
  print('get result (base64) : ${valueReadBack.value}');
  mediaReadBack = base64Decode(valueReadBack.value);
  mediaReadBackDigest = sha1.convert(mediaReadBack);
  print('media read back digest: $mediaReadBackDigest');

  atMgr.syncService.sync();
  await Future.delayed(Duration(seconds:30));

  // Read the data back again
  valueReadBack = await atClient.get(AtKey()..key = 'media'..namespace='issue474'..sharedBy = atSign..sharedWith = atSign);
  print('get result (base64) : ${valueReadBack.value}');
  mediaReadBack = base64Decode(valueReadBack.value);
  mediaReadBackDigest = sha1.convert(mediaReadBack);
  print('media read back digest: $mediaReadBackDigest');

... but I've failed to reproduce the issue so far.

yahu1031 commented 2 years ago

@kmahon99 Also could you please reconfirm that your code uses the latest version of at_client by checking up the imports.

  1. Go to any file where you import at_client/at_client_mobile.
  2. Ctrl/Cmd + click on the import
  3. And you will find the path of the file on the top bar of the code. There you will see the version.
  4. Please update the at_client and at_client_mobile versions.
yahu1031 commented 2 years ago

I have been debugging the at_dude app with @assault30 atsign and found some keys have this issue. And here are the logs of the app.

LOGS LOGS with sharedBy details

NOTE: On every SEVERE of this error, There is the key above it.

kmahon99 commented 2 years ago

@gkc I've managed to successfully retrieve the data and have identified the offending line of code.

The issue isn't with uploading the data, it's with retrieving the data, if I do this: `var items = (await AtClientManager.getInstance().atClient.getAtKeys()).where((element) => element.key!.startsWith(AuthenticationConsts.ATAPP_STORAGE_IMAGE_KEY));

  for (var item in items){

    var valueReadBack = await AtClientManager.getInstance().atClient.get(AtKey()
      ..key = item.key
      ..namespace=AuthenticationConsts.ATAPP_NAMESPACE
      ..sharedBy = await P2PUtilities.GetCurrentAtSign()
      ..sharedWith = await P2PUtilities.GetCurrentAtSign());
    userImages.add({item: base64.decode(valueReadBack.value)});
  }

the data can be retrieved, but if I do this: `var items = (await AtClientManager.getInstance().atClient.getAtKeys()).where((element) => element.key!.startsWith(AuthenticationConsts.ATAPP_STORAGE_IMAGE_KEY));

  for (var item in items){

    var valueReadBack = await AtClientManager.getInstance().atClient.get(item);
    userImages.add({item: base64.decode(valueReadBack.value)});
  }

I get a corrupted pad block error. I'm not sure why I can't pass the actual key into the get() function when I want to get its corresponding value? That would be the logical approach to retrieving data, I don't see why I have to create a brand new key and copy the data accross in order to do the lookup.

I'm also still having issues persisting the data accross sessions, once I upload the data I can retrieve it, but if I restart the app it's gone again (though I'll need to look into this further, I suspect that has something to do with my code as the secondary sync works fine).

gkc commented 2 years ago

@kmahon99 thank you for tracking down the issue!!! We will get that fixed pronto

yahu1031 commented 2 years ago

@kmahon99 Yes, For some reason, we cannot pass the atKey we got from the server directly to the get() method. That will throw some errors. Instead, you can just create a new object of AtKey and write down the key and namespace to that new object and pass it to the get() method. Ideally passing the atkey you fetched to the get method must work.

yahu1031 commented 2 years ago

@kmahon99 Can I know what is the exact key you are getting issues with?

yahu1031 commented 2 years ago

BTW here is what I have tried depending on this comment

/// Fetches the master image key from secondary.
static Future<void> getMasterImage() async {
  _logger.finer('Getting master image');
  List<AtKey> _keys = await sdkServices.atClientManager.atClient
      .getAtKeys(regex: Keys.masterImgKey.key);
  try {
    for (AtKey _key in _keys) {
      print(_key.toString());
      AtValue value = await sdkServices.atClientManager.atClient.get(_key);
      _userData.masterImage = Uint8List.fromList(
          Base2e15.decode(json.decode(value.value)['value']));
    }
    _logger.finer('Fetched master image successfully');
  } on Exception catch (e, s) {
    _logger.severe('Error getting master image', e, s);
    return;
  }
}

This is my output

flutter: FINER|2022-05-23 14:34:23.978624|AppServices|Getting master image
flutter: @mangotangostable:masterpassimg.passman@mangotangostable
flutter: FINER|2022-05-23 14:34:24.098464|UserData|Setting current master image

Please ping us on any of the social media resources. So that we can together debug your issue. If things are working fine, them please let us know and we can close if that fixes.

yahu1031 commented 2 years ago

@kmahon99 Can you please share your resolved steps that you have considered this to close.