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

[Jes Sorensen <Jes.Sorensen@redhat.com>]

On 03/23/11 14:19, Alon Levy wrote:
> From: Robert Relyea <rrelyea@redhat.com>
> 
> 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.


> +    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.

> +    if (rv < 0) {
> +        /* Error */
> +        printf("write header error\n");
> +        close(sock);
> +        qemu_mutex_unlock(&write_lock);
> +        return 16;
> +    }

Shouldn't errors go to stderr ?

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

Same here.

> +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.

> +    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.

> +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.

Cheers,
Jes