gvanem / Watt-32

Watt-32 TCP/IP library and samples.
https://www.watt-32.net/
18 stars 8 forks source link

`getpeername()` fails if TCP session is reset by remote peer #92

Closed Lethja closed 8 months ago

Lethja commented 9 months ago

Watt32s getpeername() will return an error if called after a TCP session has been reset by the remote peer. The application will not be able to access information about the connection that just closed. This is contrary to Linux and Winsock where getpeername() will continue to return successfully until an explicit close()/closesocket() is called.

Sample code to reproduce:

/* Compile for DOS4G (Open Watcom) with 
 * `WCC386 -3 -bt=dos -dWATT32=1 -i=INC -mf ECHOTEST` then 
 * `WLINK SYS dos4g LIBF LIB\WATTCPWF.LIB F ECHOTEST`
 * 
 * Compile for Linux with 
 * `gcc echotest.c`
 * 
 * Use `nc 127.0.0.1 12345` on a Linux machine to connect
 * then SIGINT (Ctrl + C) nc to initate a TCP RST from the client
 */

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

#include <arpa/inet.h>
#include <netinet/in.h>
#include <sys/socket.h>
#include <sys/types.h>

#ifdef WATT32
#include <tcp.h>
#endif

#define PORT 12345
#define BUFFER_SIZE 1024

void print_socket_info(int client_socket, const char* action) {
    struct sockaddr_in client_info;
    socklen_t client_info_len = sizeof(client_info);

    if (getpeername(client_socket, (struct sockaddr*)&client_info, &client_info_len) == 0)
        printf("%s: %s:%d\n", action, inet_ntoa(client_info.sin_addr), htons(client_info.sin_port));
    else
        printf("%s: was socket %d (unfortunetly that's all we know)\n", action, client_socket);
}

void handle_client(int client_socket) {
    char buffer[BUFFER_SIZE];
    int bytes_received;

    while (1) {
        bytes_received = recv(client_socket, buffer, sizeof(buffer), 0);

        if (bytes_received == -1) {
            print_socket_info(client_socket, "Error receiving data");
            break;
        }
        else if (bytes_received == 0) {
            print_socket_info(client_socket, "Connection closed by client");
            break;
        }
        else {
            send(client_socket, buffer, bytes_received, 0);
            print_socket_info(client_socket, "Echoed data back to client");
        }
    }
}

int main() {
    int server_socket, client_socket;
    struct sockaddr_in server_addr, client_addr;
    socklen_t client_len = sizeof(client_addr);

#ifdef WATT32
    sock_init();
#endif

    if ((server_socket = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
        perror("Error creating socket");
        exit(EXIT_FAILURE);
    }

    memset(&server_addr, 0, sizeof(server_addr));
    server_addr.sin_family = AF_INET,
    server_addr.sin_addr.s_addr = INADDR_ANY,
    server_addr.sin_port = htons(PORT);

    if (bind(server_socket, (struct sockaddr*)&server_addr, sizeof(server_addr)) == -1) {
        perror("Error binding socket");
        exit(EXIT_FAILURE);
    }

    if (listen(server_socket, 5) == -1) {
        perror("Error listening for connections");
        exit(EXIT_FAILURE);
    }

#ifdef WATT32
    client_addr.sin_addr.s_addr = htonl(my_ip_addr);
    printf("Server listening on %s:%d...\n", inet_ntoa(client_addr.sin_addr), PORT);
#else
    printf("Server listening on port %d...\n", PORT);
#endif

    while (1) {
        if ((client_socket = accept(server_socket, (struct sockaddr*)&client_addr, &client_len)) == -1) {
            perror("Error accepting connection");
            continue;
        }

        print_socket_info(client_socket, "Accepted connection");
        handle_client(client_socket);
        close(client_socket);
    }

    close(server_socket);
    return 0;
}
gvanem commented 8 months ago

Ok. Try to add a call to dbug_init() and look for EINTR. I'm not sure what the cause is

gvanem commented 8 months ago

Trying this patch:

--- a/src/getname.c
+++ b/src/getname.c
@@ -113,12 +113,14 @@ int W32_CALL getpeername (int s, struct sockaddr *name, socklen_t *namelen)
     return (-1);
   }

+#if 0
   if (!(socket->so_state & SS_ISCONNECTED))
   {
     SOCK_DEBUGF ((", ENOTCONN"));
     SOCK_ERRNO (ENOTCONN);
     return (-1);
   }
+#endif

   VERIFY_RW (name, *namelen);

it works:

Server listening on 10.0.0.7:12345...
Accepted connection: 10.0.0.10:59905
Echoed data back to client: 10.0.0.10:59905
Echoed data back to client: 10.0.0.10:59905
Echoed data back to client: 10.0.0.10:59905
Connection closed by client: 10.0.0.10:59905

But I'm not sure it's a good idea or what other implications it has.

jwt27 commented 8 months ago

Maybe:

--- a/src/getname.c
+++ b/src/getname.c
@@ -113,7 +113,7 @@ int W32_CALL getpeername (int s, struct sockaddr *name, socklen_t *namelen)
     return (-1);
   }

-  if (!(socket->so_state & SS_ISCONNECTED))
+  if (!socket->remote_addr)
   {
     SOCK_DEBUGF ((", ENOTCONN"));
     SOCK_ERRNO (ENOTCONN);

Although current behavior is technically correct, according to Linux and BSD man pages.

gvanem commented 8 months ago

@jwt27 Tried it. Works for the 1st client pressing ^C. But a new nc ... never connects!

And BTW, the code about had an issue with Dr. Watson on MSVC/clang-cl when exiting. This was needed:

  #ifdef WATT32
  #include <tcp.h>
+ #define close(s) closesocket(s)
  #endif
jwt27 commented 8 months ago

Works for me on djgpp. But I have seen the issue before where accept() sometimes ignores new connections. That seems to be a separate bug.

gvanem commented 8 months ago

@jwt27 I'm committing your fix above. Thanks.

gvanem commented 8 months ago

Fixed in https://github.com/gvanem/Watt-32/commit/d52978297f83a9526052d5a0180410f6d507396e. Closing as completed.