search

johndbritton / teleport

Virtual KVM for macOS
GNU General Public License v2.0
785 stars 131 forks source link

Use local encryption setting as default #100

Closed FliegenKLATSCH closed 2 years ago

FliegenKLATSCH commented 3 years ago

If the client is not shared (advertised via Bonjour) the server does not find the encryption setting of the client and assumes always true. This change will adjust this behaviour and defaults to the local setting on the server.

johndbritton commented 2 years ago

Thanks for the contribution!

Have you tested this change locally? Did it resolve #64 for you?

FliegenKLATSCH commented 2 years ago

2x Yes..

johndbritton commented 2 years ago

Ok, I will generate a build with these changes and give it a try.

johndbritton commented 2 years ago

A pre-release is currently building. Once that's done I'll try it out and will merge if everything is working as expected.

https://github.com/johndbritton/teleport/releases/tag/v1.3.4-pre

https://github.com/johndbritton/teleport/actions/runs/1049645004

johndbritton commented 2 years ago

Looks like the automated build and sign isn't working due to a missing gem or something. I'll have to try again later.

marcpbailey commented 2 years ago

Confirming I built this with Xcode 13 and have tested for about a day. Observations:

What good luck: I definitely agree that this fix is a positive step. On multihomed Macs (which is pretty much every machine these days), teleport has been extremely frustrating. This fix (at least for non-encrypted connections - haven't tested encryption) resolves all the issues I've been having (yes, #64 and #100), and configuration/connection/reconnection is now 100% reliable.

What bad luck: A multi homed client now sees itself as a possible host to connect to, that is at the top of the layout panel. Screenshot attached. Screen Shot 2021-11-21 at 4 44 25 pm

Appears to be harmless, even if you try to connect (nothing bad happens), but it's maybe not the best UX, and perhaps a filter preventing display of self-originated bonjour addresses would be a good idea.

I do use an avahi bonjour reflector between routed VLAN subnets on this network. The devices I have been testing are on the same bridged wired/wifi VLAN subnet so I don't think that's a factor, but can't rule it out because I can't disable the reflector non-disruptively here. It should be very easy for others to try this out and attempt to replicate; you just need two Macs with both Ethernet and wifi on the same bridged VLAN subnet (basically any home wifi access point will create this environment).

@johndbritton I would not suggest holding the build on this basis because it's such a fantastic improvement, but a release note to mention this as a possible side effect might be advisable. @FliegenKLATSCH thank you so very much for figuring this out. I had spent many person-days of effort trying to debug this from a pure network packet perspective, assuming incorrectly that the code must be solid.

For anybody else looking to test drive this branch and who hasn't used Xcode for, like, a decade, as was the case for me: I used Xcode 13 to clone the repo, switched to the 1.3.4-pre branch, then had to change the project preferences to get this to build because of Xcode 13 changes. Specifically:

Once successfully built, quit and rename/move your original teleport 1.3.3 app from the Applications folder. Remove teleport from your Mac's System Preferences>Security & Privacy>Accessibility panel. Move the newly built teleport in and launch. I did not need to delete or erase preferences. It was a beautiful thing that just started connecting properly like it was always supposed to.

johndbritton commented 2 years ago

@marcpbailey any chance you know why the automated build is failing? Haven't had time to fix that.

marcpbailey commented 2 years ago

Hi @johndbritton I'm afraid I've had no experience with automated Xcode builds. Embarrassingly, the last production compile I did with Xcode was for a PowerPC binary! However, based on what I found above, Xcode 13 won't build for MacOS 10.8, which is the minimum target in the repo project. Also, I think the certificate signing has changed, but Gatekeeper/notarisation is all super new to me so I can't be sure. Before I enabled "Automatically manage signing" I could not get the "Mac Developer" signing option to build (would fail the Sparkle framework signing script step), but I think that was because my Apple Developer subscription is not current and my cert/key/token couldn't sign libraries if I attempted to "Sign to run locally". Since the project wouldn't build as-cloned but does once I changed these things, perhaps this is a clue as to what to change to fix the auto build if you are also attempting this in Xcode13? Thanks also for all your work on this project. There's no software KVM as cool as Teleport.

johndbritton commented 2 years ago

I'll try fixing the automated build, worst case I can upload a manual build but I'd prefer not to.

johndbritton commented 2 years ago

@FliegenKLATSCH I finally got the automated builds working again and have created a prerelease that includes this fix here: https://github.com/johndbritton/teleport/releases/tag/v1.3.4-pre

marcpbailey commented 2 years ago

Thanks @johndbritton. Confirming autobuild 1.3.4-pre is notarised and works as my own build did. Additionally now confirmed on MacOS 12.2 Monterey between a MacBook Air M1 and an iMac14,2. Recommend setting the target:teleport>Info>Get Info string if you can for 1.3.5 please - otherwise there's no way to tell version in the Finder/filesystem. (screenshot comparing Get Info for your build and my earlier one attached). Screen Shot 2022-02-02 at 2 11 56 pm

johndbritton commented 2 years ago

Thanks for validating @marcpbailey. I've moved your Finder get info string request to a new issue (https://github.com/johndbritton/teleport/issues/115). I want to make that something that is automatically set every time a new version is created so I don't have to manually do it.