On Fri, Oct 09, 2015 at 04:43:37PM +0200, Marc-André Lureau wrote:
> Keeping the author of the patch in CC
>
> On Fri, Oct 9, 2015 at 4:34 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> > On Thu, Oct 08, 2015 at 05:40:33PM +0200, marcandre.lureau@xxxxxxxxxx wrote:
> >> From: Jeremy White <jwhite@xxxxxxxxxxxxxxx>
> >>
> >> This uses libpcsclite to provide direct communication with a smartcard.
> >>
> >> Signed-off-by: Jeremy White <jwhite@xxxxxxxxxxxxxxx>
> >> Reviewed-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> >> ---
> >> Makefile.am | 9 +-
> >> configure.ac | 1 +
> >> src/capcsc.c | 505 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >> src/capcsc.h | 18 ++
> >> src/card_7816.c | 2 +-
> >> src/card_7816.h | 4 +-
> >> src/libcacard.syms | 2 +
> >> src/vcard.c | 2 +-
> >> src/vcard.h | 2 +-
> >> src/vcard_emul_nss.c | 11 +-
> >> src/vcard_emul_type.c | 1 +
> >> 11 files changed, 549 insertions(+), 8 deletions(-)
> >> create mode 100644 src/capcsc.c
> >> create mode 100644 src/capcsc.h
> >>
> >> diff --git a/Makefile.am b/Makefile.am
> >> index 7939079..b1e257f 100644
> >> --- a/Makefile.am
> >> +++ b/Makefile.am
> >> @@ -15,9 +15,14 @@ libcacard_la_SOURCES = \
> >> src/vreader.c \
> >> $(NULL)
> >>
> >> +if ENABLE_PCSC
> >> +libcacard_la_SOURCES += src/capcsc.c
> >> +endif
> >> +
> >> libcacard_includedir = $(includedir)/cacard
> >> libcacard_include_HEADERS = \
> >> src/cac.h \
> >> + src/capcsc.h \
> >> src/card_7816.h \
> >> src/card_7816t.h \
> >> src/eventt.h \
> >> @@ -32,7 +37,7 @@ libcacard_include_HEADERS = \
> >> src/vscard_common.h \
> >> $(NULL)
> >>
> >> -libcacard_la_LIBADD = $(CACARD_LIBS)
> >> +libcacard_la_LIBADD = $(CACARD_LIBS) $(PCSC_LIBS)
> >> libcacard_la_LDFLAGS = \
> >> -export-symbols $(srcdir)/src/libcacard.syms \
> >> -no-undefined \
> >> @@ -56,7 +61,7 @@ if OS_WIN32
> >> vscclient_CFLAGS += -D__USE_MINGW_ANSI_STDIO=1
> >> endif
> >>
> >> -AM_CPPFLAGS = $(CACARD_CFLAGS) $(WARN_CFLAGS)
> >> +AM_CPPFLAGS = $(CACARD_CFLAGS) $(PCSC_CFLAGS) $(WARN_CFLAGS)
> >> EXTRA_DIST = \
> >> NEWS \
> >> README.md \
> >> diff --git a/configure.ac b/configure.ac
> >> index b878526..6a0a73d 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -49,6 +49,7 @@ if test "x$enable_pcsc" != "xno"; then
> >> fi
> >> if test "x$have_pcsc" = "xyes"; then
> >> enable_pcsc=yes
> >> + AC_DEFINE([ENABLE_PCSC], 1, [pcsc support])
> >> fi
> >> fi
> >
> > This could go in the patch adding the configure check.
>
> ok
>
> >
> >> AM_CONDITIONAL(ENABLE_PCSC, test "x$enable_pcsc" = "xyes")
> >> diff --git a/src/capcsc.c b/src/capcsc.c
> >> new file mode 100644
> >> index 0000000..3b68ee3
> >> --- /dev/null
> >> +++ b/src/capcsc.c
> >> @@ -0,0 +1,505 @@
> >> +/*
> >> + * 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.
> >> + */
> >> +
> >> +#include "glib-compat.h"
> >> +#include <string.h>
> >> +#include <stdio.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 {
> >> + PCSCContext *context;
> >> + int index;
> >> + char *name;
> >> + DWORD protocol;
> >> + DWORD state;
> >> + SCARDHANDLE card;
> >> + BYTE atr[MAX_ATR_SIZE];
> >> + DWORD atrlen;
> >> + int card_connected;
> >> + unsigned long request_count;
> >> +} SCardReader;
> >> +
> >> +typedef struct _PCSCContext {
> >> + SCARDCONTEXT context;
> >> + SCardReader readers[CAPCSC_MAX_READERS];
> >> + int reader_count;
> >> + int readers_changed;
> >> + GThread *thread;
> >> + CompatGMutex lock;
> >> +} PCSCContext;
> >> +
> >> +
> >> +static void delete_reader(PCSCContext *pc, int i)
> >> +{
> >> + SCardReader *r = &pc->readers[i];
> >> + g_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);
> >> + }
> >
> > Since libcacard is using glib, I think GArray/GPtrArray might be usable
> > to avoid manual memory juggling. There are few places where memory is
> > moved around though, so I'm fine if this stays this way.
> >
> >
> >> +
> >> + 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 = g_strdup(name);
> >> +
> >> + 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;
> >> + }
> >
> > Why not int matches[..] = { 0, }; or a memset?
>
> make sense
>
> >
> >> * destructor for VCardResponse.
> >> diff --git a/src/libcacard.syms b/src/libcacard.syms
> >> index 2f9d423..1b78f8d 100644
> >> --- a/src/libcacard.syms
> >> +++ b/src/libcacard.syms
> >> @@ -1,4 +1,5 @@
> >> cac_card_init
> >> +capcsc_init
> >> vcard_add_applet
> >> vcard_apdu_delete
> >> vcard_apdu_new
> >
> > I don't think you need to export capcsc_init as it's called by
> > vcard_emul_init() when needed?
> >
> >> @@ -40,6 +41,7 @@ vcard_response_new
> >> vcard_response_new_bytes
> >> vcard_response_new_data
> >> vcard_response_new_status_bytes
> >> +vcard_response_set_status_bytes
> >> vcard_select_applet
> >> vcard_set_applet_private
> >> vcard_set_atr_func
> >> diff --git a/src/vcard.c b/src/vcard.c
> >> index 667f30a..2edf1d0 100644
> >> --- a/src/vcard.c
> >> +++ b/src/vcard.c
> >> @@ -97,7 +97,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/src/vcard.h b/src/vcard.h
> >> index 16a23b5..1364dfb 100644
> >> --- a/src/vcard.h
> >> +++ b/src/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/src/vcard_emul_nss.c b/src/vcard_emul_nss.c
> >> index 7ffa0b4..d046ea9 100644
> >> --- a/src/vcard_emul_nss.c
> >> +++ b/src/vcard_emul_nss.c
> >> @@ -9,6 +9,7 @@
> >> * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> >> * See the COPYING file in the top-level directory.
> >> */
> >> +#include "config.h"
> >>
> >> /*
> >> * NSS headers
> >> @@ -1253,7 +1254,12 @@ vcard_emul_options(const char *args)
> >> opts->hw_card_type = VCARD_EMUL_CAC;
> >> opts->use_hw = PR_TRUE;
> >> args = find_blank(args + 7);
> >> - /* nssemul */
> >> +#if defined(ENABLE_PCSC)
> >> + } else if (strncmp(args, "passthru", 8) == 0) {
> >> + opts->hw_card_type = VCARD_EMUL_PASSTHRU;
> >> + opts->use_hw = PR_TRUE;
> >> + args = find_blank(args + 8);
> >> +#endif
> >> } else {
> >> fprintf(stderr, "Error: Unknown smartcard specification.\n");
> >> return NULL;
> >> @@ -1273,6 +1279,9 @@ vcard_emul_usage(void)
> >> " hw_type={card_type_to_emulate} (default CAC)\n"
> >> " hw_param={param_for_card} (default \"\")\n"
> >> " nssemul (alias for use_hw=yes, hw_type=CAC)\n"
> >> +#if defined(ENABLE_PCSC)
> >> +" passthru (alias for use_hw=yes, hw_type=PASSTHRU)\n"
> >> +#endif
> >> " soft=({slot_name},{vreader_name},{card_type_to_emulate},{params_for_card},\n"
> >> " {cert1},{cert2},{cert3} (default none)\n"
> >> "\n"
> >
> > This is the passthru alias mentioned in another commit log.
> >
> >> diff --git a/src/vcard_emul_type.c b/src/vcard_emul_type.c
> >> index 04e8d99..385e121 100644
> >> --- a/src/vcard_emul_type.c
> >> +++ b/src/vcard_emul_type.c
> >> @@ -7,6 +7,7 @@
> >> * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> >> * See the COPYING file in the top-level directory.
> >> */
> >> +#include "config.h"
> >>
> >> #include <strings.h>
> >> #include "vcardt.h"
> >
> > Is this hunk required as part of this commit?
>
> for #if defined(ENABLE_PCSC)
Ah, but this is only added in the "Enable support for passthru" commit,
not here.
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel