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

Better error message when handshaking fails #160

Closed
DenialAdams opened this issue Apr 3, 2018 · 5 comments
Closed

Better error message when handshaking fails #160

DenialAdams opened this issue Apr 3, 2018 · 5 comments

Comments

@DenialAdams
Copy link

DenialAdams commented Apr 3, 2018

When trying to understand why a TLS connection was failing, all I had to work with was an IoError(Kind(UnexpectedEof)) when I tried to write. I had to look on the serverside logs to see the reason:

Apr 03 02:22:43 murmur murmurd[24]: <W>2018-04-03 02:22:43.470 1 => <14:(-1)> Connection closed: Error during SSL handshake: error:1417A0C1:SSL routines:tls_post_process_client_hello:no shared cipher [13]

If rustls could have told me handshaking failed because of no shared cipher instead of merely giving me UnexpectedEof, it would have been helpful and certainly saved me some time.

I'm not familiar with the handshaking proces unfortunately, so if a detailed error message is just not possible for some reason, that's totally fair too.

@ctz
Copy link
Member

ctz commented Apr 4, 2018

I assume this is rustls as a client? The "no overlapping ciphersuites" error case is expected to work like this:

  • The server determines that this has happened, and signals that to the client by sending a TLS alert of type handshake_failure.
  • rustls receives that alert, and turns it into a TLSError: in this case that error will be rustls::TLSError::AlertReceived(rustls::internal::msgs::enums::AlertDescription::HandshakeFailure) which is returned from process_new_packets forever -- it's non-recoverable.

So, to clarify:

  • the only information a client has in this case is unfortunately just the handshake_failure alert.
  • it's tested in rustls that this alert-to-error promotion happens, which makes me think you are using rustls via an extra layer which isn't propagating errors well?

@DenialAdams
Copy link
Author

DenialAdams commented Apr 9, 2018

Thank you very much for the detailed response, it's much appreciated!

which makes me think you are using rustls via an extra layer which isn't propagating errors well?

I think this was my problem. I am using a rustls::Stream which of course only returns an io::Error on write, so it can't give me the HandshakeFailure, instead just an UnexpectedEof. I suppose that as we're bound by the Read and Write traits, we can't give detailed error messages here. This makes total sense, thinking about it now.

I guess this is what confused me (an amateur):

  • I have used rust-native-tls in the past, in which I used openssl::ssl::SslStream which in my mind was the equivalent of rustls::Stream. SslStream has (or maybe had) an explicit connect method before one could use it that peformed the handshaking and returned a HandshakeError if handshaking failed. I was expecting something like this and was a bit confused that I could just go ahead and issue a write.
  • After getting an UnexpectedEof when I tried to write, I didn't really know what that meant or what to look into.
  • After realizing it could be a handshake error, I looked for methods in rustls that could help me diagnose the issue but I only found is_handshaking, which always returned true (even after the handshake must have failed) which made me more confused as to the issue.

I'm not sure what you think the best way to fix these hangups is, if we should bother at all, but here are some ideas I have:

  • Improve the Stream docs to explain how establishing the connection works (done on first write? I still don't really know.) Also, explain that an UnexpectedEof could mean a handshake issue (and maybe other stuff it could signify?)
  • Clarify in docs that is_handshaking returns true even if handshaking has failed and we aren't really handshaking.
  • Add a procedure to Session that blocks until handshaking is completed and would return an error if there was a handshake failure. Alternatively/also, maybe provide something like completed_handshaking analagous to is_handshaking that returns true if the handshaking is finished.

As a novice in regards to both TLS and rustls, I may be totally barking up the wrong tree with these ideas. Let me know what you think, and feel free to close the issue. I'm willing to help out on implementing any improvements.

Thanks again.

EDIT:

I remembered another thing; I turned logging on (trace) but never saw any message about handshake failing. Is that something that could be logged?

@ctz
Copy link
Member

ctz commented Apr 29, 2018

Actually, this does appear to be a bug; albeit one I can't reproduce. Can you post your code?

You shouldn't get an UnexpectedEof when the handshake fails. Instead you should get something like Err(Custom { kind: InvalidData, error: AlertReceived(HandshakeFailure) }). In the meantime I've added tests to verify this.

@superlou
Copy link

superlou commented Jul 24, 2018

I'm trying to connect to a Mumble server at localhost:64738 using the following:

    let mut config = rustls::ClientConfig::new();
    config.root_store.add_server_trust_anchors(&webpki_roots::TLS_SERVER_ROOTS);

    let dns_name = webpki::DNSNameRef::try_from_ascii_str("localhost").unwrap();
    let mut sess = rustls::ClientSession::new(&Arc::new(config), dns_name);
    let mut sock = TcpStream::connect("localhost:64738").unwrap();
    let mut tls = rustls::Stream::new(&mut sess, &mut sock);

    let mut version_exchange = vec![0, 1, 2, 19];
    version_exchange.extend_from_slice("release\0os\0os_version\0".as_bytes());
    println!("Version Exchange: {:?}", version_exchange);
    tls.write(&version_exchange).unwrap();

I've verified that the standard Mumble client can connect, however I get a panic:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Kind(UnexpectedEof)', libcore/result.rs:945:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

This reproduces @DenialAdams's issue. In the Mumble log it indicates that there is no shared cipher suite. Is there a ClientConfig required to avoid this, or is there really no implemented cipher suite in rustls that is available in the Mumble server. The mumble defaults are described as "EECDH+AESGCM:EDH+aRSA+AESGCM:DHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA:AES256-SHA:AES128-SHA."

Edit: It might simply have been that they didn't share any cipher suites. I stumbled on this issue and updated the Mumble server to 1.3 which now gives the error WebPKIError(UnknownIssuer) which makes more sense.

@crsaracco
Copy link

So I'm trying to do the same thing as the above posters (communicate to a Mumble server); is there any way to make it work with a 1.2.x server? Something I can add to the sslCiphers variable in /etc/mumble-server.ini?

My default for 1.2.18 seems to be:

sslCiphers=EECDH+AESGCM:AES256-SHA:AES128-SHA

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

No branches or pull requests

4 participants