Skip to content
This repository has been archived by the owner on Nov 7, 2023. It is now read-only.

Windows support #286

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Windows support #286

wants to merge 13 commits into from

Conversation

zobo
Copy link

@zobo zobo commented Oct 21, 2019

Ported as much as possible to windows. Works still under the same principles and needs (OpenSSH_for_Windows_7.7p1). Uses ProxyCommand, moved ssh agent to windows pipe.
Compiled files here: https://github.com/zobo/kr/releases/tag/2.4.15-win

One big caveat is that the vendored code is quite old and the ssh agent has problems connecting to newer servers as it does not pass ssh flags with it.

I tried to go.mod the code but turned out to be a challenge due to some vendored packages not being available anymore.

@zobo zobo mentioned this pull request Oct 21, 2019
@tiernano
Copy link

just downloaded and installed and some things of note:

  • if you try do a kr pair in Powershell (under the new terminal) The QR code is not scannable. it is under cmd.exe though...
  • Windows tried blocking the app on first run... it finally allowed it though though...
  • Visual Studio Code works with it very well! the remote SSH part works very nicely with KR!

So, happy days so far!

@jasperweiss
Copy link

jasperweiss commented Oct 22, 2019

It only worked for me when manually running krd.exe in a seperate terminal. Otherwise it would say Could not connect to Krypton daemon. Make sure it is running by typing "kr restart".
So by keeping krd.exe running in a seperate window, ssh worked but git didn't.
Once I was done testing I wanted to uninstall kr and revert back to the default ssh config but unfortunately kr uninstall does not work properly and now I'm left wondering how to get normal ssh working again 🤔

@zobo
Copy link
Author

zobo commented Oct 22, 2019

Hi @Bodup Did you place all three files in the same folder?
When kr installs, it edits your .ssh/config file. A Host * definition is appended to it.
You can remove it by editing c:\users<you>.ssh\config

@zobo
Copy link
Author

zobo commented Oct 22, 2019

just downloaded and installed and some things of note:

  • if you try do a kr pair in Powershell (under the new terminal) The QR code is not scannable. it is under cmd.exe though...
  • Windows tried blocking the app on first run... it finally allowed it though though...
  • Visual Studio Code works with it very well! the remote SSH part works very nicely with KR!

So, happy days so far!

Ideally this would be a windows try app, with all functions exposed over context menu and the QR code displayed in a gui window....

@zobo
Copy link
Author

zobo commented Oct 22, 2019

So by keeping krd.exe running in a seperate windowd, ssh worked but git didn't.
Were there any errors? Something to go on. For me personally github does not work, but I know the reason, the agent component needs to be updated (however you might be facing a different issue).

@agrinman
Copy link
Contributor

@zobo just migrated kr to go modules, could you PR your changes against current master?

@zobo
Copy link
Author

zobo commented Oct 22, 2019

@zobo just migrated kr to go modules, could you PR your changes against current master?

Hi @agrinman. Will look at it asap.

@jasperweiss
Copy link

@zobo Ah, so apparently the first issue (krd isn't running when trying to run kr pair) is caused by the folder name containing a space (e.g "New Folder")

Either you can run krd manually in a separate window or rename the folder to something that doesn't contain any spaces like "kr".

Even with the folder renamed however, git doesn't work. It gives the following error:

Cloning into 'kr'...
/bin/sh: line 0: exec: C:UsersUserDesktopkrkrssh.exe: not found
ssh_exchange_identification: Connection closed by remote host

Again it seems to be caused by an incorrect path.

@zobo
Copy link
Author

zobo commented Oct 22, 2019

@Bodup Great, I can see the issue are backslashes... Will work on that, thanks!

@zobo
Copy link
Author

zobo commented Oct 22, 2019

Rebased to latest code. It's much more organized and after settling all conflicts easier to work with. Nice!
Also added ExtendedAgent support, so that now sha256 and sha512 signatures pass correctly to fallback agent.

@zobo
Copy link
Author

zobo commented Oct 25, 2019

Changed how krd.exe is started to also handle paths with white space.

@zobo
Copy link
Author

zobo commented Oct 30, 2019

@agrinman do you need anything else to merge this? It seems to work quite well here...

@agrinman
Copy link
Contributor

@agrinman do you need anything else to merge this? It seems to work quite well here...

Sorry for the delay, I will find time soon to review and test.

One thing you could help with is updating the build script, and adding the windows build matrix to the GitHub workflow [if this is something you have access to]? Also what steps are required for a single-command installation step?

Also, please take a look at our contributor policy

@zobo
Copy link
Author

zobo commented Oct 31, 2019

I’ll look at what can be done with build scripts and read up a bit on workflows.

For the oneliner, maybe a powershell download and extract combo.

Regarding contribution policy. It states that “each commit” contain the text. Do you need me to squash the branch and change all commit messages? Or can I just add it to a recent commit? Or simply to this PR comment thread?

@agrinman
Copy link
Contributor

I’ll look at what can be done with build scripts and read up a bit on workflows.

Thanks @zobo! I would imagine it would be as easy as adding windows-latest to the build matrix.

For the oneliner, maybe a powershell download and extract combo.

Sounds reasonable -- I'm not a Windows dev, so working off your experience.

Regarding contribution policy. It states that “each commit” contain the text. Do you need me to squash the branch and change all commit messages? Or can I just add it to a recent commit? Or simply to this PR comment thread?

I think it would be best to squash into one commit when we're ready to merge and add it there.

@zobo
Copy link
Author

zobo commented Oct 31, 2019

Did small fixes in unix and darwin code so that it compiles again. And tried to add workflow for win building. This is a complete stab-in-the-dark since I am not in the Beta Actions program and can't test it...

Regarding the single step installation. I was thinking about it, but to be honest, I think initially a link to a .zip would be just fine. "Extract to a folder of your liking, run 'kr pair'". The thing is, windows doesn't have a nice equivalent to /usr/local/bin and the proper way would be to create either an (MSI) installer or Windows Store application that would also take care of path, start menu etc...

There are quite a few other aspects that would be interesting to develop, but I'd focus on getting a windows build out there for people to start using it, even if with a few clicks more than the unix counterparts.

@PeterStaev
Copy link

I just tried the binaries and for me it doesn't work. Pairing worked fine, but then when I try to SSH I get the following:

CreateProcessW failed error:87
posix_spawn: Unknown error

@zobo
Copy link
Author

zobo commented Nov 5, 2019

@PeterStaev Could you try it with ssh -vvvv ... to get some verbose output, also please look at the contents of your c:\users....ssh\config . The part that was added by kr. Is the ProxyCommand in any way strange?
Did you drop the .exe files into a folder with spaces?

@PeterStaev
Copy link

The config looks ok:

# Added by Krypton
Host *
	IdentityAgent \\.\pipe\krd-agent
	ProxyCommand "L:\Krypton\krssh.exe" %h %p

verbose log:

OpenSSH_for_Windows_7.7p1, LibreSSL 2.6.5
debug1: Reading configuration data C:\\Users\\peter/.ssh/config
debug1: C:\\Users\\peter/.ssh/config line 4: Applying options for *
debug3: Failed to open file:C:/ProgramData/ssh/ssh_config error:2
debug1: Executing proxy command: exec "L:\\Krypton\\krssh.exe" host 22
debug3: spawning ""L:\\Krypton\\krssh.exe"" host 22
CreateProcessW failed error:87
posix_spawn: Unknown error

@zobo
Copy link
Author

zobo commented Nov 5, 2019

Hum.. Can you try to remove the quotes from ProxyCommand?

@zobo
Copy link
Author

zobo commented Nov 6, 2019

Apparently the quotes were the issue. Will create another build.

kr pair and other commands do not work since 49848cd commit.
@tiernano
Copy link

tiernano commented May 1, 2020

any ideas if this will ever get into production? is krypton even still being worked on? since Akamai bought the company, nothing has been updated... ironically, even their website is running slow...

@zobo
Copy link
Author

zobo commented May 3, 2020

I also haven't heard anything back...

@agrinman
Copy link
Contributor

agrinman commented May 3, 2020

@zobo: sorry for the delay here.

I think there are two more issues to work through:
1: Can we add a windows github action to test the build?
2: Not sure where we left off on this, but what's a canonical way for windows users to install kr? It could as easy as an equivalent to curl | sh. What do you think? Here's what we do for unix/linux/macos distros: https://github.com/kryptco/www/blob/master/kr (this is available at https://krypt.co/kr).

@zobo
Copy link
Author

zobo commented May 4, 2020

Hi @agrinman. I think I have access to github actions now and will look at it. I'll also see what makes most sense for install process.

Fix to run krd.exe in background.
@zobo
Copy link
Author

zobo commented May 4, 2020

Hi @agrinman. I tested GH Actions and they seem to work as expected. I also had an open PR that fixed an issue and was merged into my branch. I went back and reviewed the code and would just like to point out that perhaps you should look at analytics_ua_windows.go and let me know how to change it.

@agrinman
Copy link
Contributor

agrinman commented May 4, 2020

@zobo: just updated the ua_windows file, thanks.

@judge2020
Copy link

judge2020 commented May 4, 2020

what's a canonical way for windows users to install kr?

There might be a way to distribute via the Windows Store but I wouldn't count on it, plus it would segment availability as some people run Windows 7 or use Windows 10 without a Microsoft account.

The most compatible way would be to use an installer framework like NSIS (with modern UI 2) to create an installer+uninstaller for Windows.

@tiernano
Copy link

tiernano commented May 5, 2020

what's a canonical way for windows users to install kr?

Either Powershell or Chocolatey would be your best bet... If you look at the chocolatey install process (https://chocolatey.org/install) its a single one line script that you run and it does everything... something like that would probably be good...

@zobo
Copy link
Author

zobo commented May 5, 2020

This doesn't require the user to do more than drop the 3 .exe files in any folder, although it would be nice if kr.exe was in path.
Creating a MSIX would be possible, but the Windows Store overhead might not be worth it.
I personally always favor a .zip download over any form of installation, but I see the benefit of it...

I'd go with a PowerShell install option, similar to Chocolatey, that would create a folder somewhere under users AppData (similar to Chrome) and add the path with [System.Environment]::SetEnvironmentVariable, [System.EnvironmentVariableTarget]::User.
Perhaps even check if ssh is available.
But definitely give the user a direct download link also.

Also, we'd need to look at kr upgrade.

@agrinman
Copy link
Contributor

Just heard about winget: https://devblogs.microsoft.com/commandline/windows-package-manager-preview/

Anyone interested in taking a look what it would take to get winget install kr working?

@judge2020
Copy link

https://github.com/microsoft/winget-pkgs

Still requires an installer and uninstaller, it won't provide an easy way to place/remove kr.exe or add kr.exe to path (unless msix packages can do that, not sure).

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

Successfully merging this pull request may close these issues.

8 participants