Re: [Spice-devel] [xf86-video-qxl v6] Enable smartcard support for XSpice.
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.
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.
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.
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.
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 @@