bumptech / glide

An image loading and caching library for Android focused on smooth scrolling
https://bumptech.github.io/glide/
Other
34.62k stars 6.12k forks source link

Image encryption in DiskCache #735

Closed anotherdev closed 8 years ago

anotherdev commented 8 years ago

I want to encrypt image files that are stored in Glide DiskCache using Facebook Conceal library. What would be the best approach?

TWiStErRob commented 8 years ago

I think you'll need to create an encoder and cacheDecoder to wrap the built-in ones.

It is kind of like having a custom image format, see #718. The only difference is that this format only exists in cache and it won't be coming from external sources.

anotherdev commented 8 years ago

Thanks for quick answer! I'll try to implement it.

Quick follow-up question: I'm migrating from Fresco (because of image encryption feature). Fresco clears the memory cache when app is backgrounded, does Glide support this also? If not, is there a way to do it?

TWiStErRob commented 8 years ago

There's a Glide.clearMemory() method but, it's quite pointless to do so IMO. The JVM running your application won't be terminated when in background, so the heap consumption for the app (OS-level) should stay the same regardless of having no data in it or full of potentially useful bitmaps. If the OS needs space it'll call onLowMemory where Glide should automatically call that method.

It sounds like when resuming a Fresco-based app where onResume reloads all ImageViews (e.g. by reloading the data from DB) takes a long time to actually see images. NB I didn't try Fresco yet.

anotherdev commented 8 years ago

Yeah, I agree. Clearing memory thing is more on security side (a client request). We have to do it this way or else we must disable memory cache.

anotherdev commented 8 years ago

The encoder part seem to be straightforward.

I'm doing the decoder part. The starting point is GifBitmapWrapperStreamResourceDecoder and GifBitmapWrapperResourceDecoder. Which class & config should I use for the following?

  1. ResourceDecoder<ImageVideoWrapper, Bitmap>
  2. ResourceDecoder<InputStream, GifDrawable>
TWiStErRob commented 8 years ago

Sorry, but I don't understand your question fully, the answer may be:

  1. ImageVideoBitmapDecoder
  2. GifResourceDecoder

by looking at implementations of the ResourceDecoder interface.

TWiStErRob commented 8 years ago

To make things easier you might want to try .asBitmap() which disables animated GIF support. But if that's needed, then your best bet is to just try to match the correct generic arguments.

sjudd commented 8 years ago

You might actually be able to do this more easily in a DiskCache implementation. That way you can just encrypt all data as it goes in and comes out.

TWiStErRob commented 8 years ago

@sjudd how can you crypt all data flowing through a DiskCache on the fly if the interface defines only these two methods?

File get(Key key);
void put(Key key, Writer writer);

This was my first idea too, but I dismissed it quickly. I can imagine wrapping the writer and re-writing the file in-place, but you would have to copy the file on access to decode it to a temporary file (which kind of defeats the encryption) and you won't have a way to know when that temp file is not needed any more. What am I missing? As I see the DiskCache interface could be re-written to handle Input/Output streams more naturally in get/put so it is easier to wrap and implement them.

anotherdev commented 8 years ago

@TWiStErRob, thanks, asBitmap() simplify things. Is this correct way?

.asBitmap()
.cacheDecoder(
    new FileToStreamDecoder<>(
        new MyStreamToStreamDecryptor(
            new StreamBitmapDecoder(ctx))))
.into(iv);
TWiStErRob commented 8 years ago

Yeah, that looks good, but you still need to encode it somewhere.

It's easy to know if you're doing it right: it compiles and loads images, and the cache doesn't contain raw readable images.

anotherdev commented 8 years ago

Thanks for confirmation! For encoding I plan to basically copy BitmapEncoder and wrap the OutputStream with a CipherOutputStream in encode method. Is it ok this way?

TWiStErRob commented 8 years ago

You don't need to copy the whole class, just wrap it like you did with the decoder:

return bitmapEncoder.encode(resource, new CypherOutputStream(os));
anotherdev commented 8 years ago

Good call. 2 questions:

  1. I just implemented the encoder (SecureBitmapEncoder), but haven't impl. the decoder yet. So I expected that I could encrypt but the image should not be loaded (since no decryption yet), but it did. On the first run I see that the decoder seem to get called first.
E/SecureBitmapDecoder: decode: w: 1080 h: 432
E/SecureBitmapEncoder: getCipherOutputStream
  1. I need a way to get the filename/identifier within encode & decode method. How to get that? On decoder side I could compose FileToStreamDecoder to get the id, but I don't know for encoder side.
TWiStErRob commented 8 years ago

You can't or shouldn't access the file directly (especially to make decisions based on it). For debug purposes you can override decode in FileToStream and log it there.

I think there's a sourceEncoder and an encoder method. Read the javadoc of all the relevant methods to figure out which ones you need.

By default (RESULT cache) the data is coming from the internet so there won't be cacheDecoder needed to decrypt it. Use diskCacheStrategy(ALL) to force saving the source stream as well, in which case cacheDecoder should be used.

anotherdev commented 8 years ago

I don't want to access the file. Basically I want to know the GlideUrl, because I'm encrypting each image with different key.

This is my setup (I was using ALL all the time):

.asBitmap()
.diskCacheStrategy(DiskCacheStrategy.ALL)
.encoder(new SecureBitmapEncoder())
//.sourceEncoder(new ImageVideoWrapperEncoder(new SecureStreamEncoder(), NullEncoder.<ParcelFileDescriptor>get()))
.cacheDecoder(new FileToStreamDecoder<>(new SecureBitmapDecoder()))
.into(coverPhoto);

What I want to do is encrypt everything that end up on disk (result & source).

TWiStErRob commented 8 years ago

Then I guess you need to parametrize the encoder and decoder and pass in the encryption key (=url in your case). Encoder and decoder are model-agnostic, because it doesn't matter if it's a File or a Uri that the stream is coming from, or where that Bitmap came from.

anotherdev commented 8 years ago

For current settings:

.asBitmap()
.diskCacheStrategy(DiskCacheStrategy.ALL)
.sourceEncoder(new ImageVideoWrapperEncoder(new SecureStreamEncoder(), nullEncoder))
.encoder(new SecureBitmapEncoder())
.cacheDecoder(new FileToStreamDecoder<>(new SecureBitmapDecoder("cacheDecoder")))
.imageDecoder(new SecureBitmapDecoder("imageDecoder"))
.into(coverPhoto);

Without encryption, I tried to log in every encoder/decoder (sourceEncoder, cacheDecoder, encoder, imageDecoder). Loading image the first time trigger:

1) Why does encoder get called after cacheDecoder? Is it for saving RESULT to disk?

If the image was downloaded before and is in disk cache:

2) I don't quite understand why cacheDecoder is called twice?

When I turn on the encryption, first time download stuck at cacheDecoder. The encoder is never called and the image is not displayed.

TWiStErRob commented 8 years ago

I should have linked you the Glide flow chart, sorry for not starting with it: https://docs.google.com/drawings/d/1KyOJkNd5Dlm8_awZpftzW7KtqgNR6GURvuF6RfB210g/edit?usp=sharing


1) Why does encoder get called after cacheDecoder? Is it for saving RESULT to disk?

You're correct.

2) I don't quite understand why cacheDecoder is called twice?

Glide tries to be optimal, but also tries hard to decode the image. This means that the first cacheDecoder is trying to decode from the RESULT cache (you said it was downloaded before). This must fail somehow so Glide didn't receive anything from RESULT cache so it falls back to SOURCE cache. As I said in #718 #707 may cause a problem here if you try to use a different key for encryption in encoder and sourceEncoder.

When I turn on the encryption, first time download stuck at cacheDecoder. The encoder is never called and the image is not displayed.

This sounds like the same as above, encoder did something, but cacheDecoder can't decrypt it, so obvioulsy there's nothing loaded ("not displayed") and hence nothing can be saved to RESULT cache.


Enable log.tag.EngineRunnable logging to see what the exception is and see the EngineRunnable class and its run and decodeFromCache methods; these codify what I said above. Alternatively put a try-catch-throw in all your decode/encode methods and log yourself.

anotherdev commented 8 years ago

Thanks for the detailed info & confirmation.

It seem that the setup is correct all along. I think it is as you said. The encoding & decoding doesn't match even though I use the same cipher instance. From StackOverflow - Encryption of image files on Android, it seem that when writing bitmap, some paddings might got omitted. This also explains why I could use the same system to encrypt/decrypt text just fine (base64 conversion for text).

I tried the solution proposed in that post, but without success yet. I'll test some more.

TWiStErRob commented 8 years ago

Try to test it independently of Glide first, to get the encrypt/decrypt right with BitmapFactory and then try to integrate.

TWiStErRob commented 8 years ago

What's up?

anotherdev commented 8 years ago

I have to switch to other task before completing it, but I got the above setup working using ECB mode (not CBC or CTR). So it's working. Definitely padding issue.

TWiStErRob commented 8 years ago

If this padding is at the end of the stream and it gets omitted when writing, you can try manually flushing the stream before returning.

I'm gonna close this because your original Glide related issue has been resolved. If you find that the non-working of other modes are caused by Glide, feel free to open a new issue any time.

idish commented 7 years ago

By default (RESULT cache) the data is coming from the internet so there won't be cacheDecoder needed to decrypt it. Use diskCacheStrategy(ALL) to force saving the source stream as well, in which case cacheDecoder should be used.

Sorry for replying for an old post but, there must be something I'm missing here,

Say I'm using Result cache. On first time I'm loading the image using glide, the image would be decoded, transformed and the result image would be succesfully encrypted in disk cache using the encode glide method, The next time I try to load this image (in the same dimensions as before, so it will use the same result cache), I would expect that glide will call cacheDecoder to decode the result image from the previously saved disk cache. Otherwise, how does glide make use of the result cache if it decodes all the way once again?