FreeTAKTeam / FreeTakServer

Situational Awareness Server compatible with TAK clients
Eclipse Public License 2.0
649 stars 166 forks source link

private data packages issued with incorrect URI when using SSL #111

Closed tsvtx closed 3 years ago

tsvtx commented 3 years ago

source: clean installation of FreeTakServer 1.5.10 w/ SSL working properly issue: URI's being issued by the server to the client for retrieval of private packages are issued with http and not https when using SSL.

summary: this issue appears to be sending the client an invalid URI that causes it to timeout on private data package retrieval and report a response of 'empty reply from server' (WINTAK) or simply report that the download failed (ATAK). public data packages do not seem affected by this behavior and download without issue over SSL.

upon inspection of the network traffic, the request for the private package can be seen being issued via http vs https.

expected: https://10.0.5.4:8443/Marti/api/sync/metadata/packagehash/tool?receiver=recipient result: http://10.0.5.4:8443/Marti/api/sync/metadata/packagehash/tool?receiver=recipient

wireshark results from the client: GET /Marti/api/sync/metadata/18359b8de3960982ac29290d0ab26831aa401f47bb8a58fac77f538cec81a673/tool?receiver=tsvtx HTTP/1.1 Host: 10.0.5.4:8443 Accept: / \r\n [Full request URI: http://10.0.5.4:8443/Marti/api/sync/metadata/18359b8de3960982ac29290d0ab26831aa401f47bb8a58fac77f538cec81a673/tool?receiver=tsvtx] [HTTP request 1/1]

additionally: if the URI is manually input into a browser using https, the package downloads successfully as tool.zip to the local machine if the package is unmarked as private and retrieved using the data package retrieval in the client occurs with no issues

please let me know if additional information is needed.

brothercorvo commented 3 years ago

this is not a bug. SSL is NOT HTTPS

SSL (Secure Socket Layer) or TLS (Transport Layer Security) works on top of the transport layer, e.g. TCP. SSL can be used for more or less any protocol, HTTPS is just one common instance of it.

HTTP is an application layer protocol.

In regular, non-encrypted HTTP, the protocol stack can look like this:

When using HTTPS, the stack looks like this:

so the URL is not changing, but the communication is encrypted

tsvtx commented 3 years ago

I totally understand as I'm familiar with how the stack works. This is also why I was racking my brain on it last night and hesitated dropping this in as an issue in tracking vs seeing if anyone else had seen the behavior via the discord channel. I could have worded it a bit better above. Blame my lack of coffee usage I guess.

So if the http:// URI call is by design, then something else is at play as that call receives an ERR_EMPTY_RESPONSE from the server. I thought maybe it was supposed to be https:// because when switching the call from http to https it brought down the correct tool.zip.

I was honestly hoping it was something obvious that I was doing that was dumb. However, after combing through the docs repeatedly I can't seem to find anything config or otherwise that would cause this.

naman108 commented 3 years ago

thank you for bringing this up as I believe I may know the cause of the issue but to confirm I have a few question

  1. are all other aspects of FTS functioning properly
  2. is the same behaviour exhbited in ATAK
  3. is this behaviour arising when getting DataPackages from the server and or uploading DataPackages to the server

edit: didnt mean to close the issue sorry

tsvtx commented 3 years ago

on the close no worries, lol.

  1. Yes, all other aspects appear to be working fine. Clients can connect, send messages, drop pins, etc. They can also download DataPackages that have not been marked as private without issues after querying the server from the client. (ex. Google Maps package is published publicly and all users can download and import without issue)
  2. Yes. This was where we first observed the behavior.
  3. The behavior is experienced only on download of DataPackages marked private after notification. The DataPackage uploads as expected and creates an entry successfully in 'FreeTAKServer/FreeTAKServerDataPackageFolder/' as a hashed directory. When the client receives the notification of the package availability, it is at this point the behavior is experienced and fails to download.

Let me know if you need any further information.

naman108 commented 3 years ago

that's enough I believe I know exactly what the issue is here on line https://github.com/FreeTAKTeam/FreeTakServer/blob/89f1e03c7cc8669489ef8288291fe17be9f8ae06/FreeTAKServer/controllers/services/DataPackageServer.py#L256

this likely needs to be change to return 'http://' + IP + ':' + str(HTTPPORT) + "/Marti/api/sync/metadata/" + hash + "/tool" or return 'https://' + IP + ':' + str(HTTPPORT) + "/Marti/api/sync/metadata/" + hash + "/tool" depending on the clients connection type, we will include this in the next release

tsvtx commented 3 years ago

thanks!

tsvtx commented 3 years ago

FYI, because I like to tinker...

I modified line 256 with no change in behavior on the clients. I'm going to leave it for now to see if it introduces another issue elsewhere and will report back... I did however, locate another, more suitable instance of the package code that lined up a bit better...

https://github.com/FreeTAKTeam/FreeTakServer/blob/89f1e03c7cc8669489ef8288291fe17be9f8ae06/FreeTAKServer/controllers/services/DataPackageServer.py#L173

I modified that to the following and the test package sent with success:

return 'https://' + IP + ':' + str(HTTPPORT) + "/Marti/api/sync/metadata/" + file_hash + "/tool"

My only concern here would be hardcoding in that https... while it worked for me, I could see this introducing issues for someone who wasn't pushing this stuff over 8443 and over 8080 instead.

Don't get me wrong, I know enough to be dangerous here as I'm still on a steep learning curve in python so definitely don't take me for someone experienced here, haha. But since I don't mind taking some risks and dropping some code changes in for the sake of testing, here we are. :)

brothercorvo commented 3 years ago

this has been addressed in 1.5.12 @tsvtx , please check out this version on pip