Merge pull request #266 from jmesmon/alpn

ssl/npn+alpn: adjust protocol selection to fail if no protocols match

2 files changed, 51 insertions, 5 deletions

diff --git a/openssl/src/ssl/mod.rs b/openssl/src/ssl/mod.rs
index 20c9234c..3adf89e6 100644
--- a/openssl/src/ssl/mod.rs
+++ b/openssl/src/ssl/mod.rs
@@ -308,8 +308,11 @@ unsafe fn select_proto_using(ssl: *mut ffi::SSL,
         let client_len = protocols.len() as c_uint;
         // Finally, let OpenSSL find a protocol to be used, by matching the given server and
         // client lists.
-        ffi::SSL_select_next_proto(out, outlen, inbuf, inlen, client, client_len);
-        ffi::SSL_TLSEXT_ERR_OK
+        if ffi::SSL_select_next_proto(out, outlen, inbuf, inlen, client, client_len) != ffi::OPENSSL_NPN_NEGOTIATED {
+            ffi::SSL_TLSEXT_ERR_NOACK
+        } else {
+            ffi::SSL_TLSEXT_ERR_OK
+        }
 }
 
 /// The function is given as the callback to `SSL_CTX_set_next_proto_select_cb`.
diff --git a/openssl/src/ssl/tests.rs b/openssl/src/ssl/tests.rs
index 9198a642..344bcfe8 100644
--- a/openssl/src/ssl/tests.rs
+++ b/openssl/src/ssl/tests.rs
@@ -403,7 +403,7 @@ fn test_state() {
     assert_eq!(stream.get_state_string_long(), "SSL negotiation finished successfully");
 }
 
-/// Tests that connecting with the client using NPN, but the server not does not
+/// Tests that connecting with the client using ALPN, but the server not does not
 /// break the existing connection behavior.
 #[test]
 #[cfg(feature = "alpn")]
@@ -420,7 +420,7 @@ fn test_connect_with_unilateral_alpn() {
         Ok(stream) => stream,
         Err(err) => panic!("Expected success, got {:?}", err)
     };
-    // Since the socket to which we connected is not configured to use NPN,
+    // Since the socket to which we connected is not configured to use ALPN,
     // there should be no selected protocol...
     assert!(stream.get_selected_alpn_protocol().is_none());
 }
@@ -453,7 +453,7 @@ fn test_connect_with_unilateral_npn() {
 #[cfg(feature = "alpn")]
 fn test_connect_with_alpn_successful_multiple_matching() {
     // A different port than the other tests: an `openssl` process that has
-    // NPN enabled.
+    // ALPN enabled.
     let stream = TcpStream::connect("127.0.0.1:15419").unwrap();
     let mut ctx = SslContext::new(Sslv23).unwrap();
     ctx.set_verify(SSL_VERIFY_PEER, None);
@@ -628,6 +628,49 @@ fn test_alpn_server_advertise_multiple() {
     assert_eq!(b"spdy/3.1", stream.get_selected_alpn_protocol().unwrap());
 }
 
+/// Test that Servers supporting ALPN don't report a protocol when none of their protocols match
+/// the client's reported protocol.
+#[test]
+#[cfg(feature = "alpn")]
+fn test_alpn_server_select_none() {
+    let localhost = "127.0.0.1:15422";
+    let listener = TcpListener::bind(localhost).unwrap();
+    // We create a different context instance for the server...
+    let listener_ctx = {
+        let mut ctx = SslContext::new(Sslv23).unwrap();
+        ctx.set_verify(SSL_VERIFY_PEER, None);
+        ctx.set_alpn_protocols(&[b"http/1.1", b"spdy/3.1"]);
+        assert!(ctx.set_certificate_file(
+                &Path::new("test/cert.pem"), X509FileType::PEM).is_ok());
+        ctx.set_private_key_file(
+                &Path::new("test/key.pem"), X509FileType::PEM).unwrap();
+        ctx
+    };
+    // Have the listener wait on the connection in a different thread.
+    thread::spawn(move || {
+        let (stream, _) = listener.accept().unwrap();
+        let _ = SslStream::accept(&listener_ctx, stream).unwrap();
+    });
+
+    let mut ctx = SslContext::new(Sslv23).unwrap();
+    ctx.set_verify(SSL_VERIFY_PEER, None);
+    ctx.set_alpn_protocols(&[b"http/2"]);
+    match ctx.set_CA_file(&Path::new("test/cert.pem")) {
+        Ok(_) => {}
+        Err(err) => panic!("Unexpected error {:?}", err)
+    }
+    // Now connect to the socket and make sure the protocol negotiation works...
+    let stream = TcpStream::connect(localhost).unwrap();
+    let stream = match SslStream::new(&ctx, stream) {
+        Ok(stream) => stream,
+        Err(err) => panic!("Expected success, got {:?}", err)
+    };
+
+    // Since the protocols from the server and client don't overlap at all, no protocol is selected
+    assert_eq!(None, stream.get_selected_alpn_protocol());
+}
+
+
 #[cfg(feature="dtlsv1")]
 #[cfg(test)]
 mod dtlsv1 {