From 7ac05996388af40e78696c5ed2d9e0426eea1881 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Fri, 14 Oct 2016 21:54:53 -0700 Subject: [PATCH] Fix test_alpn_server_select_none In OpenSSL 1.1, a failure to negotiate a protocol is a fatal error, so fork that test. This also popped up an issue where we assumed all errors had library, function, and reason strings which is not necessarily the case. While we're in here, adjust the Display impl to match what OpenSSL prints out. Closes #465 --- openssl-sys/src/lib.rs | 12 +++++ openssl/src/error.rs | 96 ++++++++++++++++++++---------------- openssl/src/ssl/tests/mod.rs | 46 ++++++++++++----- 3 files changed, 100 insertions(+), 54 deletions(-) diff --git a/openssl-sys/src/lib.rs b/openssl-sys/src/lib.rs index 300ed056..026d6e6b 100644 --- a/openssl-sys/src/lib.rs +++ b/openssl-sys/src/lib.rs @@ -318,6 +318,18 @@ pub unsafe fn SSL_set_tlsext_host_name(s: *mut SSL, name: *mut c_char) -> c_long name as *mut c_void) } +pub fn ERR_GET_LIB(l: c_ulong) -> c_int { + ((l >> 24) & 0x0FF) as c_int +} + +pub fn ERR_GET_FUNC(l: c_ulong) -> c_int { + ((l >> 12) & 0xFFF) as c_int +} + +pub fn ERR_GET_REASON(l: c_ulong) -> c_int { + (l & 0xFFF) as c_int +} + extern { pub fn ASN1_INTEGER_set(dest: *mut ASN1_INTEGER, value: c_long) -> c_int; pub fn ASN1_STRING_type_new(ty: c_int) -> *mut ASN1_STRING; diff --git a/openssl/src/error.rs b/openssl/src/error.rs index f54d7bda..4dd219af 100644 --- a/openssl/src/error.rs +++ b/openssl/src/error.rs @@ -76,39 +76,79 @@ impl Error { } /// Returns the raw OpenSSL error code for this error. - pub fn error_code(&self) -> c_ulong { + pub fn code(&self) -> c_ulong { self.0 } - /// Returns the name of the library reporting the error. - pub fn library(&self) -> &'static str { - get_lib(self.0) + /// Returns the name of the library reporting the error, if available. + pub fn library(&self) -> Option<&'static str> { + unsafe { + let cstr = ffi::ERR_lib_error_string(self.0); + if cstr.is_null() { + return None; + } + let bytes = CStr::from_ptr(cstr as *const _).to_bytes(); + Some(str::from_utf8(bytes).unwrap()) + } } /// Returns the name of the function reporting the error. - pub fn function(&self) -> &'static str { - get_func(self.0) + pub fn function(&self) -> Option<&'static str> { + unsafe { + let cstr = ffi::ERR_func_error_string(self.0); + if cstr.is_null() { + return None; + } + let bytes = CStr::from_ptr(cstr as *const _).to_bytes(); + Some(str::from_utf8(bytes).unwrap()) + } } /// Returns the reason for the error. - pub fn reason(&self) -> &'static str { - get_reason(self.0) + pub fn reason(&self) -> Option<&'static str> { + unsafe { + let cstr = ffi::ERR_reason_error_string(self.0); + if cstr.is_null() { + return None; + } + let bytes = CStr::from_ptr(cstr as *const _).to_bytes(); + Some(str::from_utf8(bytes).unwrap()) + } } } impl fmt::Debug for Error { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - fmt.debug_struct("Error") - .field("library", &self.library()) - .field("function", &self.function()) - .field("reason", &self.reason()) - .finish() + let mut builder = fmt.debug_struct("Error"); + builder.field("code", &self.code()); + if let Some(library) = self.library() { + builder.field("library", &library); + } + if let Some(function) = self.function() { + builder.field("function", &function); + } + if let Some(reason) = self.reason() { + builder.field("reason", &reason); + } + builder.finish() } } impl fmt::Display for Error { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - fmt.write_str(&self.reason()) + try!(write!(fmt, "error:{:08X}", self.0)); + match self.library() { + Some(l) => try!(write!(fmt, ":{}", l)), + None => try!(write!(fmt, ":lib({})", ffi::ERR_GET_LIB(self.0))), + } + match self.function() { + Some(f) => try!(write!(fmt, ":{}", f)), + None => try!(write!(fmt, ":func({})", ffi::ERR_GET_FUNC(self.0))), + } + match self.reason() { + Some(r) => write!(fmt, ":{}", r), + None => write!(fmt, ":reason({})", ffi::ERR_GET_FUNC(self.0)), + } } } @@ -117,31 +157,3 @@ impl error::Error for Error { "An OpenSSL error" } } - -fn get_lib(err: c_ulong) -> &'static str { - unsafe { - let cstr = ffi::ERR_lib_error_string(err); - assert!(!cstr.is_null(), "bad lib: {}", err); - let bytes = CStr::from_ptr(cstr as *const _).to_bytes(); - str::from_utf8(bytes).unwrap() - } -} - -fn get_func(err: c_ulong) -> &'static str { - unsafe { - let cstr = ffi::ERR_func_error_string(err); - assert!(!cstr.is_null(), "bad func: {}", err); - let bytes = CStr::from_ptr(cstr as *const _).to_bytes(); - str::from_utf8(bytes).unwrap() - } -} - -fn get_reason(err: c_ulong) -> &'static str { - unsafe { - let cstr = ffi::ERR_reason_error_string(err); - assert!(!cstr.is_null(), "bad reason: {}", err); - let bytes = CStr::from_ptr(cstr as *const _).to_bytes(); - str::from_utf8(bytes).unwrap() - } -} - diff --git a/openssl/src/ssl/tests/mod.rs b/openssl/src/ssl/tests/mod.rs index b3500105..ce1ba8ca 100644 --- a/openssl/src/ssl/tests/mod.rs +++ b/openssl/src/ssl/tests/mod.rs @@ -726,10 +726,7 @@ fn test_alpn_server_advertise_multiple() { /// Test that Servers supporting ALPN don't report a protocol when none of their protocols match /// the client's reported protocol. #[test] -#[cfg(feature = "openssl-102")] -// TODO: not sure why this test is failing on OpenSSL 1.1.0, may be related to -// something about SSLv3 though? -#[cfg_attr(ossl110, ignore)] +#[cfg(all(feature = "openssl-102", ossl102))] fn test_alpn_server_select_none() { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let localhost = listener.local_addr().unwrap(); @@ -753,21 +750,46 @@ fn test_alpn_server_select_none() { let mut ctx = SslContext::new(Tls).unwrap(); ctx.set_verify(SSL_VERIFY_PEER); ctx.set_alpn_protocols(&[b"http/2"]); - match ctx.set_CA_file(&Path::new("test/root-ca.pem")) { - Ok(_) => {} - Err(err) => panic!("Unexpected error {:?}", err), - } + ctx.set_CA_file(&Path::new("test/root-ca.pem")).unwrap(); // Now connect to the socket and make sure the protocol negotiation works... let stream = TcpStream::connect(localhost).unwrap(); - let stream = match SslStream::connect(&ctx, stream) { - Ok(stream) => stream, - Err(err) => panic!("Expected success, got {:?}", err), - }; + let stream = SslStream::connect(&ctx, stream).unwrap(); // Since the protocols from the server and client don't overlap at all, no protocol is selected assert_eq!(None, stream.ssl().selected_alpn_protocol()); } +// In 1.1.0, ALPN negotiation failure is a fatal error +#[test] +#[cfg(all(feature = "openssl-102", ossl110))] +fn test_alpn_server_select_none() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let localhost = listener.local_addr().unwrap(); + // We create a different context instance for the server... + let listener_ctx = { + let mut ctx = SslContext::new(Tls).unwrap(); + ctx.set_verify(SSL_VERIFY_PEER); + 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(); + assert!(SslStream::accept(&listener_ctx, stream).is_err()); + }); + + let mut ctx = SslContext::new(Tls).unwrap(); + ctx.set_verify(SSL_VERIFY_PEER); + ctx.set_alpn_protocols(&[b"http/2"]); + ctx.set_CA_file(&Path::new("test/root-ca.pem")).unwrap(); + // Now connect to the socket and make sure the protocol negotiation works... + let stream = TcpStream::connect(localhost).unwrap(); + assert!(SslStream::connect(&ctx, stream).is_err()); +} #[cfg(test)] mod dtlsv1 {