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

Use local encryption setting as default #100

Merged
merged 2 commits into from
Feb 1, 2022

Conversation

FliegenKLATSCH
Copy link
Contributor

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
Copy link
Owner

Thanks for the contribution!

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

@FliegenKLATSCH
Copy link
Contributor Author

2x Yes..

@johndbritton
Copy link
Owner

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

@johndbritton
Copy link
Owner

johndbritton commented Jul 20, 2021

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
Copy link
Owner

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
Copy link

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:

  • target:teleport>General>Identity>Version: 1.3.4beta
  • target:teleport>General>Identity>Build: 1.3.4-pre
  • target:teleport>Info>Get Info string: "$(CURRENT_PROJECT_VERSION) prerelease build to solve multihoming"
  • target:teleport>General>Deployment Info>Deployment Target: "10.9" (earliest macOS now supported by Xcode)
  • target:teleport>Signing & Capabilities>Automatically manage signing: checked (Could not sign the Sparkle framework successfully without this)
  • target:teleport>Signing & Capabilities>Signing Certificate: "Development" (I'm just using a free personal Apple Developer ID and personal access token to test)
  • target:teleport>Signing & Capabilities>Team: your Apple Developer ID team
  • target:teleport>Build Settings>Architectures>Architectures>: "Standard Architectures (Apple Silicon, Intel) - $(ARCHS_STANDARD)

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
Copy link
Owner

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

@marcpbailey
Copy link

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
Copy link
Owner

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

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
Copy link
Owner

@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

Copy link
Owner

@johndbritton johndbritton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good and works locally for me, going to merge.

Please test out the prerelease and once I have a few other people saying the prerelease build is fine I'll fire off a stable build for the auto-updater.

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

@johndbritton johndbritton merged commit 9695ffa into johndbritton:main Feb 1, 2022
@marcpbailey
Copy link

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
Copy link
Owner

johndbritton commented Feb 2, 2022

Thanks for validating @marcpbailey. I've moved your Finder get info string request to a new issue (#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.

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

Successfully merging this pull request may close these issues.

3 participants