Skip to content
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

option 'keepalive' should be false in order to create Storage in browser #205

Closed
joopringelberg opened this issue Aug 26, 2024 · 10 comments
Closed

Comments

@joopringelberg
Copy link

I found that
new Storage({email: accountName, password, userAgent: "Perspectives"}
will cause endlessly repeating failures because of a 'blocked:mixed-content' error. This is because one has to run the lib in a safe (https) environment, but the 'wait' function that is used to keep the connection alive (I suppose) receives an answer with a url with the 'http' scheme. This is then subsequently called.
Using
new Storage({email: accountName, password, userAgent: "Perspectives", keepalive: false}
prevents this.
It might be useful to add this hint to the documentation (since, If I am not mistaken, you do not longer work on the source itself).
Kind regards,
Joop Ringelberg

@qgustavor
Copy link
Owner

All right, I never thought about that because I never used the class on a browser, but you can send a PR if you desire, otherwise I will look into that when I have some free time.

@joopringelberg
Copy link
Author

I am afraid I only looked into the lib enough to understand that the keepalive option causes the looping behaviour. I wouldn't know how to fix it properly - moreover so as it seems that the http url comes from mega's servers, which I assume are not under your control. I tried to simply change the scheme returned from the servers in midflight, but that didn't work.
The pattern is:
- a call is made to https://g.api.mega.co.nz/sc?sn=XXX
- giving a response response {"w":"http://w.api.mega.co.nz/YYY"}
- followed up by a call to that w-value: http://w.api.mega.co.nz/YYY
and then we have a blocked:mixed-content error.
I might be able to change the default value of keepalive to false when run in a browser context, but presumably keeping alive is not a luxury but valuable functionality (the docs mention server side initiative). I will only use file upload (and download), so I do not need the keepalive behaviour - I think.
I've followed up to the call to api.request, which calls pull, which calls wait and then we're stuck.
Conceivably there is an option or parameter to add to the server call that will make it return an https url rather than an http one. That's about where it stops for me.

Kind regards,

Joop

@qgustavor
Copy link
Owner

There must be a parameter to force it to return a HTTPS URL, like there is one for downloading and uploading, otherwise their website wouldn't work.

@qgustavor qgustavor transferred this issue from qgustavor/megajs-docs Sep 3, 2024
@qgustavor
Copy link
Owner

Transferred from megajs-docs to here, since, while it was first just an issue in the documentation, it's clear that's an implementation issue. I guess the issue is in the pull method of the API class, like with the download and upload methods, there must be some parameter to force the API to return a HTTPS URL.

Sadly MEGA's website code is a mess and, the fact they use abbreviations for everything and barely any comments makes it hard to look into their code. The part that handles resp.w in their code is in the middle of the "crypto" code but I can't find where the equivalent code for this part in MEGA's code.

@joopringelberg
Copy link
Author

That is unfortunate. They should support you! Your library is important work.

@qgustavor
Copy link
Owner

MEGA assumes developers should use the official SDK, which is written in C++ so that's not really a fit for browsers. Maybe someone can write a Node.js wrapper for it, but if they themselves are not using this SDK for their web client, is because it's not fit for a browser.

Anyway, I tried to diagnose this using developer tools only to find their web client doesn't call the "sc" endpoint, instead it calls the "wsc" endpoint. Seems the code relevant to it is here.

It called the endpoint with those query arguments: v=3&ec=&sid=[my sid]&sn=[my sn]. It returned an object like this:

{
    "a": [
        {
            "a": "ua",
            "st": "b.UHn=",
            "u": "[redated]",
            "ua": [
                "^!stbmp"
            ],
            "v": [
                "[redated]"
            ]
        }
    ],
    "w": "https://g.api.mega.co.nz/wsc/[redated]",
    "sn": "[redated]"
}

That's just all I found up to far by checking the developer console. From the arguments in the query string, there isn't something that look like the ssl parameter from uploading and downloading. Someone will have to find it in the code. It doesn't help their "crypto.js" file is 3008 lines long, but at least their code is released. There is also their C++ SDK code...

And checking the SDK code, they have documented the "sc" endpoint here. "sc" stands for "server-client" and this endpoint accepts a "ssl=1" parameter exactly as the upload and download methods.

So you can just send a pull request adding this parameter somehow. Maybe as a forceHttps argument in Storage (defaulting to true if the code detects a secure browser environment like globalThis.isSecureContext.

@qgustavor
Copy link
Owner

qgustavor commented Sep 6, 2024

I want you to vote: should we simplify the logic for API.handleForceHttps like below?

// Current implementation
function handleForceHttps (userOpt) {
  if (userOpt != null) return userOpt
  if (globalThis.Deno) return false
  return process.env.IS_BROWSER_BUILD
}

// Proposed implementation
function handleForceHttps (userOpt) {
  if (userOpt != null) return userOpt
  return globalThis.isSecureContext
}

Advantages: it's simpler and the browser/Deno version behaves the same as the Node.js version.

Disadvantage: HTTP websites will use HTTP to connecting with MEGA... which at the moment is less safe, unless MEGA found a way around that issue shown in the docs, then we need to implement it soon as it will be an advantage for Deno and Node.js too as those are sending and receiving files from MEGA using only the static file key, which is terrible for privacy. Also globalThis.isSecureContext is not supported in really old browsers.

Bonus advantage found here: w.api.mega.co.nz servers use outdated TLS settings so Firefox cannot connect to those via HTTPS, just HTTP, so it would allow applications on HTTP to use Storage in Firefox.

Why? Maybe the HTTP page is hosted in localhost. To be fair, that's bad from MEGA for not updating their TLS settings and that's bad for anyone still using HTTP nowadays, I often develop using HTTPS nowadays since it's really simple to get a certificate from Let's Encrypt and the claimed performance issues from TLS which MEGA claims are largely outdated. IIRC I only use HTTP for single-shot servers (like python -m http.server or Node.js equivalents).

@qgustavor
Copy link
Owner

qgustavor commented Sep 6, 2024

I tried to check if MEGA servers implement something to improve privacy over HTTP as they use static encryption keys for files. My test consisted of downloading a file I uploaded without encryption and checking if I could guess what was in the file despite it downloading the file using the 2018's CloudRaid thing from MEGA.

As described by MEGA: "CloudRAID files, which means files are split into 5 pieces + XOR, across 6 servers." so I guessed this XOR was generated in the client, thus would make it a bit harder to someone eavesdropping the network to know which files are being downloaded if they had access to the file key previously. But, no, I checked the pieces being downloaded from the servers: they were exactly the same as I uploaded it, so, as I uploaded them without any encryption, they were kept without any encryption with clear strings showing what the file was (a video file).

Edit: IMHO CloudRaid seems to be only be just a way to balance load between servers or to improve download speeds, not a way to improve security at all.

@xmok
Copy link
Contributor

xmok commented Sep 6, 2024

I want you to vote: should we simplify the logic for API.handleForceHttps like below?

// Current implementation
function handleForceHttps (userOpt) {
  if (userOpt != null) return userOpt
  if (globalThis.Deno) return false
  return process.env.IS_BROWSER_BUILD
}

// Proposed implementation
function handleForceHttps (userOpt) {
  if (userOpt != null) return userOpt
  return globalThis.isSecureContext
}

Advantages: it's simpler and the browser/Deno version behaves the same as the Node.js version.

Disadvantage: HTTP websites will use HTTP to connecting with MEGA... which at the moment is less safe, unless MEGA found a way around that issue shown in the docs, then we need to implement it soon as it will be an advantage for Deno and Node.js too as those are sending and receiving files from MEGA using only the static file key, which is terrible for privacy. Also globalThis.isSecureContext is not supported in really old browsers.

Bonus advantage found here: w.api.mega.co.nz servers use outdated TLS settings so Firefox cannot connect to those via HTTPS, just HTTP, so it would allow applications on HTTP to use Storage in Firefox.

Why? Maybe the HTTP page is hosted in localhost. To be fair, that's bad from MEGA for not updating their TLS settings and that's bad for anyone still using HTTP nowadays, I often develop using HTTPS nowadays since it's really simple to get a certificate from Let's Encrypt and the claimed performance issues from TLS which MEGA claims are largely outdated. IIRC I only use HTTP for single-shot servers (like python -m http.server or Node.js equivalents).

I don't use Deno but the issue itself makes sense - not being able to use the lib due to mixed content is an issue Mega itself should address but the workaround you've come up with seems OK to me as those who are already using modern browsers should be unaffected which will hopefully be the majority. Ought to be a cutoff point for the really old browsers that don't support secure context, methinks. So my vote is 🚢 ship it.

@qgustavor
Copy link
Owner

Fixed as of v1.3.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants