Use SecureTransport on OS X #2997

carlosmn · 2015-03-18T23:22:15Z

Apple provides a TLS communication library which is included with the OS, thead-safe by itself and generally a better sport than OpenSSL.

Custom certificate checking is still not implemented, but the other clone tests do work.

Get certificate extraction working
Better message in CMake about finding the frameworks
Rename GIT_SSL to GIT_OPENSSL as that's what it really means
Don't fail a handshake automatically on bad certificate

carlosmn · 2015-03-18T23:51:05Z

I'm pretty happy about not linking against OpenSSL at all.

% otool -L libgit2.dylib                                     
libgit2.dylib:
        /Users/carlos/git/libgit2/libgit2.22.dylib (compatibility version 22.0.0, current version 0.22.0)
        /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.5)
        /System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 55471.14.27)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 855.17.0)
        /usr/local/lib/libssh2.1.dylib (compatibility version 2.0.0, current version 2.1.0)
        /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1197.1.1)

carlosmn · 2015-03-19T02:18:34Z

Therzok · 2015-03-19T05:34:48Z

So many ❤️ for you.

carlosmn · 2015-03-19T09:55:58Z

There is still of course libssh2 which will still link against openssl, so we're not quite free of threading nonsense yet, but someone was trying to upstream a SecureTransport version.

ethomson · 2015-03-19T13:31:56Z

Man, I'm excited about having this. Very clean implementation.

Therzok · 2015-03-19T13:36:38Z

carlosmn · 2015-03-23T06:43:42Z

@Therzok @nulltoken btw you should be able to write an implementation of the git_stream interface in C# so you use a managed TLS stream and don't have to worry about what's available in the OS since the runtime would take care of it (part of the reason for writing this layer was to allow making the use of OpenSSL by libgit2 irrelevant by letting the bindings bypass it).

Therzok · 2015-03-23T09:32:17Z

@nulltoken Should we make the magic happen?

Therzok · 2015-03-23T09:40:52Z

@carlosmn I can see the definition, but can't find where to hook it. Any tips?

carlosmn · 2015-03-23T09:44:31Z

That function doesn't exist yet, as that depends on git_tls_stream_new() abstracting away the actual implementation first, which I only got around to in this PR.

My idea was to have a function like git_tls_stream_set() (or better name) which would set some global variable and then git_tls_stream_new() would look into that variable and call your constructor instead of the OpenSSL or SecureTransport one.

Therzok · 2015-03-23T09:56:06Z

Lgtm 👍

git_tls_stream_set_factory()?

ethomson · 2015-04-17T17:23:08Z

src/stransport_stream.c

+		return -1;
+	}
+
+	if ((ret = SSLCopyPeerTrust(st->ctx, &trust)) != noErr)


If this fails, is it possible for trust to remain unset (and then we subsequently try to CFRelease it at line 81)? Which is to say should trust have a default?

Yep, it should be SecTrustRef trust = NULL.

ethomson · 2015-04-17T17:25:15Z

This looks great to me (aside from that one question). VERY EXCITED FOR THIS.

As an alternative to OpenSSL when we're on OS X. This one can actually take advantage of stacking the streams.

Instead, provide git_tls_stream_new() to ask for the most appropriate encrypted stream and use it in our HTTP transport.

This is what it's meant all along, but now we actually have multiple implementations, it's clearer to use the name of the library.

Do not automatically fail on a bad certificate, but let the caller decide. This means we don't need our switch on errors anymore but can return a string representation from the security framework.

Anything SSL is deprecated. Let's make sure we don't try to use SSL v3 when talking to the server.

On close, we might get a return code which looks like an error but just means that the other side closed gracefully. Handle that.

ethomson · 2015-04-23T19:21:43Z

ethomson · 2015-04-23T19:23:26Z

ethomson · 2015-04-23T19:23:57Z

Use SecureTransport on OS X

ethomson · 2015-04-23T19:24:35Z

We have successfully eliminated the scourge of OpenSSL on another platform. All thanks be to @carlosmn .

carlosmn force-pushed the cmn/secure-transport branch from 5d46db3 to 41def48 Compare March 18, 2015 23:49

carlosmn mentioned this pull request Mar 18, 2015

Fix iconv search when MacPorts is installed #2933

Closed

carlosmn force-pushed the cmn/secure-transport branch from 41def48 to 4815872 Compare March 19, 2015 01:31

carlosmn force-pushed the cmn/secure-transport branch from ad9c044 to 8411b5b Compare March 19, 2015 03:30

carlosmn force-pushed the cmn/secure-transport branch 2 times, most recently from 4c7c9d4 to a077587 Compare March 19, 2015 09:03

carlosmn force-pushed the cmn/secure-transport branch 2 times, most recently from 343d202 to f9b38af Compare March 19, 2015 11:23

carlosmn changed the title ~~[WIP] Use SecureTransport on OS X~~ Use SecureTransport on OS X Mar 19, 2015

carlosmn force-pushed the cmn/secure-transport branch from 62cd829 to a8c64a9 Compare March 21, 2015 06:58

carlosmn force-pushed the cmn/secure-transport branch from a8c64a9 to 3a49e3a Compare March 24, 2015 15:32

carlosmn force-pushed the cmn/secure-transport branch 3 times, most recently from 262dcf5 to b50f0cd Compare April 13, 2015 14:16

ethomson reviewed Apr 17, 2015
View reviewed changes

carlosmn force-pushed the cmn/secure-transport branch from b50f0cd to a6c23c0 Compare April 18, 2015 15:56

carlosmn added 2 commits April 23, 2015 17:39

Add a SecureTransport TLS channel

6bb54cb

As an alternative to OpenSSL when we're on OS X. This one can actually take advantage of stacking the streams.

Abstract away the TLS stream implementation

6946a3b

Instead, provide git_tls_stream_new() to ask for the most appropriate encrypted stream and use it in our HTTP transport.

carlosmn added 2 commits April 23, 2015 17:39

Silence unused warnings when not using OpenSSL

70b852c

Rename GIT_SSL to GIT_OPENSSL

24e53d2

This is what it's meant all along, but now we actually have multiple implementations, it's clearer to use the name of the library.

carlosmn force-pushed the cmn/secure-transport branch from a6c23c0 to a93db93 Compare April 23, 2015 15:40

carlosmn added 4 commits April 23, 2015 17:43

SecureTransport: allow overriding a bad certificate

b7e1c81

Do not automatically fail on a bad certificate, but let the caller decide. This means we don't need our switch on errors anymore but can return a string representation from the security framework.

Update THREADING and CHANGELOG with SecureTransport details

85247df

SecureTransport: require TLS v1.x

65ac7dd

Anything SSL is deprecated. Let's make sure we don't try to use SSL v3 when talking to the server.

SecureTransport: handle graceful closes

44b769e

On close, we might get a return code which looks like an error but just means that the other side closed gracefully. Handle that.

carlosmn force-pushed the cmn/secure-transport branch from a93db93 to 44b769e Compare April 23, 2015 15:43

ethomson added a commit that referenced this pull request Apr 23, 2015

Merge pull request #2997 from libgit2/cmn/secure-transport

dbb4595

Use SecureTransport on OS X

ethomson merged commit dbb4595 into master Apr 23, 2015

carlosmn deleted the cmn/secure-transport branch April 26, 2015 00:53

joshaber mentioned this pull request Apr 28, 2015

Update libssh2 submodule url libgit2/objective-git#461

Merged

carlosmn mentioned this pull request May 4, 2015

Don't force use of iconv on Darwin #2918

Closed

phatblat mentioned this pull request Aug 8, 2015

Cloning with a cert fails on Mac because objective-git is not built with OpenSSL libgit2/objective-git#499

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use SecureTransport on OS X #2997

Use SecureTransport on OS X #2997

carlosmn commented Mar 18, 2015

carlosmn commented Mar 18, 2015

carlosmn commented Mar 19, 2015

Therzok commented Mar 19, 2015

carlosmn commented Mar 19, 2015

ethomson commented Mar 19, 2015

Therzok commented Mar 19, 2015

carlosmn commented Mar 23, 2015

Therzok commented Mar 23, 2015

Therzok commented Mar 23, 2015

carlosmn commented Mar 23, 2015

Therzok commented Mar 23, 2015

ethomson Apr 17, 2015

carlosmn Apr 18, 2015

ethomson commented Apr 17, 2015

ethomson commented Apr 23, 2015

ethomson commented Apr 23, 2015

ethomson commented Apr 23, 2015

ethomson commented Apr 23, 2015

Use SecureTransport on OS X #2997

Use SecureTransport on OS X #2997

Conversation

carlosmn commented Mar 18, 2015

carlosmn commented Mar 18, 2015

carlosmn commented Mar 19, 2015

Therzok commented Mar 19, 2015

carlosmn commented Mar 19, 2015

ethomson commented Mar 19, 2015

Therzok commented Mar 19, 2015

carlosmn commented Mar 23, 2015

Therzok commented Mar 23, 2015

Therzok commented Mar 23, 2015

carlosmn commented Mar 23, 2015

Therzok commented Mar 23, 2015

ethomson Apr 17, 2015

Choose a reason for hiding this comment

carlosmn Apr 18, 2015

Choose a reason for hiding this comment

ethomson commented Apr 17, 2015

ethomson commented Apr 23, 2015

ethomson commented Apr 23, 2015

ethomson commented Apr 23, 2015

ethomson commented Apr 23, 2015