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

Add SSH support on Windows platforms #994

Open
omniproc opened this issue Apr 1, 2020 · 16 comments
Open

Add SSH support on Windows platforms #994

omniproc opened this issue Apr 1, 2020 · 16 comments

Comments

@omniproc
Copy link

omniproc commented Apr 1, 2020

So pygit2 now comes with a pre-compiled libgit2. That's nice.
But on Windows platforms it seems that SSH support is not enabled in that pre-compiled libgit2 version. Even if one installs libssh2, the included libgit2 will not work with SSH.

E.g.:

bool(pygit2.features & pygit2.GIT_FEATURE_SSH)

Will still report false. Not sure if I'm missing something here but if I understand this correctly then it would be nice if the included libgit2 would enable SSH support and eventually even come with a pre-compiled libssh2 for Windows platforms.

I saw that this was on the todo list here: #902
But it's not ticked and I'm not sure it was ever done.

@omniproc
Copy link
Author

omniproc commented Apr 6, 2020

So after some digging it should be able to build libssh for windows on the current appveyor plattform with something like:

git clone --depth=1 -b libssh2-1.9.0 https://github.com/libssh2/libssh2.git %LIBSSH2_SRC%
cd %LIBSSH2_SRC%
cmake -DCRYPTO_BACKEND=WinCNG -DBUILD_SHARED_LIBS=ON -DCMAKE_INSTALL_PREFIX=%LIBSSH2%
cmake --build . --target install

and then integrate it into libssh2 with the DEMBED_SSH_PATH e.g.:

git clone --depth=1 -b v1.0.0 https://github.com/libgit2/libgit2.git %LIBGIT2_SRC%
cd %LIBGIT2_SRC%
set CMAKE_INCLUDE_PATH=%LIBSSH2%/include
set CMAKE_LIBRARY_PATH=%LIBSSH2%/lib
cmake . -DBUILD_CLAR=OFF -DCMAKE_BUILD_TYPE=Release -DEMBED_SSH_PATH=%LIBSSH2% -DCMAKE_INSTALL_PREFIX="%LIBGIT2%" -G %GENERATOR%
cmake --build . --target install

Currently this fails in my local build environment due to unresolved linking issues but I was only able to test this with VS 2019, so it might have to do with my build env beeing too new.

@jdavid
Copy link
Member

jdavid commented Apr 8, 2020

This would be very nice. Maybe you can make a branch and let AppVeyor build, see whether it works with VS 2015?

I know very little about Windows, maybe @fourplusone can help?

@omniproc
Copy link
Author

omniproc commented Apr 10, 2020

External build already works even if I only fork. But the need to push and wait (re-building libgit2 every time) while debugging isn't an optimal workflow as you can imagine 😅
However I'll try to setup a reproducable Docker for Windows based build environment and can document that later on if I can get it running.

@omniproc
Copy link
Author

omniproc commented Apr 14, 2020

Okay, the initial approachusing Docker for Windows failed. At least for me the current Windows Containers implementation, even on Server 2019, seems to unstable to use.
I also fiddled around with vcpkg which would have potential to replace the current libgit2 build process for a process with less dependencies on the build env. However I wasn't able to embedd libssh2 into libgit2 with that approach.

So the main issue here seems to be that altought provided the libssh2 dependencies can't be resolved. You can check the last lines of my test build. and find the unresolved external symbol errors there.

I then noticed that libgit2 seems to support dynamic linking of libssh2 at runtime. So if libssh2 is present, It'll use it. This seems to be possible on Windows as well. However: that would have some implications since then we'd need to include libssh2 binaries and all the binaries that it depends on in the wheel for Windows as well.
So far this would mean something like this:

if compiler_type == 'msvc':
            libgit2_dlls.append('git2.dll')
            libgit2_dlls.append('libssh2.dll')
            libgit2_dlls.append('zlib1.dll')
            libgit2_dlls.append('libcrypto-1_1-x64.dll')
            libgit2_dlls.append('libssl-1_1-x64.dll')

into the setup.py. This seems very error prone and thus I think it's still way cleaner to have libssh2 embedded into libgit2. We just have to figure out how to do it. The documentation on this is non existent or false at best. I guess this needs some deeper digging in the source of libgit2 and someone with more C++ and msvc/cmake exp. then I have.

@omniproc
Copy link
Author

omniproc commented Apr 16, 2020

So I got this compiling finally. ssh support is available on Windows. But libgit2 seems to be failing to actually make any ssh connection.

key=".path/to/id.key"
crt="./path/to/id.pub"
origin = "ssh://git@github.com/some/repo/where/ssh/keys/have/been/added"
keypair = pygit2.Keypair("git", crt, key, "")
callbacks = pygit2.RemoteCallbacks(credentials=keypair)
pygit2.clone_repository(origin, path, callbacks=callbacks)

Works on Linux, but when I run it on Windows with the embedded libssh2 I currently get: _pygit2.GitError: failed to start SSH session: socket disconnect, which seems to be from here in libgit2. And this seems to come from libssh2 itself: LIBSSH2_ERROR_SOCKET_DISCONNECT.

Just not sure why the socket is beeing disconnected yet.
So what I could provide is a build process for the libssh2 integration into the libgit2 binary of the windows build. But since libssh2 seems not to be working currently and fixing this is beyond me I'm not sure where to go from here.
Still include it and open a new issue with libssh2?

@jdavid
Copy link
Member

jdavid commented Apr 20, 2020

Cool, I think it's still worth building with libssh2.

Ideally I think libgit2 should provide binaries for Windows, so bindings don't have to. This would avoid duplicating the work in every binding, and also would speed up CI. Maybe it's worth checking what other bindings do, such as the one for Ruby.

ping @ethomson

@ethomson
Copy link
Member

Cool, I think it's still worth building with libssh2.

To be clear up front: this is ultimately Your Decision, not mine. 😁 But I do want to give some background:

The LibGit2Sharp project has decided not to ship libssh2. The rationale is that the team decided that if they were to distribute the binaries of libssh2 then they would also has to do the work of responding to critical security releases. If there's a security vulnerability in libssh2 that they were shipping then that would become a security vulnerability in LibGit2Sharp that needed to be addressed. The team, at the time, was not eager to "own" that process.

Although that means - much like the status quo of pygit2 - that users will not get SSH support by default and would need to build their own binaries. In practice, nobody does this.

I don't have in-depth knowledge about any other bindings, I'm afraid. I don't think that any of them ship libssh2, but I could be mistaken.

I then noticed that libgit2 seems to support dynamic linking of libssh2 at runtime. So if libssh2 is present, It'll use it. This seems to be possible on Windows as well. However: that would have some implications since then we'd need to include libssh2 binaries and all the binaries that it depends on in the wheel for Windows as well.

It does not support dynamic loading during runtime. The PR you're linking to was not merged for a plurality of reasons.

Just not sure why the socket is beeing disconnected yet.

Nor am I, sorry. I know very little about libssh2.

@omniproc
Copy link
Author

omniproc commented Apr 21, 2020

The LibGit2Sharp project has decided not to ship libssh2. The rationale is that the team decided that if they were to distribute the binaries of libssh2 then they would also has to do the work of responding to critical security releases. If there's a security vulnerability in libssh2 that they were shipping then that would become a security vulnerability in LibGit2Sharp that needed to be addressed. The team, at the time, was not eager to "own" that process.

Hm, but couldn't that process be much simplified and automated?
E.g.: have a fixed tagged release and in addition to that build a nightly of that release that will use the latest release of libssh2. If it compiles (including unittests): fine. Ppl. concerned with the particular flaw can just grab the nightly that includes the patch.
If it doesn't compile (including unittests) it's something that needs to be addressed anyways by libgit2.

@jdavid
Copy link
Member

jdavid commented Apr 24, 2020

Thanks @ethomson for the feedback.

Actually we're statically linking libssh2 in Linux wheels. I'd rather do the opposite of what we do now: drop libssh2 from the Linux wheels and add it to the Windows wheels. Don't know about the situation with MacOS wheels.

Ping @webknjaz @rcoup

@webknjaz
Copy link
Contributor

@jdavid I think it's important to have libssh2 shipped in manylinux1 wheels. Users who want pygit2 linked against their system libssh2 would probably use the dists that are coming from OS package managers packaged by their respective maintainers (talking about homebrew, deb, rpm, ebuild etc). The main reason I contributed manylinux wheels was that I won't have to set up the env with a build toolchain for installing from sdist (nobody wants this usually).

Also, I agree with @M451 — if building and publishing new wheels is well-automated, it's pretty easy to release often, mostly targeting the latest version of libssh2. You could even do post-releases (as per PEP 440) if only the bundled libssh2 version has changed.

Another option would probably be to depend on something external like https://pypi.org/project/ssh2-python/#history but it looks poorly maintained.

@jdavid
Copy link
Member

jdavid commented Apr 24, 2020

Ok, let's keep the current status quo then.

Just have to add libssh2 to the Windows wheels. No idea about the socket disconnect error though.

@omniproc
Copy link
Author

Okay. I'll test and provide a working PR based on appveyor.

@rcoup
Copy link
Contributor

rcoup commented Apr 27, 2020

When I was looking at macOS wheels I found this, which basically implied libssh2 isn't recommended, and doesn't do what people need (modern EC crypto, handling openssh config files, etc). I was planning to revisit once libgit2 switches to libssh.

libgit2/libgit2#5225

@wmtorode
Copy link

Has there been any movement on this? having SSH support in windows would be a huge help for me.

@omniproc
Copy link
Author

I didn't find the time to add the wheel to the windows lib yet but even with it, it would not be working on Windows (yet), see socket disconnect error that would need some deeper digging.

@mg-christian-axelsson
Copy link

What is the current status on this? Has any work been done?

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

No branches or pull requests

7 participants