Re: [Spice-devel] [xf86-video-qxl v6] Enable smartcard support for XSpice.

2015-01-08 Thread Uri Lublin

Hi Jeremy,

Ack.

Please see two minor comments below.

Thanks,
Uri.


On 12/16/2014 12:24 AM, Jeremy White wrote:

This is done by creating a Unix domain socket to which smartcard
messages are transferred, using the vscard protocol.

A further system library, spiceccid, is used to provide an interface into
pcsc-lite, specifically the pcsc-lite daemon, so that regular Unix applications
can access the passed through smartcard information.

Signed-off-by: Jeremy Whitejwh...@codeweavers.com
---
  configure.ac   |   25 ++
  examples/spiceqxl.xorg.conf.example|3 +
  src/Makefile.am|3 +-
  src/qxl.h  |2 +
  src/qxl_driver.c   |   14 +-
  src/spiceccid/Makefile.am  |   29 ++
  src/spiceccid/spice.pcsc.conf.template |7 +
  src/spiceccid/spiceccid.c  |  477 
  src/spiceqxl_smartcard.c   |  193 +
  src/spiceqxl_smartcard.h   |   31 +++
  10 files changed, 782 insertions(+), 2 deletions(-)
  create mode 100644 src/spiceccid/Makefile.am
  create mode 100644 src/spiceccid/spice.pcsc.conf.template
  create mode 100644 src/spiceccid/spiceccid.c
  create mode 100644 src/spiceqxl_smartcard.c
  create mode 100644 src/spiceqxl_smartcard.h

+++ b/src/spiceccid/spiceccid.c
@@ -0,0 +1,477 @@
+/*
+ * Copyright (C) 2014 CodeWeavers, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
THE
+ * SOFTWARE.
+ *
+ * Authors:
+ *Jeremy Whitejwh...@codeweavers.com
+ */
+
+/*
+  Chip/Smart Card Interface Devices driver for Spice
+
+This driver is built to interface to pcsc-lite as a serial smartcard
+  device.
+It translates the IFD (Interface device) ABI into the Spice protocol.
+*/
+



+
+static void process_atr(smartcard_ccid_t *ccid, VSCMsgHeader *h, char *data)
+{
+ccid-atr_len = h-length;
+if (h-length  0  h-length  sizeof(ccid-atr)) {


h-length is unsigned.
1. Why is there a need to check that h-length  0 ?
2. What happens if h-length == 0 ?


+fprintf(stderr, Supplied ATR of length %d exceeds %d maximum\n,
+h-length, sizeof(ccid-atr));
+send_reply(ccid, VSC_GENERAL_ERROR);
+return;
+}
+
+memset(ccid-atr, 0, sizeof(ccid-atr));
+memcpy(ccid-atr, data, ccid-atr_len);
+
+send_reply(ccid, VSC_SUCCESS);
+}
+
+static void process_apdu(smartcard_ccid_t *ccid, VSCMsgHeader *h, char *data)
+{
+if (ccid-state  STATE_READER_ADDED)
+push_apdu(ccid, data, h-length);


Maybe add: else { fprintf(stderr, warning ... }


+}
+
+static void process_card_remove(smartcard_ccid_t *ccid, VSCMsgHeader *h)
+{
+ccid-atr_len = 0;
+memset(ccid-atr, 0, sizeof(ccid-atr));
+send_reply(ccid, VSC_SUCCESS);
+}
+

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [xf86-video-qxl v6] Enable smartcard support for XSpice.

2015-01-08 Thread Jeremy White

On 01/08/2015 03:53 AM, Uri Lublin wrote:

Hi Jeremy,

Ack.


Thanks.



h-length is unsigned.
1. Why is there a need to check that h-length  0 ?


There is no need, changed.


2. What happens if h-length == 0 ?


Note that I believe that could only occur in the case of a malicious or 
buggy packet.  By inspection, I believe the code will all function 
correctly; passing a 0 length atr along.



+static void process_apdu(smartcard_ccid_t *ccid, VSCMsgHeader *h,
char *data)
+{
+if (ccid-state  STATE_READER_ADDED)
+push_apdu(ccid, data, h-length);


Maybe add: else { fprintf(stderr, warning ... }


Sure.

Cheers,

Jeremy
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [xf86-video-qxl v6] Enable smartcard support for XSpice.

2015-01-08 Thread Alon Levy
On 01/08/2015 04:10 PM, Jeremy White wrote:
 On 01/08/2015 03:53 AM, Uri Lublin wrote:
 Hi Jeremy,

 Ack.
 
 Thanks.
 

 h-length is unsigned.
 1. Why is there a need to check that h-length  0 ?
 
 There is no need, changed.
 
 2. What happens if h-length == 0 ?
 
 Note that I believe that could only occur in the case of a malicious or
 buggy packet.  By inspection, I believe the code will all function
 correctly; passing a 0 length atr along.

Is this to the card (hardware) or back? if to, can't this be used by a
driver to wakeup a card (i.e. dropping it will have adverse affects)?

 
 +static void process_apdu(smartcard_ccid_t *ccid, VSCMsgHeader *h,
 char *data)
 +{
 +if (ccid-state  STATE_READER_ADDED)
 +push_apdu(ccid, data, h-length);

 Maybe add: else { fprintf(stderr, warning ... }
 
 Sure.
 
 Cheers,
 
 Jeremy
 ___
 Spice-devel mailing list
 Spice-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/spice-devel

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [xf86-video-qxl v6] Enable smartcard support for XSpice.

2015-01-08 Thread Jeremy White

2. What happens if h-length == 0 ?


Note that I believe that could only occur in the case of a malicious or
buggy packet.  By inspection, I believe the code will all function
correctly; passing a 0 length atr along.


Is this to the card (hardware) or back? if to, can't this be used by a
driver to wakeup a card (i.e. dropping it will have adverse affects)?


I believe that the VSC_ATR is sent at only one place, from the gtk 
client to the remote side  (I believe it's both for power on and reset). 
 But my inspection suggests the code path (through vreader_power_on) 
always retrieves the atr before sending it.  So I'm pretty sure that the 
atr is always non zero length.


Cheers,

Jeremy
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [xf86-video-qxl v6] Enable smartcard support for XSpice.

2014-12-15 Thread Jeremy White
This is done by creating a Unix domain socket to which smartcard
messages are transferred, using the vscard protocol.

A further system library, spiceccid, is used to provide an interface into
pcsc-lite, specifically the pcsc-lite daemon, so that regular Unix applications
can access the passed through smartcard information.

Signed-off-by: Jeremy White jwh...@codeweavers.com
---
 configure.ac   |   25 ++
 examples/spiceqxl.xorg.conf.example|3 +
 src/Makefile.am|3 +-
 src/qxl.h  |2 +
 src/qxl_driver.c   |   14 +-
 src/spiceccid/Makefile.am  |   29 ++
 src/spiceccid/spice.pcsc.conf.template |7 +
 src/spiceccid/spiceccid.c  |  477 
 src/spiceqxl_smartcard.c   |  193 +
 src/spiceqxl_smartcard.h   |   31 +++
 10 files changed, 782 insertions(+), 2 deletions(-)
 create mode 100644 src/spiceccid/Makefile.am
 create mode 100644 src/spiceccid/spice.pcsc.conf.template
 create mode 100644 src/spiceccid/spiceccid.c
 create mode 100644 src/spiceqxl_smartcard.c
 create mode 100644 src/spiceqxl_smartcard.h

diff --git a/configure.ac b/configure.ac
index 14e0597..27b1cc7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -137,8 +137,31 @@ if test x$enable_xspice = xyes; then
 else
 enable_xspice=no
 fi
+
+AC_ARG_ENABLE([ccid],
+[AS_HELP_STRING([--enable-ccid],
+[Build the spiceccid SmartCard driver (default is no)])],
+[enable_ccid=$enableval],
+[enable_ccid=no])
+AC_ARG_WITH(ccid-module-dir,
+[AS_HELP_STRING([--with-ccid-module-dir=DIR ],
+[Specify the install path for spiceccid driver (default is 
$libdir/pcsc/drivers/serial)])],
+[ cciddir=$withval ],
+[ cciddir=$libdir/pcsc/drivers/serial ])
+AC_SUBST(cciddir)
+if test x$enable_ccid != xno; then
+PKG_CHECK_MODULES(LIBPCSCLITE, [libpcsclite])
+PKG_CHECK_MODULES(LIBCACARD, [libcacard])
+
+if test x$enable_xspice = xno; then
+AC_MSG_ERROR([Building with ccid requires xspice, but xspice is not 
enabled])
+fi
+fi
+
+
 AM_CONDITIONAL(BUILD_XSPICE, test x$enable_xspice = xyes)
 AM_CONDITIONAL(BUILD_QXL, test x$enable_qxl = xyes)
+AM_CONDITIONAL(BUILD_SPICECCID, test x$enable_ccid = xyes)
 
 AC_ARG_ENABLE([udev],
AS_HELP_STRING([--disable-udev], [Disable libudev support 
[default=auto]]),
@@ -168,6 +191,7 @@ fi
 AC_CONFIG_FILES([
 Makefile
 src/Makefile
+src/spiceccid/Makefile
 src/uxa/Makefile
 scripts/Makefile
 examples/Makefile
@@ -187,4 +211,5 @@ echo 
 KMS:  ${DRM_MODE}
 Build qxl:${enable_qxl}
 Build xspice: ${enable_xspice}
+Build spiceccid:  ${enable_ccid}
 
diff --git a/examples/spiceqxl.xorg.conf.example 
b/examples/spiceqxl.xorg.conf.example
index 597a5bd..d15f7f2 100644
--- a/examples/spiceqxl.xorg.conf.example
+++ b/examples/spiceqxl.xorg.conf.example
@@ -143,6 +143,9 @@ Section Device
 #  to the client.   Default is no mixing.
 #Option SpicePlaybackFIFODir  /tmp/
 
+# A unix domain name for a unix domain socket
+#  to communicate with a spiceccid smartcard driver
+#Option SpiceSmartCardFile  /tmp/spice.pcsc.comm
 EndSection
 
 Section InputDevice
diff --git a/src/Makefile.am b/src/Makefile.am
index bf50ae1..6c72bbd 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -25,7 +25,7 @@
 # _ladir passes a dummy rpath to libtool so the thing will actually link
 # TODO: -nostdlib/-Bstatic/-lgcc platform magic, not installing the .a, etc.
 
-SUBDIRS=uxa
+SUBDIRS=uxa spiceccid
 
 AM_CFLAGS = $(SPICE_PROTOCOL_CFLAGS) $(XORG_CFLAGS) $(PCIACCESS_CFLAGS) 
$(CWARNFLAGS) $(DRM_CFLAGS) @LIBUDEV_CFLAGS@
 
@@ -96,6 +96,7 @@ spiceqxl_drv_la_SOURCES = \
spiceqxl_uinput.c   \
spiceqxl_uinput.h   \
spiceqxl_audio.c\
+   spiceqxl_smartcard.c\
spiceqxl_audio.h\
spiceqxl_inputs.c   \
spiceqxl_inputs.h   \
diff --git a/src/qxl.h b/src/qxl.h
index 603faca..54995cf 100644
--- a/src/qxl.h
+++ b/src/qxl.h
@@ -157,6 +157,7 @@ enum {
 OPTION_FRAME_BUFFER_SIZE,
 OPTION_SURFACE_BUFFER_SIZE,
 OPTION_COMMAND_BUFFER_SIZE,
+OPTION_SPICE_SMARTCARD_FILE,
 #endif
 OPTION_COUNT,
 };
@@ -352,6 +353,7 @@ struct _qxl_screen_t
 
 char playback_fifo_dir[PATH_MAX];
 void *playback_opaque;
+char smartcard_file[PATH_MAX];
 #endif /* XSPICE */
 
 uint32_t deferred_fps;
diff --git a/src/qxl_driver.c b/src/qxl_driver.c
index 165f468..9ad8921 100644
--- a/src/qxl_driver.c
+++ b/src/qxl_driver.c
@@ -55,6 +55,7 @@