search

sfackler / rust-native-tls

Apache License 2.0
478 stars 201 forks source link

Alpn Support #194

Closed MarnixKuijs closed 3 years ago

MarnixKuijs commented 3 years ago

This is an exact copy of #165, but this one is merge with upstream, so all the credit should go to the original author. I really need ALPN support in native-tls. Sadly the original authors of #165 and #157 don't seem to be actively working on this anymore, so that is why I open this PR so I can get ALPN support in.

Closes #49.

MarnixKuijs commented 3 years ago

request_alpns now takes a &[&str] instead of &[&[u8]] and the builder uses Vec<String> instead of Vec<Vec<u8>>

sfackler commented 3 years ago

LGTM other than those last few questions!

MarnixKuijs commented 3 years ago

Looks like the rustdoc on the readme failed, not due to my changes, but more due to it keeping old artifacts, a clean of the CI should help.

cyang1 commented 3 years ago

I'm still around :), the original PR just didn't get any feedback unfortunately.

To add some context to the comments here -- IMO request_alpns ought to take &[u8] and not &str, as RFC 7301 defines the ALPN identification sequence as any arbitrary sequence of bytes:

Identification Sequence: The precise set of octet values that
      identifies the protocol.

This is supported by RFC 8701, which declares several non-UTF8 ALPN sequences for testing and extensibility purposes.

It's unfortunate that the Security.framework API is incorrect on this, but IMO not a reason to propagate this incorrect typing upwards, or make it impossible to specify non-UTF8 ALPNs on other backends.

Also re:

This will introduce a lower bound of macOS 10.13 - I wonder if it should be flagged as a Cargo feature in case people want to support older versions?

The OSX_10_13 feature on rust-security-framework implies alpn, but alpn doesn't inherently have any requirements. rust-security-framework will dynamically load the symbol if compiled without targeting macOS 10.13 or above.

https://github.com/kornelski/rust-security-framework/blob/c2897426a0d42b8833cb6bf943441a225817e0fd/security-framework/src/secure_transport.rs#L725-L733