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

rust-openssl 0.7 support #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

rust-openssl 0.7 support #23

wants to merge 1 commit into from

Conversation

jwilm
Copy link
Contributor

@jwilm jwilm commented Apr 8, 2016

The HTTP/2 spec requires the TLS application-layer protocol negotiation
(ALPN) extension from the TLS library. This was added to openssl 1.0.2
which became available in rust-openssl 0.7.

I would have submitted this sooner, but the feature was blocked on SslStream not being Send in early patch versions of rust-openssl 0.7.x.


This change is Reviewable

optional = true

[features]
live_tests = []
tls = ["openssl", "openssl/tlsv1_2", "openssl/npn"]
tls = ["openssl", "openssl/tlsv1_2", "openssl/npn", "openssl/alpn"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, maybe the npn feature isn't needed now.

Copy link
Owner

Choose a reason for hiding this comment

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

yeah, looks like it should work from openssl's side, so let's drop npn altogether here.

@jwilm
Copy link
Contributor Author

jwilm commented Apr 8, 2016

One other thought - this should probably be considered a breaking change since OpenSSL 1.0.2 is required where 1.0.1 sufficed previously.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.606% when pulling 0b74aeb on jwilm:openssl-0.7 into 90b666b on mlalic:master.

@mlalic
Copy link
Owner

mlalic commented Apr 25, 2016

Back when I first opted to use NPN, out of the sites that did support HTTP/2 (or h2-14 at the time), not many supported ALPN. Has this changed by now?

@jwilm
Copy link
Contributor Author

jwilm commented Apr 25, 2016

I don't know about sites not supporting ALPN, but some sites require it. The spec also says ALPN explicitly without mention of NPN.

@mlalic
Copy link
Owner

mlalic commented Apr 25, 2016

Yeah, ALPN is the only way that the standard mentions, but for historic reasons (http/2 being an evolution/standardization of spdy) NPN was how you could get to http/2, because it was what you originally used for spdy... :)

I prefer having only ALPN here though, so if it's already at the point where some sites don't even recognize NPN, I'm good with this change.

The HTTP/2 spec requires the TLS application-layer protocol negotiation
(ALPN) extension from the TLS library. This was added to openssl 1.0.2
which became available in rust-openssl 0.7.

NPN support is removed since ALPN superceded it in the spec, and some
servers don't even support it.
@jwilm
Copy link
Contributor Author

jwilm commented May 30, 2016

@mlalic sorry for the delay on updating this! It should be ready now pending appveyor results.

@awalcutt
Copy link

What's the status of this project? I'm working with Amazon's Alexa Voice Service which uses HTTP/2 and is rejecting NPN. Right now I am depending on jwilm's branch which works for me with openssl1.0.2g but it would be great to have this working in the crate. Apologies if this isn't the right forum for this question...new to Rust and open source contribution in general.

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

Successfully merging this pull request may close these issues.

4 participants