Incorporate rust-security-framework as backend for hyper::net::Ssl

[frewsxcv]

https://github.com/sfackler/rust-security-framework

A potential remedy for #709, servo/servo#7930, servo/servo#7888

Relevant conversation on #rust today with @seanmonstar and @sfackler:

10:54 PM sfackler: is there any reason why the hyper openssl backend couldn't be rewritten in rust-security-framework?
10:55 PM frewsxcv: hyper has an Ssl trait that allows other ssl impls
10:56 PM seanmonstar: yeah, am looking at it now. it's more of a question about rust-security-framework and less-so about hyper
10:56 PM — frewsxcv has never looked at OSX's Security Framework docs until now
11:15 PM seanmonstar: hyper's Ssl trait unfortunately doesn't play super nice with non-Openssl APIs. I've been meaning to file an issue about it, but it'd basically consist of splitting it into one trait for clients and one for servers
11:29 PM in something like secure transport, you create an SslContext for each connection, and have to pick at construction time if it's client or server side
11:29 PM there are then some builders to help cache configuration for client and server side stuff and handle some weird handshake logic: http://sfackler.github.io/rust-security-framework/doc/v0.1.3/security_framework/secure_transport/struct.ClientBuilder.html
11:29 PM but there isn't a unified one for servers and clients
11:30 PM sfackler: so the 1 associated Stream type is the problem?
11:30 PM no, not that, the end result is an SslStream in both cases
11:30 PM the issue is that if you want to run clients, you're going to configure a ClientBuilder, but you then can't make server-side SslStreams
11:30 PM so half of the Ssl trait has to unconditionally fail
11:31 PM same for the server side
11:36 PM sfackler: so currently, a context would be an enum, and if it's ServerCtx when wrap_client is called, it'd return an Err (or panic)?
11:37 PM probably just separate structs for the client and server side, but either way, yeah, one of the two wrap methods would not work
11:56 PM sfackler: why not just create two separate structs within the would-be 'security_framework' submodule within src/net.rs?
11:57 PM frewsxcv: I mean that's what would happen, but each of those structs could only actually support half of the trait they have to implement
11:59 PM oh, i understand
12:01 AM It'd work with an enum, but maybe it'd be better to split the traits...
12:01 AM Ssl: SslServer + SslClient ?
12:01 AM wouldn't that also be backwards compatible?
12:02 AM you couldn't use SslServer or SslClient in hyper apis though
12:03 AM oh maybe blanket impls would let you
12:03 AM seems like it should

[frewsxcv]

Note to selves: if this ever happens, we should enable OSX on Travis CI.

[seanmonstar]

Note that while the current traits require the Stream to be Clone, this is not necessary on the mio branch.

[sfackler]

Looks like this is blocked until the Clone requirement is dropped.

[frewsxcv]

Update: this is now merged in #762, but OpenSSL is still the default on OS X. In a future breaking change release, the default should probably switch to security-framework? Then #709 could probably be closed (if not already).

On a side note, @sfackler started https://github.com/sfackler/rust-native-tls which is Yet Another Layer Of Abstraction that wraps around the system native TLS bindings. So Hyper could potentially use this to offload system TLS logic.

[seanmonstar]

I was wondering, is it possible to conditionally set the default based on
OS? Or would that require a separate crate to do it?

On Sun, Apr 17, 2016, 11:02 AM Corey Farwell notifications@github.com
wrote:

Update: this is now merged in #762
#762, but OpenSSL is still the
default on OS X

hyper/Cargo.toml

Line 52 in 635622c

ssl = ["openssl", "cookie/secure"]

.
In a future breaking change release, the default should probably switch to
security-framework? Then #709
#709 could probably be closed
(if not already).

On a side note, @sfackler https://github.com/sfackler started
https://github.com/sfackler/rust-native-tls which is Yet Another Layer Of
Abstraction that wraps around the system native TLS bindings. So Hyper
could potentially use this to offload system TLS logic.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#755 (comment)

[frewsxcv]

I was wondering, is it possible to conditionally set the default based on
OS?

You can probably accomplish that with something like this

[seanmonstar]

@frewsxcv yea, that's what I meant by using a separate crate. I'm not sure I can use those directives, and also have it be a feature that you can turn on or off.

[seanmonstar]

This is has landed, so closing.