OSX fixes

[carlosmn]

The OpenSSL version on OSX (at least on Mavericks) does not have the tls version method functions, so we need to detect OSX there. It doesn't look like there's a good way to put this into the compat file without adding yet another layer of indirection, so I put it directly in the ifdef.

The second commit is just to shut up the compiler. I already know Apple want me to use SecureTransport, I don't need it hiding any useful warnings or errors from my code.

The last one shouldn't be OSX-specific, but newer Go versions have gotten quite picky about C pointer types, so this makes them agree completely. I'm on 1.4 currently but I've seen similar issues on 1.3.

[thepaul]

other two commits look good to me

[carlosmn]

The OpensSSL library itself does not expose this function, so this library does not put on any additional restrictions. grepping for SSL_OP_NO_ in /usr/include/openssl/ does not reveal any way to turn off the later versions of TLS, so I don't see that it does support it.

It is however possible to use TLSv1.1 and v1.2 with this if it does support the later TLS versions, which some bits of the headers do know about.

The SSLv23_method() function loads v1.0, v1.1 and v1.2 along with with SSL. You can (should, really) then disable SSL. This is AFAIK the only portable way to select TLS across OpenSSL versions. This also activates the HELLO compatibility where you can use an SSL HELLO but use TLS for the actual data.

From the OpenSSL documentation https://www.openssl.org/docs/ssl/SSLv23_method.html

The list of protocols available can later be limited using the SSL_OP_NO_SSLv3, SSL_OP_NO_TLSv1, SSL_OP_NO_TLSv1_1 and SSL_OP_NO_TLSv1_2 options of the SSL_CTX_set_options() or SSL_set_options() functions. Using these options it is possible to choose e.g. SSLv23_server_method() and be able to negotiate with all possible clients, but to only allow newer protocols like TLSv1, TLSv1.1 or TLS v1.2.

Applications which never want to support SSLv3 can set SSL_OP_NO_SSLv3.

[calavera]

@thepaul is there anything else missing to merge this?

[thepaul]

Ok, it looks like you are right. Using AnyVersion and subsequently setting NoSSLv2 | NoSSLv3 | NoTLSv1is probably the best approach for users of the library.

I'll let @jtolds / @zeebo make the call on whether it makes sense still to provide the TLSv1_1 and TLSv1_2 options for NewCtxWithVersion() on Mac, and simulate the same effect with SSLv23_method() and options as appropriate-- or whether it's better to be a maximally thin wrapper around OpenSSL.

[calavera]

@jtolds / @zeebo would you mind to comment on this?

[jtolio]

i'm so sorry for the delay. last commit on december 12th! aside from thepaul being on top of things we are so bad at this.

yeah i'm fine with merging this as-is. If you're using NewCtxWithVersion you are free to shoot yourself in the foot. NewCtx is supposed to do the right thing by default, but it doesn't do NoTLSv1. Should it?

Right now this is what we have as default behavior for everyone:

// NewCtx creates a context that supports any TLS version 1.0 and newer.
func NewCtx() (*Ctx, error) {
    c, err := NewCtxWithVersion(AnyVersion)
    if err == nil {
        c.SetOptions(NoSSLv2 | NoSSLv3)
    }
    return c, err
}

I'd be fine with making this require TLS 1.1 or newer too, but that's also fine to be a different PR, so i'm just going to merge this and try not to be too embarrassed for how long it took

[calavera]

Thanks @jtolds :tada: