Provide a VCARD_DIRECT implementation.

This enables a passthru mode in the spice client, where we relay
apdus from the client to the host, rather than passing through nss
and emulating a card.

Signed-off-by: Jeremy White <jwhite@codeweavers.com>
---
 Makefile.objs               |    5 +
 configure                   |   38 +++
 libcacard/capcsc.c          |  612 +++++++++++++++++++++++++++++++++++++++++++
 libcacard/capcsc.h          |   16 ++
 libcacard/libcacard.syms    |    1 +
 libcacard/vcard.c           |    2 +-
 libcacard/vcard.h           |    2 +-
 libcacard/vcard_emul_nss.c  |   28 +-
 libcacard/vcard_emul_type.c |    3 +-
 libcacard/vscclient.c       |   16 +-
 10 files changed, 706 insertions(+), 17 deletions(-)
 create mode 100644 libcacard/capcsc.c
 create mode 100644 libcacard/capcsc.h

Hey

Thanks for proposing passthrough mode. It can be useful in some cases,
although it's good to mention it's not safe to concurrently share a
card with other applications since no locking is provided (and I don't
know if and how it's possible atm).

You should fix your patch to pass qemu scripts/checkpatch.pl (too many
errors to report here)

Some review notes follow.

On Mon, Jan 19, 2015 at 4:00 PM, Jeremy White <jwhite@codeweavers.com> wrote:
> This enables a passthru mode in the spice client, where we relay
> apdus from the client to the host, rather than passing through nss
> and emulating a card.

Could you explain how you handle multiple readers, and what is the
sender PID used for?

>
> Signed-off-by: Jeremy White <jwhite@codeweavers.com>
> ---
>  Makefile.objs               |    5 +
>  configure                   |   38 +++

Although I am not very picky about splitting things, I would say you
could easily split the configure part first.

>  libcacard/capcsc.c          |  612 +++++++++++++++++++++++++++++++++++++++++++
>  libcacard/capcsc.h          |   16 ++
>  libcacard/libcacard.syms    |    1 +

Then the PCSC/passthrough card.
I haven't done a thorough review of capcsc yet, just a few comments below.

>  libcacard/vcard.c           |    2 +-
>  libcacard/vcard.h           |    2 +-
>  libcacard/vcard_emul_nss.c  |   28 +-
>  libcacard/vcard_emul_type.c |    3 +-

Then integration with vcard,

>  libcacard/vscclient.c       |   16 +-

And the vscclient part.

>  10 files changed, 706 insertions(+), 17 deletions(-)
>  create mode 100644 libcacard/capcsc.c
>  create mode 100644 libcacard/capcsc.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index abeb902..bb9659e 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -32,6 +32,11 @@ libcacard-y += libcacard/card_7816.o
>  libcacard-y += libcacard/vcardt.o
>  libcacard/vcard_emul_nss.o-cflags := $(NSS_CFLAGS)
>  libcacard/vcard_emul_nss.o-libs := $(NSS_LIBS)
> +ifeq ($(CONFIG_SMARTCARD_PCSC),y)
> +libcacard-y += libcacard/capcsc.o
> +libcacard/capcsc.o-cflags := $(PCSC_CFLAGS)
> +libcacard/capcsc.o-libs := $(PCSC_LIBS)
> +endif
>
>  ######################################################################
>  # Target independent part of system emulation. The long term path is to
> diff --git a/configure b/configure
> index cae588c..f1dd05c 100755
> --- a/configure
> +++ b/configure
> @@ -307,6 +307,7 @@ trace_file="trace"
>  spice=""
>  rbd=""
>  smartcard_nss=""
> +smartcard_pcsc=""
>  libusb=""
>  usb_redir=""
>  glx=""
> @@ -1042,6 +1043,10 @@ for opt do
>    ;;
>    --enable-smartcard-nss) smartcard_nss="yes"
>    ;;
> +  --disable-smartcard-pcsc) smartcard_pcsc="no"
> +  ;;
> +  --enable-smartcard-pcsc) smartcard_pcsc="yes"
> +  ;;
>    --disable-libusb) libusb="no"
>    ;;
>    --enable-libusb) libusb="yes"
> @@ -1368,6 +1373,8 @@ Advanced options (experts only):
>    --enable-libnfs          enable nfs support
>    --disable-smartcard-nss  disable smartcard nss support
>    --enable-smartcard-nss   enable smartcard nss support
> +  --disable-smartcard-pcsc disable smartcard pcsc passthru support
> +  --enable-smartcard-pcsc  enable smartcard pcsc passthru support
>    --disable-libusb         disable libusb (for usb passthrough)
>    --enable-libusb          enable libusb (for usb passthrough)
>    --disable-usb-redir      disable usb network redirection support
> @@ -3648,6 +3655,30 @@ EOF
>      fi
>  fi
>
> +# check for pcsclite for smartcard passthru support

(perhaps it's good to keep in mind that this code should be fairly
easy to port to winscard, perhaps a TODO would be worth here?)

> +if test "$smartcard_pcsc" != "no"; then
> +  cat > $TMPC << EOF
> +#include <winscard.h>
> +int main(void) { SCardEstablishContext(0, 0, 0, 0); return 0; }
> +EOF
> +    # FIXME: do not include $glib_* in here
> +    pcsc_libs="$($pkg_config --libs libpcsclite 2>/dev/null) $glib_libs"
> +    pcsc_cflags="$($pkg_config --cflags libpcsclite 2>/dev/null) $glib_cflags"

It seems you can remove the $glib here.

> +    test_cflags="$pcsc_cflags"
> +    if test "$werror" = "yes"; then
> +        test_cflags="-Werror $test_cflags"
> +    fi
> +    if test -n "$libtool" &&
> +      compile_prog "$test_cflags" "$pcsc_libs"; then
> +        smartcard_pcsc="yes"
> +    else
> +        if test "$smartcard_pcsc" = "yes"; then
> +            feature_not_found "pcsc" "Install libpcsclite"
> +        fi
> +        smartcard_pcsc="no"
> +    fi
> +fi
> +
>  # check for libusb
>  if test "$libusb" != "no" ; then
>      if $pkg_config --atleast-version=1.0.13 libusb-1.0; then
> @@ -4318,6 +4349,7 @@ fi
>  echo "rbd support       $rbd"
>  echo "xfsctl support    $xfs"
>  echo "nss used          $smartcard_nss"
> +echo "pcsc used         $smartcard_pcsc"
>  echo "libusb            $libusb"
>  echo "usb net redir     $usb_redir"
>  echo "GLX support       $glx"
> @@ -4674,6 +4706,12 @@ if test "$smartcard_nss" = "yes" ; then
>    echo "NSS_CFLAGS=$nss_cflags" >> $config_host_mak
>  fi
>
> +if test "$smartcard_pcsc" = "yes" ; then
> +  echo "CONFIG_SMARTCARD_PCSC=y" >> $config_host_mak
> +  echo "PCSC_LIBS=$pcsc_libs" >> $config_host_mak
> +  echo "PCSC_CFLAGS=$pcsc_cflags" >> $config_host_mak
> +fi
> +
>  if test "$libusb" = "yes" ; then
>    echo "CONFIG_USB_LIBUSB=y" >> $config_host_mak
>  fi
> diff --git a/libcacard/capcsc.c b/libcacard/capcsc.c
> new file mode 100644
> index 0000000..c7da458
> --- /dev/null
> +++ b/libcacard/capcsc.c
> @@ -0,0 +1,612 @@
> +/*
> + * Supply a vreader using the PC/SC interface.
> + *
> + * 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.
> + */
> +
> +/* avoid including prototypes.h that causes some qemu type conflicts */
> +#define NO_NSPR_10_SUPPORT
> +#include <prthread.h>
> +
> +#include "qemu-common.h"
> +
> +#include "vcard.h"
> +#include "card_7816.h"
> +#include "capcsc.h"
> +#include "vreader.h"
> +#include "vevent.h"
> +
> +#include <PCSC/wintypes.h>
> +#include <PCSC/winscard.h>
> +
> +
> +typedef struct _PCSCContext PCSCContext;
> +
> +typedef struct _APDUMsg {
> +    void *data;
> +    int len;
> +    pid_t sender;
> +    LONG rc;
> +    struct _APDUMsg *next;
> +} APDUMsg;
> +
> +typedef struct
> +{
> +    PCSCContext *context;
> +    int index;
> +    char *name;
> +    DWORD protocol;
> +    DWORD state;
> +    SCARDHANDLE card;
> +    BYTE atr[MAX_ATR_SIZE];
> +    DWORD atrlen;
> +    int card_connected;
> +    CompatGMutex request_lock;
> +    APDUMsg *requests;
> +    unsigned long request_count;
> +    CompatGMutex response_lock;
> +    APDUMsg *responses;
> +} SCardReader;
> +
> +typedef struct _PCSCContext
> +{
> +    SCARDCONTEXT context;
> +    SCardReader readers[CAPCSC_MAX_READERS];
> +    int reader_count;
> +    int readers_changed;
> +    PRThread *thread;
> +    CompatGMutex lock;
> +} PCSCContext;
> +
> +
> +static void push_request(SCardReader *r, void *data, int len)
> +{
> +    APDUMsg *a = malloc(sizeof(*a));

qemu uses quite consistently g_malloc/new/slice & friends nowadays afaict.

> +    APDUMsg **p;
> +
> +    a->data = malloc(len);
> +    a->len = len;
> +    a->sender = getpid();
> +    a->next = NULL;
> +    memcpy(a->data, data, len);
> +
> +    g_mutex_lock(&r->request_lock);
> +    for (p = &r->requests; *p; p = &(*p)->next)
> +        ;
> +    *p = a;
> +    r->request_count++;

Why not use GQueue or GAsyncQueue?

> +
> +    g_mutex_unlock(&r->request_lock);
> +}
> +
> +static APDUMsg * get_request(SCardReader *r)
> +{
> +    APDUMsg *p;
> +    g_mutex_lock(&r->request_lock);
> +    p = r->requests;
> +    if (r->requests)
> +        r->requests = p->next;
> +    g_mutex_unlock(&r->request_lock);
> +    return p;
> +}
> +
> +static void push_response(SCardReader *r, void *data, int len, LONG rc, pid_t sender)
> +{
> +    APDUMsg *a = malloc(sizeof(*a));
> +    APDUMsg **p;
> +
> +    a->data = malloc(len);
> +    a->len = len;
> +    a->sender = sender;
> +    a->next = NULL;
> +    a->rc = rc;
> +    memcpy(a->data, data, len);
> +
> +    g_mutex_lock(&r->response_lock);
> +    for (p = &r->responses; *p; p = &(*p)->next)
> +        ;
> +    *p = a;
> +
> +    g_mutex_unlock(&r->response_lock);
> +}
> +
> +static APDUMsg * get_response(SCardReader *r)
> +{
> +    APDUMsg **p;
> +    APDUMsg *ret = NULL;
> +    g_mutex_lock(&r->response_lock);
> +    for (p = &r->responses; *p && (*p)->sender != getpid(); p = &((*p)->next))
> +        ;
> +    if (*p)
> +    {
> +        ret = *p;
> +        *p = ret->next;
> +    }
> +    g_mutex_unlock(&r->response_lock);
> +    return ret;
> +}
> +
> +static void free_msg(APDUMsg *m)
> +{
> +    free(m->data);
> +    free(m);
> +}
> +
> +
> +static void delete_reader(PCSCContext *pc, int i)
> +{
> +    SCardReader *r = &pc->readers[i];
> +    g_mutex_clear(&r->request_lock);
> +    g_mutex_clear(&r->response_lock);
> +    free(r->name);
> +    r->name = NULL;
> +
> +    if (i < (pc->reader_count - 1))
> +    {
> +        int rem = pc->reader_count - i - 1;
> +        memmove(&pc->readers[i], &pc->readers[i + 1], sizeof(SCardReader) * rem);
> +    }
> +
> +    pc->reader_count--;
> +}
> +
> +static void delete_reader_cb(VReaderEmul *ve)
> +{
> +    SCardReader *r = (SCardReader *) ve;
> +
> +    g_mutex_lock(&r->context->lock);
> +    delete_reader(r->context, r->index);
> +    g_mutex_unlock(&r->context->lock);
> +}
> +
> +static int new_reader(PCSCContext *pc, const char *name, DWORD state)
> +{
> +    SCardReader *r;
> +    VReader *vreader;
> +
> +    if (pc->reader_count >= CAPCSC_MAX_READERS - 1)
> +        return 1;
> +
> +    r = &pc->readers[pc->reader_count];
> +    memset(r, 0, sizeof(*r));
> +    r->index = pc->reader_count++;
> +    r->context = pc;
> +    r->name = strdup(name);
> +    g_mutex_init(&r->request_lock);
> +    g_mutex_init(&r->response_lock);
> +
> +    vreader = vreader_new(name, (VReaderEmul *) r, delete_reader_cb);
> +    vreader_add_reader(vreader);
> +    vreader_free(vreader);
> +
> +    return 0;
> +}
> +
> +static int find_reader(PCSCContext *pc, const char *name)
> +{
> +    int i;
> +    for (i = 0; i < pc->reader_count; i++)
> +        if (strcmp(pc->readers[i].name, name) == 0)
> +            return i;
> +
> +    return -1;
> +}
> +
> +
> +static int scan_for_readers(PCSCContext *pc)
> +{
> +    LONG rc;
> +
> +    int i;
> +    char buf[8192];
> +    DWORD buflen = sizeof(buf);
> +
> +    char *p;
> +    int matches[CAPCSC_MAX_READERS];
> +
> +    g_mutex_lock(&pc->lock);
> +
> +    for (i = 0; i < CAPCSC_MAX_READERS; i++)
> +        matches[i] = 0;
> +
> +    pc->readers_changed = 1;
> +    memset(buf, 0, sizeof(buf));
> +    rc = SCardListReaders(pc->context, NULL, buf, &buflen);
> +    if (rc == SCARD_E_NO_READERS_AVAILABLE)
> +    {
> +        rc = 0;
> +        goto exit;
> +    }
> +
> +    if (rc != SCARD_S_SUCCESS)
> +    {
> +        fprintf(stderr, "SCardListReaders failed: %s (0x%lX)\n",
> +            pcsc_stringify_error(rc), rc);
> +        goto exit;
> +    }
> +
> +    for (p = buf; p && p < buf + sizeof(buf); p += (strlen(p) + 1))
> +    {
> +        if (strlen(p) > 0)
> +        {
> +            i = find_reader(pc, p);
> +            if (i >= 0)
> +                matches[i]++;
> +            else
> +            {
> +                if (! new_reader(pc, p, SCARD_STATE_UNAWARE))
> +                    matches[pc->reader_count - 1]++;
> +            }
> +        }
> +    }
> +
> +    rc = 0;
> +
> +exit:
> +    i = pc->reader_count - 1;
> +    g_mutex_unlock(&pc->lock);
> +
> +    for (; i >= 0; i--)
> +        if (! matches[i])
> +        {
> +            VReader *reader = vreader_get_reader_by_name(pc->readers[i].name);
> +            if (reader)
> +            {
> +                vreader_free(reader);
> +                vreader_remove_reader(reader);
> +            }
> +        }
> +
> +
> +    return rc;
> +}
> +
> +static int init_pcsc(PCSCContext *pc)
> +{
> +    LONG rc;
> +
> +    memset(pc, 0, sizeof(*pc));
> +
> +    rc = SCardEstablishContext(SCARD_SCOPE_SYSTEM, NULL, NULL, &pc->context);
> +    if (rc != SCARD_S_SUCCESS)
> +    {
> +        fprintf(stderr, "SCardEstablishContext: Cannot Connect to Resource Manager %lX\n", rc);
> +        return rc;
> +    }
> +
> +    return 0;
> +}
> +
> +
> +static void prepare_reader_states(PCSCContext *pc, SCARD_READERSTATE **states, DWORD *reader_count)
> +{
> +    SCARD_READERSTATE *state;
> +    int i;
> +
> +    if (*states)
> +        free(*states);
> +
> +    *reader_count = pc->reader_count;
> +
> +    (*reader_count)++;
> +    *states = malloc((*reader_count) * sizeof(**states));
> +    memset(*states, 0, sizeof((*reader_count) * sizeof(**states)));
> +
> +    for (i = 0, state = *states; i < pc->reader_count; i++, state++)
> +    {
> +        state->szReader = pc->readers[i].name;
> +        state->dwCurrentState = pc->readers[i].state;
> +    }
> +
> +    /* Leave a space to be notified of new readers */
> +    state->szReader = "\\\\?PnP?\\Notification";
> +    state->dwCurrentState = SCARD_STATE_UNAWARE;
> +}
> +
> +static int connect_card(SCardReader *r)
> +{
> +    LONG rc;
> +
> +    r->protocol = -1;
> +    rc = SCardConnect(r->context->context, r->name, SCARD_SHARE_SHARED,
> +                        SCARD_PROTOCOL_T0 | SCARD_PROTOCOL_T1,
> +                        &r->card, &r->protocol);
> +    if (rc != SCARD_S_SUCCESS)
> +    {
> +        fprintf(stderr, "Failed to connect to a card reader: %s (0x%lX)\n",
> +            pcsc_stringify_error(rc), rc);
> +        return rc;
> +    }
> +
> +    r->card_connected = 1;
> +    r->request_count = 0;
> +
> +    return 0;
> +}
> +
> +static LONG send_receive(SCardReader *r, BYTE *transmit, DWORD transmit_len,
> +                 BYTE *receive, DWORD *receive_len)
> +{
> +    const SCARD_IO_REQUEST *send_header;
> +    SCARD_IO_REQUEST receive_header;
> +    LONG rc;
> +
> +    if (! r->card_connected)
> +    {
> +        rc = connect_card(r);
> +        if (rc)
> +            return rc;
> +    }
> +
> +    if (r->protocol == SCARD_PROTOCOL_T0)
> +        send_header = SCARD_PCI_T0;
> +    else if (r->protocol == SCARD_PROTOCOL_T1)
> +        send_header = SCARD_PCI_T1;
> +    else
> +    {
> +        fprintf(stderr, "Unknown protocol %lX\n", r->protocol);
> +        return 1;
> +    }
> +
> +    rc = SCardTransmit(r->card, send_header, transmit, transmit_len,
> +                        &receive_header, receive, receive_len);
> +    if (rc != SCARD_S_SUCCESS)
> +    {
> +        fprintf(stderr, "Failed to transmit %ld bytes: %s (0x%lX)\n",
> +            transmit_len, pcsc_stringify_error(rc), rc);
> +        return rc;
> +    }
> +
> +    return 0;
> +}
> +
> +
> +static VCardStatus apdu_cb(VCard *card, VCardAPDU *apdu, VCardResponse **response)
> +{
> +    VCardStatus ret = VCARD_DONE;
> +    SCardReader *r = (SCardReader *) vcard_get_private(card);
> +    APDUMsg *resp;
> +
> +
> +    push_request(r, apdu->a_data, apdu->a_len);
> +    while (1)
> +    {
> +        resp = get_response(r);
> +        if (resp)
> +            break;
> +
> +        usleep(1);
> +    }
> +
> +    if (resp->rc || resp->len < 2)
> +        ret = VCARD_FAIL;
> +    else
> +        *response = vcard_response_new_bytes(card, resp->data, resp->len - 2, apdu->a_Le,
> +                                ((BYTE *)resp->data)[resp->len - 2],
> +                                ((BYTE *)resp->data)[resp->len - 1]);
> +
> +    free_msg(resp);
> +
> +    return ret;
> +}
> +
> +static VCardStatus reset_cb(VCard *card, int channel)
> +{
> +    SCardReader *r = (SCardReader *) vcard_get_private(card);
> +    LONG rc;
> +    unsigned long count;
> +
> +    while (get_request(r) || get_response(r))
> +        ;
> +
> +    /* vreader_power_on is a bit too free with it's resets.
> +       And a reconnect is expensive; as much as 10-20 seconds.
> +       Hence, we discard any initial reconnect request. */
> +    g_mutex_lock(&r->request_lock);
> +    count = r->request_count++;
> +    g_mutex_unlock(&r->request_lock);
> +    if (count == 0)
> +        return VCARD_DONE;
> +
> +    rc = SCardReconnect(r->card, SCARD_SHARE_SHARED,
> +                        SCARD_PROTOCOL_T0 | SCARD_PROTOCOL_T1, SCARD_RESET_CARD, &r->protocol);
> +    if (rc != SCARD_S_SUCCESS)
> +    {
> +        fprintf(stderr, "Failed to reconnect to a card reader: %s (0x%lX)\n",
> +            pcsc_stringify_error(rc), rc);
> +        return VCARD_FAIL;
> +    }
> +    return VCARD_DONE;
> +}
> +
> +static void get_atr_cb (VCard *card, unsigned char *atr, int *atr_len)
> +{
> +    SCardReader *r = (SCardReader *) vcard_get_private(card);
> +    *atr_len = r->atrlen;
> +    if (atr)
> +        memcpy(atr, r->atr, r->atrlen);
> +}
> +
> +static void delete_card_cb (VCardEmul *ve)
> +{
> +    fprintf(stderr, "TODO, got a delete_card_cb\n");
> +}
> +
> +static void insert_card(SCardReader *r, SCARD_READERSTATE *s)
> +{
> +    memcpy(r->atr, s->rgbAtr, MIN(sizeof(r->atr), sizeof(s->rgbAtr)));
> +    r->atrlen = s->cbAtr;
> +
> +    VReader *reader = vreader_get_reader_by_name(r->name);
> +    if (! reader)
> +        return;
> +
> +    if (connect_card(r))
> +        return;
> +
> +    VCardApplet * applet = vcard_new_applet(apdu_cb, reset_cb, (const unsigned char *) CAPCSC_APPLET, strlen(CAPCSC_APPLET));
> +    if (! applet)
> +        return;
> +
> +    VCard * card =  vcard_new((VCardEmul *) r, delete_card_cb);
> +    if (! card)
> +    {
> +        vcard_delete_applet(applet);
> +        vreader_free(reader);
> +        return;
> +    }
> +
> +    vcard_set_type(card, VCARD_DIRECT);
> +    vcard_set_atr_func(card, get_atr_cb);
> +    vcard_add_applet(card, applet);
> +
> +    vreader_insert_card(reader, card);
> +    vreader_free(reader);
> +}
> +
> +static void remove_card(SCardReader *r)
> +{
> +    LONG rc;
> +    memset(r->atr, 0, sizeof(r->atr));
> +    r->atrlen = 0;
> +
> +    rc = SCardDisconnect(r->card, SCARD_LEAVE_CARD);
> +    if (rc != SCARD_S_SUCCESS)
> +        fprintf(stderr, "Non fatal info: failed to disconnect card reader: %s (0x%lX)\n",
> +            pcsc_stringify_error(rc), rc);
> +    r->card_connected = 0;
> +
> +    VReader *reader = vreader_get_reader_by_name(r->name);
> +    if (! reader)
> +        return;
> +
> +    vreader_insert_card(reader, NULL);
> +    vreader_free(reader);
> +}
> +
> +static void process_reader_change(SCardReader *r, SCARD_READERSTATE *s)
> +{
> +    if (s->dwEventState & SCARD_STATE_PRESENT)
> +        insert_card(r, s);
> +    else if (s->dwEventState & SCARD_STATE_EMPTY)
> +        remove_card(r);
> +    else
> +        fprintf(stderr, "Unexpected card state change from %lx to %lx:\n", r->state, s->dwEventState);
> +
> +    r->state = s->dwEventState & ~SCARD_STATE_CHANGED;
> +}
> +
> +static void process_requests(SCardReader *r)
> +{
> +    BYTE outbuf[4096];
> +    DWORD outlen = sizeof(outbuf);
> +    LONG rc;
> +    APDUMsg *request = get_request(r);
> +
> +    if (request)
> +    {
> +        rc = send_receive(r, request->data, request->len, outbuf, &outlen);
> +        push_response(r, outbuf, outlen, rc, request->sender);
> +        free_msg(request);
> +    }
> +}
> +
> +/*
> + * This thread looks for card and reader insertions and puts events on the
> + * event queue.  PCSC is also not thread safe, so we relay requests for
> + * the smart card stack through here as well.
> + */
> +static void event_thread(void *arg)
> +{
> +    PCSCContext *pc = (PCSCContext *) arg;
> +    DWORD reader_count = 0;
> +    SCARD_READERSTATE *reader_states = NULL;
> +    LONG rc;
> +
> +    scan_for_readers(pc);
> +
> +    do {
> +        int i;
> +        DWORD timeout = INFINITE;
> +
> +        g_mutex_lock(&pc->lock);
> +        if (pc->readers_changed)
> +        {
> +            prepare_reader_states(pc, &reader_states, &reader_count);
> +            timeout = 0;
> +        }
> +        else if (reader_count > 1)
> +            timeout = CAPCSC_POLL_TIME;
> +
> +        pc->readers_changed = 0;
> +        g_mutex_unlock(&pc->lock);
> +
> +        rc = SCardGetStatusChange(pc->context, timeout, reader_states, reader_count);
> +
> +        /* If we have a new reader, or an unknown reader, rescan and go back and do it again */
> +        if ( (rc == SCARD_S_SUCCESS && (reader_states[reader_count - 1].dwEventState & SCARD_STATE_CHANGED)) ||
> +             rc == SCARD_E_UNKNOWN_READER )
> +        {
> +            scan_for_readers(pc);
> +            continue;
> +        }
> +
> +        if (rc == SCARD_E_TIMEOUT)
> +        {
> +            g_mutex_lock(&pc->lock);
> +            /* TODO - get an EINTR to speed this up? */
> +            for (i = 0; i < reader_count - 1; i++)
> +                process_requests(&pc->readers[i]);
> +            g_mutex_unlock(&pc->lock);
> +
> +            continue;
> +        }
> +
> +        if (rc != SCARD_S_SUCCESS)
> +        {
> +            fprintf(stderr, "Unexpected SCardGetStatusChange ret %lx(%s)\n", rc, pcsc_stringify_error(rc));
> +            continue;
> +        }
> +
> +        g_mutex_lock(&pc->lock);
> +        for (i = 0; i < reader_count; i++)
> +        {
> +            if (reader_states[i].dwEventState & SCARD_STATE_CHANGED)
> +            {
> +                process_reader_change(&pc->readers[i], &reader_states[i]);
> +                pc->readers_changed++;
> +            }
> +
> +        }
> +        g_mutex_unlock(&pc->lock);
> +
> +    } while (1);
> +}
> +
> +/*
> + * We poll the PC/SC interface, looking for device changes
> + */
> +static int new_event_thread(PCSCContext *pc)
> +{
> +    pc->thread = PR_CreateThread(PR_SYSTEM_THREAD, event_thread,
> +                     pc, PR_PRIORITY_NORMAL, PR_GLOBAL_THREAD,
> +                     PR_UNJOINABLE_THREAD, 0);

I would rather use GThread rather than depending on NSPR here (also
because you use GMutex and friends).

> +    return pc->thread == NULL;
> +}
> +
> +
> +static PCSCContext context;
> +
> +int capcsc_init(void)
> +{
> +    g_mutex_init(&context.lock);
> +
> +    if (init_pcsc(&context))
> +        return -1;
> +
> +    if (new_event_thread(&context))
> +        return -1;
> +
> +    return 0;
> +}
> diff --git a/libcacard/capcsc.h b/libcacard/capcsc.h
> new file mode 100644
> index 0000000..4e292cf
> --- /dev/null
> +++ b/libcacard/capcsc.h
> @@ -0,0 +1,16 @@
> +/*
> + * 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.
> + */
> +#ifndef CAPCSC_H
> +#define CAPCSC_H 1
> +
> +#define CAPCSC_POLL_TIME            50      /* ms  - Time we will poll for card change when a reader is connected */
> +#define CAPCSC_MAX_READERS          16      /* Technically, -1, as we leave a space to find new readers */
> +
> +#define CAPCSC_APPLET               "CAPCSC APPLET"
> +
> +int capcsc_init(void);
> +
> +
> +#endif
> diff --git a/libcacard/libcacard.syms b/libcacard/libcacard.syms
> index 1697515..0e44dc0 100644
> --- a/libcacard/libcacard.syms
> +++ b/libcacard/libcacard.syms
> @@ -1,5 +1,6 @@
>  cac_card_init
>  cac_is_cac_card
> +capcsc_init
>  vcard_add_applet
>  vcard_apdu_delete
>  vcard_apdu_new
> diff --git a/libcacard/vcard.c b/libcacard/vcard.c
> index d140a8e..4a1d91e 100644
> --- a/libcacard/vcard.c
> +++ b/libcacard/vcard.c
> @@ -95,7 +95,7 @@ vcard_reset(VCard *card, VCardPower power)
>  VCardApplet *
>  vcard_new_applet(VCardProcessAPDU applet_process_function,
>                   VCardResetApplet applet_reset_function,
> -                 unsigned char *aid, int aid_len)
> +                 const unsigned char *aid, int aid_len)
>  {
>      VCardApplet *applet;
>
> diff --git a/libcacard/vcard.h b/libcacard/vcard.h
> index 47dc703..c16b944 100644
> --- a/libcacard/vcard.h
> +++ b/libcacard/vcard.h
> @@ -30,7 +30,7 @@ void vcard_reset(VCard *card, VCardPower power);
>   */
>  VCardApplet *vcard_new_applet(VCardProcessAPDU applet_process_function,
>                                VCardResetApplet applet_reset_function,
> -                              unsigned char *aid, int aid_len);
> +                              const unsigned char *aid, int aid_len);
>
>  /*
>   * destructor for a VCardApplet
> diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
> index 950edee..18e6871 100644
> --- a/libcacard/vcard_emul_nss.c
> +++ b/libcacard/vcard_emul_nss.c
> @@ -25,6 +25,7 @@
>  #include <prthread.h>
>  #include <secerr.h>
>
> +#include "config-host.h"
>  #include "qemu-common.h"
>
>  #include "vcard.h"
> @@ -34,6 +35,9 @@
>  #include "vevent.h"
>
>  #include "libcacard/vcardt_internal.h"
> +#if defined(CONFIG_SMARTCARD_PCSC)
> +#include "capcsc.h"
> +#endif
>
>
>  typedef enum {
> @@ -892,6 +896,22 @@ vcard_emul_init(const VCardEmulOptions *options)
>          options = &default_options;
>      }
>
> +#if defined(CONFIG_SMARTCARD_PCSC)
> +    if (options->use_hw && options->hw_card_type == VCARD_EMUL_PASSTHRU) {
> +        if (options->vreader_count > 0) {
> +            fprintf(stderr, "Error: you cannot use a soft card and a passthru card simultaneously.\n");
> +            return VCARD_EMUL_FAIL;
> +        }
> +
> +        if (capcsc_init()) {

Why is this needed? Would it be possible to initialize implicitely
when using passthrough?

> +            fprintf(stderr, "Error initializing PCSC interface.\n");
> +            return VCARD_EMUL_FAIL;
> +        }
> +
> +        return VCARD_EMUL_OK;
> +    }
> +#endif
> +
>      /* first initialize NSS */
>      if (options->nss_db) {
>          rv = NSS_Init(options->nss_db);
> @@ -1270,5 +1290,11 @@ vcard_emul_usage(void)
>  "hw_type, and parameters of hw_param.\n"
>  "\n"
>  "If more one or more soft= parameters are specified, these readers will be\n"
> -"presented to the guest\n");
> +"presented to the guest\n"
> +#if defined(CONFIG_SMARTCARD_PCSC)
> +"\n"
> +"If a hw_type of PASSTHRU is given, a connection will be made to the hardware\n"
> +"using libpcscslite.  Note that in that case, no soft cards are permitted.\n"
> +#endif
> +);
>  }
> diff --git a/libcacard/vcard_emul_type.c b/libcacard/vcard_emul_type.c
> index 59a1458..e8f6a4c 100644
> --- a/libcacard/vcard_emul_type.c
> +++ b/libcacard/vcard_emul_type.c
> @@ -9,6 +9,7 @@
>   */
>
>  #include <strings.h>
> +#include "config-host.h"
>  #include "vcardt.h"
>  #include "vcard_emul_type.h"
>  #include "cac.h"
> @@ -48,7 +49,7 @@ VCardEmulType vcard_emul_type_from_string(const char *type_string)
>       if (strcasecmp(type_string, "CAC") == 0) {
>          return VCARD_EMUL_CAC;
>       }
> -#ifdef USE_PASSTHRU
> +#ifdef CONFIG_SMARTCARD_PCSC
>       if (strcasecmp(type_string, "PASSTHRU") == 0) {
>          return VCARD_EMUL_PASSTHRU;
>       }
> diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
> index fa6041d..8573f50 100644
> --- a/libcacard/vscclient.c
> +++ b/libcacard/vscclient.c
> @@ -41,14 +41,8 @@ print_byte_array(
>
>  static void
>  print_usage(void) {
> -    printf("vscclient [-c <certname> .. -e <emul_args> -d <level>%s] "
> -            "<host> <port>\n",
> -#ifdef USE_PASSTHRU
> -    " -p");
> -    printf(" -p use passthrough mode\n");
> -#else
> -   "");
> -#endif
> +    printf("vscclient [-c <certname> .. -e <emul_args> -d <level>] "
> +            "<host> <port>\n");
>      vcard_emul_usage();
>  }
>
> @@ -673,7 +667,7 @@ main(
>      }
>  #endif
>
> -    while ((c = getopt(argc, argv, "c:e:pd:")) != -1) {
> +    while ((c = getopt(argc, argv, "c:e:d:")) != -1) {
>          switch (c) {
>          case 'c':
>              if (cert_count >= MAX_CERTS) {
> @@ -685,10 +679,6 @@ main(
>          case 'e':
>              emul_args = optarg;
>              break;
> -        case 'p':
> -            print_usage();
> -            exit(4);
> -            break;
>          case 'd':
>              verbose = get_id_from_string(optarg, 1);
>              break;
> --
> 1.7.10.4
>
>

On 01/19/2015 11:16 AM, Marc-André Lureau wrote:
> Hey
> 
> Thanks for proposing passthrough mode. It can be useful in some cases,
> although it's good to mention it's not safe to concurrently share a
> card with other applications since no locking is provided (and I don't
> know if and how it's possible atm).

Actually, I'm beginning to think that this will work cleanly.  pcscd
works to keep clients segregated, and the libnss path, through coolkey,
eventually resolves into pcsc calls as well.  So I don't think this
pathway is fundamentally different than what we're already doing.

With that said, I have found that the spice server does not relay all
smartcard events; there are some events (e.g. a reset) that are short
circuited in the spice server.  That should be fixable without too much
effort, though, I think.

> 
> You should fix your patch to pass qemu scripts/checkpatch.pl (too many
> errors to report here)

*blush*  Yup.

> 
> Some review notes follow.
> 
> On Mon, Jan 19, 2015 at 4:00 PM, Jeremy White <jwhite@codeweavers.com> wrote:
>> This enables a passthru mode in the spice client, where we relay
>> apdus from the client to the host, rather than passing through nss
>> and emulating a card.
> 
> Could you explain how you handle multiple readers, and what is the
> sender PID used for?

From the perspective of this library as a separate entity, which is the
approach I was considering as I designed it, multiple card readers
should work well.  I am not sure if they will flow through the spice
stack and work well; I will need to buy a second reader to test that.
My hunch is that they will not.

As for the PID, that, and the whole queue system, was built because I
had the impression that pcsc-lite was not necessarily thread safe.
(Older versions had a configure check for that option).  It appears as
though that may be an unfounded fear.  I'll experiment with building a
version that eliminates this and see if it works.

> Although I am not very picky about splitting things, I would say you
> could easily split the configure part first.

I'll trust your instincts around the preferences in this project.

>>
>> +# check for pcsclite for smartcard passthru support
> 
> (perhaps it's good to keep in mind that this code should be fairly
> easy to port to winscard, perhaps a TODO would be worth here?)

Sure.

> 
>> +if test "$smartcard_pcsc" != "no"; then
>> +  cat > $TMPC << EOF
>> +#include <winscard.h>
>> +int main(void) { SCardEstablishContext(0, 0, 0, 0); return 0; }
>> +EOF
>> +    # FIXME: do not include $glib_* in here
>> +    pcsc_libs="$($pkg_config --libs libpcsclite 2>/dev/null) $glib_libs"
>> +    pcsc_cflags="$($pkg_config --cflags libpcsclite 2>/dev/null) $glib_cflags"
> 
> It seems you can remove the $glib here.

Hmm.  I tried that, and failed, in a way that seemed identical to the
check for libnss.  I'll try again.

> 
> Why not use GQueue or GAsyncQueue?

No good reason; I think when I initially wrote it, I hadn't realized
that glib was going to be a hard dependency.  (I'll also clean up the
malloc() call).

>> +static int new_event_thread(PCSCContext *pc)
>> +{
>> +    pc->thread = PR_CreateThread(PR_SYSTEM_THREAD, event_thread,
>> +                     pc, PR_PRIORITY_NORMAL, PR_GLOBAL_THREAD,
>> +                     PR_UNJOINABLE_THREAD, 0);
> 
> I would rather use GThread rather than depending on NSPR here (also
> because you use GMutex and friends).

Sure.  My instinct was to use the same mechanism already in use by this
library, which is the PR_CreateThread, but I think you're right.

>> +#if defined(CONFIG_SMARTCARD_PCSC)
>> +    if (options->use_hw && options->hw_card_type == VCARD_EMUL_PASSTHRU) {
>> +        if (options->vreader_count > 0) {
>> +            fprintf(stderr, "Error: you cannot use a soft card and a passthru card simultaneously.\n");
>> +            return VCARD_EMUL_FAIL;
>> +        }
>> +
>> +        if (capcsc_init()) {
> 
> Why is this needed? Would it be possible to initialize implicitely
> when using passthrough?

I'm not sure I understand the benefit to that.  In order to detect
reader add/remove, we need to have a monitoring thread.  We need to
start that thread somewhere.

I'll do the fairly obvious work I should have done and resubmit.

Thanks for the review!

Cheers,

Jeremy