executing commands via sockets with popen()

Question

Can anybody give me a hand trying to implement the following server and client?:

The server:

#include <stdlib.h>
#include <unistd.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <stdio.h>
#include <string.h>

int main(void) {
    int sock = socket(AF_INET, SOCK_STREAM, 0);

    struct sockaddr_in serv_addr = { 0 };
    serv_addr.sin_family = AF_INET;
    serv_addr.sin_addr.s_addr = INADDR_ANY;
    serv_addr.sin_port = htons(1234);
    bind(sock, (struct sockaddr *)&serv_addr, sizeof(serv_addr));

    listen(sock, 128);

    struct sockaddr_in cli_addr = { 0 };
    socklen_t cli_addrlen = sizeof(cli_addr);
    int acc_sock = accept(sock, (struct sockaddr *)&cli_addr, &cli_addrlen);
    printf("[+] Connected \n");
    char buf[1024];
    ssize_t nread;
    memset(buf, 0, sizeof(buf));
    int a;
    while (1) {
        nread = read(0, buf, 1024);
        write(acc_sock, buf, nread);
        memset(buf, 0, sizeof(buf));

        while ((read(acc_sock, buf, 1024)) != 0) {
            printf("%s", buf);
            memset(buf, 0, sizeof(buf));
        }
    }
}

All the servers does is scanning a command from stdin and send it to the client via sockets. Then scans the client response and prints it out to stdout.

The client:

#include <stdlib.h>
#include <unistd.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <stdio.h>
#include <string.h>

int main(int argc, const char *argv[]) {
    int sock = socket(AF_INET, SOCK_STREAM, 0);
    struct sockaddr_in serv = { 0 };
    char buf[1024];
    char command[1024];
    memset(buf, 0, sizeof(buf));
    memset(command, 0, sizeof(command));
    int nread;
    FILE *in;
    extern FILE *popen();

    serv.sin_family = AF_INET;
    serv.sin_port = htons(atoi(argv[1]));
    serv.sin_addr.s_addr = inet_addr("127.0.0.1");
    int a;  
    connect(sock, (struct sockaddr*)&serv, sizeof(serv));
    while (1) {
        nread = read(sock, buf, 1024);
        in = popen(buf, "r");
        while ((fgets(command, sizeof(command), in)) != NULL) {;
            write(sock, command, sizeof(command));
        }
        memset(buf, 0, sizeof(buf));
    }
    return 0;
}

Essentially the client receives the command scanned by the server, executes it using popen(), and sends the contents of the command line by line until NULL.

The problem is that the code suddenly stops working just after the first command. the transmission of the command and the command's output is satisfactory, however after printing the output of the first command the program just stops working. I think is a problem with fgets(), however I could be wrong. Is there any solutions for this problem?

Answer 1

Caveat: This may [or may not] fit your needs because I reversed the sense of the client and server loops in the corrected code below. As I had mentioned in my comment above:

The normal orientation for an app like this is that a client connects to a server and the client feeds the commands [read from stdin] to the server [which does popen] and feeds back the results. That's how ssh works. Your orientation is reversed. What you've got is you fire sshd and wait for ssh to connect and then sshd sends commands to ssh. In other words, the loops in the respective sides should be switched.

Reversing this was the only way things made sense to me. If the reversal doesn't work [well] for your desired use case, the code below may still give you some ideas.

I solved the hang problem by introducing the concept of a flag character to denote end-of-output. I borrowed this concept from the PPP [point-to-point] protocol over RS-232.

The flag character is just a given value (e.g. 0x10) that is not likely to be part of normal data. Since your data is most likely ascii or utf-8, any [unused] chars in the range 0x00-0x1F may be used (i.e. don't use tab, cr, newline, etc).

If you need to transmit the flag character (i.e. your data has to be the full binary range 0x00-0xFF), I've included some packet encode/decode routines that implement the escape codes used in PPP above. I've coded them, but did not actually hook them in. In this case, the flag [and escape] chars can be any binary value [usually 0xFF and 0xFE respectively].

For simplicity, I combined both sides into a single .c file. Invoke the server with -s [first].

Anyway, here's the tested code [please pardon the gratuitous style cleanup]:

// inetpair/inetpair -- server/client communication

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

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

typedef unsigned char byte;

#define BUFMAX          1024
int port = 1234;
int opt_svr;
int opt_debug;

#define FLAG            0x10
#define ESC             0x11
#define ESC_FLAG        0x01
#define ESC_ESC         0x02

#define dbgprt(_fmt...) \
    do { \
        if (opt_debug) \
            printf(_fmt); \
    } while (0)

// client
int
client(void)
{
    int sock;
    struct sockaddr_in serv = { 0 };
    char *cp;
    char buf[BUFMAX + 1];
    int nread;
    int flag;
    int exitflg;

    sock = socket(AF_INET, SOCK_STREAM, 0);

    serv.sin_family = AF_INET;
    serv.sin_port = htons(port);
    serv.sin_addr.s_addr = inet_addr("127.0.0.1");

    connect(sock, (struct sockaddr *) &serv, sizeof(serv));

    while (1) {
        cp = fgets(buf,BUFMAX,stdin);
        if (cp == NULL)
            break;

        exitflg = (strcmp(buf,"exit\n") == 0);

        // send the command
        nread = strlen(buf);
        write(sock, buf, nread);

        if (exitflg)
            break;

        while (1) {
            dbgprt("client: PREREAD\n");
            nread = read(sock, buf, 1024);
            dbgprt("client: POSTREAD nread=%d\n",nread);
            if (nread <= 0)
                break;

            cp = memchr(buf,FLAG,nread);
            flag = (cp != NULL);
            if (flag)
                nread = cp - buf;

            write(1,buf,nread);

            if (flag)
                break;
        }
    }

    close(sock);

    return 0;
}

// server
int
server(void)
{
    struct sockaddr_in serv_addr = { 0 };
    int sock;
    int acc_sock;
    char buf[BUFMAX + 1];
    char command[BUFMAX + 1];
    ssize_t nread;
    FILE *pin;
    FILE *xfin;
    char *cp;
    struct sockaddr_in cli_addr = { 0 };

    opt_debug = ! opt_debug;

    dbgprt("[+] Starting\n");

    sock = socket(AF_INET, SOCK_STREAM, 0);

    serv_addr.sin_family = AF_INET;
    serv_addr.sin_addr.s_addr = INADDR_ANY;
    serv_addr.sin_port = htons(port);
    bind(sock, (struct sockaddr *) &serv_addr, sizeof(serv_addr));

    listen(sock, 128);

    while (1) {
        socklen_t cli_addrlen = sizeof(cli_addr);

        dbgprt("[+] Waiting for connection\n");
        acc_sock = accept(sock,(struct sockaddr *)&cli_addr,&cli_addrlen);

        dbgprt("[+] Connected\n");

        xfin = fdopen(acc_sock,"r");

        while (1) {
            dbgprt("[+] Waiting for command\n");

            cp = fgets(buf,BUFMAX,xfin);
            if (cp == NULL)
                break;

            cp = strchr(buf,'\n');
            if (cp != NULL)
                *cp = 0;

            dbgprt("[+] Command '%s'\n",buf);

            if (strcmp(buf,"exit") == 0)
                break;

            pin = popen(buf, "r");
            while (1) {
                cp = fgets(command, BUFMAX, pin);
                if (cp == NULL)
                    break;
                nread = strlen(command);
                write(acc_sock, command, nread);
            }
            pclose(pin);

            command[0] = FLAG;
            write(acc_sock,command,1);
        }

        fclose(xfin);
        close(acc_sock);

        dbgprt("[+] Disconnect\n");
    }
}

// packet_encode -- encode packet
// RETURNS: (outlen << 1)
int
packet_encode(void *dst,const void *src,int srclen)
{
    const byte *sp = src;
    byte *dp = dst;
    const byte *ep;
    byte chr;
    int dstlen;

    // encode packet in manner similar to PPP (point-to-point) protocol does
    // over RS-232 line

    ep = sp + srclen;
    for (;  sp < ep;  ++sp) {
        chr = *sp;

        switch (chr) {
        case FLAG:
            *dp++ = ESC;
            *dp++ = ESC_FLAG;
            break;

        case ESC:
            *dp++ = ESC;
            *dp++ = ESC_ESC;
            break;

        default:
            *dp++ = chr;
            break;
        }
    }

    dstlen = dp - (byte *) dst;
    dstlen <<= 1;

    return dstlen;
}

// packet_decode -- decode packet
// RETURNS: (outlen << 1) | flag
int
packet_decode(void *dst,const void *src,int srclen)
{
    const byte *sp = src;
    byte *dp = dst;
    const byte *ep;
    byte chr;
    int flag;
    int dstlen;

    // decode packet in manner similar to PPP (point-to-point) protocol does
    // over RS-232 line

    ep = sp + srclen;
    flag = 0;
    while (sp < ep) {
        chr = *sp++;

        flag = (chr == FLAG);
        if (flag)
            break;

        switch (chr) {
        case ESC:
            chr = *sp++;

            switch (chr) {
            case ESC_FLAG:
                *dp++ = FLAG;
                break;
            case ESC_ESC:
                *dp++ = ESC;
                break;
            }
            break;

        default:
            *dp++ = chr;
            break;
        }
    }

    dstlen = dp - (byte *) dst;
    dstlen <<= 1;

    if (flag)
        dstlen |= 0x01;

    return dstlen;
}

int
main(int argc, char **argv)
{
    char *cp;

    --argc;
    ++argv;

    for (;  argc > 0;  --argc, ++argv) {
        cp = *argv;
        if (*cp != '-')
            break;

        switch (cp[1]) {
        case 'd':
            opt_debug = 1;
            break;

        case 'P':
            port = atoi(cp + 2);
            break;

        case 's':
            opt_svr = 1;
            break;
        }
    }

    if (opt_svr)
        server();
    else
        client();

    return 0;
}

Answer 2

The client never closes sock. Therefore the server's loop

    while ((read(acc_sock, buf, 1024)) != 0) {
        printf("%s", buf);
        memset(buf, 0, sizeof(buf));
    }

never terminates. You need some mechanism to inform server that all the command's output has been sent. Maybe something similar to HTTP chunked transfer encoding.