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

WIP: LibGit2 - mbedTLS stream support #17246

Closed
wants to merge 31 commits into from
Closed

WIP: LibGit2 - mbedTLS stream support #17246

wants to merge 31 commits into from

Conversation

wildart
Copy link
Member

@wildart wildart commented Jul 2, 2016

Provide support of SSH protocol in LibGit2:

  • add binary dependencies: mbedtls, libssh2
  • try to use git_transport to avoid custom version of libgit2
  • use custom mbedtls TLS stream in libgit2
  • custom configuration of the mbedtls
    • try 2.3.0

Update: Proxy support will be available starting from libgit2 0.25, so I removed it from the agenda of this PR.

Update 2: Using custom transport is not a good option, because we only need TLS stream support in available http transport. Instead, custom TLS stream can be registered in libgit2. This is much less work then complete transport definition. I already done this as rejected PR libgit2/3462.

Ref: #16041

@tkelman tkelman added domain:building Build system, or building Julia or its dependencies libgit2 The libgit2 library or the LibGit2 stdlib module labels Jul 2, 2016
@wildart
Copy link
Member Author

wildart commented Jul 3, 2016

@tkelman Travis fails because of old version of cmake. Is there any way to update it?

@tkelman
Copy link
Contributor

tkelman commented Jul 3, 2016

we can use a ppa or download cmake binaries, but that's going to be annoying. llvm 3.9 will require cmake 3.4.3, but until we move to that we should try to keep compatible with versions of cmake that are easily available in existing linux distros.

@wildart
Copy link
Member Author

wildart commented Jul 3, 2016

As I wrote in the PR description. There is one option to avoid using custom version of libgit2, it's a custom TLS stream registration. I already done it as a part of libgit2/3462. I could create small C library from this PR which would contain required mbedtls TLS stream setup functions. Or write whole thing in Julia, which is going to look more like MbedTLS.jl.

@wildart
Copy link
Member Author

wildart commented Jul 3, 2016

libssh2 uses this ppa for cmake in order to run on travis

@tkelman
Copy link
Contributor

tkelman commented Jul 3, 2016

It's not travis I'm concerned about, it's everyone who builds from source on linux. What do we need from new cmake, and can we live without it?

Didn't libgit2 say to do the stream registration differently? libgit2/libgit2#3465

@wildart
Copy link
Member Author

wildart commented Jul 3, 2016

For libgit2, that is exactly what have been suggested in libgit2/libgit2#3465. Apart from minor fix (libgit2/libgit2#3850), that is the way to go. But I'm suggesting to include whole mbedtls stream construction functionality as C library.

For libssh2, we need to update cmake, otherwise it will not be able to build.

@tkelman
Copy link
Contributor

tkelman commented Jul 3, 2016

what do they require as the minimum version? since llvm will require 3.4.3 eventually, maybe we should just download a binary of cmake for people on linux

(sorry about the spam close/reopening, stupid button placement is made worse by laggy phone)

@tkelman tkelman closed this Jul 3, 2016
@tkelman tkelman reopened this Jul 3, 2016
@tkelman tkelman closed this Jul 3, 2016
@tkelman tkelman reopened this Jul 3, 2016
@wildart
Copy link
Member Author

wildart commented Jul 5, 2016

I cannot figure out how the build system works. It builds libraries out of order, screwing everything.

@Keno
Copy link
Member

Keno commented Jul 5, 2016

Do you have the proper dependencies between targets?

@wildart
Copy link
Member Author

wildart commented Jul 5, 2016

I need to build them incrementally: http_parser & mbedtls -> libssh2 -> libgit2 -> mbedtlsstreams

@Keno
Copy link
Member

Keno commented Jul 5, 2016

It's a makefile script, you need to declare the appropriate dependencies.

@wildart
Copy link
Member Author

wildart commented Jul 6, 2016

What are STAGE*_DEPS variables for?

@Keno
Copy link
Member

Keno commented Jul 6, 2016

A library needs to be added to one of them in order to be built be default (or be the dependency of one mentioned in there). Not exactly sure why we have 3 of them.

@wildart wildart force-pushed the art/pkg-libs branch 4 times, most recently from 35c296c to e2d27b9 Compare July 7, 2016 16:08
@@ -66,7 +69,7 @@ before_install:
ln -s /usr/bin/gcc-5 $HOME/bin/x86_64-linux-gnu-gcc;
ln -s /usr/bin/g++-5 $HOME/bin/x86_64-linux-gnu-g++;
gcc --version;
BUILDOPTS="-j3 VERBOSE=1 FORCE_ASSERTIONS=1 LLVM_ASSERTIONS=1";
BUILDOPTS="VERBOSE=1 FORCE_ASSERTIONS=1 LLVM_ASSERTIONS=1";
Copy link
Contributor

Choose a reason for hiding this comment

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

needs to be reverted

@wildart
Copy link
Member Author

wildart commented Jul 7, 2016

x86_64 build gives me very strange error when cmake is executed: Re-run cmake no build system arguments. Then cmake is rerun without proper parameters which blows everything.

Any thoughts?

cd build/libssh2-a486c43e4a699d319a5afdb139545843769bcc0d/ && \

    cmake /home/travis/build/JuliaLang/julia/deps/srccache/libssh2-a486c43e4a699d319a5afdb139545843769bcc0d/ -DCMAKE_INSTALL_PREFIX:PATH=/home/travis/build/JuliaLang/julia/usr -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_C_COMPILER="gcc" -DCMAKE_C_COMPILER_ARG1="-m64 " -DCMAKE_CXX_COMPILER="g++" -DCMAKE_CXX_COMPILER_ARG1="-m64 " -DBUILD_SHARED_LIBS=ON -DBUILD_EXAMPLES=OFF -DCMAKE_PREFIX_PATH=/home/travis/build/JuliaLang/julia/usr -DCMAKE_INSTALL_RPATH=/home/travis/build/JuliaLang/julia/usr -DCMAKE_INSTALL_RPATH_USE_LINK_PATH=TRUE -DCMAKE_BUILD_TYPE=Release -DCRYPTO_BACKEND=mbedTLS -DENABLE_ZLIB_COMPRESSION=ON

Re-run cmake no build system arguments

-- Found mbedTLS:
--   version 2.2.1
--   TLS: /home/travis/build/JuliaLang/julia/usr/lib/libmbedtls.so
--   X509: /home/travis/build/JuliaLang/julia/usr/lib/libmbedx509.so
--   Crypto: /home/travis/build/JuliaLang/julia/usr/lib/libmbedcrypto.so
-- Found ZLIB: /usr/lib/x86_64-linux-gnu/libz.so (found version "1.2.3.4") 
-- 
-- The following features have been enabled:
 * Shared library , creating libssh2 as a shared library (.so/.dll)
 * Compression , using zlib for compression
 * diffie-hellman-group-exchange-sha1 , "new" diffie-hellman-group-exchange-sha1 method
-- The following REQUIRED packages have been found:
 * mbedTLS
 * ZLIB

-- The following features have been disabled:
 * "none" cipher
 * "none" MAC

 * Logging , Logging of execution with debug trace
-- Configuring done

-- The C compiler identification is GNU 5.4.0
-- Check for working C compiler: /home/travis/bin/gcc
-- Check for working C compiler: /home/travis/bin/gcc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Found OpenSSL: /usr/lib/x86_64-linux-gnu/libssl.so;/usr/lib/x86_64-linux-gnu/libcrypto.so (found version "1.0.1") 
-- Looking for EVP_aes_128_ctr
-- Looking for EVP_aes_128_ctr - found
....

Ref: https://travis-ci.org/JuliaLang/julia/jobs/143092080#L4339

@tkelman
Copy link
Contributor

tkelman commented Jul 7, 2016

How different would implementing the TLS stream registration in Julia look? If it would be shorter, it's likely it would be more maintainable as well.

@wildart
Copy link
Member Author

wildart commented Jul 7, 2016

It looks fairly simple: two streams one for http, another for https. Very mach as HttpServer.jl with MbedTLS.jl support, using data structures of libgit2. I'll look at this during weekend.

@Keno
Copy link
Member

Keno commented Jul 12, 2016

Ok, If I'm understanding correctly, the only problem here is the HTTPS support for generic linux binaries. I'm strongly in favor of dropping that as a release blocking item, changing this into only the things needed for the SSH support. Then, we can either ship openssl as we have been so far, or build a julia package on top of all of the existing packages to implement this. I really don't feel like maintaining a whole separate HTTPS client implementation in either C or Julia.

@tkelman
Copy link
Contributor

tkelman commented Jul 12, 2016

SSH support as a package would make sense to me, if it's even possible. Unless there are networks where https does not work at all for public packages?

@wildart wildart changed the title WIP: LibGit2 - SSH support binary dependencies WIP: LibGit2 - mbetTLS stream support Jul 12, 2016
@tkelman
Copy link
Contributor

tkelman commented Jul 12, 2016

We will need to be sure that libgit2 built against libssh built against the version of openssl that we currently use on Linux all works properly. Or build and distribute our own openssl from source.

On Windows I haven't tested this but I think unmodified libgit2 built against unmodified libssh built against the OS-native crypto should work, as long as we get the build system sorted (I only care about cross-compile from cygwin because that's how our windows binaries are built, msys2 is irrelevant and not at all release-blocking).

What's the situation on Mac, do we have to add openssl to our build system and only build it from source on Mac (and maybe Linux)? Or do we use the OS-native Apple crypto with the patched version of libssh, that needs a private header? As long as that doesn't interfere with our ability to build standalone distributable binaries I guess the private header thing isn't release blocking, It would be ideal to eventually get that crypto code reviewed and upstreamed to libssh eventually, but as long as it doesn't have glaring security holes (knock on wood) I guess we can live without that at first?

@wildart
Copy link
Member Author

wildart commented Jul 18, 2016

I'm closing this PR in favor of #17471

@wildart wildart closed this Jul 18, 2016
@tkelman tkelman deleted the art/pkg-libs branch July 29, 2016 01:01
@ViralBShah ViralBShah changed the title WIP: LibGit2 - mbetTLS stream support WIP: LibGit2 - mbedTLS stream support Aug 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:building Build system, or building Julia or its dependencies libgit2 The libgit2 library or the LibGit2 stdlib module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants