Closed dhunt84971 closed 4 years ago
Hi @dhunt84971 ,
I like the idea of having a set port for it, however there is one potential issue with that, which is what if the OS is already using that port for another application? Does it simply fail to map and update the web interface that the port was in use?
Currently the port is randomly assigned to what is available upon first request, then that value is saved on the server. If at any time that port is in use when we try to map it, we revert to a randomly assigned port that is free, and subsequently update the server with that info.
Please let me know your thoughts on this.
Yes, I get that and it does work beautifully, but in this particular scenario I am trying to access a service that is only available on an intranet from a remote computer. This works well enough but I have to enter the port number into the client application every time it changes. I think a simple error indication if the port is in use would be fine. Then I would be prompted to investigate the reason the port is being used.
Let me know if this makes sense.
I added this in the latest version as a new feature. Check it out and let me know if you see any issues, feel free to re-open or open a new issue if needed. Thanks!
Quick check on my instance does not work like described above. Still gets a "random" source port when specified as specific one
After upgrading MeshCentral to 0.5.1-j, it works. (was 0.5.1-h before upgrade)
Oh, and the parseInt(port.value) on the input from '.otherInp' breaks a hidden/undocumented/accidental feature:
Short of having the all the elements & logic needed for full relay routing support in the UI, up to now you could put something like 80&tcpaddr=192.168.1.99
into Dest Port and get a working relay map to that other IP's port 80 (web interface of a printer in my example/use case).
Hi @baikal ,
The "random" issue still occurring before your MC upgrade was likely due to the fact that the agent core needed to be refreshed for the new functionality to work. Rebooting the server forces this by initiating a new connection between the endpoints / server, or you can use the "refresh agent cores" feature when you upgrade a plugin to push new cores.
As for the parseInt breaking that functionality, I had no idea anyone was using that "accidental feature"! I'll have to take a look at that, and probably just implement the remote address as a field rather than leaving the workaround in place. Thanks for letting me know!
Hi @ryanblenis Yes, must have been agent cores no refreshed. It is certainly not a very good thing to let random strings pass to the agent on one machine which constructs a call from it, sending that one back trough MC to another one ... so I guess it was more of an attack point than a "feature" ...
Hi @baikal ,
That is a very interesting statement you made regarding this hidden "feature". It was the other item I was missing that is offered in MeshCentral Router. Unfortunately Router is only available for Windows. Does this mean this "would" have worked under Linux?
I do get the potential attack point, but an argument could be made that remotely accessing a computer makes it vulnerable enough already to take advantage of other nodes on that computer's network.
BTW, thanks Ryan (@ryanblenis) for knocking this out. It works great so far! I'll be sure to let you know if I run into any issues.
Implemented support for a destination IP from the mapped node. Note that you may need a server reboot for the update to fully take (meshcore changes plus backend changes).
Hi @dhunt84971
Yes, it did work "under linux". In fact, it still does if you change that line back to 'port': port.value,
in that file on your MC installation - it just does not feel safe. The attack point I thought of was not the relay map itself (as this can be done with other tools as well), but the fact that an unchecked string is passed around processes with high privileges on several machines in the MC-network. And could be exploited to make MC or one of the agents to do bad stuff - similar to SQL injection attacks.
But take a look at MeshCmd (part of base MeshCentral), it can do relay mapping on Linux. There is just no GUI, it is all command line stuff.
Hi @baikal and @dhunt84971 , relay mapping was added in the latest version.
Hi @ryanblenis That is great news, thank you. Now, this plugin is the most complete solution in the "mapping business" on MC: any operating system, any 2FA, including relay
There seems to be a display glitch - table widths? (chromium, ubuntu)
Let me test that on some lower resolutions and see if I can get a display fix in. The UI layer is admittedly my weak point. My brain pays attention to functionality- so apologies for my ugly UI's!
it is no so low res (1600 px window), but more of a lot of unused space on the right side:
0.1.2 published with a fix. Please let me know if you find anything else. Thanks!
This is fantastic news. Can't wait to try it out. I may have to wait until next week as our office has been closed for cleaning so everything was turned off. Hopefully someone will be in next week for me to give it a try. (Working exclusively from home these days which is part of the expanded interest in this technology.) Thanks again for this great plugin.
@ryanblenis back to the security point:
maybe using something like 'toIP': destIP.value == '' ? null : destIP.value.replace(/[^0-9a-zA-Z.:-]/g,''),
here to only let pass reasonable dns-names or IP-addresses into the mapping table?
So, as far as security, you as an MC user / admin already have access to that node and to control it in a multitude of ways. So adding additional parameters isn't something that's entirely concerning, unless there is an example or use-case where this can be used to gain access to something you don't already have or something you should not have (such as a node, or elements of it, that you don't or should not have permissions to control / access).
From an input validation (UI/UX) standpoint I can see a point, but I do want to note for anyone that comes across this that this most assuredly NOT a security issue (unless you can demo a proof of concept).
Hi @ryanblenis Sure, I do not disagree with all of that. And I have no doubts the internal "rights" handling in the MC agents only allows what it is supposed to allow. My suggestion was more a blind extrapolation of your "forcing the ports to integers" to all fields.
The reason for that was actually, while testing the new destPort option, I was getting failed results with any input using a copied version of the previous "port" input. I tracked this down to destPort being used in the tcpserver.listen() call, and the fact that it was a "string" instead of a "number". This was causing it to read as 0 and always be assigned a random port instead of the specified destPort. So I forced it on input with parseInt, and decided to do the same to the "port" function for uniformity (which did not have a functional impact because that "port" actually gets appended into a URL to make the tunnel connection, so it didn't matter if it was a string or integer.)
First, this is a great plugin. Works well and the instructions are easy to follow. Thank you for all your efforts!
Next, this is a feature request. I need the port assignment to remain static and not change when I log in. To go a step further I would actually like to assign the local port.
I have forked the project and tried to get this to work, but something about the startRoute and addMap eludes me. Even though I have assigned the localport it doesn't like it. I am sure I am doing something stupid. Any help is appreciated. Again, great job!