Re: [Qemu-devel] [PATCH v23 07/11] libcacard: add vscclient

[Alon Levy]

On Mon, Mar 28, 2011 at 02:43:38PM +0200, Jes Sorensen wrote:
> On 03/23/11 14:19, Alon Levy wrote:
> > From: Robert Relyea <address@hidden>
> > 
> > client to talk to ccid-card-passthru and use smartcard on client to
> > perform actual operations.
> > ---
> >  libcacard/Makefile    |    7 +-
> >  libcacard/vscclient.c |  730 
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 736 insertions(+), 1 deletions(-)
> >  create mode 100644 libcacard/vscclient.c
> > 
> [snip]
> > diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
> > new file mode 100644
> > index 0000000..8dde449
> > --- /dev/null
> > +++ b/libcacard/vscclient.c
> > @@ -0,0 +1,730 @@
> > +/*
> > + * Tester for VSCARD protocol, client side.
> > + *
> > + * Can be used with ccid-card-passthru.
> > + *
> > + * Copyright (c) 2011 Red Hat.
> > + * Written by Alon Levy.
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.1 or 
> > later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + */
> > +
> > +#include <sys/types.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +
> > +#include <sys/types.h>
> > +#include <sys/socket.h>
> > +#include <netdb.h>
> > +#include <netinet/in.h>
> > +#include <arpa/inet.h>
> > +
> > +#include "qemu-thread.h"
> > +#include "qemu-common.h"
> 
> Lots of unnecessary includes here.
Will fix.

> 
> 
> > +    if (verbose > 10) {
> > +        printf("sending type=%d id=%d, len =%d (0x%x)\n",
> > +               type, reader_id, length, length);
> > +    }
> > +
> > +    mhHeader.type = htonl(type);
> > +    mhHeader.reader_id = 0;
> > +    mhHeader.length = htonl(length);
> > +    rv = write(
> > +        sock,
> > +        &mhHeader,
> > +        sizeof(mhHeader)
> > +    );
> 
> Is this excessive multi-lining necessary? It makes it really hard to read.
It's a loop hole in the coding standard? ok, will fix.

> 
> > +    if (rv < 0) {
> > +        /* Error */
> > +        printf("write header error\n");
> > +        close(sock);
> > +        qemu_mutex_unlock(&write_lock);
> > +        return 16;
> > +    }
> 
> Shouldn't errors go to stderr ?
will fix.

> 
> > +    rv = write(
> > +        sock,
> > +        msg,
> > +        length
> > +    );
> > +    if (rv < 0) {
> > +        /* Error */
> > +        printf("write error\n");
> > +        close(sock);
> > +        qemu_mutex_unlock(&write_lock);
> > +        return 16;
> > +    }
> 
> Same here.
will fix.

> 
> > +static void
> > +do_command(void)
> > +{
> > +    char inbuf[255];
> > +    char *string;
> > +    VCardEmulError error;
> > +    static unsigned int default_reader_id;
> > +    unsigned int reader_id;
> > +    VReader *reader = NULL;
> > +
> > +    reader_id = default_reader_id;
> > +    string = fgets(inbuf, sizeof(inbuf), stdin);
> > +    if (string != NULL) {
> > +        if (strncmp(string, "exit", 4) == 0) {
> > +            /* remove all the readers */
> > +            VReaderList *list = vreader_get_reader_list();
> > +            VReaderListEntry *reader_entry;
> > +            printf("Active Readers:\n");
> > +            for (reader_entry = vreader_list_get_first(list); reader_entry;
> > +                 reader_entry = vreader_list_get_next(reader_entry)) {
> > +                VReader *reader = vreader_list_get_reader(reader_entry);
> > +                vreader_id_t reader_id;
> > +                reader_id = vreader_get_id(reader);
> > +                if (reader_id == -1) {
> > +                    continue;
> > +                }
> > +                /* be nice and signal card removal first (qemu probably 
> > should
> > +                 * do this itself) */
> > +                if (vreader_card_is_present(reader) == VREADER_OK) {
> > +                    send_msg(
> > +                        VSC_CardRemove,
> > +                        reader_id,
> > +                        NULL,
> > +                        0
> > +                    );
> > +                }
> > +                send_msg(
> > +                    VSC_ReaderRemove,
> > +                    reader_id,
> > +                    NULL,
> > +                    0
> > +                );
> > +            }
> > +            exit(0);
> > +        } else if (strncmp(string, "insert", 6) == 0) {
> > +            if (string[6] == ' ') {
> > +                reader_id = get_id_from_string(&string[7], reader_id);
> > +            }
> > +            reader = vreader_get_reader_by_id(reader_id);
> > +            if (reader != NULL) {
> > +                error = vcard_emul_force_card_insert(reader);
> > +                printf("insert %s, returned %d\n",
> > +                       reader ? vreader_get_name(reader)
> > +                       : "invalid reader", error);
> > +            } else {
> > +                printf("no reader by id %d found\n", reader_id);
> > +            }
> > +        } else if (strncmp(string, "remove", 6) == 0) {
> > +            if (string[6] == ' ') {
> > +                reader_id = get_id_from_string(&string[7], reader_id);
> > +            }
> > +            reader = vreader_get_reader_by_id(reader_id);
> > +            if (reader != NULL) {
> > +                error = vcard_emul_force_card_remove(reader);
> > +                printf("remove %s, returned %d\n",
> > +                        reader ? vreader_get_name(reader)
> > +                        : "invalid reader", error);
> > +            } else {
> > +                printf("no reader by id %d found\n", reader_id);
> > +            }
> > +        } else if (strncmp(string, "select", 6) == 0) {
> > +            if (string[6] == ' ') {
> > +                reader_id = get_id_from_string(&string[7],
> > +                                               VSCARD_UNDEFINED_READER_ID);
> > +            }
> > +            if (reader_id != VSCARD_UNDEFINED_READER_ID) {
> > +                reader = vreader_get_reader_by_id(reader_id);
> > +            }
> > +            if (reader) {
> > +                printf("Selecting reader %d, %s\n", reader_id,
> > +                        vreader_get_name(reader));
> > +                default_reader_id = reader_id;
> > +            } else {
> > +                printf("Reader with id %d not found\n", reader_id);
> > +            }
> > +        } else if (strncmp(string, "debug", 5) == 0) {
> > +            if (string[5] == ' ') {
> > +                verbose = get_id_from_string(&string[6], 0);
> > +            }
> > +            printf("debug level = %d\n", verbose);
> > +        } else if (strncmp(string, "list", 4) == 0) {
> > +            VReaderList *list = vreader_get_reader_list();
> > +            VReaderListEntry *reader_entry;
> > +            printf("Active Readers:\n");
> > +            for (reader_entry = vreader_list_get_first(list); reader_entry;
> > +                 reader_entry = vreader_list_get_next(reader_entry)) {
> > +                VReader *reader = vreader_list_get_reader(reader_entry);
> > +                vreader_id_t reader_id;
> > +                reader_id = vreader_get_id(reader);
> > +                if (reader_id == -1) {
> > +                    continue;
> > +                }
> > +                printf("%3d %s %s\n", reader_id,
> > +                       vreader_card_is_present(reader) == VREADER_OK ?
> > +                       "CARD_PRESENT" : "            ",
> > +                       vreader_get_name(reader));
> > +            }
> > +            printf("Inactive Readers:\n");
> > +            for (reader_entry = vreader_list_get_first(list); reader_entry;
> > +                 reader_entry = vreader_list_get_next(reader_entry)) {
> > +                VReader *reader = vreader_list_get_reader(reader_entry);
> > +                vreader_id_t reader_id;
> > +                reader_id = vreader_get_id(reader);
> > +                if (reader_id != -1) {
> > +                    continue;
> > +                }
> > +
> > +                printf("INA %s %s\n",
> > +                       vreader_card_is_present(reader) == VREADER_OK ?
> > +                       "CARD_PRESENT" : "            ",
> > +                       vreader_get_name(reader));
> > +            }
> > +        } else if (*string != 0) {
> > +            printf("valid commands:\n");
> > +            printf("insert [reader_id]\n");
> > +            printf("remove [reader_id]\n");
> > +            printf("select reader_id\n");
> > +            printf("list\n");
> > +            printf("debug [level]\n");
> > +            printf("exit\n");
> > +        }
> > +    }
> > +    vreader_free(reader);
> > +    printf("> ");
> > +    fflush(stdout);
> > +}
> > +
> > +
> > +#define APDUBufSize 270
> > +
> > +/* just for ease of parsing command line arguments. */
> > +#define MAX_CERTS 100
> > +
> > +static int
> > +connect_to_qemu(
> > +    const char *host,
> > +    const char *port
> > +) {
> > +    struct addrinfo hints;
> > +    struct addrinfo *server;
> > +    int ret;
> > +
> > +    sock = socket(
> > +        AF_INET,
> > +        SOCK_STREAM,
> > +        0
> > +    );
> 
> We have qemu_socket() for portability.
ok, will use.

> 
> > +    if (sock < 0) {
> > +        /* Error */
> > +        printf("Error opening socket!\n");
> > +    }
> > +
> > +    memset(&hints, 0, sizeof(struct addrinfo));
> > +    hints.ai_family = AF_UNSPEC;
> > +    hints.ai_socktype = SOCK_STREAM;
> > +    hints.ai_flags = 0;
> > +    hints.ai_protocol = 0;          /* Any protocol */
> > +
> > +    ret = getaddrinfo(host, port, &hints, &server);
> > +
> > +    if (ret != 0) {
> > +        printf("getaddrinfo failed\n");
> > +        return 5;
> > +    }
> 
> Juan just posted code to handle addrinfo and other bits that also
> includes ipv6 support. You may want to look at that for a later update.
> 
ok.

> > +static int on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit *incoming)
> > +{
> > +    uint32_t *capabilities = (incoming->capabilities);
> > +    int num_capabilities =
> > +        1 + ((mhHeader->length - sizeof(VSCMsgInit)) / sizeof(uint32_t));
> > +    int i;
> > +    int rv;
> > +    pthread_t thread_id;
> > +
> > +    incoming->version = ntohl(incoming->version);
> > +    if (incoming->version != VSCARD_VERSION) {
> > +        if (verbose > 0) {
> > +            printf("warning: host has version %d, we have %d\n",
> > +                verbose, VSCARD_VERSION);
> > +        }
> > +    }
> > +    if (incoming->magic != VSCARD_MAGIC) {
> 
> as mentioned in an earlier series, I think VSCARD_MAGIC needs to be
> network converted too, to handle difference in endianness between hosts.
Same answer as to your previous comment.

> 
> Cheers,
> Jes