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

TLSException doesn't cover some cases (1.3.11) #250

Closed
boris-stepanov opened this issue Oct 4, 2017 · 4 comments
Closed

TLSException doesn't cover some cases (1.3.11) #250

boris-stepanov opened this issue Oct 4, 2017 · 4 comments

Comments

@boris-stepanov
Copy link

boris-stepanov commented Oct 4, 2017

Hello and sorry for my French.
I found that handshake can raise at least one exception, that doesn't wrapped in TLSException. If client dropped connection without sending a byte then handshake on server side will raise "hPutBuf: resource vanished (Broken pipe)", that cannot be catched as TLSException. Here is the code:

$ tail server.hs
main = do
  srv <- listenOn (PortNumber 1234)
  (h, _, _) <- accept srv
  ctx <- contextNew h def
  res <- try $ handshake ctx :: IO (Either TLSException ())
  print res

First case (correct):

SERVER$  ./server
CLIENT$ echo 'incorrect data' | nc 127.0.0.1 1234
SERVER$ Left (HandshakeFailed (Error_Packet_Parsing "Failed reading: invalid header type: 105\nFrom:\theader\n\n"))

Second case (incorrect):

SERVER$  ./server
CLIENT$ nc 127.0.0.1 1234^C
SERVER$ server: <socket: 12>: hPutBuf: resource vanished (Broken pipe)
@boris-stepanov boris-stepanov changed the title TLSException doesn't cover some cases TLSException doesn't cover some cases (1.3.11) Oct 4, 2017
@ocheron
Copy link
Contributor

ocheron commented Oct 4, 2017

I don't reproduce this behavior and get expected Left (HandshakeFailed Error_EOF) instead.
However this is probably a second exception raised when handling Error_EOF.

@boris-stepanov could you test the following patch on top of tls-1.3.11 ?

diff --git a/core/Network/TLS/Handshake.hs b/core/Network/TLS/Handshake.hs
index f232a31..db7bd66 100644
--- a/core/Network/TLS/Handshake.hs
+++ b/core/Network/TLS/Handshake.hs
@@ -33,5 +33,5 @@ handshake ctx =
   where handleException f = catchException f $ \exception -> do
             let tlserror = maybe (Error_Misc $ show exception) id $ fromException exception
             setEstablished ctx False
-            sendPacket ctx (errorToAlert tlserror)
+            catchException (sendPacket ctx (errorToAlert tlserror)) (\_ -> return ())
             handshakeFailed tlserror

@boris-stepanov
Copy link
Author

boris-stepanov commented Oct 5, 2017

I tested this on lts-9.2 and lts-9.6 resolvers from stackage, both contain tls-1.3.11. Furthermore, I just fetched hs-tls, and cannot see catchException (sendPacket ctx (errorToAlert tlserror)) (\_ -> return ()) on tags 1.3.11 or 1.4.0.

@ocheron
Copy link
Contributor

ocheron commented Oct 7, 2017

Sorry if I wasn't clear enough, the patch was to apply on tls-1.3.11 source code manually.

But it's not needed now, I see how to get hPutBuf: resource vanished (Broken pipe).

Apparently netcat used to set SO_LINGER but it's been removed because it causes errors when connections are terminated :-) That must explain why you can reproduce easily and I couldn't.

We'll see what behavior is best when peer aborts with a RST instead of a FIN.
Currently the sendPacket is indeed causing a second exception when handling the first.

With an incoming RST, the first exception raised is not Error_EOF but something like this instead:

Error_Misc "<socket: 5>: hGetBuf: resource vanished (Connection reset by peer)"

@ocheron
Copy link
Contributor

ocheron commented Oct 11, 2017

Complete server:

import Control.Exception

import Data.Default.Class

import Network
import Network.TLS

import System.IO

main = do
  srv <- listenOn (PortNumber 1234)
  (h, _, _) <- accept srv
  hSetBuffering h NoBuffering
  ctx <- contextNew h (def :: ServerParams)
  res <- try $ handshake ctx :: IO (Either TLSException ())
  print res

Client example:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include <sys/socket.h>
#include <arpa/inet.h>

int main(int argc, char *argv[]) {
  struct sockaddr_in sa;
  struct linger linger;
  int sd, r;

  sd = socket(AF_INET, SOCK_STREAM, 0);
  if (sd < 0) {
    perror("Error creating socket");
    return 1;
  }

  linger.l_onoff = 1;
  linger.l_linger = 0;
  r = setsockopt(sd, SOL_SOCKET, SO_LINGER, (const char *) &linger, sizeof(linger));
  if (r < 0) {
    perror("Error setting socket option");
    return 1;
  }

  sa.sin_family = AF_INET;
  sa.sin_addr.s_addr = inet_addr("127.0.0.1");
  sa.sin_port = htons(1234);

  r = connect(sd, (struct sockaddr*) &sa, sizeof(sa));
  if (r < 0) {
    perror("Connection error");
    return 1;
  }

  return 0;
}

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

2 participants