enso-org / enso

Hybrid visual and textual functional programming.
https://enso.org
Apache License 2.0
7.34k stars 323 forks source link

Language Server: `file/checksum` fails after uploading a big file (1.4 GB) #6691

Open farmaazon opened 1 year ago

farmaazon commented 1 year ago

Follow up of https://github.com/enso-org/enso/issues/5051

After dropping the Enso AppImage (~1.4 GB), the IDE uploads the file, but then it tries to get the checksum (to make sure it's uploaded correctly), but the file/checksum operation fails with message:

{"jsonrpc":"2.0","id":701,"error":{"code":1,"message":"Service error"}}

I get the following enigmatic error log from Language Server:

 INFO ide_ci::program::command: npm⚠️ [error] [2023-05-15T08:31:17.59Z] [org.enso.languageserver.requesthandler.file.ChecksumFileHandler] Failure during [ChecksumFile] operation: ***

Due to this bug, the call to this function will be commented. Once fixed, it should be uncommented again: https://github.com/enso-org/enso/blob/81d4b08c786c1d13c47f663815351b6414d30257/app/gui/src/controller/upload.rs#L145

hubertp commented 1 year ago

While we are at it, is there any reason why insist on using SHA-3 256 for file checks while MD5 or SHA1 would be perfectly fine, and likely much faster? Internally, for the caches, we have already made that move and that saved us some time.

hubertp commented 1 year ago

And to give a better argument, on my machine checksum of Enso's AppImage:

wdanilo commented 1 year ago

I don't think anyone insists on using SHA-3 256 here and I also think that changing that to md5sum is a good idea from the perf perspective. @farmaazon what do you think?

farmaazon commented 1 year ago

I'm not sure. The entire world seems to shift from SHA-1/md5 to SHA-3. Even similar cases like ours, e.g. the digests of Linux installers. On the other hand, I cannot imagine how someone could achieve some malicious goal by exploiting collisions of SHA-1 sums when uploading files to the cloud. I wouldn't rely on this too much, however - attacks can be very inventive.

wdanilo commented 1 year ago

But, if I understand this correctly, this is just checking if the file we got is the one that user tries to upload, right? If so, what are the scenarios of a theoretical attack here? I can't imagine any of such scenarios.

farmaazon commented 1 year ago

Yes, and you can say the same about hashes of Linux installers. But still, for some reason, every Linux installer download page uses sha256 for their hashes.

Also, as a rule, not being able to imagine an attack does not mean it does not exist.

In this very case, I would say a bit of paranoia is not harm. So far, computing hashes is a tiny fraction of time spent on uploading them, so I'd say we can afford it.

jdunkerley commented 1 year ago

As would need changing the rust code - putting on hold. We should do this on the JS implementation.

vitvakatu commented 11 months ago

I suggest starting working on this one, @jdunkerley, as we’re implementing file uploading in the new gui and we’re facing the same problem.

jdunkerley commented 3 months ago

Related to #8662